Thursday, 2019-02-07

pabelangerSpamapS: mordred: okay, thanks. I'll guess I'll try and figure out how to deal with the python version via ansible (for site-package)00:02
*** dmsimard has quit IRC00:25
*** sdake has quit IRC00:37
*** sdake has joined #zuul00:41
*** sdake has quit IRC00:41
*** dmsimard has joined #zuul00:57
*** sdake has joined #zuul01:12
SpamapSpabelanger: we could probably make the zuul CLI print it out01:20
SpamapSBut as mordred says, maybe best is just to send everything to cherrypy01:21
*** sdake has quit IRC01:48
*** sdake has joined #zuul01:50
*** manjeets has quit IRC02:15
*** manjeets has joined #zuul02:16
*** sdake has quit IRC02:19
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add /{tenant}/buildsets route  https://review.openstack.org/63003503:08
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add buildsets page  https://review.openstack.org/63004103:08
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add /{tenant}/buildset/{uuid} route  https://review.openstack.org/63007803:10
openstackgerritTristan Cacqueray proposed openstack-infra/zuul-jobs master: add-build-sshkey: remove previously authorized build-sshkey  https://review.openstack.org/63262003:12
openstackgerritTristan Cacqueray proposed openstack-infra/nodepool master: Implement zookeeper-auth  https://review.openstack.org/61915503:23
*** rlandy|bbl is now known as rlandy03:43
*** sdake has joined #zuul03:43
*** sdake has quit IRC03:44
*** rlandy has quit IRC04:06
*** sdake has joined #zuul04:10
*** sdake has quit IRC05:18
*** chandankumar has joined #zuul05:45
*** chandankumar is now known as chkumar|ruck05:46
*** swest has joined #zuul06:21
*** swest has quit IRC06:27
*** swest has joined #zuul06:28
*** quiquell|off is now known as quiquell|rover06:36
*** saneax has joined #zuul06:37
quiquell|rovertobiash: thanks, didn't have much time to give youlove to it, just rebase love06:37
quiquell|rovertobiash: if you have some time today maybe you can help me with it06:39
tobiashquiquell|rover: if you want I could fix it later today or this evening06:41
quiquell|roverI will put back .keep06:42
tobiashok06:42
quiquell|roverWhat I didn't figure out is06:42
quiquell|roverHow to use ansible zuul test case06:43
quiquell|roverAnd hold node06:43
quiquell|roverHold build sorry06:43
quiquell|roverY call release in the unit test but build is still there06:43
quiquell|roverI have to find why is that06:44
quiquell|roverMaybe you can give me a clue on that06:44
tobiashquiquell|rover: holding the builds with ansible test cases is not widely used atm06:46
tobiashI noticed that you have to call release for each playbook06:46
tobiashdo you really need to hold the build in the test case?06:46
quiquell|roverI need that to get the inventory06:46
quiquell|roverIf not it's vanished06:46
quiquell|roverWill look into it06:46
tobiashyou can also instruct it to keep the build dir06:47
quiquell|roverHummm06:47
quiquell|roverThat works for me06:47
tobiashlook in test_v3 for that, I thing keep should be used somewhere in there06:47
tobiashor you can just put the change message into a file within a test playbook06:48
*** chkumar|ruck has quit IRC06:48
tobiashand assert within the playbook that the content is correct06:48
*** chandankumar has joined #zuul06:48
tobiashthat'll work too06:48
*** chandankumar is now known as chkumar|ruck06:49
*** quiquell|rover is now known as quique|rover|r--06:53
quique|rover|r--Thanks!! Will look at it06:53
*** gtema has joined #zuul06:54
*** pcaruana has joined #zuul07:02
*** bjackman has joined #zuul07:11
*** quique|rover|r-- is now known as quiquell|rover07:18
*** bjackman has quit IRC07:21
*** bjackman has joined #zuul07:23
*** quiquell|rover is now known as quiquell|rover|b07:33
*** quiquell|rover|b is now known as quique|rover|brb07:33
*** quique|rover|brb is now known as quiquell|rover07:58
*** hashar has joined #zuul08:04
*** gtema has quit IRC08:08
*** themroc has joined #zuul08:17
*** gtema has joined #zuul08:36
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add /{tenant}/buildsets route  https://review.openstack.org/63003508:39
*** jpena|off is now known as jpena08:45
*** zbr|ssbarnea has joined #zuul08:49
*** chkumar|ruck has quit IRC08:53
*** zbr has joined #zuul09:11
*** zbr|ssbarnea has quit IRC09:12
*** saneax has quit IRC09:15
*** panda|off is now known as panda09:17
quiquell|rovertobiash: Got it working thanks !!!09:20
tobiash:)09:20
quiquell|roverLe me also add the poor .keep file09:20
*** saneax has joined #zuul09:22
openstackgerritQuique Llorente proposed openstack-infra/zuul master: Escape jinja2 stuff from inventory  https://review.openstack.org/63393009:22
openstackgerritQuique Llorente proposed openstack-infra/zuul master: Escape jinja2 stuff from inventory  https://review.openstack.org/63393009:22
quiquell|rovertobiash: ^ let's see now09:23
quiquell|rovertobiash: do I have to add more test to that ?09:23
quiquell|rovermaybe trying to parse inventory with ansible09:23
*** saneax has quit IRC09:28
*** saneax has joined #zuul09:29
*** bjackman_ has joined #zuul09:31
*** bjackman has quit IRC09:33
*** chandankumar has joined #zuul09:34
tobiashquiquell|rover: commented09:35
*** chandankumar is now known as chkumar|ruck09:35
quiquell|rovertobiash: thanks ! let's do it09:36
*** zbr has quit IRC09:40
*** zbr|ssbarnea has joined #zuul09:40
*** chandankumar has joined #zuul09:50
*** bjackman_ has quit IRC09:51
*** chkumar|ruck has quit IRC09:51
*** chandankumar is now known as chkumar|ruck09:51
*** bjackman_ has joined #zuul09:57
*** zbr|ssbarnea has quit IRC10:04
*** zbr|ssbarnea has joined #zuul10:07
openstackgerritQuique Llorente proposed openstack-infra/zuul master: Escape jinja2 stuff from inventory  https://review.openstack.org/63393010:23
*** bjackman_ has quit IRC10:29
*** bjackman_ has joined #zuul10:29
openstackgerritFabien Boucher proposed openstack-infra/zuul master: [WIP] - URLTrigger based on the timer trigger  https://review.openstack.org/63524110:57
*** fdegir has quit IRC10:57
*** fdegir has joined #zuul10:58
*** sshnaidm|afk is now known as sshnaidm11:17
*** chkumar|ruck has quit IRC11:29
*** chandankumar has joined #zuul11:30
*** chandankumar is now known as chkumar|ruck11:30
*** rfolco is now known as rfolco_doctor11:38
*** bjackman__ has joined #zuul11:39
*** bjackman_ has quit IRC11:41
openstackgerritQuique Llorente proposed openstack-infra/zuul master: Escape jinja2 stuff from inventory  https://review.openstack.org/63393011:47
quiquell|rovertobiash: ^ now is all covered I think11:47
quiquell|rovertobiash: also the ansible expansion11:47
*** electrofelix has joined #zuul11:58
lennybHi, I am running zuul2 and I see a lot of my jobs in 'queued' state. I have enough free nodes in Jenkins, so I don`t understand why job is not triggered. No errors in logs.11:59
tobiashlennyb: maybe you need to restart jenkins12:00
tobiashlennyb: that solved many problems ages ago ;)12:00
tobiashat least I had often similar problems back then and a jenkins restart solved these most of the time12:01
*** electrofelix has quit IRC12:04
*** swest has quit IRC12:04
*** electrofelix has joined #zuul12:05
*** hashar has quit IRC12:10
tobiashmordred: looking at my graphs it looks like that 634598 was good because of clean code but didn't solve the memleak on the executors :(12:13
lennybtobiash: thanks, but it did not help.12:15
tobiashlennyb: hrm, maybe zuul lost its connection to jenkins12:15
tobiashlennyb: or maybe gearman is stuck12:15
lennybgearman plugin passed test. it happens a lot, restarting zuul 'fixes' the issue, but I am loosing a lot of commits.12:16
mordredtobiash: :(12:16
*** swest has joined #zuul12:20
tobiashmordred: that's the rss of our executor containers during the last 6 months: https://paste.pics/7a57a7ae4a3973198b41e4db5005724a12:20
tobiashmordred: at one point in time it started12:20
tobiashmordred: we switched from alpine to ubuntu at this time12:20
tobiashmordred: so maybe a memleak in python itself or a different java-like memory management in glibc based python12:21
mordredtobiash: so the leak correlates with the switch from alpine?12:21
tobiashyes12:21
tobiashexactly with the date when we rolled it out12:22
mordredyou are using the python shipped by ubuntu in your containers, yes?12:22
tobiashyes12:22
mordredand before with alpine were you using the python:alpine images? or normal alpine with the alpine provided python?12:22
tobiashso could be either alpine vs ubuntu or py37 vs py36 as alpine was at 3.7 at that time afaik12:22
tobiashit was normal alpine12:23
mordredyeah. there's several things possible there12:23
tobiashmaybe I should try py37 from universe to rule that out12:23
mordredcould be patches to python applied by ubuntu, or libc differences, or python versions12:23
mordredyeah - I know that 3.6 introduced a new dict impl12:23
mordredso it's possible 3.7 contains fixes for it that are important for our heavy dict usage12:23
tobiashpossibly12:24
mordredit'll be interesting to see if we see the same behavior when we start deploying from the python:slim debian-based containers with 3.712:24
mordredtobiash: it also seems like your immediate baseline memory is higher even before it starts leaking too12:25
mordredbut maybe that's just musl vs glibc12:25
mordredwe're still running on 3.5 on xenial at the moment12:26
tobiashah12:27
mordredhttp://cacti.openstack.org/cacti/graph.php?action=view&local_graph_id=64003&rra_id=all <-- there's memory graphs for one of our executors - doesn't look like we're seeing the same sorts of increases you are12:28
tobiashyes, that looks different12:29
mordredpython version and libc version are the primary differences that I'd expect to be relevant12:30
tobiashbut at least we're updating zuul faster than the executors could oom ;)12:30
tobiashyes12:30
tobiashso think I'll try py37 first12:31
mordred++ - that'll be a good data point12:31
tobiashit's available as a package in universe so should be relatively easy to use that12:31
*** bjackman__ has quit IRC12:32
SpamapStobiash: you should also consider trying jemalloc12:33
SpamapShttps://zapier.com/engineering/celery-python-jemalloc/12:34
mordredSpamapS: you are awake at an absurdly early hour12:35
SpamapSmordred: tell me about it :-P12:35
mordredSpamapS: I have very recent memories of being awake at the same hour in my timezone12:35
SpamapSGot my 4 hours in :-P12:35
mordredme too!12:36
tobiashSpamapS: thanks, reading12:36
*** bjackman has joined #zuul12:38
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Perform per repo locking on the executor  https://review.openstack.org/63549512:40
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Optionally parallelize update threads  https://review.openstack.org/63549612:40
*** jpena is now known as jpena|lunch12:44
* SpamapS may try to snag another hour before the sun comes up12:44
*** sdake has joined #zuul12:46
*** sdake has quit IRC12:51
*** sdake has joined #zuul12:52
*** sshnaidm is now known as sshnaidm|afk12:52
*** rlandy has joined #zuul13:04
*** sshnaidm|afk is now known as sshnaidm13:11
openstackgerritMonty Taylor proposed openstack-infra/zuul master: WIP Run python with jemalloc in containers  https://review.openstack.org/63550413:14
tobiashmordred: lol, just thought about doing the same thing ^13:14
mordredtobiash, SpamapS: ^^ there's a patch inspired by that blogpost - completely untested, and might be a terrible idea13:14
mordredtobiash: :)13:14
tobiashmordred: the packaged versions of jemalloc are very old as it seems13:15
tobiashboth on bionic and stretch13:16
tobiash3.6.0 is from early 2015 :-/13:16
tobiashso you might want to build it from source ;)13:16
mordredyah - although in that post even 3.5 seemed to have a significant impact on his swap usage13:17
tobiashyes13:17
mordredtobiash: totally. if it winds up being meaningful for zuul, I think building it from source in a builder image wouldn't be terrible13:17
tobiash++13:17
tobiashthe question is who wants to try this in production ;)13:18
mordredwhat could possibly go wrong? :)13:18
tobiashmordred: but I had the same ENV trick in mind, so that would also affect/improve ansible memory usage13:19
tobiashI think I'll try that after the py37 experiment13:19
pabelangerSpamapS: Yah, in fact, that is what I am doing today. So, likely okay just having cherrypy serve it, and deal with it if / when becomes an issue13:34
*** sdake has quit IRC13:47
*** jpena|lunch is now known as jpena13:48
*** rfolco_doctor is now known as rfolco13:58
openstackgerritQuique Llorente proposed openstack-infra/zuul master: Escape jinja2 stuff from inventory  https://review.openstack.org/63393013:59
quiquell|rovertobiash: ^ all good now13:59
tobiashquiquell|rover: thanks, will look later13:59
quiquell|rovercool thanks13:59
*** quiquell|rover is now known as quique|rover|eat14:06
*** bjackman has quit IRC14:23
*** irclogbot_3 has joined #zuul14:31
*** bjackman has joined #zuul14:33
*** gtema has quit IRC14:35
*** sdake has joined #zuul14:35
*** quique|rover|eat is now known as quiquell|rover14:38
*** bjackman has quit IRC14:42
*** bjackman has joined #zuul14:42
*** bjackman has quit IRC14:48
*** sdake has quit IRC14:54
*** ParsectiX has joined #zuul14:59
*** sdake has joined #zuul15:09
tobiashmordred: according to my graphs the scheduler seems to be leaky too15:15
tobiashor it's really just java-like behavior...15:16
*** quiquell|rover is now known as quiquell|off15:27
*** gtema has joined #zuul15:33
*** zbr has joined #zuul15:35
*** swest has quit IRC15:37
*** zbr|ssbarnea has quit IRC15:37
tobiashmordred: -rwxr-xr-x. 1 root root 4.3M Feb  7 15:42 /usr/local/lib/libjemalloc.so.215:56
tobiash4.3M for a malloc implementation15:56
tobiashimpressive15:56
tobiashmordred: so now I've restarted one executor as reference, updated one to py37 and one to py37+jemalloc15:58
tobiashlooking forward to the graphs tomorrow...15:58
clarkbtobiash: with the scheduler I think it is more that as it expands to load configs python can never really release that memory again15:59
tobiashhrm, that graph is continuously rising during the day and rising again the next day16:01
tobiashjust like it wouldn't reuse that memory16:01
clarkbtobiash: http://cacti.openstack.org/cacti/graph.php?action=view&local_graph_id=64792&rra_id=all that one?16:03
clarkbit looks pretty stable16:03
openstackgerritFabien Boucher proposed openstack-infra/zuul master: Propose the URL driver to implement an URL change Require Filter  https://review.openstack.org/63555416:03
tobiashyeah, maybe it stabilizes once it doesn't get more memory ;)16:03
tobiashhm, ok my executor has a limit of 4gb currently16:05
tobiashmaybe it would stabilize around that level16:05
*** pcaruana has quit IRC16:21
*** themroc has quit IRC16:22
*** chkumar|ruck has quit IRC16:38
*** chandankumar has joined #zuul16:39
*** saneax has quit IRC16:40
dmsimardIs the zuul inventory file available during the runtime of a job ?16:42
dmsimardI know the Ansible vars are available, I mean the literal file :)16:42
jktso, I'm playing with a pre-run playbook to basically call `git submodule update --init --recursive`, after fixing the repo URLs etc16:43
jktI understand that I have to add all dependant projects into tenant config and job's required_project stanza16:43
dmsimardAh, got it: {{ zuul.executor.log_root }}/zuul-info/inventory.yaml16:44
jktmy problem is that even that initial step where zuul-executor prepares these git repos takes 2.5 minutes on this VM which is allegedly already backed by SSDs16:45
clarkbdmsimard: ya a privileged play copies it to that location16:45
clarkbdmsimard: so the running job doesn't read that exact file (eg don't expect updates to it to reflect in the ansible runes)16:45
openstackgerritFabien Boucher proposed openstack-infra/zuul master: URLTrigger driver time based  https://review.openstack.org/63556716:48
corvusdmsimard: https://zuul-ci.org/docs/zuul/user/jobs.html#var-zuul.executor.inventory_file16:50
corvusjkt: i think we've talked about this, but just in case -- is your executor's job_dir on the same filesystem as it's git_dir?  https://zuul-ci.org/docs/zuul/admin/components.html#attr-executor.job_dir  and  https://zuul-ci.org/docs/zuul/admin/components.html#attr-executor.git_dir16:53
jktcorvus: we talked about this, but I don't remember that particular point17:03
jktcorvus: and yup, same fs17:04
jktdoh, nope17:04
tobiashjkt: not same fs is a real performance killer ;)17:05
jktok, actually, yes, this is all rootfs, so no tmpfs for /tmp17:05
tobiashjkt: also same mount point (if you're runnning containerized)?17:05
jkttobiash: I am not using containers; this is on centos 7, with an xfs / and nothing mounted at /tmp17:06
tobiashok17:06
jktso both /var/lib/zuul/executor-git and /tmp should be rootfs17:06
tobiashjkt: maybe this could help: https://review.openstack.org/63549617:06
jkttobiash: have you noticed that you guys come up with a solution to any of my problems just hours before I need one? :)17:07
tobiash:)17:07
*** sdake has quit IRC17:07
tobiashjkt: how many repos does your job have?17:07
jktabout a zillion, well, actually, ~15017:08
jkt:(17:08
tobiashthat change parallelizes the initial git fetch of all repos of the job17:08
tobiashok, so then I'd expect quite some speedup in your case17:08
jktthe biggest offender are the Boost C++ libraries17:09
jkttheir build system hasn't liked my attempts at pruning the dep tree *at all*17:09
corvustobiash: left a question on that about whether we can eliminate the config option :)17:09
jktalso, their submodules point to ../relative/whatever.git instead of just ../relative/whatever17:10
tobiashcorvus: fine for me, I just wanted to retain status quo with the config option17:10
*** sdake has joined #zuul17:10
jktthis is what I have now, http://paste.openstack.org/show/744695/17:11
tobiashI guess we could start with 1*cpus?17:11
corvustobiash: ah, yeah, i expect that it would be okay to improve things by default for people.  that sounds good.17:11
*** pwhalen has quit IRC17:12
tobiashcorvus: I just need to think about how to get the correct number of cpus in my cgroups restricted depoyment17:13
tobiashmultiprocessing will return 16, but the executor is restricted to 817:13
jktso, the numbers: 1min 21s for updating all repos in /var/lib/zuul/executor-git/ , which should be a no-op in this case because there are no changes17:13
tobiashbut I have that same problem with the starting builds sensor anyway17:14
jkt30s for cloning to /tmp17:14
tobiashjkt: the executor doesn't know upfront if it's a noop, but I guess the parallelization could make this 1min 21s much smaller17:14
jkt10s for checking out the respective branches in there17:14
tobiashso yes, that sounds like the parallelization will help there quite a lot17:15
jktsounds like I should build from git once again :)17:16
jktdoes the REST API of zuul-web need that JS Build stack?17:16
jktor is that just for the dashboard web app?17:16
tobiashzuul-web serves the dashboard webapp17:17
tobiashor do you offload it to apache?17:18
jktI don't offload that, nope17:18
jktI'm wondering about the /api/tenant/.../ etc17:18
tobiashthe api works without, but is quite useless without the ui17:19
jktah, I would probably lose that console streamer as well, then17:21
tobiashyes17:22
tobiashand the status page17:22
*** pwhalen has joined #zuul17:23
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Optionally parallelize update threads  https://review.openstack.org/63549617:29
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Optionally parallelize update threads  https://review.openstack.org/63549617:30
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Parallelize update threads  https://review.openstack.org/63549617:31
openstackgerritMohammed Naser proposed openstack-infra/nodepool master: docker: don't daemonize when starting images  https://review.openstack.org/63558417:39
*** jpena is now known as jpena|off17:59
*** jesusaur has quit IRC18:07
*** gtema has quit IRC18:11
tobiashcorvus, SpamapS: do we want to target the multi ansible spec for next week? I think I've addressed all comments so far.18:19
SpamapStobiash: Where's that spec? It doesn't seem to have topic:multi-ansible18:21
SpamapSor did it merge?18:21
tobiashSpamapS: https://review.openstack.org/62392718:22
tobiashI'll set the topic18:22
SpamapSthanks, just couldn't find it ;)18:23
tobiash:)18:23
corvustobiash, SpamapS: to be clear, i think the utility of having the executor run the install script on startup isn't for quick-start -- i agree, that should be at cointainer build-time.  it's for the more traditional pip-install configuration.  so that someone who does 'pip instal zuul' has it almost as easy as someone who does 'docker run zuul'18:24
tobiashcorvus: I can clarify that in the spec18:25
corvusi mention it here because i think the three of us have been going back and forth on the spec :)18:26
tobiashok :)18:26
SpamapSI see.18:26
corvusbasically it makes "pip install zuul; zuul-executor" continue to work18:27
SpamapSA fair point, and a fine bridge for folks who aren't ready to containerize stuff.18:27
corvusi think we still get new users not using containers.  though we've gotten a lot of quick-start users showing up.18:29
SpamapSYeah, I see why it's a thing and I don't mind keeping that working for a while.18:30
corvusi could see dropping auto-install as a teaching method -- to make sure that people know that's what's going on.  but it feels like maybe that may not be worth the additional friction for new users.18:32
electrofelixhughsaunders: are you still working on nodepool-agents plugin for jenkinsci to make use of nodepool? wondering if you're around to chat about some questions I have about making use of it (also asked in https://groups.google.com/d/msg/jenkinsci-dev/VA2i-_L350E/uVhO2xRLGQAJ)18:34
tobiashand probably even existing users18:35
SpamapShonestly18:39
SpamapSthis feels almost major version-ish18:39
SpamapSIt's more than a feature18:39
corvusno argument here18:40
corvusif folks think multi-ansible is v4, and zookeeper-everywhere is v5, that wfm.  :)18:40
corvustobiash: so yeah, i'll issue a last call for review on the spec monday with the aim to merge it by the end of the week if there are no major revisions required.  sound good?18:42
clarkbthe problem is wheels not running installation code right? so we somehow have to install portable virtualenvs (or something like that) into the wheel or have the wheel install (or maybe process start up) do the setup?18:42
tobiashcorvus: sounds good :)18:42
clarkbI guess the easiest things is having the executor do it on first run18:43
clarkband it can noop if it is already done for it18:43
*** ParsectiX has quit IRC18:44
tobiashclarkb: yes, it's similarly described in the spec18:46
clarkbyup, just making sure I understand. Is the contention question around whether or not zuul should do the installation on startup? I don't really think there is a straightforward method that avoids that (other than requiring a multistep install process)18:47
*** jesusaur has joined #zuul18:52
*** sdake has quit IRC18:58
tobiashclarkb: we'll support both, manual installation (by calling something like zuul-manage-ansible) and on startup installation (for the lazy pip install use case)19:01
*** remi_ness has joined #zuul19:04
*** ParsectiX has joined #zuul19:28
SpamapSugh, looks like my pbrx docker builds are failing now because of something that changed in alpine19:29
openstackgerritMerged openstack-infra/zuul master: Update git connection logging  https://review.openstack.org/63520419:29
* SpamapS accelerates switch to dockerfile builds19:29
*** saneax has joined #zuul19:38
*** jesusaur has quit IRC19:40
*** jesusaur has joined #zuul19:41
*** sdake has joined #zuul19:47
*** sdake has quit IRC19:49
*** pwhalen has quit IRC19:50
*** sdake has joined #zuul19:51
openstackgerritMohammed Naser proposed openstack-infra/zuul master: docker: add state folder  https://review.openstack.org/63561819:52
*** sdake has quit IRC19:53
*** sdake has joined #zuul19:54
openstackgerritMohammed Naser proposed openstack-infra/zuul master: docker: start process in foreground  https://review.openstack.org/63561919:55
mordredmnaser: ^^ lgtm ... for the state folder - should we mark that as a volume?19:58
mnasermordred: it looks like most of what goes in the state folder is command control socket, pid files and things that aren't really as much of state19:59
tobiashmordred: I'm just asking myself how I could overlook the foreground thingy ^19:59
mnaserso i think it should live within the container (imho)19:59
mordredtobiash: yah - same here19:59
mordredmnaser: kk19:59
tobiashmordred: how did the quickstart then work? I'd expected if zuul daemonizes pid 1 exits and that should exit the container...20:00
mordredtobiash: maybe because we're using dumb-init it still works?20:00
tobiashah, that's the reason20:00
tobiashdumb-init is pid120:00
mordredmnaser: actually - at least on the scheduler, /var/lib/zuul contains the keys, which are precious and shoudl go on a volume20:01
mnasertobiash, mordred quick start works because someone explicitly dropped the -d in the command in docker-compose20:01
mnasermordred: ah that's true20:01
mnaseri mean tbh i'd feel that /var/lib/zuul/.ssh is the real thing to use in this case20:02
mordredmnaser: no - I mean the per-project encryption keys20:02
mnaseroh20:02
mnaseri was thinking ssh keys20:02
mordredmnaser: there's /var/lib/zuul/keys which have those20:02
mnaserin that case volume makes a lot of sense.. assuming 'volume' does a mkdir20:03
tobiashmnaser: then you could just completely drop the command in the docker-compose for zuul-web and executor20:03
mordredmnaser: I believe it does - I think it makes the mount point20:03
mordredtobiash: ++20:03
mnaserok well i can revise the patch and update the quickstart20:03
mordredcool20:03
tobiashmnaser: yes, volume creates that dir20:03
tobiashmnaser: thanks20:04
mordredmnaser: yeah - thanks - great improvements20:04
SpamapSthank god for dumb-init eh? ;-)20:04
tobiashI'm using tini20:05
tobiashbut that's basically doing the same20:05
openstackgerritMohammed Naser proposed openstack-infra/zuul master: docker: start process in foreground  https://review.openstack.org/63561920:07
openstackgerritMohammed Naser proposed openstack-infra/zuul master: docker: add state folder  https://review.openstack.org/63561820:07
mnasermordred, tobiash ^ added depends-on on the nodepool change too20:07
mordredmnaser: ++20:07
*** remi_ness has quit IRC20:07
tobiashmnaser: btw, depends-on with changeid is deprecated ;)20:11
mnasertobiash: i know but i'm old school and lame20:11
mnaser:)20:11
mnaseranother fun one20:13
mnaserzuul-fingergw tries to drop perms into user 'zuul'20:13
tobiash... which doesn't exist...20:13
mnaserhttp://paste.openstack.org/show/744710/20:14
tobiashmnaser: I think that should be made optional. e.g. when running in openshift fingergw would run on an unprovileged port with perms already dropped20:15
clarkbshouldn't the zuul user exist in the imges though?20:15
corvusi added the -d in the compose file, because the quick-start was made when we built pbrx containers which didn't use -d20:16
corvusthe zuul user does not exist in the images20:16
corvusit runs as "root"20:16
clarkbshoul we create the zuul user?20:17
tobiashI don't think so20:17
mnaserbut the root isn't a real root i guess inside a container20:17
clarkbmnaser: depends on whether or not you namespace users but ya20:17
*** saneax has quit IRC20:17
tobiashit just makes things mode complicated if running e.g. in openshift20:17
corvusmordred has discussed possibly creating a zuul user.  regardless of that, the immediate issue is that we have an overly aggressive default20:17
corvusi feel like i just spelled out how to fix this20:18
corvuslet me dig up irc logs20:18
corvushttp://eavesdrop.openstack.org/irclogs/%23zuul/%23zuul.2019-02-01.log.html#t2019-02-01T19:50:0920:20
corvusfrom 19:50 through 20:0020:20
clarkbthe problem is that if you assume unprivileged ports then you need a proxy of some sort. If you don't assume privileged ports then you have to do something like the code currently does. Maybe key off the port value? also I should read ^20:20
corvusi don't see any of the described patches uploaded :(20:21
corvusdkehn: were you planning on pushing up the change to run fingergw in quick-start?20:21
clarkbah yup I think the described fix would work20:22
pabelangertobiash: what issues would use see with zuul user in openshift container?20:22
dkehncorvus: once I get it totally working20:23
tobiashpabelanger: openshift by default starts the process with a random uid and gid=0 which has no relationship to the passwd in the container. Further you cannot switch the user in this case.20:23
tobiashso if possible we should not make any assumptions on the users. If we can manage this the image will be generic enough to run anywhere20:24
clarkbdo we also need to override the port for running in a container or will we assume privelege within the container network namespace for the current user?20:24
clarkbthat assumption is likely safe given how many people use docker20:24
tobiashgranted the executor needs privileges, but the others don't and should imho also work with default restrictions20:24
tobiashin openshift you need to run fingergw on an unprivileged port like 1079 and have the service dispatch from 79 to 107920:25
pabelangertobiash: ah, good to know. When I was trying to run docker nodepool from ansible, I ended up crafting the following systemd execstart: http://paste.openstack.org/show/744712/ passing -u flag, because docker image (pbxr) was root.  This was the only way I could get volumes to properly work, as they had permissions of nodepool:nodepool (1001). Since then, I haven't really looped back to trying to run docker under20:26
pabelangersystemd20:26
pabelangerI always thought, if user inside container was nodepool, I could drop the -u flag20:26
pabelanger(but likely wrong)20:27
*** sshnaidm is now known as sshnaidm|off20:29
tobiashwhen working in openshift folders that need to be written by zuul need to be either mounted as a volune or chowned <foo>:0 and g+w if it's inside the container rootfs (a process has uid=random and gid=0 by default)20:29
tobiashftr, more info on that topic is here: https://docs.okd.io/latest/creating_images/guidelines.html#openshift-specific-guidelines20:31
*** sdake has quit IRC20:31
tobiashmost important the chapter "Support Arbitrary User IDs"20:31
*** sdake has joined #zuul20:32
pabelangertobiash: yah, I don't believe I setup any specific docker volumes via docker, was only trying to bindmount directly to filesystem20:33
tobiashin the end a docker volume also just gets bind mounted into the filesystem20:35
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Remove default user for fingergw  https://review.openstack.org/63563220:37
openstackgerritJames E. Blair proposed openstack-infra/zuul master: Remove default zookeeper hosts  https://review.openstack.org/63563320:37
corvusi think those should be in 3.7, so i set a WIP on the first, but we can go aheand and review/discuss them20:38
mnaserif you go with the github application route, how do you 'add' the ssh key to clone things (or does it clone using the api token or something?)20:41
tobiashmnaser: zuul doesn't use ssh in that case20:42
mnasertobiash: ok so that option should technically be ignored when not using a webhook (perhaps i should push up a doc change)?20:43
tobiashmnaser: it gets per installation tokens on demand that expire every hour20:43
mnaserhttps://zuul-ci.org/docs/zuul/admin/drivers/github.html#attr-<github%20connection>.sshkey20:43
mnaserthis one that is20:43
tobiashI wonder why there is even a ssh key20:44
tobiashbecause even with the api token method it should use that and https20:44
mnasertobiash: the web hook thing says you need a user with the ssh key20:44
mnaserhttps://zuul-ci.org/docs/zuul/admin/drivers/github.html#web-hook20:44
tobiashhave to re-read the source20:44
pabelangeris that the ssh key you define, if not using github app?20:44
tobiashyes20:45
mnaserso the docs should probably clarify that20:45
mnaseri guess20:45
tobiashmnaser: so rereading the code, yes when using api token you need the ssh key20:46
tobiashmnaser: but more important, when using app key a defined ssh key will break you20:46
openstackgerritMohammed Naser proposed openstack-infra/zuul master: doc: clarify sshkey option usage in github connection  https://review.openstack.org/63564020:47
mnasertobiash: ^20:47
mnaseralso mordred mind +w https://review.openstack.org/#/c/635584/ to unblock similar zuul change (i added a depends-on)20:48
tobiashmnaser: https://git.zuul-ci.org/cgit/zuul/tree/zuul/driver/github/githubconnection.py#n96920:48
mnaseryep thats what i noticed too20:48
tobiashif you declare an ssh key, it is taken regardless of the auth method20:48
tobiashand since you cannot attach an ssh key to an app this will break zuul20:48
corvusmnaser, tobiash: if you're embarking on the '-d' route, remember that has implications for logging20:49
corvusmordred: ^20:49
tobiashwe probably should document/verify conflicting settings in the github driver20:49
tobiashcorvus: which implications do you mean? I think I forgot them20:50
corvusmnaser, tobiash, mordred: tell me you've thought through the logging issue; otherwise maybe we ought to give those changes a little more thoughtL20:50
mnaserare we talking about docker implications or zuul implications20:51
corvusboth20:51
tobiashcorvus: do you mean the no-logging-on-invalid-log-config-problem?20:51
corvustobiash: i mean the fact that "-d" doesn't just mean run-in-foreground20:52
corvusthe "d" in "-d" stands for "debug"20:52
mnasero20:52
corvusso that change is actually "run zuul in full debug mode, in the foreground"20:52
mnaserokay, that's a different story, i was under the impression it was foreground20:52
corvusit's both, because zuul was written before containers existed, and this option is as old as zuul :)20:53
tobiashhrm, we're running zuul in -d mode since the beginning :)20:53
tobiashbut we're still running it in debug logging mode...20:53
mnaserhttps://github.com/openstack-infra/zuul/blob/485f1205a358c4c2297967ca70454d923b8c7b04/zuul/cmd/__init__.py#L141-L14520:53
mnaserL145 is where we go debug20:54
corvusi'm generally in favor of the change, but i'd like *someone* to assure me they've worked through the implications :)20:54
mnaseri feel like the right thing™ is to add a -f option20:54
mnaserhaving said that, i don't think i have time to do the small rewrite to add -f and make -d do -f + debug20:56
mnaserthough it seem relatively trivial20:56
pabelangerI run -d with log_config setup in zuul.conf, and belive logging works correctly20:56
tobiashah, we're running with log config: https://github.com/openstack-infra/zuul/blob/485f1205a358c4c2297967ca70454d923b8c7b04/zuul/cmd/__init__.py#L13720:56
tobiashso we need to think about what happens with no log config and -d20:57
mnaseryou get debug logging to stdout (that's what i have right now)20:57
tobiashso judging from the code the only difference I see is debug vs non-debug logging20:58
tobiashI think I would have noticed other problems if there were any21:00
tobiashI know that e.g. signal handling and command socket work with -d21:01
pabelangerit also worked well under systemd, been using it for a while in local testing21:01
tobiashcorvus: so the question is, is it ok to have debug logging in the container if no log config is configured?21:01
corvustobiash: i think we'd probably want the normal (info i think) logging -- same as you'd get running a daemon with no logging config21:03
corvusso i think that means mnaser's suggestion of splitting the arg into f and d may be the best path21:03
tobiashok, I'll push up a change that adds -f with info and leaves -d as it is (except the description)21:03
corvusthen Dockerfile should use the foreground option only21:07
tobiashkk21:17
tobiashcorvus: do we want to retain debug logging in the quick start?21:20
tobiashor switch this to info too?21:20
corvustobiash: i could go either way, but maybe let's try info.21:20
corvusif we get a lot of people showing up with questions and we say "enable debug logging" maybe we switch :)21:21
tobiash... or improve logging21:21
corvuseven better21:21
tobiashso I think it's even beneficial to use info. Then we get a better feeling if info is sufficient for finding problems.21:22
openstackgerritTobias Henkel proposed openstack-infra/zuul master: Add foreground option  https://review.openstack.org/63564921:25
openstackgerritMatthieu Huin proposed openstack-infra/zuul master: [WIP] web: add tenant and project scoped, JWT-protected actions  https://review.openstack.org/57690721:26
corvustobiash: i think the same situation applies in nodepool21:28
tobiashcorvus: already on it21:28
corvuscool21:28
corvusi wiped 63558421:29
*** sdake has quit IRC21:29
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: docker: don't daemonize when starting images  https://review.openstack.org/63558421:33
clarkbtobiash: my only concern was that side effecting self.args like that might not be safe, but we already use that method of setting nodaemon elsewhere so should be fine. +221:34
tobiashclarkb: I had the same thoughts but this way was the way of least impact ;)21:35
clarkbtobiash: https://review.openstack.org/#/c/635584/2 comment on that though21:36
openstackgerritTobias Henkel proposed openstack-infra/nodepool master: docker: don't daemonize when starting images  https://review.openstack.org/63558421:36
*** sdake has joined #zuul21:36
tobiashclarkb: updated21:37
clarkb+2 thanks21:37
tobiashclarkb, corvus: not urgent, but it would be great if you could add 616306 (resource usage stats) to your review queue. I think that would be useful for the openstack deployment too21:39
clarkbya I'll add it to my todo list21:40
tobiashthanks :)21:46
*** sdake has quit IRC21:51
*** sdake has joined #zuul21:55
*** sdake has quit IRC21:56
*** sdake has joined #zuul21:57
openstackgerritMohammed Naser proposed openstack-infra/zuul master: doc: fix sqlalchemy database url docs path  https://review.openstack.org/63567022:17
*** sdake has quit IRC22:34
*** panda is now known as panda|off22:59
*** sdake has joined #zuul23:23
*** ParsectiX has quit IRC23:48
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add buildsets page  https://review.openstack.org/63004123:52
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add buildset page  https://review.openstack.org/63007923:53
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add /{tenant}/buildset/{uuid} route  https://review.openstack.org/63007823:53
openstackgerritTristan Cacqueray proposed openstack-infra/zuul master: web: add buildset page  https://review.openstack.org/63007923:53

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