opendevreview | Merged zuul/zuul master: Use create_all for empty databases https://review.opendev.org/c/zuul/zuul/+/797180 | 00:05 |
---|---|---|
corvus | clarkb: agreed; i left followup comments | 00:11 |
clarkb | ok. It seems like functionally we may be ok since we don't depend on removing tenants from abide.tenants to clean up zk and TPCs etc | 00:12 |
clarkb | But I think we'll leak the entries in there | 00:12 |
corvus | yep, and layouts :) | 00:14 |
opendevreview | Merged zuul/zuul master: Add Postgres schema test https://review.opendev.org/c/zuul/zuul/+/798362 | 00:14 |
clarkb | ah because layouts are a tenant attribute not an abide attribute | 00:16 |
clarkb | my brain is melting a bit thinking that through. How do tpcs and layouts differ. Are tpcs the raw config and layouts the prased config? | 00:18 |
clarkb | ah no a tpc is generated for each tenant:project config and a layout is the aggregate of that within a tenant looks like | 00:19 |
clarkb | so ya we may flush the tpcs from the abide but then we'd still have the layout pointing at abunch of stuff via the tenant I think | 00:19 |
clarkb | in any case I'll try to pick this back up in the morning. Hopefully the comments I left make sense | 00:20 |
corvus | clarkb: thanks! | 00:55 |
opendevreview | Merged zuul/zuul master: Move tenant validation to separate method https://review.opendev.org/c/zuul/zuul/+/799463 | 01:05 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 01:15 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 02:07 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 02:41 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 03:02 |
*** zbr is now known as Guest164 | 05:03 | |
opendevreview | Simon Westphahl proposed zuul/zuul master: Refactor pipeline processing in run handler https://review.opendev.org/c/zuul/zuul/+/795985 | 05:49 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Refactor pipeline processing in run handler https://review.opendev.org/c/zuul/zuul/+/795985 | 06:18 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Tenant, pipeline and queue locks in Zookeeper https://review.opendev.org/c/zuul/zuul/+/795993 | 06:24 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Tenant, pipeline and queue locks in Zookeeper https://review.opendev.org/c/zuul/zuul/+/795993 | 06:30 |
*** rpittau|afk is now known as rpittau | 06:59 | |
amar_[m] | Hi team, we are migrating to zuulV3, Is there way to get the customized images (all the dependencies pre-installed) ready to run on Nodepool or any docs/steps for image customization would also be helpful | 07:04 |
opendevreview | Felix Edel proposed zuul/zuul master: Move parent provider determination to pipeline manager https://review.opendev.org/c/zuul/zuul/+/797631 | 07:07 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Combine full and smart reconfiguration events https://review.opendev.org/c/zuul/zuul/+/799464 | 07:15 |
*** jpena|off is now known as jpena | 07:19 | |
*** sshnaidm_ is now known as sshnaidm | 08:25 | |
opendevreview | Albin Vass proposed zuul/zuul-operator master: Mount connection sshkeys on executors and mergers https://review.opendev.org/c/zuul/zuul-operator/+/799998 | 09:32 |
avass[m] | corvus: that ^ looked like it was missing, at least we didn't have any sshkeys for connections on mergers/executors | 09:34 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Store tenant layout state in Zookeeper https://review.opendev.org/c/zuul/zuul/+/771461 | 09:42 |
swest | clarkb: corvus: thanks! addressed your comments and updated the changes | 09:52 |
opendevreview | Guillaume Chauvel proposed zuul/zuul master: WIP gitlab quick-start https://review.opendev.org/c/zuul/zuul/+/795540 | 10:43 |
opendevreview | Guillaume Chauvel proposed zuul/zuul master: WIP gitlab quick-start HTTPS https://review.opendev.org/c/zuul/zuul/+/800009 | 10:43 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Create indexed logger for test scheduler instances https://review.opendev.org/c/zuul/zuul/+/800016 | 11:24 |
*** jpena is now known as jpena|lunch | 11:31 | |
*** jpena|lunch is now known as jpena | 12:27 | |
*** rpittau is now known as rpittau|afk | 12:45 | |
fungi | amar_[m]: assuming you're using diskimage-builder, you just add custom elements to install whatever you want into your images at image build time. in opendev we use some elements defined here: https://opendev.org/openstack/project-config/src/branch/master/nodepool/elements | 12:45 |
fungi | amar_[m]: and then we define an abstract base image in our nodepool config our other images inherit from, with those additional elements: https://opendev.org/openstack/project-config/src/branch/master/nodepool/nodepool.yaml#L112-L118 | 12:47 |
amar__ | fungi:ya we were following the same, but first we went with a cloud-image rather to disk-image. | 13:30 |
amar__ | fungi: Could we do some tweaks(installing dependencies) to cloud-image with "guestfish" and then use it to build a vm in nodepool? Is it possible? | 13:32 |
fungi | i have no idea what guestfish is | 13:33 |
fungi | you could certainly manually make any changes you like to an image and upload it and then tell nodepool to use that instead of relying on a nodepool-builder to construct an image for you | 13:34 |
opendevreview | Felix Edel proposed zuul/zuul master: Move parent provider determination to pipeline manager https://review.opendev.org/c/zuul/zuul/+/797631 | 13:35 |
opendevreview | Felix Edel proposed zuul/zuul master: Switch to ZooKeeper backed NodesProvisionedEvents https://review.opendev.org/c/zuul/zuul/+/799833 | 13:35 |
amar__ | fungi:Sure, thanks for the info. | 13:38 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Store tenant layout state in Zookeeper https://review.opendev.org/c/zuul/zuul/+/771461 | 14:19 |
opendevreview | Simon Westphahl proposed zuul/zuul master: Don't clear connection caches during full reconfig https://review.opendev.org/c/zuul/zuul/+/800066 | 14:28 |
clarkb | amar__: fungi another option in some clouds is to make your changes then take a snapshot of that image then tell nodepool to use the snapshot | 15:00 |
clarkb | swest: those updates lgtm. I'll work on getting through some of the child changes today as well | 15:15 |
corvus | clarkb: my -1 on https://review.opendev.org/797631 is minor; assuming felixedel is gone for the day, if you happen to look at it and +2, i'll just fix that myself and we can land it. mostly saying don't let my -1 stop you from reviewing if you get to it. | 15:48 |
clarkb | corvus: ok | 15:49 |
*** marios is now known as marios|out | 16:07 | |
*** jpena is now known as jpena|off | 16:22 | |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 16:29 |
opendevreview | James E. Blair proposed zuul/zuul master: Lock node requests in fake nodepool https://review.opendev.org/c/zuul/zuul/+/787301 | 16:42 |
opendevreview | James E. Blair proposed zuul/zuul master: Use the nodeset build parameter instead of hosts/groups https://review.opendev.org/c/zuul/zuul/+/799127 | 16:45 |
opendevreview | James E. Blair proposed zuul/zuul master: Move parent provider determination to pipeline manager https://review.opendev.org/c/zuul/zuul/+/797631 | 16:47 |
opendevreview | Merged zuul/zuul master: Refactor pipeline processing in run handler https://review.opendev.org/c/zuul/zuul/+/795985 | 16:58 |
opendevreview | Merged zuul/zuul master: Combine full and smart reconfiguration events https://review.opendev.org/c/zuul/zuul/+/799464 | 17:03 |
clarkb | corvus: sorry I'm sitting down to review now | 17:10 |
opendevreview | James E. Blair proposed zuul/zuul master: Verify ZK certs can be read https://review.opendev.org/c/zuul/zuul/+/797135 | 17:10 |
corvus | clarkb: no apology necessary :) i went ahead and made my minor change to 631 | 17:10 |
clarkb | I have approved https://review.opendev.org/c/zuul/zuul/+/795993 with some super minor nits (we use a constant in some places and not in others) | 17:17 |
clarkb | and i guess its child needs to be rebased so I'll go to the next stack instead. then we can solve all the erbasing once the various stacks are erviewed | 17:18 |
corvus | yeah i noticed that too... i actually kinda liked not having the constant, because it made it easier to understand... | 17:18 |
clarkb | thats fine with me too then | 17:19 |
corvus | i wonder if we could do constants like "ZUULLOCKS" instead of "LOCKROOT" | 17:19 |
clarkb | I was more callign out the inconsistency than a strong desire to use the constant (though my comments probably imply wanting to use teh constant) | 17:19 |
corvus | yeah, constants are usually better, it just struck me that i was more likely to catch the error i did because we didn't have a constant. | 17:20 |
corvus | so i'm mulling it over. clearly not a showstopper either way :) | 17:21 |
corvus | i bet if we switched to constants and had all of them bunched up nicely together at the top and in a clearly structured way, it would be just as readable. so on balance, that's probably what we should do. | 17:22 |
fungi | i like constants grouped at the top of modules like that, makes them easier to reuse from other modules in a consistent way | 17:23 |
clarkb | corvus: does https://review.opendev.org/c/zuul/zuul/+/797631/ mean you cannot have duplicate jobs in a buildset? For some reason I thought you could and if so are we ok with this change in behavior? | 17:40 |
avass[m] | I thought that it was already impossible to have duplicate jobs in a buildset? | 17:41 |
clarkb | that could be too. I thought for some reason you could though, maybe with variants? | 17:42 |
corvus | clarkb: i believe job names have needed to be unique in a buildset for some time (when we run "duplicate jobs" for testing, we stick numbers on the end usually) | 17:44 |
corvus | so i think it's not a behavior change | 17:44 |
clarkb | ok | 17:45 |
corvus | clarkb: example: https://opendev.org/zuul/zuul/src/branch/master/zuul/model.py#L2382 only works if there's a single job with that name | 17:45 |
clarkb | I must be misremembering then | 17:46 |
clarkb | corvus: I did +2 the change but left a question on it you may be able to answer | 17:46 |
clarkb | corvus: comment on https://review.opendev.org/c/zuul/zuul/+/787301 as well | 17:49 |
clarkb | Now the big change "Store tenant layout state in Zookeeper" | 17:49 |
corvus | clarkb: agree on 797631 and i think that's worth changing. | 17:51 |
corvus | clarkb: heh, that change isn't as big as it sounds :) | 17:51 |
clarkb | corvus: the title and line count certainly imply otherwise :) | 17:52 |
clarkb | its ok I have beedle the bardcore queued up I am now ready | 17:54 |
corvus | clarkb: you may be amused by my reply on 787301 | 18:03 |
clarkb | corvus: oh ya maybe make that a normal non blocking request? | 18:05 |
clarkb | corvus: can you check my comments on https://review.opendev.org/c/zuul/zuul/+/771461 | 18:11 |
clarkb | swest: corvus for https://review.opendev.org/c/zuul/zuul/+/800066 I guess the risk here is if we miss an event for a branch creation or deletion then we have to restart zuul? | 18:15 |
clarkb | which probably isn't a problem 99% of the time, but I wonder if once this is running in the wild people will notice (because the internet is flaky) | 18:16 |
y2kenny | I have been using Nodepool on a self hosted k8s cluster (v1.18.1). I just tried upgrading Nodepool from 4.1.0 to 4.2.0 (by simply changing the docker image tag, no other changes were made) and I got the following errors. Is this expected? http://paste.openstack.org/show/807294/ | 18:17 |
corvus | clarkb, tristanC: ^ does y2kenny's error look familier (related to the k8s conf changes maybe?) | 18:18 |
fungi | it's like the kubernetes provider driver is getting initialized before the configuration has been loaded? | 18:19 |
clarkb | I know that the k8s client stuff has seen churn. Maybe you need to provide an additional file now | 18:20 |
clarkb | but no not familiar to me off the top of my head | 18:20 |
avass[m] | openshift is still pinned so I don't think that's because of a package upgrade at least: https://review.opendev.org/plugins/gitiles/zuul/nodepool/+/refs/heads/master/requirements.txt#18 | 18:20 |
avass[m] | which has been the cause for similar errors in the past | 18:21 |
clarkb | ya the change that added utils_k8s didn't change any of the deps | 18:21 |
clarkb | https://github.com/openshift/openshift-restclient-python/blob/v0.11.2/requirements.txt#L2 is the version of the k8s library we should be using via the openshift pin | 18:24 |
avass[m] | maybe this should end with an `except: Exception` and error with a logmessage: https://review.opendev.org/plugins/gitiles/zuul/nodepool/+blame/refs/tags/4.2.0/nodepool/driver/utils_k8s.py#25 | 18:24 |
corvus | clarkb: replied on 771461 | 18:25 |
clarkb | not having the exception there should raise the exception all the way up which doesn't seem to be what happened | 18:25 |
avass[m] | oh, right | 18:25 |
clarkb | avass[m]: instead _get_conf is returning a None value for conf | 18:26 |
clarkb | y2kenny: do your debug logs say "Kubernetes config file not found"? | 18:27 |
avass[m] | y2kenny: do you see any debug message before that error? | 18:28 |
clarkb | I suspect that it isn't finding a valid config and then is trying to get the in cluster config which returns None | 18:28 |
y2kenny | clarkb: um... that's weird because all else being equal I did not have the same problem for 4.1.0. Let me see if there are more error. | 18:29 |
corvus | clarkb: re 800066 i think a scheduler restart would have the same effect as the removed code (both now and in the future), since it's a cache on the connection object. | 18:30 |
clarkb | https://github.com/kubernetes-client/python-base/blob/8a969ee9ad2837528c19673cfb5127b47be03105/config/kube_config.py#L48 is the path that should be looked at | 18:30 |
tristanC | corvus: arg seems like it, let me check | 18:30 |
corvus | y2kenny: also we have functional tests for self-hosted k8s drivers, so it's pretty surprising to see this; we must be missing something subtle. | 18:31 |
clarkb | that k8s config path list hasn't changed in years | 18:32 |
avass[m] | zuul did not detect the last time the in cluster config broke so I suppose there's nothing testing that codepath? | 18:32 |
avass[m] | unless someone added tests for that back then | 18:32 |
corvus | avass: oh, we may be relying on an explicit config then? | 18:33 |
avass[m] | yeah | 18:33 |
clarkb | corvus: re 771461 do you want me to approve it and followup on the ordering thing after or wait for input on that | 18:33 |
corvus | clarkb: i think approving is fine | 18:34 |
avass[m] | corvus: or at least I assume so since the nodepool.yaml fixture is setting `context: minikube` | 18:35 |
clarkb | swest: please check my comment on https://review.opendev.org/c/zuul/zuul/+/771461 about the __eq__ operator and total_ordering. I've approved the change just now though so followup if necessary is fine | 18:36 |
avass[m] | so I guess it's also running outside the cluster | 18:36 |
corvus | avass: yep, makes sense. this might be easier to test in the operator. | 18:36 |
opendevreview | Tristan Cacqueray proposed zuul/nodepool master: kubernetes: handle situation where the configuration is empty https://review.opendev.org/c/zuul/nodepool/+/800096 | 18:36 |
corvus | (currently the operator test also uses an explicit config, but i bet we could switch it to in-cluster) | 18:36 |
tristanC | y2kenny: corvus: ^ should fix the issue | 18:37 |
y2kenny | clark, avass[m] , corvus: my current config is in /etc/nodepool/nodepool.yaml and k8s provider/driver does not specify any config location... the nodepool is launched inside the cluster in which it can allocate resources | 18:37 |
y2kenny | tristanC: Ah ok. | 18:37 |
corvus | tristanC: and client = k8s_client.CoreV1Api(conf) is okay with conf=None? | 18:37 |
corvus | (i think so, cause i think the old code did that, but just wanted to double check) | 18:37 |
clarkb | I don't know that will fix it? We'll make a client with no config | 18:38 |
tristanC | corvus: the old code did that indeed | 18:38 |
clarkb | in _get_conf you could check if new_client_from_config returns None then load the in cluster config from there? | 18:38 |
avass[m] | yeah I don't follow either | 18:40 |
clarkb | huh the old code does do that though | 18:40 |
clarkb | maybe passing None in implies in cluster config | 18:41 |
clarkb | https://github.com/kubernetes-client/python/blob/d8f283e7483848647804eab345645106b6fb357d/kubernetes/client/api_client.py#L71 is what it does on None I think | 18:43 |
corvus | None is the default for CoreV1Api | 18:44 |
corvus | also relevant: https://github.com/kubernetes-client/python-base/blob/b4d3aad42dc23e7a6c0e5c032691f8dc385a786c/config/incluster_config.py#L112 | 18:46 |
corvus | that means we expect get_conf to return None in the case of an in-cluster config | 18:46 |
clarkb | fwiw it seems the behavior around this chagnes with kubernetes library versions | 18:46 |
corvus | so in short: conf=None does mean in-cluster | 18:47 |
clarkb | ya so we might be able to simplify this further but tristanC's change should work | 18:47 |
avass[m] | we may want to add a test for that then because that sounds like it absolutely going to break again in the future :) | 18:48 |
clarkb | is in cluster config basically every pod has k8s root? | 18:49 |
fungi | sorta like how every vm in gce has credentials for the gce api, i guess. they seem to like that trust model | 18:50 |
clarkb | s/root/some level of access allowing things/ | 18:50 |
clarkb | https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#use-the-default-service-account-to-access-the-api-server looks like you can opt out of it at least | 18:51 |
clarkb | avass[m]: for testing I agree with corvus that that is probably easiset with the operator since it already assumes running the services in k8s | 18:54 |
clarkb | but you could in theory modify the k8s integration job to do it too since that has a k8s and wants a nodepool | 18:54 |
clarkb | swest: corvus: I think https://review.opendev.org/c/zuul/zuul/+/799833/ and https://review.opendev.org/c/zuul/zuul/+/796013/ needs rebases now. | 18:57 |
avass[m] | I may be able to find some time and take a look at that next week, thats my last week before vacation and last week at Volvo so I don't think it's a good idea for me to start on anything super important anyway. | 18:57 |
clarkb | https://review.opendev.org/c/zuul/zuul/+/800016 is the last change that I haven't reviewed that is in a reviewable state. I'll get to it momentarily | 18:57 |
clarkb | y2kenny: fwiw we'ev approved tristanC's fix and I expect we can make a 4.2.1 release once that lands | 19:02 |
clarkb | corvus: I'm happy to push a tag later today if that helps | 19:03 |
corvus | yeah, if we can do in the nodepool repo, that'd be best, but if we're running running a launcher in k8s there now, it'll be a bit of work | 19:05 |
corvus | clarkb, tristanC oh we should add a release note | 19:06 |
corvus | can anyone else do that real quick? | 19:07 |
clarkb | I can do a release note, do we want that ina a separate change? | 19:07 |
corvus | clarkb: id do it in the same change | 19:08 |
y2kenny | clarkb, tristanC: thanks guys!! | 19:08 |
corvus | it won't be any slower, probably a little faster | 19:08 |
clarkb | ok writing that now | 19:08 |
opendevreview | Merged zuul/zuul master: Log Ansible Error Lines More Lengthy https://review.opendev.org/c/zuul/zuul/+/790708 | 19:08 |
corvus | y2kenny: thank you for the report | 19:08 |
opendevreview | Clark Boylan proposed zuul/nodepool master: kubernetes: handle situation where the configuration is empty https://review.opendev.org/c/zuul/nodepool/+/800096 | 19:13 |
clarkb | corvus: tristanC ^ something like that? | 19:13 |
clarkb | corvus: I'll let you hit the +A button if that looks good to you. I've been told lunch is ready. Will check back in after | 19:15 |
opendevreview | James E. Blair proposed zuul/nodepool master: kubernetes: handle situation where the configuration is empty https://review.opendev.org/c/zuul/nodepool/+/800096 | 19:17 |
corvus | clarkb: was missing word 'Kubernetes' :) | 19:17 |
tristanC | clarkb: corvus: thanks! | 19:18 |
clarkb | corvus: ++ | 19:47 |
clarkb | there is a pep8 issue on that change, I'm fixing it now | 19:51 |
clarkb | just running pep8 locally really quickly to be sure I fixed it all | 19:53 |
corvus | oh indentation huh | 19:53 |
opendevreview | Clark Boylan proposed zuul/nodepool master: kubernetes: handle situation where the configuration is empty https://review.opendev.org/c/zuul/nodepool/+/800096 | 19:54 |
clarkb | corvus: yup^ | 19:54 |
opendevreview | Merged zuul/zuul master: Tenant, pipeline and queue locks in Zookeeper https://review.opendev.org/c/zuul/zuul/+/795993 | 20:13 |
clarkb | https://review.opendev.org/c/zuul/zuul/+/771461 apparently need a reabse too | 20:14 |
clarkb | I guess I should take a quick look at doing some rebases | 20:14 |
corvus | if you've got time, thanks! my working tree is dirty right now | 20:14 |
opendevreview | Merged zuul/zuul master: Create indexed logger for test scheduler instances https://review.opendev.org/c/zuul/zuul/+/800016 | 20:15 |
opendevreview | Clark Boylan proposed zuul/zuul master: Store tenant layout state in Zookeeper https://review.opendev.org/c/zuul/zuul/+/771461 | 20:16 |
opendevreview | Clark Boylan proposed zuul/zuul master: Don't clear connection caches during full reconfig https://review.opendev.org/c/zuul/zuul/+/800066 | 20:16 |
clarkb | ya I'm punting on digging int ogerrit account cleanups because thats never fun :) | 20:17 |
opendevreview | Clark Boylan proposed zuul/zuul master: Move parent provider determination to pipeline manager https://review.opendev.org/c/zuul/zuul/+/797631 | 20:27 |
clarkb | corvus: ^ I cleaned up the precedence thing in that one. The child needs rebasing due to aconflict with master. I think we probably want to land ^ first to make diffs cleaner | 20:27 |
*** mgoddard- is now known as mgoddard | 20:50 | |
opendevreview | Clark Boylan proposed zuul/zuul master: Lock tenants and pipelines during processing https://review.opendev.org/c/zuul/zuul/+/796013 | 20:54 |
clarkb | swest: corvus ^ that rebase ended up being more complicated than I expected going into it :) please review it carefully | 20:55 |
clarkb | in particular want to ensure I mapped up all function calls to the new pipeline function calls and have the order correct as well as locks held at the right time. I think I got all that right but would definitely feel better if other eyeballs checked it | 20:55 |
corvus | clarkb: ah, i hadn't seen that yet since it didn't have the sos hashtag :) | 20:57 |
clarkb | corvus: I found it in the gerrit relation chain. | 20:57 |
opendevreview | Merged zuul/nodepool master: kubernetes: handle situation where the configuration is empty https://review.opendev.org/c/zuul/nodepool/+/800096 | 20:58 |
corvus | it looks compatible with the change we just reviewed/merged; but we should definitely check with swest on that. some sos changes without the hashtag are best considered early drafts which may be obsoleted at any time :) | 20:58 |
clarkb | ya I kind of figured this one was a draft while the parent changes got finalized. But now we've finalized things and it could be updated. | 21:00 |
clarkb | corvus: ^ the nodepool change has landed | 21:01 |
opendevreview | James E. Blair proposed zuul/zuul master: Link to the build page from the status page https://review.opendev.org/c/zuul/zuul/+/800111 | 21:02 |
opendevreview | James E. Blair proposed zuul/zuul master: Always report the build page https://review.opendev.org/c/zuul/zuul/+/800112 | 21:02 |
corvus | clarkb: i'll make a tag | 21:02 |
clarkb | corvus: for https://review.opendev.org/c/zuul/zuul/+/797631/ do you want swest to review the removal of the default precedence condition? Might be a good idea since we made those changes without swest input? | 21:02 |
corvus | clarkb: you mean felixedel ? | 21:03 |
clarkb | er yes | 21:03 |
clarkb | sorry I've been jumping around changes | 21:03 |
clarkb | and then the child of that doesn't really want to be rebased until 797631 lands since its conflict is with master | 21:03 |
corvus | clarkb: i think it's okay to merge 797631 now (i believe that was a code move and we fully understand the original intent and why it doesn't apply) | 21:05 |
corvus | zuul-maint: nodepool release okay? commit 39ff609835cfe82fb97886f1300ecd42f601768c (HEAD -> master, tag: 4.2.1, origin/master, refs/changes/96/800096/4) | 21:06 |
clarkb | 39ff609835cfe82fb97886f1300ecd42f601768c as 4.2.1 lgtm | 21:06 |
clarkb | ok let me double check 797631 to give it a proper rereview after rebase then I can approve it | 21:06 |
clarkb | corvus: on 797631 the nodepool-zuul-functional job timed out. That job is non voting so I suspect this may be expected. Any concern with approving it given that build status? | 21:08 |
opendevreview | James E. Blair proposed zuul/zuul master: Use an actual database in all Zuul tests https://review.opendev.org/c/zuul/zuul/+/797181 | 21:08 |
opendevreview | James E. Blair proposed zuul/zuul master: Remove ZuulDBTestCase https://review.opendev.org/c/zuul/zuul/+/798350 | 21:08 |
opendevreview | James E. Blair proposed zuul/zuul master: Report intermediate buildsets and builds https://review.opendev.org/c/zuul/zuul/+/797182 | 21:08 |
opendevreview | James E. Blair proposed zuul/zuul master: Add nodeset to build table https://review.opendev.org/c/zuul/zuul/+/798207 | 21:08 |
opendevreview | James E. Blair proposed zuul/zuul master: Link to the build page from the status page https://review.opendev.org/c/zuul/zuul/+/800111 | 21:08 |
opendevreview | James E. Blair proposed zuul/zuul master: Always report the build page https://review.opendev.org/c/zuul/zuul/+/800112 | 21:08 |
clarkb | looks like it specifically failed to connect ot zookeeper | 21:09 |
clarkb | https://9afc2a802fc84c0c4ce8-61a5ffddd2974bf904ab38b965509754.ssl.cf1.rackcdn.com/797631/9/check/nodepool-zuul-functional/ccc1b2f/job-output.txt | 21:09 |
corvus | clarkb: i think it's structurally broken since the zk changes? | 21:09 |
clarkb | ah ya could be | 21:09 |
clarkb | I have approved 797631 | 21:10 |
corvus | though i think it usually failse, not times out | 21:10 |
clarkb | looking in build history the time out has happened before | 21:10 |
clarkb | but typical failure mode is failure | 21:11 |
clarkb | the zk connection issues are consistent though | 21:11 |
corvus | (i love it: if you add 'registered' to a skipped task, it's still registered; so you can't use when with opposing conditionals to set a registered var in ansible) | 21:12 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 21:12 |
clarkb | yup I ran into that a few weeks ago | 21:13 |
clarkb | I think in the ansible of mailman | 21:13 |
corvus | clarkb: if you're up for it, topic:sql2 is a continuation of some changes you reviewed earlier | 21:15 |
corvus | i'm trying to get some of the usability improvements that should be possible with more use of the database | 21:16 |
corvus | (clicking status page links that dump us to the raw log server are especially bugging me now) | 21:16 |
corvus | wow, "prometheus protocol" is quite the google fail | 21:21 |
clarkb | do you get results to watch the movie? | 21:21 |
corvus | that would somehow be closer. no, i learn how to gain body mass. | 21:22 |
avass[m] | corvus: you're not looking for workout instructions? | 21:22 |
avass[m] | :) | 21:22 |
corvus | "exposition format" is apparently what i want | 21:23 |
opendevreview | Merged zuul/zuul master: Store tenant layout state in Zookeeper https://review.opendev.org/c/zuul/zuul/+/771461 | 21:39 |
clarkb | corvus: it has occurred to me that ianw wants to update the zuul config monday morning au time. The config update will opint zuul at the gerrit backend server directly to simplify the gerrit server move later. Do we need to consider a zuul restart tomorrow to ensure we didn't just land anything problematic for that? | 21:40 |
clarkb | I can help with that if so | 21:40 |
fungi | corvus: 39ff609 as 4.2.1 lgtm | 21:40 |
corvus | clarkb: sgtm | 21:41 |
corvus | fungi: thx will push | 21:41 |
fungi | sorry, got sidetracked by nomnomnoms | 21:41 |
clarkb | corvus: some questions on https://review.opendev.org/c/zuul/zuul/+/797182 | 22:11 |
clarkb | also as soon as I've mentioned that I think I sorted out the answer to my first question | 22:12 |
clarkb | maybe you can confirm the answer I've written on the change | 22:12 |
opendevreview | Merged zuul/zuul master: Move parent provider determination to pipeline manager https://review.opendev.org/c/zuul/zuul/+/797631 | 22:16 |
corvus | clarkb: done. want me to add to that test? (i didn't originally because i was just testing new functionality -- the update) | 22:18 |
clarkb | corvus: no I don't think that is necessary in this change. But might be a reaonsable thing to test as we add these tests? | 22:20 |
clarkb | corvus: is createBuildEntry() in https://review.opendev.org/c/zuul/zuul/+/797182/6/zuul/driver/sql/sqlreporter.py redundant now? | 22:23 |
corvus | clarkb: i think so -- i thirk there's some cleanup to do there, i'll take a look | 22:24 |
clarkb | corvus: https://review.opendev.org/c/zuul/zuul/+/798207 has some comments | 22:31 |
opendevreview | James E. Blair proposed zuul/zuul master: Remove some unused methods in sql reporter https://review.opendev.org/c/zuul/zuul/+/800121 | 22:31 |
corvus | clarkb: ^ i think we can do that cleanup | 22:31 |
opendevreview | James E. Blair proposed zuul/zuul master: Add nodeset to build table https://review.opendev.org/c/zuul/zuul/+/798207 | 22:48 |
opendevreview | James E. Blair proposed zuul/zuul master: Link to the build page from the status page https://review.opendev.org/c/zuul/zuul/+/800111 | 22:48 |
opendevreview | James E. Blair proposed zuul/zuul master: Always report the build page https://review.opendev.org/c/zuul/zuul/+/800112 | 22:48 |
corvus | oof, our reconfiguration change just broke the zuul-operator functional test because it's looking for a log line we just changed | 22:50 |
mordred | corvus: https://review.opendev.org/c/zuul/zuul/+/800112 question with +2 | 22:51 |
clarkb | corvus: I left a comment on ^ too | 22:51 |
corvus | incoming patch bomb due to that log line change | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Use kopf operator framework https://review.opendev.org/c/zuul/zuul-operator/+/785039 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Bump API version to v1alpha2 https://review.opendev.org/c/zuul/zuul-operator/+/785047 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Run a git server in k8s for functional tests https://review.opendev.org/c/zuul/zuul-operator/+/785738 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Move ingress to functional test https://review.opendev.org/c/zuul/zuul-operator/+/785757 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Support externally managed Zookeeper and DB https://review.opendev.org/c/zuul/zuul-operator/+/785273 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Pass through extra scheduler config options https://review.opendev.org/c/zuul/zuul-operator/+/785277 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add merger support https://review.opendev.org/c/zuul/zuul-operator/+/785278 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add keystore password support https://review.opendev.org/c/zuul/zuul-operator/+/790182 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Support imagePrefix and versions https://review.opendev.org/c/zuul/zuul-operator/+/785279 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Support fingergw https://review.opendev.org/c/zuul/zuul-operator/+/785300 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add docs https://review.opendev.org/c/zuul/zuul-operator/+/785083 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Support zuul-preview https://review.opendev.org/c/zuul/zuul-operator/+/785760 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add support for zuul-registry https://review.opendev.org/c/zuul/zuul-operator/+/785761 | 22:53 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Remove extra 2 minute wait from tests https://review.opendev.org/c/zuul/zuul-operator/+/785762 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add allowUnsafeConfig database setting https://review.opendev.org/c/zuul/zuul-operator/+/785764 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Pass through environment to scheduler, web and launcher https://review.opendev.org/c/zuul/zuul-operator/+/785988 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Allow terminationGracePeriodSeconds to be configurable https://review.opendev.org/c/zuul/zuul-operator/+/785989 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Flake8 cleanups https://review.opendev.org/c/zuul/zuul-operator/+/786349 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix error with multiple nodepool providers https://review.opendev.org/c/zuul/zuul-operator/+/799917 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Add instructions and tools for running tests with kind https://review.opendev.org/c/zuul/zuul-operator/+/785763 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: WIP: add static node to functional test https://review.opendev.org/c/zuul/zuul-operator/+/799874 | 22:54 |
opendevreview | James E. Blair proposed zuul/zuul-operator master: Fix waiting for scheduler to settle in functional test https://review.opendev.org/c/zuul/zuul-operator/+/800123 | 22:54 |
mordred | clarkb: https://codesearch.opendev.org/?q=success-url&i=nope&files=&excludeFiles=&repos= | 22:54 |
mordred | https://opendev.org/inaugust/inaugust.com/src/branch/master/.zuul.yaml#L19 is an interesting example | 22:55 |
clarkb | mordred: ya so instead what you need to do is return an artifact htat is that url | 22:56 |
corvus | mordred: yep -- but on opendev i think we already made those defunct anyway? | 22:56 |
clarkb | and then the artifact url will be present on the builds page | 22:56 |
clarkb | corvus: yes I think we set the report build page flag to true | 22:56 |
corvus | like, i think if you set 'report-build-page' to true (and opendev does) than you already get this behavior, so we basically broke it for everyone using opendev a while ago | 22:56 |
mordred | cool | 22:56 |
mordred | you know - for build-javascript-deployment - doing something to automatically return a zuul-preview artifact would be neat | 22:57 |
corvus | ya | 22:57 |
opendevreview | James E. Blair proposed zuul/zuul master: Always report the build page https://review.opendev.org/c/zuul/zuul/+/800112 | 22:59 |
corvus | clarkb, mordred: ^ revised reno | 22:59 |
clarkb | y2kenny: https://hub.docker.com/layers/zuul/nodepool-launcher/4.2.1/images/sha256-dc3a7c1600a959387d9a567ea924d618ac36ae242c74f1c8e65a59db9b2c2bae?context=explore exists now | 23:02 |
corvus | i'll do the announce | 23:03 |
opendevreview | James E. Blair proposed zuul/zuul master: Split optional matrix howto steps into new docs https://review.opendev.org/c/zuul/zuul/+/800125 | 23:18 |
corvus | i think that makes the matrix howto doc look a lot nicer; and it's worth noting that when we drop the irc bridge part, that gets cut in ~half | 23:19 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!