| TheJulia | dansmith: o/ Out of curiosity, Any thoughts on https://6f2b38e507b84798c968-8da05f86af8a44d2353be3e602197c08.ssl.cf5.rackcdn.com/942496/8/check/ironic-tempest-ramdisk-bios-snmp-pxe/8e38524/controller/logs/screen-g-api.txt ? | 14:48 |
|---|---|---|
| TheJulia | Looks like a kernel artifact was flagged as mbr and thus rejected :\ | 14:49 |
| TheJulia | The only path to unbrick Ironic's CI that I can see is an equivalent patch for allowance for an artifact to be identified as mbr if listed as raw by the user | 14:54 |
| dansmith | TheJulia: that's this, yet unmerged unfortunately.. can you try to run against that real quick? https://review.opendev.org/c/openstack/glance/+/939727 | 15:06 |
| dansmith | TheJulia: unfortunately, this code has been waiting for review since november and I've become increasingly nervous about merging it at the end of the cycle, for exactly this reason >:( | 15:07 |
| TheJulia | dansmith: Looks like that won't fix it, but I'll post a change to give that a spin | 15:07 |
| dansmith | TheJulia: no? | 15:07 |
| TheJulia | Well, its matching gpt, the file is flagged as mbr by the inspector | 15:07 |
| TheJulia | so, at a glance, looks like it won't | 15:07 |
| TheJulia | but, I'll give it a spin | 15:07 |
| dansmith | you're uploading as raw right? | 15:08 |
| dansmith | oh, it's failing the safety check which this will still run over | 15:08 |
| TheJulia | yeah, raw now | 15:09 |
| dansmith | TheJulia: the one below it will let us exclude these safety checks in config | 15:09 |
| TheJulia | you mean the one above it? | 15:10 |
| TheJulia | the configuration option, the list? | 15:11 |
| dansmith | https://review.opendev.org/c/openstack/glance/+/939243/6 | 15:11 |
| TheJulia | gpt_safety_checks_nonfatal | 15:11 |
| TheJulia | i see | 15:11 |
| TheJulia | okay | 15:11 |
| dansmith | abhishekk: croelandt you seeing this? | 15:11 |
| abhishekk | me in meeting atm | 15:12 |
| dansmith | thoughts on flipping that to default to no mbr safety checks (which I added because of another false positive) and merging that might be best | 15:12 |
| dansmith | TheJulia: let me respin that defaulting to disable mbr and you can run against that okay? | 15:12 |
| TheJulia | dansmith: sure, FWIW, I think what you just proposed is reasonable and lessens the impact for operators who are doing legitimate kernel artifact uploads | 15:13 |
| abhishekk | CAN we flip the patch? I think the below one above mentioned patch is dependent on nova and cinder | 15:14 |
| TheJulia | Can you clarify why and how that would fix the current situation? | 15:16 |
| TheJulia | The bottom line, glance has late in cycle merged breaking patches. It stinks. It is not great. We need to find the cleanest/fastest path forward so we should clearly express everything we're trying to communicate. | 15:19 |
| dansmith | abhishekk: I have to fix some test conflicts, but the one with the list config doesn't depend on the one below it IIRC | 15:20 |
| dansmith | TheJulia: it would allow us to not safety check things we think are GPT/MBR but keep the safety checks on everything else (which is the important part) | 15:21 |
| dansmith | TheJulia: yeah, late in cycle, apologies for that.. I've been trying to get the proper people to review this stuff since november, which would have been a lot better time to merge :/ | 15:22 |
| abhishekk | dansmith: ack, TheJulia agree, I am sorry for the trouble caused, we will look out for quick possible solution | 15:22 |
| TheJulia | dansmith: no need to apologiese, $stuff happens :) | 15:22 |
| TheJulia | November would have been amazing, but it happens :) | 15:23 |
| dansmith | I'm just having to retool a couple of tests to make the reordering work | 15:30 |
| TheJulia | cool cool, happy to kick off a test job and review once posted, first my corgi demands his morning walk | 15:36 |
| TheJulia | Thanks dansmith! | 15:36 |
| opendevreview | Dan Smith proposed openstack/glance master: Allow ignoring specific gpt safety checks https://review.opendev.org/c/openstack/glance/+/939243 | 16:01 |
| dansmith | abhishekk: TheJulia ^ it was a little more intertwined than I thought but not for any good reason | 16:01 |
| dansmith | there's actually a piece of the GPT patch that should have been in the safety check patch, so I will extract that into another thing to land (the new FIXME on the dest inspector) and we can carefully test that before we land | 16:02 |
| TheJulia | That seems to make sense. I've updated my test change against ironic so we should have some data points soonish | 16:14 |
| opendevreview | Dan Smith proposed openstack/glance master: Add GPT disk_format https://review.opendev.org/c/openstack/glance/+/933601 | 16:19 |
| opendevreview | Dan Smith proposed openstack/glance master: Allow GPT as raw for compatibility https://review.opendev.org/c/openstack/glance/+/939727 | 16:19 |
| opendevreview | Dan Smith proposed openstack/glance master: WIP: Add glance-manage format support https://review.opendev.org/c/openstack/glance/+/939746 | 16:19 |
| opendevreview | Dan Smith proposed openstack/glance master: DNM: Test glance against latest oslo https://review.opendev.org/c/openstack/glance/+/939242 | 16:19 |
| opendevreview | Dan Smith proposed openstack/glance master: Safety check output of image conversion https://review.opendev.org/c/openstack/glance/+/942711 | 16:19 |
| dansmith | and that's hopefully the rest of the series rebased on top of that | 16:19 |
| dansmith | TheJulia: looks like we've passed some of the tempest jobs on your test patch at least - I dunno which of those would be/were affected | 16:58 |
| * TheJulia looks | 16:59 | |
| TheJulia | dansmith: yeah, so far so good as a number of the jobs were failing even before getting into tempest | 17:01 |
| abhishekk | dansmith: do you think we should also add reno for this specific patch, specifically for the mention in commit "For maximum compatibility, we ignore the mbr check by default for the first release with this check."? | 17:05 |
| abhishekk | Or just mention in commit is enough? | 17:07 |
| dansmith | abhishekk: yeah reno is good, I just wanted to get it up for testing.. abhishekk can I follow up with a reno either after the second patch or in the second one? | 17:30 |
| abhishekk | sounds good | 17:31 |
| abhishekk | * second patch | 17:31 |
| dansmith | ack | 17:32 |
| abhishekk | I will approve this once have the check results | 17:32 |
| abhishekk | ironic-tempest-ramdisk-bios-snmp-pxe passed | 18:04 |
| dansmith | yeah, everything we care about I think | 18:10 |
| TheJulia | Just following up, the test change against ironic's gate passed | 18:13 |
| dansmith | abhishekk: ^ | 18:14 |
| TheJulia | well, the one I included and the ones stacked before it since that would have checked out the entire series | 18:17 |
| JayF | I think we need 939243 + 942711 + 933601 | 18:17 |
| JayF | yeah | 18:17 |
| dansmith | not the last one just the bottom two | 18:18 |
| dansmith | the last one didn't get review from cinder people so I think it's toast for this cycle, but it shouldn't be required for you nor do I think it was tested, | 18:18 |
| dansmith | since I didn't even upload the rebased one until after the test started | 18:19 |
| abhishek_ | dansmith: ack, approving this one, you can add reno in new change if works for you! | 18:32 |
| dansmith | abhishek_: I have it ready to go, waiting for the zuul to finish on the second one | 18:32 |
| abhishek_ | this will again take 2-3 hours | 18:33 |
| abhishek_ | so I think better to approve this one | 18:34 |
| abhishek_ | better to unblock ironic sooner | 18:35 |
| abhishek_ | ? | 18:35 |
| dansmith | abhishek_: I have it in the second patch, which is not critical for them | 18:36 |
| dansmith | but I can move it to a new third if you prefer | 18:36 |
| abhishek_ | yeah that will be good | 18:36 |
| dansmith | okay | 18:36 |
| abhishek_ | since I am not going to ninja second one I can safely approve the new reno patch if this one merges | 18:37 |
| dansmith | well, then why not just let me push the reno into the second one? | 18:38 |
| abhishek_ | OK, no objection | 18:38 |
| abhishek_ | just to be confirm, second one means, https://review.opendev.org/c/openstack/glance/+/942711 this? | 18:39 |
| dansmith | yes | 18:39 |
| dansmith | I'll just wait for the job to finish in case there are other things it finds, then I'll push that up (and a fix for the bad merge on the third) | 18:40 |
| abhishek_ | ack | 18:40 |
| abhishek_ | me will not be around by then, but please ping me on slack if you need anything | 18:41 |
| dansmith | yep, not critical for that one so no worries | 18:41 |
| abhishek_ | cool | 18:41 |
| opendevreview | Dan Smith proposed openstack/glance master: Safety check output of image conversion https://review.opendev.org/c/openstack/glance/+/942711 | 19:12 |
| opendevreview | Dan Smith proposed openstack/glance master: Add GPT disk_format https://review.opendev.org/c/openstack/glance/+/933601 | 19:12 |
| opendevreview | Dan Smith proposed openstack/glance master: Allow GPT as raw for compatibility https://review.opendev.org/c/openstack/glance/+/939727 | 19:12 |
| opendevreview | Dan Smith proposed openstack/glance master: WIP: Add glance-manage format support https://review.opendev.org/c/openstack/glance/+/939746 | 19:12 |
| opendevreview | Dan Smith proposed openstack/glance master: DNM: Test glance against latest oslo https://review.opendev.org/c/openstack/glance/+/939242 | 19:12 |
| opendevreview | Merged openstack/glance master: Allow ignoring specific gpt safety checks https://review.opendev.org/c/openstack/glance/+/939243 | 21:19 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!