| @fungicide:matrix.org | yeah, that was all i could think of was new cleanups finding more cruft we didn't know we had | 00:04 |
|---|---|---|
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 816595: Store connection branch cache in ZK https://review.opendev.org/c/zuul/zuul/+/816595 | 00:16 | |
| -@gerrit:opendev.org- Dong Zhang proposed: [zuul/zuul] 816433: WIP: pin github3.py version. https://review.opendev.org/c/zuul/zuul/+/816433 | 01:13 | |
| -@gerrit:opendev.org- Dong Zhang proposed: [zuul/zuul] 816433: WIP: pin github3.py version. https://review.opendev.org/c/zuul/zuul/+/816433 | 03:33 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Ian Wienand: [zuul/nodepool] 816555: Bump DIB requirement to 3.15.0 https://review.opendev.org/c/zuul/nodepool/+/816555 | 04:59 | |
| -@gerrit:opendev.org- Dong Zhang proposed: [zuul/zuul] 816433: WIP: pin github3.py version. https://review.opendev.org/c/zuul/zuul/+/816433 | 05:36 | |
| -@gerrit:opendev.org- Dong Zhang proposed: [zuul/zuul] 816433: WIP: pin github3.py version. https://review.opendev.org/c/zuul/zuul/+/816433 | 06:27 | |
| -@gerrit:opendev.org- Dong Zhang proposed: [zuul/zuul] 816433: WIP: pin github3.py version https://review.opendev.org/c/zuul/zuul/+/816433 | 07:27 | |
| -@gerrit:opendev.org- Felix Edel proposed: [zuul/zuul] 816514: Implement managenet events directly in zuul-web https://review.opendev.org/c/zuul/zuul/+/816514 | 08:28 | |
| -@gerrit:opendev.org- Dong Zhang proposed: [zuul/zuul] 816433: WIP: pin github3.py version https://review.opendev.org/c/zuul/zuul/+/816433 | 09:06 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 769943: Example Docker compose: keycloak integration https://review.opendev.org/c/zuul/zuul/+/769943 | 09:23 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Tobias Henkel: [zuul/zuul] 816528: Pin github3.py to <3.0.0 https://review.opendev.org/c/zuul/zuul/+/816528 | 09:36 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 734082: web UI: user login with OpenID Connect https://review.opendev.org/c/zuul/zuul/+/734082 | 09:40 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: | 10:11 | |
| - [zuul/zuul] 734082: web UI: user login with OpenID Connect https://review.opendev.org/c/zuul/zuul/+/734082 | ||
| - [zuul/zuul] 734850: web UI: allow a privileged user to dequeue a change https://review.opendev.org/c/zuul/zuul/+/734850 | ||
| - [zuul/zuul] 736772: web UI: allow a privileged user to re-enqueue a change https://review.opendev.org/c/zuul/zuul/+/736772 | ||
| - [zuul/zuul] 768115: Web UI: allow a privileged user to request autohold https://review.opendev.org/c/zuul/zuul/+/768115 | ||
| - [zuul/zuul] 768199: Web UI: add Autoholds, Autohold page https://review.opendev.org/c/zuul/zuul/+/768199 | ||
| - [zuul/zuul] 810699: Web UI: Show pipeline types as icons https://review.opendev.org/c/zuul/zuul/+/810699 | ||
| - [zuul/zuul] 781858: web UI: allow a privileged user to promote a change https://review.opendev.org/c/zuul/zuul/+/781858 | ||
| - [zuul/zuul] 802559: Web UI: Add "Create Autohold Request" form, improve API error messages https://review.opendev.org/c/zuul/zuul/+/802559 | ||
| - [zuul/zuul] 769943: Example Docker compose: keycloak integration https://review.opendev.org/c/zuul/zuul/+/769943 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 13:12 | |
| - [zuul/zuul] 816686: Refresh branch cache depending on min. ltime https://review.opendev.org/c/zuul/zuul/+/816686 | ||
| - [zuul/zuul] 816687: Add branch cache ltime to trigger events https://review.opendev.org/c/zuul/zuul/+/816687 | ||
| - [zuul/zuul] 816688: Forward cache ltime with tenant reconfig event https://review.opendev.org/c/zuul/zuul/+/816688 | ||
| - [zuul/zuul] 816689: Add source interface for getting the cache ltime https://review.opendev.org/c/zuul/zuul/+/816689 | ||
| - [zuul/zuul] 816690: Store branch cache minimum ltimes in layout state https://review.opendev.org/c/zuul/zuul/+/816690 | ||
| - [zuul/zuul] 816691: Use branch cache min. ltime when loading a tenant https://review.opendev.org/c/zuul/zuul/+/816691 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 13:26 | |
| - [zuul/zuul] 816686: Refresh branch cache depending on min. ltime https://review.opendev.org/c/zuul/zuul/+/816686 | ||
| - [zuul/zuul] 816687: Add branch cache ltime to trigger events https://review.opendev.org/c/zuul/zuul/+/816687 | ||
| - [zuul/zuul] 816688: Forward cache ltime with tenant reconfig event https://review.opendev.org/c/zuul/zuul/+/816688 | ||
| - [zuul/zuul] 816689: Add source interface for getting the cache ltime https://review.opendev.org/c/zuul/zuul/+/816689 | ||
| - [zuul/zuul] 816690: Store branch cache minimum ltimes in layout state https://review.opendev.org/c/zuul/zuul/+/816690 | ||
| - [zuul/zuul] 816691: Use branch cache min. ltime when loading a tenant https://review.opendev.org/c/zuul/zuul/+/816691 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 13:30 | |
| - [zuul/zuul] 816686: Refresh branch cache depending on min. ltime https://review.opendev.org/c/zuul/zuul/+/816686 | ||
| - [zuul/zuul] 816687: Add branch cache ltime to trigger events https://review.opendev.org/c/zuul/zuul/+/816687 | ||
| - [zuul/zuul] 816688: Forward cache ltime with tenant reconfig event https://review.opendev.org/c/zuul/zuul/+/816688 | ||
| - [zuul/zuul] 816689: Add source interface for getting the cache ltime https://review.opendev.org/c/zuul/zuul/+/816689 | ||
| - [zuul/zuul] 816690: Store branch cache minimum ltimes in layout state https://review.opendev.org/c/zuul/zuul/+/816690 | ||
| - [zuul/zuul] 816691: Use branch cache min. ltime when loading a tenant https://review.opendev.org/c/zuul/zuul/+/816691 | ||
| @jim:acmegating.com | swest: question on https://review.opendev.org/816686 | 14:13 |
| @westphahl:matrix.org | corvus: whoops. yes that should be rlock | 14:14 |
| -@gerrit:opendev.org- Simon Westphahl proposed: | 14:16 | |
| - [zuul/zuul] 816686: Refresh branch cache depending on min. ltime https://review.opendev.org/c/zuul/zuul/+/816686 | ||
| - [zuul/zuul] 816687: Add branch cache ltime to trigger events https://review.opendev.org/c/zuul/zuul/+/816687 | ||
| - [zuul/zuul] 816688: Forward cache ltime with tenant reconfig event https://review.opendev.org/c/zuul/zuul/+/816688 | ||
| - [zuul/zuul] 816689: Add source interface for getting the cache ltime https://review.opendev.org/c/zuul/zuul/+/816689 | ||
| - [zuul/zuul] 816690: Store branch cache minimum ltimes in layout state https://review.opendev.org/c/zuul/zuul/+/816690 | ||
| - [zuul/zuul] 816691: Use branch cache min. ltime when loading a tenant https://review.opendev.org/c/zuul/zuul/+/816691 | ||
| @jim:acmegating.com | awesome; that stack looks good to me :) | 14:26 |
| @jim:acmegating.com | Clark: https://review.opendev.org/816566 is the stack that should allow us to run multi-sched for longer periods | 14:27 |
| -@gerrit:opendev.org- Simon Westphahl proposed: | 14:38 | |
| - [zuul/zuul] 815787: Refresh pipelines in tests when settled https://review.opendev.org/c/zuul/zuul/+/815787 | ||
| - [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | ||
| -@gerrit:opendev.org- Andre Aranha proposed: [zuul/zuul-jobs] 816385: Add fips version of jobs needed for OpenStack https://review.opendev.org/c/zuul/zuul-jobs/+/816385 | 14:56 | |
| @clarkb:matrix.org | corvus cool I'll take a look soon | 14:57 |
| @tobias.henkel:matrix.org | corvus, swest: I've a question regarding to https://review.opendev.org/c/zuul/zuul/+/816687/4/zuul/connection/__init__.py. When we clear the cache and forward that ltime with the event. Don't we need to repopulate the cache for that project as well? | 15:45 |
| @tobias.henkel:matrix.org | otherwise another scheduler might use that exact cache with the cleared project? | 15:45 |
| @tobias.henkel:matrix.org | or do I miss something here? | 15:45 |
| @jim:acmegating.com | tobiash: i think when the scheduler gets around to actually doing the reconfiguration, it will call getProjectBranches which will find the cache is empty and then it will populate the cache; then that new ltime should be used in the layout state to tell the other schedulers to refresh | 15:47 |
| @tobias.henkel:matrix.org | is the reconfig the only time when it should refresh it? | 15:47 |
| @jim:acmegating.com | i think that's the last part of the process; it should also get refreshed right before the getProjectBranches call i mentioned above (so it refreshes in order to get the new empty state) | 15:49 |
| @jim:acmegating.com | but every change (branch add/delete) i think should result in a tenant reconfig | 15:50 |
| @jim:acmegating.com | or, almost every one | 15:50 |
| @tobias.henkel:matrix.org | l | 15:50 |
| @tobias.henkel:matrix.org | * k | 15:50 |
| @jim:acmegating.com | (we don't reconfigure if it's a branch add/delete on an unprotected branch and the tenant excludes unprotected branches) | 15:51 |
| @clarkb:matrix.org | (reviewing that stack is still on my todo list, caught up on my morning things, need to eat something then these reviews are next) | 15:51 |
| @jim:acmegating.com | but i think the cache won't have changed in that case so it's fine | 15:51 |
| @tobias.henkel:matrix.org | ah yeah, I missed that the reconfig itself checks the missing project and repopulates that | 15:51 |
| @jim:acmegating.com | yeah, should be a side effect of the getProjectBranches call at scheduler.py line 1753 | 15:52 |
| @tobias.henkel:matrix.org | I think we could still improve that by updating the branch cache for that repo directly when we clear it, this would probably the workflow easier to understand | 15:53 |
| @tobias.henkel:matrix.org | maybe this also makes it more stable when having especially multi tenant repos | 15:54 |
| @tobias.henkel:matrix.org | ah wait | 15:54 |
| @tobias.henkel:matrix.org | the branch list should probably only be updated after the tenant reconfig probably | 15:54 |
| @tobias.henkel:matrix.org | so I guess it's fine as is | 15:54 |
| @jim:acmegating.com | tobiash: yes, i think so too; but i noticed comments in there about protected branches only being configured after branch creation, and i wondered if we intentionally wanted to leave the cache empty as long as possible? | 15:55 |
| @jim:acmegating.com | (at any rate, i think the current patch series mimics the single-scheduler behavior as close as possible on multi-schedulers) | 15:55 |
| @tobias.henkel:matrix.org | this part is actually handled in the scheduler here: https://review.opendev.org/c/zuul/zuul/+/816687/4/zuul/scheduler.py | 15:56 |
| @tobias.henkel:matrix.org | where it decides based on multiple heuristics if it needs to reconfigure or not | 15:57 |
| @tobias.henkel:matrix.org | anyhow, that's complicated stuff due to the missing webhook events :/ | 15:59 |
| @tobias.henkel:matrix.org | corvus: comment on 816690 | 16:08 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 816208: WIP Use AuthProvider https://review.opendev.org/c/zuul/zuul/+/816208 | 16:55 | |
| @jim:acmegating.com | tobiash: i agree | 16:57 |
| @clarkb:matrix.org | corvus: comments on https://review.opendev.org/c/zuul/zuul/+/816595 I feel like there is a little bit of asymetry there that might make queries return inconsistent results depending on ordering branch updates are applied in | 17:38 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: | 17:38 | |
| - [zuul/zuul] 816566: Use CachedBranchConnection in Gerrit driver https://review.opendev.org/c/zuul/zuul/+/816566 | ||
| - [zuul/zuul] 816567: Use CachedBranchConnection in Pagure driver https://review.opendev.org/c/zuul/zuul/+/816567 | ||
| @clarkb:matrix.org | corvus: this is probably fine with a single scheduler where we can always ensure the code order is right but once we have multiple things could get out of sync? | 17:38 |
| @clarkb:matrix.org | (also this review took me too long sorry, was trying to get a model in my head) | 17:38 |
| @jim:acmegating.com | no worries; responding now | 17:48 |
| @clarkb:matrix.org | corvus: when you're done with that one can you check my question on https://review.opendev.org/c/zuul/zuul/+/816687 (that one is mostly for me to make sure I understand the change properly) | 17:52 |
| @jim:acmegating.com | Clark: okay, novel complete on 816595 | 18:02 |
| @jim:acmegating.com | Clark: and correct on 816687 | 18:03 |
| @clarkb:matrix.org | thanks catching up now | 18:03 |
| @clarkb:matrix.org | corvus: for "two tenants can have the same project in them with different exclude-unprotected-branches settings." that helps me understand some of the complexiy there. Does the connection name ensure we lookup the correct cache for each tenant's use of the project? | 18:06 |
| @jim:acmegating.com | Clark: the branch list is universal, therefore so is the cache. it's the fact that every call to getProjectBranches comes with an "exclude_unprotected" argument, and that argument is generally directly the value of the tenant.exclude-unprotected-branches setting in the tenant config that causes us to provide the right data to each tenant. | 18:07 |
| @clarkb:matrix.org | but isn't that a boolean value? | 18:08 |
| @jim:acmegating.com | yes -- projects don't have different branches for different tenants; they just have whatever branches they have. but some tenants want to see all the branches, and some want to only see the protected ones. so only two possibilities for any given real-world project. | 18:10 |
| @clarkb:matrix.org | oh I see the list of protected branches is a code review system attribute not a project tenant config attribute | 18:10 |
| @jim:acmegating.com | yes! | 18:10 |
| @clarkb:matrix.org | what differs between project config tenant attributes is whether or not they want to see the whole picture or just the protected branches | 18:10 |
| @jim:acmegating.com | exactly | 18:11 |
| @clarkb:matrix.org | and that makes the boolean sufficient | 18:11 |
| @clarkb:matrix.org | ok and I see the code in the connection that sets the data if branches = self._branch_cache.getProjectBranches() returns None which addresses my concern about order. Since any scheduler would get None if that wasn't populated yet and then they would go populate it | 18:13 |
| @clarkb:matrix.org | essentially that works for both one scheduler or many | 18:13 |
| @clarkb:matrix.org | thank you for writing that down, it helped | 18:13 |
| @jim:acmegating.com | np :) | 18:14 |
| @clarkb:matrix.org | corvus: is it safe to approve these as I go? I approved the first two as I went because I was confident those were safe but less sure of the rest of the stack | 18:14 |
| @jim:acmegating.com | yes i think so | 18:14 |
| @clarkb:matrix.org | ok | 18:14 |
| @clarkb:matrix.org | Any concern about the unittest failure on https://review.opendev.org/c/zuul/zuul/+/816689/ ? | 18:20 |
| @clarkb:matrix.org | would https://review.opendev.org/c/zuul/zuul/+/816689/4/zuul/driver/git/gitsource.py mean we never discover new branches on a git source? | 18:22 |
| @clarkb:matrix.org | I put that as an inline comment to keep the discussion in gerrit history | 18:23 |
| @jim:acmegating.com | the test failure looks like something was still running during shutdown, buti can't tell what :/ i doubt it's related, but would be good to figure out. | 18:27 |
| @jim:acmegating.com | Clark: and rplied on 689 | 18:29 |
| @clarkb:matrix.org | oh that would explain it :) thanks approved | 18:32 |
| @clarkb:matrix.org | corvus: tobiash looking at https://review.opendev.org/c/zuul/zuul/+/816691/4/zuul/scheduler.py we load tenant with the min ltimes dict. Do we need to create that dict twice? | 18:39 |
| @clarkb:matrix.org | That is related to the comment on https://review.opendev.org/c/zuul/zuul/+/816690/ | 18:39 |
| @clarkb:matrix.org | Made note of that on 816691 swest ^ fyi | 18:42 |
| @clarkb:matrix.org | also https://review.opendev.org/c/zuul/zuul/+/815787/ is failing unittests and it updates unittests so I'll let that be for now | 18:42 |
| @jim:acmegating.com | Clark: i think in 816691 that's a different method (prime vs doTenantReconfigureEvent) and in the prime method, wi already construct the minltimes right before loadTenant | 19:15 |
| @jim:acmegating.com | oh, i see; there's input to loadTenant and then _reconfigureTenant | 19:18 |
| @jim:acmegating.com | Clark, tobiash, swest: yeah, i'm starting to think that we need to construct the min_ltimes dict both before loadTenant and again after | 19:19 |
| @jim:acmegating.com | in both cases | 19:19 |
| @jim:acmegating.com | (maybe time for a helper function :) | 19:20 |
| @jim:acmegating.com | Clark, tobiash, swest, felixedel: can you take a look at my first comment on https://review.opendev.org/814996 -- it has relevance to the branch cache stack. | 21:19 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 816595: Store connection branch cache in ZK https://review.opendev.org/c/zuul/zuul/+/816595 | 21:47 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: | 22:00 | |
| - [zuul/zuul] 816686: Refresh branch cache depending on min. ltime https://review.opendev.org/c/zuul/zuul/+/816686 | ||
| - [zuul/zuul] 816687: Add branch cache ltime to trigger events https://review.opendev.org/c/zuul/zuul/+/816687 | ||
| - [zuul/zuul] 816688: Forward cache ltime with tenant reconfig event https://review.opendev.org/c/zuul/zuul/+/816688 | ||
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 816689: Add source interface for getting the cache ltime https://review.opendev.org/c/zuul/zuul/+/816689 | 22:00 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: | 22:04 | |
| - [zuul/nodepool] 807464: Add metastatic driver https://review.opendev.org/c/zuul/nodepool/+/807464 | ||
| - [zuul/nodepool] 816757: Azure: add image-id support https://review.opendev.org/c/zuul/nodepool/+/816757 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 816767: Don't clear branch cache https://review.opendev.org/c/zuul/zuul/+/816767 | 23:06 | |
| @jim:acmegating.com | I updated my comment on 814996 and left some thoughts on 816767 | 23:13 |
| @jim:acmegating.com | i think we can still merge the branch cache stack tomorrow once swest checks in on those suggested changes. that will allow us to try running 2 schedulers again. this is all just thinking about the next steps to prep for felixedel's zuul-web changes. | 23:14 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: | 23:42 | |
| - [zuul/nodepool] 816757: Azure: add image-id support https://review.opendev.org/c/zuul/nodepool/+/816757 | ||
| - [zuul/nodepool] 807464: Add metastatic driver https://review.opendev.org/c/zuul/nodepool/+/807464 | ||
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!