Wednesday, 2018-11-28

openstackgerritJames E. Blair proposed openstack-infra/zuul master: Use the SQLAlchemy ORM  https://review.openstack.org/62042600:04
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Add artifact table  https://review.openstack.org/62042700:04
corvusthat's not out of left field -- that's the next step in two efforts: promotion pipelines, and better log display00:08
*** j^2 has quit IRC00:10
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Add artifact table  https://review.openstack.org/62042700:11
SpamapScorvus: +++00:20
SpamapSIf we can get a link between gate artifact and branch commit.. zomg.00:21
clarkbcorvus: left a note on the ORM change00:21
clarkbcorvus: a higher level thought there with more explicit ORM use is maybe we want to use sqlalchemy connection pooling too? Though with the cherrypy threading model I'm not sure that helps00:26
openstackgerritIan Wienand proposed openstack-infra/zuul master: Rework zuul nodepool stats reporting  https://review.openstack.org/62028502:23
openstackgerritIan Wienand proposed openstack-infra/zuul master: Add a statsd check for clashing keys  https://review.openstack.org/62043602:23
ianwi wonder if that will find anything else02:23
*** bhavikdbavishi has joined #zuul02:51
*** bhavikdbavishi has quit IRC03:30
openstackgerritIan Wienand proposed openstack-infra/zuul master: Add a statsd check for clashing keys  https://review.openstack.org/62043604:08
openstackgerritIan Wienand proposed openstack-infra/zuul-jobs master: upload-logs-swift: Cleanup temporary directories  https://review.openstack.org/59234004:48
openstackgerritIan Wienand proposed openstack-infra/zuul-jobs master: upload-logs-swift: Make indexer more generic  https://review.openstack.org/59285204:48
openstackgerritIan Wienand proposed openstack-infra/zuul-jobs master: upload-logs-swift: Stub out dry run in the uploader  https://review.openstack.org/59292904:48
openstackgerritIan Wienand proposed openstack-infra/zuul-jobs master: upload-logs-swift: Create a download script  https://review.openstack.org/59234104:48
openstackgerritIan Wienand proposed openstack-infra/zuul-jobs master: upload-logs-swift: Add a unicode file  https://review.openstack.org/59285304:48
*** pcaruana has joined #zuul05:09
*** bhavikdbavishi has joined #zuul05:54
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix test race in test_hold_expiration_higher_than_default  https://review.openstack.org/62022206:20
*** chkumar|away is now known as chkumar|ruck06:59
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix test race in test_hold_expiration_higher_than_default  https://review.openstack.org/62022207:24
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix test race in test_hold_expiration_higher_than_default  https://review.openstack.org/62022207:29
tobiashShrews: with that on top of the cache stack I have now 32 successful tox runs in a row and still running ^07:31
openstackgerritQuique Llorente proposed openstack-infra/zuul master: Remove uneeded if statement  https://review.openstack.org/61798407:38
tobiash53 now, still no failure07:48
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Fix stuck job caused by exception during repo update  https://review.openstack.org/59069707:50
tobiashcorvus: I overlooked your question in 610029, responded now07:56
*** hashar has joined #zuul08:03
*** quiquell has joined #zuul08:11
quiquellGood morning08:12
tobiashmorning08:12
*** bjackman has joined #zuul08:13
*** gtema has joined #zuul08:13
quiquelltobiash: Have an issue with a pipeline08:13
quiquelltobiash: is defined like this https://paste.fedoraproject.org/paste/gBAutn05MBrOh9Vx9Cbwpg08:14
quiquelltobiash: so there is no reference to my git.openstack.org connection08:14
quiquelltobiash: I still see git.openstack.org changes appearing at check08:14
quiquelltobiash: Do I have to explictily desactivate it  ?08:14
tobiashquiquell: what is the qinqon connection, that triggers on ref-updated?08:15
*** zer0c00l has joined #zuul08:16
tobiashquiquell: it should only trigger if a trigger matches08:16
quiquelltobiash: A testing repo at github, "git" driver08:16
quiquelltobiash: I have a connection with name "git.openstack.org" that points to openstack upstream gerrit08:16
tobiashquiquell: can you share the connections in the zuul.conf too?08:17
quiquelltobiash: https://paste.fedoraproject.org/paste/sP5h2JjsNm1os1wf1pinyA08:18
quiquelltobiash: they appear and disappear at check pieppeline the ones not expected08:19
*** themroc has joined #zuul08:19
quiquelltobiash: Is like zuul is render them and later discard them08:19
zer0c00lis there an example somewhere on how to make a nodepool-request?08:20
zer0c00li mean python code example?08:20
tobiashquiquell: so it enters the pipeline, zuul notices that nothing should be running and it disappears?08:20
tobiashzer0c00l: look here: https://git.zuul-ci.org/cgit/zuul/tree/zuul/nodepool.py#n5408:21
zer0c00lThank you tobiash08:21
tobiashzer0c00l: but beware that the zk model and interface is considered internal and might change without prior notice08:21
zer0c00li see08:22
quiquelltobiash: Looks like,08:22
zer0c00lso i have max-servers as 15 and min-ready as 1008:22
quiquelltobiash: Like zuul wrongly render it in the web before checking stuff08:22
tobiashquiquell: in this case the scheduler logs would be helpful08:22
quiquelltobiash: Something specific to find ?08:22
zer0c00lDoes that mean nodepool-launcher will automatically launch more nodes if it doesn't have 10 nodes in "ready" state?08:23
tobiashquiquell: maybe you found a bug in event filtering08:23
quiquelltobiash: Have it down now, will check if it appears again08:23
tobiashquiquell: ok08:24
quiquelltobiash: but pipeline looks correct ?08:24
tobiashzer0c00l: yes, it regularly checks the number of nodes in ready state and creates more if there are less than min-ready08:24
zer0c00lThank you08:24
tobiashquiquell: yes, the pipeline looks correct, so maybe you found a bug in event filtering08:25
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: Fix test race in test_hold_expiration_higher_than_default  https://review.openstack.org/62022208:30
tobiashShrews: ok, so that ran more than 100 times in a row without error on top of the caching stack, now rebased it to current master to aid landing the cache stack ^08:31
*** goncalo has joined #zuul08:49
*** jpena|off is now known as jpena08:57
*** bhavikdbavishi has quit IRC10:06
*** tobias-urdin has quit IRC10:10
quiquelltobiash: What's the format of log_config=/etc/zuul/scheduler-logging.yaml  ?10:14
*** tobias-urdin has joined #zuul10:16
*** electrofelix has joined #zuul10:39
tobiashquiquell: that's the standard python logging framework format10:40
tobiashI think it supports both the ini style and the yaml style10:40
quiquelltobiash: cool thanks10:50
*** hashar has quit IRC10:52
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Whitelist command processor thread in tests  https://review.openstack.org/62055910:57
tobiashthat should resolve a spurious test failure in zuul I discovered while running zuul tests in an endless loop ^10:58
openstackgerritBenoĆ®t Bayszczak proposed openstack-infra/zuul master: add fetch_vault_secrets Ansible module  https://review.openstack.org/62031111:01
*** bhavikdbavishi has joined #zuul11:22
*** dkehn has quit IRC11:26
*** nilashishc has joined #zuul11:55
*** jpena is now known as jpena|lunch12:02
*** bhavikdbavishi has quit IRC12:03
*** gtema has quit IRC12:05
*** goncalo has quit IRC12:26
*** goncalo has joined #zuul12:26
*** hashar has joined #zuul12:49
*** gtema has joined #zuul12:58
*** rlandy has joined #zuul13:00
*** jpena|lunch is now known as jpena13:02
*** bjackman has quit IRC13:23
*** dkehn has joined #zuul13:27
fricklerhmm, weird issue on the web interface. looking at http://zuul.openstack.org/builds?job_name=docker-publish-monasca-base&job_name=legacy-monasca-common-localrepo-upload the first change is listed as "77a540", but that starts with the second char of the id, the link is correct https://git.openstack.org/cgit/openstack/monasca-common/commit/?id=d77a54053e993d0d49d1f74e15ed76376a0c5fb713:39
tristanCfrickler: indeed, nice catch :) the substr call is wrong13:44
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: fix ref column value for newrev  https://review.openstack.org/62060213:49
openstackgerritMerged openstack-infra/zuul master: Use publish-zuul-python-branch-tarball job  https://review.openstack.org/61863414:29
mnaseris there a nodepool 'docker' driver anywhere? i see openstack/k8s/static14:32
mnasertrying to see if i can run containerized jobs in a vm with kata on it14:32
tristanCmnaser: there is a runC under review that could be adapted to use docker instead: https://review.openstack.org/53555614:38
pabelangermnaser: in the past we'd recommend the nova / docker integration for openstack, but don't think that is maintained any more14:41
mnaserpabelanger: yeah it's not unfortunately14:42
mnasertristanC: i guess that could be but still not merged14:42
pabelangerI've often thought, if nodepool could provision a node, then some how turn it static for x jobs, then you could write a playbook to leverage docker on that host14:43
*** hashar has quit IRC14:48
*** bhavikdbavishi has joined #zuul15:04
*** timrc has quit IRC15:06
pabelangermordred: if you have a moment, http://logs.openstack.org/fe/fea3ebeae0a08e19a52227a52e40b09d61d7c801/post/publish-zuul-python-branch-tarball/2f8663b/job-output.txt.gz is the new publish job, which should work like our release job. But having trouble understanding if zuul run yarn commands15:06
pabelangerwarning: no files found matching 'zuul/web/static/static/*/*'15:08
pabelangerthat doesn't look right15:08
mordredpabelanger: zuul should run yarn commands if the javascript tools are installed15:10
mordredpabelanger: so the tarball job should include the install-yarn role15:10
pabelangermordred: it does15:10
pabelangerhowever15:10
pabelangerhttp://logs.openstack.org/fe/fea3ebeae0a08e19a52227a52e40b09d61d7c801/post/publish-zuul-python-branch-tarball/2f8663b/job-output.txt.gz#_2018-11-28_14_42_14_96095115:10
pabelangerseems to skip installing yarn for some reason15:11
pabelangerlet me look why15:11
mordredcool. then it SHOULD be running the yarn commands and making the things ... so there might be an issue ... looking15:11
pabelangerokay, seems we don't install yarn, if we find a yarn.lock file15:12
pabelangerhttp://git.openstack.org/cgit/openstack-infra/zuul-jobs/tree/roles/install-yarn/tasks/main.yaml15:12
pabelangerIs that right? don't we want to install yarn package if lock file is found?15:13
pabelangerOh15:13
pabelangerit does that15:13
pabelangerit is because our yarn.lock file is missing15:14
pabelangerbecause we look in the wrong folder15:14
pabelangerI think i see the issue15:14
mordredoh - because it got moved into a subdir15:14
mordredyeah15:14
pabelangerk, let me update install-yarn15:15
pabelangerand make the lock file patch configurable15:15
*** chkumar|ruck is now known as chkumar|away15:17
mordredpabelanger: nice catch15:18
pabelangermordred: started poking into ansible roles to deploy zuul dashboard again, and couldn't figure out why wheels wouldn't work15:20
mordred:)15:20
openstackgerritPaul Belanger proposed openstack-infra/zuul-jobs master: Create yarn_lock_file_path variable for install-yarn  https://review.openstack.org/62062815:24
pabelangermordred: ^15:24
openstackgerritSorin Sbarnea proposed openstack-infra/zuul-jobs master: Use official pypi.python.org as fallback with pip  https://review.openstack.org/62063015:27
pabelangertobiash: thanks!15:29
mordredpabelanger: +A - thanks!15:31
*** quiquell is now known as quiquell|off15:41
openstackgerritTobias Henkel proposed openstack-infra/zuul master: executor: harden add_host usage  https://review.openstack.org/62063515:42
tobiashcorvus, clarkb, mordred: ^15:43
pabelangertobiash: thanks for helping drive this15:44
pabelangertristanC: you too15:45
tobiashpabelanger: no problem15:45
*** panda|rover is now known as panda|pto15:45
tobiashactually that's tristanC's patch, I just pushed it up ;)15:45
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Use the SQLAlchemy ORM  https://review.openstack.org/62042615:53
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Add artifact table  https://review.openstack.org/62042715:53
*** rlandy is now known as rlandy|brb15:55
mordredcorvus: when that add_host patch lands, perhaps we should cut a release?15:57
clarkbis there work to add a test for it yet?15:59
clarkb(I'd happily review it if so)15:59
corvusmordred: yes; unfortunately the release will come with a behavior change for semaphores....16:00
corvusmordred, tobiash, clarkb: do you think that's okay, or should we revert the semaphore change, make the release, then add it back and make another release?16:00
clarkbcorvus: is the semaphore change the one where we don't grab node locks first by default? if it is that change I think we can release with that in. I really doubt that behavior is relied on and the new behavior should be better for most users16:01
corvus(i feel like the semaphore change probably would not be too objectionable to have to deploy even for someone used to the other behavior)16:01
corvusclarkb: yes16:01
clarkb++ to releasing with semaphore change16:02
corvuslike, i don't think it's going to break anyone, it just might be mildly annoying to someone.  but most people will probably like it.16:02
mordredcorvus: yeah - I agree with it not being too objectionable16:04
corvushttps://zuul-ci.org/docs/zuul/releasenotes.html16:05
corvusthe new tenant options are the only other thing and they shouldn't be a prob16:05
mordred++16:05
*** j^2 has joined #zuul16:12
*** hashar has joined #zuul16:32
*** rlandy|brb is now known as rlandy16:34
SpamapSevery time we find one of these ansible restriction holes I am so happy we went with bubblewrap suspenders to go with the ansibelt.16:46
SpamapSFor the semaphore change, that's very much an "under the covers" behavior change IMO.16:47
SpamapSThe corner case that is affected is probably one relying on unstated behaviors of the semaphore.16:48
openstackgerritSorin Sbarnea proposed openstack-infra/zuul-jobs master: Use official pypi.python.org as fallback with pip  https://review.openstack.org/62063017:00
*** pcaruana has quit IRC17:03
*** bjackman has joined #zuul17:09
*** themroc has quit IRC17:09
*** gtema has quit IRC17:16
*** ssbarnea|bkp2 has quit IRC17:20
*** hashar has quit IRC17:27
*** ssbarnea has joined #zuul17:29
tobiash++ for release with semaphore change17:42
openstackgerritMerged openstack-infra/zuul master: Add allowed-labels tenant setting  https://review.openstack.org/61774017:52
*** sshnaidm is now known as sshnaidm|afk18:01
*** bjackman has quit IRC18:05
*** jpena is now known as jpena|off18:09
* Shrews getting sick of vim assuming i want new features enabled on each upgrade18:13
mordredShrews: ++18:13
corvusvim is getting new features?  i thought.... nevermind.18:13
Shrewsno, i don't want auto indenting. no, i definitely do not want code folding18:13
clarkbShrews: the code folding thing is particularly annoying with rst files18:14
clarkbzA ftw. I should figure out the vimrc incantation to just disable it18:15
Shrewsclarkb: that's *exactly* why i just disabled it18:15
clarkbShrews: please share the incantation if you have it :)18:15
Shrewsclarkb: set nofoldenable  in .vimrc18:15
clarkbtyty18:15
Shrewsi spent 5 minutes wondering where the hell the nodepool documentation went18:15
*** bhavikdbavishi has quit IRC18:20
Shrewspabelanger: the executor-zone value is a per-provider attribute, right? it doesn't have to be different for each pool within a provider18:21
mordredclarkb, Shrews: I can think of no circumstances where I want code folding18:23
* Shrews folds mordred like a cheap suit18:23
* mordred tries to get rid of the wrinkles18:23
pabelangerShrews: I think we can do per pool, that is how I original wrote it.18:24
pabelangerbut open to what others think18:24
Shrewspabelanger: ok, that's fine. our docs aren't really setup for common attributes within pools across drivers. just means i have to give that a bit of thought18:25
pabelangerk18:25
corvusShrews, pabelanger: it should be pool because AZ's can be pool scoped18:25
corvus(and you might want to match zuul executor zones to cloud availability zones)18:27
pabelangerYah, I think that is why I did per pool, to match how AZ's were done18:27
pabelanger+118:27
Shrewspabelanger: corvus: this exercise has revealed to me that we force each driver to parse and validate common config options (e.g., max-servers is common, but parsed in openstack driver). I think I'll spend some time fixing that up after this.18:27
clarkbalso pools are network scoped right? networking likely to be common reason for zoning18:27
fungiShrews: clarkb: what really got me lately was after a vim upgrade in debian it started grabbing all mouse events and doing strange things with cursor manipulation when all i wanted to do was highlight things to copy them into my x11 buffer and then middle-click to paste from it. not trivial to disable either18:27
corvusShrews: that sounds great18:27
corvusclarkb: ++18:28
Shrewsfungi: that sounds horrible18:28
Shrewsmaybe emacs really *is* superior18:28
fungithe "not easy to disable" part is because it hit all my servers where i need to disable it for root because `sudo vi /somefile` no longer did sane things in an x terminal when i wanted a basic x11 pointer18:29
corvusjlk: https://github.com/sigmavirus24/github3.py/pull/904 should be gtg now18:30
fungiShrews: clarkb: the "fix" turned out to be creating an /etc/vim/vimrc.local on all my servers (making sure to have it world-readable too) and then put "let g:skip_defaults_vim = 1" in there18:30
openstackgerritMerged openstack-infra/nodepool master: Fix test race in test_hold_expiration_higher_than_default  https://review.openstack.org/62022218:30
openstackgerritMerged openstack-infra/nodepool master: Cache node request zNodes  https://review.openstack.org/61880618:30
jlkcorvus: rad!18:30
fungiShrews: clarkb: i expect that probably solves your other new default behaviors as well18:31
fungiessentially stops vim from reading the kitchen-sink defaults configuration18:31
Shrewsfungi: ooh, neat. thx18:31
fungiso that your personal configuration is all that gets turned on18:31
fungithere was a time when vim shipped with example configuration you could copy and just include the features you wanted18:32
fungithen at some point they decided it was a good idea to turn that example configuration into default configuration which always gets loaded unless you explicitly say to not do that18:33
fungifrustrating to no end18:33
Shrewsfungi: hrm, that didn't work for me18:36
Shrewsoh well. good enough for now18:36
*** j^2 has quit IRC18:37
fungiShrews: what distro? maybe there's no (conditional) "source /etc/vim/vimrc.local" in your /etc/vim/vimrc18:42
*** electrofelix has quit IRC18:45
*** dkehn has quit IRC18:51
*** dkehn has joined #zuul18:53
Shrewsfungi: fedora 2918:56
fungiyou could probably add that line to the end of your /etc/vim/vimrc instead (or whatever fedora uses)18:58
*** dkehn has quit IRC18:59
openstackgerritMerged openstack-infra/zuul-jobs master: Create yarn_lock_file_path variable for install-yarn  https://review.openstack.org/62062818:59
openstackgerritJames E. Blair proposed openstack-infra/zuul-website master: Remove summit event notices  https://review.openstack.org/62068119:21
openstackgerritMerged openstack-infra/zuul master: executor: harden add_host usage  https://review.openstack.org/62063519:24
*** nilashishc has quit IRC19:35
*** mrhillsman is now known as mrhillsman|lunch19:46
*** greg-g has joined #zuul19:53
corvusthe executor patch landed... how about i go restart openstack's executors as a sanity check, then tag the release?20:27
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool master: Add arbitrary node metdata config option  https://review.openstack.org/62069120:27
tobiashSounds good20:28
openstackgerritDavid Shrewsbury proposed openstack-infra/nodepool master: Add arbitrary node metdata config option  https://review.openstack.org/62069120:30
Shrewspabelanger: ^^^20:34
*** mrhillsman|lunch is now known as mrhillsman20:35
*** openstackgerrit has quit IRC20:36
pabelangerShrews: cool, I'll review shortly. Of to meetup with family for photo with santa!20:36
corvuspabelanger: nice -- i guess you don't have as far to go to meet with santa20:37
pabelangerindeed20:37
AJaeger_pabelanger: too early for Santa...20:45
mordredcorvus: https://review.openstack.org/#/c/620427 had a sad. I think it's a timeout?20:52
mordredShrews: I'd like to discuss paint colors ...20:55
mordredShrews: for openstack, node-metadata reads to me like it would result in the key value pairs being set on the Server objects themselves, rather than being k/v data to be stored in zk. (reading the docs it's clear - but just looking at the config file my assumption would be metadata to be applied to the openstack object)20:56
mordredShrews: I'm not sure what name would be better than node-metadata that wouldn't have the same issue though20:57
clarkblabel-metadata maybe since that is a nodepool construct20:57
mordredyah - that's better - but then we have a list of labels, so label-metadata as a sibling to labels: also feels a little strange20:58
mordredmaybe pool-metadata?20:59
mordredor maybe nobody should ever listen to me about naming :)21:01
clarkbwe could also avoid "metadata" as the term and use attributes21:02
clarkbthen you have node-attributes these are nodepool ideas, and node-metadata is stuff that ends up in $cloud instance21:02
*** openstackgerrit has joined #zuul21:06
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Fix deletion of stale build dirs on startup  https://review.openstack.org/62069721:06
corvusnode attributes sounds good to me21:09
corvusmordred: that's curious.  i want to dismiss it as a random slow node (the py36 tests passed) but it is a sql change.  i wish we had more data there.21:10
corvusmaybe the sql tests are just close to the timeout limit?21:10
openstackgerritMerged openstack-infra/zuul master: web: fix ref column value for newrev  https://review.openstack.org/62060221:14
mordredcorvus: maybe?21:15
mordredcorvus: and yes- I really wish we had more than "StringException"21:15
openstackgerritMerged openstack-infra/zuul master: Use the SQLAlchemy ORM  https://review.openstack.org/62042621:28
corvuswell, obviously the new executor code is working...21:29
corvusso i'll tag 3.4.0 now?21:29
corvusor is this 3.3.1?21:29
corvusi think we've gone with point releases when the new features were minor before...21:31
corvusso maybe 3.3.121:31
corvusclarkb, mordred, tobiash: ^ ?21:31
clarkb3.3.1 wfm21:32
corvusmordred: do you mind writing the security announcement?21:40
corvusclarkb, mordred, tobiash: i'm ready to tag zuul 2728e5d4adac81b3c79c6a453676565fc10fda9d as 3.3.1  does that look right?21:40
clarkblooking21:40
clarkb2728e5d4adac81b3c79c6a453676565fc10fda9d is the commit before the orm changes. I like not tagging the orm changes so that lgtm21:42
clarkbalso that commit includes that harden add_host change21:42
corvusmordred: http://lists.zuul-ci.org/pipermail/zuul-announce/2018-June/000015.html is the last security email, you can pattern off of that21:43
Shrewsmordred: i also find openstack "node", zookeeper "node" and zookeeper "Node" object quite confusing. I'm never clear on anything21:54
mordredcorvus: sure - on it21:54
corvusShrews: at least one of those should be a "znode" :)21:54
Shrewsmordred: i say a rewrite from scratch using C++ is in order21:54
corvusmordred: that commit sha look good to you?  if so i'll push21:54
mordredShrews: ++21:54
mordredcorvus: one sec21:54
corvusShrews: i'm in21:54
mordredShrews: we can use those nice new any variables now too21:55
Shrewsaroo?21:55
mordredcorvus: yes, that sha looks great21:55
corvus3.3.1 pushed21:56
corvusi'll work on the regular release announcement email21:56
mordredShrews: oh - sorry - auto is what I meant21:56
Shrewsoh21:57
mordredShrews: auto mapret = mymap.insert(std::pair('a', 100)); seems so pleasing21:58
corvusmordred: i think one would be typing "auto" a lot.  :)21:59
corvusmaybe even more than "void".21:59
Shrewshttps://www.acodersjourney.com/c-11-auto/22:00
mordredcorvus: yes! but it's better than typing std::map<std::string, std::string> a lot :)22:02
clarkbmordred: I think you got the type wrong (values seem to be ints)22:02
clarkb:P22:02
clarkbgood thing the compiler will figure it out auto(matically)22:03
mordredclarkb: ++22:03
*** j^2 has joined #zuul22:03
mordredcorvus: I think we should un-private the storyboard story, yes? I've got it open so can do that now22:03
corvusmordred: yep, i think we're ready for that now22:03
mordredcorvus: how does this look: https://etherpad.openstack.org/p/lvQYbFXdeI22:07
clarkbmordred: maybe a note about how the bwrap sandbox should have belt and suspendered things?22:08
pabelangerand back22:08
clarkb"We do not believe this would have given jobs access to the executor host as every job is sandboxed within a bubblewrap container with minimal access to the filesystem and other resources"22:08
pabelangeronly 3.3.0 was affected too, maybe we should specifically say that too22:09
pabelangeralso too22:09
corvusmordred: lgtm22:10
mordredpabelanger, clarkb: ++22:10
pabelangerclarkb: re bwrap, things bind mounted into brwap could have been accessed22:11
mordredok. sent22:11
clarkbpabelanger: correct. ca trust chains, /bin /usr/bin/ etc22:11
clarkbpabelanger: its a very minimal set that should be safe to read anyway I think. And jobs wouldn't have had access to secrets without being reviewed first which hopefully would've caught any issues there22:12
pabelangerclarkb: /var/lib/zuul/ssh was what I was thinking22:12
pabelangerwhich is our ssh key for nodepool nodes22:12
clarkbpabelanger: ah22:13
pabelangerso, maybe worth thinking about rotating it22:13
clarkbI thought we used an ssh agent and carefully managed access to that, but maybe that happens in the container?22:13
pabelangeryah, that is inside the container22:13
pabelangerwe still need to add the original key there first22:13
clarkbpabelanger: right but the key could be added from outside the coantiner then the jobs wouldn't have access to the actual key data, just ability to use the key while running22:14
pabelangeryah, I cannot remember why we did a trusted bindmount for it to be honest22:14
pabelangerbut is something we could leak if bwrap is accessed22:14
corvusokay, pypi is updated, docs are published, and i've approved the announcements.  thanks mordred, tristanC, tobiash, clarkb!22:15
clarkbcorvus: and thank you for getting that deployed on the openstack instance22:16
corvusclarkb, pabelanger: what clarkb describes is exactly what zuul does; i don't know why openstack bind-mounts the key in, but we are definitely defeating our own security measures by doing that.22:17
clarkbya I didn't think zuul did that. Are we just configuring it poorly somewhere?22:17
corvusclarkb: yes we configure it explicitly22:17
pabelangermaybe it was before a time we added the ssh-agent22:18
pabelangerand we never removed it22:18
clarkbpabelanger: are you willing to push the change up to fix that?22:18
corvuslet's move to -infra22:18
*** dkehn has joined #zuul22:30
SpamapSHas anybody using GitHub figured out how to restrict write access on a particular branch to just the Zuul github app? IIRC, github doesn't let you do that for bot/app users.23:24
pabelangerSpamapS: I too am also interested in that23:26
pabelangerI've been wanting to try out the CODEOWNERS file, but unsure if that will help23:26
clarkbpabelanger: isn't this the thing we discussed where using two accounts would solve it23:26
clarkbcoincidentally we are talking abouit whether or not we should use two accounts for github over in -infra for different reasons23:27
pabelangerclarkb: no, that is for code review approval, you still need 2 accounts for that23:27
pabelangeras you cannot approve your own PR23:27
pabelangerfor now, I've removed the need for approved code review, and just using labels23:28
clarkbbut if you had an admin account that could push code along with zuul then you'd never try to push from your own account?23:28
clarkbI guess it wouldn't be just zuul. It would be just zuul + the admin account?23:28
pabelangerI am intersted in SpamapS use case, but for me I'm actually wanting to remove admins from repos, or write access.  However, I believe you also need write perms to even use labels23:30
*** rlandy is now known as rlandy|bbl23:33
SpamapSIMO labels are the better experience for self approve on GitHub.23:41
SpamapSpabelanger: yes, you do need write to use labels.23:41
SpamapSWhich is why what I'd really like is to just have branch protection be able to list a bot user.23:41
SpamapSbut IIRC GitHub has said that's not a thing they're doing.23:41
goernSpamapS, cant you manage that via teams? no.. maybe... a team cant restrict on branch, just repo?!23:43
SpamapSgoern: let me look..23:44
SpamapSI don't think you can put an app in a team anyway23:44
goernargh23:45
SpamapScorrect, you cannot23:46
SpamapSSo yeah, the only thing you can currently do, I think, is remove everybody's write access, and make them use things like /label foo to add labels.23:46
SpamapSwhich, IIRC, is what many large scale projects like Kubernetes and Ansible do.23:47
goernja, workflow control via labels is the way to go23:47
pabelangeryah, ansible is big with slash commands in comments23:52
clarkbI remember when we added magical comment comamnds to zuul from gerrit23:52
clarkband we were all "this is a major hack"23:53
clarkbI'm glad this is now the canonical method of doing things in github :)23:53
pabelangerI think we could create a zuul user in github, and not use github apps, but that would mean adding git user and webhook settings manually to each project I think23:55
goernnoooooo :)23:56
*** j^2 has quit IRC23:58

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