| *** marios is now known as marios|ruck | 05:04 | |
| felixedel | corvus: Thanks, I will have a look on the comments and I don't mind squashing the changes. | 05:30 |
|---|---|---|
| *** marios|ruck is now known as marios | 07:01 | |
| *** marios is now known as marios|ruck | 07:03 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 07:04 | |
| *** holser is now known as holser_ | 07:21 | |
| *** rpittau|afk is now known as rpittau | 07:29 | |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Store runtime related system attributes together https://review.opendev.org/c/zuul/zuul/+/801749 | 08:23 |
| opendevreview | Simon Westphahl proposed zuul/zuul master: Cache system config in Zookeeper https://review.opendev.org/c/zuul/zuul/+/801859 | 08:23 |
| *** holser_ is now known as holser | 09:33 | |
| *** holser is now known as holser_ | 09:38 | |
| *** jcapitao is now known as jcapitao_lunch | 10:38 | |
| opendevreview | Felix Edel proposed zuul/zuul master: Add MergerApi https://review.opendev.org/c/zuul/zuul/+/787686 | 11:37 |
| *** holser_ is now known as holser | 12:11 | |
| *** jcapitao_lunch is now known as jcapitao | 12:15 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 12:31 | |
| opendevreview | Felix Edel proposed zuul/zuul master: Add MergerApi https://review.opendev.org/c/zuul/zuul/+/787686 | 14:03 |
| opendevreview | Felix Edel proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/802299 | 14:03 |
| felixedel | corvus: I've squashed your patches into 787686 and fixed one or two minor bugs. The sharding of the merge results is not included as this breaks the management event results (see my comment in gerrit). In addition to that I've updated 802299 to work with the new version of the merger API. I forgot to squash the cleanup change, so I will do that tomorrow. | 14:15 |
| corvus | felixedel: great -- i'll take a look, and maybe i'll be ably to do some fixups and squashes during my day | 14:50 |
| *** marios|ruck is now known as marios | 15:37 | |
| *** marios is now known as marios|ruck | 15:39 | |
| corvus | tristanC, tobiash, mordred: i think you have variously reviewed the kopf changes -- many of them have 2x+2s, some only have 1. do you want to do any more reviews of that stack, or should we call it sufficiently reviewed and start merging it? i'd like to merge it mostly as a unit (at least to get close to the end of the stack) so that we end up with something [mostly] functional at the end and aren't stuck in a half-implemented state for long. | 15:51 |
| *** marios|ruck is now known as marios | 15:56 | |
| *** marios is now known as marios|ruck | 15:57 | |
| pabelanger[m] | tobiash: how do you handle stale pull review requests in github? With the migration to zuul 4.0, it is a change in our workflow for users that leave a -1 on the review but then apply gate. Which zuul doesn't equeue due to the pull request review | 16:10 |
| pabelanger[m] | Trying to see if we have a way to clear them on new pull request, kinda like how gerrit does it on a new change | 16:11 |
| pabelanger[m] | we do that too for the gate label via a pipeline noop job but not sure we can do that but looking at https://docs.github.com/en/rest/reference/pulls#dismiss-a-review-for-a-pull-request now | 16:12 |
| *** marios|ruck is now known as marios|out | 16:13 | |
| tristanC | corvus: works for me | 16:15 |
| tristanC | corvus: should I +workflow the base of the stack? | 16:18 |
| pabelanger[m] | tobiash: okay, branch protection under Require pull request reviews before merging has the setting. However, that then means we need to enforce 1 review... which is another problem | 16:18 |
| corvus | tristanC: sounds good, thanks | 16:23 |
| opendevreview | Jeremy Stanley proposed zuul/zuul master: Fix broken link markup for encryption docs https://review.opendev.org/c/zuul/zuul/+/802554 | 16:29 |
| *** sshnaidm is now known as sshnaidm|afk | 16:34 | |
| *** rpittau is now known as rpittau|afk | 16:40 | |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Web UI: Add "Create Autohold Request" form, improve API error messages https://review.opendev.org/c/zuul/zuul/+/802559 | 16:58 |
| mhu1 | okay who wants to test tenant admin features in the UI \o/ https://review.opendev.org/q/topic:%22fffaff%22+(status:open%20OR%20status:merged) | 17:03 |
| mhu1 | https://review.opendev.org/c/zuul/zuul/+/769943 and further patches on top of it let you set up everything you need with a docker compose | 17:03 |
| opendevreview | James E. Blair proposed zuul/zuul master: Shard merger api results https://review.opendev.org/c/zuul/zuul/+/802405 | 17:07 |
| opendevreview | Merged zuul/zuul-operator master: Use kopf operator framework https://review.opendev.org/c/zuul/zuul-operator/+/785039 | 17:13 |
| opendevreview | Merged zuul/zuul-operator master: Bump API version to v1alpha2 https://review.opendev.org/c/zuul/zuul-operator/+/785047 | 17:13 |
| opendevreview | Merged zuul/zuul-operator master: Run a git server in k8s for functional tests https://review.opendev.org/c/zuul/zuul-operator/+/785738 | 17:13 |
| opendevreview | Merged zuul/zuul-operator master: Move ingress to functional test https://review.opendev.org/c/zuul/zuul-operator/+/785757 | 17:13 |
| opendevreview | Merged zuul/zuul-operator master: Support externally managed Zookeeper and DB https://review.opendev.org/c/zuul/zuul-operator/+/785273 | 17:13 |
| opendevreview | Merged zuul/zuul-operator master: Pass through extra scheduler config options https://review.opendev.org/c/zuul/zuul-operator/+/785277 | 17:13 |
| opendevreview | Merged zuul/zuul-operator master: Add merger support https://review.opendev.org/c/zuul/zuul-operator/+/785278 | 17:15 |
| opendevreview | Merged zuul/zuul-operator master: Add keystore password support https://review.opendev.org/c/zuul/zuul-operator/+/790182 | 17:15 |
| opendevreview | Merged zuul/zuul-operator master: Support imagePrefix and versions https://review.opendev.org/c/zuul/zuul-operator/+/785279 | 17:15 |
| opendevreview | Merged zuul/zuul-operator master: Support fingergw https://review.opendev.org/c/zuul/zuul-operator/+/785300 | 17:15 |
| opendevreview | Merged zuul/zuul-operator master: Add docs https://review.opendev.org/c/zuul/zuul-operator/+/785083 | 17:15 |
| opendevreview | James E. Blair proposed zuul/zuul master: Remove some unused methods in sql reporter https://review.opendev.org/c/zuul/zuul/+/800121 | 17:17 |
| opendevreview | Merged zuul/zuul master: Fix broken link markup for encryption docs https://review.opendev.org/c/zuul/zuul/+/802554 | 17:49 |
| opendevreview | James E. Blair proposed zuul/zuul master: Add MergerApi https://review.opendev.org/c/zuul/zuul/+/787686 | 18:36 |
| opendevreview | James E. Blair proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/802299 | 18:37 |
| opendevreview | Merged zuul/zuul master: gitlab: Add access token name, Update docs, Fix webhook https://review.opendev.org/c/zuul/zuul/+/771184 | 18:47 |
| opendevreview | Merged zuul/zuul master: Add job node to components graph. https://review.opendev.org/c/zuul/zuul/+/762767 | 18:57 |
| opendevreview | Matthieu Huin proposed zuul/zuul master: Web UI: Add "Create Autohold Request" form, improve API error messages https://review.opendev.org/c/zuul/zuul/+/802559 | 19:47 |
| opendevreview | Merged zuul/zuul master: Add item UUID to MQTT reporter https://review.opendev.org/c/zuul/zuul/+/797165 | 20:35 |
| clarkb | corvus: couple of thoughts on https://review.opendev.org/c/zuul/zuul/+/793666 but none major enough to -1. I +2'd but havent approved as I want to review more of this stack to see if we need to land stuff to gether. Feel free to approve ahead of me if you like | 21:46 |
| clarkb | though looking at the parent of that change again while it has two +2's (why I skipped it to start) it seems that maybe I hsould review it too | 21:47 |
| opendevreview | James E. Blair proposed zuul/zuul master: Randomze choice of zoned fingergw https://review.opendev.org/c/zuul/zuul/+/802628 | 22:00 |
| corvus | clarkb: ++ ^ | 22:01 |
| opendevreview | James E. Blair proposed zuul/zuul master: Randomze choice of zoned fingergw https://review.opendev.org/c/zuul/zuul/+/802628 | 22:02 |
| clarkb | corvus: ok I reivewed the parentmost change and left a couple of notes. I think the one that matters maybe the debug logging in the test? | 22:15 |
| opendevreview | James E. Blair proposed zuul/zuul master: Cleanup unused code in test https://review.opendev.org/c/zuul/zuul/+/802631 | 22:20 |
| corvus | clarkb: i'm not too worried about the debug log; we can always add more as needed. | 22:20 |
| clarkb | corvus: what does context.check_hostname = False in the ssl change do? Does that not check the altnames or subject name in the cert? or something else? | 22:22 |
| corvus | lookin | 22:23 |
| clarkb | hrm actually it looks like we're doing client cert authenticated requiests? SO maybe we don't care about hostnames because we're verifying the client cert was issued by the common CA? | 22:24 |
| corvus | clarkb: yes, i believe that's all the case -- and i suspect the idea is to avoid having to figure out what an external facing k8s hostname would be at the time of ssl issuance | 22:25 |
| corvus | (or possibly even cert reuse by multiple clients across a cluster) | 22:25 |
| corvus | s/clients/servers/ | 22:25 |
| clarkb | is context.verify_mode = ssl.CERT_REQUIRED the key ingredient to do the client cert auth? | 22:27 |
| clarkb | actually I think that may authenticate from both sides | 22:28 |
| corvus | https://docs.python.org/3/library/ssl.html#ssl.CERT_REQUIRED is the docs for that | 22:28 |
| clarkb | ya ok we set that on both sides and hten mutually verify the other | 22:29 |
| clarkb | but then don't bothe rwith validating a hostname | 22:29 |
| corvus | yeah, and we don't want TLS_CLIENT since we want to use a cert but not validate the hostname | 22:29 |
| clarkb | which should be fine as long as you use a controlled CA for this. Do you think that is worth documenting? | 22:29 |
| clarkb | basically don't go using LE certs for this | 22:29 |
| corvus | clarkb: yeah -- or maybe it should be an option? | 22:31 |
| clarkb | ya that might be a good improvment. Then you could use an LE cert if you wanted to | 22:32 |
| clarkb | corvus: can you check my comment on https://review.opendev.org/c/zuul/zuul/+/664950 I htink that is an actual bug? | 22:34 |
| corvus | clarkb: yup that's a bug | 22:37 |
| opendevreview | James E. Blair proposed zuul/zuul master: Support ssl encrypted fingergw https://review.opendev.org/c/zuul/zuul/+/664950 | 22:39 |
| opendevreview | James E. Blair proposed zuul/zuul master: Combine fingergw certificate options https://review.opendev.org/c/zuul/zuul/+/793671 | 22:39 |
| opendevreview | James E. Blair proposed zuul/zuul master: Randomze choice of zoned fingergw https://review.opendev.org/c/zuul/zuul/+/802628 | 22:39 |
| corvus | clarkb: fixed that and squashed the cleanup change into that | 22:39 |
| clarkb | ok | 22:39 |
| clarkb | corvus: stack lgtm, I would just say maybe tack on the bit about CA choice to the docs | 22:46 |
| clarkb | and/or make hostname verification optional | 22:46 |
| corvus | clarkb: will do, thx! | 22:46 |
| clarkb | and then probably do want to try and land that entire stack together | 22:48 |
| clarkb | and we already produce fingergw docker images so that is set | 22:49 |
| opendevreview | James E. Blair proposed zuul/zuul master: Add option to check fingergw hostnames https://review.opendev.org/c/zuul/zuul/+/802634 | 23:03 |
| corvus | clarkb: if that looks good to you, let's ask tobiash to review it tomorrow before landing the stack | 23:03 |
| clarkb | lgtm | 23:06 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!