mnaser | right now, i can't 'approve' my own PR, and any other solution such as using comments or labels makes me lack some sort of rbac-iness | 00:01 |
---|---|---|
clarkb | my understanding is that is a github issue | 00:10 |
clarkb | the workarounds you describe workaround the underlying problem | 00:10 |
clarkb | https://github.community/t/do-not-require-owner-approval-if-the-pull-request-is-from-an-owner/369/51 | 00:12 |
clarkb | seems people were still complaining about this as recently as 8 days ago | 00:12 |
mnaser | yeah i found that post too unfortunately | 00:19 |
mnaser | i'm half wondering if i can build a zuul pipeline which is triggered on pr comments | 00:19 |
mnaser | and it takes those comments, checks if user is owner/etc, and then if they are, enqueue into gate.. or 'approve' the change | 00:20 |
*** wuchunyang has joined #zuul | 00:21 | |
mnaser | maybe similar to how ttx did the release stuff for openstack | 00:21 |
*** wuchunyang has quit IRC | 00:26 | |
*** hamalq has quit IRC | 00:26 | |
corvus | mnaser: you should be able to try something out relatively quickly with a pipeline triggered on a comment or label and a zero-node job run on the executor with a secret that does the github lookup and then leaves the approval | 01:01 |
corvus | since it's looking like this issue is going to persist, i think we could also look into expanding the github trigger to allow you to express "label 'approve' left by owner of repo" as a pipeline requirement. that should get the logic entirely into zuul | 01:02 |
*** Goneri has quit IRC | 01:03 | |
corvus | https://zuul-ci.org/docs/zuul/reference/drivers/github.html#attr-pipeline.require.%3Cgithub%20source%3E.review.permission | 01:04 |
mnaser | corvus: yeah -- it does seem like github is probably not changing it's path, working with their enterprise support team have yielded to not much success... | 01:04 |
dmsimard | tristanC: looks ok to me testing from https://api.us-east.open-edge.io:8080/swift/v1/AUTH_e02c11e4e2c24efc98022353c88ab506/zuul_opendev_logs_962/759969/1/check/zuul-build-dashboard-opendev/962c403/npm/html/ | 01:04 |
dmsimard | can't really comment on the javascript bits though :) | 01:04 |
corvus | mnaser: there's something really close already with "review from user with write permission" | 01:04 |
corvus | probably not exactly what's needed here, but that's the kind of pattern | 01:05 |
mnaser | corvus: yeah, the pattern fits perfectly -- if github just fixed this, it'd be super clean | 01:05 |
mnaser | i guess the nice thing about the idea you suggested with a zero node job is the flexibility it gives you | 01:06 |
corvus | tristanC: oh i entered "foo,bar" and got stuff without either foo or bar. | 01:07 |
mnaser | but then you wind up in a bit of a state-machine-y thing where you don't have an easy way of undoing this | 01:07 |
corvus | mnaser: yeah, i think putting it in the driver is the better medium-term fix (and improves UX for all zuul github users) | 01:07 |
mnaser | i guess the logic all lives here | 01:12 |
mnaser | https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/github/githubmodel.py | 01:12 |
mnaser | corvus: i hope github stores which user added which label :( | 01:13 |
mnaser | it doesn't -- so this makes it a little trickier.. | 01:15 |
mnaser | corvus: i wonder if we can leverage something like https://github.com/go-gitea/lgtm or document usage of this | 01:18 |
mnaser | also this interesting one which can run in a zero node thing -- https://github.com/NerdWalletOSS/github-lgtm | 01:23 |
*** armstrongs has joined #zuul | 01:31 | |
*** armstrongs has quit IRC | 01:41 | |
*** zenkuro has quit IRC | 02:03 | |
*** bhavikdbavishi has joined #zuul | 03:10 | |
*** bhavikdbavishi1 has joined #zuul | 03:13 | |
*** bhavikdbavishi has quit IRC | 03:15 | |
*** bhavikdbavishi1 is now known as bhavikdbavishi | 03:15 | |
*** sean-k-mooney has quit IRC | 04:16 | |
*** sean-k-mooney has joined #zuul | 04:16 | |
*** bhavikdbavishi has quit IRC | 04:21 | |
*** bhavikdbavishi has joined #zuul | 04:22 | |
*** evrardjp has quit IRC | 05:33 | |
*** evrardjp has joined #zuul | 05:33 | |
*** wuchunyang has joined #zuul | 05:42 | |
*** openstackgerrit has quit IRC | 05:46 | |
*** bhavikdbavishi1 has joined #zuul | 06:06 | |
*** bhavikdbavishi has quit IRC | 06:07 | |
*** bhavikdbavishi1 is now known as bhavikdbavishi | 06:07 | |
*** saneax has joined #zuul | 06:08 | |
*** jfoufas1 has joined #zuul | 06:54 | |
*** sshnaidm|afk is now known as sshnaidm|rover | 07:00 | |
*** bhavikdbavishi has quit IRC | 07:31 | |
*** openstackgerrit has joined #zuul | 07:47 | |
openstackgerrit | Tobias Henkel proposed zuul/zuul master: Recover from cat job failures during startup https://review.opendev.org/754636 | 07:47 |
*** mach1na has joined #zuul | 07:48 | |
*** rpittau|afk is now known as rpittau | 07:51 | |
*** bhavikdbavishi has joined #zuul | 07:54 | |
*** jcapitao has joined #zuul | 08:02 | |
*** yolanda has joined #zuul | 08:19 | |
*** mach1na has quit IRC | 08:35 | |
*** mach1na has joined #zuul | 08:38 | |
*** hashar has joined #zuul | 08:42 | |
*** jpena|off is now known as jpena | 08:54 | |
*** tosky has joined #zuul | 08:57 | |
*** jfoufas1 has quit IRC | 09:02 | |
*** openstack has quit IRC | 09:21 | |
*** openstack has joined #zuul | 09:24 | |
*** ChanServ sets mode: +o openstack | 09:24 | |
*** logan- has quit IRC | 09:27 | |
*** logan- has joined #zuul | 09:27 | |
*** vorotech has joined #zuul | 10:06 | |
*** wuchunyang has quit IRC | 10:12 | |
*** zenkuro has joined #zuul | 10:21 | |
*** bhavikdbavishi has quit IRC | 10:31 | |
*** jcapitao is now known as jcapitao_lunch | 11:31 | |
*** bhavikdbavishi has joined #zuul | 11:52 | |
*** mach1na has quit IRC | 12:06 | |
*** rlandy has joined #zuul | 12:07 | |
*** vorotech has quit IRC | 12:19 | |
*** jpena is now known as jpena|lunch | 12:25 | |
*** jcapitao_lunch is now known as jcapitao | 12:27 | |
*** bhavikdbavishi1 has joined #zuul | 12:28 | |
*** bhavikdbavishi has quit IRC | 12:30 | |
*** bhavikdbavishi1 is now known as bhavikdbavishi | 12:30 | |
*** rfolco has joined #zuul | 12:34 | |
*** jfoufas1 has joined #zuul | 12:36 | |
*** mach1na has joined #zuul | 12:46 | |
*** hashar has quit IRC | 12:50 | |
*** mach1na has quit IRC | 12:51 | |
*** vorotech has joined #zuul | 12:54 | |
*** hashar has joined #zuul | 12:56 | |
*** mach1na has joined #zuul | 13:02 | |
*** bhavikdbavishi has quit IRC | 13:24 | |
*** sshnaidm|rover has quit IRC | 13:25 | |
*** jpena|lunch is now known as jpena | 13:27 | |
*** sshnaidm|rover has joined #zuul | 13:39 | |
*** sshnaidm|rover is now known as sshnaidm|mtg | 13:44 | |
zbr | I wonder why there is no lookup for tools/test-setup.yaml playbook in addition to test-setup.sh from zuul side, writing a playbook can prove easier in some cases than shell. | 13:45 |
zbr | is that due to some security concerns? | 13:46 |
zbr | fungi: corvus: https://review.opendev.org/#/c/653130/7 please. | 13:52 |
*** jfoufas1 has quit IRC | 14:04 | |
*** nils has joined #zuul | 14:06 | |
fungi | zbr: i don't understand the question... what sort of lookup for tools/test-setup.yaml would you expect and where? | 14:17 |
zbr | now zuul looks for presence of test-setup.sh and executes it | 14:18 |
fungi | or are you suggesting that we could offer to run a tools/test-setup.yaml if it exists in a repository in the same place we currently execute tools/test-setup.sh | 14:18 |
zbr | how if it would also look for a playbook, and if present to run it. | 14:18 |
zbr | exactly, implicit. | 14:18 |
zbr | this would be user friendly as not everyone knows how to alter zuul jobs, but they may know how to write a playbook. | 14:19 |
*** holser has quit IRC | 14:20 | |
zbr | i am asking this because I seen some test-setup.sh files growing and growing and realized they would be much easier to maintain as ansible playbooks instead of shell. | 14:20 |
fungi | with my openstack developer hat on, the tools/test-setup.sh pattern grew out of having common tasks which developers may need to run (usually with root/elevated permissions) on a system to set up the dev environment before running, say, unit tests. in particular it was used to preconfigure databases or temporary filesystems which were used as local resources by some unit tests (rather than mocked) | 14:20 |
*** holser has joined #zuul | 14:20 | |
fungi | i guess replacing that with a playbook would mean that projects need to describe to developers how to get ansible up and running and execute the playbook with it, instead of telling them they need a working shell | 14:21 |
zbr | let me give you one practical example I seen: https://github.com/ansible-community/molecule-vagrant/blob/master/tools/test-setup.sh | 14:21 |
fungi | before tools/test-setup.sh projects all had their own scripts or cut-n-paste commands in readme files and they all differed. we encouraged them to put them into a consistently-named place because then we could also use them in ci jobs | 14:22 |
zbr | writing cross-platform setup shell is..., less than ideal. | 14:22 |
zbr | i am not proposing replacing it, only to *also* look for a playbook and run it if present. | 14:22 |
fungi | yeah, i get the suggestion now. it seems reasonable to me, if the developer community for a particular projects is already assumed to have ansible installed and be intimately familiar with it | 14:23 |
zbr | and I really like the layout, i was happy to see people proposing changes to the file even if they never used zuul and while the purpose of the file was not documented. | 14:23 |
zbr | in fact I think I could be able trick zuul to run the playbook if I change the shebang line ;) ... as long ansible is already present. | 14:24 |
zbr | i personally see a pattern: initial write a bash script, script grows, you make it a playbook, later you make it a zuul role (if it makes sense). | 14:26 |
corvus | zbr: you could add a pre-run playbook to a job which did include_tasks on a yaml file | 14:26 |
zbr | what I want is to make it run this playbook if it exists, without having to alter the jobs definitions. | 14:27 |
zbr | i think i know how to do it but i wanted to check if such a feature would be desirable or not | 14:27 |
corvus | altering a job definition is the only way to make it run. it's a question of which job defn to alter. if you want it to run like test-setup.sh, then it's the "unittests" job that should be altered. | 14:28 |
*** vorotech has quit IRC | 14:30 | |
*** vorotech has joined #zuul | 14:36 | |
*** vorotech has quit IRC | 14:38 | |
mnaser | would it be reasonable for zuul to pick up config projects from `main` as well if there is no other branches at all ? | 14:40 |
*** vorotech has joined #zuul | 14:40 | |
mnaser | i was confused for quite sometime now :) | 14:40 |
*** sanjayu_ has joined #zuul | 14:40 | |
*** saneax has quit IRC | 14:42 | |
*** Goneri has joined #zuul | 14:48 | |
*** Goneri has quit IRC | 14:52 | |
openstackgerrit | Matthieu Huin proposed zuul/zuul master: [DNM] zuul-client: test change-status command https://review.opendev.org/759840 | 14:58 |
fungi | mnaser: yeah, we discussed that earlier in the week or over the weekend, sean-k-mooney ran into a similar challenge | 15:01 |
fungi | it seems reasonable that "main" could be a fallback when zuul can't find "master" | 15:01 |
fungi | i don't think anyone has proposed a change to implement that yet | 15:02 |
fungi | if zuul could identify the default branch and get rid of the hard-coded master assumption that would probably be ideal, but i don't know if it's feasible (or else i assume we'd have already done it that way) | 15:03 |
tobiash | mnaser: currently you can say load-branch: main in the tenant config for those repos, later we can make an automatic fallback so this is not necessary | 15:07 |
mnaser | tobiash: yeah that's what i ended up doing, to add my confusion i had forgotten to enable branch protections too on that branch | 15:08 |
mnaser | so that confused me foor a bi tafter too :p | 15:08 |
tristanC | beside sending SIGUSR2, is there another thing we could do to investigate a scheduler memory leak? | 15:14 |
sean-k-mooney | this is what i do https://github.com/SeanMooney/ci-sean-mooney/blob/main/zuul.d/jobs.yaml#L29-L41 | 15:14 |
sean-k-mooney | as a workaroudn currently | 15:15 |
sean-k-mooney | its only an issue if the repo under test is on branch main | 15:15 |
sean-k-mooney | i guess it could be an issue the other way around | 15:15 |
sean-k-mooney | but we dont have main brances in openstack currently | 15:15 |
sean-k-mooney | but ya automatic fallback would be optimal form a ux point of view | 15:16 |
fungi | tristanC: i have notes somewhere for running the scheduler under repl, i think, which has been useful in the past for exploring objects in the running system, but could lead to performance impact | 15:23 |
mnaser | tobiash: https://zuul-ci.org/docs/zuul/reference/drivers/github.html under "Reference pipelines" -- the `- event: check_run` -- shouldn't we set `action: completed` too, it looks like i'm running a 'loop' right now | 15:26 |
mnaser | sorry, i mean `action: requested` | 15:26 |
mnaser | cause now every time zuul reports to github, it re-enqueues | 15:26 |
tristanC | fungi: i remember reading about using the zuul repl for such things, but can't find what the operator needs to do. I'd be happy to add your note to the troubleshooting doc if you can find them | 15:27 |
fungi | yeah, unfortunately it looks like the etherpad i was thinking of might be corrupt. i'm going to see if i can salvage it via the admin api | 15:28 |
tristanC | ftr here is the memory usage graph: https://prometheus.monitoring.softwarefactory-project.io/grafana/d/EtRMbV1Zk/node-exporter-full?viewPanel=78&orgId=1&var-job=node&var-name=zs.softwarefactory-project.io&var-node=zs.softwarefactory-project.io&var-port=9100&from=now-15d&to=now | 15:29 |
tobiash | mnaser: which pipeline? | 15:42 |
mnaser | tobiash: the 'check' pipeline | 15:42 |
mnaser | shouldn't we include action: requested under the 'check_run' event | 15:42 |
tobiash | probably, let me compare this with our check pipeline | 15:42 |
tobiash | mnaser: yes, we have this: http://paste.openstack.org/show/799479/ | 15:43 |
mnaser | tobiash: do you want to push up a patch to update it (or i can?) | 15:45 |
openstackgerrit | Tobias Henkel proposed zuul/zuul master: Fix reference check pipeline https://review.opendev.org/760170 | 15:45 |
tobiash | mnaser: ^ | 15:46 |
fungi | tristanC: so here's one pad of notes (i think from corvus) about methods for investigating memory leaks in the scheduler: http://paste.openstack.org/show/799481/ | 15:46 |
mnaser | tobiash: awesome, +1 | 15:46 |
tobiash | zuul-maint: small doc fix: https://review.opendev.org/760170 | 15:48 |
tristanC | fungi: thanks! | 15:48 |
fungi | tristanC: aha, for some reason i wasn't able to get the pad for that to load correctly, maybe user error, it's working for me here now https://etherpad.opendev.org/p/zuul-memory-leak | 15:48 |
tobiash | tristanC: what version are you running? | 15:49 |
tobiash | we've recently spent weeks to hunt down and fix a memleak | 15:49 |
tristanC | tobiash: 3.19.1 | 15:49 |
tobiash | that might not include those fixes | 15:49 |
fungi | tristanC: unfortunately that doesn't cover restarting the scheduler with repl loaded | 15:49 |
tobiash | let me check | 15:49 |
fungi | just what to do once you're in it | 15:50 |
tristanC | fungi: yep, got it. i'll look into adding repl usage to the doc too | 15:50 |
tobiash | tristanC: this stack solved our memleak: https://review.opendev.org/751172 (and two parents) | 15:51 |
fungi | awesome, thanks! | 15:51 |
tobiash | tristanC: that's not in 3.19.1 | 15:51 |
tristanC | tobiash: unfortunately... i guess we'll have to wait for the v4 | 15:52 |
tobiash | tristanC: so actually there were two leaks, one when loosing the zk session (so not that severe since zk session loss itself is already severe) | 15:53 |
tristanC | tobiash: thank you for fixing those! | 15:53 |
tobiash | the more important is https://review.opendev.org/751172 | 15:53 |
tobiash | which you can avoid mainly if you don't allow enqueue/dequeue via api since that's the main trigger (wrongly used enqueues or dequeues) | 15:53 |
tobiash | so if you cannot update, don't allow enqueue/dequeue to the users | 15:54 |
tristanC | tobiash: heh i guess we hit both issues, lost zk last week and enabled admin user to enqueues/dequeus :-) | 15:54 |
tobiash | tristanC: what's your zk session timeout? | 15:55 |
tristanC | minSessionTimeout: 600000 and maxSessionTimeout: 1800000 | 15:56 |
tristanC | and last zk outage lasted for more than 30min | 15:57 |
tobiash | k, then just restart zuul as well if you have a real zk outage | 15:57 |
tobiash | we had also without zk problems session timeouts due to thread contention in zuul itself so needed to increase the timeout | 15:58 |
tristanC | yeah, i think the main issue is when restart launcher, in that situation it may takes 30min for the lock to expire | 15:58 |
tobiash | or well, the memleak triggered gc runs > 40s which lead to session losses which lead to more memory leaked... | 15:58 |
tobiash | you can imagine what the result was ;) | 15:59 |
tristanC | in anycase, is there a topic for the patch blocking the v4 release? | 16:00 |
tristanC | and/or, would it be worth doing a v3.19.2 with the leak fix? | 16:01 |
tobiash | hrm, where was that etherpad again with the release plan? | 16:01 |
tobiash | tristanC: I guess 3.19.2 won't happen since 3.19.1 cause way too much trouble | 16:01 |
tobiash | here it is: https://etherpad.opendev.org/p/zuulv4 | 16:02 |
tobiash | so from what I can see this would be missing for a v4: https://review.opendev.org/630472 | 16:03 |
tobiash | and maybe make tls zk mandatory | 16:04 |
tobiash | and the ha scheduler stack up to https://review.opendev.org/716262 | 16:04 |
tristanC | tobiash: i see, thanks! | 16:07 |
tobiash | oh and remove zuul-migrate | 16:07 |
tobiash | I guess that's easy | 16:07 |
openstackgerrit | zbr proposed zuul/zuul-jobs master: Add test_setup_reset_connection setting https://review.opendev.org/653130 | 16:08 |
*** vorotech has quit IRC | 16:23 | |
*** hamalq has joined #zuul | 16:51 | |
*** rpittau is now known as rpittau|afk | 17:09 | |
*** mach1na has quit IRC | 17:09 | |
*** tosky has quit IRC | 17:20 | |
*** mach1na has joined #zuul | 17:20 | |
*** mach1na has quit IRC | 17:25 | |
*** jcapitao has quit IRC | 17:31 | |
*** sshnaidm|mtg is now known as sshnaidm|rover | 17:46 | |
*** mach1na has joined #zuul | 17:54 | |
*** sanjayu_ has quit IRC | 17:57 | |
*** mach1na has quit IRC | 17:59 | |
*** jamesmcarthur has joined #zuul | 18:01 | |
*** jpena is now known as jpena|off | 18:05 | |
*** sugaar has quit IRC | 18:09 | |
openstackgerrit | Matthieu Huin proposed zuul/zuul master: [DNM] zuul-client: test change-status command https://review.opendev.org/759840 | 18:16 |
*** hashar has quit IRC | 18:28 | |
*** jamesmcarthur has quit IRC | 18:29 | |
dmsimard | wow, mediawiki moving off gerrit to gitlab: https://www.mediawiki.org/wiki/GitLab_consultation#Outcome | 18:29 |
clarkb | the move away from gerrit has been long planned for them, the original target was phabricator iirc | 18:30 |
fungi | yeah, presumably they finally gave up on phabricator after moving a bunch of repos to it. will see if they have any better luck with gitlab | 18:41 |
*** sshnaidm_ has joined #zuul | 18:57 | |
*** sshnaidm|rover has quit IRC | 19:00 | |
*** sshnaidm_ is now known as sshnaidm|rover | 19:06 | |
*** ianychoi__ has joined #zuul | 19:17 | |
*** ianychoi_ has quit IRC | 19:21 | |
*** sshnaidm|rover is now known as sshnaidm|afk | 19:28 | |
*** hashar has joined #zuul | 19:39 | |
*** jamesmcarthur has joined #zuul | 19:40 | |
*** mach1na has joined #zuul | 19:55 | |
*** mach1na has quit IRC | 20:00 | |
*** mach1na has joined #zuul | 20:03 | |
*** mach1na has quit IRC | 20:07 | |
*** piotrowskim has quit IRC | 20:23 | |
*** yolanda has quit IRC | 20:24 | |
*** yolanda has joined #zuul | 20:25 | |
*** mach1na has joined #zuul | 20:34 | |
*** nils has quit IRC | 20:36 | |
*** tosky has joined #zuul | 20:39 | |
*** mach1na has quit IRC | 20:39 | |
*** rfolco has quit IRC | 21:06 | |
mnaser | i've got a little 'approval' system going for zuul that's pretty 'zuul-native' so far | 21:08 |
mnaser | now just to double check, i cant install any pip packages on the executor, right? | 21:09 |
mnaser | so i really need to rewrite this to do things like urllib and friends and my pygithub usage is no bueno | 21:09 |
fungi | for a bunch of stuff like that i think we just use the url lookup module in ansible | 21:10 |
mnaser | i assume i cant use shell on the executor (and i cant use my own module either?) | 21:11 |
mnaser | (but are the rules different if it's a config-project job?) | 21:11 |
ianw | mnaser: i think you can do shell in a trusted playbook | 21:13 |
ianw | just it makes it hard to test is all | 21:13 |
mnaser | ok that may be my cop out then | 21:13 |
mnaser | it has to be priv anyways | 21:13 |
ianw | you can get libraries installed, i mean things like the s3 and google cloud uploader need them. i'm not sure if there's a formalised way to do that? | 21:16 |
*** jamesmcarthur has quit IRC | 21:26 | |
fungi | there is, it's a list somewhere for additional modules manage-ansible will install in the ansible venvs | 21:29 |
fungi | should be in the zuul manage-ansible docs, i just don't have time to look right this moment | 21:29 |
*** zenkuro has quit IRC | 21:35 | |
*** zenkuro has joined #zuul | 21:35 | |
*** rfolco has joined #zuul | 21:57 | |
*** ChrisShort has quit IRC | 21:59 | |
*** shanemcd has quit IRC | 21:59 | |
*** zer0c00l has quit IRC | 21:59 | |
*** zer0c00l has joined #zuul | 21:59 | |
*** shanemcd has joined #zuul | 21:59 | |
*** ChrisShort has joined #zuul | 21:59 | |
ianw | ahh yeah ANSIBLE_EXTRA_PACKAGES ; til :) | 22:02 |
*** rfolco has quit IRC | 22:03 | |
*** zenkuro has quit IRC | 22:07 | |
*** zenkuro has joined #zuul | 22:09 | |
*** rfolco has joined #zuul | 22:30 | |
*** rlandy is now known as rlandy|bbl | 22:33 | |
*** rfolco has quit IRC | 22:35 | |
*** mach1na has joined #zuul | 22:42 | |
*** mach1na has quit IRC | 22:47 | |
*** hashar has quit IRC | 23:28 | |
*** holser has quit IRC | 23:58 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!