jeblair | jhesketh: ^ your comments in that series should be addressed | 00:29 |
---|---|---|
jhesketh | jeblair: awesome, thanks... will review today hopefully | 00:30 |
*** jkilpatr has quit IRC | 00:31 | |
*** jkilpatr has joined #zuul | 00:32 | |
*** leifmadsen has quit IRC | 00:55 | |
*** leifmadsen has joined #zuul | 00:57 | |
*** jkilpatr has quit IRC | 01:13 | |
*** dkranz has quit IRC | 01:20 | |
*** jkilpatr has joined #zuul | 01:25 | |
*** yolanda has quit IRC | 02:26 | |
*** yolanda has joined #zuul | 02:26 | |
jamielennox | jhesketh: we discussed https://review.openstack.org/#/c/446308/ at the meeting the other day, and decided that both options were useful but we would do the simple one for now and move it to nodepool when there's a concrete case | 03:09 |
jamielennox | so [executor] username= would always be available as a default and default to zuul (rather than user ansible is running as) | 03:10 |
jamielennox | and in future nodepool information would be taken in preference | 03:10 |
jhesketh | jamielennox: okay, thanks for the update.. not sure if it makes sense for the nodepool information to take precedence though from a users point of view... it does operationally but if a user has set the username and something else is used I would be confused | 03:18 |
jamielennox | jhesketh: i'm not sure zuul.conf is really user's point of view - but i guess with only 1 option it has to be a fallback choice | 03:44 |
jamielennox | jhesketh: would renaming it default_username or something help? | 03:44 |
jhesketh | jamielennox: by user I meant operator/deployer | 03:44 |
jhesketh | jamielennox: we could have the default_username and maybe use_nodepool_creds boolean? | 03:45 |
jamielennox | jhesketh: so there was no timeline on the nodepool creds side | 03:49 |
jamielennox | i can't really think of a time where you'd want to override the information provided by nodepool as it's actually creating the images | 03:50 |
jhesketh | sure, I mean when we swing back we can consider something likely only overwriting the set username if the boolean says so (for example) | 03:50 |
jhesketh | yeah, that's true... maybe it's a fallback_username? | 03:50 |
jamielennox | jhesketh: personally prefer default_ but will go with consensus, it may also be a long time before anything is capable of overriding the fallback | 03:55 |
jamielennox | i have the patches up but deemed unecessary for now | 03:55 |
jhesketh | jamielennox: sure, default works for me :-) | 03:56 |
jamielennox | jhesketh: ok, will respin | 03:56 |
jamielennox | jhesketh: thanks | 03:56 |
jamielennox | jhesketh: do you know if there are any tests that really go into executor server? i can't see anywhere i can really test this | 03:59 |
jhesketh | not sure off the top of my head sorry | 04:00 |
jhesketh | I don't think there are any unit tests, but there are functional tests afaik | 04:00 |
openstackgerrit | Jamie Lennox proposed openstack-infra/zuul feature/zuulv3: Re-add the ability to set username on zuul-executor https://review.openstack.org/446308 | 04:02 |
*** pbelamge has joined #zuul | 06:19 | |
*** isaacb has joined #zuul | 06:45 | |
*** hashar has joined #zuul | 07:44 | |
*** yolanda has quit IRC | 07:54 | |
*** yolanda has joined #zuul | 08:30 | |
*** yolanda_ has joined #zuul | 08:59 | |
*** yolanda_ has quit IRC | 09:01 | |
*** yolanda_ has joined #zuul | 09:01 | |
*** yolanda has quit IRC | 09:03 | |
*** bhavik1 has joined #zuul | 09:08 | |
*** bhavik1 has quit IRC | 09:46 | |
*** bhavik1 has joined #zuul | 09:51 | |
*** jkilpatr has quit IRC | 10:42 | |
tobiash | hi, I'm currently working on support for unmanaged images in nodepool | 10:48 |
tobiash | (on top of https://review.openstack.org/#/c/455464) | 10:49 |
tobiash | the providers section also specifies a diskimages list where some provider specific meta data can be set | 10:49 |
tobiash | (from which I think config-drive makes also sense for unmanaged images) | 10:50 |
tobiash | does it make sense to rename this to images? | 10:50 |
tobiash | thinking of something like this: | 10:51 |
tobiash | http://paste.openstack.org/show/607134/ | 10:51 |
*** jkilpatr has joined #zuul | 11:00 | |
*** isaacb has quit IRC | 11:04 | |
Shrews | tobiash: no. we just renamed it _from_ images in commit dcc3b5 | 11:05 |
tobiash | Shrews: just saw that it gets diskimages + provider specific stuff | 11:06 |
tobiash | Shrews: would you prefer to add such a section for unmanaged images? | 11:08 |
tobiash | http://paste.openstack.org/show/607137/ | 11:08 |
tobiash | or just don't support customizing config-drive for unmanaged images? | 11:08 |
tobiash | (which is I think the only option from diskimages which makes sense for unmanaged images) | 11:09 |
Shrews | it might make more sense under 'pools' sections because the provider:diskimage is there to tell us which images to build and upload, but the pools:diskimage tells us which to actually use. I will defer to jeblair on that, though | 11:12 |
*** bhavik1 has quit IRC | 11:42 | |
*** yolanda_ has quit IRC | 12:04 | |
*** yolanda has joined #zuul | 12:04 | |
*** yolanda has quit IRC | 12:04 | |
tobiash | just interested, what are you using for debugging zuul/nodepool? | 12:05 |
*** yolanda has joined #zuul | 12:05 | |
openstackgerrit | Tobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Whitelist pydevd debug threads https://review.openstack.org/458041 | 12:05 |
mordred | hey everybody - so, I _CANNOT_ believe I'm about to say this - like, not even a little bit not even at all now I want to stab myself yukc ... BUT | 12:49 |
mordred | I was looking at the work sdague as been doing to use systemd for logging in devstack, which is actually cool because you can annotate log messages with a bunch of metadata | 12:50 |
mordred | and have discovered that python-systemd has a python logging handler: https://github.com/systemd/python-systemd/blob/master/systemd/journal.py | 12:50 |
mordred | that you can configure as-normal in a python logging config file | 12:50 |
mordred | if you do - it automatically annotates log messages with additional information | 12:51 |
mordred | https://github.com/systemd/python-systemd/blob/master/systemd/journal.py#L594 | 12:51 |
mordred | such as threadName, processName, pathname, linenum, and funcName - as well as full exception text and information if you're logging an exception | 12:52 |
mordred | https://dague.net/2017/03/30/in-praise-of-systemd/ | 12:52 |
mordred | if you havnet read it yet | 12:52 |
*** dkranz has joined #zuul | 12:59 | |
tobiash | mordred: that's cool | 13:04 |
tobiash | I'm using something similar for direct to json logging where the json can be pushed straight to elasticsearch | 13:05 |
mordred | yah. it's ... I mean, I still think it's TERRIBLE for this to have been implemented as part of the init system | 13:17 |
mordred | BUT - I guess I can't fight that any more | 13:17 |
openstackgerrit | Jamie Lennox proposed openstack-infra/zuul feature/zuulv3: Don't allow auth definition in job to be a list https://review.openstack.org/458072 | 13:19 |
openstackgerrit | Tobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Support externally managed images https://review.openstack.org/458073 | 13:19 |
*** yolanda has quit IRC | 13:20 | |
tobiash | ^^ first draft for externally managed images | 13:20 |
jamielennox | mordred: :) | 13:20 |
tobiash | probably needs some beautifying but I hope I'm in the right direction | 13:21 |
*** _ari_|gone is now known as _ari_ | 13:28 | |
*** pbelamge has quit IRC | 13:29 | |
mordred | tobiash: looks great! one minor quibble/question in the docs | 13:29 |
mordred | tobiash: (that is fairly straightforward) | 13:29 |
tobiash | mordred: wow, that was quick :) | 13:31 |
tobiash | I would tend to cloudimage | 13:31 |
tobiash | And i'm not yet satisfied with the term unmanagedimages in provider config | 13:32 |
tobiash | maybe this also should be renamed to cloudimages? | 13:32 |
*** yolanda has joined #zuul | 13:33 | |
*** yolanda has quit IRC | 14:08 | |
*** yolanda has joined #zuul | 14:08 | |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Don't allow auth definition in job to be a list https://review.openstack.org/458072 | 14:34 |
*** yolanda has quit IRC | 14:38 | |
*** yolanda has joined #zuul | 14:39 | |
jeblair | tobiash, mordred: yeah, i would use the same term under pool and pool-label (because the thing in pool-label refers to the thing in provider). while i would seriously consider just using "image" for that, "cloud-image" is growing on me, so maybe we should avoid ambiguity and use that. | 14:40 |
*** isaacb has joined #zuul | 14:48 | |
jeblair | Shrews: you were looking at using daemoncontext to drop privileges; did that go anywhere? | 14:54 |
jeblair | Shrews: (i think if we can create the socket, then drop privs before starting the server, it would be nice) | 14:54 |
Shrews | jeblair: starting to look at that. the issue is we have to drop privs AFTER grabbing the fingerd port, so that throws a kink into how the executor current works | 14:55 |
jeblair | Shrews: oh, of course we have the no-daemon option.... | 14:55 |
jeblair | Shrews: yeah, the port grabbing would have to be really early | 14:55 |
Shrews | jeblair: we could sort of combine both approaches and launch the current finger code i have up within cmd/executor.py (so we'd have two daemon contexts there) | 14:56 |
Shrews | but we'd lose the advantage of accessing internal things | 14:56 |
Shrews | but it does eliminate having two separate processes | 14:56 |
Shrews | that might be a stupid idea | 14:56 |
Shrews | but something that just occurred to me | 14:57 |
jeblair | well, i think the only thing we have to do as root is bind the socket | 14:57 |
jeblair | so we can bind in cmd/executor.py then drop privs, and pass the bound socket into executor. | 14:57 |
Shrews | yeah. gonna try that out here in a bit. | 14:58 |
jeblair | cool | 14:58 |
jeblair | Shrews: if we want this to be compatible with the '-d' (no daemon) option, then we may not be able to use the daemoncontext arguments, and instead may just need to use the change_privs method you wrote but move it up a level. | 14:59 |
Shrews | yeah | 15:00 |
Shrews | jeblair: that code looks sane-ish enough for you? | 15:00 |
Shrews | i wasn't sure if we should do something with umaks | 15:01 |
Shrews | umask | 15:01 |
Shrews | brb | 15:02 |
jeblair | Shrews: probably a good idea to add that | 15:02 |
jeblair | Shrews: otherwise, yep. | 15:02 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Extend test timeout to 120s https://review.openstack.org/454806 | 15:04 |
Shrews | jeblair: one thing to note, we'll likely have to use threads instead of processes to handle the requests. | 15:13 |
Shrews | s/likely/definitely/ | 15:14 |
pabelanger | is it not possible we could use xinetd for some of the port mapping? | 15:16 |
clarkb | since mordred seems to be cool ish with systemd now. just going to throw this out there, we could use socket activation/inetd | 15:17 |
clarkb | pabelanger: jinx | 15:17 |
pabelanger | :) | 15:17 |
jeblair | i don't think we should *depend* on systemd. | 15:19 |
clarkb | jeblair: right but for those without systemd there is inetd and friends. But maybe we don't want to depend on those either? | 15:20 |
jeblair | Shrews: why definitely threads? | 15:20 |
jeblair | clarkb: i'd rather not | 15:21 |
Shrews | jeblair: because once you accept a connection, passing that client socket to a forked process has issues (e.g., i could not get the socket to close). socket sharing across processes is a notoriously difficult problem (that socketserver somehow managed to solve) | 15:21 |
jeblair | Shrews: you can't pass a bound socket into socketserver? | 15:21 |
Shrews | the socketserver lib doesn't work that way, so i'm not sure what you're asking there | 15:22 |
Shrews | i don't see how we can use that lib if we integrate directly with executor | 15:23 |
*** hashar has quit IRC | 15:26 | |
*** yolanda has quit IRC | 15:28 | |
*** yolanda has joined #zuul | 15:29 | |
jeblair | Shrews: i guess you could subclass it to accept a bound socket in the constructor and then just have server_bind return that value... but maybe that's not worth it? | 15:30 |
jeblair | Shrews: i think we probably should think more on whether internal data access is important (since that requires threads anyway) | 15:31 |
jeblair | Shrews: if it's not that important, maybe using the forking tcpserver as a separate process started by the executor command is the way to go. | 15:31 |
Shrews | jeblair: hrm, that might work, actually | 15:37 |
Shrews | base class sets self.socket in __init__, but looks like we could dump that and replace it safely | 15:40 |
Shrews | at least, in the current iteration of that library | 15:40 |
* Shrews checks py3 version... | 15:41 | |
jeblair | Shrews: i assumed it would be self.socket = self.bind_socket(). so if we saved the socket before calling super's constructor, we could just return it from bind_socket. | 15:41 |
Shrews | https://hg.python.org/cpython/file/2.7/Lib/SocketServer.py#l431 | 15:42 |
Shrews | trying to find the py3 version | 15:43 |
jeblair | Shrews: yeah, i was suggesting we override server_bind (that's what the docs say to do) | 15:43 |
jeblair | Shrews: oh, i see what you mean | 15:43 |
jeblair | well, that's lame. | 15:44 |
Shrews | py3 code seems to be identical | 15:44 |
Shrews | jeblair: yeah, i mean, we're dumping a socket they've already created if we do that. not sure how future proof that would be | 15:45 |
jeblair | Shrews: what do you think is worth more? threads+internal data access or forking? | 15:45 |
Shrews | jeblair: forking seemed to be advantageous if we have a lot of folks streaming logs. if we go w/ the other option, and now we have a whole lot more threads, i worry how that might affect executor | 15:47 |
Shrews | right now, i'm not seeing anything advantageous to accessing internal data. it would eliminate the assumption of where the ansible_log.txt file is stored, but that's the only thing at this point | 15:48 |
Shrews | and even that we could migrate to the config. if we need other logs/files to stream, then maybe there's an advantage? | 15:49 |
Shrews | and we could directly query ExecutorServer.job_workers for valid job IDs | 15:50 |
Shrews | rather than filesytem globbing | 15:50 |
jeblair | Shrews: we do have the idea of requesting specific log files from specific hosts, so this would need to be able to handle the request "get syslog from the host named compute for the build foo", which would basically mean that we need to know the contents of the inventory for the build. we can either get that from the internal data structure or by reading the inventory file on disk. that's the only other consideration i can think of. | 15:51 |
Shrews | jeblair: so that part of the code i don't think i've explored deeply enough to intelligently say it would be better one way or the other | 15:52 |
Shrews | jeblair: which internal data structure has that? | 15:53 |
Shrews | is that in AnsibleJob? | 15:54 |
jeblair | Shrews: ExecutorServer.job_workers[uuid] is an AnsibleJob. and yeah, we'd find it in there somewhere. | 15:54 |
jeblair | (the inventory isn't saved as an attribute, but we could add it) | 15:55 |
Shrews | jeblair: ok. maybe it's worth coding it up to actually see it then | 15:55 |
Shrews | jeblair: i might need some assistance from you on getting a local executor running so I can actually test it | 15:57 |
Shrews | that's the most difficult part :) | 15:57 |
*** hashar has joined #zuul | 15:59 | |
jeblair | Shrews: that's possible, but it may be easier to incorporate it into the test framework | 15:59 |
*** hashar has quit IRC | 16:02 | |
*** hashar has joined #zuul | 16:03 | |
*** isaacb has quit IRC | 16:09 | |
*** hashar has quit IRC | 16:36 | |
*** bhavik1 has joined #zuul | 16:41 | |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add simple_layout test decorator https://review.openstack.org/458168 | 17:10 |
jeblair | SpamapS, clarkb, jlk: ^ that's my idea for reducing the unecessary git repo activity (and also simplifying a lot of test configuration) | 17:10 |
jeblair | can you give that a quick look and see if the approach looks good? | 17:11 |
clarkb | sure, we should also double check test times are reduced? | 17:11 |
clarkb | testr should print out the slow tests which we can compare against | 17:11 |
*** harlowja_ has quit IRC | 17:13 | |
jeblair | clarkb: this will have 3 effects on test times: the test i applied it to should be much shorter because it only initializes the repos it needs; the other tests that share that general config should be a little shorter because i have removed one extra repo from their initialization, but i will need more changes to repeat this for the other unecessary repos in order for this reduction to accumulate; and overall, once that's applied to as many ... | 17:14 |
jeblair | ... tests as we can, there should generally be less git repo activity which should mean less io/cpu contention for all tests. | 17:14 |
jeblair | clarkb: the first is easy to measure. it's down from 3.420s to 1.357s on my local machine. | 17:15 |
*** harlowja has joined #zuul | 17:15 | |
clarkb | cool | 17:15 |
jeblair | the rest would probably need stasticially significant sampling to determine the effect. | 17:15 |
jeblair | (and also aren't that interesting until the whole patch series is done) | 17:16 |
*** harlowja has quit IRC | 17:17 | |
*** harlowja has joined #zuul | 17:17 | |
* jlk looks | 17:17 | |
SpamapS | jeblair: looking now | 17:20 |
clarkb | jeblair: if I read this correctly, its basicaly a helper that will read the layout to only configure the subset of project repos needed. Rather than setting up the full set every time? | 17:21 |
jeblair | clarkb: yes. the other way to do that is to create a config fixture with a bunch of directories that get copied over as git repos. that works really well for complex scenarios, but is annoying for simple ones (you have to make a bunch of directories), so we ended up with one of those but with a bunch of little repos with their own configs in them. this makes it simpler not to do that. :) | 17:24 |
clarkb | ok commented | 17:25 |
jeblair | clarkb: replied; lmk if that makes sense. | 17:29 |
clarkb | jeblair: thanks. Basically the end result will be default is simple_layout of "single tenant" equivalent? | 17:31 |
clarkb | I think thats a good end state. Mostly don't want to have to explicitly opt into using the decorator every time a new test is written as it seems like that would be a good way to end up with really slow tests again | 17:31 |
jlk | oh yeah a bunch of my tests would benefit from this. | 17:32 |
clarkb | ok zuul test suite and then local reproduction confirms you can't un shutdown an apscheduler backgroundscheduler. Thats unfortunate | 17:51 |
SpamapS | clarkb: doh | 17:53 |
SpamapS | clarkb: you could, however, recreate the object I assume? | 17:53 |
openstackgerrit | Clark Boylan proposed openstack-infra/zuul feature/zuulv3: Don't run apsched thread when no jobs need scheduling https://review.openstack.org/457798 | 17:56 |
clarkb | SpamapS: yup ^ | 17:56 |
clarkb | SpamapS: the annoying part is its actually fine untuil the cron trigger triggers. So you don't get an error upfront | 17:57 |
*** jkilpatr has quit IRC | 17:57 | |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add simple_layout test decorator https://review.openstack.org/458168 | 17:59 |
*** jkilpatr has joined #zuul | 18:00 | |
SpamapS | clarkb: time's a bitch | 18:04 |
*** bhavik1 has quit IRC | 18:22 | |
*** jkilpatr has quit IRC | 19:27 | |
*** jkilpatr has joined #zuul | 19:30 | |
*** jkilpatr has quit IRC | 19:30 | |
*** jkilpatr has joined #zuul | 19:30 | |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add playbook support to simple_layout in tests https://review.openstack.org/458220 | 19:57 |
jeblair | clarkb, SpamapS, jlk: ^ heh, i chose a slightly too simple test for my first move, so i needed to expand simple_layout a bit for the next one. but hey, this makes for good narrative structure in changes. so, erm, i totally planned it that way. | 19:58 |
jeblair | clarkb, SpamapS, jlk: you want the rest of these one test per patch, or should i do some omnibus patches? | 20:08 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Stop jobs when executor stops https://review.openstack.org/457753 | 20:09 |
jlk | maybe set up a hit list for spreading the work out? | 20:09 |
jlk | I don't think it's great use of your time to iterate through them... :) | 20:09 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix race in test_queue_rate_limiting_dependent https://review.openstack.org/457799 | 20:10 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Pass source to project instantiations https://review.openstack.org/451596 | 20:10 |
jeblair | jlk: normally i'd agree, but now that i've started, i think i can knock 'em out pretty quick (the last 2 took me 5m each) -- unless you're dying to get in on the action. | 20:10 |
jlk | no, not dying per se. | 20:11 |
jlk | I"d say omnibus it unless there's some that wind up being awkward to handle, and deserve their own change. | 20:11 |
jeblair | ok. i also only have a few more that are like this, then there's another class of test that needs more thinking. that may be a good opportunity for a hit list. | 20:12 |
jlk | guh, it's cold enough today that my fingers are having a hard time typing right. | 20:12 |
jeblair | sometimes i make a mug of hot water just to hold. | 20:13 |
jeblair | i understand that other people put other hot liquids in mugs which serve a dual purpose. i dunno about that. | 20:14 |
jlk | heh, yeah. my tea this morning helped with that. I need to go see if i have any more caffeine free stuff, since I"m already at my limit for the day | 20:15 |
clarkb | yes its cold here too. Annoying because yesterday was open windows weather | 20:15 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move tests to use simple_layout https://review.openstack.org/458231 | 20:43 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove unused repos from repo init https://review.openstack.org/458232 | 20:43 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove project3 https://review.openstack.org/458233 | 20:43 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use 'org/project' for tests which only operate on one project https://review.openstack.org/458237 | 20:56 |
* mordred is wearing shorts and the windows are open and the little fountains into the pool accompany the lovely classical music quite nicely | 20:56 | |
jeblair | okay, i think the next phase of this is to identify which of the uses of 'updateConfigLayout' can be replaced by simple_layout. that will let us remove all of the extra git repos from here, which are all initialized in many of the test runs: http://git.openstack.org/cgit/openstack-infra/zuul/tree/tests/fixtures/config/single-tenant/git?h=feature/zuulv3 | 20:57 |
jeblair | there are 32 of those, so that might be a good opportunity for a hit list | 20:57 |
jeblair | i will do one now, then make a list | 20:57 |
jeblair | (basically, it's a candidate if the only use of updateConfigLayout is at the top of the test method. we're still going to need this for those which change the layout inside of the test method, so we should rework those cases after the easy ones are moved) | 20:59 |
SpamapS | jeblair: ahhh optimization cycles. so soothing. | 21:02 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_tags to simple_layout https://review.openstack.org/458240 | 21:04 |
jeblair | okay, there's one. i'll see how many others there are | 21:04 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_tags to simple_layout https://review.openstack.org/458240 | 21:04 |
jeblair | git add ^ :) | 21:04 |
mordred | jeblair: silly git add | 21:10 |
jeblair | mordred: you're a silly git add | 21:11 |
SpamapS | my git add brings all the files to the yard | 21:12 |
jlk | OUT ---> | 21:12 |
clarkb | jeblair: whats the overlap in updateConfigLayout and single-tenant/zuul.yaml? | 21:12 |
clarkb | (seems like this is also cleaning up triple accounting of a bunch of stuff) | 21:13 |
SpamapS | adam_g: can you repeat the question you asked earlier about secrets/auth? :) | 21:15 |
adam_g | well, it was basically the question of what prevents someone proposing a patch that simply logs the decrypted secrets somehwere visible during/after the test | 21:17 |
jeblair | clarkb: single-tenant/zuul.yaml is what's initially loaded. then updateconfiglayout rewrites that file with an alternative. they shared the same base set because that was backwards compatible with the way tests used to work. but we really don't want either doing more than necessary. | 21:17 |
SpamapS | jeblair: ^ My answer to adam_g's question is that config/trusted repos shouldn't have check jobs. Thoughts? | 21:17 |
adam_g | SpamapS: right, that seems like a sane thing to ask of users | 21:18 |
SpamapS | Or I wonder if you can have a check job that is only triggered after +2 or something. | 21:18 |
jeblair | SpamapS: basically -- we added an 'allow-secrets' flag to pipelines, so you can say 'the check pipeline can not use secrets at all'. that way all the projects can still have check jobs, but just safe ones. | 21:19 |
jesusaur | SpamapS: well, if there's an additional trigger, it would be a new pipeline; but you could have check and super-check | 21:19 |
jeblair | SpamapS: then you can add a "safe-check" pipeline, if you want, does allow secrets and which triggers on +2 like you suggest if you want. | 21:19 |
jeblair | right that :) | 21:19 |
jeblair | apparently you have to really want it though since i said that twice | 21:20 |
SpamapS | oh yay there's suspenders on that one. :) | 21:20 |
clarkb | jeblair: comment on https://review.openstack.org/#/c/458237/1 to sort it out | 21:20 |
SpamapS | I like the idea of a trusted-check | 21:20 |
clarkb | jeblair: you'd also need trusted config to not reload its own config on proposal? eg you can add allow-secrets to check in a change and have that magically work? | 21:22 |
clarkb | (pretty sure this is how it worked at PTG at least so should be fine) | 21:22 |
jeblair | clarkb: ya. pipelines can't be dynamically reconfigured, so i think we're good there. | 21:22 |
jeblair | SpamapS, adam_g: when going back over things recently, i *think* i may have missed one of the safety checks which is "secrets can only be used by jobs defined in the same repo as the secret". that's a similar situation, in that you don't want to define a secret in the shade repo and then have cloudkitty go and use it (in a job that just prints it out). that's an easy fix though. just be aware there's a hole there. i'll fix that this week (if ... | 21:24 |
jeblair | ... indeed i did miss it). | 21:24 |
mordred | jeblair: ++ | 21:26 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Use 'org/project' for tests which only operate on one project https://review.openstack.org/458237 | 21:29 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_tags to simple_layout https://review.openstack.org/458240 | 21:29 |
jeblair | clarkb: thx ^ | 21:29 |
jeblair | https://etherpad.openstack.org/p/tYAUpHqZeo | 21:31 |
jeblair | those are the easy moves to simple_layout, if anyone wants to grab one, mark it in the etherpad. | 21:32 |
mordred | I feel like I've reviewed this patch I'm reviewing right now, but it does not show that I ever voted on it | 21:37 |
jeblair | mordred: gertty or web? | 21:38 |
mordred | jeblair: web | 21:38 |
mordred | jeblair: I mostly just recognize the content | 21:38 |
jeblair | probably just wrote the same patch twice. we do that a lot. | 21:38 |
mordred | hahaha | 21:39 |
mordred | (it's the first patch of your giant chain) | 21:39 |
jeblair | mordred: oh, yeah. i noticed you reviewed some of those but not others. i guess that wasn't on purpose. :| | 21:41 |
jlk | jeblair: these we're picking off the list, shoudl they be on top of any specific patch? | 21:44 |
jeblair | jlk: 458240 | 21:46 |
mordred | jeblair: maybe just consider that I left a +2 for all ones I didn't review - since apparently I don't know how to work buttons | 21:46 |
jhesketh | Morning | 21:49 |
mordred | morning mr jhesketh | 21:49 |
*** jasondotstar_ is now known as jasondotstar | 21:49 | |
jhesketh | o/ | 21:51 |
mordred | jhesketh: if you feel like fun, https://review.openstack.org/#/c/451597 is at the base of a big stack and I think you were the last person with a -1 on it | 21:53 |
* jeblair takes a short break | 21:55 | |
jhesketh | mordred: Yeah I failed to get to that yesterday sorry. Im not sure if I will be able to today so feel free to merge etc without me but I'll do my best to take a look asap | 21:55 |
openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: move test_disable_at to simple_layout https://review.openstack.org/458252 | 21:56 |
mordred | jhesketh: no worries - was just going through them myself and noticed that | 22:01 |
jhesketh | yeah, thanks for the heads up :-) | 22:02 |
* jlk has to step away for a bit. | 22:04 | |
openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Move check_ignore_dependencies to simple_layout https://review.openstack.org/458255 | 22:04 |
jlk | hopefully I did those two right | 22:04 |
mordred | jeblair: I have pulled the trigger on the big stack | 22:16 |
jeblair | mordred: thanks! | 22:17 |
mordred | jeblair: maybe we'll get to enjoy one of those lovely moments where zuul goes on a merge spree on its own code | 22:18 |
jeblair | mordred: with our failure rate, probably not :( | 22:18 |
mordred | jeblair: :( | 22:21 |
*** jkilpatr has quit IRC | 22:21 | |
mordred | jeblair: https://review.openstack.org/#/c/436802 looks fine to me, but is one that I would like your ok on - us using http for one thing seems like a conscious choice, but I do not remember it being one specifically | 22:22 |
clarkb | jeblair: it does seem to be somewhat better towards the tip of your stack of changes. test times down and not seeing as many fails | 22:22 |
mordred | (it's not urgent, just calling it out because I happened to be looking at it) | 22:22 |
mordred | clarkb: ++ | 22:23 |
clarkb | and the fails I did see were related to the proposed changes which is progress :) | 22:23 |
jeblair | mordred: thanks; let me cipher on that a bit when i have more brainspace | 22:25 |
mordred | jeblair: totes | 22:25 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add a project index to Tenant https://review.openstack.org/451597 | 22:35 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use new tenant project index for config references https://review.openstack.org/451928 | 22:35 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove unused Tenant.getRepo method https://review.openstack.org/451929 | 22:35 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add source to project and remove unused tenant attrs https://review.openstack.org/451969 | 22:37 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add hostname to TriggerEvent https://review.openstack.org/452348 | 22:37 |
jeblair | jlk: i think you forgot some git adds | 22:38 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_tags to simple_layout https://review.openstack.org/458240 | 22:38 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move some uses of updateConfigLayout to simple_layout https://review.openstack.org/458262 | 22:38 |
*** dkranz has quit IRC | 22:38 | |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove unused test git repos https://review.openstack.org/458263 | 22:41 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fully qualify project configuration names https://review.openstack.org/451970 | 22:42 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Replace config/project repos with config/untrusted projects https://review.openstack.org/453347 | 22:42 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove source from pipelines (1/2) https://review.openstack.org/453362 | 22:43 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove source from pipelines (2/2) https://review.openstack.org/453821 | 22:43 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add simple_layout test decorator https://review.openstack.org/458168 | 22:43 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add playbook support to simple_layout in tests https://review.openstack.org/458220 | 22:43 |
jlk | jeblair: foiled again! | 22:48 |
openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Move check_ignore_dependencies to simple_layout https://review.openstack.org/458255 | 22:50 |
openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: move test_disable_at to simple_layout https://review.openstack.org/458252 | 22:51 |
pabelanger | jeblair: I'm going to start work on https://review.openstack.org/#/c/454396/ again tomorrow. Is that something we need to land to bring zuulv3-dev.o.o back online? | 22:52 |
*** jkilpatr has joined #zuul | 22:57 | |
jeblair | pabelanger: thanks; i think we can go ahead and bring it online since the actual protection has landed, and that shows us it's generally working. but finishing that will increase our confidence in it and help prevent regressions. | 22:58 |
pabelanger | okay cool! | 22:58 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: move test_disable_at to simple_layout https://review.openstack.org/458252 | 22:58 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move check_ignore_dependencies to simple_layout https://review.openstack.org/458255 | 22:58 |
jeblair | jlk: ^ rebased on top of mine | 22:58 |
*** jkilpatr has quit IRC | 23:05 | |
jlk | thanks | 23:05 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_repo_deleted to simple_layout https://review.openstack.org/458269 | 23:06 |
jeblair | starting on some of the trickier ones now | 23:06 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove org_unknown git repo from single-client test config https://review.openstack.org/458270 | 23:28 |
clarkb | jeblair: can you see comments on https://review.openstack.org/#/c/451597/12 ? its already merged but I am slightly confused by a couple things | 23:32 |
jeblair | if secret.source_context != job.source_context: | 23:36 |
jeblair | raise Exception( | 23:36 |
jeblair | "Unable to use secret %s. Secrets must be " | 23:36 |
jeblair | "defined in the same project in which they " | 23:36 |
jeblair | "are used" % secret_name) | 23:36 |
jeblair | adam_g, SpamapS: oh look i didn't forget ^ :) | 23:36 |
jeblair | clarkb: looking | 23:36 |
*** jkilpatr has joined #zuul | 23:38 | |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Resolve project_name/project confusion for gerrit https://review.openstack.org/455604 | 23:39 |
jeblair | clarkb: responded | 23:43 |
SpamapS | jeblair: \o/ ... is there.. a test? ;-) | 23:44 |
jeblair | SpamapS: i don't think so. a lot of that is weak on negative test coverage. | 23:45 |
clarkb | jeblair: I see sothere is only ever a single k:v pair in that second inner dict? | 23:47 |
jeblair | clarkb: no, you can have a second hostname | 23:47 |
clarkb | ah | 23:47 |
jeblair | so, {"nova": {"git.openstack.org": <project nova>, "github.com": <project othernova>}} would be a valid structure of two unrelated projects both named "nova", and in that case, we would always have to specify hostnames in the zuul.yaml files for them. | 23:49 |
clarkb | right | 23:49 |
clarkb | I think I was mostly thrown off by the value that wasn't used and trying to fit it into what I saw | 23:49 |
jeblair | sorry that didn't end up in the right change | 23:50 |
openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Update trigger script to handle periodic jobs https://review.openstack.org/439821 | 23:51 |
jeblair | tobiash: it looks like if you don't define a semaphore, but use it in a job, you get an automatically created mutex by default? | 23:54 |
jeblair | tobiash: if that's correct, i think we may want to make it required to define a semaphore before using it. otherwise i think usage across projects is going to get tricky. | 23:55 |
openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Move tests to use simple_layout https://review.openstack.org/458231 | 23:57 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!