opendevreview | cid proposed openstack/ironic master: Fix double JSON encoding of error message https://review.opendev.org/c/openstack/ironic/+/931795 | 06:53 |
---|---|---|
opendevreview | cid proposed openstack/ironic-tempest-plugin master: Test double encoding of error message https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/935740 | 06:53 |
opendevreview | cid proposed openstack/ironic-tempest-plugin master: Fix test to not expect double-JSON-encoded errs https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/932544 | 06:53 |
rpittau | good morning ironic! o/ | 07:37 |
opendevreview | Merged openstack/tenks stable/2.0: Remove deprecated use of include https://review.opendev.org/c/openstack/tenks/+/935624 | 08:09 |
opendevreview | OpenStack Release Bot proposed openstack/tenks stable/2.0: Update .gitreview for stable/2.0 https://review.opendev.org/c/openstack/tenks/+/927159 | 08:31 |
opendevreview | Verification of a change to openstack/ironic master failed: Use specific fix-commit from dnsmasq https://review.opendev.org/c/openstack/ironic/+/935699 | 08:57 |
opendevreview | Pierre Riteau proposed openstack/tenks stable/2.0: Update TOX_CONSTRAINTS_FILE for stable/2.0 https://review.opendev.org/c/openstack/tenks/+/927160 | 13:53 |
lajoskatona | Hi, sorry what is the zoom meeting passcode? for some reason zoom stopped me at this point to join.... | 15:10 |
luisfdez | Link in etherpad: https://etherpad.opendev.org/p/ironic-networking | 15:11 |
lajoskatona | luisfdez: after I copied the link to zoom it asks me for passcode | 15:13 |
luisfdez | 674495 | 15:15 |
dtantsur | lajoskatona: you sure you did not miss any characters? | 15:15 |
dtantsur | ah, this | 15:15 |
lajoskatona | dtantsur: it's working now, thanks | 15:18 |
shermanm | TheJulia: thanks for putting this together! If you've got links to where I should look for writing up a reference architecture, could you shoot an email to shermanm@uchicago.edu? I'm on conference wifi so I'll probably miss IRC messages | 16:15 |
JayF | Someone tell shermanm about irccloud :P | 16:19 |
TheJulia | uhoh, we have his email address now <insert evil laugh here> | 16:19 |
TheJulia | I think the networking call was at least semi-useful | 16:27 |
opendevreview | Verification of a change to openstack/ironic master failed: Use specific fix-commit from dnsmasq https://review.opendev.org/c/openstack/ironic/+/935699 | 16:27 |
rpittau | good night! o/ | 17:13 |
JayF | \o | 17:15 |
opendevreview | Pierre Riteau proposed openstack/tenks stable/2.0: Update TOX_CONSTRAINTS_FILE for stable/2.0 https://review.opendev.org/c/openstack/tenks/+/927160 | 18:17 |
cardoe | gah there was an ironic networking call? | 18:32 |
opendevreview | cid proposed openstack/ironic master: Fix double JSON encoding of error message https://review.opendev.org/c/openstack/ironic/+/931795 | 18:44 |
JayF | https://bugs.launchpad.net/ironic-ui/+bug/2089029 uh-oh. | 19:02 |
JayF | Looks like our run of "ironic-ui just works so we keep it alive" is coming to an end. | 19:02 |
opendevreview | Merged openstack/ironic master: Use specific fix-commit from dnsmasq https://review.opendev.org/c/openstack/ironic/+/935699 | 19:14 |
JayF | So, I'm looking at helping cid with getting kea testable in devstack, and I'm extremely confused about something: I'm trying to determine if and where the dnsmasq dhcp provider is tested in Ironic. | 19:27 |
JayF | For ironic-standalone job, we do not set IRONIC_DHCP_PROVIDER, which means it falls back to the default of neutron, which doesn't seem like it should work for ironic standalone ... but then looking in the logs, we run q-dhcp even in standalone | 19:28 |
JayF | tl;dr I'm fairly convinced there are no devstack-based jobs that use/test dnsmasq dhcp interface | 19:28 |
JayF | is this correct? am I missing something? | 19:29 |
JayF | I'm digging through the bifrost jobs, and it looks almost like the jobs that test dhcp are testing ironic-inspector dhcp, and not ironic-proper | 19:33 |
JayF | If someone could spare a few minutes sometime this week or next to help cid and I unravel what is and is not being tested in terms of DHCP, it'd be very helpful to our next steps. | 19:33 |
opendevreview | Adam McArthur proposed openstack/ironic master: api: Introduce new mechanism for API versioning https://review.opendev.org/c/openstack/ironic/+/928919 | 19:48 |
opendevreview | Adam McArthur proposed openstack/ironic master: api: Add schema validation framework https://review.opendev.org/c/openstack/ironic/+/928920 | 19:48 |
opendevreview | Adam McArthur proposed openstack/ironic master: WIP: api: Add schema for allocations API https://review.opendev.org/c/openstack/ironic/+/928921 | 19:48 |
opendevreview | Adam McArthur proposed openstack/ironic-tempest-plugin master: Testing bad microversions on v1/allocations https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/935827 | 19:58 |
opendevreview | Adam McArthur proposed openstack/ironic-tempest-plugin master: Testing bad microversions on v1/allocations https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/935827 | 19:59 |
JayF | We've had some metal3-integration jobs failing due to docker hub rate limits; I was going to propose we mark it non-voting but I wanna see if https://review.opendev.org/c/openstack/project-config/+/935725 helps first (cc adamcarthur5 ) | 22:08 |
TheJulia | JayF: I guess we could amend the read policy to a matter of do we redact conductors on a reply or not | 22:12 |
TheJulia | but that is a solid sign of minimal use | 22:12 |
JayF | *blink* | 22:13 |
JayF | I think I've lost context on whatever you're asking? | 22:13 |
JayF | unless it's about ironic-ui and I just can't make the leap you're making | 22:13 |
TheJulia | ironic-ui trying to get the list of drivers in the ironic deployment | 22:13 |
JayF | ah, I see | 22:13 |
JayF | it's trying to do validation it can't do? | 22:13 |
TheJulia | its trying to make a pick list for the user | 22:13 |
TheJulia | but it can't get the data because with the new model we went and dialed that access back because it provides strucutral insight into the deployments, including conductor hostnames if memory serves | 22:14 |
JayF | Do we have any way to limit a given project to a specific conductor group or driver set? | 22:14 |
TheJulia | not really | 22:14 |
TheJulia | that would also require upfront mapping | 22:15 |
JayF | if not; it seems like a good fix for this (and this is probably what you meant) would be a microversion where we just return available drivers and no other information (redact the conductors) | 22:15 |
JayF | I think I've caught up to you | 22:15 |
JayF | lol | 22:15 |
JayF | cid: ^ that might be an RFE you could attach to that bug we triaged during your 1:1 | 22:16 |
JayF | cid: if so, it'd be a good first change for your friend who wanted to get involved :) | 22:17 |
TheJulia | we can basically change the overall role and make it admin only or something for the conductors | 22:17 |
TheJulia | or system access only | 22:17 |
TheJulia | the shifts in meaning in the overall rbac model don't help us here at all | 22:17 |
JayF | my thought was: system scope manager/admin -> you get unredacted conductors | 22:18 |
JayF | project scope manager/admin -> you get redacted conductors but driver names | 22:18 |
JayF | I thought that's what you were suggesting? | 22:18 |
JayF | and service ... well I don't really get the idea of service scope so probably treat like admin :D | 22:18 |
dking | I've been a bit out of the loop lately, but I just went to build a new image, and I'm getting import errors from ironic_python_agent. It's a change that happened since last week. | 22:18 |
JayF | logs? | 22:19 |
dking | Sorry to pop in the middle of the other conversation. I accidentally hit send. | 22:19 |
cid | JayF: Anything RBAC may not be a good first issue, from experience | 22:19 |
JayF | you would likely know better than most :) | 22:20 |
cid | I'd like to think so :D. I will take a closer look later. | 22:20 |
JayF | either way, one of us three should summarize this conversation ... essentially we /could/ allow less-privledged users to see drivers without seeing other valuable system information (like conductor hostnames) | 22:21 |
cid | ++ | 22:21 |
JayF | that would at least let us move the bug outta ironic-ui into something more maintained lol | 22:24 |
dking | While folks are still here, if I'm not interrupting and If anybody has a moment, were there any changes recently that folks know about which were likely to cause issues with disk_utils or qemu_img in ironic_python_agent? | 22:25 |
dking | The first error, from where I had a version locked to 9.12.0 was: ImportError: cannot import name 'qemu_img' from 'ironic_lib' After removing the version lock, at version 9.14.0, I get a traceback shortly after the same import ending in: oslo_config.cfg.NoSuchOptError: no such option disk_utils in group [DEFAULT] | 22:27 |
TheJulia | JayF: that was what I was thinking | 22:30 |
JayF | dking: I suspect you're mixing versions of IPA/Ironic-lib | 22:31 |
TheJulia | cid: a test or two can do amazing things. On your original chagne you mirrored some of the owner matching logic, it is just extra context you didn't have. Now you have it :) | 22:31 |
JayF | dking: are you patching anything? or building from git rather than from pypi? This is what happens when you have an older ironic-lib and newer IPA (or vice versa) | 22:31 |
JayF | dking: this change happened around the qemu CVE issue | 22:31 |
JayF | (we know this has to work in the happy path because otherwise CI wouldn't have passed) | 22:32 |
opendevreview | Adam McArthur proposed openstack/ironic-tempest-plugin master: Testing bad microversions on v1/allocations https://review.opendev.org/c/openstack/ironic-tempest-plugin/+/935827 | 22:32 |
JayF | TheJulia: cid: TBH I hadn't tied the comment and that regression together :) you can't write code in a complex system without writing bugs sometimes... I tell myself this daily | 22:32 |
dking | JayF: That could be a consideration for when I build the images, but I'm also getting the error when I even try to run my unit tests. I only list ironic-python-agent in my requirements, and that package is what installs the others. | 22:33 |
JayF | Are you using the proper constraints file? | 22:34 |
cid | TheJulia, I do now for real (around that at least). | 22:34 |
JayF | e.g. https://github.com/openstack/ironic-python-agent/blob/stable/2024.2/tox.ini#L16 | 22:34 |
dking | I am using Python 3.9, which is what ironic-python-agent-builder was installing when I last checked, which is old and maybe things are finally using a newer python version now? | 22:34 |
JayF | that's what ensures you get the supported versions of the things | 22:35 |
dking | JayF: Where would I get the official file? That might help. | 22:35 |
JayF | see the linked code :) | 22:35 |
JayF | just use the same URL tox is using, adjusted for your branch | 22:35 |
JayF | BTW; you should do this for /all openstack deliverables/ if you want to be in the tested path :) | 22:36 |
JayF | and if you're talking about 2024.2, https://governance.openstack.org/tc/reference/runtimes/2024.2.html python 3.9 is still supported | 22:36 |
dking | Is that still the default that ironic-python-agent-builder uses for the image? | 22:40 |
dking | I'm testing with the constraints file now. I build for my tests inside a container, which helps with the cleanup. | 22:40 |
JayF | I think for our image we use the default python that's installed for that distro | 22:44 |
JayF | but imbw | 22:44 |
JayF | which is likely py3.10, but again, it should work on any supported, and we unit tested that version from 3.9->3.11 | 22:44 |
dking | JayF: Well, I did find some things I had versioned locked that needed to be let up, but then even with the constraints file I'm still getting the same error. | 22:53 |
JayF | what exact upstream branch are you using? | 22:54 |
JayF | I guess it's possible if you're on an old enough branch we could have something that broke out from under us | 22:54 |
JayF | and you're not using any kind of downstream fork? | 22:55 |
dking | Currently, I'm in a docker container (python:3.9) running tox, and in my requirements.txt I have: ironic-python-agent, oslo_log, oslo_concurrency, pynetbox, and a few other things that shouldn't relate to Ironic. I've added the constraints file. The error comes when I'm running python -m unittest. I'm not using any local branches, except for my own code which imports those things. | 22:58 |
JayF | python -m unittest is not how to run Ironic tests :) | 22:59 |
JayF | I don't know if that's /the problem/ but it may muddy the waters | 22:59 |
JayF | `stestr run` is the method we support/test | 23:00 |
JayF | If you're modifying the requirements.txt I suspect that's more likely the root cause though, but without exact output and knowledge of what exact code you're running this problem is ~impossible to troubleshoot | 23:01 |
JayF | (and given my experience from running unit tests in my IDE -- which uses unittest module -- sometimes the ordering gets funky which I would believe could cause this) | 23:01 |
dking | I think I may have not communicated clearly. I'm building my own hardware manager. It fails during the unit tests (and also on the built IPA image). The same build worked last week. I re-ran my previously passing CI to confirm it wasn't from a local change. | 23:02 |
JayF | You've still not indicated what version you're working agains.t | 23:03 |
dking | Currently, when tox does a pip install of ironic-python-agent, it's getting 9.14.0 | 23:04 |
JayF | and you're using the 2024.2 constraints? | 23:06 |
TheJulia | dking: latest from whatever is in pip? | 23:06 |
JayF | https://opendev.org/openstack/releases/src/commit/ea50baad3a49f6c2eb2fede09f2d7b64c3221c94/deliverables/dalmatian/ironic-python-agent.yaml#L22 | 23:07 |
JayF | it's literally not changed a lick :/ | 23:07 |
JayF | since 2024.2 | 23:07 |
JayF | which is sad for it's own reasons because we need to push a release of that, there's a regression fix in the repo that's not in the release | 23:07 |
* JayF trying HEAD of stable/2024.2 w/ tox -epy39 | 23:08 | |
dking | TheJulia: Correct, whatever pip installs. It's currently installing 9.14.0, from what it looks like. | 23:09 |
JayF | Ran: 1027 tests in 4.8322 sec. | 23:09 |
JayF | from HEAD stable/2024.2, newly created environment | 23:09 |
JayF | all passing | 23:09 |
JayF | Is it possible you're doing multiple install calls? | 23:09 |
dking | Well, that's good to know. | 23:10 |
JayF | `pip install -c constraints.txt package` `pip install -c constraints.txt package2` will not behave the same as doing them all on one line | 23:10 |
JayF | because the second run will happily clobber versions the first one wants | 23:10 |
dking | I suppose that is possible that the same requirements.txt is called twice. | 23:10 |
JayF | yep | 23:10 |
JayF | I suspect that may be somewhat at the heart of it | 23:11 |
JayF | pip is extremely finicky about version installing | 23:11 |
TheJulia | yeah, so it is pulling in latest ironic lib released a week ago which has things removed | 23:11 |
TheJulia | so basically latest IPA release doesn't work with latest ironic-lib release as-is | 23:12 |
TheJulia | and that is all pip handling versioning, outside of development | 23:12 |
JayF | I'm confused as to how that's not breaking ipa unit tests in my test, then | 23:12 |
JayF | did we cut a bugfix release of ironic-lib without cutting a bugfix ipa? | 23:12 |
TheJulia | because when you run unit tests, it apply constraint logic | 23:12 |
TheJulia | which is *only* known to the runner | 23:12 |
TheJulia | *not* pypi | 23:12 |
TheJulia | or the ipa package itself | 23:12 |
TheJulia | JayF: sure looks like it... | 23:13 |
JayF | ugh, looks like they automatically cut one for ironic-lib | 23:13 |
JayF | https://opendev.org/openstack/releases/commit/2d4e54aede724bc8aded603bdda839ada459af80 | 23:14 |
dking | Are you saying that my issue might be legit? | 23:14 |
JayF | that should've been nack'd by us, and should not be done in the future | 23:14 |
JayF | dking: I'm saying my original claim; you're not following constraints, is proven right :) | 23:14 |
JayF | dking: you're pulling in epoxy ironic-lib alongside dalmation ipa | 23:14 |
JayF | which is no bueno, and probably is somewhat our fault given this release scenario | 23:14 |
JayF | rpittau: https://review.opendev.org/c/openstack/releases/+/934598 please do not +1 ironic-lib releases without accompanying ironic/ipa releases, and we should ask releases team to stop making these milestone ones for ironic-lib | 23:15 |
TheJulia | I need to get going, but yeah... it is our fault. This is also why when ripping stuff from libraries, we tend to encourage a complete cycle for deprecation | 23:15 |
JayF | and we need to cut an IPA and Ironic release ASAP to unbreak people | 23:15 |
JayF | I don't think we can pull releases, can we? | 23:15 |
JayF | That's the real solution: that ironic-lib needs to be pulled | 23:15 |
TheJulia | we might be able to | 23:15 |
TheJulia | yeah | 23:16 |
TheJulia | putting laptop in bag | 23:16 |
dking | Thank you all very much. Let me know if that gets pulled upstream. | 23:16 |
JayF | yeah, but do be careful to use the constraints | 23:17 |
JayF | I'm glad you found this but there are times this can happen and it not be bugged | 23:18 |
dking | JayF: Yeah, it's helpful for them to be set. I've added them now. Thank you. | 23:22 |
JayF | I was very, very confused because we never release ironic-lib without IPA/Ironic | 23:22 |
JayF | and your error mimicked something I had wrestled against (and resolved) REPEATEDLY during the CVE shenanigans | 23:22 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!