cardoe | So maybe something for the docs since I know you're working on it. I'm not terribly familiar with gerrit and the flows but I don't know how to propose a patch as a backport to a stable branch. | 00:22 |
---|---|---|
cardoe | Overall honestly though you guys should be proud. Ironic has some of the best docs of OpenStack projects. | 00:24 |
JayF | If you can link the patch here, when I get to a computer I'll take a look at backporting it for you and find the stable docs for you. | 00:52 |
cardoe | Well my point is I don’t wanna ask you to do it. I wanna do it. I already cherry-picked it locally and am using that. | 04:06 |
rpittau | good morning ironic! o/ | 06:21 |
opendevreview | Merged openstack/sushy-tools master: Python 3.12: do not use ssl.wrap_socket https://review.opendev.org/c/openstack/sushy-tools/+/923442 | 10:04 |
iurygregory | good morning Ironic | 12:05 |
opendevreview | Jacob Anders proposed openstack/ironic master: [WIP] add Targets to firmware.update on multi system BMCs https://review.opendev.org/c/openstack/ironic/+/922438 | 12:44 |
*** iurygregory_ is now known as iurygregory | 13:02 | |
opendevreview | Derek Higgins proposed openstack/sushy master: Provide vmedia username and password if required https://review.opendev.org/c/openstack/sushy/+/923524 | 14:25 |
rpittau | good night! o/ | 15:50 |
opendevreview | Derek Higgins proposed openstack/sushy master: Provide vmedia username and password if required https://review.opendev.org/c/openstack/sushy/+/923524 | 15:52 |
opendevreview | Derek Higgins proposed openstack/sushy master: Provide vmedia username and password if required https://review.opendev.org/c/openstack/sushy/+/923524 | 16:11 |
JayF | iurygregory: I think you missed https://review.opendev.org/c/openstack/ironic-lib/+/923205 when approving earlier | 16:33 |
JayF | would love for https://review.opendev.org/c/openstack/ironic/+/922612 to land too, I have some follow-up ideas for it | 16:36 |
JayF | similarly https://review.opendev.org/c/openstack/ironic/+/922484 | 16:36 |
iurygregory | JayF, yeah I totally missed that, tks! | 16:38 |
iurygregory | I will look at this two later today | 16:39 |
JayF | awesome | 16:40 |
opendevreview | Merged openstack/networking-baremetal master: Update to match latest development cycle https://review.opendev.org/c/openstack/networking-baremetal/+/923204 | 16:42 |
opendevreview | Merged openstack/virtualbmc master: Update to match latest development cycle https://review.opendev.org/c/openstack/virtualbmc/+/922589 | 16:43 |
opendevreview | Merged openstack/networking-generic-switch master: Update to match latest development cycle https://review.opendev.org/c/openstack/networking-generic-switch/+/923202 | 16:47 |
JayF | Also I know cid and I were looking for outside perspectives about the shape of the config option here: https://review.opendev.org/c/openstack/ironic/+/922243 -- if you're rushed don't even need a full review, just a second/third opinion on the shape of that config opt | 17:13 |
cid | ++ | 17:14 |
opendevreview | Merged openstack/ironic-lib master: Update to match latest development cycle https://review.opendev.org/c/openstack/ironic-lib/+/923205 | 17:14 |
opendevreview | Merged openstack/ironic master: Fix rendering of Redfish properties in the documentation https://review.opendev.org/c/openstack/ironic/+/922482 | 17:27 |
mnaser | hi folks, I think there was a regression in the ironic nova driver | 17:35 |
mnaser | https://github.com/openstack/nova/commit/6309013283e37e2cc259473af05413e01a0101af | 17:35 |
mnaser | the issue with this is I dont think it forces the request to be sent using a new enough micro version which includes portgroup_id | 17:35 |
mnaser | so instances with bonds fail to go up with caracal | 17:36 |
JayF | Can you put the details in a bug filed against Ironic + Nova? | 17:37 |
mnaser | yeah im just making sure it's not something I've missed | 17:37 |
mnaser | I'm just trying to figure out where the microversion used gets decided for there | 17:38 |
JayF | I'm 99% sure it's negotiated | 17:38 |
JayF | which is why I'm a little confused | 17:38 |
mnaser | yeah im running a tcpdump now to see how that gets captured | 17:41 |
JayF | mnaser: can you point me to *exactly where* we need portgroup_id and it's not being sent? | 17:43 |
JayF | and/or a traceback, something? | 17:43 |
JayF | I haven't done much at the Ironic<>Nova<>Neutron boundary so I'm flying mildly blind | 17:44 |
mnaser | essentially cloud-init is failing because pg_ports is empty here https://github.com/openstack/nova/blob/c39a425ba733474816e40f6633de5658468c2a87/nova/virt/ironic/driver.py#L1087-L1096 | 17:44 |
mnaser | which comes from https://github.com/openstack/nova/blob/c39a425ba733474816e40f6633de5658468c2a87/nova/virt/ironic/driver.py#L1071 | 17:45 |
mnaser | I figured it was maybe an old micro version but unfortunately.. nope, I see the response coming in with 1.88 ironic api and the port group is there | 17:45 |
JayF | https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L1053 ? | 17:45 |
mnaser | so it must be something else.. ill try hacking more | 17:45 |
mnaser | right yeah, so then technically it should get the ports that are part of that port group and add them .. however | 17:46 |
JayF | hey so I was looking in the right place | 17:46 |
JayF | that's nice, at least | 17:46 |
mnaser | I end up with an empty iface list https://usercontent.irccloud-cdn.com/file/XH6Nznut/image.png | 17:46 |
mnaser | see bond_links: [] | 17:46 |
JayF | if you have metadata service up, can you see what you get there? | 17:46 |
JayF | it's calculated a different way that might point to if it's ironic-driver-level or somewhere else | 17:47 |
JayF | just a nice data point | 17:47 |
mnaser | I dont in this case, im relying on configdrive because bonding and no network connectivity at that point | 17:47 |
mnaser | fwiw this worked perfectly fine in an older cloud, when we got to caracal, it borked | 17:47 |
JayF | yeah, makes sense | 17:47 |
JayF | assuming you're using NGS? | 17:48 |
JayF | and that all services went to caracal together? | 17:48 |
mnaser | and looking at the git blame and that code being changed I had my suspects, so im trying now to replicate manually in run time | 17:48 |
JayF | (meaning: it could be a bug somewhere else, potentially) | 17:48 |
mnaser | everything is caramel and nope, just flat | 17:48 |
JayF | caramel :D | 17:48 |
mnaser | yeah, it could be, but the network_data.json is generated in that specific part, and the bond_links are empty there specifically | 17:48 |
mnaser | lool | 17:48 |
JayF | you're running sugarstack! | 17:48 |
JayF | any patches in play here? | 17:50 |
TheJulia | Just thinking outloud, not working today, but Iv'e been wondering if ironic should read the network data and re-write given we have a better view of hardware reality | 17:56 |
JayF | Ironic driver writes network_data.json in nova | 17:56 |
JayF | it's already an "ironic" codepath | 17:56 |
JayF | in fact it's extra-gross because metadata service returns different network data than Ironic | 17:57 |
mnaser | ok I can reproduce this with an external piece of code | 17:57 |
mnaser | https://paste.opendev.org/show/bgRiGWDtstBplwPVJ4pd/ gives me no bond_links | 17:58 |
mnaser | time to dive deeper :) | 17:58 |
JayF | one thing to ponder on | 17:59 |
JayF | and I think this may be the issue | 17:59 |
JayF | I *think* we get a generator of sorts from the SDK | 17:59 |
JayF | and if we iterate ports more than once I suspect it may be empty the second time thru | 17:59 |
JayF | I'm very curious if you did list() around the initial ports / port_groups fetch if it'd make this issue disappear | 17:59 |
mnaser | lol thats actually what's happening | 18:00 |
mnaser | thats exactly it | 18:00 |
JayF | hey, I do know how this code works \o/ | 18:00 |
JayF | lol | 18:00 |
mnaser | should I wrap both with a list | 18:00 |
JayF | I would, yeah. | 18:00 |
JayF | Tradeoff of ram for correctness is not terrible | 18:01 |
JayF | ports/port_groups for a node are phyiscally constrained | 18:01 |
JayF | I'm about to step away for a bit, but if you wanna patch this awesome; it'd be great to have a unit test that breaks before the fix, too. If you go this route, link me the patch and I will solicit for reviews. | 18:03 |
mnaser | yeah I'll push it real quick now | 18:03 |
JayF | Otherwise, file a bug and I'd consider it a very high priority to fix | 18:03 |
mnaser | we've been blocked at this for 2 days now | 18:03 |
mnaser | :'( | 18:03 |
JayF | sorry :( | 18:03 |
JayF | Feel free to rope in upstream sooner next time | 18:03 |
JayF | and very explicitly: thank you for finding this, it would've bitten my downstream when we bumped up our ironic driver/ironic version to caracal in a few months | 18:04 |
mnaser | https://bugs.launchpad.net/ironic/+bug/2071972 | 18:06 |
mnaser | patch incoming | 18:06 |
mnaser | JayF: https://review.opendev.org/c/openstack/nova/+/923530 Fix port group network metadata generation | 18:11 |
mnaser | gonna cherry pick it locally and see how it goes | 18:11 |
opendevreview | Merged openstack/bifrost master: Remove CentOS Stream 8 leftovers https://review.opendev.org/c/openstack/bifrost/+/922789 | 18:22 |
cid | o/ | 20:07 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!