| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-05 | |||
| 12:56:15 | sean-k-mooney | fungi: from my perspective not doing speclicitive execution is safer and more correct | |
| 12:56:20 | sean-k-mooney | so i dont consider this to be a bug | |
| 12:56:37 | fungi | i don't see it, but not my circus, not my monkeys ;) | |
| 12:57:06 | sean-k-mooney | its called nova-tox-validate-backport | |
| 12:57:17 | sean-k-mooney | https://zuul.opendev.org/t/openstack/build/a6be643734b44f139bca7b4eb8963814 | |
| 12:57:37 | sean-k-mooney | Cherry pick hash 11301e7e3f0d81a3368632f90608e30d9c647111 not on any master or stable branches | |
| 12:57:42 | fungi | if you want backports to be delayed from being enqueued until other changes merge, and slow down throughput as a result even though zuul can ensure the correctness of the thing you actually want without that additional inefficiency, i'm not going to argue | |
| 12:58:34 | sean-k-mooney | fungi: can you expalin how zuul can ensure that a patch is merged on all newer stable branches before it will allow it to merge on the older ones | |
| 12:58:45 | fungi | yes, speculative testing | |
| 12:58:54 | sean-k-mooney | fungi: because as far as im aware it cant. it would be incorrect to use depend on | |
| 12:59:07 | sean-k-mooney | and it does not have awareness of the depeency in any other way | |
| 12:59:20 | fungi | zuul assumes the newer branch backport ahead of it will merge, but if it doesn't then testing gets rerun without that change present in the queue and the job will fail | |
| 12:59:40 | sean-k-mooney | that only true if they are both in the queu at the same time | |
| 12:59:45 | sean-k-mooney | that is not normlaly the case | |
| 13:00:07 | fungi | yes, if they're not in the queue at the same time then the job will just fail when that change isn't already merged, or succeed if it is | |
| 13:00:38 | sean-k-mooney | why woudl it fail | |
| 13:01:00 | fungi | zuul is presenting all the branches to the job with a speculative future state, but if that speculative state changes then the presumed future is invalidated and a new possible future is constructed and tests are rerun | |
| 13:01:03 | sean-k-mooney | fungi: what your stating does not fit with my mental model of how zuul works and im pretty sure it not corect | |
| 13:01:35 | sean-k-mooney | fungi: how would zuul know about the precondition | |
| 13:02:00 | sean-k-mooney | its not express in the normal jobs or via depned on | |
| 13:02:12 | fungi | zuul assumes that changes in the same queue share a possible future | |
| 13:02:15 | sean-k-mooney | we worte this script to explictly close the fact that zuul did not block this | |
| 13:02:27 | sean-k-mooney | right again we cant rely on that for this case | |
| 13:02:46 | sean-k-mooney | backporting thet same patch acroos branch and ensuring it merged in reverse chronalogical order | |
| 13:02:58 | fungi | yes, i'm not saying the job is unnecessary, i'm saying the job should be able to use the speculative future zuul has constructed instead of blocking on the actual present state of the repositories | |
| 13:03:15 | fungi | because if that future changes the job will be re-run | |
| 13:03:40 | sean-k-mooney | it would work if zuul sepcetivly merged it into the relevnet stable branch in the git repo it prepared | |
| 13:03:47 | fungi | it does | |
| 13:04:09 | sean-k-mooney | well the job still fails so apprently not. or there is a bug in our script | |
| 13:04:13 | fungi | it prepares all branches of any required-project in the job (implicitly including the project that triggered the job) | |
| 13:05:05 | fungi | if the script is using the locally prepared repository state without resetting any branches and without querying remote git states then it should see the prepared speculative future | |
| 13:05:14 | sean-k-mooney | we are not downlaoding a seperat repo we are runnign the script on the repo prepared by zuul | |
| 13:05:41 | sean-k-mooney | it is https://github.com/openstack/nova/blob/master/tools/check-cherry-picks.sh | |
| 13:05:55 | sean-k-mooney | we are doing read only operations on the repo in the script | |
| 13:06:16 | fungi | yeah, i agree i'm not immediately seeing why it wouldn't work | |
| 13:06:39 | fungi | i'll enqueue it and test the assumption that it can't pass | |
| 13:07:05 | fungi | oh, never mind, i can't enqueue it if it's not approved | |
| 13:07:05 | sean-k-mooney | ok i can re add +w | |
| 13:07:37 | sean-k-mooney | it failed in check which is why i expect it to fail in gate | |
| 13:07:49 | sean-k-mooney | but maybe that would pass in this specic case | |
| 13:07:57 | fungi | well, it failed in check because the check pipeline lacks the speculative state of the gate pipeline | |
| 13:08:00 | sean-k-mooney | because of the queing and the fact gate is a dependet pipeline | |
| 13:08:14 | fungi | if you did a depends-on in check it would likely pass there | |
| 13:08:37 | fungi | by creating the same speculative future | |
| 13:08:45 | sean-k-mooney | it woudl but the ohter juobs weoul dnot be happy with zuul doing a merge of two branches of the same repo | |
| 13:09:08 | fungi | why? it still merges them to different branches | |
| 13:09:31 | fungi | we do this all the time to test upgrade jobs like grenade | |
| 13:09:33 | sean-k-mooney | well its the same commit so would that not be a merge conflict | |
| 13:09:48 | sean-k-mooney | i know you can do depens on in the same repo | |
| 13:09:54 | fungi | changes on different branches aren't going to merge conflict with one abother | |
| 13:10:03 | sean-k-mooney | just never had it work in the same repo for the same comit form two branches | |
| 13:10:31 | fungi | the dependency will be merged to the stable/2024.1 branch, the current change will be merged to the stable/2023.2 branch | |
| 13:10:49 | sean-k-mooney | did that change at some point | |
| 13:10:51 | fungi | they don't get combined into the same branch together, so no merge conflict | |
| 13:11:04 | sean-k-mooney | good to know but i tough they woudl both be merged into the branch under test | |
| 13:11:23 | fungi | zuul has supported cross-branch dependencies this way since ~2014 | |
| 13:11:51 | sean-k-mooney | fungi: i reappoved https://review.opendev.org/c/openstack/nova/+/923291 so you can try an promotet it if you want | |
| 13:11:57 | fungi | supporting dependencies across branches was a major design requirement because of grenade | |
| 13:12:11 | fungi | it'll get enqueued automatically in the current state anyway | |
| 13:12:40 | fungi | which it just did | |
| 13:12:49 | sean-k-mooney | ok im pretty sure if hit edgecase with that but it may have been related to somethign other then the merger | |
| 13:13:04 | fungi | now to see if nova-tox-validate-backport passes | |
| 13:13:58 | sean-k-mooney | if it does that will be good to know | |
| 13:15:38 | fungi | yeah, if it does pass then that means in theory it should also be able to pass in check with a depends-on to the version on the newer branch | |
| 13:16:21 | fungi | if it fails then i should be able to trace through the steps in the job to see where assumptions about the constructed future state on the other branch are getting invalidated | |
| 13:20:03 | fungi | multi-branch speculative states are something we've had for ages because it's necessary for making sure that grenade jobs run in the gate pipeline reflect the end state when the commits on different branches later merge, so we don't land changes on two different branches that then break upgrade tests when used together | |
| 13:23:13 | fungi | validate-backport: OK | |
| 13:23:19 | fungi | looks like it's going to succeed | |
| 13:23:31 | fungi | yep, just went green | |
| 13:24:00 | fungi | so the job is actually correctly designed to support this, there's not an actual need to wait for the other change to merge before you approve | |
| 13:25:44 | fungi | and using depends-on in cases like this would have two benefits: 1. you'll get an actual passing result (you don't even necessarily have to keep the job non-voting in check if you don't want to), and 2. zuul will ensure that the changes can't get enqueued into the gate pipeline in the wrong order so you can just approve them in any order you like | |
| 13:27:05 | fungi | the only caveat i'm aware of is that you need to be careful you don't inadvertently create dependency loops when you do that, but the same can be said for any use of depends-on | |
| 13:40:06 | frickler | ok, looks like I could reenqueue the deferred patches, then | |
| 13:44:22 | dansmith | frickler: sean-k-mooney: FWIW, I'd much rather have broken iso support and angry users than have this exposure.. I expect images to start showing up on download sites that look to contain something awesome, but are just the exploit | |
| 13:44:26 | fungi | in cases like this where there are series of changes for each branch, i'd probably conservatively just set depends-on in the commit message of the first change in the series pointing to the change url for the last change in the series on the newer branch | |
| 13:44:40 | dansmith | something a user of a private cloud might download and then upload into the internal glance and then ..boom | |
| 13:44:50 | frickler | humm, third nova-multi-cell failure on https://review.opendev.org/c/openstack/nova/+/923287 in a row, looks like something more serious | |
| 13:46:25 | dansmith | frickler: why serious? one fail.. | |
| 13:46:48 | dansmith | looks like failure to talk to metadata server, | |
| 13:47:01 | dansmith | which could be the same sort of thing as prevents us from SSHing to the guest | |
| 13:47:10 | dansmith | oh actuall, | |
| 13:47:24 | dansmith | we're doing it from inside the guest, so "ssh guest curl metadata" | |
| 13:48:09 | fungi | so could be exactly the same problem in that case | |
| 13:48:17 | opendevreview | Merged openstack/nova stable/2023.1: Reject qcow files with data-file attributes https://review.opendev.org/c/openstack/nova/+/923288 | |
| 13:48:21 | frickler | ok, I miscounted, twice in a row still https://zuul.opendev.org/t/openstack/build/51577af322ee4809889698bde352efac | |
| 13:49:47 | frickler | actually https://review.opendev.org/923289 is going to fail, too, stuck in devstack not finishing, waiting for the timeout to strike | |
| 13:50:56 | frickler | going to do some gate reshuffling to override | |
| 13:52:50 | fungi | seems like a good opportunity | |
| 13:55:55 | opendevreview | sean mooney proposed openstack/nova master: [WIP] add iso file format inspector https://review.opendev.org/c/openstack/nova/+/923573 | |
| 13:57:00 | sean-k-mooney | oh i messed up that commit message | |
| 13:57:20 | sean-k-mooney | let me abandon the new review | |
| 13:57:53 | opendevreview | sean mooney proposed openstack/nova master: [WIP] add iso file format inspector https://review.opendev.org/c/openstack/nova/+/923533 | |
| 14:54:31 | frickler | and another gate failure in the cve stack https://zuul.opendev.org/t/openstack/build/90c78eec9270430a8fcaad8652348d0c | |
| 14:57:31 | dansmith | nova.exception.InternalError: Unexpected vif_type=binding_failed | |
| 14:57:36 | dansmith | I assume that means some neutron fail | |
| 14:57:38 | dansmith | sean-k-mooney: ^ | |
| 14:58:33 | dansmith | (frickler that was from the gate fail you quoted, in case it's not clear) | |
| 15:01:19 | frickler | yes, it is well possible that it is all unrelated and just unstable tests, I just wanted someone to check it | |
| 15:03:46 | dansmith | yep, definitely doesn't look related | |
| 15:19:52 | fungi | did 923288 get reenqueued into the gate accidentally? it's already merged | |