opendevreview | Dmitry Tantsur proposed openstack/ironic master: Add disable_power_off field to the node model https://review.opendev.org/c/openstack/ironic/+/930229 | 09:21 |
---|---|---|
dtantsur | Looking 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/+/930203 | 09:28 |
iurygregory | morning ironic | 09:30 |
iurygregory | dtantsur, going to start looking at it | 09:30 |
dtantsur | thx! (and good morning) | 09:30 |
cid | JayF, 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 |
cid | Good morning o/ | 09:39 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: Add timeout to SERVICEWAIT https://review.opendev.org/c/openstack/ironic/+/915318 | 10:25 |
opendevreview | Merged openstack/ironic master: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930279 | 10:45 |
opendevreview | Merged openstack/ironic master: Refactoring: get rid of AgentDeployMixin https://review.opendev.org/c/openstack/ironic/+/930203 | 11:03 |
opendevreview | Merged openstack/ironic-tempest-plugin master: Check inspection data and abortion in the standalone tests https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/927928 | 11:23 |
tkajinam | I wonder if I can bother anyone to get a few python-dracclient patches reviewed ? | 11:58 |
tkajinam | https://review.opendev.org/c/openstack/python-dracclient/+/917151 and https://review.opendev.org/c/openstack/python-dracclient/+/917125 | 11:59 |
tkajinam | and probably https://review.opendev.org/c/openstack/python-dracclient/+/909327 | 12:00 |
iurygregory | maybe some other ironic cores have +2, at least I don't have for python-dracclient =( | 12:57 |
iurygregory | dtantsur, rpittau ? ^ | 12:57 |
rpittau | I don't | 12:58 |
TheJulia | Only Dmitry does | 12:58 |
rpittau | I think dtantsur has it | 12:58 |
TheJulia | I just looked it up | 12:58 |
rpittau | yep | 12:58 |
tkajinam | https://review.opendev.org/admin/groups/a92885e6154a848edc4f319058dea4f4c8a641ae yeah seems so. idk if the other cores are still around recently | 13:14 |
rpittau | tkajinam: most of them are not | 13:17 |
tkajinam | I 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, though | 13:20 |
JayF | cid: getting everything but tests working is still a success, I bet we'll be able to get it over the hump | 14:16 |
JayF | cid: if you don't have something else for it, lets pair on it this afternoon | 14:17 |
dtantsur | tkajinam: approved | 14:17 |
dtantsur | rpittau, iurygregory, TheJulia, added ironic-core to python-dracclient-core | 14:18 |
cid | JayF: I guess. Pairing, sure! | 14:18 |
iurygregory | dtantsur, awesome tks! | 14:19 |
dtantsur | tkajinam: let us know if you actually want dracclient (ever) released. I'd rather have it deprecated and left in peace :) | 14:19 |
dtantsur | if anyone wants to put a deprecation warning to its readme/docs, I'll approve | 14:20 |
dtantsur | (we cannot retire it until it's used on stable branches of Ironic) | 14:20 |
JayF | You *can* retire future releases | 14:20 |
JayF | ala what TripleO did | 14:21 |
JayF | right? | 14:21 |
JayF | or I guess for non-cycle projects that's just ... waiting | 14:21 |
JayF | lol | 14:21 |
dtantsur | yeah | 14:21 |
opendevreview | Dmitry Tantsur proposed openstack/ironic stable/2024.2: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930473 | 14:28 |
opendevreview | Dmitry Tantsur proposed openstack/ironic stable/2024.1: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930474 | 14:28 |
opendevreview | Dmitry Tantsur proposed openstack/ironic stable/2023.2: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930476 | 14:29 |
opendevreview | Dmitry Tantsur proposed openstack/ironic bugfix/26.0: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930477 | 14:29 |
opendevreview | Dmitry Tantsur proposed openstack/ironic bugfix/25.0: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930478 | 14:30 |
opendevreview | Merged openstack/ironic bugfix/26.0: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930477 | 15:18 |
opendevreview | Merged openstack/ironic bugfix/25.0: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930478 | 15:21 |
JayF | dansmith: 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 |
dansmith | it's a much thorny-er problem than it seems, a lot of which stems from our willingness to allow anything into raw from the beginning | 16:43 |
JayF | Well 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 problem | 16:43 |
JayF | and if you can't do that, you really can't have anything nice beyond "does it have a partition table" | 16:44 |
JayF | and 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 point | 16:44 |
dansmith | yeah, 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 images | 16:44 |
JayF | honestly even like, the "anything else" case being a separate rbac flag, for instance | 16:44 |
JayF | normal users can upload things that validate as GPT, only admins can upload actual raw, for instance makes sense from an operational standpoint | 16:45 |
JayF | I have no idea if it's reasonable from an actual implementation standpoint | 16:45 |
dansmith | yeah, 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 improvement | 16:45 |
JayF | The gross middle ground is the partition image, that just has a filesystem and no partition metadata | 16:45 |
dansmith | yeah that may be reasonable for a lot of cases.. not all of them, but as a goal I think that's not terrible | 16:45 |
JayF | but Ironic wants to deprecate those anyway so that's an easy case | 16:46 |
dansmith | the funny thing is, | 16:46 |
JayF | then 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 ramdisks | 16:46 |
* TheJulia twitches about partition images | 16:46 | |
* TheJulia twitches more about partition images | 16:46 | |
* TheJulia may had additional trauma from partition images. | 16:47 | |
dansmith | the 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 enable | 16:47 |
JayF | Ironic has an extra problem in that we'll often mount disks after imaging to configure bootloaders and the like in some cases | 16:47 |
dansmith | so 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 wrong | 16:48 |
JayF | I don't think we can remove that logic in all cases, but I would be interested to figure out *exactly* those cases | 16:48 |
JayF | I mean, Ironic just yolo-qemu-img convert basically anything it was given before the recent change | 16:48 |
TheJulia | we 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 hardware | 16:48 |
dansmith | well, 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 consistency | 16:48 |
JayF | so we punted on basically the entire problem which Julia and I paid for in the security fixes | 16:48 |
dansmith | we all did | 16:49 |
TheJulia | we *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 |
JayF | I 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 long | 16:49 |
dansmith | I 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 there | 16:50 |
dansmith | so there's clear recognition that glance should be (and should have been) rejecting such mismatches | 16:50 |
TheJulia | makes sense to me | 16:50 |
TheJulia | (that it should, but there is work to be done since that is inherently a feature) | 16:51 |
dansmith | I think a lot of people assumed that was already happening, or at least to a greater degree than it was | 16:51 |
TheJulia | (which is also a bug, but yeah, pick and choose and whatnot) | 16:51 |
dansmith | you 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 off | 16:51 |
JayF | I always thought the image format / container format in glance was just user supplied metadata | 16:52 |
JayF | just simply from operating clouds and screwing up the metadata enough to know it wouldn't stop me lol | 16:52 |
TheJulia | I can see how folks made assumptions, but I had the same impression | 16:52 |
* JayF points at Ironic's 10 years of docs that said to upload a 'raw' kernel/partition image as aki/ari | 16:53 | |
dansmith | well, 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 not | 16:53 |
JayF | most of that experience was on Rackspaces' openstack stuff, which sometimes behaved differently too in that era | 16:53 |
JayF | I still occassionally un-learn a weird thing I learned there | 16:53 |
dansmith | definitely different than the libvirt driver in a lot of cases, yeah | 16:54 |
JayF | https://bugs.launchpad.net/ironic/+bug/1526759 is pretty much already implemented, yeah? between conductor drain and /v1/conductors? | 16:59 |
TheJulia | I would concur, the delta (and noted would be nice) item was the ability for an admin to toggle operation | 17:02 |
TheJulia | but... yeah | 17:02 |
TheJulia | I'd rather the platform just send us a term signal | 17:02 |
TheJulia | personally | 17:02 |
TheJulia | because otherwise, then an admin has to go look/see/know | 17:03 |
JayF | yep | 17:04 |
JayF | term signal on the app is the way to drain something pretty universally | 17:04 |
TheJulia | I closed that one out | 17:12 |
TheJulia | with such a comment | 17:12 |
JayF | ack was going to do it just now, ty | 17:14 |
JayF | sorting our bugs by heat is interesting | 17:14 |
JayF | that's how I saw it | 17:14 |
TheJulia | oh, heh | 17:16 |
TheJulia | yeah | 17:16 |
JayF | cid: if you were looking for old bugs to resolve, https://bugs.launchpad.net/ironic/+bug/1439901 is fairly straightforward and a long time desire | 17:22 |
JayF | https://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 patch | 17:23 |
JayF | cid: https://bugs.launchpad.net/ironic/+bug/1619235 might be a cousin to 1439901 | 17:24 |
JayF | https://bugs.launchpad.net/ironic/+bug/1662228 as well | 17:24 |
cid | JayF: Right! Noted | 17:33 |
cid | I will take a look a bit later. | 17:33 |
opendevreview | Merged openstack/ironic stable/2024.2: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930473 | 17:35 |
opendevreview | Merged openstack/ironic stable/2024.1: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930474 | 17:37 |
cardoe | https://review.opendev.org/c/openstack/ironic/+/926961 should be an easy win for a docs improvement | 17:45 |
opendevreview | Merged openstack/ironic stable/2023.2: Fix double transition to INSPECTFAIL on aborting in-band inspection https://review.opendev.org/c/openstack/ironic/+/930476 | 18:02 |
opendevreview | Jay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu https://review.opendev.org/c/openstack/virtualpdu/+/928823 | 18:51 |
opendevreview | Jay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu https://review.opendev.org/c/openstack/virtualpdu/+/928823 | 18:55 |
JayF | ^^ passing tests locally, will fail requirements check due to missing pyasynccore in global-req | 19:00 |
JayF | I 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 |
JayF | TheJulia: ^ u can haz snmp ci :D | 19:01 |
opendevreview | Jay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu https://review.opendev.org/c/openstack/virtualpdu/+/928823 | 19:09 |
TheJulia | Grilling chicken for lunch, will look once I’ve eaten | 19:10 |
JayF | I mean, nothing to see really | 19:10 |
JayF | we vendored in the pysnmp module, updated a bunch of references to it | 19:10 |
JayF | only real weirdness is pysnmp is BSD-2-Clause licensed; I put the BSD notice in the VENDORED.txt file | 19:11 |
JayF | but BSD and Apache 2.0 are compatible so it should be OK | 19:11 |
TheJulia | That is the path of least resistance for virtualpdu and makes sense to me | 19:22 |
TheJulia | I'm wondering how we address ironic | 19:22 |
TheJulia | or if we just offram that | 19:22 |
TheJulia | offramp | 19:22 |
TheJulia | I feel like when we expressed we might, there was panic, but no action | 19:22 |
JayF | Ironic has support for the new | 19:27 |
JayF | Virtualpdu was the painful migration | 19:27 |
JayF | So I think this means it'll be in CI at least until this stops working lol | 19:27 |
TheJulia | \o/ | 19:30 |
JayF | I would not be surprised if at some point we have to vendor in pyasn too | 19:33 |
JayF | but for now it works and I wanted to pull in as little as possible | 19:34 |
JayF | also we can probably still keep requirements-check happy if we get pyasyncore added to global-reqs, which shouldn't be hard | 19:34 |
opendevreview | Jay Faulkner proposed openstack/virtualpdu master: Vendor pysnmp v5.0.33 into virtualpdu https://review.opendev.org/c/openstack/virtualpdu/+/928823 | 19:52 |
JayF | that failed tempest the first time 'round, but I think the VM's disk went out to lunch | 19:58 |
TheJulia | Unfortunately, that does happen | 21:19 |
JayF | https://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 passing | 21:24 |
JayF | but appears to be broken IRL :( | 21:24 |
JayF | no evidence we ever even turned on the vm | 21:24 |
TheJulia | ughhh | 21:24 |
JayF | I will poke at this with devstack tomorrow if you wanna join cid, I think I should have some time | 21:25 |
JayF | I 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 behavior | 21:25 |
JayF | in case you're wondering, openstack-exporter will export empty string for missing strings, and false for missing bools, which is super reasonable behavior | 21:25 |
TheJulia | oh cool | 21:26 |
JayF | on the flip side, learned there's no (obvious) way to make gophercloud work with noauth'd ironic clouds https://github.com/gophercloud/gophercloud/issues/3185 | 21:26 |
JayF | as long as the maintainer cooperates, you can expect ironic node exports from that project to get *much* better in the next few months | 21:27 |
JayF | adamcarthur5 is basically pointed at "make openstack-exporter and gophercloud have near-complete Ironic API coverage" for a while | 21:27 |
JayF | heh, I wonder with how misdirected pysnmp is inside | 21:29 |
JayF | if having a pysnmp module actually installed alongside (for ironic) is breaking it | 21:29 |
JayF | I'll figure it out :) | 21:29 |
JayF | if that's the case I might also do the "install virtualpdu in a separate venv" change | 21:30 |
JayF | that 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 all | 21:30 |
cid | JayF: count me in | 21:40 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!