| jeblair | jlk: the 'zuul' trigger is intended to handle cases like that, though that specific case is not implemented (yet) | 00:04 |
|---|---|---|
| jlk | yeah, looks like I'd need some sort of event that marks success of a pipeline | 00:05 |
| jlk | jeblair: I'll tell you what I'm trying to solve. Github has the concept of "status" that can be set on the commit(s) of a PR. This is typically what jenkins would set or travis, or zuul. Set to pending as a reporter, set to success on success of the check gate. | 00:10 |
| jlk | jeblair: I'd like our gate pipeline to be triggered by the check pipline completing successfully | 00:10 |
| jlk | and when we set status to success, the webhook event that github sends back doesn't reference the /pull-request/, it references the /commit/, without any breadcrumbs back to which PR(s) has that commit as the head. | 00:11 |
| jlk | so instead of using github set status -> respond to status webhook event as a round-about way to trigger ourselves, I was hoping to short-circuit it within zuul itself. | 00:12 |
| mordred | jlk: out of curiosity - why have the gate pipeline be triggered by the check? wouldn't that essentially make the gate pipeline triggered by patchset upload and bypass human review? | 00:15 |
| mordred | jlk: (just making sure I grok what you're aiming to accomplish in my brainhole) | 00:15 |
| jlk | No, because the gate pipeline would have a requirement on the human review being completed | 00:15 |
| mordred | jlk: ahhhhhh. thank you - I grok now | 00:15 |
| jlk | (by a human (with write access) providing a positive review) | 00:15 |
| jeblair | so this is for the case where human review happens before check finishes, but check is still a pre-req for gate | 00:15 |
| mordred | jlk: also - wow, with the event not referencing the pull-request :( | 00:15 |
| jlk | this may help: https://github.com/BonnyCI/projman/wiki/GitHub-Pull-Request-Check-and-Gate-Design | 00:16 |
| jlk | mordred: yeah, it's weird. THe status belongs to the commit object, not the pull request object | 00:16 |
| jlk | and multiple branches may have that commit object | 00:16 |
| jlk | and even more fun, if you close a PR with a commit that has a status, then re-open it, the commit _still_ has that status, like CI was already done! | 00:17 |
| mordred | jlk: I really with the gh review system was more mature - it's so clearly a v0.1 | 00:17 |
| jeblair | jlk: aside from the event, does the actual data model also have the status associated with the commit object? | 00:18 |
| jlk | jeblair: yes | 00:18 |
| jeblair | like, if you okay a commit that is in 2 prs, and you look at both prs, they both say 'ok'... | 00:18 |
| jlk | https://developer.github.com/v3/repos/statuses/ | 00:18 |
| jlk | jeblair: that's correct | 00:18 |
| jlk | it's _weird_ | 00:18 |
| jlk | I'm really kind of tempted to make Zuul drop a positive review instead of status | 00:19 |
| jeblair | jlk: i think the zuul trigger might be an okay thing to do, but also, i feel like being able to respond to that event might be a good thing generally, so i might advocate for the github driver being able to create synthetic events for all prs which contain a commit whose status just changed | 00:19 |
| jlk | because that's attached to the PR, and it fits more easily with the "approval" model. | 00:19 |
| jeblair | jlk: that might be a good option too. | 00:19 |
| jlk | it's just a departure from how github works to date. | 00:19 |
| jlk | and it's noisy. Setting status doesn't generate emails, adding a review does | 00:20 |
| mordred | I mean - _also_ dropping a status onto the commit is a way of saying "this commit has been tested" - and it is tru that that commit will have been tested no matter which PR it might be included in | 00:20 |
| jlk | yeah | 00:21 |
| jlk | I proposed doing status + review | 00:22 |
| jlk | and using review as the trigger/requirement | 00:22 |
| mordred | jlk: any thought to just abusing PR tags like ansible does with ansibot? we could also have zuul clear previous vote-tags when someone uploads new patches to a PR (since the reviews don't auto-unset) | 00:23 |
| * mordred is just saying words now - please feel free to ignore if they are not useful words | 00:24 | |
| jlk | Tags requires that consumers of the service pre-create the tags, or grant our integration permission to create the tags | 00:25 |
| mordred | ah. gotcha | 00:25 |
| jlk | downside of tags and reviews | 00:25 |
| jlk | they don't get reset when the pr changes (new code uploaded) | 00:25 |
| jlk | so really, I think we still have to use status as a requirement, or that the positive review was from after the pr head was last changed | 00:26 |
| jlk | if zuul happened to be down when the head changed, or if the hook got dropped somehow, I don't want zuul to be relying on stale data | 00:26 |
| mordred | ++ | 00:28 |
| jlk | (not even going into how hard this is because each commit can have _multiple_ statuses from multiple providers, and we need a way to scope it down to _which_ status is required) | 00:28 |
| *** jamielennox|away is now known as jamielennox | 00:37 | |
| *** harlowja has joined #zuul | 00:52 | |
| *** saneax is now known as saneax-_-|AFK | 01:33 | |
| *** adam_g_ has joined #zuul | 02:40 | |
| *** Cibo has quit IRC | 02:41 | |
| *** adam_g has quit IRC | 02:41 | |
| *** saneax-_-|AFK has quit IRC | 02:41 | |
| *** jesusaur has quit IRC | 02:41 | |
| *** rcarrillocruz has quit IRC | 02:41 | |
| *** jamielennox has quit IRC | 02:41 | |
| *** adam_g_ is now known as adam_g | 02:41 | |
| *** Cibo has joined #zuul | 02:49 | |
| *** rcarrillocruz has joined #zuul | 02:50 | |
| *** jesusaur has joined #zuul | 02:51 | |
| *** saneax-_-|AFK has joined #zuul | 03:10 | |
| *** jamielennox|away has joined #zuul | 03:12 | |
| *** jamielennox|away is now known as jamielennox | 03:12 | |
| *** bhavik1 has joined #zuul | 05:22 | |
| *** saneax-_-|AFK is now known as saneax | 05:24 | |
| *** bhavik1 has quit IRC | 05:29 | |
| *** abregman has joined #zuul | 06:31 | |
| *** hashar has joined #zuul | 07:23 | |
| *** isaacb has joined #zuul | 07:36 | |
| *** isaacb has quit IRC | 08:01 | |
| *** hashar is now known as hasharAway | 08:28 | |
| *** openstackgerrit has quit IRC | 08:33 | |
| *** abregman is now known as abregman|mtg | 08:39 | |
| *** isaacb has joined #zuul | 08:52 | |
| *** isaacb has quit IRC | 08:54 | |
| *** isaacb has joined #zuul | 08:55 | |
| *** hasharAway is now known as hashar | 09:14 | |
| *** abregman|mtg is now known as abregman | 09:27 | |
| *** isaacb has quit IRC | 10:29 | |
| *** isaacb has joined #zuul | 11:31 | |
| *** isaacb has quit IRC | 11:43 | |
| *** isaacb has joined #zuul | 12:42 | |
| *** saneax is now known as saneax-_-|AFK | 12:49 | |
| *** hashar has quit IRC | 13:25 | |
| *** hashar has joined #zuul | 13:25 | |
| *** isaacb has quit IRC | 13:46 | |
| *** saneax-_-|AFK is now known as saneax | 13:47 | |
| *** isaacb has joined #zuul | 13:50 | |
| *** isaacb has quit IRC | 13:51 | |
| *** openstackgerrit has joined #zuul | 14:11 | |
| openstackgerrit | Lenny Verkhovsky proposed openstack-infra/nodepool: Fixed typo in info msg https://review.openstack.org/418435 | 14:11 |
| *** saneax is now known as saneax-_-|AFK | 14:46 | |
| openstackgerrit | Merged openstack-infra/nodepool: Fixed typo in info msg https://review.openstack.org/418435 | 14:54 |
| *** bhavik1 has joined #zuul | 15:14 | |
| *** bhavik1 has quit IRC | 15:19 | |
| *** abregman has quit IRC | 15:48 | |
| *** isaacb has joined #zuul | 15:57 | |
| *** isaacb_ has joined #zuul | 16:24 | |
| *** isaacb has quit IRC | 16:28 | |
| *** hashar is now known as hasharAway | 17:03 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add nodepool integration test script https://review.openstack.org/422288 | 18:07 |
| *** Shuo has quit IRC | 18:17 | |
| *** isaacb_ has quit IRC | 18:29 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Separate driver interfaces and make abstract https://review.openstack.org/418554 | 18:36 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Re-enable zuultrigger test https://review.openstack.org/408848 | 18:36 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Reorganize connections into drivers https://review.openstack.org/408849 | 18:36 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add Drivers to documentation https://review.openstack.org/422804 | 18:36 |
| jeblair | jamielennox, jlk, SpamapS, jhesketh, mordred: ^ you may be interested in that stack | 18:37 |
| jeblair | the documentation change is the only new change, the others are rebases of existing changes to fix a merge conflict | 18:38 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Fix for launched node counting https://review.openstack.org/422807 | 18:39 |
| SpamapS | mmmmmm driver fixes | 18:40 |
| jeblair | clarkb, mordred: should i hold merging the new nodepool launcher stuff from Shrews (421485) for reviews from you or proceed with merges? | 18:42 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Add nodepool integration test script https://review.openstack.org/422288 | 18:43 |
| jeblair | Shrews: 422283 and child warrant your attention when you have a moment (not urgent) | 18:44 |
| clarkb | I'd like to read it, I can do that now | 18:45 |
| Shrews | jeblair: this statement from your spec is a bit hand-wavey: "Nodepool will then decide whether the nodes should be returned to the pool, rebuilt, or deleted according to the type of node and current demand." | 18:45 |
| Shrews | jeblair: i'm not sure how to determine the return-vs-delete aspect | 18:46 |
| Shrews | or rebuilt, i guess | 18:46 |
| jeblair | Shrews: ah, it was meant to be a fairly forward-thinking statement. currently, we always delete a used node, but we'd like to support 'nova rebuild' in the future. and maybe even do other things to nodes after zuul is done with them. but to keep focus for now, i think we should aim for parity with the current functionality and ignore rebuild for now. so every 'used' node should be deleted. | 18:49 |
| jeblair | Shrews: however, zuul *can* return nodes to us that are not used. | 18:50 |
| jeblair | Shrews: so i think every 'ready' node should be put back in the pool for potential re-assignment. | 18:50 |
| Shrews | jeblair: ok. that definitely simplifies things for the moment | 18:51 |
| jeblair | Shrews: (also, 'in-use' should be treated as 'used'. if we get an 'in-use' node back, it probably means zuul died) | 18:51 |
| Shrews | jeblair: in-use + unlocked, right? | 18:52 |
| jeblair | Shrews: right | 18:52 |
| Shrews | k | 18:52 |
| Shrews | jeblair: why do i get "DIB_RELEASE not set correctly" when i run the builder with your --fake option? | 19:00 |
| *** hasharAway has quit IRC | 19:01 | |
| Shrews | i suspect my nodepool.yaml is b0rked | 19:01 |
| Shrews | - name: devstack-trusty | 19:02 |
| Shrews | rebuild-age: 360 | 19:02 |
| Shrews | elements: | 19:02 |
| Shrews | - ubuntu | 19:02 |
| Shrews | release: trusty | 19:02 |
| Shrews | but IIRC, that worked fine under the real dib | 19:02 |
| clarkb | Shrews: is there a traceback? usually I think problems like that have been related to yaml typign issues when trying to emit things to the env which had to be strings | 19:04 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Fix for launched node counting https://review.openstack.org/422807 | 19:04 |
| Shrews | clarkb: nope, no tb | 19:05 |
| jeblair | yeah, hrm, that looks good to me | 19:06 |
| Shrews | oh, fake requires it to be 21 | 19:06 |
| jeblair | i did just realize that due to a .gitignore, 422284 is missing the nodepool.yaml i wrote | 19:06 |
| jeblair | oh | 19:06 |
| jeblair | yeah, the one i wrote has that | 19:06 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Add files for zuul-nodepool integration test https://review.openstack.org/422284 | 19:07 |
| jeblair | that might be why the daemon didn't start up in the integration test :) | 19:08 |
| Shrews | it should start, but it will just keep complaining and attempting to build | 19:09 |
| jeblair | Shrews: i meant the missing file | 19:09 |
| Shrews | oh | 19:09 |
| jeblair | fixed in latest ps ^ | 19:09 |
| Shrews | jeblair: lgtm now | 19:11 |
| Shrews | jeblair: also, 422807 had an extraneous : causing failure. removed it | 19:13 |
| jeblair | whoops | 19:13 |
| clarkb | ok I reviewed 421485, left some comments and a +2, will let you decide if worth addressing anything before approving | 19:15 |
| jeblair | clarkb: ++. i responded with feedback on those. | 19:20 |
| Shrews | clarkb: thx. i think you're correct on the imageAvailable comment. i'll make the next review change that to query ZK for the list of actually uploaded and ready images. | 19:23 |
| *** isaacb_ has joined #zuul | 19:24 | |
| Shrews | because we can't assume the image is actually available, even if it *is* listed in the config | 19:24 |
| * Shrews slams head against wall in repentance | 19:25 | |
| jeblair | i mean, we can assume... but.... :) | 19:28 |
| *** openstackgerrit has quit IRC | 19:33 | |
| Shrews | oh for crying out loud... another stray : | 19:37 |
| * Shrews should just give up on today | 19:37 | |
| *** openstackgerrit has joined #zuul | 19:37 | |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Fix for launched node counting https://review.openstack.org/422807 | 19:38 |
| * jlk opens up driver change to look at | 19:42 | |
| jlk | jeblair: I meant to ask. That erroneous comment in tests I found yesterday, do you want that fixed on master or the v3 branch? | 19:42 |
| jlk | (or both) | 19:42 |
| jeblair | jlk: let's do both (we have and will do occasional master -> v3 merges, but 2 changes sounds simpler here) | 19:45 |
| jlk | can do! | 19:45 |
| *** isaacb_ has quit IRC | 20:09 | |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Query ZooKeeper to determine image availability https://review.openstack.org/422857 | 20:31 |
| Shrews | clarkb: jeblair: that turned out to be much simpler than i expected ^^^ | 20:31 |
| *** hashar has joined #zuul | 20:35 | |
| *** hashar has quit IRC | 20:35 | |
| jeblair | ++ | 20:42 |
| *** piccobit has joined #zuul | 21:02 | |
| piccobit | hi there, I've set up a Zuul test system which works fine with Jenkins Freestyle projects, but now I've tried using Jenkins Pipelines and they aren't visible to Zuul. Any hints? | 21:05 |
| jeblair | piccobit: i've heard that jenkins pipelines are a new construct which is not seen as a job by the gearman plugin for jenkins that zuul uses, so i'm not sure there's anything easy that can be done. most of us are focused on working on building zuul v3 right now which uses ansible rather than jenkins. | 21:30 |
| *** piccobit has quit IRC | 21:49 | |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul: Correct comments in requirement test cases https://review.openstack.org/422899 | 22:34 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul: Correct comments in requirement test cases https://review.openstack.org/422900 | 22:38 |
| *** piccobit has joined #zuul | 23:37 | |
| *** piccobit has quit IRC | 23:38 | |
| *** piccobit has joined #zuul | 23:43 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!