| @iwienand:matrix.org | corvus: i would say this was a reference to us helping them build and publish generic arm64 wheels | 00:00 |
|---|---|---|
| @jim:acmegating.com | okay, so from basic principals: i think in the long run if we're going to continue to build arm nodepool images, we need to flesh out the build infrastructure so we can do that on our own. i don't think we can rely on upstream projects pushing to pypi, or on openstack. i'm not saying we must block all progress on that right now, but i do think we should start on that so we don't end up in this situation again | 00:01 |
| @iwienand:matrix.org | i would say there are good ideas about doing it better, but upgrading the container to bullseye is also becoming a blocker for centos-9-stream work | 00:01 |
| @clarkb:matrix.org | > <@jim:acmegating.com> but now that we're issuing images, we should probably build wheels for those image platforms. | 00:01 |
| In this context you mean docker images? Yes, that is something that we could work on doing. My only real concern with it is we'd largely be doing it just for nodepool + arm64 so maybe targetting what nodepool needs is sufficient without doing it generically | ||
| @iwienand:matrix.org | i would be happy to take an action item to perhaps write this up as a spec, as we have a few options and a bit of a complex back story | 00:02 |
| @jim:acmegating.com | > <@clarkb:matrix.org> In this context you mean docker images? Yes, that is something that we could work on doing. My only real concern with it is we'd largely be doing it just for nodepool + arm64 so maybe targetting what nodepool needs is sufficient without doing it generically | 00:03 |
| yes. and yes, we would be largely doing it for nodepool+arm64, but i think if we want to build arm64 nodepool images, it's work that needs to be done | ||
| @jim:acmegating.com | maybe ianw can find a not-too-hard way of doing that in the spec. | 00:04 |
| @jim:acmegating.com | just brainstorming -- at a really high level, there's build wheels and put them in an archive. but there's also build a nodepool dependency image with wheels already built. | 00:05 |
| @jim:acmegating.com | so maybe we build a dependency image daily or something. | 00:05 |
| @clarkb:matrix.org | right exactly. If we're solving this just for nodepool we might be able to get away with a simpler setup | 00:06 |
| @iwienand:matrix.org | that is certainly a possibility. i think it's worth really pulling it apart and seeing what dependencies are pulled from wheels (which would be part of doing that) | 00:06 |
| @jim:acmegating.com | > <@clarkb:matrix.org> right exactly. If we're solving this just for nodepool we might be able to get away with a simpler setup | 00:07 |
| it's simpler, but also a universally applicable pattern | ||
| @iwienand:matrix.org | if it is annoying us, it is also annoying anyone with a slow raspberry pi | 00:07 |
| @clarkb:matrix.org | ianw: as an owner of a raspi that runs python for backups can confirm | 00:07 |
| @jim:acmegating.com | sure, but we can't block on asking upstreams to plz can haz wheels | 00:08 |
| @iwienand:matrix.org | agreed, but in some cases we may be able to at least lead the horse to water, as it were | 00:09 |
| @clarkb:matrix.org | agreed, but maybe we can push on that more concurrently and try ot make the ecosystem happier too (but don't block on it) | 00:09 |
| @jim:acmegating.com | it's great if they do of course, just that can be something to happen in the background | 00:09 |
| @jim:acmegating.com | sounds like we said same thing 3 ways :) | 00:09 |
| @jim:acmegating.com | okay, so having agreed we should: 1) help lead horses to water by helping upstreams build pypi wheels (long-term); and 2) not depending on that by coming up with some way to build arbitrary arm64 wheels in opendev (medium-term); 3) do we need a short-term decision? | 00:10 |
| @jim:acmegating.com | like, accept version skew of nodepool@3.9 and zuul@3.8 until zuulv5? defer whatever work is pushing for nodepool version upgrades until zuulv5? | 00:11 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/nodepool] 814830: Update Docker and bindep for Bullseye base images https://review.opendev.org/c/zuul/nodepool/+/814830 | 00:11 | |
| @jim:acmegating.com | i actually don't know the urgency of this question | 00:11 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/nodepool] 806312: Update Docker and bindep for Bullseye base images https://review.opendev.org/c/zuul/nodepool/+/806312 | 00:11 | |
| @jim:acmegating.com | (what is driving this?) | 00:11 |
| @iwienand:matrix.org | corvus: ^ is my short term proposal. the reason we want bullseye-based images is for building modern distros like 9-stream, which require updated on-host tools to maintain the status-quo of -minimal builds | 00:12 |
| @iwienand:matrix.org | (yes, we have other options like containerfile builds, which we are using for fedora, but people still like their image-based and minimal builds in other projects) | 00:13 |
| @jim:acmegating.com | hrm, we did containerfile specifically to address that problem | 00:14 |
| @iwienand:matrix.org | yes, and i'm pushing in that direction, and using it in opendev gate, and telling people it is a better idea | 00:16 |
| @iwienand:matrix.org | but it does require nesting container runtimes, etc. which is non-trival for dib users like tripleo, etc. | 00:17 |
| @jim:acmegating.com | ianw: so the tripleo project would like the zuul project to build a nodepool image on a later version of debian so that they can use that image to build 9-stream images directly on containers using that image without using containerfile? | 00:19 |
| @iwienand:matrix.org | https://review.opendev.org/c/openstack/diskimage-builder/+/811392 has been proposed, and i would like to make sure we have gate testing for it | 00:21 |
| @iwienand:matrix.org | https://review.opendev.org/c/openstack/diskimage-builder/+/814844 tests 811392 in the devstack functional tests, but requires building on a bullseye based image | 00:21 |
| @iwienand:matrix.org | i'm uncomfortable merging 811392 without a gate test | 00:22 |
| @jim:acmegating.com | ianw: can dib test that without nodepool? | 00:23 |
| @iwienand:matrix.org | (and feel somewhat justified, because already we found the issue with the cpu type via testing this) | 00:24 |
| @iwienand:matrix.org | corvus: anything is possible, but the current model of an end-to-end test of devstack+nodepool-builder+dib works well and I don't feel any great need to abandon it | 00:25 |
| @iwienand:matrix.org | i feel like it benefits both projects | 00:26 |
| @jim:acmegating.com | i think we're still putting too much weight on nodepool to do dib's gate testing. i think it's great that nodepool has a smoketest in its gate, that runs dib, but it can't carry the whole load. we've got to keep the concerns separate. | 00:26 |
| @iwienand:matrix.org | i would generally disagree with that. i think this is revaling very practical issues that affect production systems | 00:31 |
| @jim:acmegating.com | it just isn't scalable for zuul and nodepool to take on the entire gate testing burden for everything that DIB might need to build | 00:31 |
| @jim:acmegating.com | ianw: you and i did a lot of work to untangle the jobs a while back based on this. | 00:32 |
| @jim:acmegating.com | i don't object to moving nodepool to 3.9 for this. zuul is going to need to move eventually anyway. | 00:33 |
| @jim:acmegating.com | but i do not think that the dib project should rely heavily on nodepool for its gate testing. | 00:34 |
| @jim:acmegating.com | i think if a similar issue came up in the future that didn't happen to align with what we were going to do anyway, we may well decide not to go very far out of our way to accomodate dib's gate testing needs. | 00:35 |
| @iwienand:matrix.org | i feel like it has scaled, so far. and i also feel like it is a feature for nodepool-builder containers to be an excellent "reference" container that we know can actually build many bootable images | 00:35 |
| @iwienand:matrix.org | i'm not sure there are many benefits for me to merge platforms into dib that the upstream nodepool-builder containers can't build. i feel like we should really try our best to keep that in sync | 00:36 |
| @iwienand:matrix.org | maybe we hit something that fundamentally breaks that relationship -- but i would hope not | 00:37 |
| @jim:acmegating.com | ianw: i take your point on that, and i agree. i'm just very concerned that ^ may happen and i don't want you/dib to be unprepared for it. | 00:37 |
| @jim:acmegating.com | from my pov, it's great if the nodepool image happens to work for most of what dib can build | 00:38 |
| @jim:acmegating.com | but it is absolutely not a requirement that the nodepool image support everything that dib might support | 00:38 |
| @jim:acmegating.com | (just as, in the same way, we try to have the zuul image support most of what's in zuul-jobs, and i think we mostly do right now. but we might not support everything) | 00:39 |
| @iwienand:matrix.org | right, i also agree on that point, that nodepool is not tied to supporting every dib platform | 00:41 |
| @jim:acmegating.com | okay. sounds like we understand each other :) which change do you want me to +2 again? | 00:41 |
| @jim:acmegating.com | (what's the difference between 814830 and 806312?) | 00:43 |
| @iwienand:matrix.org | the change https://review.opendev.org/c/zuul/nodepool/+/806312 will update nodepool-builder to bullseye, which will mean that the dib gate can run a dib-nodepool-functional-openstack-centos-9-stream-src test, which will make me happy about merging centos-minimal 9-stream support into dib | 00:43 |
| @iwienand:matrix.org | oh, sorry, it looks like i've left the squished message, let me abandon | 00:44 |
| @jim:acmegating.com | ianw: 806312+2 cc Clark tobiash | 00:47 |
| @iwienand:matrix.org | thanks. while i have it in my head i will write up something on the wheels | 00:49 |
| @jim:acmegating.com | ianw: thanks! | 00:49 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul] 815406: Spec outline for ARM64 wheels https://review.opendev.org/c/zuul/zuul/+/815406 | 04:18 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 810920: Store change queues in Zookeeper https://review.opendev.org/c/zuul/zuul/+/810920 | 05:25 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: | 05:34 | |
| - [zuul/zuul] 811422: Save and restore bundle with item in Zookeeper https://review.opendev.org/c/zuul/zuul/+/811422 | ||
| - [zuul/zuul] 811955: Pass ZK context to deserialize method of ZKObjects https://review.opendev.org/c/zuul/zuul/+/811955 | ||
| - [zuul/zuul] 812450: Move ZuulMark from configloader to model https://review.opendev.org/c/zuul/zuul/+/812450 | ||
| - [zuul/zuul] 812451: Recursively delete all sub-nodes of ZKObjects https://review.opendev.org/c/zuul/zuul/+/812451 | ||
| - [zuul/zuul] 812466: Only retry ZK operations for Kazoo exceptions https://review.opendev.org/c/zuul/zuul/+/812466 | ||
| - [zuul/zuul] 812452: Store build sets in Zookeeper https://review.opendev.org/c/zuul/zuul/+/812452 | ||
| - [zuul/zuul] 812467: Add support for sharded ZKObjects https://review.opendev.org/c/zuul/zuul/+/812467 | ||
| - [zuul/zuul] 812673: Store RepoFiles for a build set in Zookeeper https://review.opendev.org/c/zuul/zuul/+/812673 | ||
| - [zuul/zuul] 813805: Remove project pipeline config from queue item https://review.opendev.org/c/zuul/zuul/+/813805 | ||
| - [zuul/zuul] 813809: Lookup event class names from global symbol table https://review.opendev.org/c/zuul/zuul/+/813809 | ||
| - [zuul/zuul] 813826: Store and resolve queue item's ahead/behind refs https://review.opendev.org/c/zuul/zuul/+/813826 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: | 05:34 | |
| - [zuul/zuul] 812750: Add LocalZKContext for job freezing https://review.opendev.org/c/zuul/zuul/+/812750 | ||
| - [zuul/zuul] 812760: Add RepoState object https://review.opendev.org/c/zuul/zuul/+/812760 | ||
| - [zuul/zuul] 813552: Remove Worker class https://review.opendev.org/c/zuul/zuul/+/813552 | ||
| -@gerrit:opendev.org- Zuul merged on behalf of Ian Wienand: [zuul/nodepool] 806312: Update Docker and bindep for Bullseye base images https://review.opendev.org/c/zuul/nodepool/+/806312 | 05:58 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 809532: Simplified attribute API for ZKObjects https://review.opendev.org/c/zuul/zuul/+/809532 | 06:15 | |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 810328: Temporarily enqueue cycle changes for reporting https://review.opendev.org/c/zuul/zuul/+/810328 | 07:33 | |
| -@gerrit:opendev.org- Simon Westphahl proposed on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: | 08:53 | |
| - [zuul/zuul] 813895: Move job_graph attribute to BuildSet https://review.opendev.org/c/zuul/zuul/+/813895 | ||
| - [zuul/zuul] 813913: Serialize JobGraph objects to ZK https://review.opendev.org/c/zuul/zuul/+/813913 | ||
| - [zuul/zuul] 814065: Serialize ProjectMetadata on JobGraph https://review.opendev.org/c/zuul/zuul/+/814065 | ||
| - [zuul/zuul] 814071: Add test_freeze_noop_job https://review.opendev.org/c/zuul/zuul/+/814071 | ||
| - [zuul/zuul] 814069: Remove setBase from job freeze API https://review.opendev.org/c/zuul/zuul/+/814069 | ||
| - [zuul/zuul] 814070: Create Abstract and FrozenJob classes https://review.opendev.org/c/zuul/zuul/+/814070 | ||
| - [zuul/zuul] 814242: Make FrozenJob.updateParentData a static method https://review.opendev.org/c/zuul/zuul/+/814242 | ||
| - [zuul/zuul] 814281: Remove toDict from FrozenJob https://review.opendev.org/c/zuul/zuul/+/814281 | ||
| - [zuul/zuul] 814243: Make FrozenJob a ZKObject https://review.opendev.org/c/zuul/zuul/+/814243 | ||
| - [zuul/zuul] 814329: Implement frozen job serialization/deserialization https://review.opendev.org/c/zuul/zuul/+/814329 | ||
| - [zuul/zuul] 814679: Store FrozenJob data in separate znodes https://review.opendev.org/c/zuul/zuul/+/814679 | ||
| - [zuul/zuul] 815154: Update test_inventory to be ZK-friendly https://review.opendev.org/c/zuul/zuul/+/815154 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 08:53 | |
| - [zuul/zuul] 814544: Cleanup stale items after refreshing a pipeline https://review.opendev.org/c/zuul/zuul/+/814544 | ||
| - [zuul/zuul] 814570: Reference active change queues in pipeline state https://review.opendev.org/c/zuul/zuul/+/814570 | ||
| - [zuul/zuul] 814571: Update pipeline state when modifying attributes https://review.opendev.org/c/zuul/zuul/+/814571 | ||
| - [zuul/zuul] 814772: Allow passing extra attributes to ZKObject.fromZK https://review.opendev.org/c/zuul/zuul/+/814772 | ||
| - [zuul/zuul] 814862: Bail out when a project moves between connections https://review.opendev.org/c/zuul/zuul/+/814862 | ||
| - [zuul/zuul] 814773: Move re-enqueue to pipeline processing https://review.opendev.org/c/zuul/zuul/+/814773 | ||
| - [zuul/zuul] 814899: Delete old build sets immediately https://review.opendev.org/c/zuul/zuul/+/814899 | ||
| - [zuul/zuul] 815309: Cancel jobs before resetting builds https://review.opendev.org/c/zuul/zuul/+/815309 | ||
| - [zuul/zuul] 815111: Store builds in Zookeeper https://review.opendev.org/c/zuul/zuul/+/815111 | ||
| - [zuul/zuul] 815276: Add change queues to change queue managers https://review.opendev.org/c/zuul/zuul/+/815276 | ||
| - [zuul/zuul] 815277: Refresh pipelines before checking for empty queues https://review.opendev.org/c/zuul/zuul/+/815277 | ||
| - [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 09:04 | |
| - [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | ||
| - [zuul/zuul] 815428: Fix GitHub PR (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815428 | ||
| - [zuul/zuul] 815429: Add missing logger to Build and BuildSet classes https://review.opendev.org/c/zuul/zuul/+/815429 | ||
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 09:09 | |
| - [zuul/zuul] 815428: Fix GitHub PR (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815428 | ||
| - [zuul/zuul] 815429: Add missing logger to Build and BuildSet classes https://review.opendev.org/c/zuul/zuul/+/815429 | ||
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| - [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | ||
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 808042: [zuul-web] Add pagination information when querying builds, buildsets endpoints https://review.opendev.org/c/zuul/zuul/+/808042 | 10:59 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 808043: Pagination: remove compatibility code https://review.opendev.org/c/zuul/zuul/+/808043 | 10:59 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: | 13:05 | |
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| - [zuul/zuul] 815495: Fix Gerrit change (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815495 | ||
| -@gerrit:opendev.org- Simon Westphahl proposed: | 13:07 | |
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| - [zuul/zuul] 815495: Fix Gerrit change (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815495 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 815527: Add some debug logging to change cache https://review.opendev.org/c/zuul/zuul/+/815527 | 16:50 | |
| @jim:acmegating.com | i tried to figure out the error in https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_318/811244/8/gate/zuul-tox-py36/3180ff9/testr_results.html but can't with the available logs (and of course can't reproduce it locally), so that change is an attempt to have more logs next time | 16:55 |
| -@gerrit:opendev.org- Clark Boylan proposed: | 17:07 | |
| - [zuul/zuul] 815389: Run the executor governor more often in testing https://review.opendev.org/c/zuul/zuul/+/815389 | ||
| - [zuul/zuul] 815390: Disable load sensors during testing https://review.opendev.org/c/zuul/zuul/+/815390 | ||
| - [zuul/zuul] 815529: Use all but one CPU when unittesting https://review.opendev.org/c/zuul/zuul/+/815529 | ||
| @clarkb:matrix.org | looking at the data that the other changes have been generating it seems we float just above the load limit and then back under then over again. I think that possibly we may get away with reduciing the stestr concurrency by one CPU rather than cutting it in half | 17:08 |
| @clarkb:matrix.org | that stack will continue to generate data we can look at tofigure that out | 17:08 |
| @jim:acmegating.com | Clark: i was wondering the same thing about N-1. thanks | 17:08 |
| @clarkb:matrix.org | Good news is the failure in 815389 does seem to show the governor bounce back and forth with the shorter sensor check period | 17:09 |
| @clarkb:matrix.org | we still failed one test that was bouncing around the limit but it does show that we can recover and then spike load and stop again I think | 17:10 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 815531: Increase timeout in test_build_request https://review.opendev.org/c/zuul/zuul/+/815531 | 17:15 | |
| @jim:acmegating.com | another change to address a test race (seen at https://bdd6e5877cca9dfd7335-c47321616317c48082e712d075dde0b4.ssl.cf1.rackcdn.com/810920/11/check/zuul-tox-py38/a43c989/testr_results.html ) | 17:16 |
| @ecsantos:matrix.org | Hi folks | 17:31 |
| @ecsantos:matrix.org | I'm facing a SSH issue with Nodepool. I'm passing `/var/lib/zuul/id_rsa.pub` as the public key [1] | 17:31 |
| [1] https://paste.opendev.org/show/810223/ | ||
| @ecsantos:matrix.org | But when I try `ssh -i /var/lib/zuul/id_rsa zuul@<IP address>` after Nodepool launches the node with my diskimage, I get `Permission denied (publickey)` | 17:32 |
| @ecsantos:matrix.org | Any ideas? | 17:32 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 811244: Dequeue items after we're done with them https://review.opendev.org/c/zuul/zuul/+/811244 | 17:35 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 809414: Make QueueItem a Zookeeper object https://review.opendev.org/c/zuul/zuul/+/809414 | 17:35 | |
| @clarkb:matrix.org | ecsantos: are you doing that on a fedora machine? | 17:37 |
| @clarkb:matrix.org | ecsantos: or is your openssh version >= 8.8 ? | 17:37 |
| @clarkb:matrix.org | `ssh -V` to find the version | 17:37 |
| @ecsantos:matrix.org | Clark: OpenSSH 7.4, on CentOS 7 | 17:38 |
| @clarkb:matrix.org | ok centos 7 + ubuntu focal shouldn't have any ssh-rsa deprecation issues | 17:39 |
| @clarkb:matrix.org | rules that out | 17:39 |
| @clarkb:matrix.org | maybe double check the pubkey file matches the private key? ( you can extract the pubkey from the private key though I never remember how and then compare that to what is in the pubkey) | 17:40 |
| @ecsantos:matrix.org | I built the diskimage manually with `disk-image-create` passing the same arguments Nodepool is using | 17:41 |
| @ecsantos:matrix.org | And I can SSH if I do that | 17:41 |
| @ecsantos:matrix.org | But when Nodepool builds the node I can't | 17:42 |
| @clarkb:matrix.org | ecsantos: is /var/lib/zuul/id_rsa.pub available to nodepool when it builds the images? If it is running in a container you may need to bind mount it in for example | 17:42 |
| @ecsantos:matrix.org | Clark: It is, I can see Nodepool grabbing it in the logs. Not using containers, it's an AIO Zuul + Nodepool + Gerrit installation | 17:44 |
| @jim:acmegating.com | ecsantos: a small discrepancy: you have ".ssh/" in the path in ZUUL_USER_SSH_PUBLIC_KEY: /var/lib/zuul/.ssh/id_rsa.pub according to https://paste.opendev.org/show/810223/ but here you said ssh -i /var/lib/zuul/id_rsa without ".ssh/" | 17:44 |
| @jim:acmegating.com | maybe there are actually 2 keys in play? | 17:48 |
| @jim:acmegating.com | okay, this race is really weird: https://paste.opendev.org/show/810224/ | 17:57 |
| @jim:acmegating.com | the log line i copied is emitted after the worker is deleted; the dictionary should be empty | 17:58 |
| @jim:acmegating.com | we also make sure the job worker list is empty as part of executor shutdown | 17:59 |
| @jim:acmegating.com | oh wait, i see something out of sequence | 17:59 |
| @ecsantos:matrix.org | > <@jim:acmegating.com> ecsantos: a small discrepancy: you have ".ssh/" in the path in ZUUL_USER_SSH_PUBLIC_KEY: /var/lib/zuul/.ssh/id_rsa.pub according to https://paste.opendev.org/show/810223/ but here you said ssh -i /var/lib/zuul/id_rsa without ".ssh/" | 18:02 |
| oh, sorry, my bad! I'm using `.ssh` in the SSH command path, just pasted it wrong | ||
| @clarkb:matrix.org | ecsantos: you might want to boot the image passing in a nova keyname and login using that then see how it is configured on the VM to figure out why ssh isn't working | 18:11 |
| @clarkb:matrix.org | ecsantos: before you do that though `ssh -vvv` may provide clues | 18:12 |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 810658: Store pipeline state in Zookeeper https://review.opendev.org/c/zuul/zuul/+/810658 | 18:53 | |
| @ecsantos:matrix.org | Clark: Tried doing that, still got permission denied. I'm starting to think the zuul user isn't being added for some reason | 18:54 |
| @clarkb:matrix.org | ecsantos: well the noav key won't be applied to the zuul user | 18:55 |
| @clarkb:matrix.org | ecsantos: if you use glean you have to login as root and if you use cloud-init it will be 'ubuntu' | 18:55 |
| @tobias.henkel:matrix.org | corvus: what's the latest change of the current sos stack which is ready for review? | 19:34 |
| @jim:acmegating.com | tobiash: i think the whole sos-pipeline-state topic is ready. i've gotten from 810920 through 813826 and plan on continuing (i paused to address a test race). | 19:35 |
| @jim:acmegating.com | gertty cheat-sheet: https://paste.opendev.org/show/810226/ | 19:36 |
| @tobias.henkel:matrix.org | wow | 19:37 |
| -@gerrit:opendev.org- Clark Boylan proposed: [zuul/nodepool] 815545: Add py39 testing to nodepool https://review.opendev.org/c/zuul/nodepool/+/815545 | 19:42 | |
| @clarkb:matrix.org | ianw: ^ your spec reminded me I meant to push that today now that the images are running py39 | 19:42 |
| @jim:acmegating.com | tobiash: lots of them are small and mechanical :) | 19:42 |
| @tobias.henkel:matrix.org | Clark: do you want to review those as well or shall I +3 those which have +2 from corvus and me? | 19:44 |
| @clarkb:matrix.org | tobiash: I'm planning to review this afternoon but there is also a board meeting and I'm yet to eat lunch. I think it is ok if you want to approve and I'll catch up when I'm able | 19:45 |
| @tobias.henkel:matrix.org | k | 19:45 |
| @tobias.henkel:matrix.org | corvus: q on 812452 | 20:05 |
| @jim:acmegating.com | tobiash: i think you're right | 20:07 |
| @jim:acmegating.com | should i just go ahead and fix that, or should we wait for swest? | 20:07 |
| @tobias.henkel:matrix.org | I think fixing is fine since that looks like just a forgotten attribute | 20:10 |
| @tobias.henkel:matrix.org | since it has been introduced in that change | 20:11 |
| @jim:acmegating.com | here goes | 20:14 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: | 20:15 | |
| - [zuul/zuul] 812750: Add LocalZKContext for job freezing https://review.opendev.org/c/zuul/zuul/+/812750 | ||
| - [zuul/zuul] 812760: Add RepoState object https://review.opendev.org/c/zuul/zuul/+/812760 | ||
| - [zuul/zuul] 813552: Remove Worker class https://review.opendev.org/c/zuul/zuul/+/813552 | ||
| - [zuul/zuul] 813895: Move job_graph attribute to BuildSet https://review.opendev.org/c/zuul/zuul/+/813895 | ||
| - [zuul/zuul] 813913: Serialize JobGraph objects to ZK https://review.opendev.org/c/zuul/zuul/+/813913 | ||
| - [zuul/zuul] 814065: Serialize ProjectMetadata on JobGraph https://review.opendev.org/c/zuul/zuul/+/814065 | ||
| - [zuul/zuul] 814071: Add test_freeze_noop_job https://review.opendev.org/c/zuul/zuul/+/814071 | ||
| - [zuul/zuul] 814069: Remove setBase from job freeze API https://review.opendev.org/c/zuul/zuul/+/814069 | ||
| - [zuul/zuul] 814070: Create Abstract and FrozenJob classes https://review.opendev.org/c/zuul/zuul/+/814070 | ||
| - [zuul/zuul] 814242: Make FrozenJob.updateParentData a static method https://review.opendev.org/c/zuul/zuul/+/814242 | ||
| - [zuul/zuul] 814281: Remove toDict from FrozenJob https://review.opendev.org/c/zuul/zuul/+/814281 | ||
| - [zuul/zuul] 814243: Make FrozenJob a ZKObject https://review.opendev.org/c/zuul/zuul/+/814243 | ||
| - [zuul/zuul] 814329: Implement frozen job serialization/deserialization https://review.opendev.org/c/zuul/zuul/+/814329 | ||
| - [zuul/zuul] 814679: Store FrozenJob data in separate znodes https://review.opendev.org/c/zuul/zuul/+/814679 | ||
| - [zuul/zuul] 815154: Update test_inventory to be ZK-friendly https://review.opendev.org/c/zuul/zuul/+/815154 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed on behalf of Simon Westphahl: | 20:15 | |
| - [zuul/zuul] 812452: Store build sets in Zookeeper https://review.opendev.org/c/zuul/zuul/+/812452 | ||
| - [zuul/zuul] 812467: Add support for sharded ZKObjects https://review.opendev.org/c/zuul/zuul/+/812467 | ||
| - [zuul/zuul] 812673: Store RepoFiles for a build set in Zookeeper https://review.opendev.org/c/zuul/zuul/+/812673 | ||
| - [zuul/zuul] 813805: Remove project pipeline config from queue item https://review.opendev.org/c/zuul/zuul/+/813805 | ||
| - [zuul/zuul] 813809: Lookup event class names from global symbol table https://review.opendev.org/c/zuul/zuul/+/813809 | ||
| - [zuul/zuul] 813826: Store and resolve queue item's ahead/behind refs https://review.opendev.org/c/zuul/zuul/+/813826 | ||
| - [zuul/zuul] 814544: Cleanup stale items after refreshing a pipeline https://review.opendev.org/c/zuul/zuul/+/814544 | ||
| - [zuul/zuul] 814570: Reference active change queues in pipeline state https://review.opendev.org/c/zuul/zuul/+/814570 | ||
| - [zuul/zuul] 814571: Update pipeline state when modifying attributes https://review.opendev.org/c/zuul/zuul/+/814571 | ||
| - [zuul/zuul] 814772: Allow passing extra attributes to ZKObject.fromZK https://review.opendev.org/c/zuul/zuul/+/814772 | ||
| - [zuul/zuul] 814862: Bail out when a project moves between connections https://review.opendev.org/c/zuul/zuul/+/814862 | ||
| - [zuul/zuul] 814773: Move re-enqueue to pipeline processing https://review.opendev.org/c/zuul/zuul/+/814773 | ||
| - [zuul/zuul] 814899: Delete old build sets immediately https://review.opendev.org/c/zuul/zuul/+/814899 | ||
| - [zuul/zuul] 815309: Cancel jobs before resetting builds https://review.opendev.org/c/zuul/zuul/+/815309 | ||
| - [zuul/zuul] 815111: Store builds in Zookeeper https://review.opendev.org/c/zuul/zuul/+/815111 | ||
| - [zuul/zuul] 815276: Add change queues to change queue managers https://review.opendev.org/c/zuul/zuul/+/815276 | ||
| - [zuul/zuul] 815277: Refresh pipelines before checking for empty queues https://review.opendev.org/c/zuul/zuul/+/815277 | ||
| - [zuul/zuul] 815428: Fix GitHub PR (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815428 | ||
| - [zuul/zuul] 815429: Add missing logger to Build and BuildSet classes https://review.opendev.org/c/zuul/zuul/+/815429 | ||
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| - [zuul/zuul] 815495: Fix Gerrit change (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815495 | ||
| -@gerrit:opendev.org- Tobias Henkel proposed on behalf of Simon Westphahl: [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | 20:20 | |
| -@gerrit:opendev.org- Tobias Henkel proposed on behalf of Simon Westphahl: [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | 20:21 | |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 815553: Fix test race with paused jobs https://review.opendev.org/c/zuul/zuul/+/815553 | 20:22 | |
| @jim:acmegating.com | that's another test race fix. i'm hashtagging those with "testrace" | 20:22 |
| @tobias.henkel:matrix.org | hrm, maybe we should prioritize those | 20:23 |
| @clarkb:matrix.org | corvus: should we land https://review.opendev.org/c/zuul/zuul/+/815529 for similar reasons? | 20:23 |
| @tobias.henkel:matrix.org | otherwise it will take a week to nurse the sos stack in | 20:23 |
| @jim:acmegating.com | tobiash: yeah, i think prioritizing reviews on thoes would be good. if things get really bad we can temporarily switch to 32G nodes | 20:24 |
| @jim:acmegating.com | Clark: good looking numbers on 529. +3 | 20:24 |
| @jim:acmegating.com | there was a few minutes of LA=18 on that, but was overall closter to 12, so that's nice. | 20:26 |
| @jim:acmegating.com | (above was re the 3.8 job; the 3.6 had a LA>20 for a moment | 20:27 |
| @jim:acmegating.com | i'm running local tests on 815553 right now to try to catch any issues with it | 20:29 |
| @jim:acmegating.com | passes | 20:46 |
| @tobias.henkel:matrix.org | corvus: commented on 812452 | 20:47 |
| @jim:acmegating.com | doh | 20:47 |
| @jim:acmegating.com | i left a pretty minor suggestion on https://review.opendev.org/814571 - i'm gonna go ahead and implement it now too. ccing swest here for the record | 20:48 |
| @jim:acmegating.com | (i'll fix both of those at once) | 20:48 |
| @clarkb:matrix.org | corvus: I went ahead and approved 815553 | 20:48 |
| @jim:acmegating.com | Clark: thx | 20:48 |
| @ecsantos:matrix.org | Clark: Just to let you know, found the culprit. For some reason, Nodepool is echoing the the path `/var/lib/zuul/.ssh/id_rsa.pub` as a string to the diskimage's `authorized_keys` file, instead of passing its content. Still not sure why. Thanks for all the help guys! | 20:49 |
| @clarkb:matrix.org | ecsantos: that variable may expect it to be the actual key contents and not a path. Look in the element for the zuul user and see what it does with that variable | 20:49 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: | 20:55 | |
| - [zuul/zuul] 812750: Add LocalZKContext for job freezing https://review.opendev.org/c/zuul/zuul/+/812750 | ||
| - [zuul/zuul] 812760: Add RepoState object https://review.opendev.org/c/zuul/zuul/+/812760 | ||
| - [zuul/zuul] 813552: Remove Worker class https://review.opendev.org/c/zuul/zuul/+/813552 | ||
| - [zuul/zuul] 813895: Move job_graph attribute to BuildSet https://review.opendev.org/c/zuul/zuul/+/813895 | ||
| - [zuul/zuul] 813913: Serialize JobGraph objects to ZK https://review.opendev.org/c/zuul/zuul/+/813913 | ||
| - [zuul/zuul] 814065: Serialize ProjectMetadata on JobGraph https://review.opendev.org/c/zuul/zuul/+/814065 | ||
| - [zuul/zuul] 814071: Add test_freeze_noop_job https://review.opendev.org/c/zuul/zuul/+/814071 | ||
| - [zuul/zuul] 814069: Remove setBase from job freeze API https://review.opendev.org/c/zuul/zuul/+/814069 | ||
| - [zuul/zuul] 814070: Create Abstract and FrozenJob classes https://review.opendev.org/c/zuul/zuul/+/814070 | ||
| - [zuul/zuul] 814242: Make FrozenJob.updateParentData a static method https://review.opendev.org/c/zuul/zuul/+/814242 | ||
| - [zuul/zuul] 814281: Remove toDict from FrozenJob https://review.opendev.org/c/zuul/zuul/+/814281 | ||
| - [zuul/zuul] 814243: Make FrozenJob a ZKObject https://review.opendev.org/c/zuul/zuul/+/814243 | ||
| - [zuul/zuul] 814329: Implement frozen job serialization/deserialization https://review.opendev.org/c/zuul/zuul/+/814329 | ||
| - [zuul/zuul] 814679: Store FrozenJob data in separate znodes https://review.opendev.org/c/zuul/zuul/+/814679 | ||
| - [zuul/zuul] 815154: Update test_inventory to be ZK-friendly https://review.opendev.org/c/zuul/zuul/+/815154 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed on behalf of Simon Westphahl: | 20:55 | |
| - [zuul/zuul] 812452: Store build sets in Zookeeper https://review.opendev.org/c/zuul/zuul/+/812452 | ||
| - [zuul/zuul] 812467: Add support for sharded ZKObjects https://review.opendev.org/c/zuul/zuul/+/812467 | ||
| - [zuul/zuul] 812673: Store RepoFiles for a build set in Zookeeper https://review.opendev.org/c/zuul/zuul/+/812673 | ||
| - [zuul/zuul] 813805: Remove project pipeline config from queue item https://review.opendev.org/c/zuul/zuul/+/813805 | ||
| - [zuul/zuul] 813809: Lookup event class names from global symbol table https://review.opendev.org/c/zuul/zuul/+/813809 | ||
| - [zuul/zuul] 813826: Store and resolve queue item's ahead/behind refs https://review.opendev.org/c/zuul/zuul/+/813826 | ||
| - [zuul/zuul] 814544: Cleanup stale items after refreshing a pipeline https://review.opendev.org/c/zuul/zuul/+/814544 | ||
| - [zuul/zuul] 814570: Reference active change queues in pipeline state https://review.opendev.org/c/zuul/zuul/+/814570 | ||
| - [zuul/zuul] 814571: Update pipeline state when modifying attributes https://review.opendev.org/c/zuul/zuul/+/814571 | ||
| - [zuul/zuul] 814772: Allow passing extra attributes to ZKObject.fromZK https://review.opendev.org/c/zuul/zuul/+/814772 | ||
| - [zuul/zuul] 814862: Bail out when a project moves between connections https://review.opendev.org/c/zuul/zuul/+/814862 | ||
| - [zuul/zuul] 814773: Move re-enqueue to pipeline processing https://review.opendev.org/c/zuul/zuul/+/814773 | ||
| - [zuul/zuul] 814899: Delete old build sets immediately https://review.opendev.org/c/zuul/zuul/+/814899 | ||
| - [zuul/zuul] 815309: Cancel jobs before resetting builds https://review.opendev.org/c/zuul/zuul/+/815309 | ||
| - [zuul/zuul] 815111: Store builds in Zookeeper https://review.opendev.org/c/zuul/zuul/+/815111 | ||
| - [zuul/zuul] 815276: Add change queues to change queue managers https://review.opendev.org/c/zuul/zuul/+/815276 | ||
| - [zuul/zuul] 815277: Refresh pipelines before checking for empty queues https://review.opendev.org/c/zuul/zuul/+/815277 | ||
| - [zuul/zuul] 815428: Fix GitHub PR (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815428 | ||
| - [zuul/zuul] 815429: Add missing logger to Build and BuildSet classes https://review.opendev.org/c/zuul/zuul/+/815429 | ||
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| - [zuul/zuul] 815495: Fix Gerrit change (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815495 | ||
| - [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | ||
| @clarkb:matrix.org | I'll try to start reviewing 812452 once I get dialed into this meeting and I don't have anything I need to pay too much attention to | 20:55 |
| @tobias.henkel:matrix.org | corvus: do we have an idea how large a job graph can be? (regarding 813913) | 21:01 |
| @jim:acmegating.com | tobiash: data is basically just list of job names, so i don't expect them to be very large in practice | 21:02 |
| @tobias.henkel:matrix.org | ah yeah, just noticed it's just the names | 21:03 |
| @tobias.henkel:matrix.org | I had the frozen jobs in mind | 21:03 |
| @jim:acmegating.com | they're a change or two later :) | 21:03 |
| @ecsantos:matrix.org | Clark: Weird, it checks if it's a file or a string [1] and decide between catting or echoing, I can `ls -l /var/lib/zuul/.ssh/id_rsa.pub` and it's clearly a file | 21:03 |
| [1] https://opendev.org/openstack/project-config/src/branch/master/nodepool/elements/zuul-worker/extra-data.d/60-zuul-user#L11 | ||
| @ecsantos:matrix.org | * Clark: Weird, it checks if it's a file or a string [1] and decides between catting or echoing, I can `ls -l /var/lib/zuul/.ssh/id_rsa.pub` and it's clearly a file | 21:04 |
| [1] https://opendev.org/openstack/project-config/src/branch/master/nodepool/elements/zuul-worker/extra-data.d/60-zuul-user#L11 | ||
| @ecsantos:matrix.org | OpenDev uses the string, the Software Factory folks use the path [2] | 21:06 |
| [2] https://softwarefactory-project.io/cgit/config/tree/nodepool/static_config/nodepool-builder.yaml#n128 | ||
| @clarkb:matrix.org | If I had to guess it is going to be a mount/chroot issue | 21:13 |
| @tobias.henkel:matrix.org | corvus: did you see swest's comment on 814281? | 21:14 |
| @jim:acmegating.com | tobiash: nope totally missed that. you agree with that? i think i do. | 21:16 |
| @tobias.henkel:matrix.org | yepp, I agree | 21:17 |
| @jim:acmegating.com | on it | 21:17 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: | 21:22 | |
| - [zuul/zuul] 814281: Remove toDict from FrozenJob https://review.opendev.org/c/zuul/zuul/+/814281 | ||
| - [zuul/zuul] 814243: Make FrozenJob a ZKObject https://review.opendev.org/c/zuul/zuul/+/814243 | ||
| - [zuul/zuul] 814329: Implement frozen job serialization/deserialization https://review.opendev.org/c/zuul/zuul/+/814329 | ||
| - [zuul/zuul] 814679: Store FrozenJob data in separate znodes https://review.opendev.org/c/zuul/zuul/+/814679 | ||
| - [zuul/zuul] 815154: Update test_inventory to be ZK-friendly https://review.opendev.org/c/zuul/zuul/+/815154 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed on behalf of Simon Westphahl: | 21:22 | |
| - [zuul/zuul] 814544: Cleanup stale items after refreshing a pipeline https://review.opendev.org/c/zuul/zuul/+/814544 | ||
| - [zuul/zuul] 814570: Reference active change queues in pipeline state https://review.opendev.org/c/zuul/zuul/+/814570 | ||
| - [zuul/zuul] 814571: Update pipeline state when modifying attributes https://review.opendev.org/c/zuul/zuul/+/814571 | ||
| - [zuul/zuul] 814772: Allow passing extra attributes to ZKObject.fromZK https://review.opendev.org/c/zuul/zuul/+/814772 | ||
| - [zuul/zuul] 814862: Bail out when a project moves between connections https://review.opendev.org/c/zuul/zuul/+/814862 | ||
| - [zuul/zuul] 814773: Move re-enqueue to pipeline processing https://review.opendev.org/c/zuul/zuul/+/814773 | ||
| - [zuul/zuul] 814899: Delete old build sets immediately https://review.opendev.org/c/zuul/zuul/+/814899 | ||
| - [zuul/zuul] 815309: Cancel jobs before resetting builds https://review.opendev.org/c/zuul/zuul/+/815309 | ||
| - [zuul/zuul] 815111: Store builds in Zookeeper https://review.opendev.org/c/zuul/zuul/+/815111 | ||
| - [zuul/zuul] 815276: Add change queues to change queue managers https://review.opendev.org/c/zuul/zuul/+/815276 | ||
| - [zuul/zuul] 815277: Refresh pipelines before checking for empty queues https://review.opendev.org/c/zuul/zuul/+/815277 | ||
| - [zuul/zuul] 815428: Fix GitHub PR (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815428 | ||
| - [zuul/zuul] 815429: Add missing logger to Build and BuildSet classes https://review.opendev.org/c/zuul/zuul/+/815429 | ||
| - [zuul/zuul] 815450: Create bundle items during queue deserialization https://review.opendev.org/c/zuul/zuul/+/815450 | ||
| - [zuul/zuul] 815495: Fix Gerrit change (de-)serialization https://review.opendev.org/c/zuul/zuul/+/815495 | ||
| - [zuul/zuul] 815278: DNM: execute tests with two schedulers https://review.opendev.org/c/zuul/zuul/+/815278 | ||
| @tobias.henkel:matrix.org | corvus: comment on 814243 | 21:24 |
| -@gerrit:opendev.org- Zuul merged on behalf of James E. Blair https://matrix.to/#/@jim:acmegating.com: [zuul/zuul] 815531: Increase timeout in test_build_request https://review.opendev.org/c/zuul/zuul/+/815531 | 21:27 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 810920: Store change queues in Zookeeper https://review.opendev.org/c/zuul/zuul/+/810920 | 21:27 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 811422: Save and restore bundle with item in Zookeeper https://review.opendev.org/c/zuul/zuul/+/811422 | 21:27 | |
| @jim:acmegating.com | tobiash: replied | 21:27 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul] 815406: Spec outline for ARM64 wheels https://review.opendev.org/c/zuul/zuul/+/815406 | 21:30 | |
| @clarkb:matrix.org | Why are we using `o = cls.__new__(cls)` rather than just `o = cls()` ? | 21:31 |
| @clarkb:matrix.org | we don't seem to be relying on any special `__new__` behavior, we just need a new object? | 21:32 |
| @jim:acmegating.com | Clark: which change? | 21:33 |
| @clarkb:matrix.org | corvus: https://review.opendev.org/c/zuul/zuul/+/812452/14/zuul/model.py | 21:33 |
| @clarkb:matrix.org | is it because we are trying to avoid calling `__init__` that is probably it | 21:34 |
| @tobias.henkel:matrix.org | corvus: thx, switched to +2 | 21:34 |
| @clarkb:matrix.org | though it seems __init__ is called if __new__ returns an object of type cls | 21:35 |
| @clarkb:matrix.org | * though it seems `__init__` is called if `__new__` returns an object of type cls | 21:35 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 815558: Replace asserts with exceptions https://review.opendev.org/c/zuul/zuul/+/815558 | 21:36 | |
| @jim:acmegating.com | tobiash: ^ there's the followup | 21:36 |
| @clarkb:matrix.org | corvus: not to completely distract from the work here but I'm noticing that opendev's zuul status page seems to update slowly | 21:37 |
| @tobias.henkel:matrix.org | Clark: in our system that's usually a sign of a busy scheduler | 21:37 |
| @clarkb:matrix.org | hrm ya there is a backlog of events | 21:38 |
| @jim:acmegating.com | Clark: that __init__ method has a bunch of required arguments which obviously aren't being supplied if it is being called | 21:39 |
| @clarkb:matrix.org | corvus: ya I suspect it has to do with trying to bypass `__init__` but I'm not yet sure if that is working as intended | 21:39 |
| @jim:acmegating.com | Clark: -> #opendev for the other tihng | 21:40 |
| @clarkb:matrix.org | the python docs mention object construction which maybe calling it explicitly like this bypasses and then `__init__` isn't called. If that is the case then I guess this code is correct. not sure if that deserves a comment though | 21:41 |
| @jim:acmegating.com | Clark: it's not counter-intuitive and it's not possible to write a different way, so i personally don't think i'd ding it for a lack of comment. i think the docs are saying that if you do Class() then python will call __new__ and __init__. so this is just an alternate constructor which does not call init | 21:46 |
| @clarkb:matrix.org | ya the python docs are the problem here I think. They don't explain the implicit behavior there very explicitly | 21:47 |
| @jim:acmegating.com | ++ i read that twice :) | 21:47 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed on behalf of Felix Edel: [zuul/zuul] 760807: UI: Add components page https://review.opendev.org/c/zuul/zuul/+/760807 | 21:50 | |
| @tobias.henkel:matrix.org | corvus: is a test race fix that should be prioritized? | 22:03 |
| @tobias.henkel:matrix.org | * corvus: is 815154 a test race fix that should be prioritized? | 22:03 |
| @jim:acmegating.com | tobiash: nope, is just prep for next change in series | 22:04 |
| @tobias.henkel:matrix.org | gotcha | 22:04 |
| @clarkb:matrix.org | one thing I'm struggling with in these reviews is knowing when it is ok to create a new context and when we should error if a context already exists | 22:06 |
| @clarkb:matrix.org | Depending on how the context is used to modify attributes we seem to do one or the other thing and it isn't clear to me if we've got a method for determining when to use one or the other approach | 22:07 |
| @clarkb:matrix.org | Left some minor things on https://review.opendev.org/c/zuul/zuul/+/812452 but +2'd as I don't think any are worthy of a -1 | 22:09 |
| @jim:acmegating.com | Clark: we almost never make a new context -- that's mostly done in the pipeline manager when it start processing a pipeline. once we have that context, we pass it around and alter object attributes within that existing context. that's either done via "with activeContext(context)" or obj.updateAttributes(context). | 22:15 |
| @tobias.henkel:matrix.org | corvus: q on 815111 | 22:15 |
| @clarkb:matrix.org | ya it seemed like there was a method it just wasn't super clear to me. Basically sounds like always prefer the existing context. If you're creating a context double check there isn't already one in place? | 22:16 |
| @jim:acmegating.com | Clark: replied on 452 | 22:19 |
| @jim:acmegating.com | tobiash: and replied on 815111 | 22:22 |
| @jim:acmegating.com | (i think 815111 is a correct interpretation of the current code; i'm running tests now to see if we can drop that assignment) | 22:24 |
| @jim:acmegating.com | (but if we do, probably makes sense as its own change) | 22:24 |
| @tobias.henkel:matrix.org | thanks for the explanation | 22:25 |
| @tobias.henkel:matrix.org | I think I'm done for today :) | 22:25 |
| @jim:acmegating.com | tobiash: thanks! i'll spend the rest of the day rechecking ;) | 22:25 |
| @tobias.henkel:matrix.org | I guess simon will continue rechecking tomorrow then ;) | 22:26 |
| @jim:acmegating.com | follow-the-sun rechecking | 22:27 |
| @clarkb:matrix.org | corvus: question on https://review.opendev.org/c/zuul/zuul/+/812467 | 22:30 |
| @jim:acmegating.com | Clark: that would be in case a null value gets written? | 22:32 |
| @jim:acmegating.com | which maybe would happen if we crash after truncate? | 22:33 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 815565: Remove unecessary assignment in re-enqueue https://review.opendev.org/c/zuul/zuul/+/815565 | 22:37 | |
| @jim:acmegating.com | tobiash: even thought you're sleeping now, i think we can remove that assignment ^ | 22:38 |
| @clarkb:matrix.org | yes would be for a null value. I think the current behavior is correct if we get actual invalid data back somehow. But for a null value I think that may be a special case? | 22:38 |
| @clarkb:matrix.org | special case of invalid data | 22:38 |
| @jim:acmegating.com | Clark: ack. want me to update or followup? | 22:38 |
| @clarkb:matrix.org | its probably ok to do a followup for that? the existing exception handler should catch it in the current code and we probably won't completely break? | 22:39 |
| @jim:acmegating.com | Clark: yeah. i also think it should never happen -- i believe there are state machines which aren't set to complete until that value is fully written out. so i think it's a belt-and-suspenders change. | 22:42 |
| @jim:acmegating.com | (i mean, that's not in the class itself, it's just that in all the places it's used, we either completely write out the value, or fail to update a state machine which will cause the next scheduler to generate and write out the value again) | 22:43 |
| @jim:acmegating.com | hrm. part of me wonders if we should leave that to error out. | 22:44 |
| @jim:acmegating.com | Clark: i'm actually starting to think that an exception in that case might be better. | 22:44 |
| @clarkb:matrix.org | ya I could see not treating null as special and just raise the error | 22:46 |
| @clarkb:matrix.org | and I expect that already happens since the deserialize should fail. But maybe we should more explicitly raise instead? | 22:46 |
| @jim:acmegating.com | maybe i'll regret this, but i'm inclined to just let it do its thing :) | 22:47 |
| @clarkb:matrix.org | ok | 22:48 |
| @jim:acmegating.com | happily the concurrency-1 change is now at head :) | 23:00 |
| @clarkb:matrix.org | corvus: https://review.opendev.org/c/zuul/zuul/+/812750/10/zuul/rpclistener.py line 490 what is that params[zuul][buildset] = None doing? | 23:16 |
| @clarkb:matrix.org | also handle_project_freeze_job doesn't seem to be called anywhere? | 23:16 |
| @clarkb:matrix.org | oh nevermind we getattr to call those functions | 23:17 |
| @jim:acmegating.com | Clark: i think the buildset arg is zeroed out because otherwise it's a random uuid which is useless to the caller (rest api user) and messes up the test assertions. | 23:18 |
| @clarkb:matrix.org | I see | 23:18 |
| @clarkb:matrix.org | similar to how the uuid is set | 23:18 |
| @clarkb:matrix.org | approved | 23:18 |
| @clarkb:matrix.org | corvus: https://review.opendev.org/c/zuul/nodepool/+/815545 is an easy followup to the nodepool dib stuff from yesterday if you have a moment | 23:22 |
| @jim:acmegating.com | ++ | 23:23 |
| @jim:acmegating.com | hrm, the concurrency change failed when building the docker image. | 23:28 |
| @clarkb:matrix.org | I wonder if we need to stop approving things and pushing giant stacks until we land a few fixes? we mgiht just be thrashing the docker apis in addition to zuul queues | 23:30 |
| @jim:acmegating.com | i'm keeping an eye out for that, but i don't know what failed there. i don't think it was docker api. | 23:30 |
| @clarkb:matrix.org | huh | 23:30 |
| @jim:acmegating.com | (but i'm not sure; it just died here: https://zuul.opendev.org/t/zuul/build/4a7022d384b34a2f9180bedd8977afb1/log/job-output.txt#965 | 23:31 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 815196: Use an election for scheduler periodic stats https://review.opendev.org/c/zuul/zuul/+/815196 | 23:32 | |
| @jim:acmegating.com | resolved a simple merge conflict ^ | 23:32 |
| @jim:acmegating.com | mhu: qq on https://review.opendev.org/736968 | 23:43 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!