| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-16 | |||
| 17:02:45 | bauzas | sean-k-mooney: wait, like I said, the bug is about a scheduling decision | |
| 17:02:51 | bauzas | sean-k-mooney: not a boot problem | |
| 17:02:58 | sean-k-mooney | you would use aggregate_instance_extra_specs:hide_hypervior_id in the flavor if you wanted to match on it in that filter | |
| 17:03:04 | bauzas | OVH and a couple of other folks already use this knob | |
| 17:03:13 | bauzas | which was merged a while ago | |
| 17:03:36 | sean-k-mooney | bauzas: yes i know but the current filter check all unamesapced extra specs and assert they are in the metadta | |
| 17:03:40 | bauzas | but here, what's wanted is to trick the scheduler to automatically accept hosts on such things | |
| 17:04:13 | bauzas | sean-k-mooney: I explained this can be achieved without amending with this filter | |
| 17:04:36 | sean-k-mooney | how | |
| 17:04:46 | bauzas | tbc : create a flavor with a random keypair | |
| 17:05:07 | bauzas | create aggregates with hosts having the same keypair in their metadata | |
| 17:05:11 | bauzas | and you're done | |
| 17:05:26 | bauzas | you don't need to use this specific property | |
| 17:05:44 | sean-k-mooney | your missing the point | |
| 17:06:00 | bauzas | which is ? | |
| 17:06:19 | sean-k-mooney | we cant remove hide_hypervisor_id form the current flavor without changing the behavior | |
| 17:06:44 | sean-k-mooney | and if it si set in the falvor then the AggregateInstanceExtraSpecsFilter will check it against the aggreate metadata | |
| 17:07:19 | dansmith | bauzas: so my point is, when we revert, we will allocate a second mdev, and then abandon the original one when we redefine.. but I guess like delete, that one then will get used the next time we go to spawn something that needs one | |
| 17:07:24 | sean-k-mooney | we will return false here https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L62-L68 | |
| 17:07:58 | sean-k-mooney | tframbo: the other workaround is to add the metadata key to the aggreate | |
| 17:08:22 | bauzas | dansmith: you're probably correct, we could have a window in time where we would allocate a new mdev and then releasing the other | |
| 17:08:51 | bauzas | which could lead to a capacity issue if you're out of available mdevs | |
| 17:08:59 | sean-k-mooney | tframbo: basically create two two new agggreats that conatin all hosts, then add hide_hypervisor_id=true to one and hide_hypervisor_id=false to the other | |
| 17:09:31 | bauzas | dansmith: s/probably/absolutely after re-thinking of it | |
| 17:09:35 | dansmith | bauzas: right | |
| 17:09:39 | sean-k-mooney | tframbo: if you do that we will get to here https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L69-L71 and break then return ture and pass the host | |
| 17:10:07 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in remaining servers policies https://review.opendev.org/720116 | |
| 17:10:10 | openstackgerrit | Merged openstack/nova master: Fix servers policy for admin_or_owner https://review.opendev.org/718501 | |
| 17:10:11 | bauzas | dansmith: this being said, I don't exactly know how to fix this, since this is the whole revert workflow | |
| 17:10:26 | openstackgerrit | Merged openstack/nova master: Add test coverage of existing server policies https://review.opendev.org/718348 | |
| 17:10:32 | bauzas | and I guess the problem remains for all other resources | |
| 17:10:38 | openstackgerrit | Merged openstack/nova master: Introduce scope_types in servers Policies https://review.opendev.org/662968 | |
| 17:10:48 | openstackgerrit | Merged openstack/nova master: Add new default roles in servers policies https://review.opendev.org/662971 | |
| 17:10:55 | openstackgerrit | Merged openstack/nova master: Add test coverage of existing server attributes policies https://review.opendev.org/719726 | |
| 17:11:04 | openstackgerrit | Merged openstack/nova stable/train: Reset the cell cache for database access in Service https://review.opendev.org/718934 | |
| 17:11:16 | openstackgerrit | Merged openstack/nova stable/stein: Use placement stable version for functional job https://review.opendev.org/719121 | |
| 17:11:51 | bauzas | dansmith: we could consider to special case the revert and find the existing mdev so we would pass it again instead of calling _allocate_mdevs() tho | |
| 17:12:05 | dansmith | bauzas: seems like that would be better | |
| 17:12:11 | dansmith | also avoids needing to grab the allocations right? | |
| 17:12:22 | bauzas | indeed | |
| 17:12:25 | bauzas | dansmith: <3 with love | |
| 17:12:44 | bauzas | that's what I call a top-notch review | |
| 17:15:33 | sean-k-mooney | tframbo: does my responce on the bug or above make sense too you | |
| 17:15:42 | tframbo | sean-k-mooney: yeah, we could also add the aggregate metadata "hide_hypervisor_id=true" to solve the bug. | |
| 17:16:02 | sean-k-mooney | yes | |
| 17:16:13 | tframbo | thank you sean-k-mooney and bauzas | |
| 17:16:23 | sean-k-mooney | do you also use the compute capability filter by the way | |
| 17:16:33 | sean-k-mooney | if not be aware it has the same behavior | |
| 17:16:52 | tframbo | I think the hide_hypervisor_id is bool before , sorry | |
| 17:17:57 | sean-k-mooney | tframbo: its is a bool that default to false if unset | |
| 17:18:11 | sean-k-mooney | so there isnt really a reason to ever set it to false explcitly | |
| 17:18:16 | bauzas | tframbo: sorry I dropped focus | |
| 17:18:21 | sean-k-mooney | unless you want to prevent images form settin git too true | |
| 17:20:58 | bauzas | sean-k-mooney: tframbo: so, again, the intent is to have the aggregate metadata key to be set | |
| 17:21:18 | tframbo | sean-k-mooney: yes ,we alse use the compute capability filter by the way | |
| 17:21:36 | tframbo | bauzas: yes | |
| 17:22:19 | bauzas | I honestly don't get the problem | |
| 17:22:20 | sean-k-mooney | tframbo: ok so the point i wanted to make in that case was that if you use both then you are required to not use unnamespaced keys or they can conflict | |
| 17:23:10 | bauzas | wait | |
| 17:23:26 | bauzas | the namespace is only for specifying that the key is for this filter | |
| 17:23:43 | sean-k-mooney | bauzas: the problem is the AggregateInstanceExtraSpecsFilter require that an aggreate assocaited with a host must have all extra specs in the aggregate_instance_extra_specs as well as all non namespaced extra specs in the metadta | |
| 17:23:43 | bauzas | I just feel it's a configuration problem | |
| 17:24:25 | bauzas | or prefixed by aggregate_instance_extra_specs https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L54 | |
| 17:24:29 | bauzas | and this is not a bug | |
| 17:24:30 | sean-k-mooney | bauzas: the filter also looks at non namespced extra specs and we only have 1 standard extra specs that falls into that catagory | |
| 17:25:09 | bauzas | this *works* with any random key | |
| 17:25:17 | bauzas | I don't get the point again | |
| 17:25:36 | bauzas | if you're using this flavor key for passing QEMU, fine | |
| 17:25:47 | bauzas | that's how it's intended to be used | |
| 17:25:53 | sean-k-mooney | hide_hypervisor_id is not a random custom key | |
| 17:25:59 | bauzas | but then, indeed, you need to have aggregates matching it | |
| 17:26:08 | bauzas | (if you use this filter) | |
| 17:26:08 | sean-k-mooney | its a stanard one and its the only standard extra spec without a namesapce | |
| 17:26:21 | tframbo | https://docs.openstack.org/nova/latest/user/flavors.html | |
| 17:26:27 | sean-k-mooney | so its also the only stanard extrapec the filter uncondtionally checks | |
| 17:26:32 | bauzas | this filter doesn't care a single bit about standard extra specs | |
| 17:27:14 | bauzas | you're confusing with the other filters that do care of those prefixes | |
| 17:27:23 | sean-k-mooney | bauzas: no im not | |
| 17:27:40 | bauzas | honestly, I'm done for the day | |
| 17:27:43 | sean-k-mooney | to be clear the bug was intoduceing hide_hypervisor_id without a namespace | |
| 17:27:52 | tframbo | sean-k-mooney: thank you, I will sleep , good night | |
| 17:28:01 | sean-k-mooney | tframbo: night o/ | |
| 17:28:39 | melwitt | sorry I didn't understand all the previous details but why does the filter require a namespace in order to work? should it be able to work with namespaced and non-namespaced extra specs? | |
| 17:28:51 | melwitt | *shouldn't it | |
| 17:29:35 | sean-k-mooney | melwitt: the filter iterages over all extra specs in the flavor and then asserts they match the metadta if and only if the start with the filters namespace or they have no namespce | |
| 17:29:58 | bauzas | melwitt: that's my thinkings | |
| 17:30:03 | sean-k-mooney | so for all other standard extra specs they are ingore because they have a namespace which is not the filters one | |
| 17:30:06 | bauzas | but apparently we need to namespace now... | |
| 17:30:35 | sean-k-mooney | since this extra spec has no namespace its check by defualt which no other standar extraspec is | |
| 17:30:41 | sean-k-mooney | so there is a behavioral difference | |
| 17:30:50 | bauzas | sean-k-mooney: so | |
| 17:30:52 | bauzas | sean-k-mooney: https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L55-L58 | |
| 17:30:57 | melwitt | sean-k-mooney: wait but you say "if they have no namespace", doesn't this have no namespace and therefore should be considered? | |
| 17:31:09 | melwitt | gah this is so confusing | |
| 17:31:15 | bauzas | sean-k-mooney: this conditional is here to *PREVENT* other standard keys are ARE prefixed to be read | |
| 17:31:28 | sean-k-mooney | melwitt yes this has no namespace an by the filter logic should be check | |
| 17:31:50 | sean-k-mooney | melwitt: however if you add any other standard extra spec you do not have to update the metadta to boot a vm | |
| 17:31:52 | melwitt | so ... what's the problem? that makes it sound like there's no bug | |
| 17:31:55 | sean-k-mooney | for this extra spec you do | |
| 17:32:02 | bauzas | melwitt: there is NO bug in my mind | |