| opendevreview | Merged openstack/ironic master: Add a conductor liveness check utility script https://review.opendev.org/c/openstack/ironic/+/969320 | 02:04 |
|---|---|---|
| *** hroy_ is now known as hroy | 02:36 | |
| opendevreview | nidhi proposed openstack/ironic master: Add Redfish LLDP data collection support to the Redfish inspection interface. https://review.opendev.org/c/openstack/ironic/+/967841 | 06:03 |
| opendevreview | nidhi proposed openstack/ironic master: Add Redfish LLDP data collection support to the Redfish inspection interface. https://review.opendev.org/c/openstack/ironic/+/967841 | 06:06 |
| opendevreview | nidhi proposed openstack/ironic master: Add Redfish LLDP data collection support to the Redfish inspection interface. https://review.opendev.org/c/openstack/ironic/+/967841 | 06:08 |
| opendevreview | nidhi proposed openstack/ironic master: Add PCIe function fields to redfish inspection https://review.opendev.org/c/openstack/ironic/+/963179 | 06:23 |
| rpittau | good morning ironic! happy friday ! o/ | 07:54 |
| *** mdfr1 is now known as mdfr | 07:58 | |
| opendevreview | Riccardo Pittau proposed openstack/bifrost master: Switch all CI jobs to debian based IPA https://review.opendev.org/c/openstack/bifrost/+/969886 | 10:09 |
| opendevreview | Allain Legacy proposed openstack/ironic master: Add generic switch driver support https://review.opendev.org/c/openstack/ironic/+/966469 | 13:38 |
| opendevreview | Allain Legacy proposed openstack/ironic master: Add ironic-networking network interface https://review.opendev.org/c/openstack/ironic/+/966470 | 13:38 |
| opendevreview | Allain Legacy proposed openstack/ironic master: Add standalone networking service installation guide https://review.opendev.org/c/openstack/ironic/+/966471 | 13:38 |
| opendevreview | Allain Legacy proposed openstack/ironic master: Improve exception handling in switch driver factory https://review.opendev.org/c/openstack/ironic/+/969852 | 13:38 |
| TheJulia | good morning | 14:05 |
| cardoe | is it Friday already? rewind the clock a little bit and give me some time back. | 14:30 |
| TheJulia | it is | 14:38 |
| TheJulia | cid: I've been looking at https://review.opendev.org/c/openstack/ironic-python-agent/+/958333 and I think ruff stuggled with out config help text formatting, take a look. Don't revise it as-is if we can get away with it (I'm still reviewing it), but I think a follow-up might be fine | 14:39 |
| cardoe | So one of the patches I was thinking of suggesting was pinning ruff to use the oldest version of Python we'd support in a backport. Otherwise it defaults to what the project uses, which today is 3.10 for all our stuff. | 14:40 |
| TheJulia | cardoe: so, cid did that for targetting 3.7 on his IPA change, so I don't think its an bad idea, just going to make backproting/work processes a bit painful. Easier to do with IPA because the number of outstanding patches there is always a bit less. | 14:45 |
| cardoe | ah cool | 14:48 |
| cid | \o/, tks, TheJulia for the review | 14:49 |
| TheJulia | cid: I'm maybe 30% through at this point, but I did spot the weirdness so wanted to highlight it | 14:50 |
| cid | ++. Yeah. I think if we consider this fit to merge, a follow-up of almost this size might also be need. | 14:52 |
| cardoe | I'll review it this weekend if it doesn't get closed. | 14:53 |
| cid | In that patch, I think we can upgrade the python version to 310(?) | 14:54 |
| cid | cardoe, I think it can use as much eyeballs as it can get. Since whatever we decide is the path will apply to other projects in Ironic. | 14:55 |
| cardoe | So I've been using py38 since 2024.2 supported that. | 14:55 |
| cardoe | I've done something similar to what you've done to sushy. | 14:56 |
| TheJulia | I think 3.7 is a good starting target at least for now, and we can always increment, that first hurdle is definitely a big one | 14:56 |
| TheJulia | I've definitely had a few raised eyebrows on the result, but nothing beyond the config help strings in internal worry | 14:57 |
| cardoe | My team has been using a bunch of +30k/-30k patches to a few openstack repos as a basis for some of our development cause docs aren't the best for some things. But that's outside of ironic as well. | 14:57 |
| cardoe | like our network stuff in neutron, neutron-lib defines some ABCs that are suppose to be the basis for plugin interfaces but then the tests make objects to test with and don't inherit from those ABCs. | 14:59 |
| cardoe | or cinder's interfaces are pretty much my_func(*args, **kwargs) but specific named parameters | 15:00 |
| TheJulia | to be honest, if/when we do something like this to the ironic repo, move strings to double quotes in a separate change. | 15:27 |
| *** darmach9 is now known as darmach | 16:04 | |
| opendevreview | Merged openstack/ironic master: Drop xinetd/tftpd on CentOS 9 https://review.opendev.org/c/openstack/ironic/+/964449 | 16:30 |
| opendevreview | Merged openstack/ironic master: Switch graphical console image to ubuntu by default https://review.opendev.org/c/openstack/ironic/+/964201 | 16:38 |
| opendevreview | Merged openstack/ironic master: Add a kubernetes provider for console container https://review.opendev.org/c/openstack/ironic/+/962242 | 16:38 |
| opendevreview | Jay Faulkner proposed openstack/ironic-python-agent master: Add request logging for API requests https://review.opendev.org/c/openstack/ironic-python-agent/+/958307 | 17:06 |
| opendevreview | Allain Legacy proposed openstack/bifrost master: Add support for standalone ironic networking https://review.opendev.org/c/openstack/bifrost/+/962394 | 17:07 |
| opendevreview | Doug Goldstein proposed openstack/ironic stable/2025.1: fix: ensure that the bootloader path exists https://review.opendev.org/c/openstack/ironic/+/969122 | 17:47 |
| opendevreview | Doug Goldstein proposed openstack/ironic stable/2024.2: fix: ensure that the bootloader path exists https://review.opendev.org/c/openstack/ironic/+/969123 | 17:47 |
| opendevreview | Doug Goldstein proposed openstack/ironic bugfix/31.0: fix: ensure that the bootloader path exists https://review.opendev.org/c/openstack/ironic/+/969124 | 17:48 |
| *** dking is now known as Guest32984 | 18:25 | |
| *** Guest32984 is now known as dking | 18:26 | |
| alegacy | is looking at task.node.inspection_finished_at a reliable/acceptable way to know if inspection has completed? | 18:42 |
| dking | dtantsur: Would you happen to be in for some IrSO questions? | 19:01 |
| TheJulia | alegacy: the provision_state, ideally | 19:09 |
| alegacy | TheJulia: IIUC, the provision_state would say whether inspection is currently happening, but to know if inspection had happened earlier (while the node is now in a different provision_state) is t.d.inspection_finished_at a reliable method or something else? | 19:16 |
| alegacy | I'm asking in the context of that TODO you commented on in: https://review.opendev.org/c/openstack/ironic/+/966470 | 19:17 |
| opendevreview | Merged openstack/ironic stable/2025.2: fix: ensure that the bootloader path exists https://review.opendev.org/c/openstack/ironic/+/969120 | 19:18 |
| JayF | alegacy: we should *not* assume that inspection has happened or needs to happen though, right? | 19:37 |
| JayF | alegacy: it's about if the info is there or not -- my use cases for this code we likely will try not to enable inspection | 19:38 |
| JayF | Inspection is awesome, but leads to Ironic being less consistently behaving when getting machines that aren't following the Redfish standard in the way we expect. | 19:38 |
| alegacy | JayF: we need local_link_connection info which comes from inspection (unless someone wants to add that manually I suppose, but I don't recommend) | 19:38 |
| JayF | Lots of people populate that manually, fwiw | 19:39 |
| alegacy | code currently just puts out a log, but the comment was to upgrade that to an error | 19:39 |
| alegacy | since that code needs to work before and after inspection I thought to try and differentiate those 2 cases | 19:39 |
| alegacy | JayF: so for your purposes you'd rather it just remain as a log. | 19:40 |
| JayF | I'm just saying, generally, please don't accidentally require inspection to use standalone networking :) | 19:40 |
| alegacy | here: https://review.opendev.org/c/openstack/ironic/+/966470/comment/dcc8a59d_d9b7fb1e/ | 19:40 |
| JayF | is that code reachable outside of a state change? | 19:41 |
| JayF | like I would expect a failure like that to cause something to go into [something]FAIL | 19:41 |
| JayF | put something into node.last_error, node history, etc | 19:41 |
| JayF | I think raising an exception there might just get that for you for free, but imbw | 19:41 |
| TheJulia | Yeah, operators can totally do supply the local link information, so if it is not set then raising an exception is the appropriate path | 19:43 |
| TheJulia | guardrailing and doing extra checks if inspection has been done ... not so much in large part because it is optional. | 19:43 |
| alegacy | the problem though is that for those wishing to use inspection, it is possible that the extra.switchport is set before inspection actually occurs therefore raising an exception there would raise an error and inspection wouldn't work. | 19:44 |
| alegacy | if both inspection and manually provisioning of local_link_connection is possible then I think leaving this as a log is probably the best that we can do. | 19:45 |
| TheJulia | I mean, ideally local_link_information gets reset and move on, not try to raise an error because folks do re-plug things allt he time | 19:45 |
| alegacy | if they are relying on inspection and things get replugged then re-inspection would be necessary | 19:46 |
| TheJulia | not necessarilly | 19:46 |
| TheJulia | because operationally, if you know the port you move it to, you just reset the value | 19:46 |
| alegacy | then we'll configure the wrong port! | 19:46 |
| alegacy | ok, that assumes they'll manage that manually I guess. | 19:47 |
| TheJulia | some do | 19:47 |
| TheJulia | Every team out there takes the path they are most comfortable with | 19:47 |
| TheJulia | Even if it is a good, or bad idea :) | 19:47 |
| alegacy | ok, I'll sleep on this, but I think a log is the best we can do here. I don't see how to structure this to raise an error and still have initial inspection work if port.extra.switchport is already set. | 19:52 |
| JayF | If initial inspection is running; shouldn't provision_state or target_provision_state reflect it? | 20:10 |
| alegacy | i could try looking at the provisioning_state to be more permissive if inspection is happening now... would need to make sure state transitions before the initial inspection would still be ok. | 20:16 |
| JayF | It seems like we should only run that code when we're trying to swap a network interface, which no state change should be doing pre-inspection in your use case. | 20:21 |
| JayF | at least AIUI | 20:21 |
| alegacy | JayF: TheJulia: thanks, will give that a go. | 20:23 |
| alegacy | ah yes, now I remember why I put that in as a log! ... the default bifrost workflow tries to go straight to available without passing thru inspection and part of that is to do a clean which fails this check. | 20:42 |
| alegacy | i'd have to change the underlying openstack cloud ansible module to allow it to stop at manageable and let the user do the inspection first. | 20:43 |
| alegacy | or change bifrost to manually populate local_link_connection, i suppose | 20:43 |
| TheJulia | JayF: yes, provision state should indicate it. | 20:44 |
| TheJulia | +1 to populate port data | 20:44 |
| TheJulia | granted, thats a bit of a longer walk | 20:45 |
| TheJulia | the "action of last resort" which many folks do too quickly is force delete and recreate a node in ironic | 20:45 |
| alegacy | you mean, populate local_link_connection along with port.extra.switchport in bifrost | 20:45 |
| TheJulia | and often when one does that, they have everything they can populate back as well | 20:45 |
| alegacy | in the enroll step | 20:46 |
| TheJulia | yes | 20:46 |
| TheJulia | yup | 20:46 |
| alegacy | k, in _theory_ that shouldn't be too difficult... but not a Friday afternoon gig. ;) | 20:47 |
| TheJulia | oh, yeah, definitely | 20:52 |
| TheJulia | have a great weekend, I likely won't be around next week | 20:52 |
| TheJulia | but if anyone needs me, ping me | 20:52 |
| alegacy | TheJulia: thanks, enjoy your time off | 21:06 |
| cardoe | Enjoy the time off | 21:16 |
| cardoe | JayF: alegacy: I do think we can enhance the state machine diagram page a little bit with some user story type stuff. | 21:19 |
| cardoe | But I feel like that about a lot of the Ironic docs. They're very factual. "Here is an API you can call. Here is another one." | 21:20 |
| cardoe | It would be more of a paint a picture. | 21:20 |
| opendevreview | Julia Kreger proposed openstack/ironic master: OCI Basic Auth/Tag access https://review.opendev.org/c/openstack/ironic/+/969314 | 21:35 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!