Bug 2103844
Summary: | VM nic model is empty | ||
---|---|---|---|
Product: | Container Native Virtualization (CNV) | Reporter: | Guohua Ouyang <gouyang> |
Component: | SSP | Assignee: | Karel Šimon <ksimon> |
Status: | CLOSED ERRATA | QA Contact: | Roni Kishner <rkishner> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | 4.11.0 | CC: | acardace, dholler, gkapoor, gouyang, ksimon, yzamir |
Target Milestone: | --- | ||
Target Release: | 4.12.0 | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | kubevirt-ssp-operator-container-v4.12.0-47 | Doc Type: | If docs needed, set a value |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2023-01-24 13:37:41 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Guohua Ouyang
2022-07-05 05:46:14 UTC
Move the bug to SSP as the template's default network has no model 'virtio' specified as well. @gouyang Why this behavior is a bug? Isn't "vm nic model is empty" semantically the same as 'vm nic model is "virtio"' specified in http://kubevirt.io/api-reference/main/definitions.html#_v1_interface? (In reply to Dominik Holler from comment #2) > @gouyang Why this behavior is a bug? Isn't "vm nic model is > empty" semantically the same as 'vm nic model is "virtio"' specified in > http://kubevirt.io/api-reference/main/definitions.html#_v1_interface? "Interface model. One of: e1000, e1000e, ne2k_pci, pcnet, rtl8139, virtio. Defaults to virtio." Since the defaults is virtio, why not add it in the yaml so it can present on as "virtio" instead of "-". I think we need to show "virtio" on UI instead of "-". (In reply to Guohua Ouyang from comment #3) > Since the defaults is virtio, why not add it in the yaml so it can present > on as "virtio" instead of "-". > > I think we need to show "virtio" on UI instead of "-". In which yaml or api result do you expect this change? (In reply to Dominik Holler from comment #4) > (In reply to Guohua Ouyang from comment #3) > > Since the defaults is virtio, why not add it in the yaml so it can present > > on as "virtio" instead of "-". > > > > I think we need to show "virtio" on UI instead of "-". > > In which yaml or api result do you expect this change? Add model to the interfaces like below: interfaces: - masquerade: {} name: default model: virtio (In reply to Guohua Ouyang from comment #5) > (In reply to Dominik Holler from comment #4) > > (In reply to Guohua Ouyang from comment #3) > > > Since the defaults is virtio, why not add it in the yaml so it can present > > > on as "virtio" instead of "-". > > > > > > I think we need to show "virtio" on UI instead of "-". > > > > In which yaml or api result do you expect this change? > > Add model to the interfaces like below: > interfaces: > - masquerade: {} > name: default > model: virtio Do you refer to the template or the VM representation? It's the template. Even if the model would be set in the common templates, user defined templates still might miss it. For this reason I think the UI should show the default value if the model is empty. (In reply to Dominik Holler from comment #8) > Even if the model would be set in the common templates, user defined > templates still might miss it. For this reason I think the UI should show > the default value if the model is empty. The example template `vm-template-example` which is created from YAML, actually have specified the model for default network: interfaces: - masquerade: {} model: virtio name: default If the common templates specify the model, the cloned templates should have it as well. UI is taking value from the template instead of assuming default values, so it's better to specify the model in template. (In reply to Guohua Ouyang from comment #9) > UI is taking value from the template instead of assuming default values, so > it's better to specify the model in template. I see, I just wonder where is the best place to expand the default and why one or the other is better. Even the VMI representation does not expand the default. From my point of view, a reason to not expand it in the UI would be, that the UI would reflect the content of the YAML more precisely. The reason for not expanding in the template would be, that the template would even grow more, but they are too huge already. @acardace @ksimon do you see any problem adding the additional line to the linux templates? > If the common templates specify the model, the cloned templates should have it as well.
What about the templates which were cloned before upcoming change?
The question is if UI will need any additional changes, when we add a default nic model in templates. Otherwise there should be no problems with adding it in linux templates.
Would it be helpful for UI without breaking anything if the nic model would be populated with the default value in the templates? (In reply to Dominik Holler from comment #12) > Would it be helpful for UI without breaking anything if the nic model would > be populated with the default value in the templates? TLDR; a. The UI will not ( should not :-) ) break because of template information b. the UI will show the information if the template will have the information ------- AFAIU the UI just show what the template has, ( the UI does not have a logic that fill in the defaults ) so: if template is omitting value where the desired behavior is use default, the UI will show missing value but will not know the default value. if template is using the value, the UI will show whatever value the template will use. things to consider, the UI don't (shouldn't) know the default values, because it will create duplicate logic between UI and controller and may lead to mismatch when UI think default value is X and the controller use default value Y. I can see this options: a. document that missing nic means use default. b. have some API to fill in default values, and make the UI call this API to fill in some defaults when template is missing a value. c. have specific values in the template Thanks, Yaacov. (In reply to Yaacov Zamir from comment #13) > (In reply to Dominik Holler from comment #12) > > Would it be helpful for UI without breaking anything if the nic model would > > be populated with the default value in the templates? > > TLDR; > a. The UI will not ( should not :-) ) break because of template information > b. the UI will show the information if the template will have the information > > ------- > > AFAIU the UI just show what the template has, ( the UI does not have a logic > that fill in the defaults ) > so: > if template is omitting value where the desired behavior is use default, the > UI will show missing value but will not know the default value. > if template is using the value, the UI will show whatever value the template > will use. > > things to consider, the UI don't (shouldn't) know the default values, > because it will create duplicate logic between UI and controller > and may lead to mismatch when UI think default value is X and the controller > use default value Y. > > I can see this options: > a. document that missing nic means use default. > b. have some API to fill in default values, and make the UI call this API to > fill in some defaults when template is missing a value. We will have such an API in the new instancetypes/preferferences flow. > c. have specific values in the template We will fill the values in the common-templates flow, let us know if something additional is missing. Verified on ssp-operator v4.12.0-49. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (Important: OpenShift Virtualization 4.12.0 Images security update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2023:0408 The needinfo request[s] on this closed bug have been removed as they have been unresolved for 120 days |