*** threestrands has joined #zuul | 00:03 | |
*** hwoarang has quit IRC | 00:43 | |
*** threestrands_ has joined #zuul | 01:18 | |
*** threestrands_ has quit IRC | 01:18 | |
*** threestrands_ has joined #zuul | 01:18 | |
*** threestrands_ has quit IRC | 01:19 | |
*** threestrands_ has joined #zuul | 01:20 | |
*** threestrands_ has quit IRC | 01:21 | |
*** threestrands_ has joined #zuul | 01:21 | |
*** threestrands_ has quit IRC | 01:21 | |
*** threestrands_ has joined #zuul | 01:21 | |
*** threestrands has quit IRC | 01:21 | |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Add role for installing docker and configuring registry mirror https://review.openstack.org/580730 | 03:39 |
---|---|---|
*** chandankumar has joined #zuul | 05:08 | |
*** chandankumar is now known as chkumar|ruck | 05:08 | |
tobiash | corvus: I tried to reproduce the erratic behavior in a test environment so I don't have to break production during debugging but no luck so far | 05:10 |
*** pawelzny has joined #zuul | 05:39 | |
*** yolanda has joined #zuul | 05:46 | |
tobiash | corvus: I've now managed to reproduce it first in my test env and now also as a zuul test case :) | 06:07 |
tobiash | corvus: it seems that it's important to have two config projects, one depoyment specific and one shared by all deployments | 06:08 |
tobiash | corvus: it seems that it's happening if the according pipeline is defined in a different config project | 06:16 |
openstackgerrit | Vu Cong Tuan proposed openstack-infra/zuul-jobs master: Switch to stestr https://review.openstack.org/580028 | 07:13 |
*** gtema has joined #zuul | 07:14 | |
openstackgerrit | Vu Cong Tuan proposed openstack-infra/zuul-jobs master: Switch to stestr https://review.openstack.org/580028 | 07:14 |
*** fbo|off is now known as fbo | 07:15 | |
*** hashar has joined #zuul | 07:46 | |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: Add test facility to add file contents in github tests https://review.openstack.org/580948 | 07:51 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: WIP: Reproducer for master job contamination https://review.openstack.org/580949 | 07:51 |
tobiash | corvus: I have a reproducer now ^ | 07:51 |
*** zigo has quit IRC | 08:03 | |
*** electrofelix has joined #zuul | 08:04 | |
*** aspiers[m] has quit IRC | 08:04 | |
*** zigo has joined #zuul | 08:05 | |
*** threestrands_ has quit IRC | 08:12 | |
*** aspiers[m] has joined #zuul | 08:31 | |
openstackgerrit | Vu Cong Tuan proposed openstack-infra/zuul-jobs master: Remove testr https://review.openstack.org/580028 | 08:54 |
openstackgerrit | Markus Hosch proposed openstack-infra/zuul master: Fix GitHub branch protection test https://review.openstack.org/580966 | 08:56 |
openstackgerrit | Markus Hosch proposed openstack-infra/zuul master: Reduce number of reconfigurations on branch delete https://review.openstack.org/580967 | 08:56 |
*** pawelzny has quit IRC | 08:57 | |
*** pawelzny has joined #zuul | 09:02 | |
*** gtema has quit IRC | 09:02 | |
*** sshnaidm|off is now known as sshnaidm|rover | 09:02 | |
openstackgerrit | Markus Hosch proposed openstack-infra/zuul master: Reduce number of reconfigurations on branch delete https://review.openstack.org/580967 | 09:25 |
*** gtema has joined #zuul | 10:22 | |
*** gtema has quit IRC | 10:50 | |
*** gtema has joined #zuul | 10:51 | |
*** pawelzny has quit IRC | 10:54 | |
*** pawelzny has joined #zuul | 10:57 | |
*** gtema has quit IRC | 11:07 | |
*** gtema has joined #zuul | 11:10 | |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Add role for installing docker and configuring registry mirror https://review.openstack.org/580730 | 11:57 |
rcarrillocruz | folks, is there any zuul construct to say 'run this job unless neither of these list of jobs are running' | 12:08 |
*** sshnaidm|rover is now known as sshnaidm|afk | 12:08 | |
rcarrillocruz | this job would be in periodic, checking of jobs not running on check/gate | 12:08 |
rcarrillocruz | other than a mechanism specific to this, i can only think of a semaphore | 12:08 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: WIP: fix job contamination https://review.openstack.org/581019 | 12:17 |
tobiash | corvus: after hours of debugging I found out that not the cached layout is contaminated but the one that is attached to the pipeline is the wrong one and this is used under some circumstances | 12:17 |
tobiash | this is an easy fix for this by retrieving the layout from the tenant instead of the pipeline ^ | 12:18 |
tobiash | corvus: while this fix is easy I think it's the wrong fix as this hides a deeper problem and maybe further uses of the wrong layout | 12:19 |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul master: Add job to build container images using pbrx https://review.openstack.org/580160 | 12:27 |
*** rlandy has joined #zuul | 12:34 | |
openstackgerrit | caoxufeng proposed openstack-infra/zuul master: Test for commit https://review.openstack.org/581027 | 12:46 |
mordred | rcarrillocruz: semaphore would be the only thing for that I could think of | 12:49 |
mordred | tobiash: ooh, that's exciting | 12:49 |
*** toabctl has quit IRC | 12:59 | |
rcarrillocruz | oki, thx | 12:59 |
*** toabctl has joined #zuul | 12:59 | |
*** chkumar|ruck has quit IRC | 13:01 | |
*** gtema has quit IRC | 13:09 | |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul master: Add job to build container images using pbrx https://review.openstack.org/580160 | 13:22 |
rcarrillocruz | mordred: is it possible for jobs to acquire multiple semaphores, or is it just one? | 13:23 |
*** sshnaidm|afk is now known as sshnaidm|rover | 13:42 | |
tobiash | rcarrillocruz: even if it would be possible I wouldn't recommend that. This would cry for deadlocks... | 13:45 |
tobiash | rcarrillocruz: the data structure in zuul says it's one | 13:47 |
rcarrillocruz | k, thx for confirming | 13:47 |
*** zxiiro-pto is now known as zxiiro | 14:08 | |
trishnag | Hi | 14:10 |
trishnag | Can we make zuul do squash and merge when merging PRs instead of creating new commits? | 14:10 |
rcarrillocruz | tobiash: you use GH, do you know ^ ? | 14:18 |
tobiash | trishnag, rcarrillocruz: afaik zuul doesn't support this yet | 14:18 |
rcarrillocruz | ack thx | 14:18 |
trishnag | tobiash: ack thanks. | 14:19 |
mordred | trishnag: it probably shouldn't be _super_ hard to add I'd imagine, right? | 14:21 |
mordred | gah | 14:21 |
mordred | tobiash: ^^ | 14:21 |
tobiash | mordred: changing the reporter should be easy as it's just a flag | 14:22 |
corvus | zuul has a facility for specifying alternative merge strategies for the merger, so it could be implemented without too much difficulty | 14:22 |
tobiash | mordred: but I'm not sure how to do this for the correct merge-mode (is the right one already there?) | 14:22 |
corvus | http://git.zuul-ci.org/cgit/zuul/tree/zuul/merger/merger.py#n531 | 14:23 |
mordred | corvus: I think this is more about the reporter - rather than clicking 'merge' - have zuul click 'squash-and-merge' | 14:23 |
corvus | mordred: it's both | 14:24 |
mordred | corvus: fair point | 14:24 |
corvus | (you want zuul to do the same thing github is going to do so that it actually tests the right thing) | 14:24 |
mordred | yup | 14:24 |
mordred | and implementing it in the merger also certainly sets us up for a future of zuul-push | 14:24 |
corvus | yep | 14:25 |
mordred | although - to make things more funner, it seems like merge-mode might be a project setting rather than a pipeline quality | 14:25 |
corvus | this may not sound very important, but before we managed to get zuul doing (almost) exactly the same kind of merge gerrit does (merge-resolve), we ran into problems frequently. | 14:25 |
corvus | mordred: it is, in fact, a project setting | 14:26 |
mordred | \o/ | 14:26 |
mordred | then I am correctly caffinated | 14:26 |
corvus | so the github reporter could look at the merge-mode of the project it's reporting and dtrt | 14:26 |
mordred | corvus: btw - I'm making progress on the container image build job - it has progressed from zuul error with no logs, to RETRY_LIMIT - to just having normal failures! | 14:26 |
corvus | trishnag: ^ see followup conversation above | 14:27 |
mordred | corvus: ooh, that's a good idea | 14:27 |
corvus | mordred: the system works! | 14:27 |
corvus | tobiash: i'm thrilled by your bug progress, and will start looking into it as soon as i've had breakfast! | 14:28 |
mordred | corvus: to summarize, we'd want to add a squash-and-merge strategy for the merger, then update the github driver to look at project merge-mode settings | 14:28 |
tobiash | corvus: I have a bad quick fix (see above) and a better but bigger fix that I can upload shortly | 14:28 |
corvus | mordred: ++ | 14:29 |
mordred | corvus: ooh look! http://logs.openstack.org/60/580160/10/check/zuul-build-container-images/bf13769/job-output.txt.gz#_2018-07-09_13_31_35_515785 | 14:30 |
corvus | mordred: a nice add-on might be to have the drivers validate merge-mode settings for projects, since i think we're about to have divergent choices between our drivers. | 14:30 |
mordred | corvus: it's actually building the images and is failing on actual bad content! | 14:30 |
mordred | corvus: yah | 14:30 |
mordred | corvus: honestly - is it time to start considering shifting to zuul-push? divergent click-the-merge-button settings across drivers start to seem ugly | 14:31 |
mordred | corvus: but yes, validation | 14:31 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: WIP: Fix job contamination - alternate approach https://review.openstack.org/581047 | 14:31 |
tobiash | corvus: that is an alternative approach | 14:31 |
tobiash | corvus: so the thing is that it's not poisening the parsed or unparsed layout but some state of the layout is attached to the pipeline | 14:32 |
tobiash | corvus: the first quick fix fixed it by not taking the layout from the pipeline at two places but go deeper into the tenant and get it from there | 14:33 |
*** pabelanger has quit IRC | 14:33 | |
*** pabelanger has joined #zuul | 14:33 | |
tobiash | corvus: then I realized that it somehow sounds weird to have the layout in the pipeline and most of the layout accesses were to get the tenant anyway | 14:33 |
tobiash | corvus: so the second approach is to rip out the layout from the pipeline and replace it directly from the tenant | 14:34 |
tobiash | corvus: as it turned out just a few minutes we had these issues also in some subtile ways without any config project involved | 14:35 |
openstackgerrit | Matthieu Huin proposed openstack-infra/zuul master: web: add tenant-scoped autohold, enqueue (change, ref) https://review.openstack.org/576907 | 14:45 |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul master: Update bindep file with compile profiles https://review.openstack.org/580159 | 14:46 |
openstackgerrit | Monty Taylor proposed openstack-infra/zuul master: Add job to build container images using pbrx https://review.openstack.org/580160 | 14:46 |
*** hashar is now known as hasharAway | 14:48 | |
rcarrillocruz | neat | 14:49 |
rcarrillocruz | my jobs are now using a semaphore | 14:49 |
rcarrillocruz | since they use aws creds to create aws objects, i needed a way to not incur into quota issues | 14:50 |
mordred | rcarrillocruz: ++ | 14:50 |
rcarrillocruz | by defaultl aws allow 5 vpcs per region | 14:50 |
mordred | rcarrillocruz: and this is one of those cases where the thing you're doing is legit job content and not a thing that an aws nodepool driver could otherwise solve | 14:51 |
rcarrillocruz | yeah, cos the jobs create 'things' not nodes... so the driver is not really applicable | 14:52 |
rcarrillocruz | now we get into nodepool getting into thingpool | 14:52 |
rcarrillocruz | Shrews: wink wink | 14:52 |
tobiash | corvus: have to run now, I'll probably come back later | 14:57 |
Shrews | FYI, out doing some volunteer things this morning. Will be around in the afternoon. | 15:00 |
corvus | rcarrillocruz: we also had the idea of a 'cleanup' job, which always gets run at the end of a buildset even if intermediate jobs get aborted. that's not implemented yet, but that may be helpful for your use case | 15:02 |
rcarrillocruz | wellll, that would be ideal... the reason why i asked about 'only run this periodic job if and only if no jobs in check/gate are running' was exactly for that | 15:04 |
rcarrillocruz | the tests on the repo clean up themselves | 15:04 |
rcarrillocruz | but there are chances they can be aborted | 15:04 |
rcarrillocruz | thus cruft could be left behind | 15:04 |
rcarrillocruz | now, i could create periodic 'cleanup' jobs that take semaphores tied to check/gate (that's what i plan to experiment with anyways), but if there was a cleanup concept that would be more than ideal for my use case | 15:04 |
pabelanger | mordred: corvus: to continue on with https://review.openstack.org/578230/ (zuul_return with zuul.child_jobs) we also discussed allowing a parent job to live longer to service child jobs, say for hosting artifacts (over pushing to external server), any thoughts or notes about how that could work? | 15:05 |
rcarrillocruz | corvus: does that sound as a reasonable 'cleanup' workaround till what you depicted is implemented? cannot think of another way of doing it tbh | 15:08 |
rcarrillocruz | however, i would need to have $periodic_cleanup_jobs per semaphores, since it seems only one semaphore can be locked by any given job, which is not a huge deal for us anyway | 15:09 |
mordred | pabelanger: there are thoughts on that concept written up in the container spec: https://review.openstack.org/#/c/560136 | 15:12 |
pabelanger | mordred: thanks | 15:13 |
mordred | rcarrillocruz: yah - I thnk periodic jobs on a semaphore is a good workaround for now | 15:13 |
rcarrillocruz | \o/ | 15:40 |
*** jiapei has quit IRC | 16:05 | |
openstackgerrit | Ronelle Landy proposed openstack-infra/zuul-jobs master: Update multi-node-bridge to install OVS from delorean deps repo https://review.openstack.org/581081 | 16:20 |
*** acozine1 has joined #zuul | 16:24 | |
tobiash | corvus: regarding that but, it is unrelated to github and multi tenant so it even might have happened (maybe even unnoticed) also in openstack land | 16:34 |
tobiash | s/but/bug | 16:34 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: Fix job contamination by unmerged change https://review.openstack.org/580949 | 17:27 |
tobiash | corvus: squashed and tweaked the commit message, I hope it's understandable | 17:28 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: Fix job contamination by unmerged change https://review.openstack.org/580949 | 17:30 |
*** fbo is now known as fbo|off | 17:52 | |
SpamapS | hm | 18:25 |
SpamapS | tobiash: do you have any way to kick something out of gate if it is alread in the gate? Like, a label or something? | 18:25 |
SpamapS | I have a thing called do-not-merge that will prevent it from enqueing via requirements. | 18:25 |
SpamapS | but I want something that actually dequeues it. | 18:25 |
tobiash | SpamapS: that's currently not possible (other than closing the pr) | 18:25 |
corvus | tobiash: i'm still a little unclear as to the mechanics of the bug. why was item.queue.pipeline.layout != tenant.layout? if the config-project change merged, they should be the same thing | 18:26 |
tobiash | corvus: the config-project change didn't merge | 18:26 |
tobiash | corvus: however I didn't fully understand why it's not the same but still I think taking the layout from a fixed place is cleaner (and fixes the bug) | 18:27 |
corvus | tobiash: so did it re-use the active pipeline object when creating the new layout? | 18:27 |
tobiash | corvus: that might be the case | 18:28 |
SpamapS | So, with gerrit, we can remove the +A and it doesn't merge. Is that because gerrit will reject the "merge" API call without the +A? | 18:29 |
corvus | SpamapS: yes | 18:29 |
SpamapS | I wonder if there's a pipeline hack we can do where we make a "do-not-merge" pipeline that will set a status to failed if we drop a 'do-not-merge' label or remove the 'approved' label. | 18:29 |
corvus | SpamapS: (in openstack-infra, folks push up a new patchset to cause a change to be dequeued from gate) | 18:30 |
SpamapS | Ah push new change would work | 18:30 |
tobiash | SpamapS: you also could revoke the review to prevent zuul to merge | 18:30 |
corvus | tobiash: do you have a change with just your test addition? | 18:35 |
corvus | i really need to understand the bug before we merge any fix | 18:35 |
tobiash | corvus: it's ps1 of that change | 18:35 |
corvus | tobiash: of 949? | 18:35 |
tobiash | yes | 18:35 |
corvus | thanks | 18:35 |
*** electrofelix has quit IRC | 18:36 | |
corvus | tobiash: http://git.zuul-ci.org/cgit/zuul/tree/zuul/model.py#n2991 | 18:44 |
corvus | hrm, that's part of it, but it's not the whole story | 18:45 |
SpamapS | tobiash: we don't really use reviews | 18:45 |
SpamapS | because we do need to self approve often | 18:45 |
tobiash | corvus: yes, that's the only place where the layout gets set (at least the only place I found) | 18:46 |
corvus | tobiash: oh, i think it's because it's a cached config object. that's why this requires pipelines be in a different repo than the project-config being changed. when we create the throwaway layout to validate the config for change A, we call layout.addPipeline on the pipeline objects. if your pipelines and global config are in the same repo, the cache would be invalidated and you would get brand new | 18:52 |
corvus | pipeline objects. but in this case, we're using the cached pipeline objects (which are the *live* pipelines) and we add them to the layout. because addPipeline has the side effect of setting the pipeline's layout, we've altered the pointer on the live pipeline object to point to a throwaway layout. | 18:52 |
corvus | tobiash: this kind of error would be caught for most config objects because they are made immutable when cached, but we don't do that for pipelines because we need the real pipeline objects to be added to dynamic layouts for untrusted changes. | 18:52 |
corvus | (semaphores have similar characteristics) | 18:53 |
tobiash | corvus: so if we need the live pipelines and cannot make them immutable I think it makes sense that they should not contain any layout directly | 18:54 |
corvus | tobiash: i agree that since the tenant object is expected to be constant among all layouts within the tenant, then referencing that from the pipeline, rather than the layout, is an appropriate fix. | 18:56 |
tobiash | corvus: oh yes, I don't know how I've overlooked that, the second call to addPipeline alters the existing one | 18:56 |
tobiash | I just thought it's a new one so I overlooked that detail | 18:56 |
tobiash | that would have saved me hours of debugging... | 18:57 |
openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: Switch to centos-release-openstack for ovspackage https://review.openstack.org/580518 | 18:57 |
corvus | tobiash: my print-based debug log: http://paste.openstack.org/show/725364/ | 18:58 |
corvus | tobiash: i'm inclined to think that your change is the way we should fix it. i want to think about it a little bit more over lunch in case any other subtleties jump out at me. and we might want to move the pipeline.tenant assignment into the configloader, so that the modification side effect isn't as surprising | 18:59 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: Fix job contamination by unmerged change https://review.openstack.org/580949 | 19:12 |
tobiash | corvus: like this ^ ? | 19:13 |
openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: Switch to centos-release-openstack-queens for ovs https://review.openstack.org/580518 | 19:14 |
*** adam_g has quit IRC | 19:35 | |
openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Require tenant in Pipeline constructor https://review.openstack.org/581129 | 20:22 |
corvus | tobiash: ^ that may be a little bit stronger of an assertion | 20:22 |
tobiash | corvus: lg | 20:25 |
tobiash | T | 20:25 |
tobiash | l | 20:25 |
tobiash | lgtm | 20:25 |
* tobiash apologizes for fat finger spam | 20:26 | |
corvus | tobiash: would you mind updating the commit message with the suggestions i left on there? this is a complicated bug and if we have to revisit this at some point, i'd like the full story to make it easier. other than that, change lgtm. | 20:29 |
tobiash | Sure, looking in a bit | 20:31 |
openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: Fix job contamination by unmerged change https://review.openstack.org/580949 | 20:40 |
tobiash | corvus: ^ | 20:40 |
openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Require tenant in Pipeline constructor https://review.openstack.org/581129 | 20:58 |
corvus | rebase ^ | 20:58 |
corvus | clarkb: can you look at https://review.openstack.org/580948 and 2 children when you have a moment? | 20:59 |
corvus | to clarify: 2 children of the change. not just any 2 children. | 20:59 |
*** adam_g has joined #zuul | 21:46 | |
*** acozine1 has quit IRC | 22:03 | |
*** threestrands_ has joined #zuul | 22:20 | |
*** threestrands_ has quit IRC | 22:20 | |
*** threestrands_ has joined #zuul | 22:20 | |
*** jappleii__ has joined #zuul | 22:23 | |
*** jappleii__ has quit IRC | 22:24 | |
*** jappleii__ has joined #zuul | 22:25 | |
*** threestrands_ has quit IRC | 22:26 | |
*** yolanda_ has joined #zuul | 23:06 | |
*** hasharAway has quit IRC | 23:08 | |
*** yolanda has quit IRC | 23:09 | |
openstackgerrit | James E. Blair proposed openstack-infra/zuul master: WIP: Report to gerrit over HTTP https://review.openstack.org/577027 | 23:20 |
*** ianychoi_ has joined #zuul | 23:21 | |
*** goern_ has joined #zuul | 23:22 | |
*** goern has quit IRC | 23:22 | |
*** jimi|ansible has quit IRC | 23:23 | |
*** jimi|ansible has joined #zuul | 23:24 | |
*** ianychoi has quit IRC | 23:25 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!