| SpamapS | jamielennox: with the websocket stuff we're doing, I wonder if we should just bypass wsgi altogether and make our server respond via HTTP. | 00:07 |
|---|---|---|
| jamielennox | SpamapS: for zuul? that's kind of what we're doing at the moment | 00:16 |
| jamielennox | the webapp is in the same executable as the scheduler | 00:16 |
| jamielennox | for nodepool i had a review that split it out a while ago because the webapp could just query zk for the state and report on that | 00:17 |
| jamielennox | but yea for zuul, if you're going to end up RPC-ing the scheduler anyway i'm not sure if there's a great advantage in splitting it out yet | 00:17 |
| jlk | this is one of those things where I pulled on a thread, and it's unraveling the sleeve of my sweater. | 00:19 |
| jamielennox | for a high-ish traffic zuul i would probably want to put a haproxy/kong caching layer in front of zuul anyway | 00:20 |
| jamielennox | for status.json | 00:20 |
| jlk | YAY I made it reconfigure on github push event! | 00:23 |
| jeblair | jamielennox: each time zuul recalculates the stack of changes in gate, a queueitem gets a new buildset. so you'd typically just want to report the last one to the user for logs. that's the approach you're currently taking and i think that's fine. i could *also* see someone wanting not to save logs from old buildsets and actually just only write/link to a single location for the change. giving the queue item a uuid may be useful in that case, ... | 00:24 |
| jeblair | ... but i'd rather make sure we can handle the buildset uuid case first, so lets stick with that for now, and if you/someone wants to add queueitem uuid later we can. | 00:24 |
| SpamapS | yeah, HTTP caching status.json in a sane way for many clients should be relatively straight forward | 00:32 |
| jeblair | even today, we have it cached by apache | 00:33 |
| jamielennox | jeblair: in a way like we could/should remove the in-python caching layer? | 00:34 |
| jamielennox | just make it a deployment note that you should cache above it | 00:34 |
| jeblair | jamielennox: i think the current cache deals with the fact that the webapp only works on the whole status json blob, even when you are requesting info on a single change, it pulls that out of the whole thing. | 00:35 |
| jeblair | jamielennox: so the http cache is better suited to multiple clients all fetching status.json | 00:36 |
| jeblair | jamielennox: and the internal cache reduces the cost of status/1234 followed by status/5678 | 00:36 |
| jeblair | (i'm sure all of this could be thought out better holistically, but at least for the moment, i think they both serve a purpose) | 00:37 |
| jamielennox | sounds good | 00:38 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: executor: add support for custom ansible_port https://review.openstack.org/468710 | 00:42 |
| SpamapS | jeblair: I kind of like that everybody gets the whole status.json precisely because you can push that thing all the way out to the edges of the HTTP caching world very easily with a few seconds long expires header. | 00:49 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Log and collect stats for events consistently https://review.openstack.org/468664 | 00:49 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add support for defining groups in nodesets https://review.openstack.org/467611 | 00:49 |
| SpamapS | and at what, 110kB for all of openstack's CI right now.. seems far more efficient to just keep sending it out to people once per second than doing work on the server side. | 00:52 |
| jeblair | SpamapS: yeah, that part's not bad. i think the first improvement might be to eliminate the need for the internal cache when queried for a change directly. that sounds similar to what we'll end up with for log streaming maybe. | 00:53 |
| jeblair | SpamapS: though in the future, a websocket stream that gives you updates to the whole view could make the status page *much* more efficient/responsive. | 00:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Write inventory as yaml not ini https://review.openstack.org/467634 | 00:56 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Put variables into the inventory https://review.openstack.org/467635 | 00:58 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: sql-connection: make _setup_tables staticmethod https://review.openstack.org/466455 | 01:01 |
| jamielennox | i would expect the websocket stream for status page to be possible after zuul starts storing state in zk, before that i worry about load | 01:07 |
| *** Shrews has quit IRC | 02:20 | |
| *** Shrews has joined #zuul | 02:23 | |
| SpamapS | jamielennox: or we could implement this.. https://tools.ietf.org/id/draft-ietf-hybi-websocket-multiplexing-01.txt | 03:31 |
| SpamapS | assuming it could be revived :) | 03:33 |
| SpamapS | https://tools.ietf.org/html/draft-ietf-hybi-websocket-multiplexing-11 hmm.. looks like it died in 2014 :( | 03:34 |
| SpamapS | or we could just make it HTTP/2 only | 03:34 |
| SpamapS | which has basically the same feature | 03:35 |
| *** smyers has quit IRC | 04:32 | |
| *** smyers has joined #zuul | 05:04 | |
| jhesketh | jeblair: yep, that's fine :-) (re abandons) | 05:53 |
| openstackgerrit | Jamie Lennox proposed openstack-infra/nodepool feature/zuulv3: Allow loading logging config from yaml https://review.openstack.org/445656 | 06:01 |
| *** isaacb has joined #zuul | 06:24 | |
| *** hashar has joined #zuul | 07:38 | |
| *** hashar has quit IRC | 07:38 | |
| *** hashar has joined #zuul | 07:39 | |
| *** jamielennox is now known as jamielennox|away | 08:50 | |
| *** jamielennox|away is now known as jamielennox | 09:10 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: executor: add support for custom ansible_port https://review.openstack.org/468710 | 10:43 |
| *** hashar has quit IRC | 10:44 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: executor: add support for custom ansible_port https://review.openstack.org/468710 | 11:04 |
| *** jkilpatr has joined #zuul | 11:10 | |
| *** hashar has joined #zuul | 12:15 | |
| *** isaacb has quit IRC | 12:16 | |
| *** isaacb has joined #zuul | 12:21 | |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Add boot-from-volume support for nodes https://review.openstack.org/464283 | 12:35 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Re-add the ability to set username on zuul-executor https://review.openstack.org/446308 | 12:36 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove pipeline argument from various report fncts https://review.openstack.org/464252 | 12:37 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Increase file permissions around generate keys https://review.openstack.org/461218 | 12:38 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use ssh for git-upload-pack https://review.openstack.org/436802 | 12:38 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Bail with an error on a non-existent jobroot_dir https://review.openstack.org/469524 | 12:39 |
| *** hashar has quit IRC | 12:42 | |
| *** hashar has joined #zuul | 13:27 | |
| mordred | Shrews: I'm digging in to the log stream issue and think I've got a solution | 13:43 |
| mordred | Shrews: I may need to steal some of your finger protocol work though :) | 13:43 |
| *** hashar has quit IRC | 13:44 | |
| jlk | mordred: while you're digging into there, I found an issue where subsequent starts of zuul-executor get a traceback on starting the log streamer | 13:47 |
| jlk | start once, okay. stop. start again, traceback. | 13:47 |
| mordred | jlk: AWESOME | 13:47 |
| jlk | See the top of http://paste.openstack.org/show/611566/ | 13:47 |
| jlk | wait no | 13:48 |
| mordred | jlk: yah - that seems lke it can't connect to the scheduler | 13:48 |
| jlk | http://paste.openstack.org/show/611708/ | 13:48 |
| jlk | that's the one | 13:48 |
| Shrews | mordred: to what log stream issue are you referring? | 13:50 |
| * jlk has to get the kids ready for school | 13:50 | |
| mordred | Shrews: the ansible callback pluign that fetches the log content from the remote node and then writes it to the ansible_log file | 13:51 |
| mordred | Shrews: it currently only gets the first task's worth of data | 13:51 |
| Shrews | mordred: oh, well it's not really stealing since most of that code is your code originally | 13:53 |
| Shrews | jlk: i do not think you have the latest code there | 13:54 |
| Shrews | jlk: oh, that's on startup. hrm, why is it going through shutdown then? | 13:55 |
| Shrews | seems to be erroring in the bind/activate portion of socketserver: https://hg.python.org/cpython/file/2.7/Lib/SocketServer.py#l415 | 14:00 |
| Shrews | i explicitly set allow_reuse_address so not sure what could be happening there | 14:01 |
| jeblair | jlk: whoops, we missed something on https://review.openstack.org/471518 comments inline | 14:17 |
| jeblair | SpamapS: https://review.openstack.org/462677 is an easy +3 i hope | 14:19 |
| Shrews | jlk: any chance you're not starting executor as root? i get that error (preceeded by a Permission Denied error due to the user switch) when not starting as root | 14:21 |
| jlk | oh, yea it's non-root | 14:22 |
| jlk | which is a goal | 14:22 |
| Shrews | umm, yeah. can't do that now :( | 14:23 |
| jlk | awwwwwwwwwwwwww | 14:23 |
| jlk | do not like running as "root" in containers :( | 14:23 |
| jeblair | you can set [executor] finger_port=X now | 14:24 |
| jeblair | in zuul.conf | 14:24 |
| Shrews | well, it's only started as root, but su's to the 'zuul' user (or whatever user you select) ASAP at startup | 14:24 |
| jeblair | that too ^ | 14:24 |
| jeblair | it drops privileges asap | 14:24 |
| Shrews | finger port shouldn't matter here. privileges are dropped right after the port is grabbed | 14:25 |
| jlk | hrm, okay. | 14:26 |
| jlk | I guess i was wrong about it being a second-startup bug, I was focused on something else. | 14:26 |
| jeblair | right, i'm saying two options: 1) use the standard finger port and drop privileges; or 2) change the finger port to > 1024 and start as non-root. note with 2, finger won't work. :) | 14:27 |
| jeblair | however, i think once we have the mutexer running (on the standard finger port), we could probably make #2 the standard behavior -- users generally shouldn't need to hit the executors directly. | 14:28 |
| * Shrews afk for a bit for an appointment | 14:29 | |
| mordred | jeblair: I may need a little help spotting a bug in the patch I'm working on | 14:32 |
| *** dkranz has joined #zuul | 14:33 | |
| jeblair | mordred: i read that as "I may need a little help getting a bug into the patch I'm working on". but i just woke up. :) | 14:33 |
| jeblair | (you just type "recheck" until you get the bug in!) | 14:33 |
| mordred | jeblair: :) so -the tl;dr on it is that I'm making the streaming from node to exector be unique per-task. the "start a single streaming daemon process" is fraught with lifecycle peril. I've got it all pretty much plumbed together, but it's hanging so I think I'm missing something in the read/write handoff | 14:34 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: WIP Make log streaming to executor per-task https://review.openstack.org/471799 | 14:35 |
| mordred | jeblair: there's what I've got so far - it's basically using the finger protocol to the nodes now, so we send a unique id into the command invocation which affects log file, then we send that id over the wire on connection | 14:36 |
| jeblair | mordred: i can see from skimming it, i will definitely need to look at this post-breakfast. can i get back to you in 30 mins? :) | 14:36 |
| mordred | jeblair: yes, absolutely | 14:36 |
| mordred | I'll keep poking and notify you if I find it | 14:37 |
| jeblair | kk | 14:37 |
| mordred | jeblair: ok. there may not be a problem- I was testing against a host with firewall rules :) | 14:57 |
| jeblair | mordred: yay breakfast helped! | 15:21 |
| mordred | jeblair: awesome. I actually do think I need help - but let me putup the new patch with fixes | 15:24 |
| jeblair | mordred: ok. i've been familiarizing myself with the changes. | 15:25 |
| mordred | jeblair: yay! the main bug I foudis that when I copy-pastad, I didn't change self.request to self.socket | 15:25 |
| jeblair | that'll do it | 15:25 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: WIP Make log streaming to executor per-task https://review.openstack.org/471799 | 15:26 |
| mordred | jeblair: k. that's updated | 15:26 |
| jeblair | no more default callback module? | 15:26 |
| mordred | jeblair: no - I copied that in originally because I was having import errors | 15:27 |
| mordred | jeblair: but I'm not having those anymore | 15:27 |
| mordred | jeblair: I'll chase that down later I think | 15:27 |
| jeblair | good, i didn't want to review that. :) | 15:27 |
| jeblair | mordred: what's the current problem? (also, i left a comment) | 15:33 |
| jeblair | SpamapS: i left a comment on https://review.openstack.org/468667 | 15:34 |
| mordred | jeblair: the current problem was that I wasn't seeing connections - but I finally just got a test node to see the connections- so let me see what else is wrong | 15:39 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove ansible files at startup before copy https://review.openstack.org/471518 | 15:42 |
| jlk | ugh, I'm trying ot use a single docker file for all the zuul bits, but only the executor needs to run as root. that's... hrm. | 15:47 |
| jlk | I wonder if this can be set in docker-compose | 15:47 |
| mordred | jeblair: ok. yah - so, the server side is seeing the connection, but it's not seeing the incoming data | 15:50 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix race in test_crd_gate_unknown https://review.openstack.org/471833 | 15:52 |
| jeblair | mordred: looking | 15:54 |
| jeblair | mordred: spotted; left comment | 15:56 |
| mordred | jeblair: YES! thank you - that totally works | 16:00 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Reject some untrusted config objects https://review.openstack.org/470442 | 16:01 |
| *** isaacb has quit IRC | 16:02 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Factor out duplicate code in executor repo tests https://review.openstack.org/471492 | 16:02 |
| SpamapS | jeblair: ty! | 16:08 |
| SpamapS | jlk: couldn't you have a 'FROM zuul-base'? | 16:09 |
| SpamapS | jlk: zuul-base would just setup software... | 16:09 |
| jlk | I could, but that'd be multiple docker files, which is :( | 16:09 |
| *** isaacb has joined #zuul | 16:12 | |
| *** jkilpatr has quit IRC | 16:36 | |
| *** jkilpatr has joined #zuul | 16:40 | |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove ansible files at startup before copy https://review.openstack.org/471518 | 16:47 |
| mordred | jeblair: k. that all works now -updated patch coming, thanks for the help | 16:48 |
| *** dkranz has quit IRC | 16:52 | |
| *** dkranz has joined #zuul | 16:57 | |
| jlk | wtf... /var/lib/zuul/ansible/zuul/ansible/ | 17:03 |
| SpamapS | zuul went to plaid | 17:03 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove ansible files at startup before copy https://review.openstack.org/471518 | 17:04 |
| jeblair | yeah, i'm not happy either, but it works | 17:07 |
| jeblair | '/var/lib/zuul' -- stuff for zuul | 17:07 |
| jeblair | '/var/lib/zuul/ansible' -- ansible stuff for zuul | 17:07 |
| *** isaacb has quit IRC | 17:08 | |
| jeblair | then "zuul/ansible" follows because we need to be able to import "zuul.ansible.foo" after we add '/var/lib/zuul/ansible' to pythonpath | 17:08 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: WIP: add log streaming test https://review.openstack.org/471079 | 17:08 |
| jeblair | the 'ansible' in '/var/lib/zuul/ansible' is negotioable -- we could call it 'python' instead. maybe that would help. is /var/lib/zuul/python/zuul/ansible ? | 17:10 |
| jeblair | ^ + any better ? | 17:10 |
| pabelanger | lib/python/zuul/ansible ? | 17:11 |
| jeblair | pabelanger: /var/lib/zuul/lib/python/zuul/ansible? | 17:11 |
| pabelanger | ya, doesn't python libraries go into <home>/lib/python? | 17:12 |
| jeblair | (it's an extra layer, and now we have 2 libs and 2 zuuls (but only one ansible)). <shrug> | 17:13 |
| pabelanger | I am okay with /var/lib/zuul/ansible myself, I wasn't sure if python path looked into lib/python by default | 17:13 |
| jlk | I don't really care, I just took a double-take | 17:15 |
| SpamapS | It may be better to call it /var/lib/zuul/nothingtoseehere | 17:16 |
| pabelanger | :) | 17:17 |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Allow loading logging config from yaml https://review.openstack.org/445656 | 17:18 |
| pabelanger | we should be able to launch our first job on ze01.o.o in the next 30mins | 17:18 |
| jeblair | my dashboard is at 56 changes (down from 138 yesterday) | 17:19 |
| jlk | woo! | 17:19 |
| pabelanger | jeblair: executors will still need SSH access to gerrit? | 17:49 |
| pabelanger | looking to see if we need to add the SSH key for gerrit or not | 17:50 |
| jeblair | pabelanger: yes, they are mergers | 17:57 |
| pabelanger | jeblair: right, let me make some changes | 17:58 |
| jlk | and you don't need mergers any more | 18:17 |
| pabelanger | we had some discussions on monday / friday to keep them I thought | 18:20 |
| jlk | oh I missed that. | 18:23 |
| jlk | last I talked, there weren't any gear tasks they did that executor couldn't also do | 18:23 |
| jlk | now for scale, you may want some of them to offset load on executors that do more. | 18:23 |
| pabelanger | Ya, I'm curious to see how the executors will do both ansible and merging. Right now 150-200 jobs is good spot for a launcher today | 18:24 |
| pabelanger | I'm thinking an executor might be less with merge operations too | 18:25 |
| jlk | probably. At some point for scale you may need config options to reduce the tasks that executors are allowed to do, to specifically send those jobs to merger and scale those up | 18:26 |
| pabelanger | Ya, we have 8 mergers today | 18:26 |
| jlk | currently it'll be first-come first-serve | 18:27 |
| jlk | but that may be an executor taking a merger task and leaving a merger idle | 18:27 |
| pabelanger | I suspect we'll need 10-15 executors once everything is cut over | 18:27 |
| SpamapS | My hope is that there can just be executors | 18:28 |
| SpamapS | they can just be dual purpose | 18:28 |
| SpamapS | and they won't waste resources doing that. | 18:28 |
| jlk | probably what should happen is executor config options that enable/disable specific gear tasks. And get rid of "merger" as a service all together | 18:28 |
| SpamapS | so go from 1 launcher and 8 mergers to 9 executors. | 18:28 |
| jlk | and then you have a pile of scale out executors that do merger tasks and fewer that do executor tasks | 18:28 |
| jlk | but don't do that until you have data showing you need it :D | 18:28 |
| SpamapS | jlk: the biggest thing is that if you separate merge from job running you either double merge load, or you have to have a way to move merge results to executors. | 18:29 |
| SpamapS | I guess that applies even if you have 8 executors doing it all | 18:29 |
| pabelanger | one thing I didn't do for ze01, is add a cinder volume. We'll have to do that I imagine | 18:29 |
| jlk | oh, right, because the executor is going to need that merged content locally | 18:30 |
| jlk | argh, something is not right with this fix to copying ansible bits around | 18:32 |
| jeblair | jlk: oh drat, i think i see it, and i think it's my fault | 18:35 |
| jeblair | jlk: left comment | 18:36 |
| jlk | thanks | 18:36 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove ansible files at startup before copy https://review.openstack.org/471518 | 18:53 |
| pabelanger | just seen ^ on ze01.o.o too | 18:53 |
| Shrews | jeblair: I'm having difficulty getting things synchronized when pausing ansible jobs using wait_for in test playbooks. waitForSettled() hangs with that, and I'm not quite sure what I need to change. | 18:56 |
| Shrews | err, waitUntilSettled | 18:56 |
| SpamapS | Shrews: so you should do your thing to signal it _before_ waitUntilSettled | 19:04 |
| SpamapS | should be like, release, inspect things before wait_for, signal wait_for to unpause, waitUntilSettled, inspect final result | 19:05 |
| Shrews | release? | 19:09 |
| SpamapS | Shrews: oh right you're not using hold jobs in build | 19:13 |
| SpamapS | Shrews: so in that case, just add events and start looking for the signal that the wait_for has been reached and you can do whatever it is you want to do while it's paused. | 19:13 |
| SpamapS | Shrews: signal in the broad sense... like.. a semaphore somewhere. | 19:13 |
| SpamapS | gah | 19:13 |
| SpamapS | like, a FILE ON DISK somewhere ;) | 19:14 |
| SpamapS | so few words | 19:14 |
| Shrews | SpamapS: ah, lemme try that. thx | 19:16 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Bail with an error on a non-existent jobroot_dir https://review.openstack.org/469524 | 19:17 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Make log streaming to executor per-task https://review.openstack.org/471799 | 19:17 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Strip double-timestamps from output log https://review.openstack.org/471890 | 19:17 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Update log to include expected data on task results https://review.openstack.org/471891 | 19:17 |
| mordred | jeblair: ok - I maybe went a little OCD on the logging this morning | 19:17 |
| Shrews | SpamapS: should I wait for self.builds to become populated too? b/c each build will have separate "semaphores" and i need the build object to know where it will be located | 19:18 |
| mordred | jeblair: that should fix things to actualy fetch ALL the logs and not just the logs for the first playbook as well as removing the double-timestamps for the shell tasks | 19:18 |
| Shrews | SpamapS: my testing thus far has self.builds as empty (possibly because of timing things) | 19:19 |
| SpamapS | Shrews: I see your predicament. And remind me why you're not using hold_jobs_in_build. | 19:21 |
| Shrews | SpamapS: b/c ansible jobs do not honor that (for reasons i expect jeblair can explain) | 19:21 |
| Shrews | and since i've restarted weechat, i lost that scrollback | 19:22 |
| jeblair | yeah, the hold_in_build is entirely a construct of the fake jobs | 19:22 |
| jeblair | waituntilsettled won't know that the (real) build has gotten to the sleep, so it won't be effective there -- it can only be used to wait until the jobs are finished. | 19:23 |
| jeblair | so i think SpamapS suggestion of waiting until a flag file appears is the way to go (then similarly, in the job, waiting for a flag file to continue) | 19:24 |
| jeblair | could probably use the same file -- have the job create the file. have the test wait until the file appears, then remove the file. have the job wait until the file disappears, then proceed. | 19:24 |
| SpamapS | Yeah that's what I was going to suggest exactly. | 19:24 |
| SpamapS | There is the matter of knowing where to find that file, especially if you want to pause multiple jobs at once. | 19:25 |
| Shrews | exactly | 19:25 |
| Shrews | i mean, we could create it outside the jobdir i guess. that way i don't need to know build specifics | 19:26 |
| jeblair | Shrews: or wait until self.builds is populated, then further wait for the file for each build | 19:28 |
| Shrews | jeblair: if i define 1 job, then len(self.builds) should be 1, right? | 19:29 |
| SpamapS | Yeah self.builds should get stuff in it, not the worst thing to busy wait a bit for it. | 19:29 |
| jeblair | yep | 19:30 |
| jeblair | and yep | 19:30 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Convert some leftover changeish mentions to ref https://review.openstack.org/465718 | 20:06 |
| jeblair | mordred: +2 on the first of your patches; i'll look at the second 2 when i get back | 20:09 |
| jlk | jeblair: is it safe to assume that if a gerrit event has a 'refUpdate' attribute that it is of event type ref-updated? | 20:19 |
| jlk | also, in your feedback on branch_updated things, why are you avoiding when the ref starts with "refs" ? is that only used for tags in gerrit, otherwise it's just the branch name? | 20:20 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Extend in-repo config update support to github [wip] https://review.openstack.org/471477 | 20:30 |
| jlk | so I think I addressed what you were asking for, and I've made the github code work, I just need to set up tests for this all. | 20:32 |
| *** dougbtv_ has joined #zuul | 20:34 | |
| *** dougbtv has quit IRC | 20:38 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove ansible files at startup before copy https://review.openstack.org/471518 | 20:40 |
| mordred | jeblair: I've got one more, that it would be nice to get Shrews to look at too | 21:11 |
| mordred | Shrews: (after copying code from you, I figured actually just having one copy of the code in the tree would be better than two) | 21:12 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Update log to include expected data on task results https://review.openstack.org/471891 | 21:13 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Strip double-timestamps from output log https://review.openstack.org/471890 | 21:13 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Move log streaming to share with zuul_console https://review.openstack.org/471921 | 21:13 |
| mordred | Shrews: ^^ https://review.openstack.org/471921 combines the two into mostly a single impl | 21:13 |
| mordred | SpamapS: although I'm not 100% sure the approach to dealing with the different types of files is correct - it might be better to make a base class and some subclasses to handle that little bit | 21:14 |
| mordred | gah | 21:20 |
| mordred | lemme do that real quick | 21:20 |
| *** dkranz has quit IRC | 21:24 | |
| *** fbouliane has quit IRC | 21:29 | |
| *** fbouliane has joined #zuul | 21:30 | |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Re-enable test_worker_update_metadata https://review.openstack.org/468667 | 21:46 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Re-enable test_merge_conflict_reports https://review.openstack.org/468670 | 21:46 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Re-enable test_merge_failure_reporters https://review.openstack.org/468668 | 21:46 |
| jlk | haha hahahahah hahahahahahahahaha | 21:51 |
| jlk | jeblair: so for gerrit, on a refUpdated event, the driver does not discover a list of changed files. | 21:52 |
| jlk | jeblair: in what scenario does a gerrit source refUpdate happen outside of a merge event? When does somebody push config directly in that still causes a gerrit event to happen? | 21:52 |
| jlk | yeah and the event stream from gerrit does not include file data on a refUpdate | 22:00 |
| jlk | so.... | 22:00 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Move log streaming to share with zuul_console https://review.openstack.org/471921 | 22:02 |
| jlk | damn, another bug. We're updating config for pushes to non-root branches. | 22:05 |
| mordred | jlk: you're finding all the fun stuff | 22:05 |
| jlk | this one isn't bad, because we only look at the master branch during reconfigure. We just cause the job if ANY branch changes with those files. | 22:06 |
| mordred | Shrews, jeblair: that version of https://review.openstack.org/471921 has less hacky if statements | 22:17 |
| jeblair | jlk: drat. perhaps sticking with change-merged for now is best. i suppose to handle pushes, we'll have to go and do git operations. let's put that on the backlog. | 22:28 |
| jlk | alrighty, I'll drop a TODO in there. | 22:28 |
| jlk | jeblair: so if an admin directly git pushes a change into a repo, gerrit notices this and sends out an event to the event stream? | 22:34 |
| *** jkilpatr has quit IRC | 22:34 | |
| jeblair | jlk: if they do it through gerrit, yes, that's a ref-updated event. that fires for tags as well (which are just about the same thing). also happens after a change-merged event (since merging a change will cause a ref to get updated). | 22:35 |
| jlk | how does one push through gerrit without going through a change? | 22:35 |
| jeblair | jlk: (if they git push to the repo on disk, gerrit won't know that. but no one should do that. :) | 22:35 |
| jeblair | jlk: it's an acl. we disable it in openstack. | 22:35 |
| jlk | oh okay. | 22:35 |
| jeblair | jlk: but if you enable it, you can "git push gerrit HEAD:master" or whatever and just update the repo. | 22:36 |
| jlk | kk | 22:36 |
| jeblair | (gerrit doesn't require bureaucracy, openstack does) | 22:36 |
| jeblair | mordred: your first 4 lgtm. zuul is not keen on the 5th. | 22:40 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Extend in-repo config update support to github https://review.openstack.org/471477 | 22:40 |
| jlk | ^^ that could use a real review now. There was already a test for dynamic reconfiguration in gerrit side (I know, I broke it), and there's a new github test. | 22:41 |
| jeblair | jlk: lgtm | 22:44 |
| *** openstack has joined #zuul | 23:16 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!