| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 847646: Fix merging with submitWholeTopic https://review.opendev.org/c/zuul/zuul/+/847646 | 06:45 | |
| -@gerrit:opendev.org- Dmitriy Rabotyagov proposed on behalf of Denys Mishchenko: [zuul/nodepool] 842913: Fix for post_upload_hook failure. id missing https://review.opendev.org/c/zuul/nodepool/+/842913 | 07:40 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 848234: Fix read-only branches error in zuul-web https://review.opendev.org/c/zuul/zuul/+/848234 | 09:04 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 848093: zuul-web: raise 404s when tenant isn't found https://review.opendev.org/c/zuul/zuul/+/848093 | 10:07 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 848234: Fix read-only branches error in zuul-web https://review.opendev.org/c/zuul/zuul/+/848234 | 11:30 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 848132: zuul-web: return errors as JSON https://review.opendev.org/c/zuul/zuul/+/848132 | 13:36 | |
| @vlotorev:matrix.org | Hi, could zuul-maint have a look at https://review.opendev.org/c/zuul/zuul/+/844742? It already has one CR+2. | 14:05 |
|---|---|---|
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 848132: zuul-web: return errors as JSON https://review.opendev.org/c/zuul/zuul/+/848132 | 14:07 | |
| @jim:acmegating.com | vlotorev: i think your change is fine -- but https://review.opendev.org/842290 from mhu will need a rebase after it lands (but it needs a release note added anyway, so that's not a big deal) | 14:42 |
| @mhuin:matrix.org | > <@jim:acmegating.com> vlotorev: i think your change is fine -- but https://review.opendev.org/842290 from mhu will need a rebase after it lands (but it needs a release note added anyway, so that's not a big deal) | 14:45 |
| yeah the rebase is fine, np. Do keep in mind though that the REST commands in the zuul admin CLI are deprecated, so I think we should limit changes related to that | ||
| @mhuin:matrix.org | However this is a minor QoL change so I don't see why we shouldn't have it | 14:46 |
| @mhuin:matrix.org | BTW is there a target window for the next zuul release? | 14:47 |
| @jim:acmegating.com | not yet | 14:53 |
| @mhuin:matrix.org | okay good, I can relax about that then 🙂 | 14:54 |
| @vlotorev:matrix.org | corvus: mhu: thanks. | 15:17 |
| @clarkb:matrix.org | tristanC: is the failed_when: false from https://review.opendev.org/c/zuul/zuul-operator/+/847846/2/playbooks/zuul-operator-functional/test.yaml#170 leftover from debugging? | 17:42 |
| @tristanc_:matrix.org | Clark: this task is expected to fail, the real test happens in the following tasks | 17:44 |
| @clarkb:matrix.org | why does wsdump fail? I guess that aspect of the change wasn't clear from the commit message. The commit message says the echo is there to fix it but its still returning non zero? | 17:45 |
| @tristanc_:matrix.org | Clark: if i recall correctly, the command may error because of EOF. Though it's fine, what matter is that its output contains the expected console log | 17:46 |
| @clarkb:matrix.org | Note the task in the job that ran for that ps had an rc of 0 | 17:47 |
| @clarkb:matrix.org | ok | 17:47 |
| @clarkb:matrix.org | tristanC: in https://review.opendev.org/c/zuul/zuul-operator/+/845851/19/deploy/crds/zuul-ci_v1alpha2_zuul_cr.yaml why is the database section commented out? Is that meant to be an example? | 18:04 |
| @clarkb:matrix.org | left a couple of questions on that chagne | 18:12 |
| -@gerrit:opendev.org- Zuul merged on behalf of Tristan Cacqueray: [zuul/zuul-operator] 847846: Pin pykube-ng and fix the CI https://review.opendev.org/c/zuul/zuul-operator/+/847846 | 18:21 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Albin Vass: [zuul/zuul-operator] 800265: Gracefully shutdown executor on SIGTERM https://review.opendev.org/c/zuul/zuul-operator/+/800265 | 18:21 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Tristan Cacqueray: [zuul/zuul-operator] 847978: Remove gearman configuration https://review.opendev.org/c/zuul/zuul-operator/+/847978 | 18:30 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Tristan Cacqueray: [zuul/zuul-operator] 847809: Remove CONTRIBUTE.md documentation https://review.opendev.org/c/zuul/zuul-operator/+/847809 | 18:31 | |
| -@gerrit:opendev.org- Tristan Cacqueray proposed: [zuul/zuul-operator] 845851: Update CRD apiVersion to v1 (from v1beta) https://review.opendev.org/c/zuul/zuul-operator/+/845851 | 18:46 | |
| -@gerrit:opendev.org- Clark Boylan proposed: [zuul/nodepool] 848331: Add python3.10 testing https://review.opendev.org/c/zuul/nodepool/+/848331 | 19:09 | |
| -@gerrit:opendev.org- Clark Boylan proposed: [zuul/nodepool] 848331: Add python3.10 testing https://review.opendev.org/c/zuul/nodepool/+/848331 | 19:33 | |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: | 19:46 | |
| - [zuul/zuul] 845284: Clarify missing merge requirement message https://review.opendev.org/c/zuul/zuul/+/845284 | ||
| - [zuul/zuul] 846232: Improve gerrit connection event queue logging https://review.opendev.org/c/zuul/zuul/+/846232 | ||
| @clarkb:matrix.org | corvus: https://review.opendev.org/c/zuul/zuul/+/837063 is a straightforward one that I thought I'd call out due to small linkage to the comment work for circular deps/same topic merges | 20:06 |
| @clarkb:matrix.org | I didn't approve it to give you a cahnce to look at it if you want, but I'll probably approve later today | 20:07 |
| @jim:acmegating.com | Clark: i have previously reviewed that -- thanks for the reminder | 20:34 |
| @jim:acmegating.com | Clark: but what do you mean the "comment work"? | 20:37 |
| @clarkb:matrix.org | corvus: the multi phase reporting | 20:37 |
| @clarkb:matrix.org | they are both interacting with the "make a comment in code review system" portion fo the code but should be doing so in non interferring ways | 20:38 |
| @jim:acmegating.com | Clark: i agree; i don't think the two-phase reporting should cause any problems, so i wall approve that change (gate will tell us if i'm wrong) | 20:40 |
| @jim:acmegating.com | * Clark: i agree; i don't think the two-phase reporting should cause any problems, so i will approve that change (gate will tell us if i'm wrong) | 20:40 |
| @jim:acmegating.com | (and it looks like my earlier comments are addressed) | 20:40 |
| -@gerrit:opendev.org- Thiago Paiva Brito proposed: [zuul/zuul-jobs] 848334: Remove zuul_work_dir from role https://review.opendev.org/c/zuul/zuul-jobs/+/848334 | 20:44 | |
| -@gerrit:opendev.org- Thiago Paiva Brito proposed: [zuul/zuul-jobs] 848334: Remove zuul_work_dir from role https://review.opendev.org/c/zuul/zuul-jobs/+/848334 | 21:01 | |
| -@gerrit:opendev.org- Clark Boylan proposed: [zuul/nodepool] 848331: Add python3.10 testing https://review.opendev.org/c/zuul/nodepool/+/848331 | 21:15 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 848337: Include user/driver data in node detail list https://review.opendev.org/c/zuul/nodepool/+/848337 | 21:32 | |
| -@gerrit:opendev.org- Thiago Paiva Brito proposed: [zuul/zuul-jobs] 848339: Adding git commit message guidelines check https://review.opendev.org/c/zuul/zuul-jobs/+/848339 | 22:08 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 848341: Fix race in test_aws_resource_cleanup https://review.opendev.org/c/zuul/nodepool/+/848341 | 22:24 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 848337: Include user/driver data in node detail list https://review.opendev.org/c/zuul/nodepool/+/848337 | 22:25 | |
| @clarkb:matrix.org | corvus: does the last bit in https://review.opendev.org/c/zuul/zuul/+/846055/1/releasenotes/notes/dependency-validation-000f63204da83b4a.yaml mean that you'd have to correct these problems before being able to (re)start zuul? | 22:27 |
| @jim:acmegating.com | Clark: they would turn into config errors. i actually don't think it would even change the layout (ie, the project-pipeline stanza will still be there and include the job). but the error list would be one entry longer and have the reference exception. | 22:34 |
| @clarkb:matrix.org | got it | 22:34 |
| @jim:acmegating.com | Clark: i'd like you to double check me on that though | 22:34 |
| @jim:acmegating.com | because i thought something different this morning | 22:35 |
| @jim:acmegating.com | Clark: my thinking is this is the crux: https://opendev.org/zuul/zuul/src/branch/master/zuul/configloader.py#L2398-L2401 | 22:36 |
| @clarkb:matrix.org | it should raise an exception there which will bubble up through to TenantParser.fromYaml()? | 22:36 |
| @clarkb:matrix.org | I guess that is why I'm concerned because it isn't clear to me how far an exception can go and the initial config loading survive? | 22:36 |
| @jim:acmegating.com | but https://opendev.org/zuul/zuul/src/branch/master/zuul/configloader.py#L2400 should catch it | 22:36 |
| @clarkb:matrix.org | oh yup I missed that because it wasn'y a try except | 22:37 |
| @clarkb:matrix.org | I should pay more attention | 22:37 |
| @clarkb:matrix.org | so ya it won't bubble all the way up and the rest of the layout parsing will continue | 22:37 |
| @jim:acmegating.com | i mean i was wrong about this this morning... :) | 22:38 |
| @clarkb:matrix.org | ok ya I think this will do what you describe. | 22:38 |
| @jim:acmegating.com | and https://opendev.org/zuul/zuul/src/branch/master/zuul/model.py#L7076 is a bare Exception | 22:38 |
| @jim:acmegating.com | and that's what we should be raising | 22:38 |
| @clarkb:matrix.org | If it was a ConfigurationSyntaxError then my concern would be a problem | 22:38 |
| @jim:acmegating.com | right | 22:38 |
| @jim:acmegating.com | but https://opendev.org/zuul/zuul/src/branch/master/zuul/configloader.py#L334 catches bare Exceptions and accumulates only without reraising | 22:39 |
| @clarkb:matrix.org | This would be an AttributeError or similar which the bare exception catch should handle | 22:39 |
| @jim:acmegating.com | well in the case of the missing job (my favorite conan doyle story), we explicitly raise an Exception() at https://opendev.org/zuul/zuul/src/branch/master/zuul/model.py#L7076 | 22:41 |
| @clarkb:matrix.org | Ah ok. But ya in that case this should be "fine". It will be a new error but not change behavior for end users or operators really. Zuul can still be restarted and you'll get a new entry under the blue error bell | 22:41 |
| @clarkb:matrix.org | rather than jobs that just fail to run | 22:42 |
| @jim:acmegating.com | yep. | 22:42 |
| @clarkb:matrix.org | ok I'm going to approve it | 22:42 |
| @clarkb:matrix.org | https://review.opendev.org/c/zuul/zuul/+/848137 is another small bugfix I'm going to approve | 22:42 |
| @clarkb:matrix.org | hrm actually I may need to think about this one twice too :) | 22:43 |
| @clarkb:matrix.org | getBlobKeys() is used to get the live blobs that we don't want to delete. | 22:43 |
| @clarkb:matrix.org | The old behavior here is an exception which would've stopped us from deleting anything (and wait until alter to do the deletes safely) | 22:44 |
| @clarkb:matrix.org | is it possible that when we return set() there we'd end up with a short live blobs set and delete things we shouldn't and the exception is actually desireable? | 22:44 |
| @jim:acmegating.com | Clark: i *think* the stuff in scheduler.py that gets a starting ltime for the start of the cleanup should mean that any blobs that are in the process of being created should be excluded from cleanup even if that's a short set. | 22:46 |
| @clarkb:matrix.org | `unused_blobs = blobstore.getKeysLastUsedBefore(start_ltime)` that bit? | 22:47 |
| @jim:acmegating.com | yeah... but i'm a little less sure about that now... maybe it's possible to start creating a blob after that and then hit this case... | 22:48 |
| @jim:acmegating.com | oh we hold the pipeline lock! | 22:49 |
| @jim:acmegating.com | so nothing should be being created while we hold the lock | 22:49 |
| @jim:acmegating.com | so i think the combo of starting ltime, then holding the pipeline lock so that when we get the live blob keys we know it's stable means that we should be good | 22:50 |
| @jim:acmegating.com | basically, either getBlobKeys will work as originally intended, or, if we hit that exception, then there really is nothing there (ie, nothing is in the process of being created) | 22:51 |
| @clarkb:matrix.org | and start_ltime is from before we hold the lock so we're looking at everything created before the lock is held anyway | 22:51 |
| @jim:acmegating.com | yep | 22:51 |
| @clarkb:matrix.org | ya so nothing new should be evaluated | 22:51 |
| @clarkb:matrix.org | ok ya that should make it safe to return an empty set there | 22:51 |
| @clarkb:matrix.org | and in that case we'll clean up blobs more quickly and not need to occasionally avoid the exception to claen things up | 22:52 |
| @clarkb:matrix.org | corvus: did you want to rereview https://review.opendev.org/c/zuul/zuul/+/735475? | 22:53 |
| @clarkb:matrix.org | https://review.opendev.org/c/zuul/zuul/+/817393 too though that one you reviewed far less recently | 22:53 |
| @jim:acmegating.com | done and done, thx | 22:55 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 848344: Include backing node request id in metastatic log https://review.opendev.org/c/zuul/nodepool/+/848344 | 23:07 | |
| @iwienand:matrix.org | does anyone have a problem with the dib bump in https://review.opendev.org/c/zuul/nodepool/+/847698 so we can get fresh containers? | 23:10 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/nodepool] 846432: AWS driver create/delete improvements https://review.opendev.org/c/zuul/nodepool/+/846432 | 23:24 | |
| @clarkb:matrix.org | swest: left some questions on https://review.opendev.org/c/zuul/zuul/+/848234 | 23:27 |
| @clarkb:matrix.org | > <@iwienand:matrix.org> does anyone have a problem with the dib bump in https://review.opendev.org/c/zuul/nodepool/+/847698 so we can get fresh containers? | 23:27 |
| No objections here. Approved | ||
| @jim:acmegating.com | ianw: https://review.opendev.org/848341 is a fix to the fix we were working on recently | 23:31 |
| @iwienand:matrix.org | oh right; will context switch that back in a few | 23:32 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 848172: AWS multi quota support https://review.opendev.org/c/zuul/nodepool/+/848172 | 23:41 | |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 846449: Add extra validation to dynamic-only projects https://review.opendev.org/c/zuul/zuul/+/846449 | 23:58 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!