| @clarkb:matrix.org | yes | 00:00 |
|---|---|---|
| @iwienand:matrix.org | yes with no results; e.g. https://zuul.opendev.org/t/openstack/build/ada99958a8ee4350ba6ac73c3f80670f | 00:00 |
| @iwienand:matrix.org | not sure what facilities we have for providing more info there | 00:00 |
| @jim:acmegating.com | i think there's just very error reporting for early failures; it's not generally expected to fail. i think it would have to be plumbed back through to the item buildset message to end up in the db. | 00:01 |
| @jim:acmegating.com | so it's possible, but the facility currently doesn't exist | 00:02 |
| @jim:acmegating.com | and care must be taken that the message doesn't expose any sensitive info (so we can't just blindly copy what git gives us). it may be better to be something generic like "Unable to create reference refs/heads/patchback/backports/stable-4/7f793c83f1aae99e936238dc1873251ac2c358a3/pr-4162 at 01e50ed7dac6cd25ce268d01cc457910633ccbf0 in /var/lib/zuul/builds/e89d10ecb10a4812b1dd9e4ea50fdb1a/work/src/github.com/ansible-collections/community.general/.git" | 00:04 |
| @jim:acmegating.com | might be okay to include the ValueError exception text, but i worry that exceptions like that could have complete git urls with passwords | 00:05 |
| @jim:acmegating.com | so my inclination would be to avoid that issue altogether and just indicate which ref/repo/etc failed | 00:05 |
| @clarkb:matrix.org | ya it may be sufficient to simply indicate "repo foo ref bar was unable to have its state set. Check that this is a valid ref" | 00:06 |
| @jim:acmegating.com | in this specific case, that message would still get you back to "that ref was deleted on github", so i think it's sufficient. | 00:06 |
| @iwienand:matrix.org | so i guess we fall into https://opendev.org/zuul/zuul/src/branch/master/zuul/executor/server.py#L1148 | 00:10 |
| @iwienand:matrix.org | we could definitely catch the ValueError and do a more specific failure that results in an ERROR status | 00:10 |
| @iwienand:matrix.org | raise it as an ExecutorError | 00:11 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 821711: Add IBM Cloud VPC driver https://review.opendev.org/c/zuul/nodepool/+/821711 | 00:11 | |
| @iwienand:matrix.org | if we log it as an error we should at least get something like, e.g. https://zuul.opendev.org/t/openstack/build/45d4d96d3f7e4f3fab9b5bca6f3be51a | 00:21 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829591: Protect stream handler thread from exceptions https://review.opendev.org/c/zuul/zuul/+/829591 | 00:22 | |
| @jim:acmegating.com | ianw: yeah that looks good | 00:23 |
| @jim:acmegating.com | Clark: can you re-review https://review.opendev.org/829591 real quick -- testing caught a minor error; i feel like if you re+2 it that should be enough for a reapproval | 00:24 |
| @clarkb:matrix.org | corvus: looking | 00:24 |
| @jim:acmegating.com | ianw: ^ that's the fix for the log stream error btw | 00:25 |
| @clarkb:matrix.org | yay testing | 00:25 |
| @jim:acmegating.com | yeah, i guess the streaming test i ran locally didn't exercise that part of the streaming system | 00:26 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul] 829617: merger: report missing object as ERROR https://review.opendev.org/c/zuul/zuul/+/829617 | 00:50 | |
| @iwienand:matrix.org | corvus: Clark ^ attempt 2, to give the user some info | 00:51 |
| @jim:acmegating.com | ianw: +2 with suggestion | 00:54 |
| @jim:acmegating.com | ianw: and i think a review from tobiash on that would be good | 00:55 |
| @jim:acmegating.com | ianw: oh, one more thing -- that should be testable; i think that would be good to add a unit test for that | 00:57 |
| @jim:acmegating.com | basically: hold jobs in queue, create a branch, open a fake PR, wait to settle, delete the branch from upstream, release jobs | 00:58 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829384: Add buildset start/end db columns https://review.opendev.org/c/zuul/zuul/+/829384 | 01:03 | |
| @clarkb:matrix.org | ianw: I'll have to take a look tomorrow morning, but its on my list | 01:12 |
| @iwienand:matrix.org | Clark: no worries, i'll try to get a test going | 01:44 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul] 829617: merger: report missing object as ERROR https://review.opendev.org/c/zuul/zuul/+/829617 | 06:32 | |
| @iwienand:matrix.org | > <@jim:acmegating.com> basically: hold jobs in queue, create a branch, open a fake PR, wait to settle, delete the branch from upstream, release jobs | 06:33 |
| corvus: i've tried to implement that in https://review.opendev.org/c/zuul/zuul/+/829617/3/tests/unit/test_cross_crd.py but i'm going to need a bit more hand-holding please, it is not working at all like I would have hoped and that test does not fail, as I hoped it would | ||
| -@gerrit:opendev.org- Joan Gilabert proposed: [zuul/zuul-jobs] 829063: Add patch to list of build dependencies https://review.opendev.org/c/zuul/zuul-jobs/+/829063 | 09:16 | |
| @avass:vassast.org | Is it possible for a project to be both a config-project and an untrusted project? We wanna baseline the config-project with our deployment of zuul while at the same time using zuul to test the repo we deploy zuul from. So I figured that could work if the project is both a config-project and untrusted-project with different config directories using: | 10:33 |
| https://zuul-ci.org/docs/zuul/latest/tenants.html#attr-tenant.untrusted-projects.%3Cproject%3E.extra-config-paths | ||
| @avass:vassast.org | or would zuul not like that? | 10:34 |
| @avass:vassast.org | hmm I think we encountered a big using cherry-pick submit mode in gerrit and git commit dependent changes. | 11:12 |
| Short story is that it looks like zuul does a checkout with commit `A` as I think it sees it as a git commit dependency for commit `B` without first putting `A` on top of master and then puts commit `B` on top of that: | ||
| So in this short example `A0` is tested with the state of `A-merged` while `B0` is tested with the state of `B0` instead of the state of `B-merged`: | ||
| ``` | ||
| master ---HEAD--c0--c1--c2--A-merged--B-merged | ||
| \--A0--B0 | ||
| ``` | ||
| @avass:vassast.org | * hmm I think we encountered a bug using cherry-pick submit mode in gerrit and git commit dependent changes. | 11:13 |
| Short story is that it looks like zuul does a checkout with commit `A` as I think it sees it as a git commit dependency for commit `B` without first putting `A` on top of master and then puts commit `B` on top of that: | ||
| So in this short example `A0` is tested with the state of `A-merged` while `B0` is tested with the state of `B0` instead of the state of `B-merged`: | ||
| ``` | ||
| master ---HEAD--c0--c1--c2--A-merged--B-merged | ||
| \--A0--B0 | ||
| ``` | ||
| @avass:vassast.org | quick guess is that `cherry-pick` in gerrit causes gerrit to create a new patch-set when it merges the change | 11:15 |
| @avass:vassast.org | * quick guess is that it's because `cherry-pick` in gerrit causes gerrit to create a new patch-set when it merges the change | 11:15 |
| @ekapoun1:matrix.org | Hello, is it possible to have a gerrit trigger for a specific repo? We're trying with the "repo" key under the trigger for 'my-repo' in the example below. Zuul is giving us an error saying the "repo" key isn't allowed (which is in accordance with the documentation), but is there a way to do what we are trying to do? | 11:44 |
| `` | ||
| - pipeline: | ||
| name: check | ||
| trigger: | ||
| my-repo: | ||
| - event: patchset-created | ||
| branch: '^(master|integration/.*)$' | ||
| repo: '^core/.*$' | ||
| `` | ||
| @ekapoun1:matrix.org | * Hello, is it possible to have a gerrit trigger for a specific repo? We're trying with the "repo" key under the trigger for 'my-repo' in the example below. Zuul is giving us an error saying the "repo" key isn't allowed (which is in accordance with the documentation), but is there a way to do what we are trying to do? | 11:44 |
| - pipeline: | ||
| name: check | ||
| trigger: | ||
| my-repo: | ||
| - event: patchset-created | ||
| branch: '^(master|integration/._)$' | ||
| repo: '^core/._$' | ||
| @ekapoun1:matrix.org | * Hello, is it possible to have a gerrit trigger for a specific repo? We're trying with the "repo" key under the trigger for 'my-repo' in the example below. Zuul is giving us an error saying the "repo" key isn't allowed (which is in accordance with the documentation), but is there a way to do what we are trying to do? (Sorry for the formatting, didn't get a code block working) | 11:45 |
| - pipeline: | ||
| name: check | ||
| trigger: | ||
| my-repo: | ||
| - event: patchset-created | ||
| branch: '^(master|integration/._)$'repo: '^core/._$' | ||
| @ekapoun1:matrix.org | * Hello, is it possible to have a gerrit trigger for a specific repo? We're trying with the "repo" key under the trigger for 'my-repo' in the example below. Zuul is giving us an error saying the "repo" key isn't allowed (which is in accordance with the documentation), but is there a way to do what we are trying to do? (Sorry for the formatting, didn't get a code block working) | 11:46 |
| - pipeline: | ||
| name: check | ||
| trigger: | ||
| my-repo: | ||
| - event: patchset-created | ||
| branch: '^(master|integration/._)$' | ||
| repo: '^core/._$' | ||
| @ekapoun1:matrix.org | * Hello, is it possible to have a gerrit trigger for a specific repo? We're trying with the "repo" key under the trigger for 'my-repo' in the example below. Zuul is giving us an error saying the "repo" key isn't allowed (which is in accordance with the documentation), but is there a way to do what we are trying to do? | 11:50 |
| Here's a link to pastebin as I couldn't get a code block working here: https://pastebin.com/UbJibECS | ||
| @avass:vassast.org | It looks like the cherry-pick merge mode needs to be handled differently to check if the commit id of the parent change latest patch set is merged instead of the parent commit: | 13:05 |
| https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/gerrit/gerritconnection.py#L944 | ||
| @mhuin:matrix.org | > <@gerrit:opendev.org> James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 828917: Add support for Microsoft login https://review.opendev.org/c/zuul/zuul/+/828917 | 14:44 |
| ooh, sorry, just seeing that, I'll have a look | ||
| @clarkb:matrix.org | corvus: ianw I left some notes on https://review.opendev.org/c/zuul/zuul/+/829617 around why I think the test is failing. Essentially we aren't getting enough state into the test to cause zuul to trip over the branch being removed. | 17:21 |
| @clarkb:matrix.org | ok I think I identified the methods to use to actually update the state the way we want to. | 17:32 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829756: Fix error in background tenant update of new tenant https://review.opendev.org/c/zuul/zuul/+/829756 | 18:28 | |
| @jim:acmegating.com | zuul-maint: ^ that's a simple bugfix for a potentially severe problem | 18:29 |
| @fungicide:matrix.org | corvus: looks like swest has a suggestion on it | 18:35 |
| @clarkb:matrix.org | also did you fail to add the extra tenant config file? | 18:37 |
| @jim:acmegating.com | Clark: i did! | 18:37 |
| @jim:acmegating.com | i will address both things | 18:38 |
| @clarkb:matrix.org | I've got at least one more thing if you give me a sec to finish up my review | 18:38 |
| @clarkb:matrix.org | posted | 18:39 |
| @jim:acmegating.com | ++ | 18:40 |
| @jim:acmegating.com | i also just spotted another place where we could hit a similar error | 18:40 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 829591: Protect stream handler thread from exceptions https://review.opendev.org/c/zuul/zuul/+/829591 | 18:48 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829756: Fix error in background tenant update of new tenant https://review.opendev.org/c/zuul/zuul/+/829756 | 18:50 | |
| @jim:acmegating.com | swest: tobiash Clark fungi ^ redux | 18:51 |
| @jim:acmegating.com | wait, i just spotted one more | 18:52 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829756: Fix error in background tenant update of new tenant https://review.opendev.org/c/zuul/zuul/+/829756 | 18:53 | |
| @jim:acmegating.com | swest: tobiash Clark fungi ^ ok for reals | 18:54 |
| @jim:acmegating.com | now the only remaining occurrences of "local_layout_state[" in scheduler.py are assignments/dels :) | 18:55 |
| @clarkb:matrix.org | corvus: quick question inline | 18:59 |
| @jim:acmegating.com | Clark: repld | 19:00 |
| @clarkb:matrix.org | corvus: hrm, why do we hold the read lock in the other places where we do those reads then? The updates should grab the write lock I think? | 19:02 |
| @clarkb:matrix.org | I guess it synchronizes the checking more cleanly maybe? | 19:02 |
| @jim:acmegating.com | Clark: it's not for that, it's for the other work they do | 19:02 |
| @jim:acmegating.com | likely that too | 19:03 |
| @clarkb:matrix.org | but ya it would ensure one thing processing queues at a time for each tenant | 19:04 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 829756: Fix error in background tenant update of new tenant https://review.opendev.org/c/zuul/zuul/+/829756 | 20:09 | |
| @iwienand:matrix.org | fungi: thanks for review on 828818; with the "encrypt_file_recipients could be simply populated with the key IDs from encrypt_file_keys" i was thinking of the case where we have all and sundry's keys in a single var in project (system-config in our case) and then when using this role, you have another variable defined for the particular job that has a subset of the keys | 20:47 |
| @iwienand:matrix.org | iiuc your proposal would mean you'd have to filter the keys on the project/system-config side? | 20:47 |
| @fungicide:matrix.org | ianw: do you think it's likely to have keys included which you don't want to encrypt for? also what about making the default for encrypt_file_recipients be inferred from encrypt_file_keys but still overridable? | 21:09 |
| @fungicide:matrix.org | just seems like specifying the keys and then specifying which of the keys you want to use would be redundant in most cases, as is providing copies of the keys and also providing the key ids/fingerprints | 21:10 |
| @fungicide:matrix.org | having the default interface be simply "here's a list of files, and here's some keys i want them encrypted to" seems like it would be the most convenient | 21:12 |
| @iwienand:matrix.org | fungi: it is kind of the way i've written our use of it. my theory is that all possible keys are committed in to https://review.opendev.org/c/opendev/system-config/+/828810/36/playbooks/zuul/roles/encrypt-logs/defaults/main.yaml encrypt_logs_keys. then each job encrypts {{ encrypt_logs_recpieints + encrypt_logs_job_recipieints }} which is a subset of those all possible keys | 21:13 |
| @iwienand:matrix.org | so when someone is interested in the job logs, they just have to add | 21:14 |
| vars: | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 827935: Populate missing change cache entries https://review.opendev.org/c/zuul/zuul/+/827935 | 21:14 | |
| @iwienand:matrix.org | vars: | 21:14 |
| encrypt_logs_job_recipients: | ||
| - me | ||
| to the job | ||
| @iwienand:matrix.org | so i guess i'm optimizing for the calling side, if that makes sense, rather than making the role as simple as possible. we *could* do all that filtering it does on the system-config side; but i felt like it might be generically useful to operate like that | 21:15 |
| @iwienand:matrix.org | Clark: thanks for comments on 829617 ; looking now | 21:20 |
| @fungicide:matrix.org | ianw: trying to think through this, it's almost like zuul would benefit from having something like how secrets works, where we could define these public keys independently, and then just add a list of them to the job. it would be great if we didn't need to pass keys into the role that it's not going to use, but i agree that job inheritance at least gives us a way to hide the apparently redundancy of passing the keys along with a separate list of which keys you want actually used, instead of only needing to pass a set of keys and have it use those | 22:35 |
| @iwienand:matrix.org | Clark: hrm i managed to get it actually restoring a branch | 23:05 |
| @iwienand:matrix.org | 2022-02-18 09:48:06,273 zuul.Repo.Ref DEBUG Created reference refs/heads/delete-branch at ccb04519aa9a02b031d8ffcd353e7970cc1ee6c5 in /tmp/tmpy5r174u1/zuul-test/builds/14dc48c27d7544baa50d4950733819ae/work/src/github.com/github/project2/.git | 23:05 |
| @iwienand:matrix.org | but the weird thing is i deleted that branch, and it's still coming back. | 23:05 |
| @iwienand:matrix.org | the thing i'm thinking makes it hard to replicate is that sha actually disappeared. just deleting the branch isn't quite the same as actually removing the ref | 23:19 |
| @iwienand:matrix.org | s/ref/object/ | 23:19 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829829: Don't reconfigure after every gitlab merge https://review.opendev.org/c/zuul/zuul/+/829829 | 23:38 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 829829: Don't reconfigure after every gitlab merge https://review.opendev.org/c/zuul/zuul/+/829829 | 23:39 | |
| @jim:acmegating.com | fbotristanC ^ the pagure driver may benefit from a similar change | 23:40 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!