The flavor hide_hypervisor_id value can be overridden by the image img_hide_hypervisor_id

[sean mooney]

During the implementation of enabling hypervisor hiding for windows guests
it became apparent that a latent bug exits that allows non privaldges users
to override the policy set by the admin in the flavor by uploading a custom image.

by convention back in the havan/icehouse days we used to allow the flavor to take precendece
over the image if there was a conflcit and log a warning. sometime aound liberty/mitaka we decided
that was a bad user experence for endusers as they did not recive what they asked for and started to convert all confict into a hard error. The only case where we intentionally allow the image to take prescedece over the flavor is hw:mem_page_size where it is allows if an only if the adming has set hw:mem_p[age_size to large or any expcltly. in other words unless the admin has opted in to allowing ther image to take precendece by not setting a value in the flavor or setint it to a value that allows the image to refine the choice we do not support image overriding flavors.

the current code does exactly that by the use of a logical or

 flavor_hide_kvm = strutils.bool_from_string(
                flavor.get('extra_specs', {}).get('hide_hypervisor_id'))
        if (virt_type in ("qemu", "kvm") and
                (image_meta.properties.get('img_hide_hypervisor_id') or
                 flavor_hide_kvm)):

and the new code

hide_hypervisor_id = (strutils.bool_from_string(
                flavor.extra_specs.get('hide_hypervisor_id')) or
            image_meta.properties.get('img_hide_hypervisor_id'))

exibits the same behavior.

in both cases if img_hide_hypervisor_id=true and hide_hypervisor_id=false
hypervior hiding will be enabled.

in this specific case the side-effects of this are safe but it may not be in all
cases of this pattern.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/663365

[Matt Riedemann (mriedem)]

Do you have references to back this up?

> sometime aound liberty/mitaka we decided that was a bad user experence for endusers as they did not recive what they asked for and started to convert all confict into a hard error.

Because this is not documented at all as far as I know but it seems to come up every time a new extra spec with matching image property is added.

[sean mooney (sean-k-mooney)]

i had expected this to be detailed in https://github.com/openstack/nova/blob/master/doc/source/reference/scheduler-hints-vs-flavor-extra-specs.rst

i dont know if this pattern is only used in the nova.virt.hardware module
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1511-L1516
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1530-L1532
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1539-L1540
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1543-L1546

but have a significant number of check that enforce the image cannot exceed limits defined in the
flavor or override values set by the flavor.

there are quantitavie case like the serial port count where we allow an image to requsted fewer
serial ports then the flavor allows and raise an excpetion otherwise
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L171-L211

for the numa_* values we raise an exception if both the image and flavor try to set the value
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1215-L1298

for cpu pinning we are slightly smarter and only raise exception if values confilict.
https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1301-L1378

perhaps its not as clear cut as i have always understood as you are right this does come up every time we add new extra spec but from me expericence we do allow conflcit on boolean flag between teh image and flavor and raise an exception in this event.

if you would like i can try and add a new doc to the referecne section to describe what i belive the convention has been to date to document the tribeal knowladge i perceive to be true and we can discuss if this is actully the policy we want to enforce. if we decide that we want the policy to be we assess on a case by case basis that is fine too.i

by the way the reason i mention liberty is a release or two after we added cpu pinning and numa in juno i remember a case where we change the behavior of one specific conflict to be a hard fail with an exception but i have tried to find it and i cant. we also had a summit sesson on evolving flaovrs and image propeties in vancouver https://etherpad.openstack.org/p/YVR-nova-flavors-and-image-properties where we were discussing https://review.opendev.org/#/c/178628/5/doc/source/flavors_and_images.rst but i think the convention we have used with all the epa/numa stuff predates that.

its unfortunate that that devref was never completed but i think we would benefit form having one in tree.