mikal | sean-k-mooney / melwitt / gmann / dansmith: to my mild surprise, after a mere 147 test runs the tempest test for spice-direct is now passing on the nova-ovs-hybrid-plug job. I think that means a whole bunch of changes are probably ready for re-review. Specifically: | 01:47 |
---|---|---|
mikal | spice-direct itself: https://review.opendev.org/c/openstack/nova/+/926876, https://review.opendev.org/c/openstack/nova/+/926877, https://review.opendev.org/c/openstack/nova/+/924844 | 01:47 |
mikal | tempest testing for spice-direct: https://review.opendev.org/c/openstack/tempest/+/940943, https://review.opendev.org/c/openstack/devstack/+/940838, https://review.opendev.org/c/openstack/nova/+/940835, https://review.opendev.org/c/openstack/nova/+/940873 (the last three are from sean-k-mooney) | 01:47 |
mikal | The other patch series for USB / sound support is blocked at the moment on https://review.opendev.org/c/openstack/os-traits/+/940418 merging, but it lacks a second +2. | 01:47 |
gmann | mikal: RE on os-console-auth-tokens schema: for easy way, add it in the new microversion for what you are adding the test with min_microversion. I think that should work and worth trying it instead of adding it for old/base microversion | 04:19 |
gmann | I will check tempest and devstack change tomorrow | 04:19 |
mikal | gmann: thanks. I added it to my change, we can always shift it back in time later if anyone ever needs it. | 07:01 |
opendevreview | Vasyl Saienko proposed openstack/nova master: WIP: Add trunk subports to network metadata https://review.opendev.org/c/openstack/nova/+/941227 | 07:59 |
tkajinam | this cleanup has been sitting here for some time and I wonder if it can be merged https://review.opendev.org/c/openstack/nova/+/879021 | 08:34 |
zuhal | hi the root volumes of the virtual machines remain in the attaching process, and since the cinder update volume attachment takes a long time, the Nova gives a cinder client gateway timeout error. how can I make a setting for this? | 10:36 |
sean-k-mooney | mikal: awsome news | 11:36 |
opendevreview | Merged openstack/nova master: Bump os-traits to 3.3.0 in requirements https://review.opendev.org/c/openstack/nova/+/940975 | 14:16 |
* bauzas wonders the difference between nova-next/nova-multicell and other tempest jobs when creating a new image from a volume | 14:32 | |
bauzas | because https://github.com/openstack/tempest/blob/master/tempest/scenario/test_volume_boot_pattern.py#L222 only fails with my weigher patch on those two jobs https://review.opendev.org/c/openstack/nova/+/940642 | 14:32 |
bauzas | bottom line, when turning the ImageMetaProps object into a primitive dict, we get some fields that are a CoercedList typ | 14:33 |
bauzas | and given CoercedList o.vo object doesn't have a hash method, then we fail | 14:34 |
dansmith | bauzas: that's just when using our old toplevel obj_to_primitive right? I assume because that doesn't go deep enough or know about those o.vo things | 14:38 |
sean-k-mooney | i see | 15:16 |
sean-k-mooney | class CoercedList(CoercedCollectionMixin, list): | 15:17 |
sean-k-mooney | so list does not have a hash function then | 15:17 |
sean-k-mooney | we should not get CoercedList normlaly form ImageMetaProps | 15:18 |
sean-k-mooney | oh maybe form tags | 15:19 |
sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L71 | 15:19 |
sean-k-mooney | sorry that ImageMeta | 15:20 |
sean-k-mooney | ok so it might be from hw_numa_cpus https://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L410 | 15:20 |
sean-k-mooney | in general we dont have any list fies but hw_numa_cpus and hw_numa_mem are | 15:21 |
opendevreview | Merged openstack/placement master: Bump os-traits to 3.3.0 in requirements https://review.opendev.org/c/openstack/placement/+/940976 | 15:22 |
sean-k-mooney | we can likely just updat the list type here with a simple hash funtion of that take the hash of all element in the list | 15:22 |
sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/fields.py#L48-L69 | 15:22 |
sean-k-mooney | alternitivly instead of storing the key/values in a map or set we can store them as a sting or tuple | 15:25 |
*** __ministry is now known as Guest9033 | 15:27 | |
bauzas | yeah I could avoid the set calculation I think, I need to rethink the design | 15:27 |
bauzas | and yeah, that probably because of the values, not the keys | 15:28 |
sean-k-mooney | so instead of "set(inst.image_meta.properties.to_dict().items())" do ```{ "f{key},{value}" for key, value in inst.image_meta.properties.to_dict().items()}``` | 15:28 |
opendevreview | Takashi Kajinami proposed openstack/os-vif master: Remove os-vif-linuxbridge https://review.opendev.org/c/openstack/os-vif/+/941567 | 15:29 |
bauzas | that seems a bit hacky to me but this could work indeed | 15:29 |
sean-k-mooney | f goes on outside ... ```{ f"{key},{value}" for key, value in inst.image_meta.properties.to_dict().items()}``` | 15:29 |
bauzas | yeah, I got your point, we would hash by strings :D | 15:30 |
sean-k-mooney | bauzas: it just relies on the fact that all stings are hashable | 15:30 |
sean-k-mooney | yep | 15:30 |
bauzas | this is fine | 15:30 |
sean-k-mooney | ideally i think ovo should be fixed to defien a hash on the element of a coherced list | 15:30 |
bauzas | but providing a fat comment would be nice I guess | 15:30 |
sean-k-mooney | but we proably dont want ot hold this on that | 15:31 |
sean-k-mooney | and ya a comment woudl be good | 15:31 |
bauzas | yup, FF is arriving | 15:31 |
dansmith | bauzas: sean-k-mooney I thought we were going to have a convo about a redesign? | 15:31 |
bauzas | winter is coming | 15:31 |
sean-k-mooney | we still can | 15:31 |
sean-k-mooney | dansmith: ^ | 15:31 |
dansmith | gibi is not sufficiently convinced about the usefulness of this approach anyway | 15:31 |
sean-k-mooney | in my last review i suggested creating a follow up path | 15:32 |
sean-k-mooney | to allow per key weights using teh same syntax/code form the metrics weiher | 15:32 |
bauzas | if we could add that weigher as a first approach, I'm fine with a later refactoring that would fix world's problems | 15:32 |
opendevreview | Takashi Kajinami proposed openstack/os-vif master: Drop kuryr-kubernetes-tempest https://review.opendev.org/c/openstack/os-vif/+/941568 | 15:32 |
sean-k-mooney | that way we can decced if we want the added complexity or not | 15:32 |
bauzas | sean-k-mooney: I still need to review your comments, I mostly focused on the failing jobs | 15:32 |
gibi | I'm open to a design discussion | 15:33 |
sean-k-mooney | bauzas: they were pretty minior. | 15:34 |
sean-k-mooney | basically if we have a more granualr approch to specifying weights per image property | 15:34 |
sean-k-mooney | i think there is merrit in having the behavior/syntax mirror that of the exisitng metric weigher | 15:34 |
sean-k-mooney | mainly just for consitency | 15:34 |
bauzas | okay I can take a look | 15:35 |
sean-k-mooney | i dont know if that woudl satisfy all open concerns or not but that was the tldr, get the simple version workign and passing then follow patch to extend and discuss later if we finally want to implement supprot for prefered traits | 15:36 |
bauzas | gibi: reading your comments, you know that we only compare the host properties against the exact image requested properties, right ? | 15:36 |
bauzas | I originally tried to compare all props as equal, but I eventually considered only the calculation of how many requested props were present on the host | 15:37 |
dansmith | I think gibi's point is that many irrrelevant props may be requested that dilute the scoring | 15:41 |
gibi | I'm not sure I see the consequences of it as I don't know what host properties means in this context | 15:41 |
gibi | dansmith: yes | 15:41 |
gibi | preferred traits in image metadata implemented as a weigher seems interesting and promising but haven't spent the time to think it through | 15:42 |
sean-k-mooney | gibi: its something i have wanted since before we actully had required traits | 15:42 |
gibi | as that would mean each image defines what property it cares about | 15:42 |
sean-k-mooney | if we supported prefered traits i woudl want to supprot it for both flavor and image ideally | 15:43 |
sean-k-mooney | but same overall impleation just way the host based on teh prefernce using the provider summaries | 15:43 |
bauzas | gibi: I need to validate your example in https://review.opendev.org/c/openstack/nova/+/940642/comment/932beb0e_c4933c32/ | 15:44 |
bauzas | gibi: because while I agree with the fact that VM1 will be on host1 and VM2 on host2, I need to calculate the weight for VM3 | 15:46 |
dansmith | to make it fully automatic you need to automatically export the trait when an instance is booted on a host, so perhaps some conf knob to expose traits of a thing automatically? l | 15:46 |
dansmith | auto_expose_image_props = os_type | 15:46 |
gibi | bauzas: my understanding is more matching properties means higher weights | 15:47 |
bauzas | you're right indeed, I see the problem now | 15:47 |
gibi | dansmith: good point, a full dynamic approach needs dynamic traits | 15:47 |
bauzas | well, that's not really a problem, you're diluting your need into multiple pieces | 15:47 |
dansmith | I again assert that for a cloudy workload that probably gets stronger with lots of boots of the same image | 15:47 |
bauzas | I assumed less fine-grained image properties | 15:48 |
dansmith | if you have tons of snowflake pet images then for sure it will be a very weak force | 15:48 |
dansmith | right | 15:48 |
dansmith | and I assumed this was a soft clumping optimization and not a hard packing mechanism | 15:48 |
bauzas | yup, KISS | 15:48 |
dansmith | because if you want the latter, it should use stronger guarantees | 15:48 |
gibi | dansmith: VMs from the same image attract each other I agree. So if we only have a single windows image then the solution works | 15:48 |
dansmith | gibi: or some small number of them that are probably going to be similar | 15:49 |
bauzas | we have a meeting in 20 mins | 15:49 |
bauzas | 10 mins* | 15:49 |
bauzas | but I think we need a design discussion indeed | 15:49 |
bauzas | so if you want, let's do that after the internal meeting, ie. 1700UTC | 15:50 |
bauzas | would 1700UTC work for the three of you ? sean-k-mooney gibi dansmith | 15:50 |
dansmith | I figure not gibi | 15:50 |
dansmith | but wfm | 15:50 |
bauzas | OK, I refreshed my brain on the metricsweigher | 15:51 |
sean-k-mooney | sure | 15:51 |
bauzas | fun fact, there was a note with my name, so I already walked that code | 15:52 |
gibi | dansmith: yeah this is a balancing act. if you have more windows images or if the windows images have a lot of other prop than os_type then you are losing the feature. Now the question is is this compromise acceptable for the customer requesting the feature. Telling them to limit the number of different windows VMs or limit the number of image props (it is already a lot by default from glance) might | 15:52 |
bauzas | but gosh my brain is so bad at remembering things | 15:52 |
gibi | be OK or might not. I leaning towards no. But we can probably ask :) | 15:52 |
gibi | bauzas: I can take a 30 min meeting after our internal call | 15:53 |
bauzas | gibi: sean-k-mooney's approach mimicing the metricsweigher seems a reasonable trade-off as a follow-up patch | 15:53 |
dansmith | gibi: yeah, all goes to how strongly you expect it to work. I expected weak optimization because it needs to be fully automatic | 15:53 |
dansmith | or intended to be fully automatic I mean | 15:53 |
bauzas | basically, it makes the weigh multiplier less easy to configure but that gives you a possibility to weigh per prop | 15:54 |
sean-k-mooney | for hard affinity we have the "isolating aggrates with placement" feature | 15:54 |
dansmith | I pretty much assume any weigher implementation is just trying to make better-than-random decisions, but not anything you'd bet on | 15:54 |
dansmith | bauzas: you mean my suggestion of a dict of properties and weights? | 15:54 |
dansmith | I think that gets closer to gibi's goal, but I also offered that suggestion along with a good example of why it's too simplistic | 15:55 |
sean-k-mooney | the weigher multipler is for seting the relitve weight between weighers when nomalising | 15:55 |
sean-k-mooney | the per property weight is internal | 15:55 |
sean-k-mooney | to how we calulate a weight for a given host before we normalise | 15:56 |
sean-k-mooney | so they have 2 distinct usecases | 15:56 |
sean-k-mooney | anyway shall we pick this up in an hour | 15:57 |
bauzas | yup, let's see ourselves in 1 hour | 15:58 |
bauzas | I'll ping back here | 15:58 |
bauzas | others can chime if they want | 15:59 |
gibi | need 2 minutes and I can join | 17:02 |
bauzas | okay, I'm starting a gmeet | 17:02 |
bauzas | https://meet.google.com/vye-tjns-bai <= for discussing about https://review.opendev.org/c/openstack/nova/+/940642/ | 17:03 |
sean-k-mooney | bauzas: gibi dansmith melwitt on an unrelated note. neutron removed linuxbridge supports weeks ago | 17:42 |
sean-k-mooney | os-vif and nova have linux bridge jobs | 17:42 |
sean-k-mooney | the nova one only tirggers on specific files | 17:42 |
sean-k-mooney | tehre was a mailing list post about this a while ago but i just want to get poeopel opions on how to move forward | 17:43 |
sean-k-mooney | first ill put up a patch to drop hte job form nova's check pipeline | 17:43 |
sean-k-mooney | beyond that there is a patch to remove the job form os-vif but i think we should also remove the linux brindge os-vif plugin | 17:44 |
sean-k-mooney | os-vif's policy has always been we only have pluggins for in tree neutron backends | 17:44 |
sean-k-mooney | so question 1 is do i defer the plugin removal to 2025.2 or do it in 2025.1/epoxy | 17:45 |
sean-k-mooney | second question is there is linux bridge speciric code in nova. wehn shoudl we remove that? | 17:45 |
sean-k-mooney | my propsoal is to formally deprecate the linux bridge support in nova this cycle | 17:46 |
sean-k-mooney | and remove the linux brdige code in nova in 2025.2 | 17:46 |
sean-k-mooney | you would only be able to user linux brdige in nvoa as part fo 2025.1 if you had an older neutron but since we never deprecated support you could argure that that is the best path forward | 17:47 |
sean-k-mooney | we coudl also just be more agressive and delete the code now | 17:47 |
sean-k-mooney | bauzas: gibi dansmith melwitt ^ we dont have to decied now but let me know what ye woudl perfer | 17:48 |
dansmith | idk, deprecate and lag on removal I guess | 17:49 |
sean-k-mooney | if we go with that approch ill change my review on https://review.opendev.org/c/openstack/os-vif/+/941567 | 17:50 |
sean-k-mooney | ill prepare patches to do the formal dpercations and unblock ci this evening and we can review in the irc meeting on tusday. the non clint lib freeze is next week so we need to do an os-vif release wiht the deprecation but i that thats ok | 17:58 |
*** __ministry is now known as Guest9047 | 18:01 | |
opendevreview | sean mooney proposed openstack/os-vif master: Deprecate linux bridge supprot https://review.opendev.org/c/openstack/os-vif/+/941585 | 18:36 |
opendevreview | sean mooney proposed openstack/os-vif master: remove linux bridge plugin https://review.opendev.org/c/openstack/os-vif/+/941586 | 18:36 |
opendevreview | sean mooney proposed openstack/os-vif master: Deprecate linux bridge plugin https://review.opendev.org/c/openstack/os-vif/+/941585 | 18:37 |
opendevreview | sean mooney proposed openstack/os-vif master: remove linux bridge plugin https://review.opendev.org/c/openstack/os-vif/+/941586 | 18:37 |
bauzas | sean-k-mooney: still there ? | 19:02 |
bauzas | sean-k-mooney: just a question, given the logic of the conf options for the metricsweigher sound reasonable to me, I wonder whether I should duplicate those options for my specific weigher or reuse those metrics options by adding a doc modification saying that the ImageMetaWeigher could also use them ? | 19:03 |
sean-k-mooney | yep | 19:04 |
bauzas | yep about which alternative ? dup or reuse ? | 19:04 |
sean-k-mooney | you need to duplicate them | 19:04 |
bauzas | ok | 19:04 |
sean-k-mooney | yep im here | 19:04 |
bauzas | OK then let's go for specifying them | 19:05 |
bauzas | thanks | 19:05 |
sean-k-mooney | the other reason they have to be seperated is | 19:06 |
sean-k-mooney | with the new weither if the new config option is not defied we effectivly weigh each option as 1 | 19:06 |
sean-k-mooney | i.e. evenly | 19:06 |
sean-k-mooney | but when its defiend we woudl only apply the weigts to the specified values and ignore the rest | 19:07 |
sean-k-mooney | where as for the metric weigher the cofnig option is required for it to work | 19:07 |
sean-k-mooney | instead of being optional | 19:07 |
sean-k-mooney | so we dont want to share it | 19:07 |
mikal | Morning | 19:26 |
mikal | It seems like its been fairly quiet on the spice patches overnight, apart from gmann approving the temptest test (thanks gmann!). Are there any actions items for me right now that I should know about? | 19:29 |
bauzas | sean-k-mooney: fine, then I'll just provide *one* additional config option, by default we'll stick to evenly weigh the props | 19:29 |
sean-k-mooney | +1 | 19:30 |
sean-k-mooney | mikal: i honestly didnt have time to look at the current state | 19:30 |
sean-k-mooney | but you got it to the point of workign and testign in a ci job | 19:31 |
sean-k-mooney | so i think we could move forward with merging some/all of these? | 19:31 |
mikal | sean-k-mooney: yeah, no worries. | 19:31 |
sean-k-mooney | alos we should move forward iwth os-trait patch if not allready done? | 19:31 |
mikal | sean-k-mooney: yes. The tempest test passes in the ovs-plug job, assuming that the entire rube goldberg machine of changes is landed | 19:31 |
gmann | mikal: pinged frickler for devstack change 2nd review review in qa channel | 19:31 |
mikal | sean-k-mooney: yeah, once the spice-direct series is landed the os-traits change will be the main thing blocking me. It blocks the sound / usb changes. | 19:32 |
mikal | gmann: thank you. | 19:32 |
sean-k-mooney | we need a second +2 on https://review.opendev.org/c/openstack/os-traits/+/940418 and we need to a release before the non clinet lib freeze next week | 19:32 |
mikal | sean-k-mooney: I am a little unsure if we want to compress your zuul changes into a single change with the spice API changes or keep it like this. I definitely don't want to take credit for your work by stealing your changes. | 19:32 |
sean-k-mooney | steal all the things | 19:33 |
mikal | sean-k-mooney: that freeze is a week before feature freeze, or at the same time? | 19:33 |
sean-k-mooney | im fine with squashing them if you want | 19:33 |
gmann | bauzas: not sure if you see my message early this week. but let me know what I need to do to get this spec merged? I added it in Nova meeting agenda for spec exception approval (but i would not be able to attend meeting due to time conflict). I started code change in parallel if that help, manager role is almost ready and will work on service role change https://review.opendev.org/c/openstack/nova-specs/+/937650 | 19:33 |
sean-k-mooney | mikal: the non client freeze is a week before feature freeze | 19:33 |
mikal | sean-k-mooney: I'd prefer to not squash them unless there's a really solid reason why we should. I think the history is useful. | 19:33 |
sean-k-mooney | then i think we can jsut try and move them forward as is | 19:34 |
bauzas | gmann: this cycle, I unfortunately don't have a lot of time upstream | 19:34 |
bauzas | so I saw your ping | 19:34 |
sean-k-mooney | gmann: is the code written and tested? | 19:35 |
sean-k-mooney | becasue otherwise i think this need to go to next cycle | 19:35 |
bauzas | yeah, that's my question | 19:35 |
sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/941056 | 19:35 |
sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/941347 | 19:35 |
gmann | sean-k-mooney: I have started that, manager role changes is almost ready (fixing some unit and tempest test) | 19:35 |
sean-k-mooney | are teh nova patches | 19:35 |
gmann | service role change I will work soon | 19:36 |
gmann | https://review.opendev.org/q/topic:%22bp/policy-service-and-manager-role-default%22 | 19:36 |
gmann | ^^ this is all series where service role changes i need to rebase/fix | 19:36 |
sean-k-mooney | oh there are others ok | 19:36 |
sean-k-mooney | we kind of have a lot of patches that need review so i kind of feel like this should move to next cycle | 19:37 |
sean-k-mooney | on the ohter had i dont really want to be the one to punt it so.. | 19:37 |
gmann | well, thing is I am not sure if I can spend more time on upstream activities in next cycle (depend on various factor) so I thought of getting it done in nova this cycle | 19:39 |
gmann | anyways I added it in meeting agenda and will be ok based on how team decide. unfortunately I will not be able to attend meeting but you knows the current situation on this so I will check meeting log/decision. | 19:40 |
bauzas | gmann: noted, we'll discuss this on next tues | 19:44 |
gmann | bauzas: thanks | 19:44 |
bauzas | sean-k-mooney: one last question about the weigher, do we agree on the fact that props not set on the pre-prop setting won't be weighed ? | 19:45 |
bauzas | we'll just skip them if so | 19:46 |
bauzas | ie. default of non-set props will be 0.0 | 19:46 |
opendevreview | ribaudr proposed openstack/nova master: Add managed flag to PCI device specification https://review.opendev.org/c/openstack/nova/+/937649 | 19:49 |
opendevreview | ribaudr proposed openstack/nova master: Update driver to deal with managed flag https://review.opendev.org/c/openstack/nova/+/938405 | 19:49 |
sean-k-mooney | bauzas: i think that make the most sense | 19:56 |
sean-k-mooney | bauzas: so sure you can use wights.get(name, 0.0) | 19:57 |
bauzas | that's litterally what I wrote :D | 19:57 |
bauzas | sending the WIP for the FUP in a very sec | 19:57 |
mikal | sean-k-mooney: I need to wander off and get ready for work. I'll check in again this evening but y'all look busy on other things today which is fine. | 19:58 |
sean-k-mooney | im on pto tomorroow but ill try and make time on monday | 20:00 |
mikal | No worries | 20:00 |
birbilakos_ | hello | 20:22 |
birbilakos_ | After some network changes in my 2023.2 deployment which broke network connectivity to one controller node (and reverting that back) I get bombarded by these type of errors: https://paste.opendev.org/show/b8QRG6bxWiKh9CLM79oR/ | 20:22 |
birbilakos_ | i also see that several nova actions either timeout or are very slow to go through. any ideas how to recover this? | 20:22 |
birbilakos_ | rabbitmq containers seem to be running fine, also checked the management web interface of rabbitmq and didn't notice anything strange | 20:23 |
sean-k-mooney | that just looks like issue with rabbit mq | 20:24 |
sean-k-mooney | it does not look like its really nova specific beyond tat | 20:24 |
sean-k-mooney | the messagign timeout may be because you restart nova conductor for example | 20:25 |
sean-k-mooney | and the requestrs were lost | 20:25 |
sean-k-mooney | or restart rabbitmq | 20:25 |
opendevreview | Ghanshyam proposed openstack/nova master: Add project manager role in Nova API policy rule https://review.opendev.org/c/openstack/nova/+/941347 | 20:28 |
opendevreview | Sylvain Bauza proposed openstack/nova master: Add a new ImagePropertiesWeigher https://review.opendev.org/c/openstack/nova/+/940642 | 20:46 |
opendevreview | Sylvain Bauza proposed openstack/nova master: per-property ImageMetaPropsWeigher https://review.opendev.org/c/openstack/nova/+/941601 | 20:46 |
birbilakos_ | thanks sean | 20:52 |
birbilakos_ | how do i proceed then? currently nova is reporting missing messages and also fails to do actions | 20:52 |
birbilakos_ | e.g. instance creation and deletion is a hit and miss... sometimes it works, sometimes it doesnt | 20:53 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!