Bug 2122552

(In reply to Geetika Kapoor from comment #1)
> Could you please have a look and see if this cause any problem on UI
> side?Thanks

The operating system of the VM is exposed on UI, so it can be a problem on UI if the actual OS is not matching its template version.

(In reply to Guohua Ouyang from comment #2)
> (In reply to Geetika Kapoor from comment #1)
> > Could you please have a look and see if this cause any problem on UI
> > side?Thanks
> 
> The operating system of the VM is exposed on UI, so it can be a problem on
> UI if the actual OS is not matching its template version.


Does "Fedora 34 or higher" in the UI match the fedora 35 in the disk image?

Didn't understand what your asking.

Just to clarify the issue is with the label: "name.os.template.kubevirt.io/fedora##", since we are filtering templates/VMs using that label. the description is less important.

Do you consider this a customer issue?

(In reply to Dominik Holler from comment #5)
> Do you consider this a customer issue?

I don't this is a customer issue.

Below are the annotations taking from CNV 4.12, the
    name.os.template.kubevirt.io/fedora35: Fedora 35 or higher
    name.os.template.kubevirt.io/fedora36: Fedora 35 or higher

The 2nd line should be "name.os.template.kubevirt.io/fedora36: Fedora 36 or higher".
To avoid the issue, maybe taking the number out of the annotation would be better, making it like "name.os.template.kubevirt.io/fedora: Fedora 36 or higher".

This line is created to provide information to the UI.
@yzamir how do you prefer this line should look like?

> This line is created to provide information to the UI.

The UI will take the first annotation, so if a template is annotated to be compatible with:
34 or higher, 35 or higher or 36 or higher
the UI will show "34 or higher" for that template.

This is consistent with template validation that also take the first comptible os as minimum value:
https://github.com/kubevirt/common-templates/blob/master/templates/fedora.tpl.yaml#L66

IMHO, if the template is compatible with "fedora36" the annotation can reflect it (if will not affect the minimum (e.g. first) compatible os value):
"name.os.template.kubevirt.io/fedora36: Fedora 36 or higher"
ref: https://github.com/kubevirt/common-templates/blob/master/templates/fedora.tpl.yaml#L34
Maybe we can use "osinfoname" that is defined to be the first compatible os
https://github.com/kubevirt/common-templates/blob/master/generate-templates.yaml#L280
instead of the first annotation ?
e.g. add a new annotation with "osinfoname" ?

Unless "fedora 36" is the first option, the UI will continue to show the first one, in this case "34 or higher"
(method uses "find", in js "find" method will return first instance)
ref: 
https://github.com/kubevirt-ui/kubevirt-plugin/blob/419ecf86454192b7a5e0f912f7ad875dc63ef5d8/src/utils/resources/vm/utils/operation-system/operationSystem.ts#L77
https://github.com/kubevirt-ui/kubevirt-plugin/blob/419ecf86454192b7a5e0f912f7ad875dc63ef5d8/src/utils/resources/vm/utils/operation-system/operationSystem.ts#L56
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find

---------

What is the definition of this annotations, are they still needed ?
Maybe a better annotation/label can be used ?
For example if we want to use it for filtering, a label is better, for example ? "templae.os.compatiblity: fedora34,fedora35,fedora36" ?
  if it's for display purposes maybe the "openshift.io/display-name" and "description" are better ?
Maybe we need a new annotation with "osinfoname" to indicate the best/lowest compatible os for this table ?

(In reply to Yaacov Zamir from comment #8)
> > This line is created to provide information to the UI.
> 
> The UI will take the first annotation, so if a template is annotated to be
> compatible with:
> 34 or higher, 35 or higher or 36 or higher
> the UI will show "34 or higher" for that template.
> 
> This is consistent with template validation that also take the first
> comptible os as minimum value:
> https://github.com/kubevirt/common-templates/blob/master/templates/fedora.
> tpl.yaml#L66
> 
> IMHO, if the template is compatible with "fedora36" the annotation can
> reflect it (if will not affect the minimum (e.g. first) compatible os value):
> "name.os.template.kubevirt.io/fedora36: Fedora 36 or higher"
> ref:
> https://github.com/kubevirt/common-templates/blob/master/templates/fedora.
> tpl.yaml#L34
> Maybe we can use "osinfoname" that is defined to be the first compatible os
> https://github.com/kubevirt/common-templates/blob/master/generate-templates.
> yaml#L280
> instead of the first annotation ?
> e.g. add a new annotation with "osinfoname" ?
> 
> Unless "fedora 36" is the first option, the UI will continue to show the
> first one, in this case "34 or higher"
> (method uses "find", in js "find" method will return first instance)
> ref: 
> https://github.com/kubevirt-ui/kubevirt-plugin/blob/
> 419ecf86454192b7a5e0f912f7ad875dc63ef5d8/src/utils/resources/vm/utils/
> operation-system/operationSystem.ts#L77
> https://github.com/kubevirt-ui/kubevirt-plugin/blob/
> 419ecf86454192b7a5e0f912f7ad875dc63ef5d8/src/utils/resources/vm/utils/
> operation-system/operationSystem.ts#L56
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/Array/find
> 
> ---------
> 
> What is the definition of this annotations, are they still needed ?

We not aware of any other usage than UI.

> Maybe a better annotation/label can be used ?
> For example if we want to use it for filtering, a label is better, for
> example ? "templae.os.compatiblity: fedora34,fedora35,fedora36" ?
>   if it's for display purposes maybe the "openshift.io/display-name" and
> "description" are better ?
> Maybe we need a new annotation with "osinfoname" to indicate the best/lowest
> compatible os for this table ?

We can provide whatever is most comfortable for UI. We can also keep it as it is now.

Do you like to keep it like it is now, or do you prefer to change it?

(In reply to Dominik Holler from comment #9)

> Do you like to keep it like it is now, or do you prefer to change it?

IMHO, keeping things as they are (if it's ok with QE) is best, because it will create the least number of new bugs

but we should take time to go over **all** the annotations,
a. see if they are still in use
b. see if they are still making sense

for example IMHO, this annotation may be refactored out, and the UI can use the name and annotations.description instead,
but this should be done carefully talking to UX and docs to see who may be affected (not as a bug fix)

cc:// @tnisan

(In reply to Yaacov Zamir from comment #10)
> (In reply to Dominik Holler from comment #9)
> 
> > Do you like to keep it like it is now, or do you prefer to change it?
> 
> IMHO, keeping things as they are (if it's ok with QE) is best, because it
> will create the least number of new bugs
> 

I agree, so I will close this bug.

> but we should take time to go over **all** the annotations,
> a. see if they are still in use
> b. see if they are still making sense
> 
> for example IMHO, this annotation may be refactored out, and the UI can use
> the name and annotations.description instead,
> but this should be done carefully talking to UX and docs to see who may be
> affected (not as a bug fix)
> 

From my point of view it is not worth to have this discussion, because we plan to deprecate the templates soon.
If there are any inconveniences about the labels and annotations in the meantime, we can start the discussion.

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 120 days