Wednesday, 2017-10-25

SpamapSanyway... another thing biting me right now is the inability to require an OR condition, like 'Either it has an Approved review from a person with write access, _or_ it has the 'self-approved' label.00:00
jeblairSpamapS: triggers are or'd00:14
jeblairSpamapS: so you may be able to add another trigger and do what you want.00:17
jeblairSpamapS: and yeah, they have to share a queue.  however, we might be able to have the zuul trigger emit an event when a dependent change merges which could be used as a trigger.00:17
*** jamielennox has quit IRC01:20
*** persia has quit IRC01:21
*** persia has joined #zuul01:22
*** jamielennox has joined #zuul01:23
*** jhesketh_ is now known as jhesketh02:22
tobiashSpamapS: you might want to add the zuul event parent-change-enqueued to your gate pipeline05:08
openstackgerritTobias Henkel proposed openstack-infra/zuul feature/zuulv3: Create job-output.txt together with JobDir  https://review.openstack.org/51461705:13
*** tushar has joined #zuul05:36
*** xinliang has joined #zuul05:36
openstackgerritMerged openstack-infra/zuul feature/zuulv3: Document executor/merger stats  https://review.openstack.org/51434306:02
*** umbarkar has joined #zuul06:04
*** tushar has quit IRC06:04
*** yolanda_ has joined #zuul06:12
*** bhavik1 has joined #zuul06:13
kklimondais there going to be a race between nodepool hold [node currentty in-use/locked] and zuulv3 freeing the node  for deletion?06:39
*** yolanda_ has quit IRC06:40
*** umbarkar has quit IRC06:40
*** yolanda_ has joined #zuul07:42
*** yolanda_ is now known as yolanda07:43
*** bhavik1 has quit IRC08:19
*** electrofelix has joined #zuul08:34
*** rcarrill1 is now known as rcarrillocruz09:48
*** jkilpatr has quit IRC11:11
*** yolanda has quit IRC11:19
kklimondaso, some of my logs (output of the main playbook in job-output.txt.gz) are truncated, with traceback like that in executor-debug.log: http://paste.openstack.org/show/624588/11:28
kklimondalooking at the example log (https://logs.opencontrail.org/92/36592/2/check/contrail-vnc-build-unittest-ubuntu-xenial/6e973ee/job-output.txt.gz) I see that there are some characters that are probably unicode - given that UnicodeDecodeError is raised on the last byte of the buffer, I assume we are trying to decode unicode character that's split between reads11:30
*** jkilpatr has joined #zuul11:55
tobiashkklimonda: seems that you hit a bug in the log streaming code12:10
kklimondayup - either side should handle that, not sure which though12:10
kklimondaeither sender or receiver I mean12:11
tobiashkklimonda: zuul decodes the log with utf-812:17
tobiashkklimonda: you probably have some job which outputs binary data to stdout (which I think is a bad thing but should not crash zuul)12:17
kklimondatobiash: no, I have a job that's outputting utf-8 to stdout12:17
tobiashkklimonda: well the log says it's not utf-812:18
tobiashkklimonda: ah, maybe the buffer splits a multi byte character12:19
kklimondatobiash: what seems to be happening is that zuul is trying to decode an incomplete unicode.. code point (I think?) because it spans buffer boundary12:19
kklimondaI wonder if that's related to zuul-web randomly dropping log streaming12:23
kklimondahmm12:23
openstackgerritDirk Mueller proposed openstack-infra/zuul-jobs master: Remove pep8/pyflakes  https://review.openstack.org/51504112:27
*** yolanda has joined #zuul12:27
openstackgerritTobias Henkel proposed openstack-infra/zuul feature/zuulv3: Do late decoding of log stream buffer  https://review.openstack.org/51504312:28
tobiashkklimonda: this could be a fix for this ^12:29
kklimondatobiash: let me try it really quick, I seem to be hitting that issue pretty reliably12:29
tobiashk12:29
tobiashkklimonda: I'm not sure if split works with binary strings, if yes this should work hopefully12:30
kklimondatobiash: ha, I've been thinking about splitting on \n - I was a bit worried about what could happen if someone started feeding it logs with no \n characters12:31
tobiashkklimonda: then it will not split the line and have it all on one line in the last log line at the end12:31
openstackgerritAndrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role  https://review.openstack.org/50923312:52
openstackgerritAndrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role  https://review.openstack.org/50923313:09
SpamapStobiash: Oh! Thanks, I'll try parent-change-enqueued13:11
tobiashSpamapS: I had an issue that approving changes with depends-on in the wrong order left one change not entering the gate13:15
tobiashSpamapS: then I discovered parent-change-enqueued :)13:15
SpamapStobiash: Indeed I think it's the right thing. Hopefully it crosses shared queue bounds.13:16
tobiashSpamapS: I think with not shared queues the trigger will fire but the queue will reject it13:17
openstackgerritAndreas Jaeger proposed openstack-infra/zuul-jobs master: Silence ansible-lint  https://review.openstack.org/51452613:17
tobiashSpamapS: in this case you probably need to listen on a merge event13:17
tobiashSpamapS: but I might be wrong with the queues13:18
SpamapSWell it's possible I want them shared. I'm not sure.13:18
SpamapSbut I feel like I don't.13:19
SpamapSProbably need to read the docs again.13:19
tobiasha use case for not having them shared would be e.g. nova and zuul-jobs13:19
SpamapSI'm more concerned with how to get things into the gate from github.... using the reviews API won't allow self-merge.. using labels seems to have race problems.13:20
tobiashone would still want depends on between them13:20
SpamapStobiash: exactly13:20
SpamapSin my case I'm adding jobs and then adding usage of the jobs in several repos.13:20
tobiashSpamapS: in this case you probably don't want shared queues13:21
SpamapSright, but I do want to put the children in their gate when the parent merges13:21
tobiashSpamapS: do we have a parent-change-merged event?13:22
tobiashSpamapS: I think such a thing would be the thing we want for this use case13:22
SpamapSagreed13:22
SpamapSbecause the children won't enqueue on parent-change-enqueued13:23
SpamapSsince their queue will not have the parent ahead of them13:24
tobiashSpamapS: looks like this is not existing yet: https://docs.openstack.org/infra/zuul/feature/zuulv3/admin/drivers/zuul.html13:24
tobiashSpamapS: but I think it's useful13:24
SpamapStobiash: indeed I'm looking to see if we can generate it13:24
SpamapSas long as we don't drop the needed_by_changes event I think we can do it relatively easily13:28
SpamapSs/event/attribute/13:28
tobiashSpamapS: this attribute should be also used by the parent-change-enqueued so I doubt we drop this attribute13:30
SpamapStobiash: yeah I think it will be there.13:32
SpamapSUnfortunately I don't know if I'll have time to work on that for a while.13:32
SpamapSUntil then we just have to toggle our approvals.13:32
*** jkilpatr has quit IRC13:56
*** yolanda has quit IRC14:28
openstackgerritAndrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role  https://review.openstack.org/50923315:02
openstackgerritAndrea Frittoli proposed openstack-infra/zuul-jobs master: Add a generic stage-artifacts role  https://review.openstack.org/50923315:30
jlkSpamapS: we did put in a feature request for allowing admin self-review.16:00
*** hashar has joined #zuul16:04
pabelangerjeblair: mordred: did you want to look at 514526 and comment? Its about using skip_ansible_lint tags to make ansible-lint happy. Want to make sure everybody is okay with it, since they could be sprinkled around playbooks.16:04
SpamapSjlk: I recall that we did, but don't remember if it was in the "totally no way" answer or the "mmk" answer category.16:04
SpamapSjlk: they also gave us the "totally no way" answer on defaulting to dismissing reviews on push.. then a month later added the checkbox. ;)16:04
jlkI had a conversation with review folks at github universe. They seemed pretty into allowing that16:05
jlksince they already allow admins to override required review to merge, and other such things16:05
jlkso I have some hope16:05
SpamapSjlk: right, seems like having everything in the same API vs. just brushing past it would be idea.16:05
SpamapSideal16:05
SpamapSjlk: until then... I'm not sure what to do.16:05
jlkyeah :(16:05
jlklabels :/16:05
SpamapSwhich has the delay issue for me16:06
jlkright.16:06
SpamapSthough keeping zuul busier has helped with that ;)16:06
jlktotally open to introducing a delay there16:06
SpamapSI think it is literally a matter of 10ms16:06
jlkI think we do some of that in gerrit too16:06
SpamapSmy guess is the webhook happens async in GH from the write to the db16:06
jeblairyeah, gerrit is a continuous 5s behind16:06
SpamapSorly?16:07
SpamapSAnother problem with the label approach is that only owners can apply labels.16:07
SpamapSso I have to make somebody a repo owner to let them approve patches. That kind of makes sense.. but it's a bit of a mismatch in power.16:07
jeblairyep.  race conditions on the server mean that a query immediately after an event may return old data.  so we put it into a queue and make sure 5s have elapsed before we pull it off.  the effect is like zuul being 5 seconds in the past.  which is weird, because zuul usually lives in the future.16:08
SpamapSI noticed that Kubernetes uses a system where people comment '/lgtm' or '/approved' and a bot applies the labels after checking their membership.16:08
*** bhavik1 has joined #zuul16:08
SpamapSjeblair: Zuul: a time traveler for all your testing needs.16:08
SpamapSWait no, Gozer is the traveler.16:09
jeblairthe number of people privately employed to work around the fact that github is not open source is amazing.16:09
SpamapSjeblair: :(16:09
SpamapSWhich capital to spend: Evangelize Zuul, Evangelize Gerrit, Evangelize Rust16:09
* SpamapS takes a shot16:09
jeblairpabelanger: looking16:09
jlkSpamapS: I think you can grant somebody write access ( not owner ) so that they can apply labels. Also you can restrict WHO can actually commit to a branch down to just the zuul bot user (until you move to app based)16:10
jeblairpabelanger: did you want to +3 that?16:11
pabelangerjeblair: done, thank you16:13
* SpamapS looks at gerrit queue thing16:13
tobiashjlk: I had a github consultant here today and also asked the self review question16:23
tobiashanswer was nope, not possible16:24
jlksweet!16:24
jlkwaaat?16:24
jlkthat wasn't the answer we got :(16:24
openstackgerritMerged openstack-infra/zuul feature/zuulv3: Create job-output.txt together with JobDir  https://review.openstack.org/51461716:24
tobiashhe said, doesn't make sense as the review override is the merge button for project admins...16:25
jeblairthe customer is always wrong16:25
tobiashjlk: let's hope that you talked to devs more in that topic...16:26
jlkthat was the first response, but when we explained how review triggers CI, and how you can easily add extra contributors without changing settings then it made sense to them16:26
jlktobiash: the people I was talking to were devs16:26
tobiashAn I asked about github apps in enterprise16:26
jlkwhat was the word on that?16:27
tobiashit could be added as an early access feature to 2.1216:27
tobiashearly access means that you have to enable this feature together with the support16:29
tobiash(if I understood this correctly)16:29
tobiashjlk: do you use labels for modeling the workflow flag?16:32
SpamapSThat resposne doesn't surprise me from a consultant.16:33
SpamapSI've been a consultant in a software product company before. You get told no by every product manager you talk to, until you find it in the release notes because some customer went over your head and called the CTO.16:34
SpamapSSo your default answer to customers is usually "nope can't change {anything}" to keep expectations low.16:34
jlktobiash: So, if you can't do self review, we've made labels as a thing you could use to provide a workflow vote.16:36
jlktobiash: otherwise, the recommended method is to make use of pull request reviews.16:36
tobiashjlk: does zuul already respect the branch protection settings (like for gerrit label)16:38
tobiashSuch that I don't have to require review on the pipeline config16:38
openstackgerritMerged openstack-infra/zuul-jobs master: Silence ansible-lint  https://review.openstack.org/51452616:39
tobiashFor gerrit it respects any additional mandatory label before entering the gate16:40
jlktobiash: not exactly no.16:41
jlktobiash: we have code to discover what branch protections are in place, and maybe we don't attempt to merge if it's not there? I forget how far down that path we got16:42
jlkWe wanted to get this in front of a bunch more github users before getting too complicated in the capabilities and whatnot16:42
tobiashMaybe I'll have a look into this topic in the next few weeks16:42
jeblairjlk, tobiash: GitHubConnection.canMerge has a big TODO and returns True16:43
jlkhah yes.16:43
tobiash(when I have a real github available for my v3 setup)16:43
jeblairso i'm guessing no.  but there are some thoughts in that comment about how to proceed :)16:43
jlkI know jamielennox landed some code to inspect branch protection settings16:44
jeblairactually, the way the comment is written, it almost looks like there maybe was code there before but got removed16:44
jlkoh maybe it was an open PR16:44
jlker / change request16:44
*** hashar is now known as hasharAway16:45
jlkhttps://review.openstack.org/#/c/471175/ is what I was thinking of16:46
tobiashbtw, I'll be on vacation next week and currently a bit under pressure to get a v3 setup with openshift and github productive16:46
jlktobiash: okay, reach out if you get stuck at all. Happy to help!16:47
tobiashThat's why I'm a little less active here the last few weeks...16:47
jlkso this change was mostly about other contexts that may be required (like if they use both travis and us, or whatever). We thought we could go deeper and check for other protection level things that could block us16:48
tobiashjlk: thanks16:48
jlkgraphQL would make this easier, we could dig all that out in one shot rather than a pile of GET calls :/16:48
jlkreally really really wish they'd turn on graphQL for apps16:48
tobiashjlk: I also asked about graphql and the answer was that's also usable for apps16:49
tobiashSo either this works now or I got a wrong answer16:50
jlkhrm, that's not advertised yet. At github Universe 2 weeks ago they said it's coming soon, but no commitment.16:50
jlkhttps://developer.github.com/apps/ still says only REST v316:50
jlkmaybe it's in early access16:52
tobiashPossibly16:53
SpamapSit makes the REST calls and/or it gets the 42916:54
jlkClint.16:54
jeblairJesse?16:56
* jeblair is not sure if he's playing right, but is trying16:56
jlkI was saying his name in disdain for his humor attempt.16:57
openstackgerritMerged openstack-infra/zuul feature/zuulv3: Add log streaming logging and exception handling  https://review.openstack.org/51381117:16
SpamapSLEEEERROYY JENKINS17:16
Shrewsjeblair: reading your "branch issues" email, i've re-read "Parents should have variants applied" section umpteen times. And though I get why that's a problem, I'm not sure I grok the technical reason given: "But since the inheritance has already been applied, any variants which a user may expect to apply to a parent job will not be applied."17:18
Shrewsjeblair: if variants are applied dynamically, what does inheritence already being applied have to do with the problem?17:18
Shrewsi'm missing something (obviously)17:19
*** dmsimard is now known as dmsimard|afk17:23
SpamapSShrews: I feel like I need a visual diagram for that one too17:25
SpamapSIt's all there17:25
SpamapSBut I think it has one more dimension than I can draw in my head.17:25
jeblairShrews: ah, it's because inheritance is determined when the config is loaded.  so by the time zuul runs a job, it's too late to say "oh, this change is on stable/newton, and this job's parent, devstack, has a stable/newton branch variant, so it should apply too".17:27
Shrewsjeblair: "too late" means "past the point of knowing what the inheritance is"?17:28
jeblairShrews: so basically, if your job (say "tempest") has branch variants, you're fine.  but if it's parent (say "devstack") does, you won't get them.  your tempest job will have already picked a single devstack job to inherit from, when zuul was last reconfigured.  it'll be the first one.17:28
jeblairShrews: yep.17:28
jeblairShrews: well, past the point of being able to do anything about it at least.17:28
*** bhavik1 has quit IRC17:29
Shrewsjeblair: ah, I get it now.17:29
jeblairShrews: here's the code that has a job inherit from a parent job: http://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/configloader.py?h=feature/zuulv3#n50517:30
SpamapS"FAILURE in 52s" ... fail fast!17:50
openstackgerritMonty Taylor proposed openstack-infra/zuul-jobs master: Have tox-siblings edit requirements files instead  https://review.openstack.org/51405817:56
*** electrofelix has quit IRC17:57
openstackgerritClark Boylan proposed openstack-infra/zuul feature/zuulv3: Support cgit urls on Gerrit connections  https://review.openstack.org/51516818:36
openstackgerritKrzysztof Klimonda proposed openstack-infra/zuul feature/zuulv3: [WIP] Allow autohold on specific changes  https://review.openstack.org/51516918:36
clarkb515168 fixes an item on the zuulv3-issues etherpad, though it isn't really a zuulv3 issue so I've nto set the topic to zuulv3-fixes18:37
*** yolanda has joined #zuul18:40
kklimondare 515169: I've seen requests from our developers to hold nodes for specific changes so that the failures can be investigated - I don't think that's quite possible right now, so I've started tinkering to see how that could be made to work.18:40
tobiashclarkb: left a comment18:43
clarkbtobiash: thanks18:43
clarkbkklimonda: left some comments on that change18:47
openstackgerritClark Boylan proposed openstack-infra/zuul feature/zuulv3: Support cgit urls on Gerrit connections  https://review.openstack.org/51516818:52
clarkbtobiash: ^ fixed18:52
jeblairclarkb: can you clarify one of your comments on 515169?  (i left question inline)18:57
clarkbjeblair: responded19:00
jeblairclarkb: though change without uuid lets you set the autohold before the job starts19:01
clarkbjeblair: oh thats a good point hrm19:01
jeblair(also, uuid is risky in that it could be subject to cancelation.  i think it's okay to add because if that's what you want, sure.  we probably just shouldn't depend on it for that reason)19:02
clarkbjeblair: re cgiturl comemnt, after lunch I'll make a stab at making it a template instead19:02
jeblairclarkb: ok cool.  i think i'd prefer that, but didn't want to "-1 refactor" :)19:02
jeblairclarkb: lemme know if i can facilitate that :)19:02
jeblair(the template stuff is getting more generic, so i don't think it'd be too hard; model objects are growing getSafeAttributes() methods)19:03
clarkbya should be straightforward just provide a template string that has access to base_url and project.name and sha. Then interpolate those as necessary (you might choose to not use base_url as in the cgit on different host case)19:04
clarkbfor autoholding I guess you'd have to allow change natively19:05
clarkbfor me at least I tend to not want all the jobs from a change and want a very specific nodeset (which build-uuid would support)19:05
clarkbok lunch now19:05
kklimondaclarkb: won't passing --job to autohold limit request to only nodeset from the job?19:12
kklimondaor are you talking of something else?19:12
clarkbkklimonda: ya that will too, but that doesnt help the post/tag/release/periodoc situation so was trying to think of it from that perspective19:15
kklimondaright19:16
jeblairyeah, i think we should also add 'ref' for the post-merge situation19:18
kklimondare --ref, should that be a separate command to match enqueue-ref, or should I just add --ref [--oldrev] [--newrev] to autohold itself?19:33
kklimondaalso, initially my --change was actually --changeset, to allow for autoholding on the specific patchset, but I couldn't think of a reason to do that.19:36
jeblairkklimonda: yeah, i like --change.  i'd add --ref to autohold itself.  basically it becomes a query with a bunch of 'and' conditions.  adding oldrev/newrev could be useful, but if you do, i'd definitely make them optional (just matching on the ref without matching the values is useful -- much like matching the change without the patchset)20:12
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: Move test_model.test_job_inheritance_configloader  https://review.openstack.org/50949520:14
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: Remove test_job_inheritance  https://review.openstack.org/50950920:14
kklimondajeblair: makes sense, thanks - I'll take a stab at it tomorrow. G'night :)20:18
jeblairkklimonda: thanks, good night!20:20
*** tristanC has quit IRC20:43
*** tristanC has joined #zuul20:52
*** yolanda has quit IRC20:56
*** hasharAway has quit IRC21:07
*** dmsimard|afk is now known as dmsimard21:08
openstackgerritJeremy Stanley proposed openstack-infra/zuul feature/zuulv3: Default change and patchset to NULL in SQLReporter  https://review.openstack.org/51442321:58
*** xinliang has quit IRC22:20
* SpamapS up to 5 zuul jobs and 3 nodesets now :-D22:25
SpamapSand 5 repos22:26
jamielennoxjlk, tobiash: that github protection code was working, i just hadn't written the tests for it and it was at a time where there was a fair bit of refactoring22:34
SpamapSultimately I just want a consistent experience22:36
SpamapSthe label provides the same experience whether you're self-approving or approving.22:36
jamielennoxalso on that you should be following https://platform.github.community/t/repositories-which-have-protected-branches-with-push-restrictions-have-no-ability-to-grant-push-rights-to-integrations/137622:38
jamielennoxit's starting to get movement - although i wish it had "generated healthy internal debate" a few months ago22:39
SpamapShah yeah22:42
SpamapShm the bot isn't talking in here22:47
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: Add implied branch matchers on 'master'  https://review.openstack.org/51445922:51
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: experiment with late-binding inheritance  https://review.openstack.org/51135222:51
openstackgerritJeremy Stanley proposed openstack-infra/zuul feature/zuulv3: Default change and patchset to NULL in SQLReporter  https://review.openstack.org/51442322:58
clarkbjeblair: I got nerd sniped by something else. About to start working on the change url template thing now23:11
clarkbjeblair: one problem with using a template like that is that we actually generate two different urls based on whether or not the sha1 is there23:40
clarkbI guess I can see if that code path is ever actually hit23:41
clarkbit uses event.newrev which I think will always exist for a ref updated event? it may be 0*40 though23:42
jeblairclarkb: what's the path without the sha1?  a change?  or some specific kind of post-merge event?23:43
jeblairclarkb: cause yeah, if it's change, then i think we use the connection-supplied url for that, since that's intrinsic.23:44
jeblair(ie, gerrit itself)23:44
clarkbya if thee is a change number we do gerrit itself else we ask for the git url then in the function supplying that we check if the sha1 is not false23:44
jeblairoh hrm.  yeah i wonder what that is23:45
jeblairmaybe it was just paranoia?23:45
clarkbwe always pass event.newrev as the sha value23:45
clarkbya I am thinking it may have been unnecessary belts and suspenders23:45
clarkbI'll work on a patchet assuming its not necessary so we have something more concrete to think about23:45
jeblair++ i can't think of anything else right now :)23:45
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: experiment with late-binding inheritance  https://review.openstack.org/51135223:46
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: Add implied branch matchers on 'master'  https://review.openstack.org/51445923:58
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: experiment with late-binding inheritance  https://review.openstack.org/51135223:58

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!