| jeblair | whew, glad i could help | 00:09 |
|---|---|---|
| *** xinliang has quit IRC | 00:53 | |
| *** xinliang has joined #zuul | 01:06 | |
| *** xinliang has quit IRC | 01:06 | |
| *** xinliang has joined #zuul | 01:06 | |
| *** rcarrillocruz has quit IRC | 01:42 | |
| *** rcarrillocruz has joined #zuul | 02:16 | |
| *** clarkb1 has joined #zuul | 05:19 | |
| *** pbrobinson_ has joined #zuul | 05:21 | |
| *** harlowja has quit IRC | 05:22 | |
| *** pbrobinson has quit IRC | 05:22 | |
| *** clarkb has quit IRC | 05:22 | |
| *** pbrobinson_ is now known as pbrobinson | 05:22 | |
| *** eventingmonkey has quit IRC | 05:30 | |
| *** eventingmonkey has joined #zuul | 05:33 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Add support for custom ssh port https://review.openstack.org/468752 | 06:34 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: Ensure build.start_time is defined https://review.openstack.org/480843 | 06:36 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Abstract Nodepool request handling code https://review.openstack.org/468748 | 06:37 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Abstract Nodepool provider management code https://review.openstack.org/468749 | 06:38 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Collect request handling implementation in an OpenStack driver https://review.openstack.org/468750 | 06:38 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Extend Nodepool configuration syntax to support multiple drivers https://review.openstack.org/468751 | 06:39 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Implement a static driver for Nodepool https://review.openstack.org/468624 | 06:51 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Remove FakeProvider getClient monkey-patch https://review.openstack.org/475131 | 06:54 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: github: gracefully handle unknown event https://review.openstack.org/473249 | 07:12 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: github: prevent getRepoPermission to raise AttributeError https://review.openstack.org/475368 | 07:20 |
| *** isaacb has joined #zuul | 07:24 | |
| *** bhavik1 has joined #zuul | 07:52 | |
| *** bhavik1 has quit IRC | 08:10 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: sql-reporter: add support for Ref change https://review.openstack.org/465539 | 08:52 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: sql-reporter: add support for Ref change https://review.openstack.org/465539 | 08:57 |
| *** hashar has joined #zuul | 09:06 | |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Exercising multi-node inventory https://review.openstack.org/481014 | 12:39 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Exercising multi-node inventory https://review.openstack.org/481014 | 12:41 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Exercising multi-node inventory https://review.openstack.org/481014 | 12:51 |
| *** dkranz has joined #zuul | 12:52 | |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Exercising multi-node inventory https://review.openstack.org/481014 | 12:55 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: DNM Exercising multi-node inventory https://review.openstack.org/481014 | 12:58 |
| mordred | jeblair, Shrews: I discovered a bug this morning | 14:15 |
| mordred | if you look at that job ^^ | 14:15 |
| mordred | jeblair: oh! never mind - it was just a bug in my slide | 14:15 |
| mordred | the job itself is defined properly | 14:15 |
| mordred | jeblair, jlk: gave a status update on zuul v3 internally at RH this morning. got a question about github status reporting and single vs. multiple urls | 14:19 |
| mordred | jeblair, jlk: the person asking the question said that the jenkins github integration supports setting multiple statuses on a job after I told him that our understanding was that the gh api only allowed a single status | 14:19 |
| mordred | jeblair, jlk: so - it may be worth either figuring out what jenkins does to support this or if it's not I'd love to have a good simple explanation I can give or something | 14:20 |
| SpamapS | I believe you can set many statuses on a PR | 14:35 |
| SpamapS | I can't see why you wouldn't be able to. | 14:35 |
| SpamapS | but jlk has been inside the beast.. I've just been observing it. | 14:36 |
| mordred | yah - I defer to jlk for sure | 14:37 |
| mordred | I mostly want to either understand what the difference so I can answer questions appropriately - or find out if there is something we missed somewhere | 14:38 |
| * mordred isn't finding much by trolling the internet | 14:38 | |
| mordred | I've asked the rh human if he has a link to a github PR with multiple statuses set by jenkins I could look at | 14:39 |
| mordred | jeblair, jlk, SpamapS: oh. I think I understand more things now | 14:51 |
| *** isaacb has quit IRC | 14:57 | |
| jeblair | mordred: what's that? | 15:01 |
| jeblair | i'm going to self-approve my docs stack. we need to get that merged, and i think it's been fairly well reviewed even though the current votes are spotty. | 15:04 |
| jeblair | and mordred's job changes too | 15:04 |
| mordred | jeblair: ++ | 15:06 |
| mordred | jeblair: creating more than one pull request status (like one per job) is totally a thing you can do - however, marking any of those statuses as "required" on a protected branch is a different thing and requires admin access | 15:07 |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Add run-bindep role and add it to unittests pre https://review.openstack.org/478264 | 15:07 |
| mordred | jeblair: so if we want to tell folks to put a "require passing zuul build" on a branch protection, that needs to be single - otherwise defining jobs in the repo gets subverted | 15:07 |
| jeblair | mordred: is one status per job, plus one overall status possible? | 15:08 |
| jeblair | (so only the overall status would be required) | 15:08 |
| mordred | statuses seem to be however-many you want - however, iirc, github will show overall status considering all statues - even non-voting ones | 15:09 |
| mordred | so reporting all of them broken out may produce noise/misleading overview | 15:10 |
| jeblair | makes sense | 15:11 |
| mordred | otoh - the required statuses are managable via the API - so one could also imagine a "let zuul manage required status" option that would require giving zuul admin access to the repo - and if a user opted in to that having the github plugin update the required checks entry on the branch at the start of a gate job to include each of the voting jobs | 15:12 |
| mordred | I'm not sure how valuable that would be | 15:12 |
| jeblair | mordred: yeah, but the set of voting jobs could be specific to that particular pr (with files matchers, etc) | 15:13 |
| jeblair | (i mean, we could return dummy success statuses, but then it'd be noisy again) | 15:13 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add configuration documentation https://review.openstack.org/463328 | 15:16 |
| mordred | jeblair: yes indeed. I think it would be noisy | 15:16 |
| mordred | jeblair: it seems like making it easy for someone to choose one-status or one-status-plus-status-per-job may be the biggest win | 15:17 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Reorganize docs into user/admin guide https://review.openstack.org/475928 | 15:17 |
| mordred | (people know what is noise and what isn't) | 15:17 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use oslosphinx theme https://review.openstack.org/477585 | 15:20 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Move status_expiry to webapp section https://review.openstack.org/477586 | 15:20 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Move tenant_config option to scheduler section https://review.openstack.org/477587 | 15:20 |
| jeblair | mordred: yeah, but i worry a bit about the non-voting jobs causing github's status summary to be red. we know that in gerrit people give a lot of weight to the overall status. i think people might think they want many+1 and then be surprised by that. maybe we shouldn't report non-voting as a status, but then that makes it less visible. of course it's already much less visible with only one status. | 15:20 |
| jeblair | mordred: we may need to have some experimental demonstrations. | 15:21 |
| mordred | jeblair: ++ | 15:23 |
| mordred | crap. I should have abandoned https://review.openstack.org/478264 when I squashed those patches :( | 15:53 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix test_bubblewrap_leak https://review.openstack.org/481123 | 15:53 |
| *** clarkb1 is now known as clarkb | 15:54 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Correct sample zuul.conf https://review.openstack.org/477588 | 16:09 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use executor section of zuul.conf for executor dirs https://review.openstack.org/477589 | 16:10 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use scheduler-specific log config and pidfile https://review.openstack.org/477590 | 16:10 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Permit config shadowing https://review.openstack.org/479084 | 16:11 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add TenantProjectConfig object https://review.openstack.org/479073 | 16:11 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Add v3 update slides https://review.openstack.org/481134 | 16:14 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Port in tox jobs from openstack-zuul-jobs https://review.openstack.org/478265 | 16:20 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Rename .zuul.yaml to zuul.yaml https://review.openstack.org/481140 | 16:20 |
| *** hashar is now known as hasharAway | 16:27 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Move zookeeper_hosts to zookeeper section https://review.openstack.org/477591 | 16:36 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add some information about canonical_hostname https://review.openstack.org/477592 | 16:36 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix some inconsistent indentation in docs https://review.openstack.org/477593 | 16:37 |
| *** harlowja has joined #zuul | 17:13 | |
| SpamapS | so... | 17:28 |
| SpamapS | I think the thing to do is to report success of non-voting as a successful status. | 17:29 |
| SpamapS | But to call that status something like {name}-non-voting | 17:29 |
| SpamapS | And then having an overall status that is named after the pipeline seems like the one we would suggest to users that they require for merge. | 17:29 |
| SpamapS | That way the status summary on a job that failed non-voting but passed others is green, and shows all of the voting and non-voting results, but then you have the overall pipeline's success as the github permissions latch. | 17:30 |
| SpamapS | I think if it's done any other way, it will feel half-assed to github users. | 17:31 |
| SpamapS | If you roll them all up.. it feels coarse. If you report fails that don't stop the gate, it erodes trust. | 17:31 |
| jlk | mordred: SpamapS: would love to chat about github reporting if y'all want to listen for a few minutes | 17:50 |
| jlk | although I thik I see you've covered some of the ground on what I have to offer | 17:50 |
| * jlk should read more | 17:51 | |
| SpamapS | jlk: I think you could probably verify or repudiate some of the assumptions I made. | 17:54 |
| SpamapS | But my observations tell me it should work fine this way. | 17:54 |
| jlk | Yeah so there's a couple concerns. | 17:54 |
| jlk | There's visible noise of a lot of different statuses being set on a PR | 17:54 |
| jlk | and how github rolls them up into a generic red/green | 17:54 |
| jlk | There's also the difference in views, of a user with write access vs a user without | 17:55 |
| jlk | (so what a contributor sees vs a reviewer) | 17:55 |
| jlk | and chasing the myriad of potential status for gate requirements would be ... difficult, as the user could change jobs as part of the PR | 17:55 |
| jlk | so I do like the idea of a single zuul rollup pipeline status | 17:55 |
| jlk | Good news is that status setting does not generate email | 17:56 |
| jlk | Bad news is that it's hard to highlight the "special" status, the one that might have a link to a landing page that shows all the jobs for the change with links into the logs and such. It'll get lost in the noise of the other statuses. | 17:57 |
| jlk | granted, this all really needs end users' feedback to verify our assumptions. | 17:58 |
| jlk | My assumption is that we care about individual jobs when something fails, but don't really care about it when something passes. So our UX in bonny has been to use a single status with a URL to a collection of the job logs. On pass, green status. On fail, red status _PLUS_ a comment in the PR outlining all the jobs that ran with individual links to logs, so you can quickly see which job failed and jump right to it. | 17:59 |
| jlk | that way a github notification DOES go out that CI failed, so the consumers can do something abou tit | 17:59 |
| jlk | I honestly feel like we should take what we have today, expose it to end users actually using it day to day, and then gather feedback about how the UX is, where they struggle, etc.. and then make any necessary changes after that. | 18:02 |
| SpamapS | jlk: question: can we delete statuses? | 18:08 |
| jlk | I believe so, double checking | 18:08 |
| SpamapS | because I'd want those reds to disappear on success | 18:08 |
| jlk | actually, no | 18:08 |
| jlk | well, sort of | 18:08 |
| SpamapS | (I mean if we had red for failing jobs but nothing for passing) | 18:09 |
| jlk | You can set another status, and it'll mask a previous one | 18:09 |
| jlk | github exposes the last set status for a given user/context | 18:09 |
| jlk | if you've set a fail, you can mask it with a pending or success, but you cannot delete it | 18:09 |
| SpamapS | I think the way it is now is probably fine, and you're right: let's just get somebody using it. | 18:09 |
| SpamapS | But I also think that users will want more granular statuses in the Github status API, and not in comments. | 18:10 |
| SpamapS | so, yeah, we just need some data | 18:10 |
| openstackgerrit | Jeremy Stanley proposed openstack-infra/zuul master: Include basic rsync stats in ansible log https://review.openstack.org/481179 | 18:10 |
| fungi | not a fan of adding "features" to teh zuul 2.x ansible launcher at this stage, but ^ is pretty minimal and may help us get a handle on the log volume | 18:12 |
| fungi | gives users something they can look at now to see how much space a particular job's logs occupy | 18:12 |
| dmsimard | clarkb: so re: logical split of resources in nodepool, I put on my caving hat, turned the flashlight on and delved into history to find this: https://github.com/openstack-infra/project-config/blob/98f5d32311717d696f582ae0c5b819354b89840a/nodepool/nodepool.yaml#L448 | 18:27 |
| dmsimard | clarkb: that doesn't tell me if the stuff ended up being the same tenant or not, though. I would venture to say it wasn't ? | 18:27 |
| clarkb | dmsimard: b1 through b5 were all the same tenant | 18:30 |
| dmsimard | so the purpose of the different "providers" was just to put them on different networks within the same tenant ? | 18:31 |
| dmsimard | so in practice nodepool was uploading images like 5 times to the same tenant ? | 18:31 |
| clarkb | dmsimard: yes | 18:39 |
| mordred | jeblair: we may want to put a link to https://docs.openstack.org/infra/zuul/feature/zuulv3/ in the readme - the docs we just landed publish there | 19:07 |
| mordred | jlk, SpamapS: I agree this is a needs-user-feedback item for sure | 19:09 |
| clarkb | oh I guess we do do the flavor thing in osic too, but its same size just different disks | 19:09 |
| clarkb | dmsimard: ^ | 19:09 |
| * dmsimard looks | 19:09 | |
| mordred | jlk, SpamapS: today's human's feedback was basically "jenkins reports each job and we really like that" - so we may want to figure out a way to expose a mechanism for a user to express the model they want | 19:10 |
| mordred | like, I like what you're doing in bonny and that seems like what I'd prefer | 19:10 |
| jlk | yeah, we're just going to have to put it in front of people, show a passing report and a failing report | 19:10 |
| jlk | I just found something new we need to consider. | 19:11 |
| jlk | Github reviews have a .... quirk. | 19:11 |
| dmsimard | mordred: I think one of the weaknesses of Zuul is periodic jobs | 19:11 |
| jlk | unlike Gerrit, a github reviewer can first leave either an approved or a request_changes review. | 19:11 |
| dmsimard | mordred: It will run them, but trying to determine the history/trend of a periodic job is non-trivial (or impossible) | 19:12 |
| dmsimard | Whereas I guess this is core jenkins | 19:12 |
| jlk | THEN they can leave a "comment" review, and that comment review will not mask / replace the "approved" or "changes_requested" status of the PR. A comment review does not mask previous reviews. | 19:12 |
| mordred | dmsimard: yup. tristanC has written a dashboard for zuul v3 which should fix this | 19:13 |
| * dmsimard nods | 19:13 | |
| dmsimard | I've seen it :D | 19:13 |
| jlk | So our algorithm which simply takes the last provided "review" from a user as their vote is wrong, it needs to NOT take a comment, if there was a previous vote yes/no on the matter. | 19:13 |
| clarkb | dmsimard: right today you have to rely on graphite, logs, and email | 19:13 |
| mordred | dmsimard: :) | 19:14 |
| fungi | lack of more detailed trending in zuul was always something we figured would get sorted once the mysql reporter existed | 19:14 |
| mordred | https://review.openstack.org/#/c/466561/4 <-- should make us much happier there | 19:14 |
| fungi | since it didn't preserve state itself, it couldn't really provide history | 19:15 |
| mordred | jlk: oh fun | 19:15 |
| mordred | fungi: ++ | 19:15 |
| mordred | jlk: and yes - I think putting it in front of people and gettting feedback will be a huge win :) | 19:15 |
| jlk | alright, I'm going to update our review algo to handle this comment masking. | 19:16 |
| fungi | on its face that doesn't sound a whole lot different from gerrit | 19:18 |
| fungi | i can leave a code review or workflow vote, and then i can follow up with additional comments which don't set any vote, but my prior vote carries over | 19:19 |
| mordred | jeblair: when you get back - https://review.openstack.org/#/c/481014/ doesn't show the console output from the 'echo "Only on the OSD nodes $(hostname)"' task | 19:19 |
| mordred | jeblair: it's worth noting that this is a) a multi-node job b) a task in the second play of a playbook | 19:20 |
| jlk | fungi: oh? hrm, I thought a comment (+0) would replace the +/- 1 | 19:23 |
| jlk | oh wow, github just added code owners, where you can request / require reviews from specific github users on changes to specific paths of your project, and it looks like you can include people outside your org. | 19:23 |
| mordred | jlk: that's neat | 19:33 |
| jlk | I'm wrong, doesn't work for people outside your org | 19:33 |
| jlk | oh | 19:43 |
| jlk | but in an org repo (not a personal repo) you can add collaborators that have read access, so you'll be able to assign and require reviews of them, without giving them full write access to the repository. | 19:44 |
| fungi | jlk: well, that's a quirk of the interface in gerrit, i believe. the api allows you to add comments without setting a vote, and that doesn't do the same thing as voting 0 | 19:44 |
| jlk | oh okay. | 19:45 |
| fungi | the gerrit webui just happens to always give you the option of setting a vote, defaulting to your last vote on that patch set | 19:45 |
| fungi | and leaving it alone (not changing your vote back to 0 in the webui) doesn't emit any vote in the event stream for gerrit 2.11 at least (though later gerrit releases i believe always echo the last vote for each label now or something like that?) | 19:46 |
| jeblair | SpamapS: in working on incorporating the per-build ssh keys, i had some further thoughts. i took those and synthesized them with some suggestions from mordred and i've created 2 stories. they aren't mutually exclusive -- we can do none, either, or both. (though i think for the specific case of ssh keys even if we implement both, i would probably only use one). | 21:42 |
| jeblair | SpamapS: https://storyboard.openstack.org/#!/story/2001110 and https://storyboard.openstack.org/#!/story/2001111 | 21:43 |
| jeblair | SpamapS: let me know what you think about those | 21:43 |
| * mordred has opinions, but will let SpamapS read the stories first | 21:43 | |
| jeblair | fungi, clarkb, jlk, tobiash, jamielennox: you may be interested in those too ^ | 21:44 |
| jeblair | erm, somehow all of the docs changes have started timing out on py35 jobs | 21:47 |
| fungi | i likely have opinions but need to go cook dinner | 21:49 |
| jeblair | if we need to make a spec for one or both of these, i'm fine with that. just say the word. :) | 21:49 |
| fungi | i've pulled up both stories so i can read them after dinner | 21:50 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Don't log starting to log messages to build log https://review.openstack.org/479439 | 21:57 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Move status_url to webapp config section https://review.openstack.org/480759 | 22:03 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add docs on allow-secrets https://review.openstack.org/480726 | 22:03 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix indentation error in docs https://review.openstack.org/480740 | 22:03 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Clarify canonical_hostname documentation https://review.openstack.org/479020 | 22:03 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Rename git_host to server in github driver https://review.openstack.org/477594 | 22:03 |
| jeblair | aha, found it. the git_host change conflicted with the github depends-on stuff that merged | 22:04 |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Port in tox jobs from openstack-zuul-jobs https://review.openstack.org/478265 | 22:05 |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Rename .zuul.yaml to zuul.yaml https://review.openstack.org/481140 | 22:05 |
| jlk | oooh, sorry about that | 22:05 |
| clarkb | jeblair: reading the stories, will openstack create keys for you? it is implied that it will but I thought you had to supply the key? | 22:05 |
| clarkb | huh openstack client says if you don't pass in a file then it creates one | 22:06 |
| clarkb | not sure if that happens cloud side or client side | 22:07 |
| jlk | Pretty sure nova generates a key | 22:07 |
| jlk | doesn't happen client side | 22:07 |
| clarkb | one gotcha if nova is doing it is that we may still have to generate them locally if we want to support not openstack | 22:08 |
| clarkb | api docs say one is created for you so must be nova side | 22:09 |
| jlk | Gets created in nova/compute/api.py | 22:10 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add TenantProjectConfig object https://review.openstack.org/479073 | 22:11 |
| jeblair | clarkb: exactly -- with nova i think we can do it either way (local or openstack-generated). other drivers may need to do some local generation. | 22:12 |
| clarkb | we would also need glean to set keys for more than root, or start using root everywhere | 22:14 |
| clarkb | assuming we go with the nodepool option instead of the zuul option | 22:15 |
| jeblair | clarkb: that's true. does it support that? (though, moreover, i'm guessing cloud-init doesn't...) | 22:16 |
| jeblair | so if that's not an option, then even if you do the nodepool thing, you almost still need the base job thing to bootstrap the zuul user. | 22:17 |
| clarkb | jeblair: cloud init does, glean doesn't | 22:17 |
| jeblair | oh ok. | 22:17 |
| jeblair | how does that work with the keypair extension? | 22:17 |
| clarkb | jeblair: cloud init will create and configure arbitrary users (its why we end up wtih ubuntu, centos, fedora, cloud, etc) but glean will only add keys to root | 22:17 |
| clarkb | jeblair: I think with cloud init it will add the keys provided by nova to any configured user | 22:17 |
| jeblair | okay. so that still may not be exactly what we want. :) | 22:18 |
| clarkb | jeblair: with glean we'd have to come up with some method to differentiate root/not root or also configure both | 22:18 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Permit config shadowing https://review.openstack.org/479084 | 22:18 |
| jeblair | clarkb: that starts to tie us a bit to glean, or at least makes using non-glean a bit more awkward | 22:19 |
| mordred | I'd like to avoid anything that involves putting more logic anywhere near cloud-init or glean | 22:31 |
| mordred | like, as in, I _very_ strongly am opposed to anything that requires any such smarts | 22:31 |
| mordred | we can already set, in a nodepool image config, what the remote user is - in setting up nodepool to use keypairs, that can mean "tell nodepool what user keypairs are going to get added to on your image" | 22:34 |
| mordred | it's not important that we be able to set which user that is at runtime- only that we know what user it is | 22:34 |
| jeblair | mordred: yeah, but if we rely on glean/cloud init, we're setting the key for root too, which we don't want to do | 22:35 |
| mordred | if someone wants to use a stock ubuntu cloud image that has cloud-init set up an ubuntu user and put keypairs on it, they just need to set user=ubuntu in their config | 22:35 |
| jeblair | i mean, i guess it's okay, we just need to have our 'revoke-sudo' role *also* remove the key from root's authorized_keys | 22:35 |
| mordred | jeblair: we're not - we're setting the keypair on the user that the image defines as the user to put the keys on | 22:35 |
| jeblair | mordred: glean/cloud-init do that, and clarkb says cloud-init will do it for all the users | 22:36 |
| jeblair | mordred: are you saying nodepool can somehow tell openstack and/or cloud-init to only add the keypair to the zuul user? | 22:36 |
| clarkb | what nodepool/nova do is put keypairs in the metadata I do not think those come with any additional metadata | 22:37 |
| mordred | this is what cloud-init puts in to root's .ssh/authorized-keys on a cloud-init image with a keypair: | 22:37 |
| mordred | no-port-forwarding,no-agent-forwarding,no-X11-forwarding,command="echo 'Please login as the user \"ubuntu\" rather than the user \"root\".';echo;sleep | 22:37 |
| mordred | 10" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDLsTZJ8hXTmzjKxYh/7V07mIy8xl2HL+9BaUlt6A6TMsL3LSvaVQNSgmXX5g0XfPWSCKmkZb1O28q49jQI2n7n7+sHkxn0dJDxj1N2oNrzNY7 | 22:37 |
| mordred | pDuPrdtCijczLFdievygXNhXNkQ2WIqHXDquN/jfLLJ9L0jxtxtsUMbiL2xxZEZcaf/K5MqyPhscpqiVNE1MjE4xgPbIbv8gCKtPpYIIrktOMb4JbV7rhOp5DcSP5gXtLhOF5fbBpZ+szqrTVUcBX0o | 22:37 |
| mordred | TYr3iRfOje9WPsTZIk9vBfBtF416mCNxMSRc7KhSW727AnUu85hS0xiP0MRAf69KemG1OE1pW+LtDIAEYp mordred@camelo | 22:37 |
| clarkb | mordred: that is an ubuntu specific cloud init config but yes | 22:37 |
| clarkb | mordred: it also sets taht key on the ubuntu user | 22:37 |
| mordred | yes. it's specific to how cloud-init is setup | 22:37 |
| mordred | which we have no real way of knowing | 22:38 |
| clarkb | what we would have ideally is the infra root rooter keys on root user and the zuul/nodepool key on the zuul user | 22:38 |
| clarkb | we could make glean do this (possibly based on the key comment) | 22:38 |
| jeblair | mordred, jlk: i'm not finding a built-in ansible module to manipulate yaml (or even json) files; does that jive with your understanding? | 22:39 |
| mordred | yah - but that makes humans using zuul without building images hard and requires that all zuul/nodepool users build images with glean | 22:40 |
| jlk | to manipulate? I don't think so. THere are filters you can use to generate yaml/json from data structures | 22:40 |
| mordred | ya- and I think you can read yaml/json files too? | 22:40 |
| jeblair | jlk: oh, that may be a good start, can you point me at those? | 22:40 |
| jeblair | mordred: and if that's the case, that's the complete set, actually :) | 22:41 |
| jlk | http://docs.ansible.com/ansible/playbooks_filters.html#filters-for-formatting-data | 22:41 |
| jlk | There's from_yaml and to_yaml (and the same for json) | 22:41 |
| jlk | slurp module can read the contents of a file into a variable, and you can do whatever to it from there | 22:42 |
| jeblair | jlk: so slurp -> from yaml -> manipulate -> to yaml -> file would be the whole cycle | 22:42 |
| jeblair | cool, i will play with that for a bit, thanks :) | 22:42 |
| jlk | sounds about right. | 22:43 |
| jeblair | (i'm thinking, however, that we may want to make a yaml-editing module so folks can easily set return data from jobs. we can build that into zuul pretty easily. maybe upstream it?) | 22:43 |
| jlk | slurp -> from yaml -> manipulate (set_fact?) -> copy content: "{{ data | to_yaml }}" | 22:44 |
| mordred | jeblair: out of curiosity - what's the read portion of this? | 22:44 |
| clarkb | mordred: ya I think its a good argument for not using that system | 22:45 |
| clarkb | mordred: and instead using the required base job to bootstrap | 22:45 |
| jeblair | mordred: if more than one playbook wants to return data. obviously we'll start with just the log url, but if we want to have base/post-playbook set that, and then also have flake8 return line annotations, it would modify the existing yaml return file | 22:45 |
| mordred | jeblair: ahhh - so - in my brain, I was thinking that values set in one return file would be added to vars passed in to the next playbook - and then a playbook can just write a file without having to care about what's there before | 22:46 |
| mordred | jeblair: you're thinking of keeping a file that accumulates return data over time? | 22:47 |
| jlk | hrm, hopefully not using lineinfile right? what method were you going to do to add more content to a file? | 22:47 |
| jeblair | mordred: yep, that was what i was imagining. | 22:47 |
| jlk | oh, if hte variable carries forward, I guess you just write the whole thing every time | 22:48 |
| jeblair | jlk: well, now that i'm looking at this, i'm thinking of going straight to a zuul_return ansible module and doing it in python. then it's just "tasks: [zuul_return: {key: foo, value: bar}]" and the module does the read/write itself. | 22:50 |
| jeblair | though that suggests that we should use json, rather than yaml, due to it being in the python2 stdlib. | 22:50 |
| jlk | nod | 22:50 |
| jeblair | (which is fine, this isn't especially user-facing anyway) | 22:50 |
| mordred | we'll need to special-case zuul_return in the security exclusions | 22:51 |
| mordred | oh - wait- no we won't | 22:51 |
| mordred | sorry | 22:51 |
| jeblair | mordred: do you think we should go with your approach of implicitly adding it to each run? ... actually... | 22:51 |
| mordred | brain stupid | 22:51 |
| jeblair | mordred: we can sort of combine the two approaches | 22:51 |
| mordred | jeblair: yah- I like having a zuul_return module in our library | 22:51 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Handle GitHub comment reviews carefully https://review.openstack.org/481314 | 22:51 |
| jeblair | mordred: we can have zuul_return read/modify/write each time it is invoked. but we can *also* have the executor "-e" the file on each run | 22:52 |
| jeblair | so you implicitly have read access to any previously "returned" values. | 22:52 |
| mordred | jeblair: but I think you'll want to zero the file out - because the executor wants to read the data after each playbook - and if it accumulates, the executor won't know which return data is relevant | 22:52 |
| jeblair | mordred: the executor won't do anything with the data until the end of the job. then it returns it to the scheduler. | 22:53 |
| mordred | so - like - if the flake8 job returns line annotatoins, how would the executor know during the next post playbook that that playbook didn't ALSO return flake8 annotations | 22:53 |
| mordred | jeblair: ah - I see why you're thinking accumulate now | 22:53 |
| jeblair | mordred: it doesn't care. as far as zuul is concerned, it's the *job* that returns things. | 22:54 |
| mordred | nod | 22:54 |
| jeblair | tell you what, let's just leave the idea of loading the variables between playbooks on the shelf for now | 22:54 |
| jeblair | it's easy to add later if we want | 22:54 |
| mordred | jeblair: also - incidentally - jlk taught me the other day that -e has the strongest precedence and cannot be overwritten | 22:54 |
| jeblair | mordred: good point | 22:54 |
| mordred | jeblair: so if we do want to inject the variables, we'd want to do it in the inventory and not via -e most likely (/me learned things!) | 22:55 |
| jlk | you did say "read" access | 22:55 |
| jlk | just not write access :) | 22:55 |
| mordred | :) | 22:55 |
| jeblair | mordred, jlk: okay, i'm not going to get this done today, but i know what i'm doing tomorrow. thanks. :) | 22:56 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Run the new fancy zuul-jobs versions of jobs https://review.openstack.org/480692 | 22:58 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove unnecessary loop in github test https://review.openstack.org/481315 | 23:00 |
| mordred | jlk, jeblair: just had an idea about github status and non-voting jobs | 23:07 |
| jlk | *popcorn.gif* | 23:09 |
| mordred | what about a hybrid - what if we report a status for every voting job - and report status for non-voting jobs if they pass. for _any_ job that fails, leave a comment on the PR- but group them by voting/non-voting - so you get "these jobs failed" and "these jobs failed but are non-voting" | 23:09 |
| mordred | it's similar to what bonny is doing now - in that it puts failures into the comment stream so people get the emails | 23:10 |
| mordred | and it get per-job status for all of the jobs that vote, so the github rollup is accurate- but we dont' lose the data about failing non-voting jobs | 23:10 |
| mordred | (and then maybe ALSO report a top-level url for the change in the status too - so that there is a link to the change's dashboard page should one exist) | 23:11 |
| jlk | My thought is that the "roll up" status will get lost in the noiuse | 23:13 |
| jlk | noise | 23:13 |
| jlk | I think a roll up status is critical, as you can set branch protection to care about said roll up status, and you can set pipeline requirements (and triggers) for that roll up status, since at the pipeline config level you'll never truly know the set of statuses that may come from jobs. | 23:14 |
| * jlk EODs | 23:21 | |
| mordred | jlk: ++ | 23:21 |
| mordred | jlk: enjoy the EOD | 23:21 |
| jeblair | mordred: i like your idea. i could really use some demos of these because it's getting hard for me to imagine them. and maybe the answer is one or more behavior toggles. | 23:22 |
| mordred | jeblair: yah - I think some behavior toggles are going to be helpful here - as i imagine this is an area where folks will have *opinions* about what they prefer | 23:23 |
| *** hasharAway has quit IRC | 23:37 | |
| clarkb | I wonder if we couldn't use signed ssh keys to get around some of these problems. Issue is going to be shipped the revocation updates after ssh is succesful. Also that is racy and could allow ssh elsewhere while you update. Thats too bad it doesn't help | 23:38 |
| jeblair | clarkb: i hadn't thought of that at all. thanks for brainstorming it out loud. :) | 23:42 |
| clarkb | jeblair: ya I'm pretty sure it won't help because cert revocation is silly | 23:43 |
| clarkb | jeblair: the other idea is use the ldap key stuff but that requires special openssh iirc and would require everyone run an ldap | 23:47 |
| clarkb | all these alternatives seem more complicated than just setting a key via a base job | 23:48 |
| jeblair | yeah, just doing that and skipping the nodepool thing for now is growing on me. | 23:49 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Remove the old tox-linter job https://review.openstack.org/481322 | 23:52 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Duplicate the zuul-tox jobs to just be named tox https://review.openstack.org/481323 | 23:52 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Remove the transitionary zuul-tox jobs https://review.openstack.org/481324 | 23:52 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Remove old tox jobs https://review.openstack.org/481325 | 23:52 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Consume tox instead of zuul-tox https://review.openstack.org/481326 | 23:52 |
| mordred | jeblair: ok. there's a crazy dependency chain for you | 23:52 |
| mordred | (oh, the openstakc-zuul-jobs patches aren't reported in here) | 23:54 |
| mordred | jeblair: https://review.openstack.org/#/q/topic:rename-dance | 23:54 |
| mordred | jeblair: there are times when I wish we just had a nice depends-on graph visualizer :) | 23:55 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!