Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-07-05
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
15:20:21 fungi if we're reshuffling for the current failures anyway, should probably eject that one at the same time
15:21:14 dansmith fungi: maybe someone wanted to be really really sure it landed?
15:21:23 fungi double-merged!
15:21:42 fungi a change so nice we'll merge it twice
15:21:43 dansmith there were definitely things in check and forced into gate at the same time yesterday, perhaps a zuul bug or race?
15:24:09 frickler hmm, according to my bash history, I didn't reenqueue 923288, but zuul has been doing some additional requeues of patches on top of others
15:25:00 frickler maybe a race condition when I did 923289 and zuul wasn't aware yet of 923288 having been merged
15:25:20 fungi normally only if they're not yet merged and meet criteria to be enqueued and depend on a change you've enqueued (either explicitly via depends-on or implicitly through git commit parentage)
15:26:08 fungi but yeah, can't rule out bugs either
15:26:19 frickler but likely if that buildset passes, it will result in a merge failure and cause a gate reset anyway, so we can as well abandon it now IMO
15:27:12 fungi i agree we should kick it out, i'd give 50/50 odds on whether it would actually cause a gate reset or just no-op (not sure what gerrit's behavior is if you call the submit api on an already-merged change)
15:27:22 frickler and I'm reaching EOD quickly (time to watch some soccer), so if you could take over, fungi, that'd be nice
15:27:32 fungi glad to!

Earlier   Later