Tuesday, 2019-04-30

*** mattw4 has quit IRC00:10
SpamapSmmmm I just got an idea to have a zuul pipeline that approves github PR's so you can self-approve. The password that you need to add in a comment?  "I solemnly swear that I am up to no good."01:13
*** hashar has joined #zuul01:34
pabelangerI'm pretty sure, I'm going to end up updating out check pipeline, so zuul can leave a +A on code review, then use branch protection to setup up min number of approved required.01:35
pabelangerjust to work around 'you cannot approve own PR'01:35
pabelangerbut, I'd like to some how update zuul to first check branch protection settings, before enqueing into gate, otherwise we'll just get a merge failure and unsure the reason for why the failure is exposed to user01:36
*** hashar has quit IRC02:06
*** raukadah is now known as chandankumar03:11
*** bhavikdbavishi has joined #zuul03:23
*** sshnaidm|ptg has joined #zuul03:25
*** bhavikdbavishi1 has joined #zuul03:39
*** bhavikdbavishi has quit IRC03:40
*** bhavikdbavishi1 is now known as bhavikdbavishi03:40
tobiashpabelanger: you'll be interested in https://review.opendev.org/#/c/64455703:51
*** bhavikdbavishi has quit IRC04:23
*** bhavikdbavishi has joined #zuul04:27
SpamapStobiash:yeah that makes sense.04:55
SpamapSpabelanger: the user is shown a very big red X saying "This can't merge without reviews". Even if they couldn't merge it anyway, they will see that.04:56
SpamapSpabelanger:and zuul already has a way to ignore unprotected branches... maybe that's enough?04:58
SpamapSanyway, Zuul can't +A. It can label, but it can't review. I'm looking at adding that right now.04:58
*** rfolco|ruck has quit IRC05:03
*** rfolco|ruck has joined #zuul05:04
*** sshnaidm|ptg has quit IRC05:05
*** bhavikdbavishi1 has joined #zuul05:10
*** bhavikdbavishi has quit IRC05:10
*** bhavikdbavishi has joined #zuul05:13
*** bhavikdbavishi1 has quit IRC05:14
*** rfolco|ruck has quit IRC05:17
*** rfolco|ruck has joined #zuul05:17
badboydoes zuul support draft-created gerrit event?05:56
*** rfolco|ruck has quit IRC05:58
*** quiquell|off is now known as quiquell06:04
*** rfolco has joined #zuul06:06
*** gtema has joined #zuul06:28
*** gtema has quit IRC06:40
*** threestrands has joined #zuul06:46
*** themroc has joined #zuul06:50
*** bjackman has joined #zuul07:05
*** bhavikdbavishi has quit IRC07:12
*** bhavikdbavishi has joined #zuul07:13
*** jpena|off is now known as jpena07:39
*** pcaruana has joined #zuul07:40
*** rfolco has quit IRC07:52
*** rfolco has joined #zuul07:52
*** threestrands has quit IRC07:55
*** bjackman_ has joined #zuul08:02
*** bjackman has quit IRC08:05
*** bhavikdbavishi has quit IRC08:12
*** bhavikdbavishi has joined #zuul08:26
openstackgerritFabien Boucher proposed zuul/zuul master: Pagure driver - https://pagure.io/pagure/  https://review.opendev.org/60440408:46
*** bhavikdbavishi has quit IRC09:01
*** bhavikdbavishi has joined #zuul09:07
*** electrofelix has joined #zuul09:11
*** swest has quit IRC09:17
*** swest has joined #zuul09:18
*** bhavikdbavishi has quit IRC10:00
*** bhavikdbavishi has joined #zuul10:07
*** bhavikdbavishi has quit IRC10:11
tobiashShrews: re nodepool memleak: I just tested the folloing scenario: enqueue 50 jobs at once -> 50 new vms, wait for a minute, then dequeue those jobs -> nodepool deletes all at once10:23
tobiashhttps://paste.pics/b93939da6b2445fa933106a83fec5be810:32
tobiashShrews: what is interesting is that the leak seems to be somewhere during instance *deletion*10:32
electrofelixWas the work for extracting pep8 messages for inline comments ever completed elsewhere? see https://opendev.org/zuul/zuul-jobs/commit/b35f47190a0674e44e4b5d38b8062fc717fb4cf0 for a revert from 6 months ago10:36
*** bhavikdbavishi has joined #zuul10:56
*** jpena is now known as jpena|lunch10:59
*** gtema has joined #zuul11:00
*** bhavikdbavishi has quit IRC11:00
AJaegerelectrofelix: no, it was not11:01
AJaegerelectrofelix: at least I don't remember it beeing completed11:01
electrofelixAJaeger: thanks11:21
electrofelixtobiash: can you fill me in on what was wrong with the above change? as the log referenced in the commit message no longer exists11:21
tobiashelectrofelix: ugh, I don't know anymore, I think that was some kind of emergency revert because that broke every pep8 job (I didn't author this functionality)11:23
electrofelixso just generally broke everything, trying to put together a local demo so I might look to add it locally and fix up whatever was the breakage to show off the review comment behaviour11:25
*** bjackman_ has quit IRC11:25
tobiashelectrofelix: It might have been a corner case, honestly I don't know anymore, but this should be testable  speculatively11:25
AJaegerelectrofelix: you might want to check the discussion on #openstack-infra and #zuul for the discussion about the change, there are archives on eavesdrop.openstack.org11:28
*** bjackman_ has joined #zuul11:28
*** bjackman__ has joined #zuul11:37
*** bjackman_ has quit IRC11:39
*** bhavikdbavishi has joined #zuul11:45
*** jpena|lunch is now known as jpena12:18
*** altlogbot_1 has quit IRC12:44
*** bhavikdbavishi has quit IRC12:44
*** rlandy has joined #zuul12:46
*** altlogbot_2 has joined #zuul12:47
*** bhavikdbavishi has joined #zuul12:49
*** bhavikdbavishi has quit IRC12:53
*** bhavikdbavishi has joined #zuul13:02
Shrewstobiash: that’s super weird.13:07
ShrewsMore threads are started to delete the instances, so I’d expect at least a temporary mem increase though13:09
ShrewsIf it doesn’t drop, maybe we aren’t cleaning those up properly? But nothing in our code has changed there13:10
tobiashShrews: objgraph also tells me that we seem to leak frame objects, some kind of thread leak?13:11
tobiashbut the stack trace doesn't look like we leak theads (at least no alive threads)13:12
tobiashShrews: it might make sense to have a thread pool for deletion?13:12
ShrewsPossibly. I forget offhand how we deal with tracking those and not at my computer atm13:14
tobiashk, basically we create the thread, call start and forget about it13:15
pabelangerHmm, is the following change_url from zuul.projects correct?13:21
pabelangerchange_url: https://api.github.com/repos/ansible/ansible/pulls/5558913:21
pabelangerthat came from: https://logs.zuul.ansible.com/12/12/364df274b0d0dde8e03bfe0ccd2f5e8de47dacc1/check/network-vyos-test/23329a1/zuul-info/inventory.yaml13:21
*** bjackman__ has quit IRC13:25
pabelangermordred: so, we in ansible-network have a use case, where we are using tox to run ansible-playbook, but our role current doesn't have setup.py / setup.cfg. So for tox siblings, it doesn't work by default, due to: https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/tox/library/tox_install_sibling_packages.py#L16313:27
*** gtema has quit IRC13:28
pabelangermordred: do you have an idea how we could support that usecase? Where the project wanting to use tox siblings, doesn't use setuptool13:28
pabelangersetuptools*13:28
pabelangermy first guess is to expose tox_siblings_name and set it via ansible varialble13:29
pabelangervariable*13:29
tobiashpabelanger: I saw a patch somewhere that fixes the change url in zuul13:34
tobiashor, I think from job perspective that might be correct (as this is the url you use to interact with the change via api)13:35
*** themroc has quit IRC13:43
*** bjackman__ has joined #zuul13:45
pabelangertobiash: ack, I'll look shortly13:51
tobiashhrm, I don't find it atm, but I think I saw a change in that direction in the last two weeks13:52
tobiashor maybe I dreamed that ;)13:53
Shrewstobiash: if we lose the zk session while the launcher is running, do we have to re-register the nodeCacheListener to begin receiving events again?13:53
tobiashShrews: that should handle that by itself13:54
*** rfolco is now known as rfolco|ruck13:54
*** zbr is now known as zbr|rover13:55
clarkbpabelanger: does your repo being tox'd have a specifier of dependencies in some other form?13:58
clarkbI think the idea was to leverage the existing specification of dependencies so that you don't have to track it multiple times13:58
pabelangerclarkb: yah, we just have (test-)requirements.txt files tht tox.ini references. But not setup.cfg / setup.py files.13:59
clarkbyou are expected to install those yourself then?14:01
pabelangerso, the use case is, we to tox -evenv -- ansible-playbook foo.yaml, where we then pull in ansible via requirements.txt. Because we want to do cross-project testing on ansible/ansible PRs, using tox siblings was the idea.14:03
pabelangerHowever, where we run tox from, isn't really a python project. It is an ansible role, so a little weird carrying setup.py / setup.cfg files14:03
pabelangerbut okay to carry tox requirement files14:03
clarkbpabelanger: have you seen opendev/system-config :P14:04
clarkb(I personally don't see it sa weird if it solves a problem)14:04
clarkbpabelanger: what setup.py is doing for us is solving the problem of determining which libs are potential siblings14:05
clarkbit is possible we may ant to do that for simply havingrequirements.txt too14:05
pabelangerclarkb: yah, on the project you want to depends on, I think that is correct. But for the parent project, I only see logic to ensure we don't pip install ourself, which shouldn't be an issue if there is no setup.py / setup.cfg I think14:07
pabelangerhttps://opendev.org/zuul/zuul-jobs/src/branch/master/roles/tox/library/tox_install_sibling_packages.py#L17214:08
pabelangerI think we only use the name value to do a simple compare14:08
pabelangerwhich, could also come from zuul.projects.short_name, if we expose an ansible variable14:08
dmsimardI'm going to help move the openstack/packstack github repo, just double checking -- there's nothing else to do but the actual repository move, right ? I mean, other than setting up the replication with the upload-git-mirror job.14:08
*** bjackman__ has quit IRC14:09
dmsimard^ was for #openstack-infra, my bad14:13
*** pcaruana has quit IRC14:15
*** bjackman__ has joined #zuul14:36
fungireminder, zuul bof starting in room 507 roughly 15 minutes from now, for those of you attending the open infra summit. there will also be a zuul working session immediately after that in the same room, so we basically have the room from 9:00-10:30am mdt14:45
*** pcaruana has joined #zuul14:46
Shrewstobiash: I wonder if us not doing a join() on the NodeDeleter thread is the cause of the leak...  I think we should look into a threadpool as you suggested, either way. I can look at that after the summit unless you beat me to it.14:51
tobiashShrews: ++ for threadpool after summit is fine :)14:52
tobiashmaybe I have time to assist but cannot promise14:53
clarkbconcurrent.futures.ThreadPoolExecutor?14:53
Shrewsyeah14:54
Shrewsclarkb: looks like they are letting people in now14:55
*** Miouge has joined #zuul14:57
clarkbyup got in thanks14:57
*** mattw4 has joined #zuul15:19
*** bhavikdbavishi has quit IRC15:30
*** johnsom has joined #zuul15:31
*** chandankumar is now known as raukadah15:40
Shrewstobiash: any chance you can recreate your 50 node delete test, but supplement it with an additional 50 node create & delete immediately after? Curious to see the memory usage after that 2nd round.15:53
*** mattw4 has quit IRC15:54
*** mattw4 has joined #zuul15:55
*** pcaruana has quit IRC15:55
tobiashI can try later, on my way home now15:56
Shrewstobiash: oh, no rush. thx15:56
tobiashBut I'm unsure about the exact scenario you describe15:56
Shrewstobiash: my thinking is that the mem jump is normal (for thread usage) and not all of the memory will be released back (as your graph shows), but I would NOT expect a similar spike when the process is repeated15:58
Shrewswe might be chasing a red herring here15:58
*** enriquetaso has joined #zuul15:59
tobiashUnderstood, so several runs in a sequence16:04
Shrewsyep16:05
*** StaceyF has joined #zuul16:06
*** quiquell is now known as quiquell|off16:16
SpamapSThe authenticity of host '[review.opendev.org]:29418 ([2001:4800:7819:103:be76:4eff:fe04:9229]:29418)' can't be established.16:18
*** mattw4 has quit IRC16:18
SpamapSRSA key fingerprint is SHA256:RXNl/GKyDaKiIQ93BoDvrNSKUPFvA1PNeAO9QiirYZU.16:18
SpamapSIs the key fingerprint printed somewhere behind https?16:18
*** mattw4 has joined #zuul16:18
clarkbSpamapS: yes, one sec for a link16:20
SpamapSclarkb:ty16:20
clarkbSpamapS: https://review.opendev.org/#/settings/ssh-keys there (you have to log in)16:20
SpamapSugh16:20
* SpamapS has to go find his yubikey for that. :-P16:20
SpamapSclarkb:ty, I'll trudge upstairs and find it. ;)16:21
clarkbfwiw it is the same hostkey as review.openstack.org iirc16:21
clarkbso if you have that one confirmed you can cross check against that16:21
clarkb(but don't take my word for it, gerrit will tell you exactly what it is :) )16:22
paladoxgerrit apparently no longer does (from the new ui) :(16:23
openstackgerritClint 'SpamapS' Byrum proposed zuul/zuul master: Add support for submitting reviews on GitHub  https://review.opendev.org/65654116:23
SpamapSclarkb:no I appreciate that, and I think sending folks to the ssh-keys settings page is the right thing.16:29
SpamapS(I did in fact just compare it to the review.openstack.org one ;-)16:29
SpamapSpaladox:doh!16:29
SpamapSI'd suggest we publish that key fingerprint in a few places. Like.. anywhere that will let us. ;)16:29
paladoxheh16:30
paladoxIt should be easy to implement in the new ui, just needs a rest api to provide that info :)16:30
*** StaceyF has quit IRC16:34
SpamapSWhy does it feel like Gerrit is always in "new UI" mode?16:36
SpamapSlike, what is the old UI at this point?16:36
clarkbSpamapS: we are running the old ui now16:37
clarkbwhich is the second major ui. On the next upgrade we'll get the third major UI16:37
clarkbthe third major UI is basically going back to the first major ui but with regular js tooling instead of gwt16:37
SpamapShah16:37
SpamapSwell my only UI is gertty ;-)16:38
SpamapSclarkb:2019-04-30 16:27:37.847272 | ubuntu-bionic | EEXIST: file already exists, mkdir '/home/zuul/src/opendev.org/zuul/zuul/web/build'  <-- is that the same yarn problem from before?16:39
clarkbSpamapS: no I think the eralier issue was permissions to run the yarn command16:40
tobiashSpamapS: remove the .keep file deletion from your change16:41
SpamapStobiash: weird, why does git delete that?16:42
SpamapStobiash:and thanks16:42
tobiashSpamapS: it gets deleted by yarn and when you do git add . it's part of the commit...16:43
openstackgerritClint 'SpamapS' Byrum proposed zuul/zuul master: Add support for submitting reviews on GitHub  https://review.opendev.org/65654116:43
SpamapStobiash:ohh, that's annoying16:43
*** enriquetaso has quit IRC16:44
paladoxSpamapS per clarkb, though polygerrit is awesome :)16:45
tobiashyes, I made that mistake several times as well ;)16:46
*** enriquetaso has joined #zuul16:48
paladoxI recently sent some mail to our list (wmf) asking all our users to try out the new ui :)16:48
*** enriquetaso has left #zuul16:50
*** bjackman__ has quit IRC16:54
*** jpena is now known as jpena|off16:54
tobiashShrews: the memory graphs are not that clear anymore :/17:52
tobiashShrews: but taking objdumps before and after each iteration it looks like we're leaking exactly one logger per created/destroyed vm17:55
*** tjgresha has quit IRC18:04
tobiashShrews: I now did an objdump and we leak one Logger object per instance somewhere in the instance creation process.18:05
tobiashs/objdump/objdump in between creation and deletion18:05
*** tjgresha has joined #zuul18:06
tobiashShrews: found this: https://mail.python.org/pipermail/python-list/2011-November/615602.html18:16
tobiash"Loggers are static objects managed by the module itself. When you create18:16
tobiashone, it won't be removed until you leave the shell. Thas is way it is18:16
tobiashadvise not to create thousands of loggers with different names.18:16
tobiashThat's the module design, nothing you can do about it."18:16
tobiashso because we create a logger per node we essentially leak them because they are not destroyed18:17
pabelangerI thought we did loggers by provider18:18
Shrewstobiash: which logger? In the NodeDeleter? Sorry, not at my computer now18:21
tobiashno, this: self.log = logging.getLogger("nodepool.NodeLauncher-%s" % node.id)18:21
ShrewsOh18:22
tobiashin the NodeLauncher18:22
tobiashevery logger we create there will stay forever18:22
ShrewsLet’s stop doing that then. :)18:22
tobiashI'll prepare a patch that changes this to use a logadapter18:22
Shrews++18:22
tobiashlike zuul is doing that in the github event log18:22
pabelanger+118:22
SpamapS\o/18:40
SpamapSNice to see a team effort on debugging pay off18:41
*** electrofelix has quit IRC18:45
*** jamesmcarthur has joined #zuul18:55
ShrewsSpamapS: fo shizzle18:59
*** pcaruana has joined #zuul19:10
openstackgerritTobias Henkel proposed zuul/nodepool master: Fix leaked loggers  https://review.opendev.org/65657519:11
*** sshnaidm|ptg has joined #zuul19:23
*** jamesmcarthur has quit IRC19:37
*** sshnaidm|ptg has quit IRC19:37
*** gtema has joined #zuul19:41
*** sshnaidm|ptg has joined #zuul20:19
*** jamesmcarthur has joined #zuul20:21
*** sshnaidm|ptg has quit IRC20:21
*** sshnaidm|ptg has joined #zuul20:21
*** pcaruana has quit IRC20:23
*** jamesmcarthur has quit IRC20:35
*** jamesmcarthur has joined #zuul20:38
*** jamesmcarthur has quit IRC20:54
*** jamesmcarthur has joined #zuul20:56
*** jamesmcarthur has quit IRC21:04
*** jamesmcarthur has joined #zuul21:05
*** jamesmcarthur has quit IRC21:45
*** tjgresha has quit IRC21:46
*** jamesmcarthur has joined #zuul21:47
*** jamesmcarthur has quit IRC22:05
openstackgerritTobias Urdin proposed zuul/zuul-jobs master: Use PDK to build puppet module  https://review.opendev.org/62753423:00
*** ianychoi_ has quit IRC23:03
*** ianychoi_ has joined #zuul23:04
*** gtema has quit IRC23:32
*** rlandy has quit IRC23:40
*** mattw4 has quit IRC23:41
*** jamesmcarthur has joined #zuul23:47
*** sshnaidm|ptg has quit IRC23:50
*** jamesmcarthur has quit IRC23:55

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