Wednesday, 2024-09-25

opendevreviewDmitry Tantsur proposed openstack/ironic master: Add disable_power_off field to the node model  https://review.opendev.org/c/openstack/ironic/+/93022909:21
dtantsurLooking for a 2nd +2 again: https://review.opendev.org/c/openstack/ironic/+/930279 https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/927928 and (somewhat larger) https://review.opendev.org/c/openstack/ironic/+/93020309:28
iurygregorymorning ironic09:30
iurygregorydtantsur, going to start looking at it09:30
dtantsurthx! (and good morning)09:30
cidJayF, I wish :melting_face . I got the jobs passing locally and unit tests instantly now require manual intervention to end. I don't know what it is yet, just pointing away from a pip-installed version to a local copy comes with unexpected errors.09:39
cidGood morning o/09:39
opendevreviewDmitry Tantsur proposed openstack/ironic master: Add timeout to SERVICEWAIT  https://review.opendev.org/c/openstack/ironic/+/91531810:25
opendevreviewMerged openstack/ironic master: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93027910:45
opendevreviewMerged openstack/ironic master: Refactoring: get rid of AgentDeployMixin  https://review.opendev.org/c/openstack/ironic/+/93020311:03
opendevreviewMerged openstack/ironic-tempest-plugin master: Check inspection data and abortion in the standalone tests  https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/92792811:23
tkajinamI wonder if I can bother anyone to get a few python-dracclient patches reviewed ?11:58
tkajinamhttps://review.opendev.org/c/openstack/python-dracclient/+/917151 and https://review.opendev.org/c/openstack/python-dracclient/+/91712511:59
tkajinamand probably https://review.opendev.org/c/openstack/python-dracclient/+/90932712:00
iurygregorymaybe some other ironic cores have +2, at least I don't have for python-dracclient =(12:57
iurygregorydtantsur, rpittau ? ^12:57
rpittauI don't12:58
TheJuliaOnly Dmitry does12:58
rpittauI think dtantsur has it12:58
TheJuliaI just looked it up12:58
rpittauyep12:58
tkajinamhttps://review.opendev.org/admin/groups/a92885e6154a848edc4f319058dea4f4c8a641ae yeah seems so. idk if the other cores are still around recently13:14
rpittautkajinam: most of them are not13:17
tkajinamI wasn't aware that draclcient is no longer used since https://review.opendev.org/c/openstack/ironic/+/922340 was merged. it's kept in hardware vendor sig but if that sig is no longer active then it may make sense to retire the repo.13:20
tkajinam(that's out of ironic's own scope, though13:20
JayFcid: getting everything but tests working is still a success, I bet we'll be able to get it over the hump14:16
JayFcid: if you don't have something else for it, lets pair on it this afternoon14:17
dtantsurtkajinam: approved14:17
dtantsurrpittau, iurygregory, TheJulia, added ironic-core to python-dracclient-core14:18
cidJayF: I guess. Pairing, sure!14:18
iurygregorydtantsur, awesome tks!14:19
dtantsurtkajinam: let us know if you actually want dracclient (ever) released. I'd rather have it deprecated and left in peace :)14:19
dtantsurif anyone wants to put a deprecation warning to its readme/docs, I'll approve14:20
dtantsur(we cannot retire it until it's used on stable branches of Ironic)14:20
JayFYou *can* retire future releases14:20
JayFala what TripleO did14:21
JayFright?14:21
JayFor I guess for non-cycle projects that's just ... waiting14:21
JayFlol14:21
dtantsuryeah14:21
opendevreviewDmitry Tantsur proposed openstack/ironic stable/2024.2: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047314:28
opendevreviewDmitry Tantsur proposed openstack/ironic stable/2024.1: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047414:28
opendevreviewDmitry Tantsur proposed openstack/ironic stable/2023.2: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047614:29
opendevreviewDmitry Tantsur proposed openstack/ironic bugfix/26.0: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047714:29
opendevreviewDmitry Tantsur proposed openstack/ironic bugfix/25.0: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047814:30
opendevreviewMerged openstack/ironic bugfix/26.0: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047715:18
opendevreviewMerged openstack/ironic bugfix/25.0: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047815:21
JayFdansmith: thanks for taking me around the bend to understanding re: that glance spec. Sometimes a desire to shape a better future leads to having blinders to the current reality :)16:41
JayF(thinking about how awful the code would be if we were detecting all possible forms of valid disk image data in "raw-like" formats basically seemed like the most terrible thing we could implement)16:42
dansmithit's a much thorny-er problem than it seems, a lot of which stems from our willingness to allow anything into raw from the beginning16:43
JayFWell yeah, when I started thinking about the actual-technical-detail it would take to properly categorize disk images stored as raw bytes ... it's just an intractable problem16:43
JayFand if you can't do that, you really can't have anything nice beyond "does it have a partition table" 16:44
JayFand while it'd be nice to read in a users' intent when they upload an image, that's basically completely unreliable and so is just basically user-facing metadata at that point16:44
dansmithyeah, I'm honestly torn between saying "we should identify everything we accept and reject anything else" and "quantify the important things" and make sure we handle the "anything else" case(s) appropriately as data and not first-class images16:44
JayFhonestly even like, the "anything else" case being a separate rbac flag, for instance16:44
JayFnormal users can upload things that validate as GPT, only admins can upload actual raw, for instance makes sense from an operational standpoint16:45
JayFI have no idea if it's reasonable from an actual implementation standpoint16:45
dansmithyeah, I really think that if nova (et al) can properly distinguish between "this is a bootable PC-like disk" and "that is something else that is just data to the user" that will be a big improvement16:45
JayFThe gross middle ground is the partition image, that just has a filesystem and no partition metadata16:45
dansmithyeah that may be reasonable for a lot of cases.. not all of them, but as a goal I think that's not terrible16:45
JayFbut Ironic wants to deprecate those anyway so that's an easy case16:46
dansmiththe funny thing is,16:46
JayFthen the rest of the cases are /mostly/ legacy kernel/root disk separated boot setups (does nova even support this as a first class thing now?) + admin-facing stuff like IPA ramdisks16:46
* TheJulia twitches about partition images16:46
* TheJulia twitches more about partition images16:46
* TheJulia may had additional trauma from partition images.16:47
dansmiththe vfat thing of recent weeks was nova doing that (raw filesystem in an image) internally in a way that is not really valid/useful anyway, while also being of the opinion that raw filesystems in disks is probably not something we want to generally enable16:47
JayFIronic has an extra problem in that we'll often mount disks after imaging to configure bootloaders and the like in some cases16:47
dansmithso the lax nature of just never being sure of what is in a given container was not just a user-focused thing, but internally we're lazy and wrong16:48
JayFI don't think we can remove that logic in all cases, but I would be interested to figure out *exactly* those cases16:48
JayFI mean, Ironic just yolo-qemu-img convert basically anything it was given before the recent change16:48
TheJuliawe cannot because we have to hunt for bootloader config (since some distributions don't like standards, or need their preference override file to be used to prevent bugs/crashes on some hardware16:48
dansmithwell, the reason we have the bare vfat thing is because manipulating a partition inside an image is hard, but that's a bad reason to just disavow all responsibility and consistency16:48
JayFso we punted on basically the entire problem which Julia and I paid for in the security fixes16:48
dansmithwe all did16:49
TheJuliawe *also* get to enjoy the fun of having to fix user issues and a swath of users who insist "you'll take partition images from our dead hands"16:49
JayFI try really hard to get junior folks I work with to understand that we're all just figuring things out, and the shining example of that is a bunch of people who've been working on clouds for decades learning a new scary thing about a tool we've been using for that long16:49
dansmithI have had probably ten people reach out to me since the CVEs were published, worried about the fact that glance still lets you upload a qcow file and call it a raw, to which I have to explain that there's still work to be done there16:50
dansmithso there's clear recognition that glance should be (and should have been) rejecting such mismatches16:50
TheJuliamakes sense to me16:50
TheJulia(that it should, but there is work to be done since that is inherently a feature)16:51
dansmithI think a lot of people assumed that was already happening, or at least to a greater degree than it was16:51
TheJulia(which is also a bug, but yeah, pick and choose and whatnot)16:51
dansmithyou can get a vmware node to try to boot a qcow (which it does not support) by putting your qcow in something you claim is a vmdk and we'll send it off16:51
JayFI always thought the image format / container format in glance was just user supplied metadata16:52
JayFjust simply from operating clouds and screwing up the metadata enough to know it wouldn't stop me lol16:52
TheJuliaI can see how folks made assumptions, but I had the same impression16:52
* JayF points at Ironic's 10 years of docs that said to upload a 'raw' kernel/partition image as aki/ari16:53
dansmithwell, there are places where that metadata matters and places where it doesn't so I think it depends on what you've bumped into and not16:53
JayFmost of that experience was on Rackspaces' openstack stuff, which sometimes behaved differently too in that era 16:53
JayFI still occassionally un-learn a weird thing I learned there16:53
dansmithdefinitely different than the libvirt driver in a lot of cases, yeah16:54
JayFhttps://bugs.launchpad.net/ironic/+bug/1526759 is pretty much already implemented, yeah? between conductor drain and /v1/conductors?16:59
TheJuliaI would concur, the delta (and noted would be nice) item was the ability for an admin to toggle operation17:02
TheJuliabut... yeah17:02
TheJuliaI'd rather the platform just send us a term signal17:02
TheJuliapersonally17:02
TheJuliabecause otherwise, then an admin has to go look/see/know17:03
JayFyep17:04
JayFterm signal on the app is the way to drain something pretty universally17:04
TheJuliaI closed that one out17:12
TheJuliawith such a comment17:12
JayFack was going to do it just now, ty 17:14
JayFsorting our bugs by heat is interesting17:14
JayFthat's how I saw it17:14
TheJuliaoh, heh17:16
TheJuliayeah17:16
JayFcid: if you were looking for old bugs to resolve, https://bugs.launchpad.net/ironic/+bug/1439901 is fairly straightforward and a long time desire17:22
JayFhttps://bugs.launchpad.net/ironic/+bug/2045191 would be nice to address too, but I'm not sure we should without someone with the hardware willing to test the patch17:23
JayFcid: https://bugs.launchpad.net/ironic/+bug/1619235 might be a cousin to 143990117:24
JayFhttps://bugs.launchpad.net/ironic/+bug/1662228 as well17:24
cidJayF: Right! Noted17:33
cidI will take a look a bit later.17:33
opendevreviewMerged openstack/ironic stable/2024.2: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047317:35
opendevreviewMerged openstack/ironic stable/2024.1: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047417:37
cardoehttps://review.opendev.org/c/openstack/ironic/+/926961 should be an easy win for a docs improvement17:45
opendevreviewMerged openstack/ironic stable/2023.2: Fix double transition to INSPECTFAIL on aborting in-band inspection  https://review.opendev.org/c/openstack/ironic/+/93047618:02
opendevreviewJay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu  https://review.opendev.org/c/openstack/virtualpdu/+/92882318:51
opendevreviewJay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu  https://review.opendev.org/c/openstack/virtualpdu/+/92882318:55
JayF^^ passing tests locally, will fail requirements check due to missing pyasynccore in global-req19:00
JayFI would suggest if that passes everything else otherwise that we remove the requirements check from there, as it's: 1) not sold as something co-installable (we tell people not to run it) and 2) if we ever had an incompatible reuqirement introduced to ironic, the actual tempest tests would catch it (I think that's unlikely given we just vendored pysnmp, though)19:01
JayFTheJulia: ^ u can haz snmp ci :D 19:01
opendevreviewJay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu  https://review.opendev.org/c/openstack/virtualpdu/+/92882319:09
TheJuliaGrilling chicken for lunch, will look once I’ve eaten19:10
JayFI mean, nothing to see really 19:10
JayFwe vendored in the pysnmp module, updated a bunch of references to it19:10
JayFonly real weirdness is pysnmp is BSD-2-Clause licensed; I put the BSD notice in the VENDORED.txt file19:11
JayFbut BSD and Apache 2.0 are compatible so it should be OK19:11
TheJuliaThat is the path of least resistance for virtualpdu and makes sense to me19:22
TheJuliaI'm wondering how we address ironic19:22
TheJuliaor if we just offram that19:22
TheJuliaofframp19:22
TheJuliaI feel like when we expressed we might, there was panic, but no action19:22
JayFIronic has support for the new19:27
JayFVirtualpdu was the painful migration19:27
JayFSo I think this means it'll be in CI at least until this stops working lol19:27
TheJulia\o/19:30
JayFI would not be surprised if at some point we have to vendor in pyasn too19:33
JayFbut for now it works and I wanted to pull in as little as possible19:34
JayFalso we can probably still keep requirements-check happy if we get pyasyncore added to global-reqs, which shouldn't be hard19:34
opendevreviewJay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu  https://review.opendev.org/c/openstack/virtualpdu/+/92882319:52
JayFthat failed tempest the first time 'round, but I think the VM's disk went out to lunch19:58
TheJuliaUnfortunately, that does happen21:19
JayFhttps://53a68ba6d05d22d0f44d-c5982ef1d1780edfccf5471d12724c1b.ssl.cf5.rackcdn.com/928823/8/check/ironic-tempest-wholedisk-bios-snmp-pxe-virtualpdu-src/3117aff/controller/logs/screen-virtualpdu.txt works locally with unit tests passing21:24
JayFbut appears to be broken IRL :( 21:24
JayFno evidence we ever even turned on the vm21:24
TheJuliaughhh21:24
JayFI will poke at this with devstack tomorrow if you wanna join cid, I think I should have some time21:25
JayFI just had a fun little devstack testing session with Adam; we're working on openstack-exporter and I got to use our microversion code to disappear node fields to test behavior21:25
JayFin case you're wondering, openstack-exporter will export empty string for missing strings, and false for missing bools, which is super reasonable behavior21:25
TheJuliaoh cool21:26
JayFon the flip side, learned there's no (obvious) way to make gophercloud work with noauth'd ironic clouds https://github.com/gophercloud/gophercloud/issues/318521:26
JayFas long as the maintainer cooperates, you can expect ironic node exports from that project to get *much* better in the next few months21:27
JayFadamcarthur5 is basically pointed at "make openstack-exporter and gophercloud have near-complete Ironic API coverage" for a while21:27
JayFheh, I wonder with how misdirected pysnmp is inside21:29
JayFif having a pysnmp module actually installed alongside (for ironic) is breaking it21:29
JayFI'll figure it out :)21:29
JayFif that's the case I might also do the "install virtualpdu in a separate venv" change21:30
JayFthat would reduce the worry space for that app significantly, as it stops needing to chase library updates with openstack and just has to work on modern python versions at all21:30
cidJayF: count me in21:40

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