Filter NAD by namespace when creating new nic raw #265
Conversation
kubevirt-bot
added
do-not-merge/work-in-progress
|
@rawagner Thanks for finding the issue that caused the bug 💯 , As you suggested off line the bug happen when we filter by missing namespace, In this PR I'm trying to filter by namespace on the web-ui repo side. Does that make sense, do we need this, can it break something eles ? |
yaacov
force-pushed
the
filter-nad-by-namespace
branch
from
faf2725
to
e0c5ece
Compare
yaacov
changed the title
kubevirt-bot
removed
the
do-not-merge/work-in-progress
|
@yaacov filtering on web-ui side is great! I do not think it will break anything |
| @@ -85,8 +85,9 @@ const NIC_TYPE_VM = 'nic-type-vm'; | |||
| const NIC_TYPE_CREATE = 'nic-type-create'; | |||
|
|
|||
| export const NicRow = (onChange, onAccept, onCancel) => ({obj: nic}) => { | |||
| const namespace = (nic.vm && nic.vm.metadata.namespace) || (nic.vmTemplate && nic.vmTemplate.metadata.namespace); | |||
There was a problem hiding this comment.
you can use getNamespace from web-ui-components - getNamespace(nic.vm) || getNamespace(nic.vmTemplate)
yaacov
force-pushed
the
filter-nad-by-namespace
branch
from
bb89d7f
to
e0c5ece
Compare
| @@ -85,8 +95,9 @@ const NIC_TYPE_VM = 'nic-type-vm'; | |||
| const NIC_TYPE_CREATE = 'nic-type-create'; | |||
|
|
|||
| export const NicRow = (onChange, onAccept, onCancel) => ({obj: nic}) => { | |||
| const namespace = getNamespace(nic.vm) || getNamespace(nic.vmTemplate); | |||
There was a problem hiding this comment.
getNamespace(nic.vm || nic.vmTemplate)
| @@ -85,8 +95,9 @@ const NIC_TYPE_VM = 'nic-type-vm'; | |||
| const NIC_TYPE_CREATE = 'nic-type-create'; | |||
|
|
|||
| export const NicRow = (onChange, onAccept, onCancel) => ({obj: nic}) => { | |||
| const namespace = getNamespace(nic.vm || nic.vmTemplate); | |||
There was a problem hiding this comment.
@suomiy @rawagner hi, I made a mistake here :-(
nic.vm can be defined ( actually it is a required prop, so it is always defined ) but be without namespace label, in this case:
getNamespace(nic.vm || nic.vmTemplate) will be empty while getNamespace(nic.vm) || getNamespace(nic.vmTemplate) will hold the namespace of the vm template.
I will have to fix that in a new PR :-(
Do you know if we can make vm a none required prop, so if called from a template page it can be undefined ?
Should I just use the longer phrase ?
Help :-)
There was a problem hiding this comment.
or something like this
getNamespace(nic.vmTemplate || nic.vm) // order important (vm might be template vm)or the original approach but I would add some comment so nobody refactors that ;)
Please also add the comment to the props, that vm can be template vm (was not yet created)
There was a problem hiding this comment.
👍 Thanks, the '(nic.vmTemplate || nic.vm)' looks nice to me, I currently do not have a running okd :-( , will make a PR ASAP.
When getting network resource for a nic we want to filter it by namespace.
Currently we filter by namespace on the web-ui-component side [1] but we may want to remove this filter for cases that do not have a namespace.
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1687917
[1] kubevirt/web-ui-components#316