rpittau | good morning ironic! o/ | 07:50 |
---|---|---|
dtantsur | TheJulia: left a question on the HTTP boot patch, ptal | 09:51 |
opendevreview | Merged openstack/python-ironicclient master: Update python classifier in setup.cfg https://review.opendev.org/c/openstack/python-ironicclient/+/905813 | 10:12 |
*** dmellado747 is now known as dmellado74 | 10:58 | |
iurygregory | good morning Ironic | 11:04 |
*** dmellado745 is now known as dmellado74 | 11:07 | |
opendevreview | Merged openstack/ironic master: Validate [deploy] image_server_auth_strategy https://review.opendev.org/c/openstack/ironic/+/905639 | 11:18 |
opendevreview | Merged openstack/ironic-python-agent-builder stable/2023.2: Drop TripleO job https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/905725 | 11:25 |
opendevreview | Merged openstack/ironic-lib master: [codespell] - Adding CI for codespell tox target https://review.opendev.org/c/openstack/ironic-lib/+/905624 | 11:29 |
opendevreview | Merged openstack/ironic-python-agent-builder stable/2023.1: Drop TripleO job https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/905726 | 11:38 |
opendevreview | Merged openstack/ironic-python-agent-builder stable/yoga: Drop TripleO job https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/905833 | 11:38 |
opendevreview | Merged openstack/ironic-python-agent-builder stable/xena: Drop TripleO job https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/905834 | 11:38 |
iurygregory | dtantsur, https://review.opendev.org/c/openstack/ironic/+/905626 is ready? (I know it worked for honza, just wondering if you agree with zane's comment) =) happy to +2 it already | 11:38 |
dtantsur | iurygregory: I'll check the comments first, hold on please (need more coffee) | 11:39 |
iurygregory | dtantsur, sure take your time | 11:39 |
iurygregory | I'm adding to my radar and adding the hashtag to it | 11:39 |
*** priteau_ is now known as priteau | 12:14 | |
iurygregory | dtantsur, just checking but https://github.com/openstack/ironic-python-agent-builder/commit/9da0cf41d446e11651a5bd8b111cfd13a02d76b6 is the fix for https://bugs.launchpad.net/ironic-python-agent-builder/+bug/2043112 ? | 13:29 |
dtantsur | iurygregory: yep. the automation does not close these bugs properly.. | 13:31 |
iurygregory | dtantsur, I think you missed in the commit message .-. | 13:31 |
dtantsur | Ah, right | 13:31 |
dtantsur | But I did see this issue with other patches | 13:31 |
dtantsur | IIRC an IPA-builder patch actually closed the Ironic half of the bug. Or something like that. | 13:32 |
dtantsur | Anyway, thanks for tracking it | 13:32 |
iurygregory | yw =), ack I will keep an eye to see if I can find the ones that automation failed | 13:32 |
iurygregory | yup automation is a bit crazy indeed https://bugs.launchpad.net/networking-baremetal/+bug/2046962 https://review.opendev.org/c/openstack/networking-baremetal/+/903995/ | 13:37 |
iurygregory | JayF, https://bugs.launchpad.net/ironic-python-agent/+bug/2047654 I think we can mark as Fix Released | 13:41 |
iurygregory | https://bugs.launchpad.net/ironic/+bug/2049754 this one is interesting, maybe there is something missconfigured in Kolla ? | 13:51 |
opendevreview | Merged openstack/ironic-python-agent-builder stable/zed: Drop TripleO job https://review.opendev.org/c/openstack/ironic-python-agent-builder/+/905832 | 14:04 |
JayF | iurygregory: was going to use that to track across all repos but I'm okay if it's closed during that | 14:11 |
iurygregory | JayF, it makes sense to re-use across all repos, I will add some of them and mark ipa as released | 14:14 |
TheJulia | dtantsur: replied | 14:58 |
dtantsur | thanks, will check a bit later (was not a blocking concern) | 15:11 |
TheJulia | It could be reasonable to enable such a mode as a flag or config, just I wouldn't do it out of the box given some of the input we've gotten and context we have. | 15:12 |
TheJulia | if that is *just* "hey, use UEFIHTTP instead of PXE" | 15:12 |
iurygregory | https://bugs.launchpad.net/ironic/+bug/2021995 the only patch for ironic was https://review.opendev.org/c/openstack/ironic/+/885549, so I think we can mark as Fix Released? | 15:30 |
rpittau | iurygregory: sounds good | 15:44 |
opendevreview | Dmitry Tantsur proposed openstack/ironic master: Don't create a hardlink to a symlink when handling file:// URLs https://review.opendev.org/c/openstack/ironic/+/905626 | 15:51 |
dtantsur | iurygregory: finally ^^ | 15:51 |
JayF | that pr topic just startled some grizzled unix admin out of a nightmare | 16:01 |
JayF | lol | 16:01 |
iurygregory | dtantsur, ack =) | 16:02 |
iurygregory | anyone has thoughts on https://bugs.launchpad.net/ironic-inspector/+bug/1635253 ? I think we already do this, just wanted to confirm (since it says that inspector change is in progress but I couldn't find any lol) | 16:02 |
dtantsur | No idea if an inspector change was even needed | 16:03 |
rpittau | the inspector change probably was not needed | 16:04 |
iurygregory | tks! | 16:08 |
Sandzwerg[m] | <TheJulia> "Sandzwerg: nothing we would be..." <- I really have no clue. I'm not aware of any automation but I could be wrong. Need to dig a bit deeper into a DHCP issue though. | 16:36 |
TheJulia | hmmmm | 16:41 |
TheJulia | okay | 16:41 |
*** tosky_ is now known as tosky | 16:41 | |
Sandzwerg[m] | Like these are the first nodes we try with redfish only. Although switching to redfish+ipxe worked without any issues. So maybe I'm missing something important or I'm holding something wrong | 16:44 |
rpittau | good night! o/ | 16:52 |
TheJulia | dtantsur: w/r/t inspector, how do you feel about https://review.opendev.org/c/openstack/ironic-inspector/+/906011/2/ironic_inspector/cmd/__init__.py ? | 17:12 |
JayF | Uugh, if we can't do 906012, 906011 is also really not okay either | 17:19 |
JayF | for purposes of me having a complete thought to bring to itamar downstream when I WTF in his direction about this whole thing; what's the reason we can't/don't monkey_patch() in wsgi caseS? | 17:21 |
JayF | I know we don't; but I don't remember why/what the issue is with doing so | 17:21 |
TheJulia | well, it at least isolates down to the command file invocation | 17:23 |
JayF | I also know how wsgi entrypoints are being done is changing, I think next cycle with PBR changes | 17:23 |
JayF | and eventlet U-C update was pushed | 17:23 |
JayF | so I wonder if there's an intersecting point there where we can dodge this problem | 17:23 |
JayF | pushed->reverted | 17:27 |
TheJulia | ... not sure I follow your train of thought there without further context as to what your thinking | 17:27 |
dtantsur | TheJulia: ironic_inspector.cmd.wsgi:initialize_wsgi_app | 17:28 |
dtantsur | we cannot use cmd either, unless we move the script | 17:28 |
TheJulia | so we've had it wrong all along then | 17:29 |
TheJulia | and we've been monkey patching eventlet in wsgi all along | 17:29 |
dtantsur | \o/ | 17:31 |
JayF | Which means either: 1) we've had bugs or 2) the idea we can't was wrong | 17:31 |
dtantsur | Could be the source of some weird issues we've had | 17:31 |
dtantsur | Well. Some WSGI containers are compatible with eventlet. Gunicorn, for instance, has an option for that. | 17:31 |
dtantsur | It's possible that even an eventlet-unaware container can more or less work. But that's an uncharted territory, especially in the presence of OS threads. | 17:32 |
TheJulia | I well, we could move it to each individual file *except* the wsgi one | 17:34 |
TheJulia | well, where it makes sense | 17:34 |
TheJulia | for autodoc though, we would *still* need to catch RuntimeError | 17:34 |
JayF | How does this work in Ironic? | 17:34 |
JayF | Ironic doesn't have this class of issue, right? | 17:34 |
TheJulia | ironic doesn't invoke werkzeug | 17:34 |
dtantsur | ironic.api.wsgi:initialize_wsgi_app - not under cmd | 17:35 |
TheJulia | it likely gets autodoc'ed though | 17:35 |
JayF | so ironic autodoc does not get monkey_patched and it doesn't matter | 17:35 |
JayF | it does in inspector because werkzeugt | 17:35 |
JayF | hmm | 17:35 |
dtantsur | Ironic monkey patches in | 17:35 |
TheJulia | yup, it gets loaded somewhere along the path of autodoc's run and when it sees the monkey patch attempt it blows up | 17:35 |
JayF | I wonder if instead of autodoc_fake_import = [eventlet] I could autodoc_fake_import = [werkzeug] and get something to work | 17:35 |
dtantsur | $ ag -l monkey_p | 17:36 |
dtantsur | ironic/cmd/__init__.py | 17:36 |
dtantsur | ironic/tests/unit/__init__.py | 17:36 |
dtantsur | ironic/tests/base.py | 17:36 |
dtantsur | and doc/source.conf.py | 17:36 |
JayF | doc/source/conf.py is *in the docs runner* not in the autodocs run | 17:36 |
JayF | which is a separate python instance | 17:36 |
JayF | (sadly, or else it'd be the easy fix for inspector) | 17:36 |
TheJulia | le sigh | 17:36 |
JayF | I am like .00001% tempted to try and monkey patch sphinx autodoc to call monkey_patch() itself before going the autodoc'ing lol | 17:37 |
dtantsur | :D | 17:37 |
dtantsur | Good that we did not finish the migration of Ironic to Flask | 17:38 |
JayF | Eh, this is a speed bump, we can figure this out. | 17:38 |
JayF | I'm at least for once happy to have a problem that 1) reproduces every time 2) without a $xx,xxx piece of hardware | 17:39 |
JayF | even if it's *this one* | 17:39 |
TheJulia | I'm just unhappy it blocks me until we've got it sorted out | 17:39 |
JayF | Could we do something more drastic? Disable autodoc on inspector? | 17:40 |
JayF | I would prefer that to the PR to except RuntimeError | 17:40 |
TheJulia | ... we could actually, we've largely signaled "it is going away" | 17:40 |
dtantsur | Given the inspector's status... possibly | 17:41 |
dtantsur | yeah | 17:41 |
dtantsur | Some links are going to be broken, both inside and outside the docs | 17:41 |
JayF | I *think* there's a middle ground here | 17:41 |
JayF | where we could have autodoc generate some files we could commit to source/ | 17:41 |
dtantsur | meanwhile, https://review.opendev.org/c/openstack/ironic/+/905626 has 2x +2, I can haz a W+1? | 17:41 |
TheJulia | I'm not sure it would be a ton, but we could commit the source from the folder | 17:41 |
TheJulia | ... maybe | 17:41 |
TheJulia | errr | 17:41 |
TheJulia | no, that is just pointer details | 17:41 |
JayF | dtantsur: yeah will +2a | 17:41 |
JayF | TheJulia: that might be the trick | 17:42 |
JayF | TheJulia: if we tell it to autodoc specific modules, but not ironic_inspector.where_we_monkey_patch that might do the trick? | 17:42 |
dtantsur | it's quite possible that we only care about the plugin (= hooks) API | 17:42 |
JayF | that is not awesome | 17:42 |
TheJulia | JayF: afaik it is not in the option list for autodoc | 17:42 |
TheJulia | well, exclude is not from what I saw | 17:43 |
JayF | TheJulia: dtantsur: I am +1 to nv'ing the inspector docs job OR disabling autodoc to unblock | 17:43 |
JayF | and I will take a priority action today to try and find a softer landing? | 17:43 |
JayF | either from a tech side on making autodoc work, or on a side of mitigating downsides from turning it off | 17:43 |
* dtantsur needs to go now, trusting you to make the right call | 17:43 | |
JayF | but lets not block work on a docs build issue when we can just disable the job and have docs remain up while we fix the root cause | 17:44 |
TheJulia | I need to step outside, once done with that, I can turn off autodoc and see where we're at | 17:44 |
TheJulia | or, actually | 17:44 |
TheJulia | we could just outright disable docs builds completely via nv-ing it for now | 17:44 |
TheJulia | well, just don't gate on it at the moment | 17:44 |
JayF | that's what I'm saying | 17:44 |
TheJulia | okay, that is easy | 17:45 |
JayF | the nv is probably the better route for unblocking now | 17:45 |
JayF | I'll put an ironic meeting agenda item about it | 17:45 |
JayF | just so we have a pin in place for it not to get dropped | 17:45 |
TheJulia | errr | 17:45 |
TheJulia | docs come from the pti template | 17:45 |
* TheJulia wonders how we override a failing docs job | 17:45 | |
JayF | # spinhx-build -fdsgsfdgsfd | 17:45 |
JayF | in tox.ini ;) | 17:45 |
JayF | or `echo sphinx-build -faflhgsgkdfjghs` | 17:46 |
JayF | pick your poison | 17:46 |
TheJulia | its not a matter of changing the flag | 17:46 |
JayF | I'm saying make it noop | 17:46 |
TheJulia | since it is a RuntimeError which gets raised | 17:46 |
TheJulia | oh, heh | 17:46 |
JayF | but that doesn't matter, it'll still run in post and try to publish | 17:46 |
JayF | which we do need to disable | 17:46 |
TheJulia | well post failing isn't awful, as long as it doesn't wipe out what has already been posted | 17:56 |
JayF | well that's more my concern; if we have a tox job run that doesn't populate build/ | 17:57 |
JayF | will it just happily upload a blank | 17:57 |
* TheJulia tries locally | 17:58 | |
JayF | TheJulia: I'd suggest asking g mann or someone opendev before going too far down the rabbithole yourself, we can't be the first folks to wanna do this | 17:58 |
TheJulia | I just explicitly asked, since noop state might blow up publishing in bad ways we can't predict without knowing the fine details there | 18:01 |
TheJulia | and fixing it afterwards right now wouldn't be happy path | 18:01 |
opendevreview | Julia Kreger proposed openstack/ironic-inspector master: CI: Disable docs voting for now https://review.opendev.org/c/openstack/ironic-inspector/+/906059 | 18:07 |
JayF | TheJulia: that proposed patch doesn't disable the post job, I think? | 18:11 |
TheJulia | post job can silently fail and that won't break us afaik | 18:11 |
JayF | ack; that makes sense | 18:12 |
TheJulia | and CI didn't fail on the change, but nor did it fail overall | 18:33 |
TheJulia | There is also a distinct lack of t'pol videos of her going "interesting" or "facinating" | 18:35 |
iurygregory | should I +W https://review.opendev.org/c/openstack/ironic-inspector/+/906059 in this case? | 18:45 |
TheJulia | dunno | 18:45 |
TheJulia | I rechecked https://review.opendev.org/c/openstack/ironic-inspector/+/905353 | 18:46 |
iurygregory | ack | 18:46 |
TheJulia | and the docs job did pass | 18:56 |
TheJulia | so... \o/ | 18:56 |
frickler | well the eventlet bump in u-c got reverted | 18:59 |
TheJulia | ahh, so percieved bugs -1 | 19:03 |
opendevreview | Verification of a change to openstack/ironic master failed: Don't create a hardlink to a symlink when handling file:// URLs https://review.opendev.org/c/openstack/ironic/+/905626 | 19:21 |
JayF | TheJulia: yeah, I'm still going to wire up a master eventlet job to exercise the failure though | 20:25 |
JayF | TheJulia: https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#skipping-members WDYT about a `ironic_inspector/eventlet_patcher/__init__.py` which, on import, monkey-patches. We then import that first thing from the places we monkey-patch currently, and we set skip-members for the eventlet_patcher module | 20:26 |
JayF | I've not tested it but have been reading code and docs and don't see a reason this wouldn't work | 20:26 |
opendevreview | Merged openstack/ironic stable/2023.1: Revert "Revert "RBAC: Fix allocation check"" to use Unauthorized https://review.opendev.org/c/openstack/ironic/+/905902 | 20:52 |
opendevreview | Merged openstack/ironic-inspector master: Fix RBAC access for service/admin user access https://review.opendev.org/c/openstack/ironic-inspector/+/905353 | 21:09 |
opendevreview | Jay Faulkner proposed openstack/ironic-inspector master: Add jobs targetting eventlet master to exp queue https://review.opendev.org/c/openstack/ironic-inspector/+/906070 | 22:57 |
opendevreview | Jay Faulkner proposed openstack/ironic-inspector master: Add jobs targetting eventlet master to exp queue https://review.opendev.org/c/openstack/ironic-inspector/+/906070 | 23:10 |
opendevreview | Jay Faulkner proposed openstack/ironic-inspector master: Add jobs targetting eventlet master to exp queue https://review.opendev.org/c/openstack/ironic-inspector/+/906070 | 23:29 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!