Thursday, 2023-08-31

opendevreviewMerged openstack/ironic-python-agent-builder stable/xena: Add checksum generation support  https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/89296800:03
opendevreviewMerged openstack/ironic master: Utilize the JSON-RPC port  https://review.opendev.org/c/openstack/ironic/+/87921504:45
mnasiadkammalchuk: I don’t think https://bugs.launchpad.net/bugs/2033528 should be marked invalid - the bug is valid, implementation should be discussed in Gerrit06:50
mnasiadkaJayF: ^^ sorry to complain, but that’s a bit unwelcoming to mark bug as invalid based on patch content ;-)06:54
rpittaugood morning ironic! o/07:27
rpittauJayF: you anticipated me by a jiff for the ansible 8 test on bifrost :)07:28
opendevreviewVerification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm  https://review.opendev.org/c/openstack/ironic/+/89322508:53
opendevreviewVerification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm  https://review.opendev.org/c/openstack/ironic/+/89322509:33
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319510:24
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319510:50
iurygregorygood morning Ironic11:45
opendevreviewMerged openstack/ironic master: Add missing release mappings for 22.0 and 22.1  https://review.opendev.org/c/openstack/ironic/+/89323212:06
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319512:21
opendevreviewVerification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm  https://review.opendev.org/c/openstack/ironic/+/89322512:39
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319512:44
opendevreviewJulia Kreger proposed openstack/ironic master: DNM: Test grub+ovn-dhcp-opt-210  https://review.opendev.org/c/openstack/ironic/+/89328713:11
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319513:20
opendevreviewVerification of a change to openstack/ironic master failed: Correct bindep.txt entries for bookworm  https://review.opendev.org/c/openstack/ironic/+/89322513:20
iurygregoryok, I think I have some idea about what is wrong in the python-ironic-inspector-client13:33
iurygregorybut tbh I have no idea on how to fix LOL13:33
iurygregorymaybe is a matter of tearDown of other tests or the order tests run13:35
iurygregoryI'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 works13:36
iurygregory  functional-py310: OK (50.70=setup[6.16]+cmd[44.55] seconds)13:36
iurygregory  congratulations :) (50.83 seconds)13:36
iurygregorythoughts?13:36
opendevreviewJulia Kreger proposed openstack/ironic-python-agent master: Add get_service_steps logic to the agent  https://review.opendev.org/c/openstack/ironic-python-agent/+/89086413:37
opendevreviewJulia 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/+/89337113:37
iurygregoryops, not self.node_uuid, but self.uuid13:37
opendevreviewIury Gregory Melo Ferreira proposed openstack/python-ironic-inspector-client master: [WIP] Test Fix  https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/89197613:41
iurygregorylet's see how CI will react to it...13:41
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319513:49
TheJuliaiurygregory: you got functional to pass, I think that is a good sigh in general13:53
iurygregoryyeah13:54
iurygregoryzuul gave +1, so I think it's probably something in the tearDown or the order we run the tests, maybe the "node" was inspected already13:57
opendevreviewRafal Lewandowski proposed openstack/bifrost master: Fix for lack of log rotation in Bifrost  https://review.opendev.org/c/openstack/bifrost/+/89319514:00
JayFiurygregory: so https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/891976 is good then? yeah? You wanna un-WIP it?14:07
iurygregoryJayF, so I consider this a workaround, I probably don't have enough time to figure out why it fails when we use self.uuid14:10
iurygregoryif we are ok to have this I can un-WIP and add some Notes in the patch14:10
JayFlets put an explicit TODO to fix that and not drop the task overall14:10
JayFbut we need passing tests here to get a release in14:10
iurygregoryack14:10
iurygregorylet me update the patch14:10
JayFty14:11
iurygregorywant me to add a more fancy way to generate the uuid or we should keep hardcoded?14:13
opendevreviewIury Gregory Melo Ferreira proposed openstack/python-ironic-inspector-client master: Fix Gate  https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/89197614:20
opendevreviewJulia Kreger proposed openstack/ironic master: log the version of the conductor starting  https://review.opendev.org/c/openstack/ironic/+/89337914:22
opendevreviewJulia Kreger proposed openstack/ironic master: log the version of the conductor starting  https://review.opendev.org/c/openstack/ironic/+/89337914:22
rpittauiurygregory: 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
iurygregoryrpittau, oh nice!14:24
JayFI just realized the work to shardify our networking-baremetal library got dropped (by me) this cycle14:25
opendevreviewMerged openstack/ironic-python-agent master: preserve/handle config drives on 4k block devices  https://review.opendev.org/c/openstack/ironic-python-agent/+/88879414:30
iurygregoryJayF, TheJulia the patch is green :D14:40
iurygregoryJayF, I saw we probably want https://review.opendev.org/c/openstack/python-ironicclient/+/891140 so we can cut the release14:41
iurygregoryTheJulia, if you don't have time I can solve the merge conflict on it ^14:42
JayFhttps://review.opendev.org/c/openstack/ironic/+/891630 hasn't landed yet14:42
JayFwhich I guess the API still might exist?14:42
iurygregoryoh =(14:42
JayFI think the client for service steps might miss14:42
TheJuliayeah, it misses most likely14:42
JayFnot ideal but if you're a clever enough deployer to use service steps, you can upgrade to a newer client14:42
TheJuliagiven it should have released last week14:42
JayFI'll go poke the releases pr14:42
TheJuliamost are14:42
TheJuliaweird, I thoguth i fixed the docs for that14:44
iurygregoryJayF, we need to update the hash :D14:44
TheJuliaapi exists, but for all that *really* matters that can merge next week and be in our final release for the cycle14:44
JayFyeah so I gotta wait for gate to hit14:44
JayFyeah14:44
iurygregoryI'm on it https://review.opendev.org/c/openstack/releases/+/892945/1/deliverables/bobcat/python-ironicclient.yaml14:44
* TheJulia burries self back in the power sync loop for a little bit14:44
JayFyou have that sha even before it lands?14:44
JayFoh, ironicclient14:45
JayFgot it14:45
JayFthanks14:45
iurygregoryit will need your +1 again =)14:46
JayFor any release manager14:46
JayFI thought you were on that list but maybe not14:46
JayFyeah you are release manager14:47
JayFyour +1 should set PTL-Approved +114:47
iurygregorysince I pushed I'm not sure I can +1 (but I will do)14:47
JayFif you pushed it might even be automatic14:50
JayFI did +1 tho14:50
JayFiurygregory: 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
iurygregoryack14:57
rpittaubye everyone, see you 3 weeks! o/15:01
JayFo/15:16
TheJulia3 weeks, hell, I need more pto15:16
iurygregoryJayF, monday is holiday in the US right?15:30
JayFyes, and you volunteered to run the meeting :)15:31
iurygregorynow 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
iurygregoryI think I will be alone in the channel LOL15:32
JayFI delegated meeting chair to you, I have no objection if you wanna cancel it, just make sure to update the agenda and email the list15:32
JayFI'm also OK if you wanna do startmeeting -> anyone here? -> no quorum -> end :D 15:32
iurygregoryoh good =)15:32
iurygregoryI will do this 15:32
opendevreviewMerged openstack/ironic master: Correct bindep.txt entries for bookworm  https://review.opendev.org/c/openstack/ironic/+/89322516:12
JayFSo 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 behavior16:31
JayFTheJulia: iurygregory ^ curious if you have thoughts on this16:32
TheJuliait may already be used or manually managed16:32
TheJuliaso new default behavior that stomps would basically be bad16: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 hate16:33
JayFI 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-compute16:33
JayFTheJulia: It was explained to me as Nova dislikes when behavior changes for the user without it being API discoverable16:33
JayFwait, I got it16:33
JayFTheJulia: johnthetubaguy: Nova always puts a potential_lessee into instance_info16:33
TheJuliait is an operator decision to make though based upon their security posture and operating state16:33
johnthetubaguyCuriously, 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
JayFif automatic_lessee enabled in Ironic, on the deploy call we set lesseee16:33
TheJuliathis is acceptable16:34
JayFto whatever nova says to set it to16:34
JayFjohnthetubaguy: would that be acceptable to nova?16:34
johnthetubaguyah... interesting point16:34
JayFjust give the metadata to Ironic, Ironic can choose the behavior16:34
TheJulia++16:34
JayFand that keeps the docs on the feature identical for Ironic + Nova16:34
JayFI really like this as an idea16:34
johnthetubaguydo we pass that info already?16:35
JayFNo16:35
JayFbasically I'd change my patch to just be "send the user over in a new instance_info field" 16:35
JayFthen a separate Ironic patch would check instance_info on deploy and set the lessee from that if one is provided16:35
TheJuliaand we can patch ironic to "do the needful" from there16:35
JayFwell, user, I think it's really project ID16:35
TheJuliaand then operator's behavior based upon config can take over16:35
JayFbut the value we want in node.lessee 16:35
johnthetubaguyyeah, project_id and user_id to match the instance_uuid16:36
JayFNow I'll point out16:36
johnthetubaguythen Ironic does what it needs to do with that... that works16:36
JayFthis has the same negative impact that Nova wanted to avoid16:36
JayFwhere the behavior is changed via config and not API discoverable (in Nova's api)16:36
JayFin Ironic's api you can just ask for the node and see if you 404 lol16:36
johnthetubaguyI am probably over interpreting the concern about API discoverability16:38
JayFeither way, I like this16:38
JayFwe're removing a decoder ring16:38
JayFno "configure X way if your env is X, configure Y way is the env is Y"16:38
JayFwhich felt gross anyway16:38
JayFjohnthetubaguy: would this change likely still need a specless bp in nova? 16:39
JayFnbd if yes, just determining what paperwork I need to do :D 16:39
TheJuliaunrelated: I'm becoming more and more convinced of eventlet weirdness being part of metal3's pain16:39
johnthetubaguyProbably, as you its hard to unsee something as a "feature"16:40
JayFTheJulia: https://review.opendev.org/c/openstack/ironic/+/887996 I feel pretty strongly that this will improve our eventlet story16:40
JayFjohnthetubaguy: 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 topic16:40
TheJuliaApproved, see if derek can reproduce this issue I'm looking at tomorrow with it16:41
JayFTheJulia: 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
johnthetubaguyJayF: 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
JayFand also sharding has a chance! 16:42
JayFI 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 it16:43
johnthetubaguyhappy to help, nice to move these bits forward.16:43
JayFI also have to keep updating the contributor docs16:43
JayFoh yeah, not sure I said this here16:43
JayFthere's an MLH fellow starting downstream here in around a month16:43
JayFwho will be focusing on openstack development, first project (with consent of releases team) will be enhancing that automation to understand bugfix releases16:44
TheJuliahttps://docs.sqlalchemy.org/en/20/dialects/sqlite.html#disabling-connection-pooling-for-file-databases16:45
JayFthat is interesting16:47
johnthetubaguyTheJulia: 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't16:48
johnthetubaguyquite remember how.16:48
JayFthat is ... exactly the failure case I mentally modeled could happen without us monkey patching OS16:48
JayFthings getting hung up on I/O that happens in the os module16:48
JayFbut that is the #1 place we see most eventlet shenanigans: image streaming16:49
JayFthat was likely in IPA and we've fixed upsteaam16:49
TheJuliagah, unfortunately metal3 is likely using sqlalchemy 1.416:49
JayFI mean, so are we  :) 16:49
JayFat least for another release, most likely, too16:49
johnthetubaguyJayF: oh, I see what you mean, I am not sure what is counted under os=false16:50
TheJuliayeah, what is interesting is I suddenly got a database is locked error in logs16:50
TheJuliaand I've traced things failing prior (and invovled) to a heartbeat being processed, I can see it triggers task.upgrade_lock()16:51
TheJuliaand... nothing for like 50 seconds16:51
JayFjohnthetubaguy: 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.py16:51
TheJuliaand to be precise it is task.upgrade_lock(retry=False)16:51
JayFoh snap16:51
TheJuliaso not entirely OS, unless sqlalchemy or pymysql maybe16:51
JayFTheJulia: os=false impacts sqlite more b/c it's doing file I/O16:52
TheJuliawhen things *finally* exit, it lis like an exception occurs16:52
johnthetubaguyJayF: oh... like wait and reading files, yeah... that could be a problem.16:52
TheJuliawell, sqlite calls should be native c16:52
TheJuliabut there could be state/locking in py code16:52
JayFI see what you mean16:52
johnthetubaguynot using pymysql or whatever the pure python one is?16:52
JayFjohnthetubaguy: this is for our sqlite support we're talking about16:53
JayFjohnthetubaguy: working out some DB and file locking issues for our metal3/BMO users over in cncf land16:53
JayFjohnthetubaguy: 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
opendevreviewJulia Kreger proposed openstack/ironic master: Log an exception from heartbeat  https://review.opendev.org/c/openstack/ironic/+/89341716:54
johnthetubaguyah, sorry, I see, sqlite doing native c will frequently be bad with eventlet I guess, eek.16:54
JayFwell, I certainly hope that's not the problem or else we might be extra-screwed lol16:55
JayFsince we can't really fix that at a base level if it's the problem16:55
TheJuliait was all good with sqlalchemy 1.3 when we had autocommit16:55
TheJuliathings would just work16:55
johnthetubaguyah nasty, just as well its 6pm here... as an excuse to bravely run away (shudders)16:59
TheJuliaheh17:00
TheJuliagoodnight17:00
JayFjohnthetubaguy: I will note, CI failed on some of those sharding patches :)17:00
JayFjohnthetubaguy: but people are +2'ing them anyway, I assume they have a better handle than me on what is real vs noise17:00
johnthetubaguythat first one has some unrelated py38 specific timestamp nonsense that I hoped someone else would tell me got fixed already, I will check the others17:01
JayFthank 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 lmk17:02
JayFhopefully we won't have RC patches on it but if we need them we'll find it and get em17:02
johnthetubaguyyeah, 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
opendevreviewJulia Kreger proposed openstack/ironic master: Log an exception from heartbeat  https://review.opendev.org/c/openstack/ironic/+/89341717:13
opendevreviewJay Faulkner proposed openstack/ironic master: DNM: Testing against newer alembic  https://review.opendev.org/c/openstack/ironic/+/89342418:01
opendevreviewJay Faulkner proposed openstack/ironic master: DNM: Testing against newer alembic  https://review.opendev.org/c/openstack/ironic/+/89342418:28
opendevreviewMerged openstack/ironic master: Fully monkey patch eventlet for consistent behavior  https://review.opendev.org/c/openstack/ironic/+/88799618:28
opendevreviewJay Faulkner proposed openstack/ironic-inspector master: [DNM] Testing Gate  https://review.opendev.org/c/openstack/ironic-inspector/+/84089119:18
opendevreviewJay Faulkner proposed openstack/ironic-inspector master: Fix bindep for python 3.11 tests in Debian Bookworm  https://review.opendev.org/c/openstack/ironic-inspector/+/89343219:19
opendevreviewMerged openstack/python-ironic-inspector-client master: Fix Gate  https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/89197619:41
JayFupdating the PR for that release now19:42
opendevreviewJulia Kreger proposed openstack/ironic master: Add service steps and initial docs  https://review.opendev.org/c/openstack/ironic/+/89163020:41
samcat116Does Bifrost support using a bridge interface?21:13
TheJuliashould just work21:14
TheJuliaif pre-configured21:14
JayFand it does just work in our testenv cases21:14
JayFthat's literally how it works in the gate, yeah?21:15
TheJuliayeah21:15
TheJuliavirbr021:15
samcat116nice, figured id ask before breaking this box21:15
TheJuliaEnjoy!21:15
samcat116I wonder if the bifrost kolla container would support running in normal bridged network mode instead of host networking if you disable-dhcp21:17
JayFI suspect all the knobs are there if you know the right ways to turn them21:18
JayFI do not :)21:18
iurygregoryTheJulia, thanks for review o/21:49
opendevreviewJulia Kreger proposed openstack/ironic master: DNM: Try to get lock logging with metal3  https://review.opendev.org/c/openstack/ironic/+/89343822:12

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!