| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-05 | |||
| 11:37:56 | sean-k-mooney | oh ok | |
| 11:39:07 | sean-k-mooney | ill send them but we might need to handel that on 2023.1 seperatly | |
| 11:39:13 | sean-k-mooney | we will see what the timeing looks like | |
| 11:40:26 | frickler | sean-k-mooney: ah, right, maybe drop the W+1 from 923291 for now, then | |
| 11:41:49 | sean-k-mooney | sure done | |
| 11:44:45 | frickler | thx, promoted all the other changes now, we'll see more in 2h hopefully | |
| 11:45:38 | sean-k-mooney | let me know if i can help with anything else, ill check on them perodicly | |
| 11:50:40 | frickler | ack | |
| 12:45:30 | fungi | looks like https://review.opendev.org/c/openstack/nova/+/923291 got unapproved at some point, are we ready to reapprove it? | |
| 12:47:15 | fungi | oh, though it's not explicitly listed in the ossa so i guess it's not as urgent | |
| 12:47:59 | sean-k-mooney | i unapproved it because the 2023.2 version failed and it would fail if it went to the gate on the backport check | |
| 12:48:25 | sean-k-mooney | fungi: so ill reappoave it when https://review.opendev.org/c/openstack/nova/+/923287 is merged | |
| 12:48:58 | fungi | hopefully the backport check wouldn't fail when the 2023.2 was ahead of it in the gate unless that test isn't properly speculative, but it seems there's no harm in waiting on that one either | |
| 12:49:06 | sean-k-mooney | it will | |
| 12:49:16 | sean-k-mooney | its intentially not sepculiive | |
| 12:49:35 | fungi | what's the reasoning for that? | |
| 12:49:58 | sean-k-mooney | the reason it exits is to make sure we never merge soemthign in an older branch if tis not merged in the newer branch | |
| 12:50:12 | sean-k-mooney | we can overried it by marking the change stable-only | |
| 12:50:28 | sean-k-mooney | which is what we normally do for CVEs but didnt this time | |
| 12:50:41 | fungi | yeah, but making it check zuul's speculative state would still accomplish that, because it would get retested if the newer branch's version gets kicked out for any reason and then would fail | |
| 12:51:15 | sean-k-mooney | fungi: if you know how to update the script to do that feel free to propse it :) | |
| 12:51:39 | fungi | i don't even know what job you're talking about, but if you have a pointer to the script i can take a look | |
| 12:51:47 | sean-k-mooney | https://github.com/openstack/nova/blob/master/tools/check-cherry-picks.sh | |
| 12:51:52 | fungi | it's likely just a matter of making the script do less than it does now | |
| 12:52:05 | sean-k-mooney | this also need to work locally | |
| 12:54:32 | sean-k-mooney | the end goal of the script is to make sure we dont regress going form 2023.1 to 2023.2 ectra | |
| 12:54:43 | fungi | i guess i'll need to look at the context where the script is run, because i don't see it doing anything that should inherently break speculation, but if some earlier step is checking out from the origin remote instead of the current branch state that would break it | |
| 12:55:45 | sean-k-mooney | it runs in check as non voteing and gate as voting | |
| 12:56:13 | fungi | i mean the job that runs that script | |
| 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 | |