*** rpittau|afk is now known as rpittau | 07:24 | |
opendevreview | Felix Edel proposed zuul/zuul master: Make reporting asynchronous https://review.opendev.org/c/zuul/zuul/+/691253 | 09:07 |
---|---|---|
opendevreview | Simon Westphahl proposed zuul/zuul master: Create config cache ltime before requesting files https://review.opendev.org/c/zuul/zuul/+/800659 | 10:22 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Ensure config cache stages are used correctly https://review.opendev.org/c/zuul/zuul/+/800660 | 10:22 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Ensure config cache stages are used correctly https://review.opendev.org/c/zuul/zuul/+/800660 | 10:31 |
*** dviroel|out is now known as dviroel | 11:02 | |
opendevreview | Felix Edel proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/787686 | 11:07 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Lock tenants, pipelines, queues during processing https://review.opendev.org/c/zuul/zuul/+/796013 | 11:10 |
swest | clarkb: corvus: I updated 796013 and moved the locking into doTenantReconfigureEvent() | 11:13 |
*** iurygregory_ is now known as iurygregory | 12:07 | |
opendevreview | Simon Westphahl proposed zuul/zuul master: Lock tenants, pipelines, queues during processing https://review.opendev.org/c/zuul/zuul/+/796013 | 12:36 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Create config cache ltime before requesting files https://review.opendev.org/c/zuul/zuul/+/800659 | 12:36 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Ensure config cache stages are used correctly https://review.opendev.org/c/zuul/zuul/+/800660 | 12:36 |
swest | corvus: clarkb: I've stacked 800659 and 800660 on top of 796013 since they had a conflict | 12:38 |
*** chkumar|rover is now known as chandankumar | 14:05 | |
corvus | swest: i have a question on the latest ps of https://review.opendev.org/796013 | 14:12 |
corvus | i think i answered it | 14:18 |
corvus | since it's an extra enhancement on top of clarkb's suggestion, i'll wait for him to re-review | 14:18 |
clarkb | corvus: 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 yesterday | 15:00 |
clarkb | I haven't approved the first two because I wasn't sure if we wanted them to go in together | 15:00 |
clarkb | feel free to approve them if not | 15:00 |
corvus | yeah, i'm going to review the third after breakfast and we'll see where we are | 15:00 |
clarkb | alright my time in the TC meeting is done. I need breakfast too. I'll jump into that review in a bit | 15:38 |
opendevreview | James E. Blair proposed zuul/zuul master: Ensure config cache stages are used correctly https://review.opendev.org/c/zuul/zuul/+/800660 | 16:12 |
corvus | i just fixed some docstring nits | 16:13 |
*** rpittau is now known as rpittau|afk | 16:17 | |
*** marios is now known as marios|out | 16:25 | |
*** sshnaidm is now known as sshnaidm|afk | 16:35 | |
clarkb | corvus: 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 |
clarkb | basically any getCurrentLtime() create a checkpoint and is likely to force caches to update | 17:02 |
corvus | clarkb: correct | 17:03 |
corvus | er, well, getting the ltime doesn't force caches to update | 17:03 |
clarkb | it does if you then pass that as the min_ltime, but ya just getting the value doesn't | 17:03 |
corvus | right; don't mean to be pedantic, just clear :) | 17:04 |
clarkb | ++ | 17:04 |
corvus | assuming 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 |
opendevreview | James E. Blair proposed zuul/zuul master: Use the ZK cache for smart reconfiguration events https://review.opendev.org/c/zuul/zuul/+/800973 | 17:08 |
clarkb | corvus: I've +2'd the stack. Will defer to you on approving it | 17:09 |
corvus | clarkb: i'll +w | 17:12 |
opendevreview | James E. Blair proposed zuul/zuul master: Use the ZK cache for smart reconfiguration events https://review.opendev.org/c/zuul/zuul/+/800973 | 17:12 |
corvus | clarkb: discussion of my point on that change can continue in that ^ | 17:12 |
clarkb | corvus: ++ I think that helps me re "basically any getCurrentLtime() create a checkpoint and is likely to force caches to update" | 17:14 |
clarkb | the extra cache updating was a side effect of how we had written things but not necessary | 17:15 |
clarkb | now I wonder if it is safe to restart without your change in opendev? don't we do a lot of smart reconfigures? maybe its fine | 17:15 |
corvus | clarkb: pretty sure we're already running with the change that introduced that behavior | 17:15 |
clarkb | oh | 17:16 |
clarkb | https://review.opendev.org/c/zuul/zuul/+/800660/9/zuul/scheduler.py is it not that that does it? | 17:16 |
corvus | clarkb: hrm, i thought i traced it back to ca0d37997391ef7f9c02dd6372bb3bc2dd587271 but i may be wrong | 17:18 |
corvus | clarkb: yeah, you are right, experimentation says 800660 introduces the behavior, so my code-reading was wrong | 17:18 |
corvus | i'll unapprove 800660 | 17:19 |
clarkb | ok | 17:20 |
corvus | clarkb: 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 |
corvus | oh, is it because it used the unparsed branch cache, so it never looked at the files cache? | 17:25 |
corvus | that's configloader.py lines 1793-1797 on the old side | 17:25 |
clarkb | oh 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 case | 17:25 |
clarkb | corvus: oh yup that wasn't passing in the event ltime so it must've thought it was valid? | 17:26 |
clarkb | and the hit the continue | 17:26 |
corvus | clarkb: 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 that | 17:32 |
clarkb | yes I think your update is likely going to be helpful for opendev and squashing is a good idea | 17:33 |
corvus | i 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 accordingly | 17:34 |
corvus | avass: yes, it should create/destroy launchers and update the generated nodepool.yaml files accordingly | 17:35 |
avass[m] | okay then it's probably a bug, I'll take a closer look at that later/tomorrow | 17:36 |
corvus | avass: 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 |
corvus | avass: 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 |
corvus | avass: quick fix: kill and restart the operator pod :) | 18:02 |
avass[m] | corvus: got it, should be easy to do for now :) | 18:02 |
corvus | avass: i'm testing a fix; it'll be a little bit since a test cycle requires a full rebuild | 18:05 |
opendevreview | Merged zuul/zuul master: Lock tenants, pipelines, queues during processing https://review.opendev.org/c/zuul/zuul/+/796013 | 18:26 |
*** dviroel is now known as dviroel|brb | 19:53 | |
*** dviroel|brb is now known as dviroel | 20:27 | |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 20:48 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers https://review.opendev.org/c/zuul/zuul-operator/+/799998 | 20:48 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Set component command with args instead of command https://review.opendev.org/c/zuul/zuul-operator/+/800263 | 20:48 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Configure debug logs for merger https://review.opendev.org/c/zuul/zuul-operator/+/800264 | 20:48 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Make nodepool external_config mount more generic https://review.opendev.org/c/zuul/zuul-operator/+/800786 | 20:48 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Document externalConfig https://review.opendev.org/c/zuul/zuul-operator/+/800791 | 20:48 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix config update detection https://review.opendev.org/c/zuul/zuul-operator/+/800991 | 20:48 |
corvus | avass, 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 |
opendevreview | James E. Blair proposed zuul/zuul master: Fix race when canceling a node request https://review.opendev.org/c/zuul/zuul/+/800994 | 21:00 |
corvus | that snagged one of the changes we approved earlier | 21:00 |
avass[m] | corvus: I'll see if I can take a look at those tomorrow! | 21:01 |
corvus | the 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/664949 | 21:15 |
*** dviroel is now known as dviroel|out | 21:24 | |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 22:23 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers https://review.opendev.org/c/zuul/zuul-operator/+/799998 | 22:24 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Set component command with args instead of command https://review.opendev.org/c/zuul/zuul-operator/+/800263 | 22:24 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Configure debug logs for merger https://review.opendev.org/c/zuul/zuul-operator/+/800264 | 22:24 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Make nodepool external_config mount more generic https://review.opendev.org/c/zuul/zuul-operator/+/800786 | 22:24 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Document externalConfig https://review.opendev.org/c/zuul/zuul-operator/+/800791 | 22:24 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix config update detection https://review.opendev.org/c/zuul/zuul-operator/+/800991 | 22:24 |
opendevreview | James E. Blair proposed zuul/zuul master: Always report the build page https://review.opendev.org/c/zuul/zuul/+/800112 | 23:46 |
corvus | this 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 |
corvus | i made new screenshots | 23:49 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!