Monday, 2021-05-31

*** dmellado_ has joined #zuul00:07
*** dmellado has quit IRC00:11
*** dmellado_ is now known as dmellado00:11
*** marios has joined #zuul05:12
*** swest has joined #zuul05:36
*** jpena|off is now known as jpena07:33
opendevreviewSimon Westphahl proposed zuul/zuul master: Remove layout attribute from queue items  https://review.opendev.org/c/zuul/zuul/+/79262207:53
opendevreviewSimon Westphahl proposed zuul/zuul master: Optimize layout re-calculation after re-enqueue  https://review.opendev.org/c/zuul/zuul/+/79262307:53
opendevreviewSimon Westphahl proposed zuul/zuul master: Remove layout attribute from queue items  https://review.opendev.org/c/zuul/zuul/+/79262208:00
opendevreviewSimon Westphahl proposed zuul/zuul master: Optimize layout re-calculation after re-enqueue  https://review.opendev.org/c/zuul/zuul/+/79262308:00
*** rpittau has joined #zuul08:24
*** ttx has joined #zuul09:15
*** tosky has joined #zuul10:09
*** jpena is now known as jpena|lunch11:31
*** jangutter has joined #zuul12:08
opendevreviewSimon Westphahl proposed zuul/zuul master: Show pipeline event queue lengths in Zuul web  https://review.opendev.org/c/zuul/zuul/+/79378112:23
*** jpena|lunch is now known as jpena12:32
mhuHello zuul-maint, here's a simple fix in zuul-client's doc: https://review.opendev.org/c/zuul/zuul-client/+/78900712:45
mhuzuul-maint, here's also support for builds & buildsets querying in the CLI: https://review.opendev.org/c/zuul/zuul-client/+/751070 https://review.opendev.org/c/zuul/zuul/+/758783 https://review.opendev.org/c/zuul/zuul-client/+/752909 https://review.opendev.org/c/zuul/zuul/+/75898512:51
*** measure20[m] has left #zuul13:03
opendevreviewMatthieu Huin proposed zuul/zuul-client master: Add support for XDG-compliant config file  https://review.opendev.org/c/zuul/zuul-client/+/79379613:45
mhuswest, what are the event queue lengths for (re: https://review.opendev.org/c/zuul/zuul/+/793781 ) - I'm looking at the openstack tenant status page and all the lengths are set to 0 regardless of what's currently in the pipelines13:51
swestmhu: it's showing the length of the pipeline event queues in Zookeeper. if there are no unprocessed event the length is expected to be 013:54
fungiright, on opendev you'll see those spike up when there are bursts of reviews submitted or changes merged which cause events or results to queue up awaiting processing, but that's independent of the pipeline queues14:01
fungithey represent internal fifo queues zuul keeps potential triggering conditions in until it has a chance to evaluate them to find out if it should enqueue or report something14:03
swestI'm just not sure though if we want to display this as prominently as in my proposal. I'm open to other ideas14:03
mhuok thanks that explains it. I think it deserves at least a mention of Zookeeper or some similar kind of explanation, as it might be confusing as is14:05
fungiit mostly tends to come up when people looking at the status page are scratching their heads wondering why a change they pushed hasn't shown up in any queue yet, or why there's an item which has completed all its builds but is still just sitting in a pipeline with nothing ahead blocking it from merging14:05
fungithose are generally represented by one or more conditions held in the events or results queues14:05
fungi(a gerrit patchset-uploaded event, an internal buildset completion event...)14:06
mhuI think the confusion also stems from the use of the term "queue" in dependent pipelines themselves, IIUC these are completely different concepts14:09
fungiyes, different class of queue, but they still are both first-in/first-out queue structures14:12
fungithe other common term is "pipe" but we already have pipelines so that could also get confusing14:13
*** y2kenny has joined #zuul14:18
fungion a different topic, anybody happen to know if there is weirdness around builds not seeing configuration changes when the zuul config file is updated by a merge commit which has been pushed for review? i have an example where someone changed a nodeset on the master branch of their project, then proposed a merge from master to a feature branch, and the builds for that did not use the new nodeset.14:18
fungii downloaded the merge commit change and confirmed the changed nodeset value does appear in my local tree, and the build inventory indicates (in its inheritance path) that the job definition was taken from the branch that merge commit is being proposed to. also the project is just a normal untrusted project so it shouldn't be anything to do with config project protections14:18
y2kennyHi, I noticed the latest zuul-launcher on docker hub is still 4.1.0 (https://hub.docker.com/r/zuul/nodepool-launcher/tags?page=1&ordering=last_updated) is that intentional?14:39
fungiy2kenny: it has a different checksum than the 4.1.0 image14:40
avass[m]y2kenny: nodepool doesn't sync it's releases with zuul14:41
fungiwe generally keep the major version number the same between them, but yes the minor versions can be (and often are) different14:42
fungiand the latest version of nodepool is indeed 4.1.014:43
fungibut the "latest" tag there i think represents commits after nodepool's 4.1.0 tag14:43
avass[m]yeah14:43
*** timburke has joined #zuul14:50
*** ricolin has joined #zuul15:00
y2kennyfungi, avass[m]: ok thanks.15:04
opendevreviewMatthieu Huin proposed zuul/zuul-client master: Clarify ~/.zuul.conf and --use-config option spelling  https://review.opendev.org/c/zuul/zuul-client/+/78900715:05
opendevreviewMatthieu Huin proposed zuul/zuul-client master: Add support for XDG-compliant config file  https://review.opendev.org/c/zuul/zuul-client/+/79379615:05
corvusswest, mhu: maybe we should drop the queue lengths to the bottom of the page to indicate they are more 'system status' types of info15:15
corvusfungi: yes, i have observed the same in gerrit's gerrit; a full reconfiguration will be needed for zuul to see the update15:16
fungicorvus: full reconfiguration will cause it to notice the changed configuration in the merge commit and apply it, or you mean it needs to be backported separately and merged before the merge commit is proposed?15:17
mhucorvus: yes, and maybe have a "show/hide FIFO queue statistics" button or something like that15:18
mhuas it seems to be some advanced, "under the hood" kind of info15:19
fungiif it's embedded in the page footer with the version info or something, i don't know that it needs a toggle15:19
fungiat least in opendev it's not advanced under the hood information, we have to point our users to it on a regular basis15:20
mhufungi, swest added queue info for every pipeline though15:20
fungiahh, okay15:20
corvusi haven't reviewed the ui change yet; so i'm only speaking in generalities15:25
*** dmsimard has joined #zuul15:25
corvusfungi: er, i mean that after the merge commit merges, a full reconfiguration will be needed for zuul to adopt the changes15:26
corvusnothing will make it notice the change behind the merge commit15:26
fungicorvus: got it, so if they want zuul configuration changes from master to take effect in a feature branch, they'll need to separately backport those as a parent of the proposed merge?15:30
corvusyeah that should work15:30
guillaumecfungi: corvus  I have 2 patches for that: https://review.opendev.org/c/zuul/zuul/+/762886/ https://review.opendev.org/c/zuul/zuul/+/76288715:31
guillaumecie: configuration changes with merge commits15:32
corvusgchauvel: nice, thanks!15:32
guillaumecthey both need some rework, they are no longer mergeable :)15:33
*** rpittau is now known as rpittau|afk16:07
*** mnaser has joined #zuul16:40
*** marios has quit IRC16:40
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: WIP test stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382916:42
*** jpena is now known as jpena|off16:45
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: WIP test stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382916:46
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: WIP test stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382917:01
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: WIP test stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382917:13
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: WIP test stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382917:24
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Handle no-sudo in stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382917:35
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Tidy up file matcher for bindep jobs  https://review.opendev.org/c/zuul/zuul-jobs/+/79383417:41
corvuszbr: ^ you added the line i'm proposing we remove there; i suspect it was either a copy-pasta or was from before zuul automatically ran jobs with changed definitions. can you confirm?17:42
*** opendevreview has quit IRC17:47
avass[m]corvus: I generally try to avoid setting facts unless I need to since it can cause issues for subsequent roles if they use a variable with the same name since ansible has no scopes. There can also be variables set with higher precedence so set_fact won't work (see: https://opendev.org/zuul/zuul-jobs/commit/6f2ff3f1b8aa899d2a01d73e7de081ce5f9cffcd), now that's true for registered variables too but I try to avoid that by following python private18:31
avass[m]variable conventions (`_have_sudo` instead of `have_sudo`).18:31
corvusoh yeah, i forgot we have a rule, that should be called "zjhavezudo" i think18:32
avass[m]I don't think we have a rule for that, the `zj_` prefix is for loop variables and we have a linting rule for that18:32
corvusoh that may have eaten underscores18:32
corvushrm, you're right, that is only for loops18:32
corvusavass: sounds like a good addition to https://zuul-ci.org/docs/zuul-jobs/policy.html :)18:33
avass[m]I think I already started on it somewhere :)18:34
corvusavass: but in the mean time, sounds like you suggest we should either give that var a tail, or just use the result expression in those 3 places and skip the set fact?18:34
*** opendevreview has joined #zuul18:36
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Handle no-sudo in stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382918:36
avass[m]yeah, maybe even giving the registered variable a tail since that would also be affected, so `_sudo_result`18:36
corvusavass: ^ i chose skipping set_fact because it eliminates a task :)18:36
avass[m]an earlier set_fact: sudo_result would take precedence over a registered sudo_result and would be annoying to debug :)18:38
corvusavass: of course the same would be true for sudoresult :)18:38
avass[m]though _sudo_result would have the same issue if set as a fact and registered.18:38
corvusavass: i feel like this is a can of worms that may be worth opening (maybe we should name-scope all our variables), but maybe we could draw the line somewhere so my patch doesn't get caught up in it?  i'd draw the line at "consistent with the rest of the file and doesn't add any new potentially tricky usages"18:39
corvuswhich is coincidentally what i just uploaded ;)18:40
corvusavass: but if you feel we should start doing something else for all new variables, i'm happy to change it18:40
avass[m]yeah, my rule of thumb is to just not set facts unless needed (usually when it needs to persist across playbooks)18:40
*** measure20[m] has joined #zuul18:47
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Handle no-sudo in stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382918:55
avass[m]corvus: you can actually do that without registering sudo_result18:56
avass[m]but I don't think I like it :)18:56
corvusheh, okay.  it sounds scary.  :)18:57
corvusavass: are you ok with that version of the patch?  or do you want me to add the tail or do "stageoutputsudoresult" ?18:57
avass[m]I'm fine with it18:57
corvusgrr, sorry... should i _stage_output_sudo_result ?18:57
corvusok, i'll leave it as is, and now i know how to type underscores :)18:58
avass[m]I'm mostly just trying to figure out how to avoid this if in the future we have hundreds of variables sprinkled in the all roles that could lead to hard to trace bugs for people downstream18:59
avass[m]current idea is to somehow use block/rescue18:59
avass[m]but namespacing variables is probably nicer and easier19:00
corvusmaybe we could put all vars in a dict with a unique name; like setfact _zjstageoutput: {}; then access _zjstageoutput.sudoresult19:01
corvusi forgot to quote that again19:01
corvus * maybe we could put all vars in a dict with a unique name; like set_fact `_zj_stage_output: {}`; then access `_zj_stage_output.sudo_result`19:01
avass[m]in that case it's seems easier to just do `_zj_stage_output_sudo_result` or `_stage_output_sudo_result`19:03
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Tidy up file matcher for bindep jobs  https://review.opendev.org/c/zuul/zuul-jobs/+/79383419:03
corvusprobably.  though the dict gets you one-stop initialization in case there's an existing var conflict.19:04
corvusthe risk may not be worth the extra typing though19:04
avass[m]in that case the question is if we want to fail if anything collides or try our best to run the role anyway?19:09
avass[m]is it possible to register variables in a dict?19:09
avass[m]doesn't look like it: `Invalid variable name in 'register' specified: 'my_var.a'`19:10
corvusthen nevermind :)19:16
opendevreviewMatthieu Huin proposed zuul/zuul-client master: Add support for XDG-compliant config file  https://review.opendev.org/c/zuul/zuul-client/+/79379619:26
*** opendevreview has quit IRC19:48
*** opendevmeet has joined #zuul19:53
avass[m]curl apparently has issues with travisCI new open source policies: https://github.com/curl/curl/issues/715019:54
avass[m]mnaser: oh I see you've already commented :)19:55
*** timburke has quit IRC22:15
*** opendevreview has joined #zuul22:40
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Handle no-sudo in stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382922:40
corvusianw: ^22:41
ianwcorvus:   become: "{{ not sudo_result.failed }}" won't be true now because the task won't have failed?  might have to check return code?22:44
corvushrm maybe22:45
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Handle no-sudo in stage-output  https://review.opendev.org/c/zuul/zuul-jobs/+/79382922:48
corvusianw: testing should catch that sort of thing now at least :)22:48
ianwyep!  thanks, that looks correct to the slightly unreliable ansible interpreter in my head :)22:49
opendevreviewJames E. Blair proposed zuul/zuul-jobs master: Tidy up file matcher for bindep jobs  https://review.opendev.org/c/zuul/zuul-jobs/+/79383422:52
*** tosky has quit IRC22:59

Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!