| opendevreview | Cyril Roelandt proposed openstack/ironic-prometheus-exporter master: Zuul: do not use USE_PYTHON3 https://review.opendev.org/c/openstack/ironic-prometheus-exporter/+/953498 | 00:37 |
|---|---|---|
| opendevreview | Syed Haseeb Ahmed proposed openstack/ironic master: Add interface name to inventory for inspection hooks https://review.opendev.org/c/openstack/ironic/+/953414 | 04:11 |
| opendevreview | Syed Haseeb Ahmed proposed openstack/ironic master: Add interface name to inventory for inspection hooks https://review.opendev.org/c/openstack/ironic/+/953414 | 04:35 |
| opendevreview | Syed Haseeb Ahmed proposed openstack/ironic master: Add interface name to inventory for inspection hooks https://review.opendev.org/c/openstack/ironic/+/953414 | 04:40 |
| cardoe | https://bugs.launchpad.net/ironic/+bug/2115471 is the bug I made. Haseeb’s patch above is related. | 05:33 |
| opendevreview | cid proposed openstack/ironic master: Fix inspection rules validation https://review.opendev.org/c/openstack/ironic/+/953464 | 05:53 |
| opendevreview | Syed Haseeb Ahmed proposed openstack/ironic master: Add interface name to inventory for inspection hooks https://review.opendev.org/c/openstack/ironic/+/953414 | 06:27 |
| rpittau | good morning ironic! happy friday! o/ | 07:09 |
| abongale | good morning ironic! | 08:44 |
| *** darmach2 is now known as darmach | 11:33 | |
| TheJulia | good morning | 13:58 |
| TheJulia | cid: I took a look at https://review.opendev.org/c/openstack/ironic/+/953464 and I concur, its really a bugfix in my mind | 14:00 |
| cid | TheJulia, thanks a lot, \o/ | 14:02 |
| iurygregory | good morning | 14:05 |
| iurygregory | hey TheJulia tks for the review in https://review.opendev.org/c/openstack/ironic/+/953395 o/ | 14:06 |
| iurygregory | I'm planning on adding the unit tests, was just waiting for some feedback to see if we will go with this approach or keep thing as it is and fix in IPE https://review.opendev.org/c/openstack/ironic-prometheus-exporter/+/952249 | 14:07 |
| dtantsur | it definitely should not be fixed in every possible consumer of our notifications | 14:24 |
| dtantsur | also see my comments | 14:24 |
| opendevreview | Julia Kreger proposed openstack/ironic master: trivial: note ipmitool code which can be removed with eventlet https://review.opendev.org/c/openstack/ironic/+/953579 | 14:26 |
| iurygregory | dtantsur, I don't think it has to do with oslo, but on how _sensor2dict was implemented | 14:31 |
| dtantsur | I mean, both probably? Otherwise, where is this format coming from at all? | 14:36 |
| iurygregory | too many layers :D , well the fix can be in the ironic code at least | 14:42 |
| opendevreview | Takashi Kajinami proposed openstack/ironic-tempest-plugin master: Replace deprecated assertItemsEqual https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/953589 | 14:47 |
| cardoe | dtantsur / TheJulia: https://bugs.launchpad.net/ironic/+bug/2115471 there's the bug for my IRC rant from yesterday | 15:04 |
| cardoe | I'd appreciate your feedback on my approach. | 15:06 |
| cardoe | cid: I reviewed yours my only feedback is add a release note as a bugfix. | 15:06 |
| cid | cardoe, ++, tks. | 15:07 |
| TheJulia | iurygregory: the approach seems reasonable to me, fwiw | 15:07 |
| iurygregory | TheJulia, ack | 15:07 |
| iurygregory | dtantsur, you are right, something turns the enum representation 'health': <Health.OK: 'OK'> into 'health': {'_value_': 'OK', '_name_': 'OK', '__objclass__': "<enum 'Health'>"}, the fix in sensor2dict avoid us sending the enum representation so we don't end up with the crazy output | 15:09 |
| iurygregory | (did a quick test using the data from sushy and the logic we have in ironic) https://paste.opendev.org/show/bNMtYmnW59SisOiHMFer/ | 15:11 |
| TheJulia | I guess that is why it might be helpful to have a unit test, helps folks looking at it grasp the constraints of it | 15:11 |
| iurygregory | yeah, seems like when we added this part of code 6yrs ago we didn't add unit tests to _sensor2dict | 15:16 |
| dtantsur | iurygregory: okay, then it's important to note that enums don't have _value_, they have just value | 15:42 |
| iurygregory | ack, I'm updating things now | 15:43 |
| opendevreview | cid proposed openstack/ironic master: Log executed steps during cleaning/servicing/deploy https://review.opendev.org/c/openstack/ironic/+/952637 | 16:22 |
| opendevreview | cid proposed openstack/ironic master: Log executed steps during cleaning/servicing/deploy https://review.opendev.org/c/openstack/ironic/+/952637 | 16:26 |
| opendevreview | cid proposed openstack/ironic master: Fix inspection rules validation https://review.opendev.org/c/openstack/ironic/+/953464 | 16:32 |
| dtantsur | cid: btw, with the migration to cheroot, we might have lost the pretty convenient logging of every request | 16:43 |
| TheJulia | yeouch | 16:44 |
| TheJulia | YEOUCH :( | 16:44 |
| * TheJulia cries | 16:44 | |
| dtantsur | don't cry TheJulia, it's Friday, you will be okay | 16:45 |
| dtantsur | iurygregory: I've noticed that BMO does regular requests to /v1/nodes/<node>/firmware | 17:09 |
| dtantsur | I hope this API is not synchronous.. but it might | 17:09 |
| dtantsur | in any case, Ironic does not update the components frequently, I think we need to fix BMO to not request it so often either | 17:10 |
| iurygregory | =O | 17:10 |
| iurygregory | maybe the logic on HFC controller? trying to pull information till is available | 17:11 |
| iurygregory | since is automatically created the HFC, and we need to show the information in status, maybe we can change some logic ? | 17:12 |
| dtantsur | iurygregory: yeah, maybe we only refresh information on certain transitions? | 17:12 |
| dtantsur | do you remember if the API is sync or async in Ironic? | 17:12 |
| dtantsur | I initially thought it was synchronous, but now I think we return cached information from the DB | 17:13 |
| iurygregory | should be async | 17:13 |
| dtantsur | which is not THAT bad (although BMO does hammer Ironic quite a lot) | 17:13 |
| iurygregory | yeah, we return info from the DB | 17:13 |
| dtantsur | okay, it's not too horrible | 17:15 |
| * dtantsur is looking for a source of sudden slowness in one of the BMO actions | 17:15 | |
| dtantsur | https://kubernetes.slack.com/archives/CHD49TLE7/p1750940309756449 if you want to follow along | 17:16 |
| iurygregory | tks | 17:18 |
| -opendevstatus- NOTICE: Gerrit is being restarted to pick up a configuration change. You may notice a short outage. | 17:36 | |
| opendevreview | Verification of a change to openstack/ironic master failed: fix: handle unexpected 'loop' in actions field https://review.opendev.org/c/openstack/ironic/+/953323 | 17:48 |
| opendevreview | Iury Gregory Melo Ferreira proposed openstack/ironic master: Fix Sensor Data values https://review.opendev.org/c/openstack/ironic/+/953395 | 19:25 |
| iurygregory | done :D | 19:25 |
| iurygregory | have a great weekend ironic o/ | 19:36 |
| opendevreview | Julia Kreger proposed openstack/ironic master: trivial: follow-up on I07172e48207e09c0858298e34eea038c776d3c74 https://review.opendev.org/c/openstack/ironic/+/953682 | 22:08 |
| TheJulia | cid: ^^^ just following up on your change | 22:09 |
| cid | \o/ | 22:16 |
| TheJulia | cid: I'm staring to wonder if we have some issues around the DB we're also going to need to fix | 22:34 |
| TheJulia | or if it is, much as you highlighted in your change, the keepalive stuff triggering excess database interaction | 22:35 |
| TheJulia | https://review.opendev.org/c/openstack/ironic/+/952939 | 22:35 |
| TheJulia | Getting a local test machine with your change but with eventlet to get a running conductor to at least... poke and prod | 22:35 |
| cid | Yeah. If that sideeffect background thread might be problematic. | 22:36 |
| TheJulia | at least if the logs from the job logs I looked at are any indication, things halted as soon as we touched the database | 22:37 |
| TheJulia | so.... hopefully it is just one thing | 22:37 |
| TheJulia | and hopefully its pretty clearly contrasting once we find it | 22:37 |
| cid | Hopefully! | 22:38 |
| TheJulia | https://www.irccloud.com/pastebin/AHfrgInN/ | 22:39 |
| TheJulia | hehehe | 22:39 |
| cid | That looks like some deadlocks is going on. | 22:39 |
| TheJulia | yup | 22:39 |
| cid | I just know there'll be a few testing to be done. I will get the new devstack Jay left me and try my hands on some poking myself as well in the coming week. | 22:39 |
| TheJulia | yeah, oh.. hmmmmmm | 22:41 |
| cid | It never successfully loaded the 'sqlalchemy' backend. | 22:42 |
| TheJulia | I'm not even sure it is successfully launching | 22:42 |
| TheJulia | at all | 22:42 |
| cid | The conductor,, that is? | 22:43 |
| TheJulia | yeah | 22:43 |
| TheJulia | checking to see if service.launch is doing anything | 22:43 |
| TheJulia | so, we're still heading down the eventlet service launcher route even without the monkeypatch | 22:44 |
| TheJulia | so we're falling into a bad default state, most likely | 22:44 |
| TheJulia | or we need to change the invocation | 22:44 |
| cid | Oh, wow, heh | 22:44 |
| TheJulia | i added LOG.debug(launcher) in the conductor.py file | 22:44 |
| TheJulia | DEBUG ironic.command.conductor [-] <oslo_service.backend.eventlet.service.ServiceLauncher object at 0x71c77f3f1820> {{(pid=632709) main /opt/stack/ironic/ironic/command/conductor.py:106}} | 22:45 |
| opendevreview | Merged openstack/ironic master: trivial: note ipmitool code which can be removed with eventlet https://review.opendev.org/c/openstack/ironic/+/953579 | 22:46 |
| TheJulia | it at least explains why we're deadlocking | 22:50 |
| cid | And that's progress :) | 22:50 |
| TheJulia | okay, oslo.service, by default, routes you down the eventlet path in 4.1.1 which is what I end upw ith by default | 22:51 |
| TheJulia | unfortunately it looks like the olso.service docs got ignored when it comes to eventlet stuffs | 22:54 |
| TheJulia | i see it | 22:56 |
| cid | The docs or the bug :D | 22:58 |
| TheJulia | yes ? :) | 22:59 |
| TheJulia | so it looks like we explicitly need to call init_backend | 22:59 |
| TheJulia | Except: oslo_service.backend.exceptions.BackendAlreadySelected: The 'eventlet' backend is already set up | 22:59 |
| TheJulia | so just like eventlet, we need to do it super early | 23:02 |
| TheJulia | because importing oslo.service explicitly calls it | 23:02 |
| TheJulia | and sets a backend | 23:02 |
| cid | That makes sense. So, init the new backend before any oslo_service.service imports. | 23:03 |
| TheJulia | thats what it looks like, albeit its not the happiest set of code either when I try it | 23:04 |
| TheJulia | so I'm thinking the pattern is something like from oslo_service import backend and then backend.init_backend("threading"), but no dice so far | 23:05 |
| TheJulia | so we need to move up our oslo.service version | 23:16 |
| TheJulia | cid, I think we need to split your patch into two changes, keep __init__.py the same for now | 23:17 |
| TheJulia | and then continue to work the removal of eventlet as a single patch | 23:18 |
| TheJulia | i figured out the pattern to register the default as threading, but that requires oslo.service 4.2.1 | 23:19 |
| TheJulia | now... because it is a thread, I *suspect* our wait logic might also need to go | 23:19 |
| TheJulia | oh yeah | 23:20 |
| TheJulia | we're going to need to do more work | 23:21 |
| TheJulia | yeah, our launcher needs slight retooling | 23:21 |
| TheJulia | nothing major, the next step is some of the way we have signal handling | 23:21 |
| TheJulia | https://www.irccloud.com/pastebin/7FhCI6LB/ | 23:22 |
| TheJulia | lets sync up next week, and maybe see about starting to shuffle some of it around | 23:22 |
| TheJulia | I'm going to rev your change in the mean time | 23:23 |
| opendevreview | cid proposed openstack/ironic master: WIP: Migrate conductor threads to a thread pool https://review.opendev.org/c/openstack/ironic/+/952939 | 23:23 |
| cid | ++, Alright. I will ping you next week. | 23:24 |
| cid | re: "...I'm going to rev your change in the mean time" Sure. | 23:24 |
| * cid Goes to get some sleep | 23:25 | |
| cid | Enjoy your weekend, TheJulia, ironic \o/ | 23:25 |
| opendevreview | Julia Kreger proposed openstack/ironic master: WIP: Migrate conductor threads to a thread pool https://review.opendev.org/c/openstack/ironic/+/952939 | 23:27 |
| opendevreview | Julia Kreger proposed openstack/ironic master: WIP: Set a threading backend https://review.opendev.org/c/openstack/ironic/+/953683 | 23:27 |
| TheJulia | stevebaker[m]: fyi, you might care about ^^^, we're going to have to move around signal handling stuffs | 23:32 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!