| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-16 | |||
| 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 | |
| 17:32:19 | bauzas | IMHO the bug should be consider Invalid if not Expired | |
| 17:32:44 | sean-k-mooney | bauzas: i strongly dissagre. as i said the bug is not in the filter | |
| 17:32:57 | sean-k-mooney | the bug is that we added a stanard extra spec without a namespace | |
| 17:33:19 | bauzas | and what's the impact then ? | |
| 17:35:04 | sean-k-mooney | by intoducing a flavor extra spec without a namespace, to use the feature enable by that extra spec it addtionally required the operator to update the aggreate metatad and host capabilty if they use the ComputeCapabilitiesFilter or AggregateInstanceExtraSpecsFilter | |
| 17:35:05 | bauzas | oh, the fact that you need to create aggregates in order to use it, let me bet ? | |
| 17:35:13 | sean-k-mooney | yes | |
| 17:35:50 | sean-k-mooney | that is a behavioral differen form every other extra spec that is defined by nova | |
| 17:35:55 | bauzas | holy f..., gotcha | |
| 17:36:38 | sean-k-mooney | melwitt: do you follow too ^ | |
| 17:36:48 | bauzas | okay, so indeed the fix is not about the filter | |
| 17:36:59 | bauzas | it's about where we use this extraspec for booting | |
| 17:37:01 | melwitt | I see, ok. just read through the bug and comments again. yeah, I think so. this is an extra spec meant to be used without having to add a matching metadata key on an aggregate | |
| 17:37:27 | sean-k-mooney | ya so the fix is add a namespaced version of the extra spec and compat code | |
| 17:37:43 | sean-k-mooney | then deprecate teh non namespaced one | |
| 17:37:47 | melwitt | yeah, I think that makes sense. agree that's the way to fix | |
| 17:37:53 | bauzas | sean-k-mooney: easy fix then https://review.opendev.org/#/c/555861/10/nova/virt/libvirt/driver.py | |
| 17:38:25 | bauzas | sean-k-mooney: just add another key there with a prefix and just provide a deprecation warning for the existing one | |
| 17:38:39 | sean-k-mooney | yep | |
| 17:38:45 | bauzas | that's all flavors and aggregates, we don't need to care about the interop | |
| 17:39:11 | sean-k-mooney | im wondering if we should also add a config option for the filter to disable checking unnamesapced extraspecs | |
| 17:39:14 | bauzas | tlrambo dropped but I'll leave a comment in the bug | |
| 17:39:22 | bauzas | sean-k-mooney: oh please don't | |
| 17:39:29 | bauzas | sean-k-mooney: it was a review problme | |
| 17:39:34 | bauzas | not a code problem | |
| 17:40:12 | sean-k-mooney | well the reason for doing it is i would prefer to drop the non namespced approch entirely eventually or maybe depreate the filters | |
| 17:40:25 | bauzas | NO again in capitals :) | |
| 17:40:40 | sean-k-mooney | given custom traits could used for this. you also suggested this last week by the way | |
| 17:40:49 | sean-k-mooney | this is why im tinking about it | |
| 17:41:32 | sean-k-mooney | anyway lets jus tdo the minima dirver fix for now | |
| 17:41:39 | bauzas | sean-k-mooney: the only difference is that we don't have traits on placement aggregates, right? | |
| 17:42:10 | sean-k-mooney | bauzas: correct they live on RPs | |
| 17:42:21 | sean-k-mooney | so i thikn the compute capablity filter can defiently go. | |
| 17:42:28 | bauzas | from what I understood from the very-long-standing battle of allocation ratios is that some operators do care about having a grouping system for managing their fleet of computes | |
| 17:42:41 | sean-k-mooney | this one woudl requirte us to creat a sharing resouce provider per host aggreate | |
| 17:42:49 | bauzas | (even if that can be done programmatically by something else) | |
| 17:43:18 | bauzas | my old grandma' was sayin' : "if that works, don't touch it" | |
| 17:43:37 | bauzas | and loooots of ops do manage aggregates thru this filter | |
| 17:43:57 | sean-k-mooney | we have had custoemr bitten by this in the past as an fyi. specifcly the conflict betwwen the capablity filter and aggreate one | |
| 17:44:11 | bauzas | so unless we come up with a solid upgrade plan for replacing it with very simple abstractions, don't touch it | |
| 17:44:26 | sean-k-mooney | bauzas: yep agree | |
| 17:44:29 | bauzas | sean-k-mooney: we resolved it with namespaces, right? | |
| 17:44:33 | sean-k-mooney | yes | |
| 17:44:42 | sean-k-mooney | basicaly they were adding pinned=true | |
| 17:44:47 | bauzas | problem solved. | |
| 17:44:58 | sean-k-mooney | they just namespaced it | |
| 17:45:06 | bauzas | ++ | |
| 17:51:32 | dansmith | man, so busy this morning I missed out on 50% of my usual coffee consumption.. it must be TEOTWAWKI | |
| 17:53:23 | sean-k-mooney | i try to some degree contol my caffein intake including normaly not drinking coffee at the weekends but i can totally feel teh difference when i dont have any | |
| 17:53:55 | sean-k-mooney | given i only drink 1-2 cups a day i dont know if it woudl be more noticable if i drank more or less | |
| 17:55:32 | sean-k-mooney | if i drank more i think it would have less of an effect when i drank it but likely more of an effect when i didnt which is why i reduced my cafee intake in the first place | |
| 19:55:33 | openstackgerrit | melanie witt proposed openstack/nova stable/stein: Reset the cell cache for database access in Service https://review.opendev.org/720587 | |
| 20:39:53 | openstackgerrit | melanie witt proposed openstack/nova stable/rocky: Reset the cell cache for database access in Service https://review.opendev.org/720592 | |
| 20:56:27 | openstackgerrit | melanie witt proposed openstack/nova stable/queens: Reset the cell cache for database access in Service https://review.opendev.org/720596 | |
| 20:56:57 | openstackgerrit | melanie witt proposed openstack/nova stable/rocky: Reset the cell cache for database access in Service https://review.opendev.org/720592 | |
| 21:40:58 | openstackgerrit | Merged openstack/nova master: libvirt: Remove VIR_DOMAIN_BLOCK_REBASE_RELATIVE flag check https://review.opendev.org/702021 | |
| 22:37:19 | openstackgerrit | Merged openstack/nova master: images: Make JSON the default output format of calls to qemu-img info https://review.opendev.org/711679 | |
| #openstack-nova - 2020-04-17 | |||
| 00:37:15 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add docs and releasenotes for BP policy-defaults-refresh https://review.opendev.org/720129 | |
| 00:40:10 | gmann | lbragstad: can you verify the steps for migration to new policy. I cannot find the keystone exact doc please let me know if there is any so that i can link that in my doc - https://review.opendev.org/#/c/720129/4..5/doc/source/configuration/policy-new-defaults.rst@227 | |
| 00:51:33 | openstackgerrit | melanie witt proposed openstack/nova master: Enable test_volume_backed_live_migration in tempest https://review.opendev.org/528104 | |
| 00:53:12 | lbragstad | gmann sounds good - i'm about to pick up the review i started earlier for https://review.opendev.org/#/c/720129/4/doc/source/configuration/policy-new-defaults.rst | |
| 00:53:39 | lbragstad | i'll take a look at the new patch set | |