Tuesday, 2021-03-02

corvuswas there perhaps a tenant reconfig?00:00
corvus(ie, did we merge a change shortly before that?)00:00
fungifor the zuul tenant?00:00
corvusyeah -- to any project in the tenant (ie, could be an opendev or openstack project)00:00
fungioh, right00:01
corvusi'm wondering if there's some race with the web ui when the scheduler is reconfiguring or something00:01
fungicorvus: 777511 maybe?00:02
fungichanged zuul.d/projects.yaml in openstack/project-config when it merged at 23:24:22 which was right before i looked again00:03
*** harrymichal has quit IRC00:04
fungiso maybe we were in some transient incomplete state or something which the merge of that ended up resetting00:10
*** tosky has quit IRC00:17
corvusyeah.  that sounds plausible, but i don't immediately see the mechanism for that happening.00:18
corvusi don't think i'm going to pull on that thread right now, but i'll keep it in mind as we work through the config-in-zk changes00:18
fungiyep, i'm really not sure how to work out at what point we got into that state, but will keep an eye out for recurrences in case there's some sort of pattern to it00:24
openstackgerritMerged zuul/zuul master: Prune git tags on fetch  https://review.opendev.org/c/zuul/zuul/+/76175600:37
*** hamalq has quit IRC00:41
*** rlandy has quit IRC00:43
*** ikhan has quit IRC00:52
*** ikhan has joined #zuul00:56
*** iurygregory has quit IRC01:07
*** saneax has quit IRC01:16
*** iurygregory has joined #zuul01:30
*** ikhan has quit IRC01:54
*** vishalmanchanda has joined #zuul03:45
*** saneax has joined #zuul04:03
*** Tahvok has quit IRC04:03
*** ykarel|away has joined #zuul04:09
*** Tahvok has joined #zuul04:14
*** ykarel|away is now known as ykarel04:14
*** evrardjp has quit IRC05:33
*** evrardjp has joined #zuul05:33
*** jfoufas1 has joined #zuul05:45
*** piotrowskim has joined #zuul07:02
*** ykarel_ has joined #zuul07:27
*** ykarel has quit IRC07:30
*** ykarel_ is now known as ykarel07:51
*** jcapitao has joined #zuul07:52
openstackgerritDong Zhang proposed zuul/zuul master: Display branch of queue in status page  https://review.opendev.org/c/zuul/zuul/+/77761308:05
*** nils has joined #zuul08:09
*** rpittau|afk is now known as rpittau08:22
*** tosky has joined #zuul08:35
*** harrymichal has joined #zuul08:46
*** hashar has joined #zuul08:56
*** jpena|off is now known as jpena08:57
*** SaifAddin has joined #zuul08:59
openstackgerritDong Zhang proposed zuul/zuul master: Display branch of queue in status page  https://review.opendev.org/c/zuul/zuul/+/77761309:33
*** piotrowskim has quit IRC09:44
*** jangutter has joined #zuul10:41
*** jangutter has quit IRC10:42
*** jangutter has joined #zuul10:43
*** jangutte_ has quit IRC10:44
*** jangutter_ has joined #zuul11:30
*** jangutter has quit IRC11:33
*** saneax has quit IRC12:00
*** jcapitao is now known as jcapitao_lunch12:03
*** saneax has joined #zuul12:20
*** jpena is now known as jpena|lunch12:31
*** rlandy has joined #zuul12:39
*** jcapitao_lunch is now known as jcapitao13:00
*** rlandy has quit IRC13:01
*** rlandy has joined #zuul13:02
*** rlandy is now known as rlandy|mtg13:12
tobiashcorvus: I've posted a question on 673840 (Optionally allow zoned executors to process unzoned jobs)13:27
*** harrymichal has quit IRC13:29
*** jpena|lunch is now known as jpena13:33
*** jangutter has joined #zuul13:33
*** jangutter_ has quit IRC13:37
*** rlandy|mtg is now known as rlandy13:56
corvustobiash: thx will fix14:09
corvustobiash: 2 questions:14:10
corvus1) in circular deps, does the entire bundle run with the same merged zuul configuration of every item of the bundle (ie, if A<->B, do both A and B run with .zuul.yaml from A+B)?14:10
corvus2) global repo state doesn't depend on the circular deps change; when we merge it, will it dtrt with circular deps?14:12
*** jangutter has quit IRC14:16
*** jangutter has joined #zuul14:16
*** fdegir has joined #zuul14:19
*** jangutter_ has joined #zuul14:38
tobiashcorvus: we don't run global repo state yet in prod so I think we should rebase it on top of it. Regarding 1): every item in the bundle will do its own merge but with a stable sorting of the dependencies14:38
tobiashcorvus: so yes I think global repo state could differ between the items based on the target branch state during initial merge14:39
tobiashcorvus: so global repo state only enforces the whole repo state within an item14:40
*** jangutter has quit IRC14:41
tobiashthe question is, do we accept this at first as an improvement with todo to cover cdep or do we want to add cdep handling within global repo state?14:41
tobiashone solution could be to deduplicate the initial merge in a cycle14:42
corvustobiash: that actually doesn't sound that concerning to me.  i agree that if the entire bundle had the same state for required-projects it would be better, but i don't think it's wrong for different items in the bundle to have different states for required projects; as long as they have the same state for all the items in the pipeline.14:42
Open10K8SHi team. How could the commit be merged with not merged depends-on commit? https://review.opendev.org/c/openstack/openstack-helm/+/777983 has depends-on https://review.opendev.org/openstack/openstack-helm-infra/777980.  Any policy changed OR I was wrong till now?14:43
tobiashI think in most cases this will be true but for cycles I think global repo state would be not guaranteed over all those items14:43
corvustobiash: and regarding 1 -- with your answer and another look at the code, it looks like that does what i would expect; i'm happy there.14:44
corvustobiash: hrm, i don't understand the last thing you wrote14:44
tobiashforget my last sentence14:45
corvusok14:45
tobiashcorvus: let me rephrase. I think the initial merge which is done prior to job freezing is done independently for each item within the cycle but with a deterministic ordering of the items14:46
tobiashso the repo state of repos that are not part of the dependency queue can be different between the items14:47
corvustobiash: that makes sense; here is my own attempt to summarize: with cdep we expect every item in a bundle to have the same repo state for all of the projects in the pipeline ahead of it plus the projects in the bundle.  with the global state change, we also fix the required-projects state for every project involved, but the state of required-projects may vary between items.14:48
tobiashcorrect14:48
corvusok.  sounds good to me.  i think we could improve that but i don't think it's "wrong".14:49
tobiashand if we want to improve the last part we would need to put deduplicaton of the merges on top14:49
tobiash++14:49
Open10K8Scorvus: tobiash: anyway, depends-on is working in CI but not freezing now, you mean?14:51
corvustobiash: i was thinking there might be a moderate performance improvement if we send out our "best guess" for what the extra merge jobs will be to get the repo state during the initial merge phase (right now, the change does the "extra" repos after the merge of the primary repos).  our best guess will usually be right (unless there's a config change).  so then the later merge would only need to happen if a14:51
corvusnew project was added.14:51
corvusOpen10K8S: we're discussing changes for new features in review14:51
corvusOpen10K8S: 685354 and 738603 to be specific14:52
tobiashcorvus: how would you generate the best guess?14:52
corvusOpen10K8S: regarding your question, the depends-on link in 777983 is invalid (try following it)14:53
corvustobiash: from the current layout (before we get the potentially updated dynamic layout)14:53
tobiashok, that makes sense14:55
corvustobiash: i suppose that would come with an extra job freeze14:55
corvustobiash: so it might be a trade off with adding a little more cpu up front in order to save some time waiting on merge jobs14:55
corvustobiash: i don't think it's something we have to do now; maybe an optimization to keep in mind if it starts to feel sluggish14:56
tobiashyeah14:57
tobiashatm our bottleneck is scheduler cpu actually :/14:57
corvusyeah, i think there are some things that are going to be more attractive after 5.0 :)14:57
tobiashat some point before 5.0 we might need to dig into optimizing job freezing one more time14:58
tobiashI think we still had an issue that the schema things are slow to instanciate14:58
tobiashbut we need to profile this again14:58
openstackgerritJames E. Blair proposed zuul/zuul master: Optionally allow zoned executors to process unzoned jobs  https://review.opendev.org/c/zuul/zuul/+/67384015:02
corvustristanC: are you interested in reviewing "--validate-tenants" option? https://review.opendev.org/542160  i don't see that you've reviewed it before.  i don't know if that would be useful to you or not but i figured you should know about it.15:06
openstackgerritMatthieu Huin proposed zuul/zuul master: web UI: user login with OpenID Connect  https://review.opendev.org/c/zuul/zuul/+/73408215:07
*** ykarel has quit IRC15:17
tobiashcorvus: commented on 67384015:26
corvustobiash: ah yep.  those tests are going to take a long time to time out :)15:30
openstackgerritJames E. Blair proposed zuul/zuul master: Optionally allow zoned executors to process unzoned jobs  https://review.opendev.org/c/zuul/zuul/+/67384015:30
*** openstackgerrit has quit IRC15:35
*** wuchunyang has joined #zuul16:08
*** wuchunyang has quit IRC16:12
*** openstackgerrit has joined #zuul16:19
openstackgerritMatthieu Huin proposed zuul/zuul master: Add authentication-realm attribute to tenants  https://review.opendev.org/c/zuul/zuul/+/73558616:19
openstackgerritMatthieu Huin proposed zuul/zuul master: web UI: allow a privileged user to dequeue a change  https://review.opendev.org/c/zuul/zuul/+/73485016:19
openstackgerritMatthieu Huin proposed zuul/zuul master: web UI: allow a privileged user to re-enqueue a change  https://review.opendev.org/c/zuul/zuul/+/73677216:19
openstackgerritMatthieu Huin proposed zuul/zuul master: Web UI: allow a privileged user to request autohold  https://review.opendev.org/c/zuul/zuul/+/76811516:20
*** jfoufas1 has quit IRC16:25
tobiashzuul-maint: I'd appreciate a review on a small change that adds support for the yaml merge operator ('<<'): https://review.opendev.org/c/zuul/zuul/+/76353216:33
avasstobiash: now if yaml only supported merging lists as well :)16:40
fungizbr: i'm curious, why are you adding me as a requested reviewer on changes like that? ^ i also pay attention in irc16:49
*** sduthil has quit IRC16:50
*** sduthil has joined #zuul16:51
zbrfungi: because you are one of the few that can made a decision on that, irc channel should not be seen an implied/required part of notifying possible reviewers.16:51
zbror to rephrase, is my way of saying that I do think that this review worth your attention16:52
fungiwe have 13 (last time i counted) zuul maintainers, so "few" is relative16:52
corvusand they were all pinged by tobiash with zuul-maint16:52
corvusand i have confidence that every one of them can make good decisions on a change like that16:53
fungii appreciate that you think so highly of me, but this is basically like when you were previously "assigning" reviews to me (and others) which i also asked you to stop doing, at least until we worked out how to disable that "feature"16:53
corvus(there are centainly changes that suggest reviews from folks with particular areas of expertise, but that isn't one of them)16:53
zbrit is possible to disable add reviewer feature, but i doubt it would send the right signal other contributors16:55
fungiit's not a huge nuisance in my case, because i don't rely on gerrit's default dashboard and i heavily filter its e-mail notifications already, but generally adding reviewers to a change is somewhere between ineffective and annoying depending on the reviewer16:56
zbrapparently there is a very low chance for a review to get noticed unless manual pinging is done either by ird or adding someone as reviewer.16:59
*** sshnaidm is now known as sshnaidm|afk17:00
fungii generally don't notice when people add me as a requested reviewer in gerrit, because i'm already a reviewer on so many changes it gets lost in the noise17:00
zbri am now aware of a guidance against adding reviewers, and I personally find it less intrusive than manual mentions on irc17:01
fungi(in this particular case it was pointed out to me by another reviewer that i had been added)17:01
corvusthat was me; i try to make sure everyone's review comments are addressed before merging a change, and i could not find fungi's review comments, so i asked why he was a reviewer17:02
corvusi found the situation confusing17:02
corvusi asked clarkb the same thing17:03
zbri will keep in mind to not add extra reviewers for zuul project changes i look at, that is something new for me as on tripleo it was required.17:06
fungithanks for understanding. different communities with different review volumes and different team sizes are going to have varied norms17:08
*** saneax has quit IRC17:18
*** rlandy is now known as rlandy|lunch17:23
*** jcapitao has quit IRC17:25
*** rpittau is now known as rpittau|afk17:42
*** rlandy|lunch is now known as rlandy|brb17:58
*** jpena is now known as jpena|off18:02
openstackgerritMerged zuul/zuul master: Add --validate-tenants option to zuul scheduler  https://review.opendev.org/c/zuul/zuul/+/54216018:06
openstackgerritMerged zuul/zuul master: Handle the yaml merge operator  https://review.opendev.org/c/zuul/zuul/+/76353218:06
*** harrymichal has joined #zuul18:18
*** hashar has quit IRC18:21
pabelangerworking on enabling TLS on zookeeper. I'm guessing it is a hard cut-over, or can zookeeper be configured to accept both tls / non-tls?18:26
pabelangerso I can do say nodepool first, then zook18:27
pabelangererr18:27
pabelangerzuul18:27
corvuspabelanger: can do both18:27
pabelangertyty18:28
corvuspabelanger: there are separate lines in the zoo.cfg file for each, and they're on different ports18:29
clarkbpabelanger: in fact we continue to confogure it to allow both because it makes local interaction much simpler18:29
clarkbthe firewall prevents external access via not ssl, but admins can interact with the server easily this way (we may end up changing it though not sure yet)18:29
pabelangergreat idea18:29
corvusyeah, i think that's okay as long as we firewall it down to just the zk servers18:29
corvusdefinitely don't allow non-tls connections from the executors :)18:30
*** rlandy|brb is now known as rlandy18:37
*** hamalq has joined #zuul18:38
*** nils has quit IRC19:07
*** hashar has joined #zuul20:30
*** ikhan has joined #zuul20:50
*** bhagyashri|rover has quit IRC21:06
*** bhagyashris has joined #zuul21:07
ianwcorvus: not sure if you saw https://groups.google.com/g/repo-discuss/c/_oFBJXBLv8s/m/-E9kcxpjAwAJ about the checks api21:16
*** SaifAddin has quit IRC21:17
ianwi was wondering what our thoughts are around that being declared stable v the zuul plugin, which is receiving some work to be configurable and have a rest interface etc.21:17
ianwhttps://sourcegraph.com/github.com/GerritCodeReview/gerrit/-/blob/polygerrit-ui/app/api/checks.ts is probably the most useful reference21:17
ianwit feels like there's a bit of a mis-match there, but not so much it couldn't be papered over.  you've have to know what tenant to query; and integrating the status of running jobs v completed would be ... interesting21:20
corvusianw: unfortunately, i haven't had a chance to really study it :(21:32
tristanCcould we provide a new /change endpoint that provides running and completed build for a given (change, patchset) tuple?21:32
tristanCcreate* a new /change endpoint21:33
ianwtristanC: yeah, otherwise running builds would seem tohave to query the status.json21:35
corvustristanC, ianw: i suspect we're going to need a new endpoint, probably provided by the gerrit driver, to return data specifically for this21:35
corvuseither that, or encode a lot of mapping logic in js, right?21:36
corvusat a high level, we need something that turns buildsets into check runs; that could either be in python code in a new gerrit-zuul-driver endpoint, or in java/js in the gerrit zuul plugin?21:37
corvusthe tenant mapping is the thing i'd be most concerned about21:38
tristanCcorvus: or perhaps we could re-use the /status/change/ endpoint to also return completed build21:39
corvustristanC: yeah could be an option21:39
ianwyeah, i was thinking that would probably require something like what is being written for the zuul-summary plugin, where you set a config option for projects giving their URL/tenant21:39
corvusianw: yeah i think that might be the way21:39
corvusianw: maybe a list for that value21:39
corvusie project -> [url/tenant, url/tenant]21:39
tristanCcorvus: instead of a list, could the url either be whitelabel or global, and for later we could return build for all tenants?21:40
corvusexample use case: plugins/zuul may want to have checks from one tenant on ci.gerritcodereview.com and two tenants on opendev.org21:40
corvustristanC: i don't think so -- tenants are meant to be completely separate installations and data shouldn't bleed between them21:41
ianw(https://gerrit-review.googlesource.com/c/plugins/zuul-results-summary/+/298465 is what i'm referring to for reference)21:41
ianwanyway, seeing as there was a offer to help build the front-end on the list, i can take an action item to follow up on that in a thread if we'd like21:42
tristanCcorvus: but if you have a global /api access, then that's just a convenience over querying each tenant scoped endpoint21:42
corvustristanC: there are [almost] no global /api endpoints21:43
corvustristanC: i think you're suggesting we implement a new global api endpoint in zuul to perform the convience of repeating the query on each tenant?21:44
tristanCcorvus: e.g. https://zuul.opendev.org/api21:44
corvusright, almost everything there is tenant-specific21:45
openstackgerritlotorev vitaly proposed zuul/zuul master: Fix artifacts example in documentation  https://review.opendev.org/c/zuul/zuul/+/77823821:45
corvustristanC: i think we should stick with our "tenants are separate" model here.  i think the zuul plugin is going to need to handle multiple installations regardless (opendev's zuul and gerrit's zuul are two separate systems which will want to report on the same project) so configuring another tenant should be about the same21:46
corvusianw: have you seen any suggestion about how systems are supposed to decide to run jobs?21:47
corvus(previously, gerrit was responsible for creating check run instances and you could query the api to find check runs that had not started yet)21:47
tristanCcorvus: alright, i guess we'll have to manually configure all the tenant which run jobs for a given gerrit21:48
corvustristanC: if we wanted to have the checks plugin iterate over all the tenants, that might be fine.  i'm just really wary of publishing a zuul api model that rolls up tenants21:51
tristanCcorvus: that makes sense, but the list can be long then21:53
corvuswe can do both; have the checks plugin iterate if you give it a global url, and if you don't want it to, give it a tenant url21:54
corvus"Copyright (C) 2020 The Android Open Source Settings"  looks like a search/replace mixup :)21:54
corvusianw: is it possible the backend side of this is staying the same?21:56
corvusif they are keeping that, then that might help with the tenant mapping21:58
corvusi think i need a summary of how the whole system (not just the ui layer) is expected to wokr21:58
corvuswork21:58
ianwcorvus: hrm, i don't think this is responsible for triggering jobs?  it's only querying?22:03
corvus"Gerrit requests check runs and results from the plugin by change number and patchset number." makes me think no.  which means the conversation we just had is relevant.  but it also makes me wonder how they're going to handle triggering jobs.22:03
corvusianw: yeah22:03
corvusso if we wanted to use this on opendev, we use stream-events to trigger jobs, then query some api by change/patchset to get check runs (buildsets or builds) and results.22:05
corvuson gerrit's zuul we use (???) to trigger jobs.  :)22:05
*** ikhan has quit IRC22:05
corvusregarding builds/buildsets -- we could consider mapping a checkrun to either.  the main reason we could only do buildsets in the orig checks api was because they had to be defined in advance, but since we can return them after zuul calculates the jobs, we could return builds.  if we use builds, the organizing them (by pipeline) is going to be tricky.  if we do that, we may have to prefix each build name with22:08
corvusthe pipeline name.22:08
*** vishalmanchanda has quit IRC22:54
*** harrymichal has quit IRC23:01
*** hashar has quit IRC23:10
*** ikhan has joined #zuul23:22

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