| opendevreview | Merged openstack/ironic master: Use driver_internal_info methods for ilo driver https://review.opendev.org/c/openstack/ironic/+/818507 | 02:35 |
|---|---|---|
| opendevreview | Merged openstack/ironic master: Use driver_internal_info methods for drac driver https://review.opendev.org/c/openstack/ironic/+/818506 | 02:38 |
| *** pmannidi is now known as pmannidi|AFK | 06:04 | |
| arne_wiebalck | Good morning, Ironic! | 07:39 |
| opendevreview | Aija Jauntēva proposed openstack/ironic master: Fix prepare ramdisk for 'wait' states https://review.opendev.org/c/openstack/ironic/+/823311 | 08:29 |
| opendevreview | Aija Jauntēva proposed openstack/ironic master: Fix Redfish RAID for non-immediate controllers https://review.opendev.org/c/openstack/ironic/+/823312 | 08:29 |
| rpittau | good morning ironic! o/ | 08:31 |
| *** mnasiadka_ is now known as mnasiadka | 08:43 | |
| opendevreview | Riccardo Pittau proposed openstack/ironic bugfix/18.1: Use stable/xena upper-constraints https://review.opendev.org/c/openstack/ironic/+/824451 | 08:48 |
| *** sshnaidm|afk is now known as sshnaidm | 08:49 | |
| opendevreview | Aija Jauntēva proposed openstack/ironic master: Fix validating input for redfish update_firmware https://review.opendev.org/c/openstack/ironic/+/823701 | 09:04 |
| opendevreview | Riccardo Pittau proposed openstack/ironic bugfix/19.0: Fix Redfish RAID deploy steps https://review.opendev.org/c/openstack/ironic/+/824425 | 09:07 |
| opendevreview | Aija Jauntēva proposed openstack/ironic master: Fix validating input for redfish update_firmware https://review.opendev.org/c/openstack/ironic/+/823701 | 09:11 |
| opendevreview | Riccardo Pittau proposed openstack/bifrost master: Follow up to "Run bifrost on CentOS Stream 9" https://review.opendev.org/c/openstack/bifrost/+/824186 | 09:13 |
| arne_wiebalck | hey rpittau o/ | 09:22 |
| rpittau | hey arne_wiebalck :) | 09:22 |
| arne_wiebalck | dtantsur: TheJulia: re missing/dropping quotes on etags (which is against the http standard), we have followed up with the vendor/manufacturer and they confirm this is indeed a bug which will be fixed in the upcoming release of their firmware | 09:24 |
| rpittau | that's good news. hopefully it'll be ready soon :) | 09:26 |
| dtantsur | morning ironic | 09:33 |
| dtantsur | arne_wiebalck: \o/ | 09:33 |
| dtantsur | TheJulia: sorry, I have to leave early on Wednesdays | 09:34 |
| arne_wiebalck | rpittau: end of March it seems | 09:34 |
| opendevreview | Merged openstack/ironic bugfix/18.1: Add dhcp options for each ip_version once https://review.opendev.org/c/openstack/ironic/+/824015 | 09:51 |
| opendevreview | Merged openstack/ironic bugfix/18.1: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824167 | 09:52 |
| opendevreview | Aija Jauntēva proposed openstack/ironic master: Add more sources to redfish firmware upgrade https://review.opendev.org/c/openstack/ironic/+/822781 | 10:07 |
| opendevreview | Dmitry Tantsur proposed openstack/ironic master: Do not fail inspection on invalid MAC https://review.opendev.org/c/openstack/ironic/+/824523 | 10:33 |
| opendevreview | Verification of a change to openstack/ironic bugfix/19.0 failed: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824166 | 10:35 |
| opendevreview | Verification of a change to openstack/ironic bugfix/19.0 failed: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824166 | 10:54 |
| iurygregory | good morning Ironic o/ | 11:26 |
| opendevreview | Merged openstack/ironic stable/xena: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824163 | 11:48 |
| opendevreview | Merged openstack/ironic stable/wallaby: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824164 | 11:56 |
| opendevreview | Verification of a change to openstack/ironic stable/victoria failed: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824165 | 11:56 |
| *** outbrito_ is now known as outbrito | 12:02 | |
| opendevreview | Merged openstack/ironic bugfix/18.1: Clarify driver load error message https://review.opendev.org/c/openstack/ironic/+/819678 | 12:03 |
| opendevreview | Merged openstack/ironic bugfix/19.0: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824166 | 13:35 |
| *** emilien-oftc is now known as EmilienM | 13:35 | |
| opendevreview | Merged x/sushy-oem-idrac master: Migrate constants to enums https://review.opendev.org/c/x/sushy-oem-idrac/+/817028 | 13:43 |
| rpittau | this https://review.opendev.org/c/openstack/ironic/+/824451 should provide a definitive fix for 18.1 branch CI, at least from dependencies perspective | 13:44 |
| iurygregory | I will review after the meetings I have in my plate =) | 14:23 |
| TheJulia | good morning | 14:39 |
| iurygregory | good morning TheJulia =) | 14:49 |
| dtantsur | TheJulia: morning | 14:54 |
| rpittau | good morning TheJulia :) | 14:54 |
| TheJulia | dtantsur: no worries | 15:00 |
| TheJulia | dtantsur: when you have a moment, could you take a glance at https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/824247 ? We've issues being driven by wsgi + haproxy + python3 + eventlet which we need to route aroudn as it has ground much of downstream work to a halt | 15:01 |
| dtantsur | I will, just let me have the (overdue) lunch first | 15:02 |
| TheJulia | okay, no worries | 15:02 |
| TheJulia | Thanks! | 15:02 |
| TheJulia | o/ stendulker | 15:11 |
| stendulker | TheJulia: o/ | 15:15 |
| dtantsur | TheJulia: is there a reason we don't have to do https://review.opendev.org/c/openstack/python-ironic-inspector-client/+/824247/3/ironic_inspector_client/common/http.py in ironicclient/openstacksdk/etc? | 15:41 |
| TheJulia | dtantsur: we actually return bodies in almost everything else | 15:41 |
| TheJulia | inspector is unique in empty bodies afaik | 15:41 |
| dtantsur | mmm, not PUT /states/...? | 15:41 |
| TheJulia | we return a body in ironic | 15:41 |
| TheJulia | remember a while back where I was like, why don't we just change inspector to return an empty dict :) | 15:42 |
| dtantsur | heh | 15:42 |
| dtantsur | TheJulia: https://docs.openstack.org/api-ref/baremetal/?expanded=change-node-power-state-detail#change-node-power-state doesn't mention any body though | 15:43 |
| TheJulia | https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/node.py#L1121-L1122 <-- has me wondering why we do that | 15:59 |
| dtantsur | TheJulia: I think HTTP ACCEPTED *should* (even *must*? dunno) have a Location | 15:59 |
| TheJulia | I *think* it is multi-cases | 16:00 |
| TheJulia | err, multi-cased | 16:00 |
| TheJulia | different frameworks perhaps, I wonder what exactly we return as headers | 16:01 |
| dtantsur | hmm, https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location | 16:01 |
| TheJulia | got a running ironic someplace? | 16:01 |
| dtantsur | yep | 16:01 |
| TheJulia | I did read someplace in some browsers can honor 204 with location. Or maybe it was 202 | 16:02 |
| dtantsur | I would expect 202, but MDN does not mention it | 16:02 |
| TheJulia | sounds a lot kike content-length support | 16:02 |
| TheJulia | apparently the only thing that *doesn't* support is is Safari's engine | 16:02 |
| dtantsur | okay, what to check in my ironic? | 16:03 |
| TheJulia | dtantsur: if you wouldn't mind | 16:04 |
| TheJulia | but https://github.com/openstack/ironic/blob/aed88ed93e5147f2d4ab5efc3c7fcd93076fae93/ironic/api/method.py#L63-L81 | 16:04 |
| TheJulia | I could have sworn that that endpoint returns an emtpy {} | 16:05 |
| TheJulia | Inspectors wsgi framework is different though | 16:06 |
| dtantsur | TheJulia: https://paste.opendev.org/show/812095/ | 16:06 |
| TheJulia | Content-Length: 0 is what prevents it from happening on the encoding issue | 16:10 |
| TheJulia | but the framework overrides that | 16:10 |
| TheJulia | and prevents it from being set I believe | 16:10 |
| dtantsur | wait, we don't return Content-Length in inspector? Oo | 16:11 |
| TheJulia | nope, the framework prevents it | 16:12 |
| TheJulia | it also likes to make the encoding chunked | 16:13 |
| dtantsur | okay, I think that's the actual bug we should really, really fix | 16:13 |
| TheJulia | yeah, but we're literally dead in the water downstream | 16:13 |
| dtantsur | mmm? | 16:13 |
| TheJulia | none of our QA folks are able to use inspector period | 16:13 |
| dtantsur | I guess you can cherry-pick this inspector patch if in hurry, but what about other clients? | 16:14 |
| dtantsur | I think not returning content-length may upset many potential consumers | 16:14 |
| * dtantsur is really curious WHY | 16:14 | |
| TheJulia | it is content length or encoding pattern | 16:14 |
| TheJulia | unfortunately 202 and 204 seem to be the super weird edge case | 16:14 |
| dtantsur | "A user agent SHOULD send a Content-Length in a request message when no Transfer-Encoding is sent and the request method defines a meaning for an enclosed payload body. For example, a Content-Length header field is normally sent in a POST request even when the value is 0 (indicating an empty payload body)." | 16:15 |
| dtantsur | A server MUST NOT send a Content-Length header field in any response with a status code of 1xx (Informational) or 204 (No Content). | 16:16 |
| dtantsur | oh heck | 16:16 |
| TheJulia | lol | 16:16 |
| dtantsur | HTTP is a bloody disaster, who even thought restful APIs were a good idea? | 16:16 |
| TheJulia | maybe that is why we're getting flask injecting chunked encoding | 16:16 |
| dtantsur | "A server MUST NOT send a Transfer-Encoding header field in any response with a status code of 1xx (Informational) or 204 (No Content)." :) | 16:17 |
| * TheJulia tosses wet cats at everything | 16:17 | |
| rpittau | "wet" cats? :D | 16:18 |
| TheJulia | Yes, mordred used to claim to toss wet cats at people | 16:18 |
| TheJulia | although, for all the times he said it to me, they never appeared. Curious ;) | 16:18 |
| *** sshnaidm is now known as sshnaidm|afk | 16:18 | |
| dtantsur | yeah, I've also not seen it myself | 16:19 |
| TheJulia | at one point, I believe after he shifted to mostly sdk stuff, he stopped using the expression | 16:19 |
| rpittau | no more cats to throw | 16:20 |
| TheJulia | Last photo I saw of him on facebook I believe had 8 or 9 kittens in it | 16:20 |
| rpittau | fantastic :D | 16:20 |
| dtantsur | ah, right, he started mass-fostering | 16:20 |
| dtantsur | TheJulia: have you figured out if it's flask or mod_wsgi? | 16:22 |
| TheJulia | not sure yet | 16:22 |
| TheJulia | currently looking at werkzeug | 16:22 |
| TheJulia | so one conudnrum is content encoding | 16:24 |
| TheJulia | the transfer-encoding is a hop to hop flag, so things are allowed to modify it along the path | 16:24 |
| TheJulia | so I see the 204 being excluded | 16:25 |
| dtantsur | I don't think it's 100% correct | 16:25 |
| dtantsur | you cannot switch from transfer-encoding to content-length and whole body | 16:25 |
| dtantsur | (unless you really want to cache the whole body first) | 16:25 |
| TheJulia | you can if you download the entire thing into a proxy and modify it | 16:26 |
| TheJulia | https://github.com/pallets/werkzeug/blob/f7b53ee8a81f015c6c85659959004328237a3c8c/src/werkzeug/middleware/http_proxy.py#L132-L139 Hmm | 16:28 |
| TheJulia | but that shouldn't be int he code path | 16:28 |
| dtantsur | I don't think we use ProxyMiddleware | 16:32 |
| TheJulia | yeah, that is what I meant not in the code path | 16:32 |
| dtantsur | ah right | 16:32 |
| * TheJulia peers into the depths of mod-wsgi | 16:32 | |
| dtantsur | TheJulia: I assume I won't be able to reproduce the bug on bifrost because of no WSGI? | 16:33 |
| TheJulia | unlikely | 16:33 |
| TheJulia | actually, if you've got inspector, what do your headres look like since you just have a bare process | 16:34 |
| rpittau | bye everyone, I'm off tomorrow, see you on monday! o/ | 16:34 |
| dtantsur | have a good weekend rpittau | 16:35 |
| dtantsur | TheJulia: for which API call? | 16:35 |
| TheJulia | any of the empty body response api calls | 16:35 |
| TheJulia | the popular one is introspection rule delete | 16:35 |
| dtantsur | okie, let me just figure out the basic auth :D | 16:36 |
| TheJulia | seeing shell code compiled into a c program makes my head hurt | 16:37 |
| dtantsur | TheJulia: https://paste.opendev.org/show/812098/ | 16:38 |
| dtantsur | ffs | 16:38 |
| dtantsur | it does violate the RFC | 16:38 |
| dtantsur | I think we must fix that as a top priority | 16:39 |
| TheJulia | hmm | 16:39 |
| TheJulia | which means it is flask, or werkzueg | 16:40 |
| * TheJulia goes back to flask | 16:40 | |
| dtantsur | TheJulia: I really wonder if we do something weird in _generate_empty_response | 16:40 |
| TheJulia | harold tried reverting it to what we did before | 16:40 |
| TheJulia | and still the same thing happened | 16:40 |
| TheJulia | with what we did before, mod_wsgi was claiming the request had failed | 16:41 |
| TheJulia | which should have been a hint to the encoding | 16:41 |
| TheJulia | but.... | 16:41 |
| TheJulia | I'm trying to remember on what exactly apache was barfing on | 16:42 |
| TheJulia | hmmm so werkzug does force a content-type response | 16:48 |
| * TheJulia unlocks the work computer to see if a test env is still up | 16:50 | |
| TheJulia | oohhh ahhh | 16:51 |
| dtantsur | I'll try to reproduce the issue on a minimal flask installation | 16:59 |
| TheJulia | likely good, I just tried something I was hoping unfortuantely doesn't fix it | 16:59 |
| dtantsur | hmm, I cannot reproduce it on a simple example | 17:04 |
| dtantsur | what the hell | 17:06 |
| TheJulia | how did you generate the response? | 17:08 |
| dtantsur | TheJulia: https://paste.opendev.org/show/812101/ | 17:09 |
| TheJulia | wth indeed | 17:12 |
| dtantsur | interestingly, my simple example uses HTTP 1.0, I wonder why | 17:12 |
| TheJulia | your response body? | 17:13 |
| dtantsur | TheJulia: this is very different from what I see in bifrost: https://paste.opendev.org/show/812102/ | 17:13 |
| TheJulia | wtf | 17:14 |
| dtantsur | I wonder if it's something in wsgi_service | 17:17 |
| TheJulia | that feels unlikely, but... hmm | 17:20 |
| TheJulia | oh.... | 17:20 |
| dtantsur | yep, this is it | 17:21 |
| dtantsur | TheJulia: minimal reproducer: https://paste.opendev.org/show/812103/ | 17:22 |
| dtantsur | I wonder if it's coming from eventlet | 17:24 |
| dtantsur | yeah, it's even eventlet. it's ALWAYS eventlet. | 17:26 |
| dtantsur | TheJulia: a new, even minimaler reproducer: https://paste.opendev.org/show/812104/ | 17:28 |
| TheJulia | heh | 17:28 |
| TheJulia | yup, I'm reading through the eventlet wsgi code and it looks like it is entirely modeled around chunked encoding transfers | 17:28 |
| dtantsur | bloody eventlet. I wonder why ironic is not affected. | 17:29 |
| TheJulia | and it explicitly adds it if content-length is not in the header list | 17:29 |
| dtantsur | well, at least for 204 it's a violation of the RFE. wanna report it? | 17:29 |
| TheJulia | yeah, I'll open it, got a link to the RFC doc ? | 17:30 |
| dtantsur | TheJulia: https://httpwg.org/specs/rfc7230.html#header.transfer-encoding | 17:31 |
| TheJulia | Thanks | 17:31 |
| TheJulia | so Ironic returns a content-length header which bypasses the eventlet logic | 17:46 |
| TheJulia | I'll likely have to do the same in since it is the lesser evil | 17:46 |
| dtantsur | TheJulia: fwiw I tried that | 17:47 |
| TheJulia | hmm, I did notice we do it in a very particular way with ironic | 17:48 |
| TheJulia | I need to go find that again | 17:48 |
| dtantsur | yeah, my naive approach with response.headers did not work | 17:48 |
| arne_wiebalck | bye everyone, see you tomorrow o/ | 18:04 |
| dtantsur | o/ | 18:07 |
| opendevreview | Merged openstack/ironic stable/victoria: Ensure 'port' is up2date after binding:host_id https://review.opendev.org/c/openstack/ironic/+/824165 | 18:50 |
| opendevreview | Julia Kreger proposed openstack/ironic-inspector master: Return a content-length on HTTP204 to prevent client failures https://review.opendev.org/c/openstack/ironic-inspector/+/824643 | 20:02 |
| stevebaker[m] | good morning | 20:06 |
| TheJulia | good morning stevebaker[m]! | 20:07 |
| TheJulia | you'll love https://review.opendev.org/c/openstack/ironic-inspector/+/824643 | 20:08 |
| stevebaker[m] | TheJulia: I've just got through the backscroll | 20:12 |
| stevebaker[m] | wow | 20:13 |
| stevebaker[m] | TheJulia: so I bet even the latest eventlet does this | 20:14 |
| TheJulia | https://github.com/eventlet/eventlet/issues/746 | 20:15 |
| TheJulia | 0.33 released like 45 minutes ago | 20:15 |
| stevebaker[m] | well good sleuthing | 20:23 |
| TheJulia | Thank Dmitry, I didn't realize the headers were verboten on 204 | 20:35 |
| stevebaker[m] | I think having to run "strace grub2-install" inside a diskimage-builder chroot is a strong sign that your day has gone wrong :( | 22:15 |
| stevebaker[m] | TheJulia: it looks like grub on centos9 detects whether the system is efi by reading /sys/firmware/efi/fw_platform_size, and refuses to install legacy grub. But the mounted sysfs comes from the host | 22:21 |
| stevebaker[m] | oh an explicit --target=i386-pc helps | 22:33 |
| stevebaker[m] | oh there is already code to handle this, but it needs a tweak | 22:36 |
| TheJulia | :) | 22:36 |
| TheJulia | that explains another recent discussion | 22:37 |
| stevebaker[m] | TheJulia: this block should handle it but this conditional is a bit off https://opendev.org/openstack/diskimage-builder/src/branch/master/diskimage_builder/elements/bootloader/finalise.d/50-bootloader#L99 | 22:39 |
| TheJulia | yeah, not surprising | 22:49 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!