| jlk | hahahahah | 00:00 |
|---|---|---|
| jlk | so this patch I've worked the last hour or so on | 00:00 |
| jlk | turns out, you've done the work in the fix for full project name which just merged, and all that's left of my patch is a new set of tests, no code change. Guess I'll abandon that one! | 00:01 |
| jeblair | jlk: yay? :) are the tests useful? | 00:02 |
| jlk | probably not. | 00:02 |
| jeblair | okiedokie | 00:03 |
| jlk | I mean, it does test gerrt and github together in one tenant | 00:03 |
| jeblair | jlk: that sounds worthwhile | 00:03 |
| jlk | so... judgement call? I'll include the tests commit since they work, but feel free to nix it on review | 00:03 |
| jlk | jeblair: I've got another rebase about to happen. I'm not ignoring your review comments, I just needed to rebase on the bugfix from today, to land a few more of my patches. I'll get to your reviews soon | 00:19 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: support github pull request labels https://review.openstack.org/444511 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Allow github trigger to match on branches/refs https://review.openstack.org/445625 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Set filter according to PR/Change in URL https://review.openstack.org/446782 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Support for dependent pipelines with github https://review.openstack.org/445292 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Add basic Github Zuul Reporter. https://review.openstack.org/443323 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Configurable SSH access to GitHub https://review.openstack.org/444034 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Add 'push' and 'tag' github webhook events. https://review.openstack.org/443947 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Add 'pr-comment' github webhook event https://review.openstack.org/443959 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Support for github commit status https://review.openstack.org/444060 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Better merge message for GitHub pull reqeusts https://review.openstack.org/445644 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Test gerrit and github drivers in same tenant https://review.openstack.org/448257 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Encapsulate determining the event purpose https://review.openstack.org/445242 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Allow using webapp from connections https://review.openstack.org/439831 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Merge pull requests from github reporter https://review.openstack.org/444463 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Support GitHub PR webhooks https://review.openstack.org/439834 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: GitHub file matching support https://review.openstack.org/446113 | 00:22 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Log GitHub API rate limit https://review.openstack.org/446150 | 00:22 |
| *** dmellado has quit IRC | 01:00 | |
| *** _ari_ has quit IRC | 01:00 | |
| *** _ari_ has joined #zuul | 01:00 | |
| *** dmellado has joined #zuul | 01:02 | |
| *** TaylorH has joined #zuul | 01:53 | |
| *** yolanda has quit IRC | 05:28 | |
| *** yolanda has joined #zuul | 06:49 | |
| *** isaacb has joined #zuul | 07:06 | |
| *** yolanda has quit IRC | 07:25 | |
| *** yolanda has joined #zuul | 07:41 | |
| *** yolanda has quit IRC | 08:24 | |
| *** yolanda has joined #zuul | 08:41 | |
| *** yolanda has quit IRC | 09:26 | |
| *** yolanda has joined #zuul | 09:43 | |
| lennyb | Hi Team, does zuul support multiple gerrits ? Working with different gerrit servers and gerrit users? | 10:40 |
| *** jkilpatr has quit IRC | 10:45 | |
| *** jkilpatr has joined #zuul | 11:05 | |
| *** yolanda has quit IRC | 11:19 | |
| *** yolanda has joined #zuul | 11:34 | |
| *** dkranz has quit IRC | 11:57 | |
| *** dkranz has joined #zuul | 12:56 | |
| *** yolanda has quit IRC | 13:06 | |
| *** yolanda has joined #zuul | 13:23 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Allow using webapp from connections https://review.openstack.org/439831 | 13:44 |
| mordred | jeblair, jlk: left some responses to jeblair's comments on the older patchesets of a couple of those - there are some fun/execellent questions in there! | 13:51 |
| jlk | lennyb: yes it does. There is even a test in the tree for multiple gerrits. | 14:48 |
| *** isaacb has quit IRC | 14:59 | |
| *** isaacb has joined #zuul | 15:12 | |
| pabelanger | Today zuul for rdoproject.org does this, it connects both to review.rdoproject.org and review.o.o | 15:15 |
| *** yolanda has quit IRC | 15:31 | |
| lennyb | jlk, thanks, I will look for it | 15:36 |
| *** jlk has quit IRC | 16:01 | |
| *** jlk has joined #zuul | 16:02 | |
| *** jlk has quit IRC | 16:02 | |
| *** jlk has joined #zuul | 16:02 | |
| *** isaacb has quit IRC | 16:09 | |
| jeblair | mordred: thinking about your comments on 443947 -- while i'm open to some amount of helpful standardization, i don't think we should do too much help with 'merge'. there is a gerrit 'change-merged' event, and, all things being equal, that's the one we really want to use for post. if we could get the merge commit in there, it would be. we have our own reasons for not using it, but there are equally compelling reasons for using it despite that ... | 16:10 |
| jeblair | ... -- mostly so that you can make a job that reports back on the change. | 16:10 |
| clarkb | SpamapS: jlk are you able to run the full test suite successfully now? | 16:10 |
| jeblair | mordred: i'm more open to saying ref-updated is equivalent to push, but i want to stay away from confusing change-merged with ref-updated. | 16:11 |
| jeblair | mordred: but it seems like you were mostly advocating (ref-updated[refs/heads] / push_event[refs/heads]) -> push; (change-merged / pull_request[closed]) -> merge; and (ref-updated[refs/tags] / push_event[refs/tags]) -> tag | 16:13 |
| jeblair | mordred: if we do perform a mapping, that seems ideal. i confess i'm still very torn between straight passthrough vs mapping. | 16:17 |
| jeblair | mapping seems very user friendly (especially for those of us who will use both). gerrit and github triggers can share documentation about one set of actions. otoh, we will probably have some actions that only apply to one (github pull request review request removed, for one). i worry that once we get past the basics, it will actually become harder to reconcile different systems actions, and harder to document that as well. | 16:22 |
| jeblair | (also, i hesitate to invoke the 'naming' boogeyman, but "opened" may not be sufficiently descriptive, but as you point out, we can't call the mapped version of the event 'change' or 'pull_request' for fear of confusing people) | 16:23 |
| clarkb | drive by comment: chances are this is something you modify in config very rarely too. So extra overhead to be explicit per backend is low in the grand scheme of things | 16:24 |
| clarkb | and makes it easier to allow users to get what they want out of $backend | 16:24 |
| jeblair | mordred: so i think i'm still leaning slightly toward passthrough naming, but just making that as sensible and consistent as we can. in the earlier change, you suggested being even more mechanical with 'pull_request.closed'. i'd be okay with that too. i mean, we can even do "pull_request: {action: closed}". our config language does allow for that. | 16:28 |
| *** jamielennox is now known as jamielennox|away | 16:31 | |
| mordred | jeblair: nod. and yes- you are correct on your description of what I was advocating for in the mapping case | 16:43 |
| mordred | mapping ref-updated -> push and change-merged -> merge (or maybe not, maybe that one would still be change-merged / pull_request.closed - since the event is an event describing an action on the change/pull-request | 16:44 |
| mordred | jeblair: I don't feel strongly on either side, so if you're leaning more towards passthrough that works for me too | 16:45 |
| mordred | jeblair: I think my main argument for mapping is to make the language in this case a zuul language, so people can think about "push" and "tag" - neither of which have code review actions associated - rather than thinking of the mechanics their code review system/git system might use to accomplish that | 16:46 |
| mordred | like - outside of openstack, one could also maybe imagine people who want to do traditional post-merge CI with zuul (why???) - and for them they'd have a "push" to respond to with gh but a ref-updated for gerrit and that just seems weird | 16:48 |
| mordred | but I don't think it's extreme or bad weird | 16:48 |
| mordred | or problematic in any way - I think that ^^ is the sum total of any argument I can make in favor of mapping :) | 16:49 |
| jeblair | mordred: oh, so maybe a sort of mapping-light, where 'push' and 'tag' are mapped, but pull-request.* and change.* are not....? | 16:49 |
| SpamapS | clarkb: sorry your thing has been 2 or 3 layers down on my stack.. haven't gotten to it yet. | 16:50 |
| clarkb | SpamapS: its all merged now. Just wondering if people can run the tests lcoally now | 16:51 |
| clarkb | then I can raelly decide its solved | 16:51 |
| mordred | jeblair: yah | 16:55 |
| jeblair | mordred: i think one thing that keeps having me come back to pass-through is other triggers (email, irc, etc) -- would we try to normalize email and irc triggers?. probably not. though, honestly, that doesn't make a very compelling argument against just mapping push and tag in github and gerrit. | 16:57 |
| SpamapS | clarkb: well that makes it easy.. running now | 16:58 |
| SpamapS | oh hey we need to fix something in RecordingExecutorServer ... | 16:59 |
| SpamapS | http://paste.openstack.org/show/608107/ | 17:00 |
| SpamapS | if you play with python3, that happens | 17:00 |
| mordred | jeblair: yah - I'd imagine if we get email triggers that do stewart's kernel mailing-list patch workflow, there would likely want to be some sort of hybrid plugin that also knows how to poll a git repository and look for changes that could potentially emit push and tag events? | 17:02 |
| mordred | jeblair: I mean, that's WILD speculation since we haven't designed that backend at all | 17:02 |
| jeblair | mordred: yeah, i wrote a git driver for v3, but it doesn't have a trigger yet. it can only be used as a source. but it could be extended as you describe fairly easily. | 17:03 |
| mordred | jeblair: ++ | 17:03 |
| clarkb | SpamapS: the shutil in that paste is for python2.7. Perhaps related to why it breaks under python3? | 17:05 |
| mordred | I do think, no matter what we call them, it's clear to me that there are three different logical events that jobs may want to respond to: push (a new ref was added to the repo) change-merged/pr-closed (a proposed change was landed) and tag (a tag was created/deleted) | 17:05 |
| jeblair | mordred: push==a ref was created or its target was changed, right? | 17:06 |
| jeblair | (or deleted) | 17:06 |
| SpamapS | clarkb: !! Ran 241 (+165) tests in 288.339s (+284.268s) | 17:14 |
| SpamapS | nice | 17:14 |
| SpamapS | clarkb: that's quite amazing. :-D | 17:14 |
| SpamapS | clarkb: regarding the paste.. that's me running tox -epy27 and getting that streaming over and over because my py3 runs left __pycache__ dirs lying around | 17:15 |
| clarkb | cool | 17:15 |
| clarkb | SpamapS: ah | 17:15 |
| SpamapS | I think I have a patch | 17:15 |
| mordred | jeblair: yah - I tihnk | 17:15 |
| mordred | jeblair: so | 17:15 |
| jeblair | SpamapS: is there more bwap documentation than the readme? https://github.com/projectatomic/bubblewrap | 17:23 |
| jeblair | just wondering exactly what --dev does | 17:42 |
| *** jkilpatr_ has joined #zuul | 17:44 | |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Ignore directories when copying job dir components https://review.openstack.org/460234 | 17:45 |
| SpamapS | jeblair: I ported about 30% of bubblewrap to rust.. that's how I learned what it does. Was considering writing up more docs actually ;-) | 17:45 |
| jeblair | SpamapS: ++. :) in the mean time -- how are --proc and --dev special? | 17:45 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Add a finger protocol log streamer https://review.openstack.org/456721 | 17:46 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Allow for specifying root job directory https://review.openstack.org/456691 | 17:46 |
| *** jkilpatr has quit IRC | 17:46 | |
| SpamapS | jeblair: --proc just sticks a namespaced procfs in the chroot. The arg is the path under the chroot. | 17:46 |
| SpamapS | jeblair: I think --dev sets up a devpts. | 17:47 |
| * SpamapS reads code | 17:47 | |
| SpamapS | more than devpts | 17:48 |
| SpamapS | static const char *const devnodes[] = { "null", "zero", "full", "random", "urandom", "tty" }; | 17:48 |
| SpamapS | it makes those | 17:48 |
| SpamapS | bind mounts them specifically | 17:48 |
| jeblair | SpamapS: sweet; so that makes a safe version of /dev | 17:48 |
| jeblair | SpamapS: does the namespacing of procfs prevent escape that way? | 17:49 |
| SpamapS | and /dev/shm, stdio symlinks in proc.. | 17:49 |
| SpamapS | ptmx | 17:49 |
| SpamapS | jeblair: it should | 17:49 |
| jeblair | cool | 17:49 |
| SpamapS | jeblair: but it's probably a good idea to crank that down a bit and restrict access to the proc nodes one needs. | 17:50 |
| SpamapS | (with a MAC) | 17:50 |
| jeblair | SpamapS: i wonder if we can cause ansible not to require proc | 17:50 |
| SpamapS | jeblair: probably. the setup module on localhost is what wants it. | 17:50 |
| SpamapS | ssh needed /dev/null IIRC | 17:51 |
| jeblair | SpamapS: 'setup module on localhost' -- you aren't by chance referring to one of the modules we disabled, are you? | 17:51 |
| SpamapS | jeblair: IIRC that one is still enabled but perhaps not. | 17:52 |
| mordred | we did not disable it | 17:52 |
| jeblair | which module specifically is this? | 17:52 |
| SpamapS | fact gathering | 17:53 |
| SpamapS | most of the ansible_ vars are set in there | 17:53 |
| mordred | yah - fact gathering is perfomed by the setup module | 17:53 |
| mordred | which you can run on your local machine to see with "ansible localhost -m setup" | 17:53 |
| SpamapS | Wonder if you can disable it. | 17:53 |
| mordred | I mean, you'd have to set gather_facts to explicit in the config | 17:54 |
| mordred | which would then change the characteristics of running playbooks for most people in unexpected ways | 17:54 |
| mordred | that said - we could likely do a "if localhost: return {}" | 17:54 |
| mordred | so that it still functions but doesn't do as muchlocal introspection - since we're not expecting people to do a bunch of localhost operations | 17:55 |
| SpamapS | worth looking through them to see if we need any fo them. | 17:55 |
| mordred | and the ones we do expect them to do shouldn't be relying on a rich set of gathered facts | 17:55 |
| mordred | yah | 17:55 |
| SpamapS | in fact a lot of them would be useful in escaping ansible ;) | 17:56 |
| mordred | I'm not seeing any that seem particularly useful | 17:56 |
| * SpamapS tries it inside bwrap | 17:58 | |
| SpamapS | Wow you get _a lot_ less with bwrap | 17:58 |
| SpamapS | which makes me happy | 17:58 |
| SpamapS | http://paste.openstack.org/show/608114/ | 17:58 |
| SpamapS | with bwrap | 17:58 |
| SpamapS | oh interesting | 18:00 |
| jeblair | is that without proc? | 18:00 |
| SpamapS | jeblair: that's with proc | 18:00 |
| * SpamapS finds it interesting that lsb_release assumes Debian if it doesn't have a /etc/lsb-release | 18:01 | |
| SpamapS | oh wait no | 18:01 |
| SpamapS | jeblair: no /proc | 18:01 |
| SpamapS | I removed it at one point but forgot to remove the comment saying we need it ;) | 18:02 |
| SpamapS | so yeah that's likely why it shows so much less | 18:02 |
| SpamapS | and \o/ | 18:02 |
| jeblair | neato | 18:02 |
| SpamapS | "ansible_python_version": "3.5.2", | 18:04 |
| SpamapS | that surprises me | 18:04 |
| SpamapS | I didn't think it could use python3 | 18:04 |
| clarkb | latest ansible has experimental python3 support iirc | 18:10 |
| mordred | yah. | 18:15 |
| mordred | and it's not ansible itself that's the problem at the moment - the big issue is the modules | 18:15 |
| mordred | so I believe the ansible code is all py3 compat now - but a non-zero number of the modules are not | 18:16 |
| jeblair | clarkb: how do you feel about generally doing pass-through events, except splitting ref-updated into 'push' (refs/heads updated) and 'tag' (refs/tag updated)? on the one hand, it's us making synthetic events; on the other, it means that post and release pipelines can drop their negative lookahead regexes. | 18:26 |
| clarkb | I think if we do that it might be worth while implemetning that on top of continued support for ref-updated * | 18:29 |
| *** jkilpatr_ has quit IRC | 18:37 | |
| *** jkilpatr has joined #zuul | 18:39 | |
| SpamapS | jeblair: thanks for your comments on the bwrap thing. I wonder if I can just move the script into python as a driver. | 19:06 |
| SpamapS | jeblair: The script doesn't really do any scripty things | 19:06 |
| SpamapS | and this would allow me to just pass the real FD's in, rather than arbitrarily grabbing 11/12 | 19:07 |
| jeblair | SpamapS: that would work if we're willing to omit the option for local customization for now (i am) | 19:21 |
| mordred | jeblair, SpamapS: ++ | 19:22 |
| jeblair | clarkb: that suggestion is reasonable. also, i don't want to make it harder for folks to match ref-updated events on, say, /refs/notes or other random ref paths... | 19:24 |
| jeblair | i think i'm leaning toward thinking we should do as close to mechanical pass-through as possible for now, and not split the push event into push/tag and let's look at adding synthetic helper events to both drivers as a separate change later. that separates this potential enhancement from the critical path of landing github. | 19:25 |
| jeblair | i won't consider this finalized until we release anyway :) | 19:26 |
| mordred | jeblair: I think that's a great plan | 19:26 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Add support for github enterprise https://review.openstack.org/449258 | 19:40 |
| mordred | jlk: ^^ do we need to support self-signed certs in any way for ghe deployments? or leave that out of bounds and just expect people to install CAs as appropriate at the system level? | 19:45 |
| jlk | out of bounds I think. | 19:46 |
| jlk | I'd really rather not drag ssl into the mix. | 19:46 |
| mordred | jlk: me either | 19:47 |
| clarkb | its really easy to trust a self signed cert | 19:47 |
| clarkb | you just add the cert itself to your recognized authorities | 19:47 |
| clarkb | (so ya don't bother) | 19:47 |
| mordred | well - it is if you aren't using python requests with the built-in CA bundles | 19:47 |
| clarkb | oh right | 19:47 |
| mordred | which is amongst the reasons all the openstack services and clients have all of those ssl params everywhere | 19:48 |
| mordred | BUT | 19:48 |
| mordred | I agree - let's opt for more sanity | 19:48 |
| *** jamielennox|away is now known as jamielennox | 19:52 | |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Add trigger capability on github pr review https://review.openstack.org/449365 | 20:25 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: [WIP] Fix nodepool integration job https://review.openstack.org/460299 | 20:29 |
| *** dkranz has quit IRC | 21:06 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Catch and log url pattern formatting errors https://review.openstack.org/450468 | 21:14 |
| *** yolanda has joined #zuul | 21:25 | |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Add support for requiring github pr head status https://review.openstack.org/449390 | 21:27 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool feature/zuulv3: Fix zuul-nodepool integration test https://review.openstack.org/460325 | 21:40 |
| SpamapS | No handlers could be found for logger "paste.httpserver.ThreadPool" | 21:41 |
| SpamapS | I assume these are just from the recent log changes that helped us get better runs? | 21:41 |
| jeblair | SpamapS: where do you see that? | 21:43 |
| SpamapS | jeblair: running tests with tox -epy27 | 21:43 |
| clarkb | SpamapS: ya I think it may be because other things can still try and touch logging directly | 21:44 |
| clarkb | the fixtures fixture that we replaced actually unloads all logging, saves that state, loads its own logging in, then remove its logging and replaces saved state when done | 21:45 |
| clarkb | its quite a bit more involved | 21:45 |
| jeblair | we can add a logger if needed, if we identify which test is missing it. or maybe we add a default logger. | 21:45 |
| clarkb | jeblair: in this case I think its trying to remove a handler that isn't managed by it | 21:45 |
| clarkb | and has already been delat with? | 21:46 |
| clarkb | it also appears to be harmless other than the noise | 21:46 |
| clarkb | we might just squelch it | 21:46 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Adds github triggering from status updates https://review.openstack.org/453844 | 21:49 |
| Shrews | jeblair: what do you think about adding a 'user=XYZ' option to the executor section of zuul.conf to control who the daemons run as? or should we just continue assuming 'zuul' user? | 21:49 |
| *** jkilpatr has quit IRC | 21:51 | |
| SpamapS | Shrews: is this for the "listen on finger then drop privs" ? | 22:02 |
| Shrews | SpamapS: yes | 22:02 |
| SpamapS | Shrews: if so, definitely annoying not to be able to set the username to use. | 22:02 |
| Shrews | right, but wondering if the executor section is the correct place, or something more generic ([zuul] ?) in case it's a pattern we want to repeat | 22:07 |
| SpamapS | executor makes sense to me | 22:11 |
| SpamapS | could also want a different user per component | 22:11 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Add support for bwrap https://review.openstack.org/453851 | 22:12 |
| SpamapS | jeblair: ^^ redone as a pure python driver, complete with entrypoint (for testing) | 22:12 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Add support for bwrap https://review.openstack.org/453851 | 22:13 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Add support for bwrap https://review.openstack.org/453851 | 22:17 |
| * SpamapS finishes sanding off the edges | 22:17 | |
| *** jkilpatr has joined #zuul | 22:31 | |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Implement pipeline requirement on github reviews https://review.openstack.org/453845 | 22:52 |
| jlk | that one hurt me. I think I need a break. | 22:56 |
| jeblair | SpamapS, Shrews: yeah, executor section sounds good | 22:56 |
| jeblair | SpamapS: oh neat, a new driver! | 22:58 |
| jlk | interesting use of the driver interface | 23:01 |
| mordred | SpamapS: neat! | 23:03 |
| clarkb | connections is really beginning to feel overloaded :) | 23:05 |
| jeblair | clarkb: well, it's a driver that does not implement the connection interface | 23:06 |
| clarkb | ya but the connections object ends up being the caretaker of all the drivers | 23:06 |
| jeblair | true | 23:06 |
| clarkb | (its responsible for setting them up and cleaning them up when done) | 23:07 |
| jeblair | i sort of expected this to just end up as a function in the executor itself; i don't think it *needs* to be more complicated than that. but the WrapperInterface is an interesting idea, and i guess if we expect to maybe have other implementations, worth considering? | 23:08 |
| clarkb | ya I think its mostly a terminology problem and less functionality | 23:10 |
| jeblair | mordred: can you look at http://logs.openstack.org/25/460325/1/check/gate-nodepool-python27-ubuntu-xenial/655ee56/testr_results.html.gz ? | 23:11 |
| jeblair | keystoneauth1.exceptions.auth_plugins.MissingRequiredOptions: Auth plugin requires parameters which were not given: auth_url | 23:12 |
| jeblair | (the second test; the first is the kazoo delete race) | 23:13 |
| mordred | that seem non-optimal | 23:13 |
| * mordred is looking further | 23:14 | |
| SpamapS | jeblair: when I started, I used the drivers because I thought I might have leftover 'getent' subprocesses to cleanup.. but then I found pwd/grp | 23:17 |
| mordred | jeblair: hrm. I thought we'd dropped support in nodepool for ... oh, we did that in v3 | 23:17 |
| mordred | jeblair: hang on - crap. I'm really bad at debugging - don't listen to me | 23:19 |
| jeblair | mordred: sorry, the topic is 'master' because i'm really bad at git. but i think that's really on the v3 branch. :) | 23:19 |
| jeblair | (pretty much every git command i type now is 'git reset --hard') | 23:20 |
| SpamapS | Oh hm, I may actually have pipes to clean up | 23:21 |
| SpamapS | clarkb: agreed on terminology. I think it's really a driver registry. | 23:22 |
| mordred | jeblair: it's the only git command you need | 23:22 |
| mordred | jeblair: ok. I have reproduced it locally | 23:23 |
| SpamapS | ah yep.. I'm leaking read pipes on the parent. Have to close them after fork.. which is complicated. :-P | 23:24 |
| jeblair | yeah, we should rename it. | 23:27 |
| jeblair | after landing the github series ;) | 23:28 |
| SpamapS | ok, read pipe leak handled | 23:32 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Add support for bwrap https://review.openstack.org/453851 | 23:32 |
| SpamapS | jeblair: this would totally work as a simple function in the executor. I like the layer of indirection though, as I am just a wee bit skeptical that bubblewrap will be the forever best choice. ;) | 23:33 |
| SpamapS | also having the driver allows me to make the entry point really easily. | 23:34 |
| mordred | jeblair: ok - I would like to learn WAY more about what's going on here - but I can give you the short version, and then explain the things I want to understand further | 23:40 |
| mordred | jeblair: the tl;dr is that that test is testing something we don't actually support anymore, so it's not actually a valid test | 23:41 |
| mordred | jeblair: I'm not 100% sure why it worked before today | 23:41 |
| mordred | BUT - we made an os-client-config release today which removed a broken fallback from keystoneauth to keystoneclient which had been there for ages. so my theory is that the bug we fixed in occ was causing this test to fail open, which is why we didn't notice it was a bogus test | 23:42 |
| mordred | jeblair: that's the theory - I would like to prove that that's actually the case and not just that we actually introduced a behavior change in occ that this nodepool change has correctly found | 23:43 |
| mordred | jeblair: but the test_nodepool_provider_config is there to test configuring clouds without using clouds.yaml - so it doesn't set up a clouds.yaml file, which means it can't find the config values (I'm more than a little bit unhappy that the error is what the error is) | 23:45 |
| jeblair | mordred: that sounds reasonable. this probably isn't urgent; we can double check all that with clarkb, pabelanger, and Shrews tomorrow. | 23:46 |
| mordred | jeblair: well - I've got a patch coming to set "cloud" to be a required parameter :) | 23:47 |
| mordred | since, well, it very much is a required parameter | 23:47 |
| jeblair | cool | 23:47 |
| SpamapS | cloud config requires cloud | 23:49 |
| mordred | jeblair: given the lack of cloud parameter, I don't think I _can_ give a better error message from occ - the call in nodepool asked for "the cloud config" - with no clouds.yaml file and no input parameters, which _basically_ equates down to "gimme the one from the environment" - but there were none of those either, so it got all the way to "wow, I cannot build an auth plugin from Null parameters" :) | 23:49 |
| mordred | I'd trap that earlier - but asking for the unnamed cloud config is actually a fairly frequent real thing | 23:49 |
| openstackgerrit | Monty Taylor proposed openstack-infra/nodepool feature/zuulv3: Enforce cloud as a required config value https://review.openstack.org/460354 | 23:56 |
| mordred | jeblair: ^^ | 23:56 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!