Thursday, 2021-07-15

*** rpittau|afk is now known as rpittau07:24
opendevreviewFelix Edel proposed zuul/zuul master: Make reporting asynchronous  https://review.opendev.org/c/zuul/zuul/+/69125309:07
opendevreviewSimon Westphahl proposed zuul/zuul master: Create config cache ltime before requesting files  https://review.opendev.org/c/zuul/zuul/+/80065910:22
opendevreviewSimon Westphahl proposed zuul/zuul master: Ensure config cache stages are used correctly  https://review.opendev.org/c/zuul/zuul/+/80066010:22
opendevreviewSimon Westphahl proposed zuul/zuul master: Ensure config cache stages are used correctly  https://review.opendev.org/c/zuul/zuul/+/80066010:31
*** dviroel|out is now known as dviroel11:02
opendevreviewFelix Edel proposed zuul/zuul master: Execute merge jobs via ZooKeeper  https://review.opendev.org/c/zuul/zuul/+/78768611:07
opendevreviewSimon Westphahl proposed zuul/zuul master: Lock tenants, pipelines, queues during processing  https://review.opendev.org/c/zuul/zuul/+/79601311:10
swestclarkb: corvus: I updated 796013 and moved the locking into doTenantReconfigureEvent()11:13
*** iurygregory_ is now known as iurygregory12:07
opendevreviewSimon Westphahl proposed zuul/zuul master: Lock tenants, pipelines, queues during processing  https://review.opendev.org/c/zuul/zuul/+/79601312:36
opendevreviewSimon Westphahl proposed zuul/zuul master: Create config cache ltime before requesting files  https://review.opendev.org/c/zuul/zuul/+/80065912:36
opendevreviewSimon Westphahl proposed zuul/zuul master: Ensure config cache stages are used correctly  https://review.opendev.org/c/zuul/zuul/+/80066012:36
swestcorvus: clarkb: I've stacked 800659 and 800660 on top of 796013 since they had a conflict12:38
*** chkumar|rover is now known as chandankumar14:05
corvusswest: i have a question on the latest ps of https://review.opendev.org/79601314:12
corvusi think i answered it14:18
corvussince it's an extra enhancement on top of clarkb's suggestion, i'll wait for him to re-review14:18
clarkbcorvus: swest: the first two changes were easy reviews for me due to the previous reviews I did. I'll need to dig into the third a bit more as I didn't manage to do that yesterday15:00
clarkbI haven't approved the first two because I wasn't sure if we wanted them to go in together15:00
clarkbfeel free to approve them if not15:00
corvusyeah, i'm going to review the third after breakfast and we'll see where we are15:00
clarkbalright my time in the TC meeting is done. I need breakfast too. I'll jump into that review in a bit15:38
opendevreviewJames E. Blair proposed zuul/zuul master: Ensure config cache stages are used correctly  https://review.opendev.org/c/zuul/zuul/+/80066016:12
corvusi just fixed some docstring nits16:13
*** rpittau is now known as rpittau|afk16:17
*** marios is now known as marios|out16:25
*** sshnaidm is now known as sshnaidm|afk16:35
clarkbcorvus: swest: just to make sure i understand one important detail. zstat = self.client.set("/zuul/ltime", b"") in getCurrentLtime() will bump the ltime forward because we are doing a write?17:02
clarkbbasically any getCurrentLtime() create a checkpoint and is likely to force caches to update17:02
corvusclarkb: correct17:03
corvuser, well, getting the ltime doesn't force caches to update17:03
clarkbit does if you then pass that as the min_ltime, but ya just getting the value doesn't17:03
corvusright; don't mean to be pedantic, just clear :)17:04
clarkb++17:04
corvusassuming all the caches are older than that, and you specify a min that is 'current', then yeah, they'll be considered invalid -- but: another scheduler might come around and update them right after you do that, and by the time you get around to consulting the cache, it might actually be valid again.17:05
opendevreviewJames E. Blair proposed zuul/zuul master: Use the ZK cache for smart reconfiguration events  https://review.opendev.org/c/zuul/zuul/+/80097317:08
clarkbcorvus: I've +2'd the stack. Will defer to you on approving it17:09
corvusclarkb: i'll +w17:12
opendevreviewJames E. Blair proposed zuul/zuul master: Use the ZK cache for smart reconfiguration events  https://review.opendev.org/c/zuul/zuul/+/80097317:12
corvusclarkb: discussion of my point on that change can continue in that ^17:12
clarkbcorvus: ++ I think that helps me re "basically any getCurrentLtime() create a checkpoint and is likely to force caches to update"17:14
clarkbthe extra cache updating was a side effect of how we had written things but not necessary17:15
clarkbnow I wonder if it is safe to restart without your change in opendev? don't we do a lot of smart reconfigures? maybe its fine17:15
corvusclarkb: pretty sure we're already running with the change that introduced that behavior17:15
clarkboh17:16
clarkbhttps://review.opendev.org/c/zuul/zuul/+/800660/9/zuul/scheduler.py is it not that that does it?17:16
corvusclarkb: hrm, i thought i traced it back to ca0d37997391ef7f9c02dd6372bb3bc2dd587271 but i may be wrong17:18
corvusclarkb: yeah, you are right, experimentation says 800660 introduces the behavior, so my code-reading was wrong17:18
corvusi'll unapprove 80066017:19
clarkbok17:20
corvusclarkb: looking at 800660, on the old side, it always set the required ltime to the event ltime, so it should always consider the zk file cache invalid -- why is it not emitting cat jobs to refresh it then?17:22
corvusoh, is it because it used the unparsed branch cache, so it never looked at the files cache?17:25
corvusthat's configloader.py lines 1793-1797 on the old side17:25
clarkboh good question. For some reason I was thinking the if not smart block handled this but that only more forcefully causes caches to update on the non smart case17:25
clarkbcorvus: oh yup that wasn't passing in the event ltime so it must've thought it was valid?17:26
clarkband the hit the continue17:26
corvusclarkb: okay, i switched my vote to -1; i think we should squash my followup into it.  if you agree, i think we can wait for swest to weigh in on that17:32
clarkbyes I think your update is likely going to be helpful for opendev and squashing is a good idea17:33
corvusi really like the new version of _cacheTenantYaml; it reads really well if you read it straight through now ("if the files cache is valid for.... if the unparsed config cache is valid for... etc)17:33
avass[m]corvus: I think the operator doesn't react to changes to the nodepool config secret, I expect that's a bug and not indented behaviour right?17:34
avass[m]I mean, I'd expect it to react to any changes to the secret and modify the environment accordingly17:34
corvusavass: yes, it should create/destroy launchers and update the generated nodepool.yaml files accordingly17:35
avass[m]okay then it's probably a bug, I'll take a closer look at that later/tomorrow17:36
corvusavass: i'll look into it too -- but the idea is that kopf should notify us on every secret change in the system (which is not great, but not too bad -- only way i can think to improve that would be to add another CRD, or maybe we could do something with labels?) and we check to see if it's the nodepool config secret, and if it is, then we re-run the nodepool bits.  that's in operator.py.17:43
corvusavass: oh, i think the nodepool secret update detection may only work after an operator restart -- ie, if you start the operator and build a system from scratch, it won't know to watch that secret.  should be fixable.18:01
corvusavass: quick fix: kill and restart the operator pod :)18:02
avass[m]corvus: got it, should be easy to do for now :)18:02
corvusavass: i'm testing a fix; it'll be a little bit since a test cycle requires a full rebuild18:05
opendevreviewMerged zuul/zuul master: Lock tenants, pipelines, queues during processing  https://review.opendev.org/c/zuul/zuul/+/79601318:26
*** dviroel is now known as dviroel|brb19:53
*** dviroel|brb is now known as dviroel20:27
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Add static node to functional test  https://review.opendev.org/c/zuul/zuul-operator/+/79987420:48
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers  https://review.opendev.org/c/zuul/zuul-operator/+/79999820:48
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Set component command with args instead of command  https://review.opendev.org/c/zuul/zuul-operator/+/80026320:48
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Configure debug logs for merger  https://review.opendev.org/c/zuul/zuul-operator/+/80026420:48
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Make nodepool external_config mount more generic  https://review.opendev.org/c/zuul/zuul-operator/+/80078620:48
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Document externalConfig  https://review.opendev.org/c/zuul/zuul-operator/+/80079120:48
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Fix config update detection  https://review.opendev.org/c/zuul/zuul-operator/+/80099120:48
corvusavass, tristanC: i figured out the test race condition that was affecting the last few changes; it relates to adding the static node test; so i updated 799874 with a fix for that (also improved the pod test slightly i think).  then 800991 is a fix for the issue avass noticed earlier.  the rest are just rebases.20:50
opendevreviewJames E. Blair proposed zuul/zuul master: Fix race when canceling a node request  https://review.opendev.org/c/zuul/zuul/+/80099421:00
corvusthat snagged one of the changes we approved earlier21:00
avass[m]corvus: I'll see if I can take a look at those tomorrow!21:01
corvusthe enhanced distributed executor stack (the spec we recently approved to route fingergw to inacessible network segments) is ready to go; it has at least a +2 on every change: it starts at https://review.opendev.org/66494921:15
*** dviroel is now known as dviroel|out21:24
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Add static node to functional test  https://review.opendev.org/c/zuul/zuul-operator/+/79987422:23
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers  https://review.opendev.org/c/zuul/zuul-operator/+/79999822:24
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Set component command with args instead of command  https://review.opendev.org/c/zuul/zuul-operator/+/80026322:24
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Configure debug logs for merger  https://review.opendev.org/c/zuul/zuul-operator/+/80026422:24
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Make nodepool external_config mount more generic  https://review.opendev.org/c/zuul/zuul-operator/+/80078622:24
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Document externalConfig  https://review.opendev.org/c/zuul/zuul-operator/+/80079122:24
opendevreviewJames E. Blair proposed zuul/zuul-operator master: Fix config update detection  https://review.opendev.org/c/zuul/zuul-operator/+/80099122:24
opendevreviewJames E. Blair proposed zuul/zuul master: Always report the build page  https://review.opendev.org/c/zuul/zuul/+/80011223:46
corvusthis is cool -- that change needed an update to the quick-start because it improves the UX for new users -- the word "finger" no longer appears in the tutorial.  :)23:47
corvusi made new screenshots23:49

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