| *** threestrands has joined #zuul | 00:03 | |
| armstrongs | yeah lines up with https://opendev.org/zuul/zuul/commit/0684df0dd191427d000f0cee2e18ccdc07f5f3c8 | 00:03 |
|---|---|---|
| *** mattw4 has quit IRC | 00:05 | |
| corvus | armstrongs, tristanC: the log copying is usually in a trusted post playbook (i assume that's the case in your installation as well). we can/should probably avoid using the vars blacklist in the trusted context. | 00:06 |
| armstrongs | yeah its in post playbook | 00:07 |
| armstrongs | and its trusted as part of the config jobs | 00:08 |
| *** zxiiro has joined #zuul | 00:14 | |
| *** Defolos has quit IRC | 00:19 | |
| openstackgerrit | Tristan Cacqueray proposed zuul/zuul master: executor: do not blacklist host-vars for trusted context https://review.opendev.org/710890 | 00:24 |
| tristanC | corvus: good idea, here is an implementation ^ | 00:24 |
| armstrongs | thats awesome thank you | 00:33 |
| *** erbarr has quit IRC | 01:09 | |
| *** yolanda has quit IRC | 01:15 | |
| armstrongs | just wanted to say the github checks integration looks brilliant in the new version of zuul | 01:30 |
| openstackgerrit | Merged zuul/zuul master: executor: do not blacklist host-vars for trusted context https://review.opendev.org/710890 | 01:39 |
| *** armstrongs has quit IRC | 01:42 | |
| *** jamesmcarthur has joined #zuul | 01:45 | |
| *** jamesmcarthur has quit IRC | 02:13 | |
| *** jamesmcarthur has joined #zuul | 02:21 | |
| *** Goneri has quit IRC | 02:41 | |
| *** jamesmcarthur has quit IRC | 02:50 | |
| *** jamesmcarthur has joined #zuul | 02:53 | |
| *** jamesmcarthur has quit IRC | 02:58 | |
| *** jamesmcarthur has joined #zuul | 03:04 | |
| *** rlandy has quit IRC | 03:15 | |
| *** jamesmcarthur has quit IRC | 03:50 | |
| *** raukadah is now known as chandankumar | 04:04 | |
| *** sgw has joined #zuul | 04:38 | |
| *** felixedel has joined #zuul | 04:51 | |
| *** evrardjp has quit IRC | 05:35 | |
| *** evrardjp has joined #zuul | 05:35 | |
| *** saneax has joined #zuul | 05:35 | |
| *** igordc has joined #zuul | 06:09 | |
| *** igordc has quit IRC | 06:13 | |
| *** toabctl has quit IRC | 06:18 | |
| *** toabctl has joined #zuul | 06:22 | |
| *** sanjayu_ has joined #zuul | 06:29 | |
| *** saneax has quit IRC | 06:31 | |
| *** felixedel has joined #zuul | 06:31 | |
| *** sanjayu__ has joined #zuul | 06:53 | |
| *** sanjayu_ has quit IRC | 06:55 | |
| *** yolanda has joined #zuul | 07:21 | |
| *** armstrongs has joined #zuul | 08:10 | |
| *** jcapitao has joined #zuul | 08:10 | |
| *** jcapitao has quit IRC | 08:11 | |
| *** jcapitao has joined #zuul | 08:11 | |
| *** armstrongs has quit IRC | 08:19 | |
| *** tosky has joined #zuul | 08:20 | |
| *** Defolos has joined #zuul | 08:28 | |
| *** sanjayu_ has joined #zuul | 08:30 | |
| *** sanjayu__ has quit IRC | 08:32 | |
| *** jpena|off is now known as jpena | 08:41 | |
| *** sshnaidm|pto has quit IRC | 08:45 | |
| *** avass has joined #zuul | 08:54 | |
| *** sshnaidm|pto has joined #zuul | 08:59 | |
| *** threestrands has quit IRC | 09:09 | |
| *** hashar has joined #zuul | 09:14 | |
| *** sshnaidm|pto is now known as sshnaidm | 09:17 | |
| *** sugaar has joined #zuul | 09:31 | |
| *** zxiiro has quit IRC | 09:43 | |
| webknjaz | Is there any way to get some Zuul resources for open source projects? | 10:24 |
| *** masterpe has quit IRC | 10:56 | |
| *** openstackstatus has quit IRC | 10:59 | |
| *** sshnaidm has quit IRC | 11:07 | |
| *** armstrongs has joined #zuul | 11:10 | |
| *** sshnaidm has joined #zuul | 11:12 | |
| *** sshnaidm has quit IRC | 11:13 | |
| *** sshnaidm has joined #zuul | 11:13 | |
| *** felixedel has quit IRC | 11:19 | |
| *** masterpe has joined #zuul | 11:19 | |
| *** hashar has quit IRC | 11:21 | |
| *** hashar has joined #zuul | 11:22 | |
| *** felixedel has joined #zuul | 11:30 | |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Cache getUser in Github connection https://review.opendev.org/710985 | 11:56 |
| *** hashar has quit IRC | 11:57 | |
| *** armstrongs has quit IRC | 11:57 | |
| *** jcapitao is now known as jcapitao_lunch | 12:14 | |
| *** rlandy has joined #zuul | 12:24 | |
| *** jpena is now known as jpena|lunch | 12:32 | |
| openstackgerrit | Benedikt Löffler proposed zuul/zuul master: Fix override variables in zuul_return https://review.opendev.org/711002 | 13:13 |
| *** harrymichal has joined #zuul | 13:16 | |
| *** bschanzel has joined #zuul | 13:21 | |
| *** jpena|lunch is now known as jpena | 13:32 | |
| *** harrymichal has quit IRC | 13:32 | |
| bschanzel | Hi tristanC, for nodepool you implemented the kubernetes/openshift/openshiftpods drivers. I've got a few questions regarding them (the "pod that behaves like a machine" workflow). The kubernetes and openshift drivers create a new namespace for every node request (and a single pod therein). The openshiftpods provider, however, reuses a single | 13:33 |
| bschanzel | namespace for all the node/pods it creates. Isn't this a behaviour we want to have in the kubernetes+openshift dirvers as well? This would eliminate the namespace creation overhead (don't know how big that is tho) and would allow for multi-node-builds (so pods can communicate with each other). Another question is regarding using OpenShift as a | 13:33 |
| bschanzel | backend for nodepool. How do you cope with the "random uid" security feature of OpenShift? (Ansible does not like this, as it can not infer it's ansible_user var). | 13:33 |
| *** harrymichal has joined #zuul | 13:35 | |
| *** harrymichal_ has joined #zuul | 13:36 | |
| tristanC | bschanzel: the namespace creation overhead shouldn't matter. though adding support for multiple pods in a nodeset could be useful for kubernetes+openshift yes | 13:38 |
| tristanC | bschanzel: and for the random uid, i don't know how to satisfy ansible, (beside not using the ansible_user var) | 13:40 |
| *** armstrongs has joined #zuul | 13:40 | |
| tristanC | bschanzel: i think the premise is that multi-pod jobs would request a namespace, and that the pod provisioning would be performed by the job directly | 13:41 |
| tristanC | bschanzel: this is more flexible as you can setup custom resources, for example to configure service and replicaset | 13:42 |
| armstrongs | I was looking at passing custom k8s labels down to zuul nodepool when we provision zuul containers so the containers are spun up on specific hardware. Has anyone looked into this yet or had a similar use case? In theory nodepool would just need the k8s labels passed | 13:44 |
| tristanC | armstrongs: i think that is also covered by requesting a namespace and do the provisioning as part of the job | 13:46 |
| tristanC | in other word, i'm not sure we should add more feature to the `pod that behaves like a machine` workflow, and if you need custom labels, or multi-pod setup, then you should use the namespace node type and do the configuration as part of the job playbook | 13:48 |
| tristanC | though it may be too complex to manage such things in the job with ansible, then i wouldn't mind adding such feature to the existing `pod that behaves like a machine` workflow | 13:51 |
| tristanC | corvus: Shrews: what do you think about above feedback? | 13:51 |
| bschanzel | tristanC regarding namespace creation: currently, using pod type labels (`pod that behaves like a machine`) do not support multi-node-builds because of different namespaces. You can spin up multiple pods, but Ansible then only plays against the first one. Not sure if this is an Ansible problem, tho. Anyway, since multi-node-builds are a feature of | 14:04 |
| bschanzel | Zuul, we maybe want to look into how we can make this work with pod type labels. Maybe by using a single namespace instead of spawning separate namespaces for each pod/node. | 14:04 |
| tristanC | bschanzel: perhaps there is an issue when using the same label name... could you share the nodeset you are using? | 14:06 |
| bschanzel | tristanC for the random uid, one could also set the correct values into `/etc/passwd` in a pre-task and then gather facts again for Ansible to correctly set its vars. Much like the official solution to the problem (https://docs.openshift.com/container-platform/3.11/creating_images/guidelines.html#openshift-specific-guidelines). There it is | 14:09 |
| bschanzel | suggested to have a custom entrypoint in place which sets the `/etc/passwd` entry. The entrypoint for the kubernetes/openshift pods in the corresponding drivers are hard-coded to `/bin/bash` so this approach wont work. While it makes sense to set an appropriate entrypoint in the driver, we might want to consider setting it to `sh` instead of | 14:09 |
| bschanzel | `bash`, since `bash` might not be available in a container? | 14:09 |
| fungi | just need to make sure they don't contain any bashisms (or ideally any non-posix bourne code) | 14:11 |
| bschanzel | tristanC for the multi-node-build problems: Could be you're right. I'll reevaluate that and provide you with the details. | 14:11 |
| corvus | bschanzel, tristanC: note that the recommendations have changed in openshift 4; crio doesn't require a custom entrypoint to modify /etc/passwd: https://docs.openshift.com/container-platform/4.3/openshift_images/create-images.html#images-create-guide-openshift_create-images | 14:21 |
| *** Goneri has joined #zuul | 14:21 | |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Report aborted changes via Github checks API https://review.opendev.org/711023 | 14:21 |
| bschanzel | fungi not really at this place. The entrypoint is hard coded to `/bin/bash -c 'while true; sleep ...'` so the pod won't die immediately but lives until killed by nodepool. `/bin/bash` can easily be replaced by `sh` here so don't require a container image to provide bash. | 14:22 |
| fungi | ahh, so it's not appearing in a shebang | 14:23 |
| corvus | yeah, that does seem safer | 14:23 |
| mnaser | webknjaz: we (at vexxhost) offer a hosted zuul service and we do make it available for open source projects, feel free to reach out over IRC or mnaser@vexxhost.com and I can try and see what to do for you :) | 14:23 |
| fungi | i concur, should just be able to switch that over easilt | 14:23 |
| fungi | easily | 14:23 |
| fungi | webknjaz: also if you host your open source project in opendev, it has a zuul deployment integrated with its gerrit code review system | 14:24 |
| bschanzel | corvus: regarding newer OKD versions: very valuable input! Thank you! | 14:24 |
| webknjaz | fungi: mnaser: Thanks for the info! I was thinking that it may make sense to explore this for CherryPy. And maybe suggest better CI coverage for pip. | 14:25 |
| mnaser | webknjaz: there's many options :) but on my side we're totally ok with you using whatever code hosting platform you want to use | 14:26 |
| mnaser | but, happy to support cherrypy and getting zuul coverage for it | 14:27 |
| fungi | yeah, that would be really cool if cherrypy started using zuul | 14:27 |
| webknjaz | yeah, I figured | 14:27 |
| fungi | (regardless of which zuul deployment it uses) | 14:27 |
| webknjaz | I thought it'd be cool to have multiple CIs if possible | 14:28 |
| mnaser | i think it's interesting to keep in mind that the power of zuul comes from the gating it does too | 14:28 |
| mnaser | so it's one thing to have it report to changes and do CI, it's another (with a whole slew of benefits) for it to do the gating of the project | 14:29 |
| webknjaz | Well, gating sounds nice but probably won't work well atm because of the limited maintainer resources | 14:29 |
| mnaser | webknjaz: if anything gating helps with limited maintainer resources imho :p | 14:30 |
| webknjaz | it does but it also blocks processes | 14:30 |
| webknjaz | it'd be much more useful to have human beings helping to debug issues and figuring out complicated fixes | 14:31 |
| mnaser | right, but all that is totally still possible. it's just the merging of the code ends up being done by zuul, not a human :) | 14:31 |
| webknjaz | currently, we don't do that because it's just two maintainers having limited amount of time and not doing many concurrent merges | 14:33 |
| corvus | webknjaz: the zuul project itself has limited maintainer resources, and having gating very much helps with that :) | 14:33 |
| fungi | it sometimes takes a bit of a leap of faith before you come to realize that humans are usually a poor judge of when a change is broken | 14:33 |
| fungi | and that the assessment of whether a change is broken is inherently racy | 14:34 |
| fungi | (just because tests passed when the change was written doesn't mean it's still passing them when it's approved/merged) | 14:34 |
| webknjaz | I'm not arguing with that, I like the ideas of bors-ng too | 14:35 |
| fungi | but yeah, sometimes it takes unpleasant experiences to come around to the idea of enforcing retesting at time of change approval | 14:37 |
| webknjaz | I think for me it's mostly "somebody needs to integrate this" which requires time, plus limited CI resources are the main blockers | 14:39 |
| fungi | i'd personally rather troubleshoot a problem with a ci job which is preventing me from landing a good change than troubleshoot a bug i've merged which could have been caught by my ci jobs if they'd been rerun | 14:39 |
| mnaser | webknjaz: once you integrate and have zuul runs jobs, you can have it gate | 14:39 |
| mnaser | it can be the same exact ci jobs that you'd gate on | 14:39 |
| mordred | webknjaz: the limited CI resources issue is an unfortunate problem for many open source projects :( | 14:39 |
| fungi | the former situation inconveniences me, the latter inconveniences my users | 14:39 |
| *** sgw has quit IRC | 14:45 | |
| mnaser | webknjaz: anyhow, i'd be happy to get you up and running on zuul so if you're up for it :) | 14:46 |
| corvus | tristanC: should we release the blacklist fix? as 3.17.1 or 3.18.0? (the "-d" deprecation landed, but at this point it's just a warning message, so maybe .1 is okay?) | 14:46 |
| webknjaz | @mnaser sure, I'll text you privately to avoid spamming this channel | 14:46 |
| tristanC | corvus: 3.17.1 sounds good | 14:46 |
| corvus | webknjaz: once you have things set up with mnaser, if you have questions about jobs, etc, that make sense in an upstream context, feel free to chat here :) we like user feedback and improving zuul-jobs. | 14:48 |
| mnaser | yeah, given my simplistic view of things, it looks like it'll just be mostly inheriting base tox jobs | 14:49 |
| webknjaz | sure, thanks! | 14:49 |
| webknjaz | yeah, we use tox as a glue for multiple CIs :) | 14:49 |
| mnaser | we have a bunch of super-well-tested tox base jobs so it is likely extremely straight forward :) | 14:50 |
| *** michael-beaver has joined #zuul | 14:53 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: DNM: test depends-on cherrypy https://review.opendev.org/711034 | 14:54 |
| corvus | webknjaz: ^ i'm not sure if we've exercised this before; i thought i'd see if a cross-source dependency test between zuul and cherrypy worked :) | 14:55 |
| webknjaz | I'd first start with cherrypy/cheroot repo, it's cleaner/smaller | 14:58 |
| corvus | well, for this, even a failure is a success :) | 14:59 |
| *** jcapitao_lunch is now known as jcapitao | 15:00 | |
| *** sgw has joined #zuul | 15:07 | |
| corvus | zuul-maint: does this look good for a release? commit 3acc00a30eb967556e7e6484f17e1b9019d51c85 (HEAD -> master, tag: 3.17.1, origin/master, origin/HEAD, gerrit/master, refs/changes/90/710890/1) | 15:08 |
| corvus | oh... | 15:09 |
| corvus | the kubectl socat thing landed too | 15:09 |
| corvus | https://zuul-ci.org/docs/zuul/reference/releasenotes.html#in-development | 15:09 |
| tobiash | yes, was just about to ask about that one | 15:09 |
| corvus | that's starting to look more like a 3.18.0 isn't it? | 15:09 |
| tobiash | and file comments for github checks also landed | 15:09 |
| fungi | given that, 3.18.0 at 3acc00a sounds reasonable | 15:10 |
| tobiash | corvus: I think that makes sense | 15:10 |
| Shrews | corvus: does this one contain the change that requires a nodepool release? | 15:10 |
| corvus | and i think this may warrant a full opendev restart | 15:10 |
| corvus | Shrews: yep | 15:10 |
| corvus | Shrews: so maybe that should exist first too, eh? | 15:10 |
| Shrews | :) | 15:11 |
| armstrongs | the required tag version wasn't on nodepool yet btw | 15:11 |
| tobiash | oh, the kubectl requires a nodepool release | 15:11 |
| corvus | armstrongs: yeah, we need to release that first | 15:11 |
| armstrongs | :) | 15:11 |
| armstrongs | have been waiting on this one excited to test the streaming | 15:11 |
| tobiash | and we'd need an upgrade notice that kubectl users need to upgrade nodepool first | 15:12 |
| armstrongs | i need to get out more | 15:12 |
| corvus | tobiash: yes, that's in the release notes. but we also did tone it down a bit -- it will not hard-fail if it's wrong; it will only emit a warning to the console + log | 15:12 |
| Shrews | tobiash: i don't think they *need* to, only if they want streaming. should still work as-is, i think, but corvus can confirm | 15:12 |
| Shrews | yeah, that | 15:12 |
| tobiash | ah ok | 15:13 |
| corvus | we don't exercise this code path in opendev. i have tested it manually, but does anyone else want to confirm nodepool HEAD before we release it? | 15:13 |
| *** sanjayu_ has quit IRC | 15:14 | |
| corvus | otherwise, i'll plan to restart opendev zuul (but not nodepool), then tag nodepool and then zuul | 15:14 |
| tobiash | we're behind a few changes for nodepool | 15:15 |
| Shrews | yeah, i think clarkb restarted nodepool the other day for that bug you found and I fixed | 15:15 |
| corvus | tobiash: you want to try it out locally, or should i just go for it? | 15:17 |
| tobiash | corvus: I don't mind, as we're following master anyway, so if you'd like my feedback I can update it and give feedback tomorrow probably | 15:18 |
| *** chandankumar is now known as raukadah | 15:18 | |
| tobiash | I think the only real life risk would be https://review.opendev.org/710343 | 15:19 |
| tobiash | but that doesn't look suspicious | 15:19 |
| tobiash | so I'd be good with just releasing it | 15:19 |
| corvus | i think opendev is running that one | 15:20 |
| Shrews | yes, that's the bug i was refering to | 15:20 |
| Shrews | pretty tame change, IMO | 15:20 |
| tobiash | ++ for release | 15:20 |
| corvus | 2020-02-28 00:36:17 UTC Restarted nodepool launcers on nodepool==3.11.1.dev35 # git sha aba9b4e to pick up proper fix for missing node issue that was previously worked around with a restart. | 15:20 |
| corvus | yep | 15:20 |
| corvus | that's been in prod for a few days | 15:20 |
| corvus | okay, i'll get started on the zuul restart, then start tagging | 15:21 |
| *** jamesmcarthur has joined #zuul | 15:23 | |
| *** harrymichal_ has quit IRC | 15:23 | |
| *** harrymichal has quit IRC | 15:23 | |
| clarkb | Shrews: I did | 15:35 |
| *** Goneri has quit IRC | 15:36 | |
| *** felixedel has quit IRC | 15:41 | |
| corvus | pushed nodepool 3.12.0 | 15:44 |
| *** jcapitao is now known as jcapitao_afk | 15:49 | |
| *** bschanzel has quit IRC | 15:51 | |
| *** harrymichal has joined #zuul | 16:03 | |
| *** mattw4 has joined #zuul | 16:11 | |
| *** jcapitao_afk is now known as jcapitao | 16:21 | |
| *** jamesmcarthur has quit IRC | 16:23 | |
| *** jamesmcarthur has joined #zuul | 16:23 | |
| *** jamesmcarthur has quit IRC | 16:30 | |
| *** jpena is now known as jpena|brb | 16:46 | |
| corvus | nodepool 3.12.0 is up, and opendev's zuul seems to be functioning and merging changes and stuff, so i'll go ahead and tag zuul | 16:46 |
| corvus | commit 3acc00a30eb967556e7e6484f17e1b9019d51c85 (HEAD -> master, tag: 3.18.0, origin/master, origin/HEAD, gerrit/master, refs/changes/90/710890/1) | 16:46 |
| mnaser | ok cool | 16:46 |
| mnaser | so i have simple mostly passing jobs for basic linting stuff for cheroot :) | 16:47 |
| corvus | mnaser: nice! | 16:47 |
| mnaser | https://github.com/cherrypy/cheroot/pull/267/checks?check_run_id=482794547 | 16:47 |
| mnaser | now i've got a tricky issue with ensure-tox installing inside py2.7 < | 16:47 |
| mnaser | :< | 16:47 |
| mnaser | going back to that fun thing | 16:47 |
| clarkb | corvus: this picks up the ansible args modification for trusted playbooks? | 16:48 |
| corvus | clarkb: yes -- that is to say that we do not blacklist the ssh args for trusted playbooks | 16:48 |
| mnaser | so as of now, ensure-tox defaults to installing via py2.7 instead of py3.x | 16:48 |
| mnaser | https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/ensure-tox/tasks/main.yaml | 16:48 |
| clarkb | corvus: I confirm that commit hash matches HEAD for me and 3.18.0 looks like a good number | 16:49 |
| mnaser | so i wonder if both py2 and py3 are installed on the system, we should maybe default to py3 first instead of py2? | 16:49 |
| clarkb | mnaser: at this point it is probably safe to flip the order of those if checks. | 16:50 |
| clarkb | (since people should in general be moving to python3) The concern will be that any tox targets that don't specify a python version will default to the version tox is installed under | 16:50 |
| mnaser | yeah that's what i'm thinking too | 16:50 |
| clarkb | we may want to announce that change then merge it in a week or two as a result | 16:50 |
| corvus | maybe we should add a backwards compat var? | 16:50 |
| clarkb | corvus: ya that may be a good middle ground too "prefer_python2" default to false | 16:51 |
| corvus | maybe add the var, that can merge immediately, then change the default in 2 weeks? | 16:51 |
| clarkb | ++ | 16:51 |
| mnaser | corvus: i like that more, so it can get cherrypy moving for example | 16:51 |
| corvus | yeah, and it's an escape valve for people who (for some reason i can't imagine but i'm sure exists and we would discover if we did this) can't switch right now | 16:52 |
| corvus | (and knowing opendev, that reason is probably lurking in there somewhere :) | 16:52 |
| corvus | apparently ensure-tox is broken on 3 platforms: https://review.opendev.org/707238 | 16:53 |
| clarkb | well we currently preinstall tox in opendev and the spec I wrote to potentially stop doing that should be ok with mnaser's planned behavior | 16:53 |
| *** jamesmcarthur has joined #zuul | 16:54 | |
| *** felixedel has joined #zuul | 16:55 | |
| *** felixedel has quit IRC | 16:55 | |
| *** harrymichal has quit IRC | 16:55 | |
| clarkb | corvus: I think it is failing beacuse tox_executable is only set when installed there? | 16:56 |
| *** harrymichal has joined #zuul | 16:56 | |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: ensure-tox: add prefer_python2 https://review.opendev.org/711056 | 16:56 |
| clarkb | oh hrm its a defaults set var too | 16:56 |
| corvus | clarkb, mnaser: i think we should straighten out that change and https://review.opendev.org/707439 first | 16:56 |
| mnaser | corvus, clarkb: i'm struggling to come up with a good commit message because people talking loudly around me :) | 16:56 |
| corvus | i know it's a mess, and i don't understand what's going on, but all those errors are related to python2/python3 errors, so it seems like getting the testing worked out first would be really good | 16:57 |
| clarkb | corvus: ++ | 16:57 |
| mnaser | yeah | 16:57 |
| corvus | it looks like the second change fixes centos+debian but breaks suse? | 16:57 |
| mnaser | so it reports that tox is installed at /usr/bin/tox | 16:57 |
| clarkb | it would be helpful if zbr|pto mentioned why `tox` doesn't work on centos/debian | 16:57 |
| clarkb | mnaser: ya then it tries to run `tox` and fails | 16:58 |
| corvus | yeah, i'm curious why we're not just running "tox" | 16:58 |
| corvus | the "tox" role runs "tox" by default | 16:58 |
| mnaser | yeah but it runs python -m tox | 16:58 |
| mnaser | and i wonder if it should run python3 -m tox ? | 16:59 |
| corvus | unless tox_executable is set | 16:59 |
| mnaser | yes but in the failure case it is not set | 16:59 |
| mnaser | opensuse-15 | skipping: Conditional result was False | 16:59 |
| mnaser | and also the erro rmessage - /usr/bin/python: No module named tox | 16:59 |
| mnaser | so i think under opensuse maybe ansible is actually using py2 instead of py3, but tox is installed under py3.. maybe? | 17:00 |
| fungi | i don't think it's ansible-specific | 17:00 |
| fungi | `python` always means python 2.x | 17:00 |
| fungi | so regardless of whether ansible is run via python or python3, when it calls a `python` interpreter that's still going to be python 2.x | 17:01 |
| mnaser | ok so i guess maybe tox is installed inside python3 | 17:02 |
| mnaser | and ansible uses 'python' for opensuse | 17:03 |
| *** jamesmcarthur has quit IRC | 17:03 | |
| *** harrymichal has quit IRC | 17:04 | |
| *** harrymichal has joined #zuul | 17:05 | |
| mnaser | https://github.com/ansible/ansible/blob/308723c3ca1122363e419070f1fa1d76ff5611a9/lib/ansible/executor/interpreter_discovery.py | 17:05 |
| fungi | the role is invoking the (unversioned) `python` interpreter right? | 17:05 |
| corvus | okay, going back to basics on 707238 this is the line that's failing: https://opendev.org/zuul/zuul-jobs/src/branch/master/test-playbooks/ensure-tox.yaml#L21 | 17:05 |
| mnaser | https://github.com/ansible/ansible/blob/ec967bce7629758ba8cd8f1d7bd3b63af0e630ba/lib/ansible/config/base.yml#L1463-L1477 | 17:05 |
| corvus | and that's failing on centos7, 8, and debian stretch because, i guess, those systems default to python2 | 17:06 |
| fungi | looks from corvus's link like it's invoking `python3` instead | 17:06 |
| corvus | we're talking about different "it's" here | 17:06 |
| fungi | tox installation vs tox invocation, right | 17:07 |
| corvus | i'm not sure there's anything wrong with the roles right now, the issue is the test we have for ensure-tox is too naive for all the platforms | 17:07 |
| mnaser | corvus: i agree with that statement, it assumes python3 | 17:07 |
| corvus | so, very specifically, we need to figure this out: inside the test called "Test ensure-tox when tox_executable is already set and tox is installed" what's the right way to invoke tox to verify it's installed? | 17:07 |
| clarkb | corvus: well the current role fails on centos because `tox` isn't found as an executable | 17:07 |
| clarkb | corvus: ++ | 17:08 |
| corvus | clarkb: centos7 is failing on the "python3 -m tox" line https://zuul.opendev.org/t/zuul/build/b377fb84295b4f8fa99df40d79f954b8 | 17:08 |
| mnaser | corvus: wouldn't we just need to test that we should be able to successfully execute tox_executable in that scenario? | 17:08 |
| fungi | and ensure-tox installs tox for python2 if it's available, or via python3 if not, unless overridden | 17:08 |
| clarkb | corvus: not in https://review.opendev.org/#/c/707238/2 I don't think | 17:09 |
| corvus | mnaser: yeah, i think what that test is trying to do is say "lets assume the operator said the way to run tox in my system is with this command" | 17:09 |
| corvus | mnaser: so when i wrote that test, i picked a command that worked for bionic, but not everything else | 17:09 |
| clarkb | oh you are right, where does that come from when the change doesn't set the default tox_executable value and the jobs themselves don't override it? | 17:09 |
| corvus | mnaser: maybe we just need to make that conditional on platform? | 17:10 |
| corvus | it's a *test* failure, not a role failure, so we can make the test adaptive to the platform | 17:10 |
| *** zxiiro has joined #zuul | 17:10 | |
| corvus | clarkb: it's supposed to test the case where, say, you've installed tox on your images in /path/to/venv/tox and you want to specify that's the way to run tox | 17:11 |
| corvus | we could maybe go and make a venv and just do that i guess? | 17:11 |
| mnaser | corvus: that seems reasonable to me | 17:12 |
| clarkb | ya that might be a reasonable appraoch. Ignore what is there. Install tox on the "system" then check we can run it from there with the var overridden | 17:12 |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: WIP tox https://review.opendev.org/711058 | 17:12 |
| corvus | mnaser, clarkb: pseudocode for that ^ | 17:12 |
| corvus | clarkb: i just noticed that centos8 has a different failure there https://zuul.opendev.org/t/zuul/build/f64c9d6c80474536b08dd417c0974a12/console | 17:13 |
| mnaser | corvus: yes, that makes sense | 17:13 |
| mnaser | corvus: i can pick that up and expand it? | 17:14 |
| corvus | mnaser: sure, i'll try to figure out what's up with centos8 | 17:14 |
| corvus | mnaser, clarkb: oh wait | 17:15 |
| corvus | mnaser, clarkb: nm | 17:16 |
| corvus | ok i see the fix for centos8, i'll do that in a separate change | 17:17 |
| *** harrymichal has quit IRC | 17:18 | |
| *** harrymichal has joined #zuul | 17:18 | |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: test-ensure-tox: Ignore errors uninstalling tox the second time https://review.opendev.org/711068 | 17:20 |
| corvus | it kinda looks like pip3 behaves differently on centos8 than elsewhere. that seems strange to me but oh well | 17:21 |
| fungi | different how? | 17:21 |
| *** harrymichal has quit IRC | 17:21 | |
| corvus | like maybe "pip uninstall -y tox" returns non-zero when tox is not installed, whereas it returns 0 in the same conditions on other platforms | 17:22 |
| corvus | but i'm not sure about that, i don't have all the info needed to confirm that | 17:22 |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: ensure-tox: use provided tox_executable for tests https://review.opendev.org/711058 | 17:22 |
| openstackgerrit | Clark Boylan proposed zuul/zuul-jobs master: DO NOT MERGE test base-test with no virtualenv perms modifications https://review.opendev.org/680178 | 17:22 |
| mnaser | corvus: ^ if that passes i guess i can squash | 17:23 |
| corvus | mnaser: yeah, ditto on 711068 | 17:24 |
| mnaser | corvus: time to look into making zuul gate commit squashes so we don't mess each others work =P | 17:24 |
| *** jpena|brb is now known as jpena | 17:25 | |
| corvus | :) | 17:25 |
| mnaser | welp | 17:26 |
| mnaser | didnt run the jobs | 17:26 |
| mnaser | i need to update files in the parent patch | 17:27 |
| mnaser | to include the test playbook | 17:27 |
| mnaser | corvus: i'll just squash my change while i'm at it | 17:27 |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: Run ensure-tox on all platforms https://review.opendev.org/707238 | 17:29 |
| corvus | ill do that too then | 17:29 |
| mnaser | ^ squashed my change and added the test playbooks to files | 17:29 |
| *** armstrongs has quit IRC | 17:29 | |
| mnaser | cool, lunch time here | 17:29 |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: Run ensure-tox on all platforms https://review.opendev.org/707238 | 17:30 |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: ensure-tox: add prefer_python2 https://review.opendev.org/711056 | 17:31 |
| corvus | mnaser: ^ i stacked that on top | 17:31 |
| mnaser | corvus: ah cool, that change worked in the cheroot PR too so i know it works to that extent :) | 17:32 |
| mnaser | it'd be nice if someone can take ownership of driving the "send an email to ML about this change and flip that bit to false by default" | 17:34 |
| mnaser | i cant promise i can keep up with it :p | 17:34 |
| *** evrardjp has quit IRC | 17:35 | |
| *** evrardjp has joined #zuul | 17:35 | |
| *** armstrongs has joined #zuul | 17:35 | |
| armstrongs | For log streaming on containers in zuul 3.18.0 and nodepool 3.12.0 it's pausing on each command for 30 seconds after they execute with | [Zuul] Log Stream did not terminate and it isn't streaming the logs from the container. Is there a specific version of socat that is needed for it to work? | 17:44 |
| armstrongs | im on socat-1.7.3.2-6.fc28.x86_64 | 17:46 |
| corvus | armstrongs: i can't imagine a specific version would be required | 17:46 |
| clarkb | log stream did not terminate is on the remote side stopping the daemon isn't it? | 17:46 |
| clarkb | I think the ansible plugin thing tries to stop it and wait for it before giving up when the play is over | 17:47 |
| armstrongs | i see it happen on every task on the play | 17:47 |
| corvus | yeah, but if there is also no initial output the problem could be earlier | 17:47 |
| armstrongs | so ls and pwd test commands are taking 30 seconds each before moving on | 17:48 |
| clarkb | corvus: meaning maybe it doesn't start so cannot stop? | 17:52 |
| corvus | clarkb: yeah | 17:52 |
| corvus | armstrongs: oh | 17:52 |
| corvus | armstrongs: did you add zuul-console to your base job? | 17:52 |
| armstrongs | no is that a requirement? | 17:53 |
| corvus | armstrongs: yes, i think it is, and i should have included that in the release note, sorry | 17:53 |
| corvus | armstrongs: add the "start-zuul-console" to your base job pre playbook | 17:53 |
| corvus | armstrongs: add the "start-zuul-console" role to your base job pre playbook | 17:53 |
| armstrongs | hehe ok | 17:53 |
| corvus | clarkb: thanks for phrasing it like that :) | 17:54 |
| corvus | we should probably add that to https://opendev.org/zuul/zuul-base-jobs/src/branch/master/playbooks/base/pre.yaml | 17:54 |
| *** jcapitao has quit IRC | 17:58 | |
| *** igordc has joined #zuul | 17:59 | |
| armstrongs | works great now thank you | 17:59 |
| armstrongs | :) | 17:59 |
| *** Defolos has quit IRC | 18:06 | |
| corvus | armstrongs: thanks, and sorry; i'll send a followup note to the announce list | 18:10 |
| mnaser | hmm | 18:12 |
| mnaser | fedora-29 failing | 18:12 |
| mnaser | chmod: cannot access '/root/.local': No such file or directory | 18:12 |
| mnaser | i think we need to rerun update-test-platforms.py too | 18:13 |
| corvus | mnaser: i think that's a red herring | 18:17 |
| corvus | Cannot download 'http://mirror.ord.rax.opendev.org/fedora/updates/29/Everything/x86_64/': Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried. | 18:17 |
| mnaser | oh, i missed that | 18:17 |
| corvus | i think that's the actual error | 18:17 |
| corvus | yeah, the zuul "this is probably the error" thing picked the wrong task :/ | 18:17 |
| mnaser | i guess f29 is gone | 18:17 |
| mnaser | so maybe we can replace that job with centos 8 | 18:17 |
| mnaser | or we already have centos-8 there | 18:18 |
| mnaser | so maybe just drop that platform, i guess | 18:18 |
| corvus | yeah, i'll do that | 18:18 |
| clarkb | ++ to dropping platform. I think the infra team needs to schedule the removal globally | 18:18 |
| corvus | i'll look at the linter error too | 18:18 |
| clarkb | but usually I defer to ianw on that as he tends to manage the fedora images | 18:19 |
| mnaser | corvus: i think the linter error is because i changed the bsae job definition to include the test playbook in 'files' and didnt update that file | 18:19 |
| corvus | oh it's the update-test-platforms | 18:20 |
| mnaser | yep | 18:20 |
| corvus | and yeah, 29 has already been dropped from the script, we just need to drop it from the patch | 18:21 |
| corvus | okay, so both errors are the same problem :) | 18:21 |
| *** jamesmcarthur has joined #zuul | 18:22 | |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: Run ensure-tox on all platforms https://review.opendev.org/707238 | 18:28 |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: ensure-tox: add prefer_python2 https://review.opendev.org/711056 | 18:28 |
| openstackgerrit | Merged zuul/zuul master: Add destructor for SshAgent https://review.opendev.org/709609 | 18:29 |
| *** jpena is now known as jpena|off | 18:41 | |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: ensure-tox: add prefer_python2 https://review.opendev.org/711056 | 19:03 |
| mnaser | AJaeger: addressed your comments, thanks :p | 19:03 |
| *** michael-beaver has quit IRC | 19:13 | |
| *** mattw4 has quit IRC | 19:25 | |
| *** hashar has joined #zuul | 19:42 | |
| *** felixedel has joined #zuul | 19:59 | |
| *** felixedel has quit IRC | 19:59 | |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: ensure-tox: add prefer_python2 https://review.opendev.org/711056 | 20:01 |
| mnaser | ^ is ready for some prime time :> | 20:16 |
| openstackgerrit | Merged zuul/zuul-jobs master: Run ensure-tox on all platforms https://review.opendev.org/707238 | 20:21 |
| *** armstrongs has quit IRC | 20:21 | |
| *** tosky has quit IRC | 20:22 | |
| webknjaz | some feedback | 20:28 |
| webknjaz | re:cheroot is it possible to report zuul results only via Checks API w/o legacy Status API? Also, it'd be nice to have separate checks mapped to separate check runs under check suite, not dumped as a list on one page. | 20:28 |
| webknjaz | also: it's easy to get lost in zuul UI, it'd be great to put the results/logs @ checks pages (per check run) | 20:29 |
| webknjaz | plus it seems like re-run button doesn't work... | 20:30 |
| mnaser | webknjaz: so the re-run was a thing i missed at my side which was not making the check requested a thing to trigger for your pipeline, which has since been fixed now | 20:33 |
| mnaser | webknjaz: i also can remove legacy status api (you can have whichever one you want, we report on both) | 20:33 |
| mnaser | the checks being mapped to different check runs is a good suggestion. tobiash has been doing a bunch of work around that so i'm wondering if he has thoughts on that | 20:34 |
| *** jamesmcarthur has quit IRC | 20:36 | |
| *** jamesmcarthur has joined #zuul | 20:37 | |
| webknjaz | cool | 20:37 |
| *** jamesmcarthur has quit IRC | 20:38 | |
| tobiash | it's absolutely possible to just use checks api and no status api, we switched one of our tenants already to do that | 20:39 |
| tobiash | regarding multiple jobs as different checks, it's on purpose that we do a check run per pipeline due to zuul's working model regarding gating | 20:40 |
| tobiash | we require gate runs | 20:40 |
| tobiash | also having a check run per job will complicate branch protection as it won't be possible to use file filters on jobs in a useful manner | 20:41 |
| tobiash | so we decided on a check run per pipeline (builldset) | 20:41 |
| mordred | tobiash: is that because of the dynamic nature of which jobs are going to be run for a given change? | 20:41 |
| tobiash | mordred: yes, each buildset can have its distinct of jobs that are run | 20:43 |
| tobiash | and required checks are static | 20:43 |
| mordred | nod | 20:43 |
| tobiash | so for gating a check per pipeline is the only thing that makes sense in a generic way | 20:44 |
| mordred | yah | 20:44 |
| mnaser | is this a limitation of the github api that doesnt allow us to d othis? | 20:44 |
| mordred | yeah | 20:44 |
| tobiash | mnaser: it's a difference in the working models | 20:45 |
| mnaser | ok i think i'm following now, the issue is that required checks is something you put into branch protection right? | 20:45 |
| mordred | the gerrit checks api currently has the same limitation - pre-defined "checks" ... the subchecks work corvus has been working on will be the type of thing we'd need | 20:45 |
| mnaser | so if you wanted to enforce "dont merge unless gate passes" -- you cant do that if you define checks as whole collection of jobs? | 20:46 |
| mordred | it's been interesting to watch the development of the use case to explain to people who don't already exist in a speculative zuul world why it's important | 20:46 |
| tobiash | not only the checks api, consider gating. Zuul verifies before enqueue if a change will be mergeable if the gate jobs succeed. But at that time it cannot know which jobs this will be | 20:46 |
| mnaser | because you constantly have to pretty much keep that in sync with your gating jobs? | 20:46 |
| *** Goneri has joined #zuul | 20:47 | |
| mnaser | i wonder if there's any reasonable way we can work around that :\ | 20:47 |
| tobiash | technically we could make jobs reporting an optional feature, but that will prevent gating at all so I'd be opposed to that | 20:47 |
| mnaser | tobiash: could you not technically use regex to workaround this in your pipeline config for example? | 20:48 |
| mnaser | enqueue if checks API "tenant/pipeline/.*" are all ran? | 20:49 |
| mnaser | esp if we model the job names to include the pipeline in the prefix for example | 20:49 |
| tobiash | mnaser: before zuul enqueues a change into the gate it asks github for required checks and ignores those it will set on a successful gate run. However zuul knows about the jobs only after enqueue into the gate so it basically cannot determine if merge requirements will be met before enqueing into the gate. | 20:51 |
| tobiash | further the static nature of requiring checks in github doesn't meet the dynamic job graph in zuul | 20:52 |
| mordred | that's the main key | 20:54 |
| mordred | really checks in the github api, because of their static nature, are much more appropriately mapped to the zuul pipeline - while the individual jobs zuul runs don't have a corresponding structure | 20:55 |
| tobiash | yeah | 20:55 |
| mordred | from a reporting perspective, it woudl be neat if github had a way for something running a check to report a list of "I'm going to do and report on these X things for this instance of this check" | 20:56 |
| tobiash | we initially thought that check suite and check runs would match pipelines and jobs, but actually check suites don't have any meaning regarding workflows | 20:56 |
| mordred | because that way the zuul jobs could be exposed directly | 20:56 |
| mordred | tobiash: yeah - sounds like there is a missing construct | 20:56 |
| tobiash | afaik a check run can report intermediate results so we might be able to improve on that in the future | 20:57 |
| tobiash | or maybe not, have to ask Felix about that ;) | 20:58 |
| tobiash | at least we can link to the running buildset in zuul when it's in progress | 20:59 |
| tobiash | and link to the overall buildset result in zuul-web | 21:00 |
| tobiash | when finished | 21:00 |
| *** jamesmcarthur has joined #zuul | 21:01 | |
| mordred | ++ | 21:01 |
| webknjaz | @tobiash: I think it's easy to workaround on the Zuul side | 21:06 |
| webknjaz | Just report a special check run that becomes green only if gating on Zuul side is green. And make users use just that check run for branch protection. | 21:07 |
| *** igordc has quit IRC | 21:08 | |
| *** igordc has joined #zuul | 21:08 | |
| *** igordc has quit IRC | 21:10 | |
| tobiash | webknjaz: technically that might be possible but I think it will confuse most users | 21:10 |
| mnaser | as webknjaz mentions this, i wonder if maybe we had job-specific checks *and* pipeline speciic checks in github | 21:11 |
| *** igordc has joined #zuul | 21:12 | |
| mnaser | but i'm not super intimiately familiar with the github api | 21:12 |
| *** igordc has quit IRC | 21:12 | |
| mnaser | i dont think it might be _that_ confusing to have both the pipeline and individual jobs even tho imho yes it is a little annoing to an extent | 21:13 |
| mordred | mnaser: the bigger problem is that you have to pre-define in github what your checks are beforehand - but with zuul the list is dynamic. so you'd completely lose dynamic jobs in zuul if you went and pre-defined a check in github for each job | 21:14 |
| mnaser | mordred: please forgive my ignorance here but what do you mean exactly by pre-define .. is it an API call we make to github to create this thing? | 21:15 |
| webknjaz | mnaser: only previously reported checks show up in branch protection | 21:15 |
| webknjaz | but it seems okay to have some special prefixed checks and it's important only during setup | 21:16 |
| mordred | webknjaz: what do you mean "only during setup" | 21:16 |
| webknjaz | when zuul is first added/configured | 21:18 |
| mordred | looking at https://developer.github.com/v3/repos/statuses/ - github status is also missing the concept of non-voting | 21:20 |
| mnaser | mordred: right but in this case we can ignore statuses, because it seems like checks is "the way to go" moving forwards | 21:20 |
| webknjaz | but there could be some special job name like `__gate__` that would be required in the branch protection, others wouldn't matter as long as zuul tracks that right | 21:20 |
| webknjaz | @mordred that's outdated, checks api is the new way to go | 21:21 |
| tobiash | technically the reporting could report both, but if zuul would do that it must be optional (per pipeline or even per project) because we have projects with 30+ jobs running on each change which will end up in a huge checks summary at the end which makes the important information (did the pipeline succeed?) hard to spot | 21:21 |
| corvus | tobiash: where is that summary? | 21:22 |
| mordred | webknjaz: yeah - I was reading the checks docs and it linked me to that - so *shurg* | 21:22 |
| tobiash | corvus: I mean that box with the red and green checkmarks at the bottom of a pr | 21:22 |
| webknjaz | red checks are displayed on top | 21:22 |
| corvus | ok lemme sign in because that doesn't show up for anonymous | 21:22 |
| mnaser | i think it may not show up for merged stuff | 21:23 |
| corvus | yeah i don't see it at https://github.com/cherrypy/cheroot/pull/267 | 21:23 |
| mnaser | corvus: https://github.com/cherrypy/cheroot/pull/262 has it | 21:23 |
| mnaser | scroll to end of page and you'll see that long list | 21:24 |
| corvus | i see that now, thx | 21:24 |
| mnaser | and you can kinda see how it pretty much explodes the output of https://github.com/cherrypy/cheroot/pull/262/checks | 21:24 |
| corvus | yeah, "test suite" is a good example here | 21:24 |
| fungi | and as long as you're logged in, yep | 21:25 |
| fungi | it did not show up for me without authenticating the webui | 21:26 |
| tobiash | yes, weird | 21:26 |
| mnaser | tobiash: i wonder if even tho there are failing checks, the checks list would still report like "everything is passing" there | 21:26 |
| mnaser | i mean, failing "non-voting" checks | 21:27 |
| fungi | github, like most social networking platforms these days, seems to assume everyone keeps their browsers logged into services all the time | 21:27 |
| tobiash | mnaser:it probably will say some checks are not successful, but not block the merge | 21:28 |
| mnaser | tobiash: ah yes gotcha. i think it could be feasible then with the ability to disable it | 21:28 |
| mnaser | assuming that the zuul team feels like its an ok workaround (and i'll call it a workaronud because it doesnt seem right -- but it seems like a good UX thing) | 21:28 |
| tobiash | the main reason why I'd keep individul jobs reporting options is user support effort, because as soon as a user checks zuul/gate/tpx-py37 as required check it will inhibit all changed from entering the gate | 21:29 |
| tobiash | and I know I'll get a lot of those questions | 21:30 |
| tobiash | because from workflow perspective pipeline only is the correct match | 21:30 |
| mnaser | tobiash: yeah i almost wish there was a field in the checks api that was like "you cannot use this as a required check" | 21:30 |
| tobiash | mnaser: what I'd wish would be 'require a check suite success' and 'allowing non-voting check run' | 21:31 |
| corvus | the approach of reporting both pipeline and job checks seems like it might work, but i'm hesitant to endorse it; it seems like the pitfalls may not exceed the benefits (which is mostly: avoid an extra click to get to this page: https://github.com/cherrypy/cheroot/pull/262/checks?check_run_id=483204693 ) | 21:32 |
| mordred | I don't see any reason _not_ to add the ability to do that as an option - because it is how github is structured - but I agree with tobiash, I think it should be optional - and I kind of lean towards thinking there should need to be an option named "I really know what I'm doing here" to enable it | 21:32 |
| mnaser | yeah whats the point of a check suite if we're not going to allow check runs in ther eboo | 21:32 |
| corvus | mnaser: i can't decode the last part of that sentence | 21:33 |
| mnaser | it's been a long day, let me try again | 21:33 |
| mnaser | what's the point of check suites if they don't really mean anything inside github other than "a group of checks" | 21:33 |
| mordred | and I'd think we'd need to be able to detect the error condition tobiash mentions reliably so that we could return an error like "you marked a job as a required check run which is unpossible please uncheck it to continue" | 21:33 |
| * tobiash likes the "I really know what I'm doing here" option | 21:33 | |
| mnaser | ideally i'd love to imagine mapping check suites => pipeline | 21:34 |
| corvus | mnaser: yeah, that was what we thought we were going to do, and that would work great, but then "branch protection" hit. | 21:34 |
| corvus | if there were some other way to disable merges from anyone other than zuul other than adding the check as a required branch protection, i think it would be feasible | 21:35 |
| corvus | mordred: i'm just not sure all the extra complexity to handle that is worth it | 21:35 |
| mnaser | so for conclusion: "The final conclusion of the check. Can be one of success, failure, neutral, cancelled, timed_out, or action_required. " | 21:35 |
| mnaser | so non-voting things can probably return a value of "neutral" but it sucks that it won't really give much more info | 21:36 |
| mnaser | (this is for the checks api) -- so i mean i guess you can consider a non voting job is "neutral" in zuuls world | 21:36 |
| corvus | mnaser: yeah, that feels like it could be misleading, but plausible; it means we wouldn't be able to use neutral for anything else in the future though. | 21:37 |
| mnaser | yeah | 21:37 |
| corvus | mordred: unlike in gerrit's current checks system, where you actually have to leave the system to find the result of the jobs, at least in github the're on this page: https://github.com/cherrypy/cheroot/pull/262/checks?check_run_id=483204693 | 21:38 |
| pabelanger | you can block merges in github with branch projections (includes admins) and drop write permissions on write (use triage for labels). I've done a POC for that, only issue to solve is git tags and approved pull review | 21:38 |
| pabelanger | but we've been using checks api since friday on zuul.a.c, so far nobody has really complained (or even noticed) | 21:40 |
| corvus | pabelanger: did you drop status too? | 21:40 |
| pabelanger | no | 21:40 |
| pabelanger | we still need that to trigger for gate | 21:40 |
| tobiash | pabelanger: you can trigger with checks as well | 21:40 |
| pabelanger | tobiash: I haven't figured out how yet | 21:40 |
| tobiash | let me dig it up | 21:41 |
| pabelanger | awesome! | 21:41 |
| tobiash | pabelanger: check out https://zuul-ci.org/docs/zuul/reference/drivers/github.html#value-pipeline.trigger.%3Cgithub%20source%3E.event.check_run | 21:42 |
| *** mattw4 has joined #zuul | 21:42 | |
| pabelanger | tobiash: I'm looking for required, like we do with : https://github.com/ansible/project-config/blob/master/zuul.d/pipelines.yaml#L49 | 21:44 |
| pabelanger | I can't seem to figure out how to ensure 'check' jobs finished | 21:44 |
| tobiash | pabelanger: http://paste.openstack.org/show/790277/ | 21:45 |
| pabelanger | which status gives us | 21:45 |
| tobiash | those are the check run triggers we use in our gate | 21:46 |
| tobiash | it reacts on any successful check run as well as on clicking the rerun button in github | 21:46 |
| tobiash | and the requirements should be handled as usual by branch protection | 21:47 |
| corvus | (you could also consider dropping the clean check requirement) | 21:47 |
| pabelanger | is event cached some how? for example, I don't want the job to enqueue into gate, without first applying gate label | 21:47 |
| tobiash | yes, we do that on a few projects as well (only gate is a required status in branch protection) | 21:48 |
| corvus | (almost everyone is happier without clean check unless you are a giant project with 1k repos) | 21:48 |
| tobiash | corvus: it depends, as soon as a project has a constant gate queue of maybe 5-10 clean check is essential for gate performance | 21:49 |
| *** Defolos has joined #zuul | 21:49 | |
| corvus | tobiash: you have a giant project :) | 21:49 |
| tobiash | well that number assumes job runtimes of around an hour which makes gate resets time and resource costly ;) | 21:50 |
| mnaser | as much as I agree on the whole thing about jobs in github, it bothers me a little bit | 21:50 |
| corvus | main thing is, i was suggesting that pabelanger might want to consider not requiring it for zac | 21:50 |
| tobiash | with smaller projects and faster jobs clean-check is indeed not as important | 21:50 |
| * mnaser doesn't want to stray away from the default zuul architecture but wants to keep a nice contributor experience | 21:51 | |
| pabelanger | corvus: I think we want to get there, but we are just starting to gate things for collections. I'd like to try and avoid a much gate resets to start, then maybe move to clean check | 21:51 |
| mnaser | by contributor, i mean project contributor aka zuul user | 21:51 |
| pabelanger | but right now, I think we want to require checks api first, along with label | 21:51 |
| corvus | pabelanger: fair :) | 21:51 |
| pabelanger | and I can't see how to do that without status | 21:51 |
| tobiash | pabelanger: it works exactly like with status actually, just different event types | 21:52 |
| tobiash | pabelanger: what was your non-checks pipeline config? | 21:52 |
| corvus | mnaser: yeah, i'm just saying i'm not convinced that it's a bad experience. yes, if github operated the way we would like it to, it would be better, but we have 100% failure rate in asking github to change how they work to better accomodate us. i worry that the suggested solution of reporting both pipeline and job status as check runs will be more confusing and therefore a worse user experience than "click | 21:53 |
| corvus | on the check to see the jobs" | 21:53 |
| pabelanger | tobiash: https://github.com/ansible/project-config/blob/7af5b7c49170bd7942fa32f2bcbd6baff52bfb0f/zuul.d/pipelines.yaml#L19 was when we only used checks API, but it broken gate due to https://github.com/ansible/project-config/blob/7af5b7c49170bd7942fa32f2bcbd6baff52bfb0f/zuul.d/pipelines.yaml#L50 as value was empty | 21:54 |
| pabelanger | so now, we have both checks api and status for 'check' pipeline right now | 21:55 |
| mnaser | corvus: i wonder if there is a way for zuul to actually report invalid github configurations | 21:55 |
| tobiash | corvus: that 100% failure rate might drop a little bit in the future: https://developer.github.com/enterprise/2.20/v4/object/pullrequest/#reviewdecision :) | 21:56 |
| mnaser | which seems like would allow us to do things the way we want to, while warning/alerting/refusing to work if things aren't setup properly | 21:56 |
| corvus | tobiash: fingers crossed :) | 21:56 |
| tobiash | corvus: that's not public available yet (it made it accidentally into the docs) but it's at least in the works | 21:56 |
| corvus | mnaser: i'm not sure that there is a way to configure github to work the way we want it to | 21:57 |
| mnaser | corvus: oh you're right. unless you can add a check suite to a github repo as a requirement? | 21:58 |
| *** hashar has quit IRC | 21:58 | |
| tobiash | pabelanger: why do you require the status in the pipeline and not in the branch protection? Btw, status requirements work with checks as well (maybe you need to remove the [bot]) | 21:58 |
| corvus | mnaser: yeah; it sounds like pabelanger has been looking at other ways of doing that other than a branch protection requiring a check; if that works out, it may be a possibility | 21:58 |
| mnaser | i mean finding a way to address the user support concerns that tobiash had without him having to talk to his users (and instead by zuul saying "your repo config is bad) | 21:58 |
| corvus | mnaser: we will all have those concerns if pipelines and jobs are intermingled -- we'll have to explain the difference to everyone | 21:59 |
| mnaser | :( | 21:59 |
| pabelanger | tobiash: we do add it into branch protection (check and gate) but we don't want to enqueue jobs into gate, until check is green. | 21:59 |
| pabelanger | that is why we require check status | 22:00 |
| *** dpawlik has quit IRC | 22:00 | |
| tobiash | pabelanger: but zuul automatically verifies that, you don't need to list it in the pipeline config | 22:00 |
| pabelanger | really? | 22:00 |
| pabelanger | when did that happen | 22:00 |
| tobiash | pabelanger: since ages ;) | 22:00 |
| pabelanger | oh | 22:00 |
| pabelanger | okay then, let me try | 22:00 |
| tobiash | that was one of my first changes in the github api almost two years ago ;) | 22:01 |
| corvus | pabelanger: so if you add zuul's check run as a branch protection, then it won't enqueue into a dependent pipeline | 22:01 |
| pabelanger | k, that should be easy enough to test | 22:01 |
| pabelanger | all projects have that setup already | 22:01 |
| corvus | pabelanger, tobiash: iiuc, for a clean-check gating config, you would probably add both things as branch protection? | 22:01 |
| tobiash | pabelanger: and the checks api support also added includes check runs into that consideration | 22:01 |
| corvus | both things = both the check pipeline check, and the gate pipeline check | 22:01 |
| tobiash | corvus: yes | 22:02 |
| pabelanger | yah, we are doing both ATM | 22:02 |
| tobiash | and without clean check requirement, just require gate | 22:02 |
| pabelanger | I'll test that for tomorrow | 22:02 |
| tobiash | and the great thing is this also works with every other app (we use the WIP bot a lot) | 22:02 |
| corvus | so when an item is enqueued into gate, the check pipeline check branch protection will perform the 'clean check' verification, and the gate pipeline check branch protection will automatically be waived by zuul because it knows it will set it, but github will still enforce it. | 22:02 |
| tobiash | requiring WIP in the branch protection also keeps wip PRs from entering the gate | 22:03 |
| corvus | tobiash, pabelanger: we might want to add these to require/reject too, so that people can encode this into zuul | 22:03 |
| corvus | (ie, so someone could have a check pipeline that ignored WIP prs) | 22:03 |
| pabelanger | ++ | 22:04 |
| tobiash | corvus: isn't that already a thing? | 22:06 |
| tobiash | pabelanger: ftr, that implemented the required status checking for gate pipelines: https://review.opendev.org/535680 | 22:07 |
| corvus | tobiash: no, that's the thing pabelanger was originally asking for -- we have https://zuul-ci.org/docs/zuul/reference/drivers/github.html#attr-pipeline.require.%3Cgithub%20source%3E.status but nothing equivalent for check runs | 22:07 |
| tobiash | corvus: we do, github branch protection doesn't differenciate between status and check runs and so does zuul for requirements | 22:08 |
| corvus | tobiash: but only for something that leaves a status | 22:08 |
| corvus | tobiash: you can't say: pipeline.require.github.check_run: wip | 22:09 |
| tobiash | corvus: zuul asks github for status and check runs and puts them into a list (if not we might be missing a corner case) | 22:09 |
| tobiash | corvus: you can say 'pipeline.require.github.status: wip' and it shouldn't make a difference if wip has been reported as check run or status | 22:10 |
| corvus | tobiash: then why didn't https://github.com/ansible/project-config/blob/7af5b7c49170bd7942fa32f2bcbd6baff52bfb0f/zuul.d/pipelines.yaml#L50 work for pabelanger ? | 22:10 |
| tobiash | it could be that the check_run doesn't contain ' [bot]' as the user field | 22:11 |
| tobiash | so the regex might just need a little tweaking | 22:11 |
| corvus | tobiash: is it wise to combine them like that? why not keep them separate in zuul? | 22:12 |
| corvus | given that they are separate in triggers and reporters, i would expect them to be separate in requirements as well | 22:13 |
| tobiash | corvus: we wanted to handle it the same as the branch protection which doesn't know that difference either | 22:13 |
| corvus | tobiash: gotcha | 22:13 |
| corvus | tobiash, pabelanger: this is probably worth a doc update to https://zuul-ci.org/docs/zuul/reference/drivers/github.html#attr-pipeline.require.%3Cgithub%20source%3E.status | 22:13 |
| tobiash | we could change that however, but we thought the way of least surprise is doing the same as branch protection | 22:14 |
| corvus | tobiash, pabelanger: at least to describe the combined status/checks behavior, and if we can get to the bottom of the regex thing too, that would be good | 22:14 |
| tobiash | oh, sure the docs should be fixed | 22:14 |
| pabelanger | I should have more info tomorrow, but sadly I have to run now. Agree, if I can help will share more info | 22:16 |
| corvus | pabelanger: thanks! | 22:17 |
| tobiash | corvus: just double checed, github api returns the app slug as creator of the check run while the old status checks always pretended a user and adding [bot] after the username | 22:20 |
| tobiash | so that's something github cleaned up with check runs | 22:21 |
| tobiash | so removing '[bot]' from that regex or make that optional should do the trick | 22:21 |
| webknjaz | @mnaser: GH shows the merge button if branch protection checks pass but overall status still may occur as red | 22:21 |
| tobiash | like status: "ansible-zuul.*:ansible/check:success" | 22:21 |
| corvus | tobiash: cool, so that might be worth a little note in the doc update | 22:22 |
| tobiash | k, so doc update needs to add check_run and the [bot] oddity | 22:22 |
| corvus | ++ | 22:22 |
| webknjaz | @corvus: that's two extra clicks because you'd still need to look at logs. so reporting both makes a lot of sense on my side | 22:22 |
| tobiash | I'll ask Felix if he is able to do this this week | 22:23 |
| webknjaz | @mnaser: the point of checks suites is to group by GitHub App mostly, which makes sense b/c you can restart groups and also different apps have different privileges | 22:24 |
| *** jamesmcarthur has quit IRC | 22:24 | |
| *** jamesmcarthur has joined #zuul | 22:27 | |
| corvus | webknjaz: hey check this out: https://zuul.opendev.org/t/zuul/build/5beb4033821d410abe09ea9a00305c90/log/job-output.txt#1372 and https://zuul.opendev.org/t/zuul/build/5beb4033821d410abe09ea9a00305c90/log/tox/py35-siblings.txt#147 (see the last line) confirm that we installed that PR | 22:32 |
| corvus | webknjaz: so if we ever need to test a cherrypy PR with zuul, we can do so with something like https://review.opendev.org/711034 :) | 22:33 |
| *** mattw4 has quit IRC | 22:41 | |
| webknjaz | I see | 22:43 |
| corvus | (er, to be clear, i mean with zuul as a consuming project, ie, run zuul's unit test suite with a cherrypy pr to see if it fixes a bug or something) | 22:46 |
| webknjaz | yea, I figured | 22:46 |
| * webknjaz goes to bed | 22:47 | |
| *** mattw4 has joined #zuul | 22:47 | |
| openstackgerrit | James E. Blair proposed zuul/nodepool master: Use explicit provides/requires for container jobs https://review.opendev.org/710115 | 22:48 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Use explicit provides/requires for container jobs https://review.opendev.org/710116 | 22:48 |
| *** avass has quit IRC | 22:49 | |
| *** mattw4 has quit IRC | 22:53 | |
| *** mattw4 has joined #zuul | 23:00 | |
| *** mattw4 has quit IRC | 23:03 | |
| *** jamesmcarthur has quit IRC | 23:06 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Update k8s log streaming release note https://review.opendev.org/711124 | 23:09 |
| corvus | zuul-maint: could we speedy-approve that ^ ? | 23:10 |
| clarkb | looking | 23:11 |
| fungi | done | 23:12 |
| fungi | hopefully nobody else needs to discover that dependency | 23:13 |
| corvus | i managed to not get around to approving the release email yet, so i'm resending it now with that spliced in | 23:14 |
| *** jamesmcarthur has joined #zuul | 23:16 | |
| fungi | convenient | 23:20 |
| *** jamesmcarthur has quit IRC | 23:20 | |
| fungi | looks good! | 23:21 |
| mnaser | Appreciate thoughts on https://review.opendev.org/#/c/711056/ — I can try helping with eventually flipping the switch I’d prefer python 2 to false too eventually | 23:32 |
| *** jamesmcarthur has joined #zuul | 23:32 | |
| clarkb | mnaser: left one documentation comment to make ^ easier | 23:33 |
| mnaser | ah yes | 23:41 |
| *** sgw has quit IRC | 23:42 | |
| openstackgerrit | Mohammed Naser proposed zuul/zuul-jobs master: ensure-tox: add prefer_python2 https://review.opendev.org/711056 | 23:44 |
| *** jamesmcarthur has quit IRC | 23:53 | |
| corvus | mnaser: +2 ; i think maybe leave it for AJaeger to +3 tomorrow since he weighed in earlier? | 23:54 |
| *** sgw has joined #zuul | 23:56 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!