| mordred | SpamapS: I see two forms: | 00:03 |
|---|---|---|
| mordred | job.sendWorkException(errors.encode('utf8')) | 00:03 |
| mordred | and | 00:03 |
| mordred | job.sendWorkException(traceback.format_exc()) | 00:03 |
| mordred | the one where we're calling it and it's not doing RPCException is one of the job.sendWorkException(traceback.format_exc()) forms | 00:04 |
| jeblair | mordred: the lack of RPCFailure suggests that job.exception is evaling to false | 00:06 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add driver-specific pipeline requirements https://review.openstack.org/466105 | 00:06 |
| jeblair | jlk: ^ for real now; passes tests locally | 00:07 |
| jeblair | (will likely fail the rpc tests in zuul) | 00:07 |
| mordred | jeblair: yah - so if it's just an encoding error somehow from the traceback.format_exc() I'd expect that to fail python string processing before it even gets to actuallyy sending the Packet | 00:08 |
| jeblair | yeah, puzzling | 00:08 |
| mordred | and I don't see, at least yet, how anything here could break the way sendWorkException works | 00:08 |
| mordred | I swear it's going to be a missing comma - which are my favorite kind of puzzle ... but I will now interact with meat | 00:11 |
| jeblair | i've replicated it with zuulv3 tip | 00:13 |
| jeblair | i think i see it | 00:18 |
| jeblair | 00:20 < openstackgerrit> James E. Blair proposed openstack-infra/gear master: Fix exception setter https://review.openstack.org/466131 | 00:20 |
| mordred | jeblair: yes. I agree - that is sadpanda | 00:22 |
| SpamapS | DERRRRPPP | 00:22 |
| SpamapS | that is definitely a Delta Comfort+ Fail | 00:23 |
| SpamapS | If I had gotten a full 1st class upgrade I'd have noticed that | 00:23 |
| jeblair | is there some way we can blame united? | 00:23 |
| SpamapS | (and written tests for the setters) | 00:23 |
| SpamapS | jeblair: yes, United drove me to Delta | 00:23 |
| mordred | we should blame all of the airlines | 00:23 |
| SpamapS | and all of the airlines drove me to drink | 00:24 |
| mordred | although I'm holding out hope that aeromexico is going to be glorious | 00:24 |
| jeblair | united has created a new kind of pain -- some of their new armrests have the call button right where your elbow goes, so on the 5.5 hr flight to boston, it was "DING!" every 5 seconds as someone on the plane hit the button. the fa's made announcements telling people to be careful to no avail. | 00:30 |
| jeblair | mordred, SpamapS: i +2d both of your patches; hopefully they will land after rechecks | 00:32 |
| SpamapS | jeblair: the most recent recheck of 466112 still picked up 0.9.0 | 00:33 |
| SpamapS | jeblair: I have to detach for the evening, so another recheck will be needed at 466112 | 00:34 |
| SpamapS | it's just about done with the current one | 00:34 |
| jlk | jeblair: LOL that United flight sounds like another case of 1 unit test and 0 integration tests. | 00:40 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Add paste to the default list of loggers in tests https://review.openstack.org/466118 | 00:44 |
| *** _ari_ has quit IRC | 01:04 | |
| *** pabelanger has quit IRC | 01:04 | |
| *** fbo has quit IRC | 01:04 | |
| *** dmellado has quit IRC | 01:04 | |
| *** pabelanger has joined #zuul | 01:08 | |
| *** _ari_ has joined #zuul | 01:08 | |
| *** fbo has joined #zuul | 01:12 | |
| *** jkilpatr has quit IRC | 01:17 | |
| *** dmellado has joined #zuul | 01:22 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove errant basicConfig calls in test suites https://review.openstack.org/466112 | 01:34 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: connections: only configure sql on the server https://review.openstack.org/466148 | 02:10 |
| *** jamielennox is now known as jamielennox|away | 03:46 | |
| *** jamielennox|away is now known as jamielennox | 03:53 | |
| *** bhavik1 has joined #zuul | 05:25 | |
| *** bhavik1 has quit IRC | 06:33 | |
| *** adam_g has quit IRC | 07:00 | |
| *** adam_g has joined #zuul | 07:01 | |
| *** DangerousDaren has joined #zuul | 07:02 | |
| *** adam_g has quit IRC | 07:12 | |
| *** adam_g has joined #zuul | 07:13 | |
| *** adam_g has quit IRC | 07:21 | |
| *** adam_g has joined #zuul | 07:22 | |
| *** Cibo_ has joined #zuul | 09:04 | |
| *** hashar has joined #zuul | 09:17 | |
| *** Cibo_ has quit IRC | 09:42 | |
| *** bhavik1 has joined #zuul | 09:50 | |
| *** openstack has joined #zuul | 10:00 | |
| *** hashar has joined #zuul | 10:04 | |
| *** jkilpatr has joined #zuul | 11:00 | |
| *** bhavik1 has quit IRC | 11:18 | |
| *** hashar has quit IRC | 11:19 | |
| *** dougbtv_ is now known as dougbtv | 11:43 | |
| *** hashar has joined #zuul | 12:29 | |
| *** hashar has quit IRC | 12:35 | |
| *** hashar has joined #zuul | 12:35 | |
| *** dkranz has joined #zuul | 12:48 | |
| *** dkranz has quit IRC | 13:07 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove use of hacking lib https://review.openstack.org/466067 | 13:09 |
| Shrews | oh wow. that merged. neato | 13:12 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: WIP: Add web-based console log streaming https://review.openstack.org/463353 | 13:27 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Re-enable F405 pep8 errors https://review.openstack.org/466285 | 13:36 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Re-enable E305 pep8 errors https://review.openstack.org/466286 | 13:39 |
| *** hashar has quit IRC | 13:40 | |
| Shrews | jeblair: i'm going to leave (E402 module level import not at top of file) and (W503 line break before binary operator) as ignored by pep8 b/c we can't avoid the first, and the 2nd is just silly | 13:47 |
| Shrews | well, i guess we *could* avoid the 1st, actually | 13:50 |
| Shrews | it would require fixing up the alembic version files, which, iirc, are autogenerated | 13:52 |
| Shrews | and the ansible stuff. bleh... i'd rather ignore | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: skip py3 failing tests https://review.openstack.org/463903 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: fix webapp tests for py3 https://review.openstack.org/463902 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: misc py3 changes https://review.openstack.org/463901 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: base64 changes for py3 https://review.openstack.org/463900 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Encoding changes in tests for py3 https://review.openstack.org/463899 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Explicitly decode decrypted secrets for py3 https://review.openstack.org/464049 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: view changes for py3 https://review.openstack.org/463898 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: writing yaml to disk needs bytes https://review.openstack.org/463897 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Fixes for test_model in py3 https://review.openstack.org/463896 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: None does not compare to int https://review.openstack.org/463895 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: make Job and ZuulRole hashable https://review.openstack.org/463894 | 13:53 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: py3 hashlib error https://review.openstack.org/463893 | 13:54 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: py3 Changes in __del__ for gitpython https://review.openstack.org/463892 | 13:54 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: fix imports in py3 https://review.openstack.org/463891 | 13:54 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Use gear Text interface https://review.openstack.org/461468 | 13:54 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Fix github driver tests for py3 https://review.openstack.org/466078 | 13:54 |
| openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul feature/zuulv3: Enable py3 tests https://review.openstack.org/466079 | 13:54 |
| * SpamapS wakes and rebases | 13:54 | |
| Shrews | all your rebase are belong to you | 13:54 |
| SpamapS | now that we sorted gear 0.9.1, would be nice to start landing those | 13:54 |
| SpamapS | I wanna rock right now now, I'm not internationally known, but I've been known to rebase a microphone | 13:55 |
| Shrews | RE-base... not ROB Base. silly | 13:55 |
| jeblair | Shrews: ++ on the pep8s | 13:56 |
| * Shrews watches the non-grey beards scratch their heads in confusion | 13:56 | |
| Shrews | SpamapS: jeblair: any idea what's up with this zuul test fail? http://logs.openstack.org/85/466285/1/check/gate-zuul-python27-ubuntu-xenial/946889c/testr_results.html.gz | 14:05 |
| Shrews | i guess that's a better question for jlk, actually | 14:05 |
| SpamapS | Shrews: I think it may be racey | 14:05 |
| SpamapS | I've seen it a couple of times now | 14:06 |
| SpamapS | also holy crap | 14:06 |
| jeblair | Shrews: yeah, it's been flaky lately. it times out while the test is running. i haven't looked enough yet to know if it's just slow or theres something more wrong. | 14:06 |
| SpamapS | I have never seen that testr results thing | 14:06 |
| SpamapS | how many years of time have I wasted not discovering it? | 14:06 |
| * SpamapS sees clock and goes to shuffle children to school | 14:06 | |
| *** rcarrillocruz has quit IRC | 14:09 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: fix imports in py3 https://review.openstack.org/463891 | 14:15 |
| pabelanger | morning | 14:17 |
| jeblair | pabelanger, Shrews: i've +2d all of SpamapS's zuul py3 stack if you want to hit some +3s | 14:19 |
| pabelanger | sure, let me grab a coffee and give a look | 14:19 |
| jeblair | Shrews, SpamapS: aha -- that test does have a race, i just caught it locally | 14:20 |
| mordred | jeblair: should we yaml.safe_dump in https://review.openstack.org/#/c/463897 (as a followup) | 14:30 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Ignore exceptions in fake nodepool https://review.openstack.org/466302 | 14:31 |
| jeblair | SpamapS, jlk, Shrews: ^ that should fix the flappy test_github_driver test | 14:32 |
| jeblair | mordred: it's "just tests" but probably a good idea to develop good habits and prevent copy pasta mistakes later :) | 14:33 |
| mordred | jeblair: ++ | 14:33 |
| mordred | cool - wasn't sure if it was an on-purpose use of dump | 14:34 |
| jeblair | mordred: on that note, if there *are* any on-purpose uses of dump/load, we should probably comment them explaining | 14:34 |
| mordred | jeblair: I +A'd through to "skip py3 failing tests" ... should we add a py3 unittest job to the gate before landing the next two? | 14:34 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use gear Text interface https://review.openstack.org/461468 | 14:34 |
| mordred | jeblair: ++ | 14:34 |
| mordred | jeblair: it's one of those places where the comment is useful - kind of like "yes, this dict as a default value for this parameter is not a mistake" | 14:35 |
| jeblair | mordred: pabelanger had a nv thing in progress i thought? | 14:35 |
| mordred | oh cool | 14:35 |
| mordred | jeblair: actually - you know - https://review.openstack.org/#/c/463903 could come with a .zuul.yaml addition of python3 test ... | 14:35 |
| jeblair | mordred: though SpamapS also has a zuul3 py3 job enabled at the end :) | 14:35 |
| jeblair | (that was his special surprise) | 14:36 |
| mordred | ok cool | 14:36 |
| Shrews | it was NOT a cookie, which disappointed me | 14:36 |
| pabelanger | nodepool have -nv python35 test now, and experimental dsvm-nodepool-py35 too | 14:37 |
| pabelanger | I also added tox-py35 for zuulv3, so .zuul.yaml change should work | 14:38 |
| jeblair | pabelanger: oh, it was nodepool not zuul -- do you want to add a zuulv2 py35 job for zuul too? | 14:38 |
| pabelanger | jeblair: sure | 14:38 |
| mordred | jeblair, SpamapS: stack approved - nice final patch! | 14:39 |
| mordred | pabelanger: that reminds me - I should add a py3 functional test for shade ... | 14:41 |
| *** hashar has joined #zuul | 14:41 | |
| pabelanger | jeblair: start out -nv or start to voting? | 14:41 |
| pabelanger | mordred: Ya, I am curious to see how nodepool does | 14:41 |
| jeblair | pabelanger: looks like maybe start out voting :) | 14:42 |
| mordred | pabelanger: that'll be a good py3 test for shade too | 14:42 |
| pabelanger | jeblair: ack | 14:42 |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Use six.reraise for python3 https://review.openstack.org/463594 | 14:44 |
| Shrews | pabelanger: why do we need to wrap map() in list() for https://review.openstack.org/466069 if we don't use the return value? | 14:48 |
| pabelanger | Shrews: For some reason, python3 wouldn't run the map() function and wrapping it as a list was the only way I got it working | 14:48 |
| Shrews | heh. new one to me | 14:49 |
| pabelanger | when I looked at docs, it wrapped list(map()) when passing a list to the function | 14:49 |
| pabelanger | https://docs.python.org/3/howto/functional.html#built-in-functions | 14:50 |
| pabelanger | however, that did return a result | 14:50 |
| pabelanger | so, happy to update it, if there is a better way | 14:51 |
| Shrews | nah, it's fine. i just had never encountered that one before | 14:51 |
| pabelanger | ya, took a while to figure out why our diskimages were not getting deleted | 14:53 |
| *** dkranz has joined #zuul | 15:05 | |
| mordred | pabelanger: what's your nodepool dsvm python3 job? | 15:05 |
| pabelanger | gate-dsvm-nodepool-py35-src | 15:07 |
| pabelanger | http://git.openstack.org/cgit/openstack-infra/project-config/tree/jenkins/jobs/nodepool.yaml#n64 | 15:07 |
| pabelanger | however, I see a typo | 15:07 |
| pabelanger | let me fix real quick | 15:07 |
| *** hashar has quit IRC | 15:13 | |
| pabelanger | any objections if I manually update main.yaml on zuulv3-dev.o.o? Would like to pull in https://review.openstack.org/#/c/466021/ to add nodepool to projects list | 15:14 |
| SpamapS | I figured a voting test in zuulv3-dev would be a sort of medium strength backstop | 15:23 |
| SpamapS | I probably should add a full gate/check in project-config though. | 15:23 |
| SpamapS | this one is blowing up our pass rate.. https://review.openstack.org/466302 Needs a second +2 | 15:25 |
| SpamapS | mordred: pabelanger ^ | 15:25 |
| SpamapS | That approved stack will take a bit to land w/o it. | 15:25 |
| SpamapS | jlk: https://review.openstack.org/465912 <-- might be of interest to you (Dockerfile from tobiash) | 15:27 |
| SpamapS | alpine based.. sexy | 15:28 |
| SpamapS | https://pkgs.alpinelinux.org/package/edge/community/x86_64/bubblewrap <-- yay | 15:28 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: WIP: Add non-voting tox-py35 job https://review.openstack.org/466324 | 15:30 |
| pabelanger | neat, ^ cased an exception in zuulv3 | 15:32 |
| pabelanger | but didn't get posted back to gerrit | 15:33 |
| jeblair | pabelanger: what's the error? | 15:33 |
| pabelanger | http://paste.openstack.org/show/610054/ | 15:33 |
| jeblair | pabelanger: https://review.openstack.org/466079 btw | 15:34 |
| pabelanger | Oh | 15:35 |
| pabelanger | :) | 15:35 |
| jeblair | still, let's see if that reporting error persists | 15:35 |
| SpamapS | Would be very handy to see that comment in Gerrit | 15:36 |
| jeblair | yeah, it tried, but failed | 15:36 |
| jeblair | so either gerrit flaked, or there's something wrong with the gerrit command. for instance, i don't think we've sent many messages with "<>" in them to date. | 15:36 |
| jeblair | yeah, it failed twice, i think there's an actual bug | 15:38 |
| jeblair | also it has double quotes | 15:39 |
| jeblair | i think it's the quotes | 15:40 |
| jeblair | (i just manually posted the comment by escaping them) | 15:40 |
| mordred | yay bug! | 15:41 |
| SpamapS | That sounds like something our gerrit reporter could do automatically. | 15:42 |
| jeblair | yep | 15:42 |
| jeblair | https://docs.python.org/dev/library/shlex.html#shlex.quote | 15:42 |
| jeblair | we're about to become python3 only anyway, right? :) | 15:43 |
| jeblair | https://docs.python.org/2/library/pipes.html#pipes.quote | 15:43 |
| jeblair | wonder if there's a six for that | 15:44 |
| jeblair | yep | 15:44 |
| jeblair | from six.moves import shlex_quote | 15:44 |
| SpamapS | six and a six pack are all you need to port to python3 anymore ;) | 15:46 |
| *** DangerousDaren has quit IRC | 15:49 | |
| jeblair | this is weird. if you use the 'gerrit review' ssh command with the '--message' and '--submit' args, you can leave a message, vote, and submit all in one command. but if you use '--json' instead of '--message', you can't submit in the same command (meaning there will be two gerrit events). this is the same behavior as the rest api, perhaps not coincidentally. | 16:13 |
| jeblair | (i'm looking into the --json argument because shlex_quote ends up putting in too many quotes for gerrit. also, --json lets us do file-level comments which is something i want to add anyway) | 16:14 |
| SpamapS | another fail to land py3k due do 465912 not landing yet. | 16:15 |
| jeblair | SpamapS: i hope we don't need a dockerfile to land changes | 16:16 |
| SpamapS | err | 16:16 |
| SpamapS | 466302 | 16:16 |
| * SpamapS eye fail | 16:16 | |
| SpamapS | but yeah, 465912 is _CRITICAL_ ;) | 16:17 |
| jeblair | i +3d the test fix | 16:19 |
| SpamapS | hooray | 16:21 |
| jeblair | at any rate, i'm somewhat inclined to proceed with --json and start accepting that we may have a two-event review-and-merge process | 16:22 |
| jeblair | though... i bet that is going to break *all of* the tests. | 16:23 |
| jeblair | self.assertEqual(len(A.reported), 2) --> self.assertEqual(len(A.reported), 3) | 16:23 |
| mordred | jeblair: --json seems like it's more compelling than two-event is yucky - although the test update patch is totally gonna blow | 16:24 |
| SpamapS | I concur with both of your assessments. | 16:24 |
| mordred | jeblair: we could also use both ... it's rare that you'd want to report exceptions or leave per-line comments when you're submitting | 16:25 |
| mordred | so use --message --submit for submitting successfully - and use json for all the times we're not submitting | 16:26 |
| jeblair | yeah...okay. so i made a commitment to reduce the review queue this week. json is going to be a rabbit hole. i'll shelve it for now, and maybe just do a simple re.sub on " to fix that bug for now. | 16:26 |
| mordred | jeblair: ++ | 16:26 |
| jeblair | mordred: that's a possibility... though all things being equal, i'd like to keep things flexible for the time where we do want to do a line review on a submit. :) | 16:26 |
| * SpamapS notices the rabbits have been busy in our garden this week | 16:27 | |
| * jeblair is not sure where the metaphors begin and end | 16:28 | |
| jeblair | oh wait, i think shlex may work after all | 16:30 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Quote message when reporting to Gerrit https://review.openstack.org/466345 | 16:33 |
| jeblair | SpamapS, mordred, pabelanger: ^ apparently that does work | 16:33 |
| jeblair | neat. we landed our voting python3 job before we finished landing the python3 fixes. | 16:35 |
| jeblair | pabelanger: ^ | 16:37 |
| pabelanger | oh | 16:37 |
| pabelanger | let me revert for now | 16:37 |
| jeblair | pabelanger: i'm on it | 16:37 |
| pabelanger | k | 16:37 |
| pabelanger | jeblair: did you see my comment before about adding nodepool project to zuulv3-dev.o.o? Any objections? | 16:38 |
| *** rcarrillocruz has joined #zuul | 16:38 | |
| jeblair | pabelanger: i'm still not in a rush to do that | 16:38 |
| pabelanger | k, I was thinking it would be a good project to start working on devstack job | 16:38 |
| pabelanger | atleast a simple wrapper job to current bash setup | 16:39 |
| jeblair | pabelanger: i'd like to focus more on getting the basic jobs set up and the standard library first | 16:39 |
| pabelanger | k | 16:39 |
| pabelanger | I have a few patches going for openstack-zuul-jobs topic | 16:39 |
| SpamapS | heh, and that's why I started with a .zuul.yaml python3 job first ;-) | 16:39 |
| jeblair | i'd like us to set up the other job/role repos we talked about. until we do that, we're forcing the bonnyci folks to have their own forks, which is not good | 16:39 |
| pabelanger | right now getting docs sphinx publisher doing | 16:39 |
| pabelanger | right, I think we said zuul-base-job as the new repo? | 16:40 |
| jeblair | pabelanger: yeah, i think zuul-jobs for 'tox', and zuul-base-jobs for 'base'. (probably still plural for consistency even if we think it'll only have one job) | 16:41 |
| jeblair | SpamapS, mordred: sound about right ^? | 16:41 |
| pabelanger | is there a way to have no_log: True only log on a failure of the task? | 16:51 |
| *** jkilpatr has quit IRC | 16:58 | |
| SpamapS | jeblair: so the idea is that we can reuse tox, but don't have to reuse all the infra-isms in zuul-base-jobs? | 17:00 |
| pabelanger | does bonnyci have a tox jobs today? | 17:04 |
| pabelanger | Trying to understand what bits you wouldn't want from http://git.openstack.org/cgit/openstack-infra/project-config/tree/jenkins/scripts/run-tox.sh | 17:05 |
| jeblair | i'll write some of this up in an etherpad, gimme a min. | 17:05 |
| *** jkilpatr has joined #zuul | 17:11 | |
| jeblair | SpamapS, pabelanger: this is what i'm thinking: https://etherpad.openstack.org/p/zuul-jobs | 17:11 |
| jeblair | pabelanger: there's a whole bunch of testr stuff in there. that shouldn't be in a tox job in zuul-jobs (tox != testr). so, if possible, we should have zuul-jobs/tox "just run tox" or as close to that as we can get. then we should have openstack-zuul-jobs/tox-testr add our testr stuff (or maybe that's useful enough to put in zuul-jobs). then openstack-zuul-jobs/tox-py35 should inherit from that and set the environment name. | 17:14 |
| jeblair | pabelanger: er, the context of my last statement is the run-tox.sh script you asked about :) | 17:14 |
| SpamapS | I haven't looked at what jamielennox has setup in our zuulv3 just yet. | 17:17 |
| mordred | jeblair: agree with the general description. fwiw - I think "process testr results" is potentially a post-process role that can be in zuul-jobs that tox-testr uses, but could just as easily be used if someone did a non-tox-based testr job | 17:22 |
| jeblair | mordred: that makes sense | 17:23 |
| mordred | jeblair: I'm saying that just in service of thinking about a general catalog of jobs and job pieces ... I don't think a ton of people are going to rush up wanting testr post-processing | 17:23 |
| jeblair | pabelanger: i'm confused by 466049. in your commit message, you write about wrapping keys() in list() because keys() returns an iterator now. but there are only a couple of places where that's actually necessary (most of those changes are areas where we're already iterating). but the subject of the commit is "Python3: RuntimeError: dictionary changed size during iteration". that makes me think that you saw some place where we were relying on ... | 17:25 |
| jeblair | ... freezing the list of keys. | 17:25 |
| jeblair | pabelanger: do all of the instances you changed really need that? | 17:25 |
| SpamapS | ehh | 17:27 |
| pabelanger | jeblair: no, I did a grep / regex match. I can go back and validate which ones were causing issues | 17:27 |
| SpamapS | jeblair: http://logs.openstack.org/02/466302/1/gate/gate-zuul-python27-ubuntu-xenial/03dac6a/testr_results.html.gz | 17:27 |
| SpamapS | Doesn't fix the problem unfortunately | 17:28 |
| SpamapS | I think that test might be bonghits | 17:28 |
| jeblair | pabelanger: okay, that's probably worth doing | 17:28 |
| jeblair | SpamapS: oh, that may still be the same problem | 17:29 |
| SpamapS | or there's a real problem that the driver is exposing | 17:29 |
| SpamapS | did not get the Disappeared debug log | 17:29 |
| jeblair | oh, hrm | 17:30 |
| SpamapS | well | 17:31 |
| SpamapS | it may be the same problem from another angle | 17:31 |
| SpamapS | whatever failure there is, some cleanup gets skipped I think | 17:31 |
| jeblair | (i wish we logged what node was still around) | 17:31 |
| SpamapS | since we're not going to fulfill requests | 17:32 |
| jeblair | node request 100-0000000004 was canceled | 17:32 |
| SpamapS | jeblair: what if you wrapped the fullfillRequest call in that handler instead of _run? | 17:32 |
| SpamapS | so the loop finishes | 17:33 |
| SpamapS | I think that may be it actually | 17:33 |
| jeblair | it should start over again | 17:33 |
| SpamapS | we're left with several cancelled disappeared nodes | 17:33 |
| SpamapS | unless _running is already false | 17:33 |
| jeblair | yeah, i think that's the case -- we created a node for a node request, but then left it sitting around | 17:34 |
| jeblair | that loop doesn't clean up such nodes, however, i think perhaps it should | 17:34 |
| *** jkilpatr_ has joined #zuul | 17:36 | |
| SpamapS | it's the requests that's causing the state assertion fail though, so perhaps we also just need to make sure we still "fullfill" the request on disappeared node? | 17:36 |
| SpamapS | like maybe the handler needs to be even deeper | 17:37 |
| *** jkilpatr has quit IRC | 17:38 | |
| jeblair | SpamapS: well, the disappearing in this case is the request, so 0004 (which was canceled) should not show up in the list | 17:38 |
| jeblair | which is why i'm curious what request was still there | 17:38 |
| jeblair | looks like 0006 from the log lines above it | 17:39 |
| SpamapS | so | 17:42 |
| SpamapS | _false is already set | 17:42 |
| SpamapS | we call shutdown before assertFinalState | 17:42 |
| jeblair | SpamapS: there wasn't an exception though, so we didn't exit that loop prematurely | 17:43 |
| SpamapS | perhaps we didn't even make it through the loop at all before the test ended? | 17:44 |
| jeblair | also, scratch what i said about cleaning up in the loop, that didn't make sense | 17:44 |
| jeblair | SpamapS: what i've gathered from the logs is: the 0006 request still existed when the test shutdown. it had previously been fulfilled by the fake nodepool. this means that zuul ignored the completed request. that smells like a real bug. | 17:46 |
| jeblair | "Resubmitting lost node request <NodeRequest 100-0000000004 <NodeSet OrderedDict()>>" is also very fishy. | 17:46 |
| jeblair | i wonder if 0006 is 0004 resubmitted | 17:47 |
| SpamapS | indeed feels like a real race | 17:49 |
| SpamapS | not just a testing one | 17:49 |
| SpamapS | perhaps something needs to be paused? | 17:49 |
| jeblair | i think i see the race: http://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/nodepool.py?h=feature/zuulv3#n38 | 17:51 |
| jeblair | context switch between lines 42 and 45 means we can get the watch notification and run this entire function before switching back: http://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/nodepool.py?h=feature/zuulv3#n93 | 17:52 |
| jeblair | line 98 being the key there | 17:52 |
| *** Cibo_ has joined #zuul | 17:52 | |
| SpamapS | would be nice to have a log of that broad except Exception: | 17:52 |
| jeblair | SpamapS: on line 44? we would if we hit it. | 17:53 |
| SpamapS | oh I read that wrong right | 17:53 |
| jeblair | i think the fix is to put line 45 after line 40 | 17:54 |
| SpamapS | so does self.requests access need to be serialized? | 17:54 |
| *** tobiash_ has joined #zuul | 17:54 | |
| SpamapS | jeblair: agreed, that would allow us to keep it lock free since the other side only needs read access | 17:55 |
| jeblair | ya. a lock would work but i don't think we need it | 17:55 |
| jeblair | SpamapS: hrm, there's also a del self.requests in a different code path in the watch function | 17:56 |
| jeblair | that could race too | 17:57 |
| SpamapS | right I just notied that too | 17:57 |
| jeblair | we could just wrap those in try/except | 17:57 |
| SpamapS | but I think deleting concurrently is ok | 17:57 |
| SpamapS | oh yeah they both might try | 17:57 |
| SpamapS | so yeah making the deletes handle a missing one would again keep it lock free | 17:58 |
| SpamapS | since you don't care who deletes it, as long as one of them does. | 17:58 |
| jeblair | yeah, if we got notification the request was fulfilled and right before we sent it back to the scheduler, we decided to cancel the request | 17:58 |
| jeblair | ^ is the race path | 17:58 |
| * SpamapS grumbles Rust | 17:58 | |
| jeblair | SpamapS: yep. part of why i want to try extra hard to avoid locks is that _updateNodeRequest is in the main ZK thread, so any time we spend in there our interaction with ZK is paused | 17:59 |
| jeblair | (onNodesProvisioned just adds an event to the scheduler queue for it to deal with the next time around, it doesn't do any real work) | 17:59 |
| jeblair | oh, what if we made the delet inside of the watch callback the only delete | 18:01 |
| jeblair | we can add another way to check that the request is canceled, like a flag on the request itself. that might be tidier? | 18:02 |
| SpamapS | jeblair: I like single-writer-thread paradigms. | 18:07 |
| SpamapS | jeblair: however, I also think just not caring if the delete fails is sufficient here. | 18:09 |
| *** harlowja has joined #zuul | 18:13 | |
| tobiash_ | hi | 18:23 |
| jeblair | SpamapS: in looking at this, i also see another potential problem -- once we have handed over the node request to the scheduler, it's going to expect to delete it. but if we cancel it after we submit that event to the scheduler, the cancel method may delete it first. that should be fine -- it'll just log an exception and mark the request failed (we canceled it, so we don't care anyway). but it's still a bit messy, and i think the only way ... | 18:23 |
| jeblair | ... around that is a lock. | 18:23 |
| tobiash_ | are there any objections enriching some of the logging in zuul with the extra keyword (https://docs.python.org/2/library/logging.html#logging.debug)? | 18:24 |
| tobiash_ | when using log formatters using addition meta data from that it could be simplified to e.g. trace the lifecycle of a change in zuul via e.g. kibana | 18:25 |
| tobiash_ | s/addition/additional/ | 18:25 |
| tobiash_ | same might be handy for tracing the lifecycle of nodes in nodepool | 18:26 |
| SpamapS | jeblair: agreed, messy cleanup, or locking, is the choice :-P | 18:27 |
| jeblair | tobiash_: well, i think all of that information should be in the log message itself. if adding it to extras helps systems like kibana, i think that's fine, but we should make sure it ends up in the formatted message too (i know that we can use the extras in a formatter defined in the standard logging config, but i'd prefer to keep that as simple as possible, and not dependent on the code) | 18:30 |
| jeblair | (maybe we should add some logging helper functions for change or node related messages) | 18:32 |
| tobiash_ | jeblair: that sounds good | 18:32 |
| tobiash_ | I have the feeling (will have to verify that) that when I look at the filtered log for a change number in kibana I see less then I would watch directly the debug logging. | 18:34 |
| tobiash_ | especially trying to figure out why something was not triggered as expected | 18:34 |
| SpamapS | Indeed, a few things could probably use some info logging love. | 18:35 |
| SpamapS | Which is probably the real reason those bits are missing, not that the message is there, but missing an id. | 18:35 |
| SpamapS | tobiash_: I like the idea though, of having context extras | 18:36 |
| tobiash_ | I also would like to have both (filtering by extras often produces less noise like matches in timestamps) | 18:37 |
| pabelanger | jeblair: mordred: did we have a patch up for zuul console to redirect stdout for shell? | 18:38 |
| pabelanger | right now we do not see that in ansible_log.txt | 18:38 |
| tobiash_ | also often when processing one change often dependents or dependees are mentioned in the log too which is easier to filter by metadata than text matching | 18:38 |
| jeblair | pabelanger: zuul_stream callback plugin should do it | 18:42 |
| SpamapS | I thought we had a patch up for 'command' | 18:42 |
| SpamapS | but didn't see it for 'shell' | 18:42 |
| SpamapS | https://review.openstack.org/460753 | 18:43 |
| pabelanger | maybe that is it. This is shell | 18:43 |
| pabelanger | but, I really should be using command I think :) | 18:43 |
| jlk | only 300+ unread messages in this channel Somewhere somebody talked to me. | 18:43 |
| SpamapS | yes you should. :) pwn your environment. :) | 18:44 |
| SpamapS | jlk: that was me, pointing at a patch to add a Dockerfile to zuul from tobiash_ | 18:44 |
| SpamapS | and we might have cursed your name while jeblair found a race in nodepool.py exposed by the github tests | 18:44 |
| jlk | is that nodepool bug my fault? | 18:46 |
| pabelanger | jeblair: is it a lot of work to support bidirectional CRD? | 18:47 |
| jlk | oh doesn't look like it | 18:47 |
| jeblair | pabelanger: more than we're going to do before 3.0, and we will probably not turn it on for openstack. | 18:52 |
| tobiash_ | btw, thanks for the comments on the dockerfile patch | 18:53 |
| tobiash_ | where should I move it to? | 18:53 |
| * jlk still needs to look at that | 18:53 | |
| tobiash_ | tools/docker/ ? | 18:53 |
| SpamapS | I think a contrib/docker makes more sense. | 18:53 |
| SpamapS | maybe even contrib/docker/alpine | 18:54 |
| tobiash_ | ok, so contrib would be new | 18:54 |
| tobiash_ | ok, I'm fine with that | 18:54 |
| pabelanger | jeblair: what is the reason for not enabling it in openstack? | 18:55 |
| SpamapS | Yeah, it's a pretty standard place to stuff "things that help us integrate with other projects" | 18:55 |
| tobiash_ | same for the nodepool docker file (https://review.openstack.org/#/c/465852/)? | 18:55 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Improve safety around canceling node requests https://review.openstack.org/466302 | 19:01 |
| jeblair | SpamapS, jlk: ^ single patch double fix | 19:02 |
| jeblair | pabelanger: to force stepwise upgrades | 19:03 |
| jeblair | tobiash_: yes please | 19:04 |
| tobiash_ | jeblair: ok, will update both on monday probably | 19:04 |
| pabelanger | jeblair: makes sense, thanks | 19:04 |
| SpamapS | jeblair: nice and succinct | 19:05 |
| pabelanger | so, I just push up a few changes to our openstack-zuul-jobs / openstack-zuul-roles (this are specific to openstack-infra) but I've started making them more generic so we can start thinking of moving some thing into zuul-jobs: https://review.openstack.org/#/q/topic:openstack-zuul-jobs | 19:06 |
| pabelanger | major changes are starting to be explicit about passing variables from playbooks to roles | 19:06 |
| jeblair | pabelanger: cool. also, we should set gerritbot notifications in here on zuul-jobs and zuul-base jobs when we create those. we can probably continue to ignore openstack-zuul-jobs. | 19:06 |
| jeblair | Shrews, mordred: can you review https://review.openstack.org/466302 ? | 19:08 |
| pabelanger | jeblair: sure, I'll get a patch up for zuul-jobs / zuul-base-job(s) shortly | 19:08 |
| pabelanger | then we can bikeshed on naming | 19:08 |
| Shrews | doing so now | 19:08 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Also send stdout back in the Result object https://review.openstack.org/460753 | 19:08 |
| pabelanger | I wouldn't mind also upgrading zuulv3-dev.o.o to master to get ^ | 19:10 |
| Shrews | jeblair: +0 comment there | 19:11 |
| jeblair | Shrews: derp, thx | 19:12 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Improve safety around canceling node requests https://review.openstack.org/466302 | 19:12 |
| jeblair | okay, please +3 that one; once it lands, we can recheck the entire py3 stack and get it merged | 19:14 |
| SpamapS | even less lines :) | 19:14 |
| Shrews | done | 19:14 |
| jlk | *fewer | 19:15 |
| SpamapS | jlk: thanks, nerd. ;) | 19:15 |
| *** Cibo_ has quit IRC | 19:15 | |
| * SpamapS needs to get back to executor-security soon :-P | 19:16 | |
| * jeblair feeds | 19:16 | |
| jlk | :D | 19:16 |
| SpamapS | pabelanger: btw, I looked into paramiko's docs and I think the ssh-agent stuff they do isn't really helpful | 19:17 |
| SpamapS | pabelanger: they don't actually manage ssh-agent, they just help paramiko threads connect to it | 19:17 |
| jlk | At blue box, somebody wrote a ssh agent muxer | 19:18 |
| jlk | that might be of use. | 19:18 |
| jlk | https://github.com/blueboxgroup/sshagentmux | 19:19 |
| jlk | though I have no idea if it solves any of hte problems you're trying to solve | 19:19 |
| pabelanger | SpamapS: that is what I seen too. I wonder if that is something we should ask upstream if there is any interested in adding it? | 19:23 |
| SpamapS | jlk: no it's not the same problem | 19:23 |
| SpamapS | this one is actually 1:1 | 19:23 |
| SpamapS | agent <-> build | 19:23 |
| SpamapS | pabelanger: yeah I dunno, it feels generic, but I also don't know how many people want to spin up _multiple_ ssh agents :) | 19:24 |
| pabelanger | also, I take it since nobody objected I'll proceed with zuulv3-dev update to master in 20mins | 19:24 |
| pabelanger | SpamapS: true | 19:25 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Improve safety around canceling node requests https://review.openstack.org/466302 | 19:26 |
| SpamapS | yay | 19:28 |
| * SpamapS rechecks py3k | 19:28 | |
| jlk | geez, it's no wonder I struggled so much with the patch series. | 19:29 |
| SpamapS | jlk: honestly.. races like those are why I get silly about Rust all the time. | 19:31 |
| SpamapS | Rust's only concurrency problem now is there's simply no way to detect deadlocks. | 19:31 |
| SpamapS | (all languages have that) | 19:31 |
| SpamapS | but I digress.. yay python3 | 19:32 |
| * SpamapS wanders off to feed as well | 19:34 | |
| *** tobiash_ has quit IRC | 19:39 | |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: py3 Changes in __del__ for gitpython https://review.openstack.org/463892 | 19:45 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: py3 hashlib error https://review.openstack.org/463893 | 19:48 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: make Job and ZuulRole hashable https://review.openstack.org/463894 | 19:53 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: None does not compare to int https://review.openstack.org/463895 | 19:53 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fixes for test_model in py3 https://review.openstack.org/463896 | 19:53 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: writing yaml to disk needs bytes https://review.openstack.org/463897 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: view changes for py3 https://review.openstack.org/463898 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Encoding changes in tests for py3 https://review.openstack.org/463899 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: base64 changes for py3 https://review.openstack.org/463900 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: misc py3 changes https://review.openstack.org/463901 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: fix webapp tests for py3 https://review.openstack.org/463902 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Explicitly decode decrypted secrets for py3 https://review.openstack.org/464049 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: skip py3 failing tests https://review.openstack.org/463903 | 19:54 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix github driver tests for py3 https://review.openstack.org/466078 | 19:55 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Enable py3 tests https://review.openstack.org/466079 | 19:55 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove pipeline argument from various report fncts https://review.openstack.org/464252 | 19:56 |
| jlk | lordy that's a lot of merges | 19:56 |
| pabelanger | okay, starting upgrade of zuulv3-dev | 19:57 |
| pabelanger | and started again | 20:01 |
| jlk | jeblair: were you going to continue hacking on https://review.openstack.org/#/c/466105/ today? Do you think it needs any major structural changes, or should I be able to start porting my later changes on this structure? | 20:07 |
| jeblair | jlk: yeah, i'll clean it up in a few. i *think* it should be relatively safe to start porting, if the api lgty. | 20:10 |
| SpamapS | wewt | 20:23 |
| SpamapS | pabelanger: we can revert your revert too | 20:23 |
| SpamapS | and start gating on py3 | 20:23 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Add driver-specific pipeline requirements https://review.openstack.org/466105 | 20:24 |
| *** jkilpatr_ has quit IRC | 20:24 | |
| jeblair | i unwiped https://review.openstack.org/466349 | 20:24 |
| SpamapS | I have to say... the experience of editing your own .zuul.yml to run jobs is.. fantastic. | 20:25 |
| pabelanger | +3 on revert of python35 zuul job | 20:28 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Quote message when reporting to Gerrit https://review.openstack.org/466345 | 20:44 |
| jeblair | jlk: woot 466105 is clean now | 20:44 |
| jeblair | (even passes py3) | 20:44 |
| * SpamapS is going to start defaulting to -epy35 | 20:45 | |
| *** jkilpatr has joined #zuul | 20:45 | |
| jeblair | SpamapS: yeah, also, i think we should move openstack-infra to running it in py3 and drop the py2 tests before too long. once we land the websocket/asyncio stuff, at least part of this will effectively be a py3-only app. | 20:47 |
| mordred | jeblair: ++ | 20:48 |
| SpamapS | jeblair: indeed, I'll start the wheels on that for Bonny too | 20:48 |
| mordred | jeblair: I can't think of any reason to keep the other version | 20:48 |
| jeblair | mordred: it's nice being an app, not a library :) | 20:48 |
| mordred | ++ | 20:48 |
| mordred | we should double-check with tobiash to make sure that won't screw him before we pull the trigger- but he seems to be using a docker container, so I imagine it should be non-hard | 20:49 |
| jeblair | mordred: i think you just docker a new docker, right? | 20:50 |
| jeblair | pabelanger: got a second to talk about jobs? | 20:52 |
| mordred | jeblair: that's my understanding, yes | 20:52 |
| pabelanger | jeblair: sure | 20:53 |
| jeblair | pabelanger: i'm looking at 466332 and 466330, and that adds a net-info role to the base job. the net-info role is openstack specific. | 20:53 |
| jeblair | pabelanger: so on one hand, it's fine to add that to openstack-zuul-jobs. but if you're planning on staging things there and moving them into zuul-base-jobs later, then that's a layer violation | 20:53 |
| jeblair | pabelanger: so how would you like to proceed? review for layer violations now, or later when you move stuff out? | 20:54 |
| pabelanger | what is a layer violation is this context | 20:54 |
| jeblair | pabelanger: adding something openstack-specific to something that shouldn't be. | 20:55 |
| jeblair | pabelanger: lowest layers should be usable by everyone, higher layers are more site-local. | 20:55 |
| pabelanger | so, I was going to add host-info for zuul-jobs which contained things like executor info | 20:55 |
| pabelanger | and keep net-info as openstack specific | 20:56 |
| pabelanger | maybe we should rename it to openstack-info and zuul-info roles | 20:56 |
| jeblair | that sounds safer (we want to assume as little as possible about the host in the base job) | 20:58 |
| jeblair | (like, not even assume it's running linux) | 20:58 |
| pabelanger | ya | 20:58 |
| pabelanger | okay, I'll rename net-info to openstack-info | 20:58 |
| jeblair | (we will assume we can rsync data on/off the host, but that's about it. if we can't rsync, *that's* the sort of case where someone should build their own base job :) | 20:59 |
| jeblair | pabelanger: i suspect for openstack we won't use the base job defined in zuul-base-jobs; we'll just use the roles from it. | 21:05 |
| pabelanger | ya | 21:05 |
| jeblair | (so this is probably going to be weird until we actually make the zuul-base-jobs repo) | 21:05 |
| jeblair | pabelanger: why no_log in 466352+ | 21:08 |
| jeblair | ? | 21:08 |
| pabelanger | jeblair: less logging for rsync | 21:09 |
| pabelanger | but I can remove it | 21:09 |
| pabelanger | I'd actually like to only log if an error | 21:09 |
| jeblair | pabelanger: oh, got it. yeah i hate those. | 21:09 |
| * SpamapS EOD/EOW's | 21:14 | |
| SpamapS | jeblair: thanks for ramming py3 in! It's great to see progress on that. | 21:14 |
| jeblair | SpamapS: thank you! | 21:14 |
| jeblair | pabelanger: regarding 466360, remind me why we can't do http://paste.openstack.org/show/610071/ ? | 21:15 |
| *** yolanda has quit IRC | 21:18 | |
| pabelanger | jeblair: I think we'll leak variables from the role into the playbook. It seems that include_role will load the role for that task, then remove it | 21:19 |
| jeblair | that seems counter-intuitive to me :( | 21:19 |
| *** yolanda has joined #zuul | 21:20 | |
| pabelanger | which, I think may provide better protection for variable scoping | 21:20 |
| pabelanger | my logic is, I don't want a role to set a variable, that is consumed from another role / playbook | 21:20 |
| jeblair | pabelanger: sure, i just don't understand why it would be scoped like that | 21:20 |
| pabelanger | if we wanted to do that, we could use facts | 21:20 |
| pabelanger | let me get the docs link I was reading | 21:21 |
| pabelanger | jeblair: also, if we use include_role we can do them for pre and post tasks. Something we might want to do for a playbook | 21:22 |
| pabelanger | I'm a fan of treating roles more like a task :) | 21:23 |
| jeblair | pabelanger: i want our basic jobs to be as easy to read as jjb. because i want people who don't understand much about ansible to be able to edit them in the same way they do jjb. | 21:23 |
| jeblair | otherwise, all of this will end up making *more* work for us, not less | 21:24 |
| jlk | I"d been meaning to ask, why is everything a role in zuul land? | 21:24 |
| jlk | like, single-task roles are weird | 21:24 |
| pabelanger | jlk: easy mode until we import all of JJB, then we could refactor | 21:24 |
| pabelanger | jeblair: Right, I've found include_role to be straightforward. But that said, I was considering using something like ansible-review to help users write / edit our playbooks. It offers up an interface to write syntax checks for things | 21:26 |
| pabelanger | we'd basically create rules for checks: https://github.com/willthames/ansible-review | 21:27 |
| jeblair | pabelanger: that does not match my goals for zuulv3 for openstack. it's *really* important to me that people with almost no understanding of ansible be able to be at least as successful editing jobs as they are with jjb and project config. | 21:27 |
| jeblair | pabelanger: honestly, i think that's a pretty low bar. :) | 21:27 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Remove pipeline argument from various report fncts https://review.openstack.org/464252 | 21:29 |
| jeblair | pabelanger: i think my suggestion for role arguments will work | 21:33 |
| jeblair | pabelanger: https://etherpad.openstack.org/p/hImFwPbSD1 | 21:33 |
| jeblair | pabelanger: that shows the role using the variable, but it not leaking into the task which follows | 21:35 |
| pabelanger | jeblair: Hmm | 21:36 |
| jeblair | pabelanger: i also tried it with the same role repeated but no variable, that also correctly errored | 21:36 |
| jeblair | and of course, if i remove the {{foo}} it works | 21:36 |
| pabelanger | let me test again locally | 21:36 |
| pabelanger | it used to leak | 21:37 |
| pabelanger | also, there is another use case for include_role, which my commit didn't reference. If we need to run a task between 2 role, using roles setting doesn't allow that | 21:37 |
| pabelanger | since roles and task run at different stages | 21:37 |
| mordred | yes. that is the main use-case for include_role in my brainhole - to be able to intersperse roles and tasks in a defined sequence | 21:38 |
| pabelanger | so, with that in mind, I've just defaulted to include_role for playbooks to pick one way for doing them | 21:39 |
| jeblair | yeah. i don't think we need to do that in these fundamental jobs though | 21:39 |
| jeblair | i'd rather we use the easiest way for novices to edit. either a list of roles or a list of tasks is fine. using both is complex. but tasks with include_role are so much more complicated than a list of roles. | 21:40 |
| jlk | you mean you don't want people to have to understand pre_tasks, roles, tasks, post_tasks, and how handlers interact with all of those? | 21:41 |
| jeblair | jlk: right. we're throwing enough new zuul stuff at them already (though i have tried really hard to make all that just dtrt 90% of the time if you edit it without understanding). :) | 21:42 |
| jlk | i get it. I honestly have to go back to my book when I'm sorting out order of operations. I can't remember it all the time. | 21:43 |
| jeblair | ansible is really powerful, and i'm sure we're going to have jobs that end up using some pretty snazzy ansible stuff because it's actually easier. but the challenge for us setting up these jobs is to make them so that someone who just wants to add a new tox job to their project can do it without asking us how. :) | 21:44 |
| jlk | bingo | 21:50 |
| jlk | We in Bonny land created a base job that would just execute script in a known path. Super quick-start. | 21:50 |
| pabelanger | jeblair: okay, etherpad updated with leaked vars | 21:54 |
| jeblair | pabelanger: help me out there -- what's leaking? | 21:54 |
| pabelanger | jeblair: blah variable | 21:54 |
| pabelanger | coming from roles/testrole/defaults/main.yaml | 21:54 |
| pabelanger | first play shows leak, 2nd protects | 21:55 |
| pabelanger | with include_role | 21:55 |
| jeblair | pabelanger: so setting defaults leaks | 21:55 |
| jeblair | not role arguments | 21:56 |
| pabelanger | and vars | 21:56 |
| pabelanger | vars.yaml | 21:56 |
| pabelanger | yes... | 21:56 |
| pabelanger | so, some places we do setup defaults, to be sane | 21:56 |
| jeblair | we could not do that. i'm not sure a run-tox role needs a default. | 21:56 |
| pabelanger | we could stop doing that, and force all playbooks to pass variables | 21:57 |
| pabelanger | I think if we default to include_role, and standardize on that, we avoid the problem of roles have defaults or not. In both cases, we protect the variable leak. However, it means jobs require a little more code | 21:59 |
| pabelanger | and, from what I see with project-config patches, most people copypasta things anyways | 21:59 |
| jeblair | 2.5x the number of lines | 22:00 |
| pabelanger | it looks like private: true / false doesn't matter either | 22:02 |
| pabelanger | when I toggled it, nothing happend | 22:02 |
| pabelanger | so maybe a bug | 22:02 |
| jlk | so I'm reluctant on include_role | 22:06 |
| jlk | it's still tech preview, and there's been numerous bugs with it | 22:06 |
| jeblair | i think as long as roles are the unit of composability we're working with, we should make it as easy as possible to use them. i think role arguments are more easily read and understood. | 22:06 |
| jlk | "This module is flagged as preview which means that it is not guaranteed to have a backwards compatible interface." | 22:06 |
| jeblair | (as an aside, the first backwards-incompatible change we have to make over 1800 repositories is going to be... what's the word... fun?) | 22:07 |
| jlk | that is certainly A word | 22:07 |
| pabelanger | yes, from what I have seen, each release of ansible required playbook / role changes. So, tech preview / stable, something has broken me. However, I know this will get better over time | 22:08 |
| * SpamapS isn't here | 22:09 | |
| SpamapS | I'm guessing we'll eventually have to allow jobs to specify their ansible version and install multiple ansibles. | 22:09 |
| SpamapS | we should fight against that tooth and nail | 22:09 |
| jlk | https://github.com/ansible/ansible/issues?utf8=✓&q=is%3Aissue%20is%3Aopen%20include_role 48 open issues involving include_role | 22:09 |
| pabelanger | haven't found an issue personally | 22:11 |
| *** harlowja has quit IRC | 22:12 | |
| jeblair | clarkb: can you take a look at https://review.openstack.org/455711 and my comment there? I'm on the fence as to whether the issue I raise is a +1 or a -1. | 22:13 |
| clarkb | jeblair: when the gate is long that can be quite an amount of time | 22:15 |
| clarkb | jeblair: so I think it is a valid concern | 22:15 |
| jeblair | yeah; however, i don't have any alternative technical suggestions about how to solve the problem the patch is addressing :/ | 22:16 |
| jlk | oh yeah | 22:29 |
| jlk | so this is a problem with github too | 22:29 |
| jlk | one github reporter is to set a "pending" status on the PR, which zuul will do immediately. But if there are no jobs for the project, there is no "success" report, and the status is left pending indefinitely. | 22:30 |
| jlk | Is there no way to tell "jobs are configured for this pipeline" quickly, and use that to report the message? or would it be a backup because the merger has to examine the repo for job definition? | 22:31 |
| jeblair | jlk: yeah, we don't actually know if there are any jobs until the merger runs, and we don't do that until the active window | 22:35 |
| jeblair | jlk: so i think this patch would fix that, but at the cost of not setting pending until it reaches the active window | 22:36 |
| jeblair | i guess if we want our cake and to eat it too, we'll have to compromise on that and perform the merge outside the active window | 22:36 |
| jlk | what defines "active window"? | 22:37 |
| *** harlowja has joined #zuul | 22:57 | |
| *** jamielennox is now known as jamielennox|away | 23:00 | |
| jlk | TypeError: __init__() takes at least 2 arguments (2 given) | 23:00 |
| jlk | THANKS | 23:00 |
| jeblair | jlk: it's how deep down the gate queue zuul runs jobs; we start it at 20 changes deep, and it halves each time a change fails, and increments each time a change merges. it's a rough approximation of how flaky the gate is in general. so if lots of things are failing, we don't do as many parallel tests. | 23:05 |
| jlk | ah. | 23:06 |
| jlk | Now that we can scale merger workers (not executor) we could at least allow that part to run rampant. | 23:06 |
| clarkb | yes. the main concern is cloud resources | 23:07 |
| jeblair | jlk: yeah, it's *mostly* about job resources (nodes). we also conserved merger resources because there was no reason not to, but now we have a reason. | 23:07 |
| jeblair | nova merges still take some time (~30 seconds), so it's not nothing. | 23:07 |
| jeblair | however, all of these solutions have some weird edge cases if you think about a change being behind a change that added it to a pipeline, but then the change adding it fails out. that could still leave us with a start report but not success or failure. | 23:08 |
| jeblair | the active window consideration doesn't really change that though | 23:08 |
| jlk | that is a weird consideration | 23:09 |
| jeblair | so i'm kind of inclined to say tobiash's change is better than what we have now, and the active window change would be a further improvement on that. i'm still at a loss of how to deal with the added-to-pipeline-in-failed-change-ahead problem. maybe we just need to remember when we've reported start and report failure in that case even if it's not currently in the pipeline? | 23:10 |
| jlk | Sounds like we need an additional report state | 23:11 |
| jlk | start, success, failure, dequeue ? | 23:12 |
| jeblair | hrm, that's an option | 23:12 |
| jlk | in the github case, we could clear the status | 23:13 |
| jlk | it'd still be a bit weird that it would go pending and then clear | 23:14 |
| jlk | actually, no, can't remove a status | 23:14 |
| * SpamapS still "isn't here" | 23:22 | |
| SpamapS | Is that not a "failed" status then? | 23:23 |
| SpamapS | if not, then I'd say don't report pending until you actually are _running_ | 23:23 |
| jlk | in the case where it was enqueued due to a previous pending change, and the pending change failed, I suppose that could make the downstream one "failed", in a sense | 23:24 |
| SpamapS | I think I'd rather (if we can) tie that to whether or not there are any jobs running. | 23:24 |
| jlk | it'd be a weird thing with gate | 23:24 |
| SpamapS | Might mean a proxy layer between. | 23:25 |
| jlk | jeblair: re-looking at your driver pipeline requirements change, why is there so much duplicate code between GerritEventFilter and GerritRefFilter ? What's the separation of duties there? | 23:25 |
| jlk | EventFilter is "this has to be part of the event to go in" and RefFilter is "this ref/change has to have all these things met to go in" | 23:26 |
| jlk | ? | 23:26 |
| jeblair | jlk: yes. they can theoretically be very different, but they share a lot of things in the case of gerrit (checking usernames, etc). so that reflects the api, but we should be able to collapse more of that shared code. | 23:28 |
| jeblair | jlk: mostly, i wanted to get the api right to unblock the github stuff | 23:28 |
| jlk | yeah | 23:28 |
| jlk | I'm finding myself duplicating code in github too, so I think there will be a good place to refactor later to simplify. | 23:29 |
| jeblair | ++ | 23:29 |
| jlk | Maybe an anonymous function for things like matchStatus() | 23:29 |
| jeblair | yeah, also, note the mix-in class i did for approval | 23:29 |
| jeblair | i have to run some errands now; have a good weekend! :) | 23:29 |
| jlk | later. | 23:30 |
| * SpamapS wasn't here anyway | 23:32 | |
| jlk | stop being here | 23:32 |
| SpamapS | I keep having to order things for pickup and then I sit down and you all talk about interesting things | 23:34 |
| SpamapS | also procrastinating on packing the car :-P | 23:34 |
| *** harlowja has quit IRC | 23:34 | |
| *** jamielennox|away is now known as jamielennox | 23:40 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!