Monday, 2017-09-18

openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: Serve keys from canonical project name  https://review.openstack.org/50480700:03
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: Add support for result data in child jobs  https://review.openstack.org/50480800:03
openstackgerritJames E. Blair proposed openstack-infra/zuul feature/zuulv3: WIP: add abstract job attribute  https://review.openstack.org/50480900:03
dmsimardpabelanger, tristanC: bah https://github.com/Logan2211/ci-stack00:03
dmsimardso many people working on their own zuul/nodepool deployment thingies00:04
dmsimardFYI multinode things are ready to review: https://review.openstack.org/#/q/topic:zuulv3-multinode00:22
dmsimardThe implementation is in zuul-jobs and it is integration tested in openstack-zuul-jobs00:22
dmsimardclarkb ^00:22
SpamapSnow to figure out what's going on with my status urls02:59
*** jesusaur has quit IRC03:08
*** jesusaur has joined #zuul03:27
openstackgerritzhangyangyang proposed openstack-infra/nodepool master: Remove py26 support  https://review.openstack.org/50483603:37
* SpamapS is very confused how to get web-based log streaming to work04:14
tobiashSpamapS: how far have you got? Maybe I can give you a hint.05:20
tobiashSpamapS: zuul-web must be running and needs to be able to connect to the executors on the finger port05:22
tobiashSpamapS: if you use apache as a proxy in front of zuul-web you need to handle the websocket path differently as apache doesn't seem to be able to handle websocket streaming and normal web content with one proxy05:23
tobiashSpamapS: I have something like this in the apache settings for zuul-web: http://paste.openstack.org/show/621303/05:24
SpamapStobiash: I can manually paste the uuid into the url to the static/stream.html just now :)05:24
SpamapS(and I"m using nginx, so I just learned how to upgrade the ws:// 's05:24
tobiashSpamapS: so the streaming part itself works?05:24
SpamapStobiash: yes, streaming works05:25
SpamapSbut a) I get no statuses set at all on my github PR05:25
tobiashSpamapS: I think ngingx is better in this regard05:25
SpamapSb) comments are finger://05:25
SpamapShttps://www.nginx.com/blog/websocket-nginx/05:25
SpamapS^^ that one got me working05:25
SpamapSpretty simple, seems like it would be easy in Apache too05:25
SpamapSbut yeah maybe nginx handles better internally05:26
SpamapSanyway, right now i think I just have to figure out how to get zuul to comment with the right URL05:26
tobiashSpamapS: difference about nginx and apache is that this nginx settings work for mixed websocket and non-websocket stuff and for apache you need to have seperate settings05:26
SpamapSand I can do my demo tomorrow without feeling embarrassed that i have to copy/paste the UUID out of the logs into my streaming URL05:27
tobiashSpamapS: for the correct comments your base job needs to set a special var05:27
tobiashSpamapS: look here: https://github.com/openstack-infra/project-config/blob/master/playbooks/base/post-logs.yaml05:28
tobiashSpamapS: https://github.com/openstack-infra/zuul-jobs/blob/master/roles/upload-logs/tasks/main.yaml#L3805:28
SpamapSYeah that's a good point05:29
tobiashSpamapS: this is the task that tells zuul about the log url which will replace the finger url when reporting to github05:29
SpamapSI think the bonnyci jobs I cribbed from need some secrets setup which I didn't setup05:29
SpamapSsince they copy logs from executor -> logs host05:29
SpamapSalso my status.json doesn't get any urls in it which seems odd05:30
SpamapSI'd at least expect finger:// urls while the job runs05:30
SpamapSguessing I'm missing something in my pipeline config that will set the pattern05:31
tobiashthat looks strange05:33
SpamapSyea it's kinda driving me crazy :-P05:36
SpamapSbecause tracing through this there are a ton of 'status urls'05:37
*** yolanda has joined #zuul06:11
*** bhavik1 has joined #zuul06:19
*** hashar has joined #zuul06:46
*** bhavik1 has quit IRC07:17
qwchi! quick question, we're using zuul 2.5.1 and i am trying to use templates with multiple parameters, but it doesn't allow it, did i something wrong with the syntax ?07:57
qwc- name: mytemplate07:58
qwc  parameter1: blah07:58
qwc  parameter2: blahblah07:58
qwc(when i want to use it in a project of course. ;)07:58
qwcmordred: maybe ?08:01
*** electrofelix has joined #zuul08:15
qwcwell nevermind, found a workaround for now. but still would like to know how it is possible to use multiple parameters in a template definition of a project, because i get exceptions there everytime when i have more than one... just highlight me, quassel will save it and notify me. :)08:55
*** _ari_ has quit IRC09:07
*** weshay has quit IRC09:07
*** _ari_ has joined #zuul09:07
*** weshay has joined #zuul09:10
*** pabelanger has quit IRC09:48
*** _ari_ has quit IRC09:49
*** _ari_ has joined #zuul09:50
*** pabelanger has joined #zuul09:51
*** rcarrillocruz has quit IRC10:04
*** rcarrillocruz has joined #zuul10:10
*** jkilpatr has quit IRC10:49
*** jkilpatr has joined #zuul11:09
*** dkranz has joined #zuul12:04
Shrewsjeblair: was the nodepool problem you were hunting on Friday due to the config issue?12:40
openstackgerritDavid Moreau Simard proposed openstack-infra/zuul-jobs master: Only uninstall rdo-release if it was installed in the first place  https://review.openstack.org/50493613:02
fbo_hi, I have a zuulv3 deployment and I'm trying to define a job in a repo declared in "untrusted-projects" in main.yaml but either for "shell" or "command" ansible module I'm getting "ERROR: Executing local code is prohibited". Is it a normal behavior ? or should I define my project under config-projects ?13:53
dmsimardfbo_: untrusted jobs can't execute code on the executor13:57
dmsimardi.e, targetting or delegating to localhost13:57
fbo_dmsimard: well the exec of the "shell" or "command" command will happen on the slave not on the executor in my case. The task is not maked delegated. But well ok I'm trying to declare the repo in config-projects then.14:02
dmsimardfbo_: okay, if you can't figure it out, I would need to see an example job/playbook that's not working so I can understand better14:03
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: WIP: Honor cloud quotas before launching nodes  https://review.openstack.org/50383814:06
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Make max-servers optional  https://review.openstack.org/50428214:06
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Support cores limit per pool  https://review.openstack.org/50428314:06
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Don't fail on quota exceeded  https://review.openstack.org/50305114:06
openstackgerritTobias Henkel proposed openstack-infra/nodepool feature/zuulv3: Support ram limit per pool  https://review.openstack.org/50428414:06
*** hashar is now known as hasharAway15:28
SpamapS/var/log/zuul/zuul-scheduler-debug.log:2017-09-17 22:35:08,898 DEBUG zuul.GithubReporter: Reporting change <Change 0x7f8d940c1da0 2,e463c389bf3bff6473a6aa2ddda05986d714d0ed>, params {'comment': True, 'status': 'failure', 'status-url': 'http://zuul1.cloud.phx3.gdg/{tenant.name}/{pipeline.name}/{change.project.canonical_name}/{change.number}/{buildset.uuid}/'}, status:15:41
SpamapSblank status15:41
SpamapSI wonder if I missed something in my pipeline config15:42
Diabelkohello15:45
pabelangerSpamapS: status is failure in your paste15:46
pabelangeroh15:46
pabelangerat the end15:46
SpamapSpabelanger: right15:47
pabelangerodd, that you have 2 status fields15:47
mordredSpamapS: you have a misplaced '15:47
mordredno? oh - nevermind15:47
mordredI can't read either15:47
SpamapSactually15:47
jeblairSpamapS: i see http://paste.openstack.org/show/621341/ in the github test fixtures15:47
SpamapSthat log message is borken15:47
*** maxamillion has quit IRC15:48
DiabelkoDo you guys have a date in mind for Zuulv3 release?15:48
SpamapShttps://github.com/openstack-infra/zuul/blob/feature/zuulv3/zuul/driver/github/githubreporter.py#L114-L11815:48
SpamapSDiabelko: hi! Not really. We'll think about locking down 3.0 once OpenStack is fully migrated onto zuulv315:49
mordredDiabelko: we're planning openstack rollout next wednesday (sept 25) - after that we expect  it to be a couple of months before we cut a 3.0 release and point other people at using/running it15:49
Diabelkomhm, got it15:49
mordredDiabelko: (we want up update docs and some other things like the puppet-openstackci repo 3rd party operators use before hand)15:49
Diabelkois there any difference in job definitions and layouts I should be aware of before migrating?15:49
jeblairDiabelko: we'll have a roadmap for the v3 release after we do the openstack cutover15:50
mordredDiabelko: yes. a lot - lemme get you some links15:50
Diabelkowe're in the process of migrating from Jenkins to Zuulv2.515:50
Diabelkoand I need some info to make sure I'm not wasting a lof of effort in rewriting those jobs ;)15:50
SpamapSso many answers. :)15:50
Diabelkomordred: thanks15:50
mordredDiabelko: yah - you're at a 'fun' time, as v3 is runnable but not yet documented, and 2.5 is definitely on its way out the door ...15:51
mordredDiabelko: https://docs.openstack.org/infra/zuul/feature/zuulv3/user/config.html is the reference docs for job content15:51
jeblairDiabelko: the best thing you can do is avoid using jenkins plugins -- if you can make your jobs just use shell snippets, it will be easier to migrate.15:51
mordredDiabelko: we added this: https://docs.openstack.org/infra/manual/zuulv3.html for openstack users which might also be helpful15:51
Diabelkoawesome15:52
mordredDiabelko: http://git.openstack.org/cgit/openstack-infra/project-config/tree/zuul.yaml http://git.openstack.org/cgit/openstack-infra/zuul-jobs and http://git.openstack.org/cgit/openstack-infra/openstack-zuul-jobs all have v3 content in them if you prefer looking at examples15:52
Diabelkoand yes, I am aware 2.5 is going to be obsolete pretty soon15:52
Diabelkoall right, thanks15:53
DiabelkoI'll take a look at that15:53
mordredDiabelko: we have a migration script that will convert JJB to zuulv3 - it's not NEARLY perfect and contains openstack-specific transformations in a few places15:53
mordredDiabelko: but once we use it next week for openstack, should be a decent starting point for other folks who also want to do the transition15:53
SpamapSquestion about that log line I pasted earlier15:53
SpamapSshouldn't the {}'s be resolved?15:54
Diabelkomhm, sure15:54
Diabelkoit seems like I better convince people to hold off a little bit15:54
SpamapSurl is the result of item.formatUrlPattern15:54
mordredDiabelko: yah - a few more weeks and you'll have a much better path straight to v315:55
Diabelkoespecially that I know there's dependency graph coming15:55
mordred++15:55
mordredSpamapS: yes - formatUrlPattern seems like it should be producing expanded urls15:56
SpamapSyeah so I'm confused now15:56
mordredSpamapS: so - the status: at the end of the line is a red herring15:58
SpamapSmordred: yeah figured that out15:59
SpamapSI think that's a little borken15:59
mordredk. good15:59
mordredyah - easy fix for that15:59
SpamapS(though it does help me narrow down the Reporting log messages)15:59
mordredSpamapS: just so I'm fully up to speed on where you're at and what you're looking at16:01
SpamapShttp://paste.openstack.org/show/621342/16:01
SpamapSthere's my pipeline configs.. any red flags?16:01
mordredSpamapS: the issue you're running in to is that while the job is running there is no update to the PR status with a link to the per-change zuul status page right?16:02
SpamapSmordred: right16:02
SpamapSmordred: also secondary issue: the link in comments is finger://16:02
mordredSpamapS: yah16:02
SpamapSHowever, web streaming _does work_ if I copy the build id out of logs into my url bar ;)16:02
mordredSpamapS: if you just open up the full status page while a job works, does *that* show the stream urls?16:03
SpamapSoh wait hah16:03
mordredSpamapS: or are the links in the status page themselves broken?16:03
SpamapSno status-url in start:16:03
SpamapSlet's try that.. just for fun16:03
SpamapSI have no "status page"16:03
SpamapSI have status.json16:03
SpamapS(I may have a status page, but I don't know where BonnyCI is stuffing it)16:04
mordredgotcha. you're wanting the stream urls to go directly into the status reporting then16:04
mordredSpamapS: so - at the moment, the logic for emitting finger:// vs. stream.html urls is in formatJSON16:06
SpamapS2017-09-18 09:06:04,786 DEBUG zuul.ExecutorClient: Build <gear.Job 0x7f8d940de320 handle: b'H:10.36.156.42:20' name: executor:execute unique: df4ce646ce914da097c43b827d7ddabe> update {'worker_log_port': 9099, 'worker_hostname': 'zuul1.cloud.phx3.gdg', 'url': 'finger://zuul1.cloud.phx3.gdg:9099/df4ce646ce914da097c43b827d7ddabe', 'worker_name': 'zuul1.cloud.phx3.gdg'}16:06
mordredSpamapS: if you want zuul to report static/stream.html instead of finger:// - we may need to do a little work16:07
*** maxamillion has joined #zuul16:07
SpamapS2017-09-18 09:05:58,655 DEBUG zuul.GithubReporter: Reporting change <Change 0x7f8d940c1da0 2,e463c389bf3bff6473a6aa2ddda05986d714d0ed>, params {'comment': False, 'status': 'pending', 'status-url': 'http://zuul1.cl16:07
SpamapSoud.phx3.gdg/zweb/static/stream.html?uuid={buildset.uuid}&logfile=console.log'}, status:16:08
SpamapSOk, so _that_ looks right except the unresolved URL16:08
SpamapSThat was fixed by adding a status-url to the pipeline16:08
SpamapSmordred: I honestly don't care about status.json right now16:08
SpamapSI just want the PR status to get updated right16:08
mordredSpamapS: right.I know - what I'm saying is that the url you want is produced currently in formatJSON so I was thinking it was possible we might need to write a patch to expose the right thing for yo - but it seems you're much closer, so that's good16:09
jlkSpamapS: did you figure out your reporting URL thing?16:09
SpamapSjlk: nope, still working through it16:09
mordredjlk: he's working on that right now16:09
jlkah okay16:09
SpamapSjlk: I am guessing Bonny's zuulv3 isn't quite functional on this level either.16:10
jlkI apologize in advance for any boneheadedness you run into.16:10
jeblairSpamapS: s/buildset.uuid/build.uuid/16:10
jlkSpamapS: it might not be. I know that jamielennox had some things up for reporting URL type things, we used to define it in the pipeline, and I don't know if it's the same still16:10
SpamapSOhh this is annoying and a github problem I think16:10
SpamapSjeblair: THANKS!16:10
SpamapSso.. I'm using a personal repo and I have the zuul user as a collaborator16:11
SpamapSand I THINK there are issues with collaborators clearing statuses16:11
SpamapSjlk: do you remember anything like that?16:11
SpamapSI'm actually getting the right URL(wrong uuid until jeblair fixed me) on new PR's... just not on rechecks.16:11
jlkclearing statuses?16:12
SpamapSjlk: yeah, the PR statuses of pending/failure/success ...16:12
jlkWe don't "clear" status so much as set a new one.16:13
jlkIt _should_ set a "pending" status on the start of the pipeline job16:13
SpamapSoh I think I know what happened16:13
SpamapSthis is weird16:13
SpamapSso because my pending status had no url16:13
SpamapSnew statuses couldn't16:13
SpamapSnow the urls are working16:13
jlkhuh, weird16:14
jlkthat's an odd scenario16:14
SpamapSOk I think I have status urls now16:15
SpamapSstill finger in comments tho16:15
SpamapS    tox-linters finger://zuul1.cloud.phx3.gdg:9099/57a374b77ae0470c81895b541803e827 : RETRY_LIMIT in 1m 13s16:15
jlkthat finger stuff is relatively new, I wonder if that's an option toggle.16:16
jeblairwell, you get a finger url if the log copying playbook doesn't get run or return the log url16:16
SpamapSoh good point16:17
SpamapSI think somebody already pointed that out to me actually16:17
SpamapSneed a zuul_result16:17
jeblairyep16:17
SpamapSand in this case I'm standalone so I just have to copy the logs into somewhere on the same box that has a webroot16:17
SpamapSthis is Unix16:18
SpamapSI *know* this.16:18
jeblairyou *can* override it, but the point of zuul_result was so that you don't have to have the url pattern in two places (zuul config and log playbook)16:18
jlkYou make a zuul_result or you keep getting the finger.16:19
SpamapSit puts the url pattern in the results or it gets the finger16:19
jlkbasically16:21
mordredSpamapS: last week there was a lurking joke that we should add an emoji to the status page next to the stream.html with the finger url - and it should be a finger emoji16:21
mordredSpamapS: we figure there would be no negative unintended implications of small finger emojis16:22
SpamapSwhere is the example for zuul_result?16:22
* SpamapS needs to crib it :-P16:22
SpamapSmordred: of course. fingers are universal.16:23
SpamapSnearly 100% inclusive16:23
SpamapSIt's not called zuul_result is it?16:25
* SpamapS thinks we said the wrong word16:25
SpamapSzuul_return16:26
* SpamapS goes back to hacking16:26
jlkalright, time to figure out where I was before my brain got all melty at the end of last week.16:29
SpamapSghhrraaahhh github! (statuses seem to be a best-effort.. sometimes works, sometimes not)16:33
jlkIs there high priority things to review today?16:35
jlkSpamapS: the API is just saying "LOL NOPE" ?16:35
SpamapSjlk: actually I never see what the API ever says16:35
SpamapSbut it's just that status urls were working16:35
SpamapSand now they're not16:35
SpamapSand it's not clear why :-P16:35
jlktaking the call but not actually doing the thing?16:35
jlkhuh...16:35
jlklike, our code is not sending it right, or they're code is not accepting it right?16:35
SpamapS2017-09-18 09:34:54,974 ERROR zuul.QueueItem: Error while formatting url for job None: unknown attribute 'dict' object has no attribute 'uuid' in pattern http://zuul1.cloud.phx3.gdg/zweb/static/stream.html?uuid={b16:36
SpamapSjeblair: build.uuid?16:36
jlkugh python. Tell me the name of the variable, that'd be so much more helpful16:36
SpamapSjlk: I'm also just in 1 hour before demo panic mode.. so you can probably ignore most things I put a ! on ;)16:36
SpamapSneed to quit touching it16:37
* jlk deletes crass response16:37
SpamapSjlk: oh I know it's build16:37
SpamapSI had buildset.uuid and it was the wrong uuid apparently ;)16:37
SpamapSbut build doesn't seem to have a .uuid16:38
jlkI wonder if we're not setting that in the github driver?16:38
SpamapSbuild.uuid? that's not really in the github driver16:39
SpamapSthat's an internal zuul object that the formatter gets16:39
jeblairSpamapS: oh weird, buildset and build should both be valid.  the github tests use buildset.uuid.  but that doesn't make sense to me; i think build.uuid is what you'd want16:40
jeblairbut then, i don't actually know what this url is for... so.16:40
SpamapSjeblair: is it possible this is before build.uuid is set for some reason?16:40
SpamapSjeblair: I need the uuid to pass to stream.html16:40
SpamapSwhich I thought would be build.uuid16:40
jeblairSpamapS: yeah, i think if there's no build object this can happen16:41
SpamapSbuildset.uuid is the thing above that16:41
jeblairSpamapS: oh of course, this is for overall item status, so build doesn't make sense anyway16:41
SpamapSso buildset.uuid was the thing?16:41
jeblairSpamapS: yeah, that will get you a value, but build.uuid will not in this case.16:41
SpamapScause it didn't look right when I tried it16:42
jeblairSpamapS: having said that -- what is this url for? :)16:42
SpamapSjeblair: github has triggered a start in a check pipeline, I want to set the "Details" link on the 'pending' status16:42
SpamapSI wonder if there's a model mismatch at play here16:42
jeblairSpamapS: right, but what is at this url?16:42
SpamapSjeblair: static/stream.html16:42
SpamapSso people can watch16:42
SpamapSso the problem .. now I see it16:43
SpamapSthere's multiple streams possible16:43
SpamapSbut this is one trigger16:43
jeblairSpamapS: oh, i see that now, sorry; was confusing it with an earlier url.16:43
SpamapSso I opened a pr on  fooorg/barrepo and it may fire off 5 jobs.. but we only set one status16:43
jlkyeah I don't know if Zuul itself has a url that's suitable to look at all the potential jobs for a given pipeline + build16:43
jlker pipeline + change16:43
jeblairSpamapS: right.  so i think the idea there was to link to the status page, but filtered for the one item.  then you can follow that to the 5 jobs.16:43
SpamapSright and I don't know if I have a status page16:44
SpamapSjlk: do you know if bonny's status page works for v3 with the tenant prefixed status.json?16:44
SpamapSI can't even find the status page16:44
jlkI don't, I think that's a jamielennox question.16:44
SpamapSyeah me too ok16:44
jlkalso there's work going on currently to move all this over to webapp16:44
jlker zuul-web16:45
SpamapSyeah16:46
mordredI believe the curent SpamapS thing is a stop-gap to just get a URL there that doesn't then involve copy/edit/paste - with the plan being the one-item-status-page thing16:46
mordredbut getting him all the way to that for today's demo is highly unlikely :)16:46
SpamapSI am just going to try and jam in the usual status html and see if that works16:46
mordredjlk: in answer to your question from earlier - gerrit is down for upgrade today, so no, no pressing zuul reviews (unless you have them locally in a gertty, in which case "all of them")16:47
jlkI don't see much in gertty16:47
jlkI'll go back to banging on webooks via zuul-web16:48
mordredjlk: cool.16:49
SpamapSweird, the status app is in the zuul-scheduler role, but for some reason it didn't get put onto my server16:52
SpamapSwoot status page working16:55
mordredSpamapS: woot!16:56
mordredSpamapS: and with the correct links?16:56
* SpamapS fist pumps17:00
SpamapSPR -> status page works -> log stream works17:00
SpamapSnow result logs aren't working17:00
SpamapSbut I can like.. watch that17:00
SpamapSand see why17:00
jlkhuzzah!17:01
SpamapS2017-09-18 17:01:14.500235 | localhost | the field 'args' has an invalid value, which appears to include a variable that is undefined. The error was: 'zuul_log_path' is undefined17:01
SpamapSthat'll do it17:01
jlkjeblair: for gearman jobs, did you want them all to start with "zuul:" ?  I'm registering a job for each github based connection, and so it'll look like: "zuul:github:<connection_name>:payload". Seems long, but does that make sense?17:30
*** electrofelix has quit IRC17:35
SpamapS2017-09-18 17:36:16.648727 | [Zuul] Log Stream did not terminate17:36
SpamapSgetting these all over. what's that?17:36
SpamapSmordred: Shrews ^ ?17:36
Shrewsi have not seen that before17:36
pabelangerwe did get that a while back, not sure it is still an issue. zuul_stream was still running I think?17:37
* SpamapS is going to blame centos17:38
jlkI thought mordred was chasing something like that17:39
Shrewslooks like the thread the callback plugin creates to do the streaming does not terminate as it should17:40
SpamapSI won't worry about it17:40
pabelangerya, I don't think it impacted zuulv3.o.o when I saw it17:40
SpamapSIt's annoying in the logs17:41
pabelangermore like a warning17:41
pabelanger+117:41
pabelangerSpamapS: do you get any streaming? or just the spamming of it?17:41
pabelangerif no other streaming, maybe check firewall ports17:42
SpamapSI get streaming17:43
SpamapSit's great17:43
Shrewspabelanger: did the nodepool issues get resolved?17:46
jeblairjlk: i think we can drop zuul from that; i think '<driver>:' is probably sufficiently namespaced...17:46
Shrewsand was it just the config?17:46
jlkjeblair: alrighty17:47
SpamapSI see you guys talking about 'keep' all the time17:47
SpamapShow do I get my executor to 'keep' ?17:47
* SpamapS wants logs to stick around17:47
jeblairSpamapS: "zuul-executor keep"17:47
SpamapSoh17:47
jlkjeblair: also, how do we feel about the github driver exclusively owning the URL path of /connection/{connection}/payload   ?17:47
SpamapSlike17:47
SpamapSin the systemd job?17:47
jeblairjlk: you might consider dropping 'connection_name' from that to make it more robust against name changes17:48
SpamapSor does it poke something into a command socket?17:48
SpamapSand then if the latter, is there an unkeep?17:48
jeblairjlk: heh, my last comment was about gearman job names17:48
jeblairjlk: you might consider dropping 'connection_name' from *the gearman function name* to make it more robust against name changes17:48
jlkjeblair: hrm, but it's a different handler for each connection17:49
pabelangerSpamapS: almost, I have a change up to no longer share providers across launchers17:49
pabelangerSpamapS: sorry17:49
pabelangerShrews: ^17:49
pabelangerShrews: once gerrit is back online, I hope to finish it up17:49
jlkeach connection object will get their own handler. If you have two github connections, each are separate connections17:49
jlker separate job handlers17:49
jeblairjlk: hrm17:49
jlkjust like they each would register their own http handler in the old method17:50
Shrewspabelanger: i sort of feel like we should build some protection against that (or just plain ol' fully support it, which is much more work)17:51
Shrewsnot quite sure how to do the former though (or the latter for that matter)17:51
jeblairjlk: yeah.  if we wanted to do my suggestion, we'd have to add *another* layer of indirection in there (a dispatcher in the github driver to hand gearman jobs to the different connection threads.  it's possible, but maybe not worth it?)17:54
jeblairjlk: so let's do your thing for now.  :)17:54
jlkseems complicated :)17:54
jlkhow do you feel about github owning the "payload" final target though?  so that zuul_url/connection/<name>/payload   is exclusively github driver's playground?17:55
pabelangerShrews: Ya, I didn't realize it was incorrect until jeblair linked the zuulv3 spec again.17:55
tobiashShrews: or document that this can lead to races about max servers (which could be toleratable in some deployments)17:55
jlk"payload" seems awfully generic, and other drivers may need to do the webhook like ingest.17:55
pabelangerwe can run multiple nodepool-launcher, just not with same provider info17:56
jeblairShrews, pabelanger: yeah, the short version is that you can wedge a provider if you ask for a number of nodes sufficiently close to the quota of the provider *and* there is insufficient turnover (eg because min-ready is too high and causing a node to sit forever).  in tracking this down, we found that we had multiple launchers running for the same providers which exacerbated the effects (basically, caused the problem to affect 2 requests instead ...17:59
jeblair... of 1).17:59
jeblairShrews, pabelanger: we also found that, in this specific case, that some of the nodes in the stuck request had errored.  if nodepool had been able to clear the errored nodes it would have been able to un-wedge itself sooner, but doing that is also a substantial change (you don't want it to end up in an infinite loop if the cloud is erroring)18:00
jeblairShrews, pabelanger: i think the thing that would have helped this situation the *most* would be a way to cycle out ready nodes when a provider is 'paused' waiting for quota.18:01
jeblairthat's probably the weakest point in the algorithm, the easiest to fix, and will make the most difference.  it certainly would have avoided the problem this time.  and in fact, "delete the ready node" is what i did to unwedge it.18:02
pabelangerah cool18:03
jeblairjlk: i think we need to namespace the url *either* with /github/  (ie, /<driver>/), or declare that /connection/<name>/ is owned by the driver.  i don't think anything else depends on connection/name  (except keys, and they are moving), so i think either is okay?18:04
* mordred likes driver18:04
mordredalthough ...18:04
jlkoh I hadn't thought about it, that /connection/<name>/ is inherently owned by the connection, which is mapped to the driver.18:05
mordredjlk: if we had a 'global' or a 'driver' version, how do we map from a payload to the correct connection?18:05
mordredah right18:06
jeblairmordred: yah, i think it's /github/payload/<connection> or /connection/<connection>/payload18:06
jlkbut the way zuul-web is written, we have to sit on all of the potential connection names. It registers a path of /connection/{connection}/payload, and sends them all to the handle github function. We could put a different function there that figures out which driver gets it, but18:06
mordredjlk: I think we should write a new function - so that the github driver registers a per-connection endpoint18:07
jlk /github/payload/{connection} seems reasonable to me. It'll look pretty okay in the github app configuration too18:07
mordredmeaning after the call, we'd wind up with /connection/github/payload and only that18:07
jlkmordred: it kind of dues18:07
jlkwell sortof18:07
jlkzuul-web works differently18:07
jlkI don't think there's a way to live-add a new endpoint18:07
jlkI think that's what you're suggesting, is that we add one18:08
jlkright now we register a new gear job18:08
mordredyah - I've got some half-finished code from a while back adding that - but pointing you at it will need to wait until gerrit is back :)18:08
jlkwe've got half that code from registering the path on webapp18:09
jlkI had hoped to delete all that though :D18:09
jlkIf we sufficiently namespace by the driver, I think we're okay18:09
jlklet a driver own a top level /<path>18:09
jlkwhich matches the gear job of <driver>:something18:10
mordredah - that's fine by me18:10
mordredjlk: I'd love to keep a container in the url if we can -- like /driver/{driver}/payload or whatnot - so that potentially in the future we could add a GET /driver and get a list of available drivers18:13
mordredjlk: but I don't feel _super_ stringly about that - just a thing to think about as you poke at it18:13
jlkI'd have to be /driver/{driver}/payload/{connection}  or something like that18:13
jlkdoes that seem reasonable?18:14
*** hasharAway is now known as hashar18:21
mordredjlk: seems reasonable to me - can you tell me again just so I'm caught up why /connection/{connection}/payload isn't working?18:53
mordredjlk: (I'm not sure I've pieced togetherthe entire state properly in my head)18:53
* rbergeron waves at the humans18:56
* mordred waves at the rbergeron19:00
* SpamapS finishes demo and passes out19:00
mordredSpamapS: how did it go?19:00
SpamapSmordred: ++19:00
mordredSpamapS: woot!19:00
SpamapSmordred: There's a lot of knowledge of Jenkins-Pipeline (the Groovy thing) so it was a new and interesting audience to show Zuul to.19:01
clarkbdid zuul coin pipeline before jenkins? I've been noticing the term get used a lot but not sure if zuul inherited it from somehwere19:02
SpamapSI wonder if we could implement the Blair algorithm in Groovy19:02
SpamapSclarkb: I don't think so19:02
SpamapSpipeline is a 10x overloaded term19:02
SpamapSFYI, fixed python3.5 has hit *-updates, so infra can remove it from the ppa19:05
mordredwoot19:05
pabelangeryay19:27
dmsimardSpamapS: what was the purpose ? you're trying to replace pure jenkins by nodepool+zuul ?19:27
SpamapSdmsimard: sort of...19:58
SpamapSdmsimard: it's more to show that there is another way :)19:58
jeblairwell, we *were* using the word before the jenkins "workflow-aggregator" plugin started to rebrand itself as 'pipeline'.  ;)20:02
jeblairbut i borrowed it from the vfx industry.20:02
*** hashar has quit IRC20:03
pabelangerILM :D20:04
jlkmordred: I'm wary of using /connection/{connection}/payload, because in zuul-web we have to register that route as a function, and the function is github specific. If I add a second connection, say gitlab, that also needs to deliver a payload, we'd have to come up with another verb than "payload" for it to make use of.20:51
mordredjlk: AH. gotcha. thank you20:51
jlkmordred: alternatively, we make a "handle payload" function that determines what driver the connection uses and then routes to a sub-function specific to that driver.20:51
mordredjlk: that makes it very clear to me where the issue is20:51
mordredjlk: yup. I like that a lot20:52
jlkdefine "that", which you like20:53
*** dkranz has quit IRC20:53
jlkmordred: option A) /driver/{driver}/payload/{connection}  or option B) a generic handle_payload that calls out to appropriate driver level handlers after introspection?20:56
jlk(I'm not even sure how to make option B) work)20:56
jlkI guess github could just be hardcoded to know about "/payload" as a final path, or /payload/connection or..20:57
mordredjlk: both sound good - I think a) solves the immediate problem ...20:57
ShrewsAre we having a zuul meeting today?20:58
jlkI'll run with A and see where it gets us for reviews and such.20:58
mordredjlk: for b) if we had a /connection/{connection}/payload - and then had that call a gearman function called payload:<connection name> - then it should work?20:59
jlkShrews: I'll likely miss the meeting, have to play a spot of family taxi.20:59
clarkbits probably worth having a quick sync up meeting? I'm guessing gerrit will still be reindexing at meeting time and so we shouldn't be too distracted by gerrit upgrade21:00
mordredjlk: or - add a method to the connection plugin interface and have /connection/{connection}/payload call connection.handleWebhookPayload for the correct connection - but have thedefault impl of that plugin method be "submit gearman function payload:<connection>"21:00
mordredclarkb: wfm21:00
jlkmordred: not exactly. We are doing github signature validation at the zuul-web level, before passing along to gearman. I couldn't figure out a way to pass things through gearman that didn't break the body signature.21:00
jlkmordred: so we'd have to have some logic in zuul-web to deal with "what driver is this" if we have to do anything driver specific.21:01
jlkalternatively, if I could figure out how to take an aiohttpd Request object and shove it through gearman and still have a valid signature, that'd be neat!21:01
mordredjlk: nod. so a) is the right choice for now - since that allows us to register a function with zuul-web on a per-driver basis21:06
Shrewsjlk: maybe it could be pickled? never used that, but i understand that library is useful for such things21:06
jlkthe object has to be able to be serialized, and for whatever reason the aiohttp object can't.21:07
Shrewsi wouldn't recommend such complexity, anyway  :)21:07
jlkI can validate the signature of the binary blob, but I can't transport that binary blob. If I encode it as utf-8 I can transport it, but it breaks the signature21:07
jlkhrm.21:09
jlkthinking about this more21:09
jlkI think A) is still problematic, sortof. Maybe not.21:10
jlk /driver/{driver}/payload/{connection} -> _handlePayloadPost -> _handle{Driver}Post (where we code one for Github now, and send 404s for anything else, but allows for new drivers to write a handler here).21:11
*** jkilpatr has quit IRC21:39
SpamapSpickle is almost never the right answer. :-/21:41
SpamapSjlk: you can likely serialize the parts that will matter later.. the headers and the body, individually.21:42
jlkUnless the question is "pickle backs?"21:42
SpamapSwe can pickle backs?21:42
jlkhttps://en.wikipedia.org/wiki/Pickleback21:43
SpamapSjlk: you can base64 encode it.21:43
jlkhrm.21:44
SpamapSIs it expected to be a purely binary blob though?21:44
SpamapSI guess if you don't know what to expect.. you can't assume until you get to your logic engine on the other side of gearman.21:44
jlkit comes in as binary, but you can decode it into json21:44
jlkright21:45
jlkwe want zuul-web to do as little with it as possible21:45
SpamapScould just have a cascade there.21:45
SpamapStry: json_decode except ValueError: try: base64encode21:45
SpamapSthen do that in reverse on the other side21:46
SpamapSactually you can wrap the base64 or json in an envelope with the type21:46
jlkugh. so it's bytes object21:47
jlkcan't "decode('base64')" it21:47
SpamapSyou can't really do that in py3k anymore anyway IIRC21:48
SpamapSyou have to use the base64 module directly21:48
clarkbya21:48
SpamapSwelcome... TO THE FUUUUUTURE21:48
jlkhuh21:48
SpamapSbase64 isn't really an encoding21:48
SpamapSbecause it doesn't encode characters.21:49
SpamapSthat was a bug in py27 that they allowed it21:49
SpamapSpy2k I guess21:49
jlknope21:51
jlkdamn21:51
jlkif I use base64.decodebytes(body)  (where body is the bytes object) it alters it enough to break the signature.21:51
jlkalthough.21:51
SpamapSjlk: that shouldn't be. :-/21:51
jlk¯\_(ツ)_/¯21:52
jlkwe do a secret.encode('utf-8'), body, hashlib.sha1).hexdigest()21:53
jlkbody if passed in as bytes works, body if passed in as base64.decodebytes(bytes) it does not21:53
SpamapSyeah.. hrm21:54
SpamapSso body is something like b"a b c d\0" and it works, but when you decode, you get like, b"a b c d"?21:54
jlkmaybe?21:54
dmsimardAre we doing a Zuul meeting in 5 ?21:55
dmsimardjeblair, mordred, clarkb, Shrews, SpamapS, jlk ^21:55
jlkI'm out.21:55
clarkbI'll be around though will want to focus on the gerrit upgrade once reindexing is done21:56
clarkbmaybe we can do a quick sync up and then get back to gerrit things?21:56
clarkbwe can also use infra meeting tomorrow for zuul things if it helps21:56
dmsimardAlthough the ptg just ended, it's probably worth pointing out what's on our plate if we want to flip the switch next monda21:56
dmsimardmonday*21:56
jeblairyeah, i think the better thing might be to (mostly?) skip this one and sync up at infra meeting21:56
SpamapSjlk: I typically use base64.b64encode and base64.b64decode ...21:56
dmsimard+1 for infra sync21:56
SpamapSI wouldn't mind hearing a zuul-specific recap of zuuly things discussed at ptg21:57
SpamapSSounds like there were some designish things around zuul-web discussed.21:57
clarkblets plan for doing ptg recap tomorrow in infra meeting (its mostly zuul things)22:00
*** jkilpatr has joined #zuul22:17
SpamapShmmm... I thought we got rid of the 30s lag after playbooks finished when we added --die-with-parent ?22:18
SpamapS2017-09-18 22:18:13.528128 | TASK [fetch-stestr-output : Register stestr directory]22:19
SpamapS2017-09-18 22:18:43.674119 | [Zuul] Log Stream did not terminate22:19
openstackgerritMonty Taylor proposed openstack-infra/zuul-jobs master: Rename tox_upper_constraints_file to tox_constraints_file  https://review.openstack.org/50234822:47
mordredSpamapS: I also thought we had that fixed22:48
jeblairit was a 60s lag i think22:49
jeblairand i believe it is fixed22:49
mordredSpamapS: you may want to poke a little bit more on that Log Stream did not terminate ... check logs for tracebacks - it might be something different/evil22:51
openstackgerritJesse Keating proposed openstack-infra/zuul feature/zuulv3: Move github webhook from webapp to zuul-web  https://review.openstack.org/50426723:09
jlkmordred:  jeblair: ^^ that does the thing we talked about, leaving space for other drivers to hook into their own /payload path so that GitHub doesn't squat on all of it.23:09

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