| *** harlowja has quit IRC | 00:38 | |
| *** xinliang has joined #zuul | 01:40 | |
| *** xinliang has quit IRC | 01:40 | |
| *** xinliang has joined #zuul | 01:40 | |
| *** https_GK1wmSU has joined #zuul | 02:08 | |
| *** https_GK1wmSU has left #zuul | 02:08 | |
| *** harlowja has joined #zuul | 02:22 | |
| *** harlowja has quit IRC | 02:44 | |
| *** harlowja has joined #zuul | 03:38 | |
| tristanC | fwiw, I wrote a blog post about what's new in ZuulV3: https://github.com/redhat-openstack/website/pull/1019 | 04:40 |
|---|---|---|
| *** harlowja has quit IRC | 05:10 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Fix test_cache_hard_links when on tmpfs https://review.openstack.org/489377 | 05:16 |
| tobiash | SpamapS: ups... ^^^ better? | 05:16 |
| *** bhavik1 has joined #zuul | 05:48 | |
| *** harlowja has joined #zuul | 05:51 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Improve cleanup of test disk accountant https://review.openstack.org/489477 | 05:52 |
| tobiash | SpamapS: ^^ addresses your comments in 489368 | 05:53 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: Add max-nodes-per-job tenant setting https://review.openstack.org/489481 | 05:56 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: Add max-nodes-per-job tenant setting https://review.openstack.org/489481 | 05:58 |
| tobiash | tristanC: nice approach to the max-nodes-per-job | 06:08 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Don't ignore inexistent jobs in config https://review.openstack.org/488758 | 06:11 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Don't ignore inexistent jobs in config https://review.openstack.org/488758 | 06:12 |
| *** bhavik1 has quit IRC | 06:27 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Use zuul attributes in semaphore documentation https://review.openstack.org/489499 | 06:40 |
| *** harlowja has quit IRC | 06:51 | |
| *** yolanda__ has joined #zuul | 07:14 | |
| *** yolanda__ is now known as yolanda | 07:15 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Use zuul attributes in nodeset section https://review.openstack.org/489506 | 07:16 |
| SpamapS | tobiash: excellent | 07:29 |
| *** rcarrillocruz has quit IRC | 07:45 | |
| *** rcarrillocruz has joined #zuul | 07:45 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: Add max-nodes-per-job tenant setting https://review.openstack.org/489481 | 07:54 |
| tristanC | tobiash: thanks :-) | 07:54 |
| *** clarkb has quit IRC | 08:04 | |
| *** hashar has joined #zuul | 08:06 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Refactor provider config to driver module https://review.openstack.org/488384 | 08:08 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Fix test_cache_hard_links when on tmpfs https://review.openstack.org/489377 | 08:16 |
| *** yolanda has quit IRC | 08:17 | |
| *** clarkb has joined #zuul | 08:17 | |
| *** yolanda has joined #zuul | 08:18 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul feature/zuulv3: Add max-nodes-per-job tenant setting https://review.openstack.org/489481 | 08:19 |
| tobiash | hrm, doc patch failed test_timer_smtp | 08:42 |
| tobiash | looks like some gearman glitch: http://logs.openstack.org/99/489499/1/check/gate-zuul-python35/58e37a3/testr_results.html.gz | 08:42 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Implement a static driver for Nodepool https://review.openstack.org/468624 | 09:07 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Implement an OpenContainer driver https://review.openstack.org/468753 | 09:07 |
| *** jkilpatr has quit IRC | 10:44 | |
| *** jkilpatr has joined #zuul | 11:15 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Make github ssl verification configurable https://review.openstack.org/489573 | 11:46 |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul feature/zuulv3: Make github ssl verification configurable https://review.openstack.org/489573 | 11:48 |
| * rcarrillocruz waves | 12:32 | |
| rcarrillocruz | folks, correct me if i'm wrong but i don't see a way to set secgroups in Nodepool. Would you accept a patch to pass thru secgroups in label basis ? | 12:33 |
| *** dkranz_ has joined #zuul | 12:36 | |
| tobiash | rcarrillocruz: I'd like support for this | 12:43 |
| tobiash | rcarrillocruz: would also be on the list of my use cases for hardening my deployment later | 12:43 |
| rcarrillocruz | yah, in my use case i'm testing different network operating systems. Some vms have netconf, which has 830 by default, some doesn't | 12:44 |
| rcarrillocruz | some have http/https management interfaces | 12:44 |
| rcarrillocruz | i dislike having a mix of ports in default | 12:44 |
| openstackgerrit | Merged openstack-infra/nodepool feature/zuulv3: Move the fakeprovider module to the fake driver https://review.openstack.org/488383 | 13:24 |
| SpamapS | rcarrillocruz: I'd think any valid parameters for shade would be accepted. | 14:11 |
| mordred | SpamapS, rcarrillocruz: there are a few shade parameters we should disallow because they'd be bonghits in that context - but in general I believe I agree | 14:24 |
| rcarrillocruz | Yeah , not overexpose all params, but secgroups seems something a bunch of folks would like | 14:24 |
| rcarrillocruz | Will hack on it | 14:25 |
| clarkb | I wouldnt do it per label though | 14:42 |
| clarkb | it should be on the provider specific instamce description | 14:43 |
| clarkb | sinc different providers may have different names completely out of our control | 14:43 |
| SpamapS | yeah don't make it a nodepool concept :) | 14:43 |
| tobiash | hi | 15:33 |
| dmsimard | Oi, release candidate of ara 0.14 (with py3 support) is out https://github.com/openstack/ara/releases/tag/0.14.0rc2 | 15:33 |
| tobiash | I have a concept question about possibilities of protecting the project config from broken projects | 15:34 |
| dmsimard | mordred: ^ should be able to git+https://git.o.o/openstack/ara@tag that into zuul v3 to see if it breaks stuff | 15:34 |
| tobiash | lets say the owner of an untrusted project pushed some broken (e.g. syntax error) zuul.yaml onto a random branch | 15:34 |
| tobiash | (direct push) | 15:35 |
| tobiash | current behavior is that zuul is no longer able to evaluate any configs after that | 15:35 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool feature/zuulv3: Implement an OpenContainer driver https://review.openstack.org/468753 | 15:39 |
| jeblair | tobiash: yeah. we could automatically drop projects with syntax errors from the config | 15:40 |
| tobiash | jeblair: or at least the broken branches | 15:40 |
| tobiash | jeblair: tried out github connection today | 15:40 |
| tobiash | scenario is: I have a project tobiash/sandbox with a valid zuul.yaml | 15:41 |
| tobiash | then I create branch broken with a patch breaking zuul.yaml and create a pull request | 15:41 |
| tobiash | zuul correctly complains about the syntax error | 15:41 |
| tobiash | but when restarting zuul-scheduler -> config broken | 15:42 |
| tobiash | because it parses also the config of the non-protected branch 'broken' | 15:42 |
| jeblair | tobiash: that's a way of using github with zuul that i don't think we've anticipated. but yeah, we could auto-drop that branch. | 15:43 |
| jeblair | tobiash: i would expect that projects using zuul would not do random development in the project's own branch, but rather in other repos and submit pr's from the other repos, not the main project's repo | 15:44 |
| tobiash | jeblair: that's probably valid for most users | 15:44 |
| tobiash | jeblair: but we maybe need the centralized repo workflow which simplifies working with multiple repos in parallel | 15:45 |
| jeblair | tobiash: how does that simplify that? | 15:58 |
| tobiash | jeblair: when projects need to have multiple (50+) repos layed out in the workspace we currently use git-repo to manage that | 15:59 |
| tobiash | jeblair: this wouldn't work with personal copies | 15:59 |
| tobiash | jeblair: and I know that these are too many repos and git-repo won't work with github... | 16:00 |
| tobiash | jeblair: so we are currently trying to find a good way to move a project like this into the github workflow | 16:01 |
| tobiash | jeblair: that probably implies to drastically reduce the number of repos belonging together | 16:01 |
| tobiash | jeblair: but I currently don't have a good solution in mind for this problem... | 16:02 |
| jeblair | tobiash: i don't see a reason you shouldn't have as many repos as you want. i'm not familiar with what github tools are available, but a tool like git-repo which clones 50 projects from a central location and then pushes to user versions of those projects seems practical. | 16:04 |
| jeblair | tobiash: put another way, i think the workflow of having a read-only master repo which only merges pull requests from other repos is feasible, regardless of the number of repos involved. | 16:05 |
| tobiash | jeblair: yes, that depends on the tooling | 16:10 |
| tobiash | jeblair: I still have the hope I can push this project towars less loosely coupled repos | 16:10 |
| jeblair | tobiash: that's your decision of course -- but zuul is designed to support *lots* of loosely coupled repos, so i want to make sure there isn't any limitation there. however, it's also designed to support gating, so the further you get from a central, gated project the more likely you're going to run into areas where it doesn't work as well. | 16:12 |
| jeblair | tobiash: so, while we can add the protection against broken branches, having broken branches at all is going to be annoying or painful regardless. so better to try to avoid them in the first place. | 16:13 |
| mordred | dmsimard: fwiw - there is a pre-release pipeline in zuul that you can put on ara so that it'll publish pre-release things to pypi too ... modern pip won't intsall pre-release versioned things unless they are explicitly requested, so it's safe to do so | 16:13 |
| dmsimard | mordred: orly ? I always thought that pip would pick up the rc's due to higher NVR | 16:13 |
| mordred | nope | 16:16 |
| mordred | that got fixed a few years ago | 16:16 |
| tobiash | jeblair: hrm, that sounds like a lot of work for tooling, but I agree that this would be probably the cleanest solution | 16:16 |
| mordred | dmsimard: if it's a proper pre-release version, you either have to give pip the --pre option or you have to explicitly request a prereleaes version | 16:17 |
| dmsimard | mordred: it's not something magically handled by pbr ? | 16:17 |
| * dmsimard loves pbr black magic | 16:18 | |
| mordred | dmsimard: so like if you do "pip install -U 'ara>=0.14.0rc2'" and then release an rc3, it'll pick it up - but if you just do "pip install -U 'ara>=0.13'" it will not pick up any of the 0.14 preleases | 16:18 |
| mordred | dmsimard: it's not - this is base pip functionality now :) | 16:18 |
| dmsimard | okay, TIL | 16:18 |
| tobiash | jeblair: for the self protection of branches, it might be easier to be able to limit the branches in the tenant config | 16:28 |
| jeblair | tobiash: that seems like a fine idea. | 16:29 |
| tobiash | jeblair: with this we could limit the configloader to the protected branches and support both, shared and forking workflows | 16:29 |
| tobiash | jeblair: and regarding project owners force pushing defects into their projects: I think this could be solved by using tenants more fine granular | 16:30 |
| tobiash | jeblair: I think (hope) that broken configs just affect the tenant(s) referring the broken project | 16:31 |
| jeblair | pabelanger: what do you make of this error? http://logs.openstack.org/09/489409/4/check/tox-linters/d900252/job-output.txt.gz#_2017-08-01_01_35_40_890395 | 16:36 |
| pabelanger | jeblair: looks like an issue with role path for ansible-lint | 16:37 |
| pabelanger | zuul_console seems to be missing | 16:37 |
| mordred | zuul_console is in the zuul module path - which we're probably not running ansible-lint with | 16:38 |
| jeblair | pabelanger: did we accidentally approve a change that broke that? (my change doesn't touch that afaik) | 16:38 |
| pabelanger | maybe | 16:38 |
| pabelanger | and it wasn't reported because github thing | 16:39 |
| pabelanger | let me look quickly | 16:39 |
| pabelanger | jeblair: left comment, we need to install zuul in test-requirements.txt | 16:40 |
| pabelanger | otherwise, we don't have zuul_console in our path | 16:41 |
| jeblair | pabelanger: oh, *that's* why that was there, thank you. | 16:42 |
| jeblair | pabelanger: i thought it had been added for some other reason. i will update my patch to put it back and leave a comment for why it's there | 16:42 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Switch to using zuul-sphinx https://review.openstack.org/489409 | 16:44 |
| *** hashar has quit IRC | 16:44 | |
| pabelanger | jeblair: thanks, I should have left a comment | 16:45 |
| jeblair | pabelanger: i should have remembered. :) | 16:45 |
| jlk | o/ | 16:50 |
| tobiash | jlk: when using github, do I have to configure something other than the webhook in the project? | 17:06 |
| jlk | tobiash: there are two ways to configure | 17:06 |
| tobiash | jlk: in my test with github enterprise zuul comments on the pull request, but I don't see any flag | 17:07 |
| jlk | one is a webhook + a secret + using the API key of a user with write access | 17:07 |
| jlk | the other is an "App" install, which is a github App, that you locally feed the App key into zuul configuration, and the repository in question has to "install" the app for their repo. | 17:07 |
| jlk | tobiash: what "flag" are you expecting? | 17:08 |
| tobiash | jlk: I've set status: 'success' in the reporter section of the check pipeline | 17:08 |
| tobiash | jlk: so I expect to see a 'label' | 17:08 |
| tobiash | jlk: probably a dumb question as I'm new to github workflows | 17:09 |
| jlk | status != label | 17:09 |
| jlk | status is a bit of metadata set on the hash sum | 17:09 |
| jlk | one sec | 17:09 |
| jlk | See https://github.com/BonnyCI/hoist/pull/432 | 17:09 |
| pabelanger | mordred: speaking of github, did you want to update openstack-zuul to use SSL without validation | 17:09 |
| jlk | there's a couple statuses set, check_github has a green status | 17:10 |
| tobiash | jlk: I'll probably have to enable branch protection before seeing the status | 17:11 |
| jlk | I don't think so | 17:12 |
| jlk | status can be set whether branch protection is enabled or not | 17:12 |
| jlk | The API key you've given zuul, does it have write access to the repository in question? | 17:12 |
| jlk | Have you checked the scheduler logs to see if it attempted to report on the pull request? | 17:13 |
| tobiash | jlk: ok, added the zuul users to the collaborators, and now I have the following in the scheduler log | 17:19 |
| tobiash | jlk: http://paste.openstack.org/show/617154/ | 17:19 |
| jlk | that's... weird | 17:21 |
| tobiash | jlk: do I have to define a status somewhere? | 17:21 |
| jeblair | read docs! | 17:22 |
| jlk | no, there's always just the 3 options for status | 17:22 |
| jeblair | tobiash, jlk: https://docs.openstack.org/infra/zuul/feature/zuulv3/admin/drivers/github.html | 17:22 |
| jlk | hrm, that sounds like an oauth issue | 17:22 |
| jeblair | tobiash, jlk: please make sure those are correct :) | 17:23 |
| jlk | the status reporter is correct | 17:23 |
| jlk | tobiash: can you fully stop and fully start zuul again? Make sure there isn't a duplicate oath token somewehre? | 17:24 |
| dmsimard | pabelanger: btw I'm flipping the switch for nodepool boot from volume on review.rdo, I'll let you know how it goes | 17:27 |
| tobiash | jlk: stopped scheduler, waited 2min, started scheduler | 17:27 |
| tobiash | jlk: the issue persists | 17:27 |
| jlk | awesome.... I hope it's not something specific to GHE. What version is your GHE? | 17:28 |
| tobiash | 2.9.5 | 17:28 |
| pabelanger | dmsimard: cool | 17:30 |
| jlk | tobiash: huh. A bunch of versions old, but only a few months old. | 17:32 |
| tobiash | jlk: yepp, hoping it will be a newer version soon | 17:32 |
| tobiash | jlk: what access rights are needed for the access token? | 17:33 |
| jlk | "code" or repo:status | 17:33 |
| jlk | https://developer.github.com/apps/building-integrations/setting-up-and-registering-oauth-apps/about-scopes-for-oauth-apps/ | 17:34 |
| jlk | oh wait, that's uh, different | 17:34 |
| dmsimard | pabelanger: so far so good, seeing new VMs with volumes | 17:35 |
| tobiash | jlk: now I've activated all but still the same error | 17:35 |
| jlk | what version of github3.py do you have? | 17:35 |
| mordred | pabelanger: on it | 17:37 |
| tobiash | jlk: c82e90e5 from branch develop | 17:38 |
| tobiash | looks like the latest in develop | 17:39 |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Switch to using zuul-sphinx https://review.openstack.org/489409 | 17:39 |
| jlk | damn I don't see any recent changes there that would make that a problem | 17:39 |
| mordred | pabelanger: openstack-zuul in github is now configured to use ssl but not verify | 17:40 |
| jlk | tobiash: what if you do: status: success | 17:44 |
| jlk | not in quotes | 17:44 |
| tobiash | jlk: trying | 17:44 |
| jlk | my life config uses it without quotes. | 17:45 |
| jlk | https://github.com/j2sol/project-config/blob/master/zuul.yaml | 17:45 |
| tobiash | jlk: ok, what's interesting is if zuul is not collaborator: comment, but no status, if zuul is collaborator: nothing | 17:47 |
| jlk | whaaat | 17:47 |
| jlk | what level of collaborator is it? | 17:47 |
| tobiash | jlk: the quotes didn't change anything | 17:47 |
| jlk | fffff | 17:47 |
| tobiash | in the project settings, there is no level I can change there (Push access to the repository) | 17:48 |
| tobiash | adding now the comment: true (which is missing in my pipeline) | 17:48 |
| tobiash | hm, still the same | 17:50 |
| tobiash | I should probably first try curl | 17:50 |
| pabelanger | mordred: thanks | 17:53 |
| *** dmsimard is now known as dms | 17:54 | |
| *** dms is now known as dmsimard | 17:55 | |
| tobiash | jlk: you probably won't believe it | 17:56 |
| tobiash | jlk: I found the reason.... | 17:56 |
| tobiash | ... | 17:56 |
| tobiash | jlk: the pipeline description... | 17:56 |
| tobiash | jlk: you don't have one | 17:56 |
| tobiash | jlk: I have a multiline pipeline description, which broke that thing | 17:57 |
| *** dmsimard is now known as dms | 17:59 | |
| *** dms is now known as dmsimard | 17:59 | |
| *** dmsimard is now known as dms | 18:02 | |
| jlk | ooooh right, huh. | 18:03 |
| jlk | we grab the description? I thought we just grabbed the pipeline title. | 18:03 |
| *** dms is now known as dmsimard | 18:03 | |
| jlk | oh we do | 18:03 |
| jlk | context is the pipeline name, but the description is the description. Guess that needs some protection against multiline? | 18:04 |
| tobiash | jlk: have to figure out if it's multiline or some special chars | 18:05 |
| jlk | okay | 18:05 |
| jlk | yeah the API says it just takes a "string" | 18:06 |
| jlk | :/ | 18:06 |
| *** dmsimard is now known as dmoreaus | 18:07 | |
| *** dmoreaus is now known as dmsimard | 18:07 | |
| jeblair | jlk, tobiash: maybe the github driver should only use the first line if it's multiline. that way we still get self-documenting pipelines. | 18:08 |
| jeblair | but can pull out a "summary" line | 18:08 |
| jlk | yeah, that would need documentation alright. Or convert new lines to \n? not sure how it would appear in the github UI | 18:11 |
| *** EmilienM is now known as emacchi | 18:14 | |
| *** emacchi is now known as EmilienM | 18:14 | |
| tobiash | looks like the length of the string | 18:17 |
| tobiash | documentation says 1024 bytes, but 200 chars are already too much | 18:17 |
| jlk | where did you see a 1024 byte limit? | 18:24 |
| tobiash | jlk: ups, forgot to paste the link: https://developer.github.com/v3/repos/statuses/ | 18:25 |
| jlk | oh haha | 18:25 |
| jlk | I was already on that page, just glossed over that part. | 18:26 |
| tobiash | jlk: ok, the limit which works is 140 chars | 18:26 |
| jlk | basically we could truncate. How many bytes is 200 chars? | 18:26 |
| jlk | (how does one even measure that in python) | 18:26 |
| clarkb | jlk: the answer is it depends :) | 18:26 |
| jlk | :( | 18:26 |
| clarkb | jlk: utf8 is variable width from one to four bytes per character | 18:26 |
| jlk | ugh. | 18:27 |
| jlk | and we'd need to know what encoding github is using on the backend | 18:27 |
| jlk | ugghhhhh | 18:27 |
| tobiash | well these 140 chars limit was with just aaaaaaaaaaaaaa... | 18:28 |
| tobiash | so I don't see any method which would translate 140 equal chars to 1024 bytes | 18:28 |
| pabelanger | woot | 18:29 |
| pabelanger | http://zuulv3.openstack.org/keys/github/gtest-org/ansible.pub | 18:29 |
| tobiash | maybe just a bug in the docs? | 18:29 |
| tobiash | pabelanger: yay | 18:29 |
| pabelanger | http://zuulv3.openstack.org/keys/gerrit/openstack-infra/zuul.pub errors | 18:29 |
| pabelanger | ERR_CONTENT_LENGTH_MISMATCH what I get in chrome | 18:30 |
| pabelanger | need to debug that still | 18:30 |
| tobiash | jlk, jeblair: what do you think about separating the pipeline description from the status description? | 18:35 |
| tobiash | jlk, jeblair: when reading it feels a bit odd like "Newly uploaded patchsets enter this pipeline to receive an initial vote. For running a build again, add a comment 'recheck'." | 18:36 |
| jlk | as in let the user define it at the status: block? | 18:36 |
| tobiash | as a status description in github | 18:36 |
| tobiash | jlk: possibly | 18:36 |
| tobiash | I think pipeline heading <-> github status description could be different texts | 18:37 |
| jlk | could be. I honestly haven't seen many descriptions in use | 18:39 |
| jlk | I think Travis uses a description like "The Travis CI build is in progress" | 18:39 |
| jlk | or "The Travis CI build failed" | 18:39 |
| tobiash | jlk: yes, which is completely different than "When a commit is tagged as a release, this pipeline runs jobs that publish archives and documentation." | 18:40 |
| jlk | We could do something like that. | 18:40 |
| tobiash | (taken from zuulv3.o.o) | 18:40 |
| jlk | Could you write up a quick story on storyboard and I'll pick it up after lunch? | 18:42 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: WIP: Update tox-tarball to use tox role https://review.openstack.org/489705 | 18:43 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: Use py35 tox envlist for documentation https://review.openstack.org/489707 | 18:45 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul feature/zuulv3: Use py35 tox envlist for documentation https://review.openstack.org/489707 | 18:45 |
| *** jkilpatr has quit IRC | 18:47 | |
| tobiash | jlk: like this https://storyboard.openstack.org/#!/story/2001138 ? | 18:49 |
| jlk | sure. I may re-word it a bit as a problem statement on status length. The thrust of the work is to stop using pipeline description as the status description. | 18:50 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: WIP: Update tox-tarball to use tox role https://review.openstack.org/489705 | 18:56 |
| *** jkilpatr has joined #zuul | 18:58 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: WIP: Update tox-tarball to use tox role https://review.openstack.org/489705 | 19:06 |
| tobiash | jlk: so now check and gate are triggered and run :) | 19:20 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/zuul feature/zuulv3: Rename Node.hold_reason to 'comment' https://review.openstack.org/489717 | 19:20 |
| tobiash | jlk: thanks for your help | 19:20 |
| tobiash | jlk: now the merge doesn't work, but I have to figure out that tomorrow | 19:21 |
| *** hashar has joined #zuul | 19:21 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: WIP: Update tox-tarball to use tox role https://review.openstack.org/489705 | 19:26 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: Simplify run tox task https://review.openstack.org/487551 | 19:26 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 19:30 |
| mordred | jeblair, pabelanger: ^^ | 19:30 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Add --detail option to nodepool list command https://review.openstack.org/489720 | 19:30 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Add hold job to nodepool list output https://review.openstack.org/489721 | 19:30 |
| jeblair | mordred: that's story 2001136 btw | 19:31 |
| mordred | jeblair: neat! | 19:32 |
| mordred | jeblair: can required-projects go ina variant? | 19:32 |
| jeblair | mordred: yep. they get dict-merged. | 19:33 |
| mordred | cool | 19:33 |
| pabelanger | mordred: cool, will look shortly | 19:36 |
| mordred | piddle - it doens't fully quite work - looking in to that | 19:39 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: WIP: Update tox-tarball to use tox role https://review.openstack.org/489705 | 19:39 |
| *** jkilpatr_ has joined #zuul | 19:44 | |
| jeblair | mordred: also if you could update roles/tox/README that'd be swell | 19:45 |
| mordred | jeblair: ++ | 19:45 |
| mordred | jeblair: I shall - as soon as I fix it to actually work :) | 19:45 |
| jeblair | mordred: your priorities are all backwards! ;) | 19:46 |
| *** jkilpatr has quit IRC | 19:46 | |
| mordred | jeblair: this is frequently true | 19:46 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: DNM - testing https://review.openstack.org/489724 | 19:48 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: DNM - testing https://review.openstack.org/489724 | 19:53 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: DNM - testing https://review.openstack.org/489724 | 19:58 |
| mordred | jeblair, pabelanger: crappit. the tox --force-dep option doesn't work for requirements files. sigh | 20:05 |
| mordred | slighly different option coming | 20:06 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: DNM - testing https://review.openstack.org/489724 | 20:06 |
| pabelanger | mordred: jeblair: actually, I have an idea. Working on example | 20:07 |
| jeblair | mordred: drat! | 20:07 |
| mordred | pabelanger: neat. is yours to just edit the requirements.txt file before running? | 20:08 |
| pabelanger | mordred: https://review.openstack.org/#/c/489724/ was my first example using --force-dep. But you said that won't work | 20:09 |
| pabelanger | http://zuulv3.openstack.org/static/stream.html?uuid=ce3ce629c3fb4d8893762a2ac0d90dbc&logfile=console.log is job | 20:09 |
| pabelanger | I was then going to try pip install ~/src/git.openstack.org/openstack-infra/zuul in pre-run task | 20:10 |
| mordred | yah - so the thing is - we have to protect from things that areant' in the requirements files | 20:10 |
| pabelanger | --force-dep seem to work for test-requirements.txt | 20:10 |
| mordred | because of the openstack/requirements repo | 20:10 |
| pabelanger | mordred: right, I think we specify which repos we want to --force-dep on, rather then looping over src dir | 20:11 |
| mordred | 2017-08-01 20:08:43.454300 | ubuntu-xenial | Obtaining zuul from git+git://git.openstack.org/openstack-infra/zuul@feature/zuulv3#egg=zuul (from -r /home/zuul/src/git.openstack.org/openstack-infra/zuul-jobs/test-requirements.txt (line 11)) | 20:11 |
| pabelanger | Yup | 20:11 |
| pabelanger | I like the idea of 489724, since it works today without adding tox_use_siblings or ansible library. But requires some changes to job setup in repo | 20:13 |
| jeblair | pabelanger: why does force-dep work for test-requirements? | 20:15 |
| pabelanger | jeblair: no, just confused | 20:16 |
| jeblair | there shouldn't be anything special about it, other than it's the second one on the cmdline | 20:16 |
| pabelanger | I was looking at the wrong stream | 20:16 |
| jeblair | pabelanger: oh ok. | 20:16 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: DNM - testing https://review.openstack.org/489724 | 20:18 |
| mordred | pabelanger: yah - so, I like tox_use_siblings because it doesn't require user-visible magic in the job definition | 20:18 |
| mordred | pabelanger: but I think we'll basically both just have to get a thing that works then let others decide :) | 20:18 |
| pabelanger | mordred: https://github.com/tox-dev/tox/issues/534 | 20:19 |
| pabelanger | seems to be a bug | 20:19 |
| mordred | yup | 20:21 |
| mordred | sorry - I should have both been more clear when I said it doesn't work and also pushed up the WIP patch I have for poking at it | 20:22 |
| jeblair | mordred, pabelanger: fwiw, i think the value here is having this work automatically, without needing to change things in every repo | 20:23 |
| mordred | yuo | 20:23 |
| mordred | yes | 20:23 |
| pabelanger | jeblair: Ya, there is value in automatic | 20:24 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: Update tox-tarball to use tox role https://review.openstack.org/489705 | 20:24 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: Update tox-tarball to use tox role https://review.openstack.org/489705 | 20:26 |
| pabelanger | mordred: jeblair: ^ dropped our shell script for create tarball / wheels and now depends on the tox role | 20:27 |
| jeblair | https://docs.openstack.org/infra/zuul-jobs/ exists! | 20:28 |
| pabelanger | jeblair: excellent! | 20:29 |
| SpamapS | that's pretty sweet | 20:40 |
| SpamapS | now if only you could get this guy to stop chain sawing 50 feet from me... :-P | 20:40 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 20:49 |
| mordred | jeblair, pabelanger: ^^ something more like that | 20:49 |
| mordred | basically - let tox do an install like normal, then check to see what packages it installed, then overwrite them if there are any matching projets zuul has checked out - then actually run tox without --notest to run the tests | 20:51 |
| jlk | bike shed time | 20:52 |
| mordred | heck yes | 20:52 |
| jlk | due to a bug that tobiash found today, I'm changing github reporter driver code with regard to what it sets as a description for a commit status | 20:53 |
| jlk | We used to just take the pipeline description and shove it in, but that ran into length concerns (max 1024 bytes). | 20:53 |
| jlk | I looked at what Travis does, it sets a description like "The Travis CI build is pending" or "The Travis CI build passed" | 20:54 |
| jlk | We could do similar, using the pipeline name, e.g. "check pipeline is pending" | 20:55 |
| jlk | "check pipeline succeeded" | 20:55 |
| jlk | I was trying to use the same term as the status state: pending, success, error, failure | 20:56 |
| jlk | but englishing is hard. "check pipeline result success" ? | 20:56 |
| SpamapS | perhaps go with more absurd choices.. "check pipeline is lobster" | 20:57 |
| jlk | "success, check pipeline is" | 20:58 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Add --detail option to nodepool list command https://review.openstack.org/489720 | 20:58 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Add hold job to nodepool list output https://review.openstack.org/489721 | 20:58 |
| jlk | so I'm stuck here, trying to think of a good way to word this, so that I can write my (failing) test, so that I can create the code to pass the test. | 21:00 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 21:01 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Add README for tox role https://review.openstack.org/489750 | 21:01 |
| mordred | jlk: what about "The Zuul {pipeline} pipeline status is {status}" | 21:02 |
| mordred | jlk: or just "The {pipeline} pipeline status is {status}" - so you get "The check pipeline status is pending" or "The check pipeline status is success" | 21:03 |
| Shrews | avoid the englishing: "{pipeline} status: {status}" | 21:04 |
| mordred | jlk: OR - put in a nice little if/elif/elif block with "if status == 'pending': msg = 'This change is pending in the check pipeline" elif status == 'success': msg = 'This change was successful in the check pipeline' ... etc | 21:04 |
| jlk | Hrm, I think I like Shrews' suggestion most of all | 21:05 |
| jlk | fewer things to translate should zuul ever be translated. | 21:05 |
| mordred | ++ | 21:06 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Add --detail option to nodepool list command https://review.openstack.org/489720 | 21:08 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool feature/zuulv3: Add hold job to nodepool list output https://review.openstack.org/489721 | 21:08 |
| pabelanger | mordred: left comment. I think we should consider moving that logic into tox-pre playbook, and skip adding it into the role if that make sense | 21:11 |
| pabelanger | mordred: https://review.openstack.org/#/c/489705/7/playbooks/tox/tarball.yaml would be an example of how we could call the role twice, but using different parameters | 21:11 |
| mordred | pabelanger: ah - gotcha. yeah. I think we can make that work | 21:12 |
| mordred | pabelanger: and you're right- we don't want to do the re-install each of those times :) | 21:12 |
| mordred | pabelanger: update coming | 21:13 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 21:21 |
| mordred | pabelanger: how about that ^^ ? (I mean, assuming it works and whatnot) | 21:21 |
| jeblair | mordred: i think we may also want that documented in the 'tox' job | 21:26 |
| pabelanger | mordred: ya, the way I was thinking won't work unless we consider include_role task. Since we cannot call a role from a role or task between roles | 21:27 |
| pabelanger | mordred: it like makes more sense to go back to PS3, and keep logic in same role | 21:28 |
| mordred | pabelanger: I'm confused | 21:29 |
| pabelanger | mordred: let me hackup an example | 21:30 |
| mordred | pabelanger: what is it that won't work? if we put it in the pre-playbook it'll happen before any tox invocations happen - so it should only happen once | 21:30 |
| mordred | and as long as you're using the tox base job, all should b fine, yeah? | 21:30 |
| jeblair | pabelanger: i think we can consider include_role. i don't have a blanket -2 on it. just don't want to build our jobs out of it exclusively. i think a primary role including a secondary role may be a good use. | 21:30 |
| jeblair | (sorry if that distracts -- just wanted to put that out there) | 21:31 |
| mordred | (also - I agree about documenting in the job variables - sorry I missed that one :) ) | 21:32 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 21:41 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Rename tox_command_line in docs to tox_extra_args https://review.openstack.org/489758 | 21:41 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: DNM: Override tox requirments with zuul git repos https://review.openstack.org/489761 | 21:45 |
| pabelanger | mordred: ^ I modified your patch to better explain what I was suggesting | 21:45 |
| pabelanger | but a new change-id | 21:45 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Docs: add a glossary https://review.openstack.org/489270 | 21:46 |
| pabelanger | basic difference is we don't need to change a new role, we could reuse existing tox role but just pass in different variable | 21:46 |
| pabelanger | less copypasta | 21:46 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Docs: add a :default: argument to zuul:attr https://review.openstack.org/489303 | 21:48 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Use zuul attributes in semaphore documentation https://review.openstack.org/489499 | 21:48 |
| jeblair | pabelanger, mordred: can you explain https://review.openstack.org/487948 to me? | 21:48 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Remove zuul_url from merger config https://review.openstack.org/489378 | 21:48 |
| openstackgerrit | Merged openstack-infra/zuul feature/zuulv3: Improve cleanup of test disk accountant https://review.openstack.org/489477 | 21:48 |
| *** jkilpatr_ has quit IRC | 21:49 | |
| pabelanger | jeblair: it relates to 487551. Because we setup tox_chdir to src/{{ zuul.project.canonical_name }} in the tox job, we need to override it in 487948 otherwise we will run tox on zuul-jobs repo not openstack-infra/zuul | 21:50 |
| jeblair | pabelanger: what's zuul_work_dir used for? | 21:51 |
| pabelanger | jeblair: we still depend on it for publishing things I believe | 21:51 |
| pabelanger | we could set tox_chdir to zuul_work_dir in 487551 also | 21:52 |
| pabelanger | and skip using src/{{ zuul.project.canonical_name }} | 21:52 |
| jeblair | pabelanger: basically, i'm starting to lose track of what variables we're using for what. | 21:53 |
| jeblair | pabelanger: 487948 is a red flag because we're setting two vars to the same value | 21:53 |
| pabelanger | agree, part of the issue I think is zuul_work_dir is not avaiable to all playbooks. So some are exposed to it, others are not. But agree, it is confusing when to use it and when not too | 21:54 |
| jeblair | pabelanger: what is the difference between tox_chdir and zuul_work_dir? | 21:55 |
| pabelanger | jeblair: just a rename, I opted to tox_chdir in this case to be more inline with existing ansible task that use chdir setting | 21:56 |
| jeblair | pabelanger: okay, i'm not seeing the whole plan here. maybe you could write something up in an etherpad or something to help me understand how you want to effect this change across all usages of zuul_work_dir. | 21:58 |
| pabelanger | jeblair: sure, the general idea was to remove the zuul_work_dir from being directly accessed from the tox role, thats main reason for creating tox_chdir. I have to test again, but I believe I was having issues overriding zuul_work_dir if it was already defined | 22:02 |
| jeblair | pabelanger: are you saying there's a bug? | 22:07 |
| pabelanger | jeblair: not a bug, just that we don't define zuul_work_dir in our inventory file. It is only scoped at the role level | 22:07 |
| pabelanger | so, some playbooks, won't have zuul_work_dir setup | 22:08 |
| jlk | tobiash: still around? | 22:08 |
| jeblair | pabelanger: right -- roles that need it set it to the default. it was added to a bunch of roles with essentially identical default values. | 22:08 |
| jeblair | pabelanger: is that a problem? | 22:08 |
| *** jkilpatr has joined #zuul | 22:08 | |
| pabelanger | jeblair: possible. I think we had zuul_work_dir defined in our inventory file at one point? In the case of the tox-py35-on-zuul it defines in the inventory file, where other jobs don't. So it gets a little confusing on how to use it | 22:11 |
| jeblair | pabelanger: the idea was that normally you don't define it. our tox roles chdir to the zuul.project directory and run. but if you wanted to cross-test, you have to define it. | 22:12 |
| pabelanger | jeblair: Ya, so the easier way forward would be just to rename tox_chdir back to zuul_work_dir. | 22:14 |
| jeblair | pabelanger: we've started renaming stuff already? | 22:15 |
| mordred | ah - I believe there is confusoin | 22:16 |
| mordred | we used to set a variable called zuul_workspace - and we decided that we did not need to set that | 22:16 |
| pabelanger | jeblair: ya, I decided to rename it to better fit with the other tox role variable names. | 22:16 |
| jeblair | pabelanger: where is it used now? | 22:16 |
| pabelanger | jeblair: sorry, which it? | 22:17 |
| jeblair | pabelanger: where is tox_chdir used now | 22:17 |
| mordred | we've never set zuul_work_dir from zuul or the inventory - it was added as a default variable to a bunch of roles - for things where it made sense for the role to be operating in "the project that was triggered" | 22:17 |
| pabelanger | jeblair: just 487551, unmerged | 22:17 |
| pabelanger | mordred: okay, was confusing it with zuul_workspace | 22:18 |
| mordred | yah | 22:18 |
| mordred | it's actually important to keep its name not tied to a specific role | 22:18 |
| mordred | because it gets used in multiple roles that may be in the same job | 22:19 |
| mordred | so the user may need to set zuul_work_dir as a paramter to a job invocation | 22:19 |
| mordred | and all of the roles that the job uses need to be able to pick that up | 22:19 |
| jeblair | pabelanger: okay, when you said the way forward was to rename it back, i thought you meant we would have to rename something. what we're actually talking about here is *not* renaming something. | 22:19 |
| pabelanger | jeblair: yes, remove tox_chdir and just use zuul_work_dir | 22:20 |
| pabelanger | we'll have to reserve that for all roles too | 22:20 |
| mordred | http://git.openstack.org/cgit/openstack-infra/openstack-zuul-jobs/tree/zuul.yaml#n8 | 22:20 |
| mordred | is the example of needing it | 22:21 |
| mordred | and/or using it | 22:21 |
| pabelanger | mordred: right https://review.openstack.org/487948/ was how jeblair started asking about it | 22:21 |
| mordred | ah - nod | 22:21 |
| pabelanger | it removed zuul_work_dir from tox role and changed it to tox_chdir | 22:21 |
| pabelanger | which mean we needed to setup a new variable | 22:22 |
| pabelanger | so, I can remove tox_chdir | 22:22 |
| mordred | I grok how we got where we are now :) | 22:22 |
| pabelanger | mordred: jeblair: https://review.openstack.org/#/c/487551/4/roles/tox/defaults/main.yaml also is some history of rename, now that I remember | 22:23 |
| mordred | yah - I think what we were missing was a shared understanding of what was going on with zuul_work_dir - which in isolation on each of these patches can be easy to lose track of | 22:24 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul-jobs master: Simplify run tox task https://review.openstack.org/487551 | 22:25 |
| pabelanger | mordred: right, it means zuul_work_dir will be added to a lot of defaults/main.yaml files, to get the working dir of the cloned project | 22:26 |
| jeblair | pabelanger: cool. what did we decide on the zero-tests check? | 22:26 |
| openstackgerrit | Jesse Keating proposed openstack-infra/zuul feature/zuulv3: Simplify github status descriptions https://review.openstack.org/489767 | 22:26 |
| pabelanger | jeblair: I am working on adding that into unittest I think | 22:26 |
| pabelanger | jeblair: plan is to keep it | 22:26 |
| jeblair | ok | 22:26 |
| *** hashar has quit IRC | 22:28 | |
| mordred | jlk: I love your really long pipeline name | 22:37 |
| jlk | had to get creative | 22:37 |
| SpamapS | checkicalifragigateishexpialipostous | 22:39 |
| pabelanger | jeblair: do you mind helping me understand why public keys for gerrit projects in zuulv3.o.o are not loading properly | 22:39 |
| pabelanger | jeblair: github connection seems to work fine | 22:39 |
| pabelanger | eg: http://zuulv3.openstack.org/keys/github/gtest-org/ansible.pub works | 22:40 |
| pabelanger | http://zuulv3.openstack.org/keys/gerrit/openstack-dev/sandbox.pub error | 22:41 |
| mordred | pabelanger: I don't see any errors in the log for that | 22:47 |
| pabelanger | ya, I couldn't see anything wrong in debug logs | 22:47 |
| mordred | pabelanger: sandbox.pem exists on disk | 22:50 |
| pabelanger | mordred: ya, and with correct permissions | 22:50 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Docs: use zuul:attr in project definition https://review.openstack.org/489774 | 22:55 |
| mordred | jeblair: is there a chance that _generateKeys then _loadKeys is hitting an issue with buffering somehow? and because we just added openstack-dev/sandbox.pub but gtest-org/ansible.pub was added before the previous restart is why one works and the other doesn't? | 22:55 |
| jeblair | mordred, pabelanger: looking | 22:56 |
| mordred | I mean - I expect the code in those methods to work | 22:57 |
| jeblair | pabelanger, mordred: has zuul been restarted since adding that project to the tenant? | 23:00 |
| pabelanger | jeblair: sandbox, no. but same issue exists for zuul.pub too | 23:00 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Add README for tox role https://review.openstack.org/489750 | 23:02 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Override tox requirments with zuul git repos https://review.openstack.org/489719 | 23:02 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul-jobs master: Rename tox_command_line in docs to tox_extra_args https://review.openstack.org/489758 | 23:02 |
| mordred | hrm. that's more vexing | 23:02 |
| jeblair | pabelanger, mordred: apache says that's a 500 rror | 23:02 |
| jeblair | pabelanger, mordred: i assume we're missing some logging config for that | 23:05 |
| mordred | jeblair: we don't seem to have much logging in zuul/webapp.py | 23:05 |
| jeblair | mordred: well, i mean i assume that paste would log an exception but we're not set up to log it | 23:06 |
| mordred | oh - maybe we need to add ... yah | 23:06 |
| mordred | was just about to say that | 23:06 |
| mordred | jeblair: you doing that? if not, I will ... | 23:07 |
| jeblair | mordred: i'm not sure what to add | 23:07 |
| mordred | well - in our test suite we do paste=INFO | 23:07 |
| jeblair | mordred: oh, what we should do is make the root logger also go to the main and debug logs. | 23:08 |
| jeblair | mordred: so yeah, 'paste' probably. but actually, i think ^ would take care of it. | 23:08 |
| jeblair | we shouldn't need a special handler for paste | 23:08 |
| jeblair | i'll propose a change | 23:09 |
| mordred | jeblair: kk | 23:09 |
| jeblair | remote: https://review.openstack.org/489775 Send the root logger to debug and normal logs | 23:10 |
| jeblair | i'll do that manually on v3 now | 23:10 |
| mordred | jeblair: +2 | 23:12 |
| jeblair | mordred: zuul startup is slowed noticably by the gat jobs for al lof the ansible branches | 23:13 |
| jeblair | s/gat/cat/ | 23:13 |
| mordred | jeblair: they have a non-zero number of branches don't they | 23:14 |
| mordred | jeblair: I really can't wait to see how long startup is on the entire corpus of openstack | 23:14 |
| jeblair | mordred: that didn't log an error either. i'll add an explicit paste section. | 23:15 |
| jeblair | mordred: startup will go faster with 16 mergers+executors | 23:16 |
| mordred | jeblair: indeed | 23:16 |
| jeblair | mordred: 2 things: one, we shouldn't merge my root logger change | 23:17 |
| jeblair | (i think it needs some propogate settings to avoid logging things twice) | 23:18 |
| jeblair | two: paste isn't logging anything useful | 23:18 |
| jeblair | 2017-08-01 23:17:01,863 DEBUG paste.httpserver.ThreadPool: Added task (0 tasks queued) | 23:18 |
| jeblair | is all i get when i hit that 500 error | 23:18 |
| mordred | jeblair: that's not particularly useful | 23:18 |
| mordred | jeblair: might it be webob that we want logging for? | 23:21 |
| jeblair | mordred: i'm pretty sure the root logger did get us all warning+ logs at least | 23:22 |
| jeblair | the only problem there is it logs everything twice | 23:22 |
| mordred | gotcha | 23:22 |
| jeblair | mordred: i'm inclined to install a locally patched zuul with lots of debug info | 23:22 |
| mordred | jeblair: yah | 23:24 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Wrap handle_keys with debug statement https://review.openstack.org/489777 | 23:24 |
| mordred | jeblair: that was the first thing that came to mind ^^ | 23:24 |
| jeblair | mordred: yeah, i'll include that | 23:24 |
| pabelanger | sorry for asking and running away, #dadops | 23:25 |
| jeblair | and i borked it by doing 'pip install' | 23:25 |
| jeblair | restarted | 23:26 |
| jeblair | KeyError: 'gerrit-infra' | 23:27 |
| jeblair | bad regex? | 23:27 |
| jeblair | wait, no, the path we're passed is this: /keys/gerrit-infra/zuul-jobs.pub | 23:28 |
| jeblair | so it's something before handle_keys | 23:28 |
| mordred | something consuming tenant_name perhaps? | 23:28 |
| jeblair | tenant_name = request.path.split('/')[1] | 23:28 |
| jeblair | path = request.path.replace('/' + tenant_name, '') | 23:28 |
| jeblair | yep. | 23:28 |
| jeblair | if your tenant name is also in your project name. oops. | 23:29 |
| mordred | yah - because we don't hav ea handle_keys section i nthe upper pre-tenant try section | 23:29 |
| pabelanger | ah, that explains it | 23:29 |
| jeblair | okay, so here's how i think we should fix it: | 23:30 |
| jeblair | we want to change that to use project canonical names anyway, which make sense in a per-tenant context | 23:31 |
| jeblair | so let's do that, and drop the tenant stripping code entirely | 23:31 |
| jeblair | shall i do that? | 23:32 |
| mordred | jeblair: if we're going to do that - should we go ahead and move getting keys to zuul-web? or do the halfway thing and do the aiohttp patch in place? or just muscle this one in? | 23:32 |
| pabelanger | jeblair: I can take a stab at it in the morning, if you don't get to it first | 23:33 |
| jeblair | oh, hrm, we still need to strip the tenant name because most of the handlers expect that as an argument | 23:33 |
| jeblair | so we need to actually fix that. | 23:34 |
| jeblair | mordred: i'm kind of going for mininum needed to unblock job creation | 23:34 |
| jeblair | i'm not opposed to the other stuff, just that i don't want folks to sit around and wait for either of those. | 23:35 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix webapp path parsing https://review.openstack.org/489779 | 23:37 |
| jeblair | mordred, pabelanger: ^ i think that should do it? | 23:37 |
| mordred | jeblair: yah - I think that's simple enough | 23:37 |
| openstackgerrit | Monty Taylor proposed openstack-infra/zuul feature/zuulv3: Try handling keys before removing tenant name https://review.openstack.org/489780 | 23:38 |
| pabelanger | great | 23:38 |
| mordred | jeblair: we can also add that ^^ if we want to support keys without tenant | 23:38 |
| mordred | jeblair: but I don't know that we do want to support that | 23:38 |
| mordred | jeblair: oh - actually - the split is going to leave path witout a leading '/' | 23:39 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix webapp path parsing https://review.openstack.org/489779 | 23:40 |
| jeblair | mordred: ^ fixed | 23:40 |
| jeblair | mordred: i think everything in the web needs to be tenant-scoped | 23:40 |
| mordred | jeblair: cool | 23:40 |
| jeblair | (i realize that doesn't make sense with the current connection based thing, but i think tenant+project name is the right way to move forward) | 23:41 |
| mordred | jeblair: I think we need to circle back around on that and our rewrite/proxy rules - because we're rewriting /keys currently | 23:41 |
| mordred | jeblair: I agree | 23:41 |
| jeblair | mordred: well, we're doing what's needed to make zuulv3.o.o the view on the 'openstack' tenant | 23:42 |
| mordred | jeblair: oh - wait - this is more broken | 23:42 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix webapp path parsing https://review.openstack.org/489779 | 23:42 |
| mordred | jeblair: OH - because we pass in zuul_status_url with an openstack appended | 23:43 |
| mordred | jeblair: anywho, sorry for the trickle of a review, but - I think that code is going to fail on leading / | 23:44 |
| mordred | jeblair: "/openstack/gerrit/openstack-infra/zuul".split('/', 1) -> ['', 'openstack/gerrit/openstack-infra/zuul'] | 23:44 |
| mordred | I'm not sure if there's a leading / in the initial webob path though | 23:45 |
| jeblair | mordred: there was already a split there, i assumed that part worked at least | 23:46 |
| jeblair | mordred: oh, subscript 1 | 23:47 |
| jeblair | sigh | 23:47 |
| mordred | jeblair: I couldn't figure out why the earlier thing worked... subscript 1 indeed | 23:47 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul feature/zuulv3: Fix webapp path parsing https://review.openstack.org/489779 | 23:48 |
| mordred | jeblair: YES | 23:48 |
| mordred | jeblair: I am now certain that will work | 23:48 |
| jeblair | \o/ | 23:50 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!