opendevreview | Merged openstack/ironic-python-agent-builder stable/xena: Add checksum generation support https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/892968 | 00:03 |
---|---|---|
opendevreview | Merged openstack/ironic master: Utilize the JSON-RPC port https://review.opendev.org/c/openstack/ironic/+/879215 | 04:45 |
mnasiadka | mmalchuk: I don’t think https://bugs.launchpad.net/bugs/2033528 should be marked invalid - the bug is valid, implementation should be discussed in Gerrit | 06:50 |
mnasiadka | JayF: ^^ sorry to complain, but that’s a bit unwelcoming to mark bug as invalid based on patch content ;-) | 06:54 |
rpittau | good morning ironic! o/ | 07:27 |
rpittau | JayF: you anticipated me by a jiff for the ansible 8 test on bifrost :) | 07:28 |
opendevreview | Verification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm https://review.opendev.org/c/openstack/ironic/+/893225 | 08:53 |
opendevreview | Verification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm https://review.opendev.org/c/openstack/ironic/+/893225 | 09:33 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 10:24 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 10:50 |
iurygregory | good morning Ironic | 11:45 |
opendevreview | Merged openstack/ironic master: Add missing release mappings for 22.0 and 22.1 https://review.opendev.org/c/openstack/ironic/+/893232 | 12:06 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 12:21 |
opendevreview | Verification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm https://review.opendev.org/c/openstack/ironic/+/893225 | 12:39 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 12:44 |
opendevreview | Julia Kreger proposed openstack/ironic master: DNM: Test grub+ovn-dhcp-opt-210 https://review.opendev.org/c/openstack/ironic/+/893287 | 13:11 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 13:20 |
opendevreview | Verification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm https://review.opendev.org/c/openstack/ironic/+/893225 | 13:20 |
iurygregory | ok, I think I have some idea about what is wrong in the python-ironic-inspector-client | 13:33 |
iurygregory | but tbh I have no idea on how to fix LOL | 13:33 |
iurygregory | maybe is a matter of tearDown of other tests or the order tests run | 13:35 |
iurygregory | I've attempted to change the node_uuid (we use self.node_uuid) but if I add a random uuid there the test will be green, maybe is a matter that other test executed introspection and when we attempt to abort in the test it just works | 13:36 |
iurygregory | functional-py310: OK (50.70=setup[6.16]+cmd[44.55] seconds) | 13:36 |
iurygregory | congratulations :) (50.83 seconds) | 13:36 |
iurygregory | thoughts? | 13:36 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent master: Add get_service_steps logic to the agent https://review.opendev.org/c/openstack/ironic-python-agent/+/890864 | 13:37 |
opendevreview | Julia Kreger proposed openstack/ironic-python-agent master: Add mlnx deploy_step entry to enable deploy time firmware https://review.opendev.org/c/openstack/ironic-python-agent/+/893371 | 13:37 |
iurygregory | ops, not self.node_uuid, but self.uuid | 13:37 |
opendevreview | Iury Gregory Melo Ferreira proposed openstack/python-ironic-inspector-client master: [WIP] Test Fix https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/891976 | 13:41 |
iurygregory | let's see how CI will react to it... | 13:41 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 13:49 |
TheJulia | iurygregory: you got functional to pass, I think that is a good sigh in general | 13:53 |
iurygregory | yeah | 13:54 |
iurygregory | zuul gave +1, so I think it's probably something in the tearDown or the order we run the tests, maybe the "node" was inspected already | 13:57 |
opendevreview | Rafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost https://review.opendev.org/c/openstack/bifrost/+/893195 | 14:00 |
JayF | iurygregory: so https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/891976 is good then? yeah? You wanna un-WIP it? | 14:07 |
iurygregory | JayF, so I consider this a workaround, I probably don't have enough time to figure out why it fails when we use self.uuid | 14:10 |
iurygregory | if we are ok to have this I can un-WIP and add some Notes in the patch | 14:10 |
JayF | lets put an explicit TODO to fix that and not drop the task overall | 14:10 |
JayF | but we need passing tests here to get a release in | 14:10 |
iurygregory | ack | 14:10 |
iurygregory | let me update the patch | 14:10 |
JayF | ty | 14:11 |
iurygregory | want me to add a more fancy way to generate the uuid or we should keep hardcoded? | 14:13 |
opendevreview | Iury Gregory Melo Ferreira proposed openstack/python-ironic-inspector-client master: Fix Gate https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/891976 | 14:20 |
opendevreview | Julia Kreger proposed openstack/ironic master: log the version of the conductor starting https://review.opendev.org/c/openstack/ironic/+/893379 | 14:22 |
opendevreview | Julia Kreger proposed openstack/ironic master: log the version of the conductor starting https://review.opendev.org/c/openstack/ironic/+/893379 | 14:22 |
rpittau | iurygregory: good finding! I think the problem is in the teardown, it does not regenerate the id of the node and so the abort link is there, this happens also in inspector unit tests, if we try to execute the abort operation (currently we don't in CI, but I tested it) | 14:23 |
iurygregory | rpittau, oh nice! | 14:24 |
JayF | I just realized the work to shardify our networking-baremetal library got dropped (by me) this cycle | 14:25 |
opendevreview | Merged openstack/ironic-python-agent master: preserve/handle config drives on 4k block devices https://review.opendev.org/c/openstack/ironic-python-agent/+/888794 | 14:30 |
iurygregory | JayF, TheJulia the patch is green :D | 14:40 |
iurygregory | JayF, I saw we probably want https://review.opendev.org/c/openstack/python-ironicclient/+/891140 so we can cut the release | 14:41 |
iurygregory | TheJulia, if you don't have time I can solve the merge conflict on it ^ | 14:42 |
JayF | https://review.opendev.org/c/openstack/ironic/+/891630 hasn't landed yet | 14:42 |
JayF | which I guess the API still might exist? | 14:42 |
iurygregory | oh =( | 14:42 |
JayF | I think the client for service steps might miss | 14:42 |
TheJulia | yeah, it misses most likely | 14:42 |
JayF | not ideal but if you're a clever enough deployer to use service steps, you can upgrade to a newer client | 14:42 |
TheJulia | given it should have released last week | 14:42 |
JayF | I'll go poke the releases pr | 14:42 |
TheJulia | most are | 14:42 |
TheJulia | weird, I thoguth i fixed the docs for that | 14:44 |
iurygregory | JayF, we need to update the hash :D | 14:44 |
TheJulia | api exists, but for all that *really* matters that can merge next week and be in our final release for the cycle | 14:44 |
JayF | yeah so I gotta wait for gate to hit | 14:44 |
JayF | yeah | 14:44 |
iurygregory | I'm on it https://review.opendev.org/c/openstack/releases/+/892945/1/deliverables/bobcat/python-ironicclient.yaml | 14:44 |
* TheJulia burries self back in the power sync loop for a little bit | 14:44 | |
JayF | you have that sha even before it lands? | 14:44 |
JayF | oh, ironicclient | 14:45 |
JayF | got it | 14:45 |
JayF | thanks | 14:45 |
iurygregory | it will need your +1 again =) | 14:46 |
JayF | or any release manager | 14:46 |
JayF | I thought you were on that list but maybe not | 14:46 |
JayF | yeah you are release manager | 14:47 |
JayF | your +1 should set PTL-Approved +1 | 14:47 |
iurygregory | since I pushed I'm not sure I can +1 (but I will do) | 14:47 |
JayF | if you pushed it might even be automatic | 14:50 |
JayF | I did +1 tho | 14:50 |
JayF | iurygregory: yeah looking at the sequence, you as release manager pushing a change got us +1 PTL Approved, then after that I +1'd | 14:56 |
iurygregory | ack | 14:57 |
rpittau | bye everyone, see you 3 weeks! o/ | 15:01 |
JayF | o/ | 15:16 |
TheJulia | 3 weeks, hell, I need more pto | 15:16 |
iurygregory | JayF, monday is holiday in the US right? | 15:30 |
JayF | yes, and you volunteered to run the meeting :) | 15:31 |
iurygregory | now I have the feeling we should probably cancel the meeting, rpittau will be out, you and TheJulia and Dmitry (we probably won't have quorum...) | 15:31 |
iurygregory | I think I will be alone in the channel LOL | 15:32 |
JayF | I delegated meeting chair to you, I have no objection if you wanna cancel it, just make sure to update the agenda and email the list | 15:32 |
JayF | I'm also OK if you wanna do startmeeting -> anyone here? -> no quorum -> end :D | 15:32 |
iurygregory | oh good =) | 15:32 |
iurygregory | I will do this | 15:32 |
opendevreview | Merged openstack/ironic master: Correct bindep.txt entries for bookworm https://review.opendev.org/c/openstack/ironic/+/893225 | 16:12 |
JayF | So I was talking to johnthetubaguy about my change to bring automatic_lessee style behavior to Nova integrated use cases ( https://review.opendev.org/c/openstack/nova/+/888290 ) and we're trying to think of a good reason that setting lessee should not be the default nova behavior | 16:31 |
JayF | TheJulia: iurygregory ^ curious if you have thoughts on this | 16:32 |
TheJulia | it may already be used or manually managed | 16:32 |
TheJulia | so new default behavior that stomps would basically be bad | 16:32 |
johnthetubaguy | (I was thinking as extreme as removing the config option in your patch, so we always set it.) | 16:32 |
* TheJulia doesn't grok the conf opt hate | 16:33 | |
JayF | I think TheJulia might be thinking there are cases where the person leasing the node from Ironic could be then plugging those leased nodes into a nova-compute | 16:33 |
JayF | TheJulia: It was explained to me as Nova dislikes when behavior changes for the user without it being API discoverable | 16:33 |
JayF | wait, I got it | 16:33 |
JayF | TheJulia: johnthetubaguy: Nova always puts a potential_lessee into instance_info | 16:33 |
TheJulia | it is an operator decision to make though based upon their security posture and operating state | 16:33 |
johnthetubaguy | Curiously, I think your patch request might fail if the lessee is already set, becuase its an add, not saying that is a fix, just a thing. | 16:33 |
JayF | if automatic_lessee enabled in Ironic, on the deploy call we set lesseee | 16:33 |
TheJulia | this is acceptable | 16:34 |
JayF | to whatever nova says to set it to | 16:34 |
JayF | johnthetubaguy: would that be acceptable to nova? | 16:34 |
johnthetubaguy | ah... interesting point | 16:34 |
JayF | just give the metadata to Ironic, Ironic can choose the behavior | 16:34 |
TheJulia | ++ | 16:34 |
JayF | and that keeps the docs on the feature identical for Ironic + Nova | 16:34 |
JayF | I really like this as an idea | 16:34 |
johnthetubaguy | do we pass that info already? | 16:35 |
JayF | No | 16:35 |
JayF | basically I'd change my patch to just be "send the user over in a new instance_info field" | 16:35 |
JayF | then a separate Ironic patch would check instance_info on deploy and set the lessee from that if one is provided | 16:35 |
TheJulia | and we can patch ironic to "do the needful" from there | 16:35 |
JayF | well, user, I think it's really project ID | 16:35 |
TheJulia | and then operator's behavior based upon config can take over | 16:35 |
JayF | but the value we want in node.lessee | 16:35 |
johnthetubaguy | yeah, project_id and user_id to match the instance_uuid | 16:36 |
JayF | Now I'll point out | 16:36 |
johnthetubaguy | then Ironic does what it needs to do with that... that works | 16:36 |
JayF | this has the same negative impact that Nova wanted to avoid | 16:36 |
JayF | where the behavior is changed via config and not API discoverable (in Nova's api) | 16:36 |
JayF | in Ironic's api you can just ask for the node and see if you 404 lol | 16:36 |
johnthetubaguy | I am probably over interpreting the concern about API discoverability | 16:38 |
JayF | either way, I like this | 16:38 |
JayF | we're removing a decoder ring | 16:38 |
JayF | no "configure X way if your env is X, configure Y way is the env is Y" | 16:38 |
JayF | which felt gross anyway | 16:38 |
JayF | johnthetubaguy: would this change likely still need a specless bp in nova? | 16:39 |
JayF | nbd if yes, just determining what paperwork I need to do :D | 16:39 |
TheJulia | unrelated: I'm becoming more and more convinced of eventlet weirdness being part of metal3's pain | 16:39 |
johnthetubaguy | Probably, as you its hard to unsee something as a "feature" | 16:40 |
JayF | TheJulia: https://review.opendev.org/c/openstack/ironic/+/887996 I feel pretty strongly that this will improve our eventlet story | 16:40 |
JayF | johnthetubaguy: sure, makes sense. I'm going to write that up soonish to see if we can get consensus before PTG, and if not maybe make it a quick topic | 16:40 |
TheJulia | Approved, see if derek can reproduce this issue I'm looking at tomorrow with it | 16:41 |
JayF | TheJulia: feel free to pull me in if you see anything new/weird. I'd much rather roll-forward this change than roll it back. | 16:42 |
johnthetubaguy | JayF: star, that sounds good. As you mention, it stops the co-dependent configuration problem, which is nice. | 16:42 |
JayF | \o/ thanks for your help | 16:42 |
JayF | and also sharding has a chance! | 16:42 |
JayF | I seriously am going to take a whole day, at least, next week-ish to test sharding and maybe even try to add a tempest test for it | 16:43 |
johnthetubaguy | happy to help, nice to move these bits forward. | 16:43 |
JayF | I also have to keep updating the contributor docs | 16:43 |
JayF | oh yeah, not sure I said this here | 16:43 |
JayF | there's an MLH fellow starting downstream here in around a month | 16:43 |
JayF | who will be focusing on openstack development, first project (with consent of releases team) will be enhancing that automation to understand bugfix releases | 16:44 |
TheJulia | https://docs.sqlalchemy.org/en/20/dialects/sqlite.html#disabling-connection-pooling-for-file-databases | 16:45 |
JayF | that is interesting | 16:47 |
johnthetubaguy | TheJulia: given you mentioned eventlet, in case its a helpful data point, I have in the past hit nonsense where all the DB connections timeout, and http calls fail due to too many eventlet threads thrashing inside ironic-conductor... I tracked down the problem to some python based checksum of images being generated I think (i.e. it didn't yield the thread properly). I think I configured myself out of that whole, and I can't | 16:48 |
johnthetubaguy | quite remember how. | 16:48 |
JayF | that is ... exactly the failure case I mentally modeled could happen without us monkey patching OS | 16:48 |
JayF | things getting hung up on I/O that happens in the os module | 16:48 |
JayF | but that is the #1 place we see most eventlet shenanigans: image streaming | 16:49 |
JayF | that was likely in IPA and we've fixed upsteaam | 16:49 |
TheJulia | gah, unfortunately metal3 is likely using sqlalchemy 1.4 | 16:49 |
JayF | I mean, so are we :) | 16:49 |
JayF | at least for another release, most likely, too | 16:49 |
johnthetubaguy | JayF: oh, I see what you mean, I am not sure what is counted under os=false | 16:50 |
TheJulia | yeah, what is interesting is I suddenly got a database is locked error in logs | 16:50 |
TheJulia | and I've traced things failing prior (and invovled) to a heartbeat being processed, I can see it triggers task.upgrade_lock() | 16:51 |
TheJulia | and... nothing for like 50 seconds | 16:51 |
JayF | johnthetubaguy: one of those cases where the easiest way to grok it is to see it: https://github.com/eventlet/eventlet/blob/master/eventlet/green/os.py | 16:51 |
TheJulia | and to be precise it is task.upgrade_lock(retry=False) | 16:51 |
JayF | oh snap | 16:51 |
TheJulia | so not entirely OS, unless sqlalchemy or pymysql maybe | 16:51 |
JayF | TheJulia: os=false impacts sqlite more b/c it's doing file I/O | 16:52 |
TheJulia | when things *finally* exit, it lis like an exception occurs | 16:52 |
johnthetubaguy | JayF: oh... like wait and reading files, yeah... that could be a problem. | 16:52 |
TheJulia | well, sqlite calls should be native c | 16:52 |
TheJulia | but there could be state/locking in py code | 16:52 |
JayF | I see what you mean | 16:52 |
johnthetubaguy | not using pymysql or whatever the pure python one is? | 16:52 |
JayF | johnthetubaguy: this is for our sqlite support we're talking about | 16:53 |
JayF | johnthetubaguy: working out some DB and file locking issues for our metal3/BMO users over in cncf land | 16:53 |
JayF | johnthetubaguy: one of those things where TheJulia is feeling massive pain cleaning this up, but I think mysql users are going to benefit too because we'll have our locking handled properly | 16:53 |
opendevreview | Julia Kreger proposed openstack/ironic master: Log an exception from heartbeat https://review.opendev.org/c/openstack/ironic/+/893417 | 16:54 |
johnthetubaguy | ah, sorry, I see, sqlite doing native c will frequently be bad with eventlet I guess, eek. | 16:54 |
JayF | well, I certainly hope that's not the problem or else we might be extra-screwed lol | 16:55 |
JayF | since we can't really fix that at a base level if it's the problem | 16:55 |
TheJulia | it was all good with sqlalchemy 1.3 when we had autocommit | 16:55 |
TheJulia | things would just work | 16:55 |
johnthetubaguy | ah nasty, just as well its 6pm here... as an excuse to bravely run away (shudders) | 16:59 |
TheJulia | heh | 17:00 |
TheJulia | goodnight | 17:00 |
JayF | johnthetubaguy: I will note, CI failed on some of those sharding patches :) | 17:00 |
JayF | johnthetubaguy: but people are +2'ing them anyway, I assume they have a better handle than me on what is real vs noise | 17:00 |
johnthetubaguy | that first one has some unrelated py38 specific timestamp nonsense that I hoped someone else would tell me got fixed already, I will check the others | 17:01 |
JayF | thank you :) if you wanna be involved in the shard testing party I'm throwing next week ,or have a specific corner case you wanna cover lmk | 17:02 |
JayF | hopefully we won't have RC patches on it but if we need them we'll find it and get em | 17:02 |
johnthetubaguy | yeah, certainly if there is something nasty let me know and I can jump in. (My week has the potential to be mad next week, though not sure how mad yet!) | 17:03 |
opendevreview | Julia Kreger proposed openstack/ironic master: Log an exception from heartbeat https://review.opendev.org/c/openstack/ironic/+/893417 | 17:13 |
opendevreview | Jay Faulkner proposed openstack/ironic master: DNM: Testing against newer alembic https://review.opendev.org/c/openstack/ironic/+/893424 | 18:01 |
opendevreview | Jay Faulkner proposed openstack/ironic master: DNM: Testing against newer alembic https://review.opendev.org/c/openstack/ironic/+/893424 | 18:28 |
opendevreview | Merged openstack/ironic master: Fully monkey patch eventlet for consistent behavior https://review.opendev.org/c/openstack/ironic/+/887996 | 18:28 |
opendevreview | Jay Faulkner proposed openstack/ironic-inspector master: [DNM] Testing Gate https://review.opendev.org/c/openstack/ironic-inspector/+/840891 | 19:18 |
opendevreview | Jay Faulkner proposed openstack/ironic-inspector master: Fix bindep for python 3.11 tests in Debian Bookworm https://review.opendev.org/c/openstack/ironic-inspector/+/893432 | 19:19 |
opendevreview | Merged openstack/python-ironic-inspector-client master: Fix Gate https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/891976 | 19:41 |
JayF | updating the PR for that release now | 19:42 |
opendevreview | Julia Kreger proposed openstack/ironic master: Add service steps and initial docs https://review.opendev.org/c/openstack/ironic/+/891630 | 20:41 |
samcat116 | Does Bifrost support using a bridge interface? | 21:13 |
TheJulia | should just work | 21:14 |
TheJulia | if pre-configured | 21:14 |
JayF | and it does just work in our testenv cases | 21:14 |
JayF | that's literally how it works in the gate, yeah? | 21:15 |
TheJulia | yeah | 21:15 |
TheJulia | virbr0 | 21:15 |
samcat116 | nice, figured id ask before breaking this box | 21:15 |
TheJulia | Enjoy! | 21:15 |
samcat116 | I wonder if the bifrost kolla container would support running in normal bridged network mode instead of host networking if you disable-dhcp | 21:17 |
JayF | I suspect all the knobs are there if you know the right ways to turn them | 21:18 |
JayF | I do not :) | 21:18 |
iurygregory | TheJulia, thanks for review o/ | 21:49 |
opendevreview | Julia Kreger proposed openstack/ironic master: DNM: Try to get lock logging with metal3 https://review.opendev.org/c/openstack/ironic/+/893438 | 22:12 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!