*** mattw4 has quit IRC | 00:10 | |
SpamapS | mmmm 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 #zuul | 01:34 | |
pabelanger | I'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 |
pabelanger | just to work around 'you cannot approve own PR' | 01:35 |
pabelanger | but, 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 user | 01:36 |
*** hashar has quit IRC | 02:06 | |
*** raukadah is now known as chandankumar | 03:11 | |
*** bhavikdbavishi has joined #zuul | 03:23 | |
*** sshnaidm|ptg has joined #zuul | 03:25 | |
*** bhavikdbavishi1 has joined #zuul | 03:39 | |
*** bhavikdbavishi has quit IRC | 03:40 | |
*** bhavikdbavishi1 is now known as bhavikdbavishi | 03:40 | |
tobiash | pabelanger: you'll be interested in https://review.opendev.org/#/c/644557 | 03:51 |
*** bhavikdbavishi has quit IRC | 04:23 | |
*** bhavikdbavishi has joined #zuul | 04:27 | |
SpamapS | tobiash:yeah that makes sense. | 04:55 |
SpamapS | pabelanger: 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 |
SpamapS | pabelanger:and zuul already has a way to ignore unprotected branches... maybe that's enough? | 04:58 |
SpamapS | anyway, 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 IRC | 05:03 | |
*** rfolco|ruck has joined #zuul | 05:04 | |
*** sshnaidm|ptg has quit IRC | 05:05 | |
*** bhavikdbavishi1 has joined #zuul | 05:10 | |
*** bhavikdbavishi has quit IRC | 05:10 | |
*** bhavikdbavishi has joined #zuul | 05:13 | |
*** bhavikdbavishi1 has quit IRC | 05:14 | |
*** rfolco|ruck has quit IRC | 05:17 | |
*** rfolco|ruck has joined #zuul | 05:17 | |
badboy | does zuul support draft-created gerrit event? | 05:56 |
*** rfolco|ruck has quit IRC | 05:58 | |
*** quiquell|off is now known as quiquell | 06:04 | |
*** rfolco has joined #zuul | 06:06 | |
*** gtema has joined #zuul | 06:28 | |
*** gtema has quit IRC | 06:40 | |
*** threestrands has joined #zuul | 06:46 | |
*** themroc has joined #zuul | 06:50 | |
*** bjackman has joined #zuul | 07:05 | |
*** bhavikdbavishi has quit IRC | 07:12 | |
*** bhavikdbavishi has joined #zuul | 07:13 | |
*** jpena|off is now known as jpena | 07:39 | |
*** pcaruana has joined #zuul | 07:40 | |
*** rfolco has quit IRC | 07:52 | |
*** rfolco has joined #zuul | 07:52 | |
*** threestrands has quit IRC | 07:55 | |
*** bjackman_ has joined #zuul | 08:02 | |
*** bjackman has quit IRC | 08:05 | |
*** bhavikdbavishi has quit IRC | 08:12 | |
*** bhavikdbavishi has joined #zuul | 08:26 | |
openstackgerrit | Fabien Boucher proposed zuul/zuul master: Pagure driver - https://pagure.io/pagure/ https://review.opendev.org/604404 | 08:46 |
*** bhavikdbavishi has quit IRC | 09:01 | |
*** bhavikdbavishi has joined #zuul | 09:07 | |
*** electrofelix has joined #zuul | 09:11 | |
*** swest has quit IRC | 09:17 | |
*** swest has joined #zuul | 09:18 | |
*** bhavikdbavishi has quit IRC | 10:00 | |
*** bhavikdbavishi has joined #zuul | 10:07 | |
*** bhavikdbavishi has quit IRC | 10:11 | |
tobiash | Shrews: 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 once | 10:23 |
tobiash | https://paste.pics/b93939da6b2445fa933106a83fec5be8 | 10:32 |
tobiash | Shrews: what is interesting is that the leak seems to be somewhere during instance *deletion* | 10:32 |
electrofelix | Was 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 ago | 10:36 |
*** bhavikdbavishi has joined #zuul | 10:56 | |
*** jpena is now known as jpena|lunch | 10:59 | |
*** gtema has joined #zuul | 11:00 | |
*** bhavikdbavishi has quit IRC | 11:00 | |
AJaeger | electrofelix: no, it was not | 11:01 |
AJaeger | electrofelix: at least I don't remember it beeing completed | 11:01 |
electrofelix | AJaeger: thanks | 11:21 |
electrofelix | tobiash: can you fill me in on what was wrong with the above change? as the log referenced in the commit message no longer exists | 11:21 |
tobiash | electrofelix: 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 |
electrofelix | so 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 behaviour | 11:25 |
*** bjackman_ has quit IRC | 11:25 | |
tobiash | electrofelix: It might have been a corner case, honestly I don't know anymore, but this should be testable speculatively | 11:25 |
AJaeger | electrofelix: you might want to check the discussion on #openstack-infra and #zuul for the discussion about the change, there are archives on eavesdrop.openstack.org | 11:28 |
*** bjackman_ has joined #zuul | 11:28 | |
*** bjackman__ has joined #zuul | 11:37 | |
*** bjackman_ has quit IRC | 11:39 | |
*** bhavikdbavishi has joined #zuul | 11:45 | |
*** jpena|lunch is now known as jpena | 12:18 | |
*** altlogbot_1 has quit IRC | 12:44 | |
*** bhavikdbavishi has quit IRC | 12:44 | |
*** rlandy has joined #zuul | 12:46 | |
*** altlogbot_2 has joined #zuul | 12:47 | |
*** bhavikdbavishi has joined #zuul | 12:49 | |
*** bhavikdbavishi has quit IRC | 12:53 | |
*** bhavikdbavishi has joined #zuul | 13:02 | |
Shrews | tobiash: that’s super weird. | 13:07 |
Shrews | More threads are started to delete the instances, so I’d expect at least a temporary mem increase though | 13:09 |
Shrews | If it doesn’t drop, maybe we aren’t cleaning those up properly? But nothing in our code has changed there | 13:10 |
tobiash | Shrews: objgraph also tells me that we seem to leak frame objects, some kind of thread leak? | 13:11 |
tobiash | but the stack trace doesn't look like we leak theads (at least no alive threads) | 13:12 |
tobiash | Shrews: it might make sense to have a thread pool for deletion? | 13:12 |
Shrews | Possibly. I forget offhand how we deal with tracking those and not at my computer atm | 13:14 |
tobiash | k, basically we create the thread, call start and forget about it | 13:15 |
pabelanger | Hmm, is the following change_url from zuul.projects correct? | 13:21 |
pabelanger | change_url: https://api.github.com/repos/ansible/ansible/pulls/55589 | 13:21 |
pabelanger | that came from: https://logs.zuul.ansible.com/12/12/364df274b0d0dde8e03bfe0ccd2f5e8de47dacc1/check/network-vyos-test/23329a1/zuul-info/inventory.yaml | 13:21 |
*** bjackman__ has quit IRC | 13:25 | |
pabelanger | mordred: 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#L163 | 13:27 |
*** gtema has quit IRC | 13:28 | |
pabelanger | mordred: do you have an idea how we could support that usecase? Where the project wanting to use tox siblings, doesn't use setuptool | 13:28 |
pabelanger | setuptools* | 13:28 |
pabelanger | my first guess is to expose tox_siblings_name and set it via ansible varialble | 13:29 |
pabelanger | variable* | 13:29 |
tobiash | pabelanger: I saw a patch somewhere that fixes the change url in zuul | 13:34 |
tobiash | or, 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 IRC | 13:43 | |
*** bjackman__ has joined #zuul | 13:45 | |
pabelanger | tobiash: ack, I'll look shortly | 13:51 |
tobiash | hrm, I don't find it atm, but I think I saw a change in that direction in the last two weeks | 13:52 |
tobiash | or maybe I dreamed that ;) | 13:53 |
Shrews | tobiash: 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 |
tobiash | Shrews: that should handle that by itself | 13:54 |
*** rfolco is now known as rfolco|ruck | 13:54 | |
*** zbr is now known as zbr|rover | 13:55 | |
clarkb | pabelanger: does your repo being tox'd have a specifier of dependencies in some other form? | 13:58 |
clarkb | I think the idea was to leverage the existing specification of dependencies so that you don't have to track it multiple times | 13:58 |
pabelanger | clarkb: yah, we just have (test-)requirements.txt files tht tox.ini references. But not setup.cfg / setup.py files. | 13:59 |
clarkb | you are expected to install those yourself then? | 14:01 |
pabelanger | so, 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 |
pabelanger | However, 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 files | 14:03 |
pabelanger | but okay to carry tox requirement files | 14:03 |
clarkb | pabelanger: have you seen opendev/system-config :P | 14:04 |
clarkb | (I personally don't see it sa weird if it solves a problem) | 14:04 |
clarkb | pabelanger: what setup.py is doing for us is solving the problem of determining which libs are potential siblings | 14:05 |
clarkb | it is possible we may ant to do that for simply havingrequirements.txt too | 14:05 |
pabelanger | clarkb: 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 think | 14:07 |
pabelanger | https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/tox/library/tox_install_sibling_packages.py#L172 | 14:08 |
pabelanger | I think we only use the name value to do a simple compare | 14:08 |
pabelanger | which, could also come from zuul.projects.short_name, if we expose an ansible variable | 14:08 |
dmsimard | I'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 IRC | 14:09 | |
dmsimard | ^ was for #openstack-infra, my bad | 14:13 |
*** pcaruana has quit IRC | 14:15 | |
*** bjackman__ has joined #zuul | 14:36 | |
fungi | reminder, 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 mdt | 14:45 |
*** pcaruana has joined #zuul | 14:46 | |
Shrews | tobiash: 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 |
tobiash | Shrews: ++ for threadpool after summit is fine :) | 14:52 |
tobiash | maybe I have time to assist but cannot promise | 14:53 |
clarkb | concurrent.futures.ThreadPoolExecutor? | 14:53 |
Shrews | yeah | 14:54 |
Shrews | clarkb: looks like they are letting people in now | 14:55 |
*** Miouge has joined #zuul | 14:57 | |
clarkb | yup got in thanks | 14:57 |
*** mattw4 has joined #zuul | 15:19 | |
*** bhavikdbavishi has quit IRC | 15:30 | |
*** johnsom has joined #zuul | 15:31 | |
*** chandankumar is now known as raukadah | 15:40 | |
Shrews | tobiash: 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 IRC | 15:54 | |
*** mattw4 has joined #zuul | 15:55 | |
*** pcaruana has quit IRC | 15:55 | |
tobiash | I can try later, on my way home now | 15:56 |
Shrews | tobiash: oh, no rush. thx | 15:56 |
tobiash | But I'm unsure about the exact scenario you describe | 15:56 |
Shrews | tobiash: 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 repeated | 15:58 |
Shrews | we might be chasing a red herring here | 15:58 |
*** enriquetaso has joined #zuul | 15:59 | |
tobiash | Understood, so several runs in a sequence | 16:04 |
Shrews | yep | 16:05 |
*** StaceyF has joined #zuul | 16:06 | |
*** quiquell is now known as quiquell|off | 16:16 | |
SpamapS | The 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 IRC | 16:18 | |
SpamapS | RSA key fingerprint is SHA256:RXNl/GKyDaKiIQ93BoDvrNSKUPFvA1PNeAO9QiirYZU. | 16:18 |
SpamapS | Is the key fingerprint printed somewhere behind https? | 16:18 |
*** mattw4 has joined #zuul | 16:18 | |
clarkb | SpamapS: yes, one sec for a link | 16:20 |
SpamapS | clarkb:ty | 16:20 |
clarkb | SpamapS: https://review.opendev.org/#/settings/ssh-keys there (you have to log in) | 16:20 |
SpamapS | ugh | 16:20 |
* SpamapS has to go find his yubikey for that. :-P | 16:20 | |
SpamapS | clarkb:ty, I'll trudge upstairs and find it. ;) | 16:21 |
clarkb | fwiw it is the same hostkey as review.openstack.org iirc | 16:21 |
clarkb | so if you have that one confirmed you can cross check against that | 16:21 |
clarkb | (but don't take my word for it, gerrit will tell you exactly what it is :) ) | 16:22 |
paladox | gerrit apparently no longer does (from the new ui) :( | 16:23 |
openstackgerrit | Clint 'SpamapS' Byrum proposed zuul/zuul master: Add support for submitting reviews on GitHub https://review.opendev.org/656541 | 16:23 |
SpamapS | clarkb: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 |
SpamapS | paladox:doh! | 16:29 |
SpamapS | I'd suggest we publish that key fingerprint in a few places. Like.. anywhere that will let us. ;) | 16:29 |
paladox | heh | 16:30 |
paladox | It should be easy to implement in the new ui, just needs a rest api to provide that info :) | 16:30 |
*** StaceyF has quit IRC | 16:34 | |
SpamapS | Why does it feel like Gerrit is always in "new UI" mode? | 16:36 |
SpamapS | like, what is the old UI at this point? | 16:36 |
clarkb | SpamapS: we are running the old ui now | 16:37 |
clarkb | which is the second major ui. On the next upgrade we'll get the third major UI | 16:37 |
clarkb | the third major UI is basically going back to the first major ui but with regular js tooling instead of gwt | 16:37 |
SpamapS | hah | 16:37 |
SpamapS | well my only UI is gertty ;-) | 16:38 |
SpamapS | clarkb: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 |
clarkb | SpamapS: no I think the eralier issue was permissions to run the yarn command | 16:40 |
tobiash | SpamapS: remove the .keep file deletion from your change | 16:41 |
SpamapS | tobiash: weird, why does git delete that? | 16:42 |
SpamapS | tobiash:and thanks | 16:42 |
tobiash | SpamapS: it gets deleted by yarn and when you do git add . it's part of the commit... | 16:43 |
openstackgerrit | Clint 'SpamapS' Byrum proposed zuul/zuul master: Add support for submitting reviews on GitHub https://review.opendev.org/656541 | 16:43 |
SpamapS | tobiash:ohh, that's annoying | 16:43 |
*** enriquetaso has quit IRC | 16:44 | |
paladox | SpamapS per clarkb, though polygerrit is awesome :) | 16:45 |
tobiash | yes, I made that mistake several times as well ;) | 16:46 |
*** enriquetaso has joined #zuul | 16:48 | |
paladox | I recently sent some mail to our list (wmf) asking all our users to try out the new ui :) | 16:48 |
*** enriquetaso has left #zuul | 16:50 | |
*** bjackman__ has quit IRC | 16:54 | |
*** jpena is now known as jpena|off | 16:54 | |
tobiash | Shrews: the memory graphs are not that clear anymore :/ | 17:52 |
tobiash | Shrews: but taking objdumps before and after each iteration it looks like we're leaking exactly one logger per created/destroyed vm | 17:55 |
*** tjgresha has quit IRC | 18:04 | |
tobiash | Shrews: I now did an objdump and we leak one Logger object per instance somewhere in the instance creation process. | 18:05 |
tobiash | s/objdump/objdump in between creation and deletion | 18:05 |
*** tjgresha has joined #zuul | 18:06 | |
tobiash | Shrews: found this: https://mail.python.org/pipermail/python-list/2011-November/615602.html | 18:16 |
tobiash | "Loggers are static objects managed by the module itself. When you create | 18:16 |
tobiash | one, it won't be removed until you leave the shell. Thas is way it is | 18:16 |
tobiash | advise not to create thousands of loggers with different names. | 18:16 |
tobiash | That's the module design, nothing you can do about it." | 18:16 |
tobiash | so because we create a logger per node we essentially leak them because they are not destroyed | 18:17 |
pabelanger | I thought we did loggers by provider | 18:18 |
Shrews | tobiash: which logger? In the NodeDeleter? Sorry, not at my computer now | 18:21 |
tobiash | no, this: self.log = logging.getLogger("nodepool.NodeLauncher-%s" % node.id) | 18:21 |
Shrews | Oh | 18:22 |
tobiash | in the NodeLauncher | 18:22 |
tobiash | every logger we create there will stay forever | 18:22 |
Shrews | Let’s stop doing that then. :) | 18:22 |
tobiash | I'll prepare a patch that changes this to use a logadapter | 18:22 |
Shrews | ++ | 18:22 |
tobiash | like zuul is doing that in the github event log | 18:22 |
pabelanger | +1 | 18:22 |
SpamapS | \o/ | 18:40 |
SpamapS | Nice to see a team effort on debugging pay off | 18:41 |
*** electrofelix has quit IRC | 18:45 | |
*** jamesmcarthur has joined #zuul | 18:55 | |
Shrews | SpamapS: fo shizzle | 18:59 |
*** pcaruana has joined #zuul | 19:10 | |
openstackgerrit | Tobias Henkel proposed zuul/nodepool master: Fix leaked loggers https://review.opendev.org/656575 | 19:11 |
*** sshnaidm|ptg has joined #zuul | 19:23 | |
*** jamesmcarthur has quit IRC | 19:37 | |
*** sshnaidm|ptg has quit IRC | 19:37 | |
*** gtema has joined #zuul | 19:41 | |
*** sshnaidm|ptg has joined #zuul | 20:19 | |
*** jamesmcarthur has joined #zuul | 20:21 | |
*** sshnaidm|ptg has quit IRC | 20:21 | |
*** sshnaidm|ptg has joined #zuul | 20:21 | |
*** pcaruana has quit IRC | 20:23 | |
*** jamesmcarthur has quit IRC | 20:35 | |
*** jamesmcarthur has joined #zuul | 20:38 | |
*** jamesmcarthur has quit IRC | 20:54 | |
*** jamesmcarthur has joined #zuul | 20:56 | |
*** jamesmcarthur has quit IRC | 21:04 | |
*** jamesmcarthur has joined #zuul | 21:05 | |
*** jamesmcarthur has quit IRC | 21:45 | |
*** tjgresha has quit IRC | 21:46 | |
*** jamesmcarthur has joined #zuul | 21:47 | |
*** jamesmcarthur has quit IRC | 22:05 | |
openstackgerrit | Tobias Urdin proposed zuul/zuul-jobs master: Use PDK to build puppet module https://review.opendev.org/627534 | 23:00 |
*** ianychoi_ has quit IRC | 23:03 | |
*** ianychoi_ has joined #zuul | 23:04 | |
*** gtema has quit IRC | 23:32 | |
*** rlandy has quit IRC | 23:40 | |
*** mattw4 has quit IRC | 23:41 | |
*** jamesmcarthur has joined #zuul | 23:47 | |
*** sshnaidm|ptg has quit IRC | 23:50 | |
*** jamesmcarthur has quit IRC | 23:55 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!