MTV-3177: NADs not showing on Mappings tab edit mode #1853
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1853 +/- ##
==========================================
- Coverage 36.81% 6.75% -30.07%
==========================================
Files 158 1009 +851
Lines 2548 19358 +16810
Branches 599 4006 +3407
==========================================
+ Hits 938 1307 +369
- Misses 1428 18044 +16616
+ Partials 182 7 -175 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| name: `${network.namespace}/${network.name}`, | ||
| }; | ||
| } | ||
| networkMap[network.uid] = { |
There was a problem hiding this comment.
I don't think that this fix is correct since target networks should be filtered by target projects. Let's wait for backend approval for this:
https://issues.redhat.com/browse/MTV-3177?focusedId=28157294&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-28157294
There was a problem hiding this comment.
I see your point, I think it's very weird that the backend allows to set something that wouldn't work and doesn't throw an error about it, than our bug is that we allow to see nad from any namespace on the NetworkMap details page
There was a problem hiding this comment.
I see your point, I think it's very weird that the backend allows to set something that wouldn't work and doesn't throw an error about it,
Doesn't it throw an error? Maybe when trying to migrate VMs with invalid mappings that will result of a VM without a valid NIC then it will fail? Need to test it and see what happens.
than our bug is that we allow to see nad from any namespace on the NetworkMap details page
For NetworkMap CR, there is no option to set target project value. So it's not a bug since no filtering can be applied. The filtering should be applied once you select an existing NetworkMap for a specific plan IIUC...
There was a problem hiding this comment.
Looking into CNV docs, it does specify that the NAD should live in the same namespace as the VM/pod, so I'll revert this change as it's not a bug and keep only the other minor fixes.
And +1 on no sure filter on NM CR as it might or might not have a plan owner
src/plans/details/tabs/Mappings/components/PlanMappingsActionsBar/PlanMappingsActionsBar.tsx
Show resolved
Hide resolved
avivtur
force-pushed
the
missing-nads-from-target-mappings
branch
3 times, most recently
from
8426c96 to
8049af6
Compare
| variant={ButtonVariant.secondary} | ||
| onClick={reset} | ||
| onClick={() => { | ||
| reset(); |
There was a problem hiding this comment.
@avivtur Can you please just fix the prototype for reset param since I'm pretty sure that it will be fixed again as part of eslint fixes...
Signed-off-by: Aviv Turgeman <[email protected]>
avivtur
force-pushed
the
missing-nads-from-target-mappings
branch
from
8049af6 to
62929c0
Compare
|
…v2v#1853) Signed-off-by: Aviv Turgeman <[email protected]>
…v2v#1853) Signed-off-by: Aviv Turgeman <[email protected]>
…v2v#1853) Signed-off-by: Aviv Turgeman <[email protected]>
3 participants
📝 Links
https://issues.redhat.com/browse/MTV-3177
📝 Description
This PR fixes:
🎥 Demo
Before:
mapping-tab-bugs.mp4
After:
mapping-tab-bugs-fixed.mp4
📝 CC://