| clarkb | corvus: pabelanger on the fixing skipped success checking I left a comment with a refactor of that loop. I had a really hard time understanding corvus' comment initially when mapped to the for loop there due to how its branching happened. I tried to simplify by only jumping if we determing it was an unsuccessful build | 00:16 |
|---|---|---|
| corvus | yes, i don't think it's a one-line fix | 00:17 |
| clarkb | basically rather than continuing on the "ok" cases just check for the bad cases and return if we find them | 00:18 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: WIP: Add upload-logs-swift role https://review.openstack.org/584541 | 00:27 |
| corvus | okay, that's not quite ready to land, but it's 99% of the way there. just a few more things to tidy up | 00:27 |
| corvus | (i ran into another ceph-radosgw / swift difference which turned out to be a rabbit hole: with ceph, you need an index.html file at the container root in order for any indexes at any other path to work) | 00:28 |
| *** rlandy has joined #zuul | 00:52 | |
| openstackgerrit | Ian Wienand proposed openstack-infra/nodepool master: Use pipelines for stats keys https://review.openstack.org/585577 | 01:15 |
| *** GonZo2000 has quit IRC | 01:17 | |
| openstackgerrit | Ian Wienand proposed openstack-infra/nodepool master: Use pipelines for stats keys https://review.openstack.org/585577 | 01:18 |
| *** rlandy has quit IRC | 01:24 | |
| *** jiapei has joined #zuul | 01:38 | |
| *** maeca has quit IRC | 02:00 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool master: Implement an OpenShift resource provider https://review.openstack.org/570667 | 04:07 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: executor: add support for generic build resource https://review.openstack.org/570668 | 04:07 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul-base-jobs master: Add base openshift job https://review.openstack.org/570669 | 04:07 |
| tristanC | mordred: corvus: as far as i understand the container-build-resources spec, this is a working implementation of both use-case on openshift: https://review.openstack.org/#/q/topic:container-build-resources | 04:10 |
| tristanC | https://review.openstack.org/570669 includes base jobs documentation and examples | 04:11 |
| *** swest has joined #zuul | 04:35 | |
| *** swest has quit IRC | 04:48 | |
| *** swest has joined #zuul | 04:49 | |
| *** dtruong2 has joined #zuul | 05:07 | |
| *** jiapei has quit IRC | 05:18 | |
| openstackgerrit | Ian Wienand proposed openstack-infra/nodepool master: Use pipelines for stats keys https://review.openstack.org/585577 | 05:23 |
| *** nchakrab has joined #zuul | 05:36 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: web: add /{tenant}/pipelines route https://review.openstack.org/541521 | 06:11 |
| *** pcaruana has joined #zuul | 06:33 | |
| *** dtruong2 has quit IRC | 06:34 | |
| openstackgerrit | Ian Wienand proposed openstack-infra/nodepool master: Use pipelines for stats keys https://review.openstack.org/585577 | 06:46 |
| *** yolanda_ has quit IRC | 07:00 | |
| *** yolanda has joined #zuul | 07:22 | |
| *** bbayszczak has joined #zuul | 07:40 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool master: Implement an OpenShift resource provider https://review.openstack.org/570667 | 07:40 |
| *** nchakrab has quit IRC | 08:20 | |
| *** hellofreenodemyo has joined #zuul | 08:30 | |
| *** hellofreenodemyo has quit IRC | 08:31 | |
| *** nchakrab has joined #zuul | 08:44 | |
| *** openstackgerrit has quit IRC | 08:48 | |
| *** panda|off is now known as panda | 08:57 | |
| *** GonZo2000 has joined #zuul | 09:59 | |
| *** GonZo2000 has joined #zuul | 09:59 | |
| *** sshnaidm|ruck is now known as sshnaidm|afk | 10:29 | |
| *** sshnaidm|afk is now known as sshnaidm|ruck | 11:00 | |
| *** panda is now known as panda|lunch | 11:26 | |
| *** samccann has joined #zuul | 11:51 | |
| *** rlandy has joined #zuul | 12:43 | |
| *** bbayszczak has quit IRC | 12:43 | |
| *** panda|lunch is now known as panda | 12:53 | |
| *** maeca has joined #zuul | 13:04 | |
| *** maeca has quit IRC | 13:05 | |
| *** ssbarnea has quit IRC | 13:05 | |
| *** openstackgerrit has joined #zuul | 13:24 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: WIP: Support job pause https://review.openstack.org/585389 | 13:24 |
| tobiash | that should be feature complete and also works in my test environment ^ | 13:25 |
| tobiash | it lacks docs and a bit of test coverage yet | 13:25 |
| *** cmurphy is now known as cmurphy|vacation | 13:29 | |
| pabelanger | tobiash: cool! | 13:33 |
| *** dkranz has joined #zuul | 14:12 | |
| *** nchakrab has quit IRC | 14:13 | |
| *** nchakrab has joined #zuul | 14:14 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul master: Fix zuul reporting build failure with only non-voting jobs https://review.openstack.org/584990 | 14:17 |
| *** ssbarnea has joined #zuul | 14:17 | |
| pabelanger | clarkb: thanks, i've updated ^ based on your comment (code) | 14:17 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool master: Implement an OpenShift resource provider https://review.openstack.org/570667 | 14:48 |
| *** pcaruana has quit IRC | 14:52 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul master: Fix zuul reporting build failure with only non-voting jobs https://review.openstack.org/584990 | 14:59 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul-base-jobs master: Add base openshift job https://review.openstack.org/570669 | 15:01 |
| clarkb | corvus: haven't done a comprehensive review of swift upload change yet but did note a few things I noticed | 15:04 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool master: Use hostname for launch exception https://review.openstack.org/585781 | 15:07 |
| corvus | clarkb: ah thanks! | 15:08 |
| *** samccann has quit IRC | 15:27 | |
| *** sshnaidm|ruck is now known as sshnaidm | 15:46 | |
| *** panda is now known as panda|rover | 15:46 | |
| *** sshnaidm is now known as sshnaidm|bbl | 15:56 | |
| *** panda|rover is now known as panda|rover|off | 15:57 | |
| *** dtruong2 has joined #zuul | 15:57 | |
| *** samccann has joined #zuul | 15:58 | |
| *** myoung is now known as myoung|lunch | 16:08 | |
| *** chkumar|rover is now known as chandankumar | 16:24 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Add upload-logs-swift role https://review.openstack.org/584541 | 16:26 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Add a python unit test framework https://review.openstack.org/585813 | 16:26 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Restore zuul_swift_upload.py script https://review.openstack.org/585814 | 16:26 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: DNM: Run tox-py35 job https://review.openstack.org/585815 | 16:26 |
| *** elyezer has quit IRC | 16:29 | |
| *** samccann has quit IRC | 16:30 | |
| *** samccann has joined #zuul | 16:30 | |
| *** GonZo2000_ has joined #zuul | 16:34 | |
| *** GonZo2000 has quit IRC | 16:36 | |
| *** dtruong2 has quit IRC | 16:42 | |
| corvus | mordred, clarkb, tobiash, jhesketh: ^ i think that stack is ready now | 16:47 |
| clarkb | cool I'll try to make time soon to take another look, once the random infra fires are dealt with | 16:47 |
| corvus | i'll start working on usage in project-config | 16:48 |
| *** GonZo2000_ has quit IRC | 16:48 | |
| *** elyezer has joined #zuul | 16:51 | |
| Shrews | corvus: oh, that unit test framework for roles is neat | 17:03 |
| corvus | Shrews: \o/ | 17:03 |
| clarkb | corvus: Shrews do those test run in the zuul jobs? | 17:07 |
| corvus | clarkb: they'll run under tox-py35 like normal python projects. that doesn't run on the zuul-jobs repo right now, but we can add it once the framework lands. | 17:07 |
| corvus | (i pushed up a change to .zuul.yaml at the end of the stack to temporarily run it so we can verify it) | 17:08 |
| clarkb | ok couple more comments on https://review.openstack.org/#/c/584541/3 neither is is likely to be world breaking right now, but probably good to sort them early | 17:10 |
| clarkb | pabelanger: corvus https://review.openstack.org/#/c/584990/4 lgtm | 17:14 |
| Shrews | corvus: i wonder if the default for zuul_site_upload_logs should be more conservative | 17:15 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool master: Use hostname for launch exception https://review.openstack.org/585781 | 17:15 |
| Shrews | maybe 'failure' | 17:15 |
| *** sshnaidm|bbl is now known as sshnaidm | 17:16 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Add upload-logs-swift role https://review.openstack.org/584541 | 17:16 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: DNM: Run tox-py35 job https://review.openstack.org/585815 | 17:16 |
| corvus | Shrews: well, we put that in there when everything was on fire, and we wanted to limp along while we restored the system. it's a good knob to have in an emergency, but the experience of only having log files when jobs fail is very bad so i wouldn't want that to be the default. | 17:17 |
| clarkb | ++ | 17:18 |
| Shrews | fair enough | 17:18 |
| corvus | clarkb, pabelanger: 584990 lgtm; tobiash do you want one more look at that, or should i +W it ? | 17:19 |
| corvus | ianw: ^ also | 17:20 |
| clarkb | considering I helped rewrite that one loop having another set of eyeballs on it might be nice | 17:20 |
| clarkb | corvus: oh I think I know why the ret = '' worked fine, compressing an empty file results in 37Byte output or similar | 17:21 |
| clarkb | so we'll always overwrite the default ret value | 17:21 |
| clarkb | that said I think making it b'' helps make it clear that value is binary string | 17:22 |
| clarkb | (that is what zlib returns) | 17:22 |
| corvus | ah. ++ | 17:22 |
| pabelanger | corvus: clarkb: great! | 17:25 |
| Shrews | pabelanger: ignore my comment on the node hostname thingy | 17:28 |
| Shrews | i totally spaced on the id being IN the hostname | 17:29 |
| Diabelko | http://logs.opencontrail.org/70/44770/10/check/contrail-sanity-centos7-kolla-ocata/f62646b/job-output.txt.gz#_2018-07-25_16_52_39_134178 can someone please explain it to me? this is the playbook and the review: https://review.opencontrail.org/#/c/44766/23/playbooks/contrail-sanity-kolla-base/run.yaml | 17:29 |
| Diabelko | suddenly variable contrail_version changes :D | 17:29 |
| Diabelko | the issue I believe is not actually caused by that review and in fact is the foundation of our pipeline | 17:30 |
| Diabelko | because if it wouldn't suddenly change, our jobs wouldn't work :D | 17:30 |
| clarkb | Diabelko: you are aksing why the subsequent echos of that variable are different? | 17:31 |
| Diabelko | yes | 17:32 |
| Diabelko | https://review.opencontrail.org/#/c/44766/25 this is the current patchset actually where I've added those debug tasks | 17:33 |
| dmsimard | I forgot how awful jenkins was :( | 17:35 |
| clarkb | Diabelko: where is the variable defined? it isn't supplied as part of the inventory | 17:36 |
| corvus | Diabelko: i guess any set_fact could change it, or even (as we learned here the other day) any module could change it by returning an "ansible_facts" dictionary item. maybe add more debug statements and find out which task is changing it to help narrow it down? | 17:38 |
| tobiash | corvus: 584990 lgtm | 17:38 |
| corvus | Diabelko: those all look like pretty simple commands (with only the package task being a library module). i suspect the answer is going to be weird and surprise us. :) | 17:39 |
| clarkb | corvus: the two debug tasks run back to back so ya probably going to be surprising :) | 17:39 |
| Diabelko | clarkb: it's returned by one of the pre-tasks http://logs.opencontrail.org/70/44770/10/check/contrail-sanity-centos7-kolla-ocata/f62646b/ara/result/551f82b8-46b9-4073-a711-cfe7c531eda6/ | 17:39 |
| corvus | clarkb: oh, i thought there were a bunch of tasks between them | 17:40 |
| Diabelko | (we read a file and return it to zuul) | 17:40 |
| corvus | i may be looking at the wrong place | 17:40 |
| clarkb | corvus: there is a debug at start of tasks file and at end, the end of one is back to back with start of toher according to ara | 17:40 |
| Diabelko | exactly that | 17:41 |
| corvus | okay, which tasks file are we looking at? | 17:41 |
| Diabelko | I can even add small role with exactly the same stuff to see if it happens in between | 17:41 |
| pabelanger | are you using include_role? | 17:41 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul master: Default to 0o022 for python-daemon https://review.openstack.org/585068 | 17:42 |
| clarkb | corvus: https://review.opencontrail.org/#/c/44766/25/roles/prepare-template-for-ansible-deployer/tasks/main.yaml runs then https://review.opencontrail.org/#/c/44766/25/roles/contrail-ansible-deployer/tasks/main.yaml runs if I am reading ara correctly | 17:42 |
| Diabelko | pabelanger: yes, there's probably include somehwere in bvetween | 17:43 |
| pabelanger | ianw: clarkb: tobiash: corvus: ^585068 was something I noticed looking at executor logs, current suggestion was umask setting. If you'd like to add to your review pipeline | 17:43 |
| Diabelko | and 'include' specifically, not 'include_role' which doesn't work for us on ansible 2.3 and older zuul | 17:43 |
| corvus | oh. i understand. yes, that's weird, there should be nothing between those two debug statements? | 17:43 |
| Diabelko | exactly | 17:44 |
| pabelanger | Diabelko: include_role should be okay with 2.2, but that is another discussion :) | 17:44 |
| Diabelko | pabelanger: mmm, I might be wrong then, but somehow I remember the decision of sticking to include only | 17:45 |
| *** samccann has quit IRC | 17:47 | |
| *** maeca has joined #zuul | 17:48 | |
| clarkb | Diabelko: contrail_version doesn't seem to be in that zuul_return value? | 17:49 |
| *** maeca1 has joined #zuul | 17:49 | |
| *** samccann has joined #zuul | 17:50 | |
| Diabelko | oh my | 17:51 |
| Diabelko | I think I got it | 17:51 |
| * corvus is on the edge of his seat | 17:51 | |
| Diabelko | we have a default in role prepare-template-for-ansible-deployer | 17:52 |
| Diabelko | for contrail_version | 17:52 |
| Diabelko | then we run role ansible-deployer | 17:52 |
| Diabelko | which takes that default | 17:52 |
| *** maeca has quit IRC | 17:52 | |
| Diabelko | oh wait, no | 17:52 |
| Diabelko | it's different default | 17:52 |
| Diabelko | corvus: sorry for getting your hopes up then :P | 17:53 |
| clarkb | roles/sanitytest/defaults/main.yaml seems to be where the ocata version comes from at least | 17:53 |
| Diabelko | yes | 17:53 |
| *** myoung|lunch is now known as myoung | 17:53 | |
| Diabelko | we have contrail-zuul-jobs and contrail-project-config repos on github, if it's easier for you | 17:55 |
| Diabelko | https://github.com/juniper/contrail-zuul-jobs/ https://github.com/juniper/contrail-project-config/ | 17:55 |
| clarkb | is it possible that ansible loads defaults for future roles early? | 17:58 |
| clarkb | I pretty stumped otherwise | 17:59 |
| corvus | i've been looking for something like a vars file getting copied over and read during the next role, but so far haven't found anything like that | 18:00 |
| openstackgerrit | Merged openstack-infra/zuul master: Fix zuul reporting build failure with only non-voting jobs https://review.openstack.org/584990 | 18:00 |
| corvus | Diabelko: prepare-template-for- | 18:01 |
| corvus | grr | 18:01 |
| corvus | Diabelko: prepare-template-for-ansible-deployer has a defaults/main.yaml which matches what it outputs. sanitytest has a defaults/main.yaml which matches what it outputs. contrail-ansible-deployer doesn't have contrail_version in its defaults/main.yaml. in fact, other than the debug output, there are no references to it in that role. all of this is what clarkb just said but with more words, i think. | 18:07 |
| corvus | if that's the case -- are we basically looking at case of referencing an undefined value? | 18:09 |
| corvus | (and, to clark's point, being surprised that the undefined value happens to be a future value....) | 18:09 |
| Diabelko | but isn't it the case that contrail_version value should stay the way it was at prepare-template-for-ansible-deployer? | 18:09 |
| corvus | i'm not sure that role defaults have an impact outside of the role | 18:10 |
| pabelanger | yah, with include_role you can control that | 18:11 |
| Diabelko | corvus: ansible docs says it works for included or dependent roles | 18:11 |
| Diabelko | I am not sure what they mean by the latter | 18:11 |
| clarkb | these roles aren't included though right? just listed as roles? or maybe that is equivalent? | 18:12 |
| corvus | pabelanger: in this case, these 3 roles are run in sequence in a playbook | 18:12 |
| corvus | Diabelko: i think dependent roles are where a role declares an explicit dependency on another | 18:12 |
| corvus | Diabelko: like this: https://docs.ansible.com/ansible/2.5/user_guide/playbooks_reuse_roles.html#role-dependencies | 18:12 |
| tobiash | corvus: is the warning about the incomplete api supposed to land with the stack or do you want to remove it? | 18:13 |
| Diabelko | corvus: got it, thanks | 18:13 |
| tobiash | corvus: re swift stack | 18:13 |
| corvus | so i think i would expect these variables to only have effect within each role. i'm not 100% sure on that. but if that's the case, that at least explains why prepare-template and sanity have different values. it doesn't explain why deployer has a value at all. | 18:14 |
| corvus | tobiash: yes. i also left the role name "wip-" just to make the point even more clear :) | 18:15 |
| corvus | tobiash: i don't want to accept any backwards-compatibility liability yet | 18:15 |
| tobiash | corvus: ah so the plan is to test it for some time as tech preview? | 18:15 |
| tobiash | ok, got it | 18:16 |
| corvus | tobiash: hopefully not long. but i'd at least like to see it work a little bit :) | 18:16 |
| tobiash | corvus: I plan to try this role in our environment soon (hopefully short after the buildset resources are polished) | 18:16 |
| corvus | tobiash: and maybe you can test it out locally too since you had urgent need of it. | 18:17 |
| pabelanger | I would expect, if a role set a variable in defaults/main.yaml, and used in a play, other playbook tasks would be able to use the variable. However, I am not sure if the next role would see that variable, unless explicitly passed in using vars setting | 18:17 |
| pabelanger | I think you'd use set_fact for that, inter role variables | 18:19 |
| Diabelko | might it be the case ansible tries to preload variables if they are no longer needed to make it faster or I just went way too far? | 18:19 |
| *** nchakrab has quit IRC | 18:19 | |
| corvus | clarkb, jhesketh, Shrews: the old zuul_swift_upload script doesn't pass the linters test, so i can't have it in its own patch. i'm going to pull it out of the stack. if you want to diff against it, you can do so locally. | 18:21 |
| Diabelko | all right, thank you guys anyway, I'll just put proper defaults and values | 18:21 |
| Diabelko | but it was cool to see | 18:21 |
| Diabelko | uncool to debug though | 18:22 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Add upload-logs-swift role https://review.openstack.org/584541 | 18:23 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: DNM: Run tox-py35 job https://review.openstack.org/585815 | 18:23 |
| corvus | tobiash, clarkb, mordred: oh, hrm. secrets can only be key/value pairs. we don't support dictionaries as secrets. but ansible variables can be complex data structures (lists, dicts).... | 18:27 |
| corvus | that sort of puts a crimp in my plan to make a clouds.yaml secret and pass it as input to the role. see https://review.openstack.org/585830 | 18:27 |
| tobiash | corvus: you can just add yaml or json as string and use the json filter to make a dict in ansible | 18:28 |
| corvus | tobiash: but that won't be readable in the config file | 18:28 |
| corvus | any reason we can't extend zuul secrets to support complex data structures? | 18:29 |
| tobiash | corvus: I'm not opposed to that | 18:29 |
| clarkb | the encrypted values would still just be bytestrings? | 18:29 |
| corvus | tobiash: we can implement your idea as a workaround at least to keep things moving | 18:30 |
| tobiash | corvus: but as a quick fix it's possible to encode a yaml file as string and use this for parsing in ansible: '{{ some_variable | from_yaml }}' | 18:30 |
| corvus | clarkb: yeah, i'm just suggesting that we allow them to be in nested dictionaries or lists within the secret | 18:31 |
| corvus | we'd have to recurse through them to decrypt | 18:32 |
| corvus | gotta run to lunch now, biab. | 18:34 |
| mhu | hello, can the change adding dequeueing in the CLI get some reviewing please? https://review.openstack.org/#/c/95035/ it's sitting at +2 | 18:39 |
| *** maeca1 has quit IRC | 18:41 | |
| *** dtruong_ has joined #zuul | 18:45 | |
| *** dtruong has quit IRC | 18:49 | |
| clarkb | corvus: when you return, latest patchset on https://review.openstack.org/584541 seems to revert the b'' change and the reorg of imports? | 18:51 |
| *** maeca has joined #zuul | 18:54 | |
| clarkb | mhu: left a question | 18:57 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Add upload-logs-swift role https://review.openstack.org/584541 | 19:07 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: DNM: Run tox-py35 job https://review.openstack.org/585815 | 19:07 |
| corvus | clarkb: thx fixed | 19:07 |
| *** myoung is now known as myoung|bbl | 19:12 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: WIP: Support job pause https://review.openstack.org/585389 | 19:23 |
| openstackgerrit | Merged openstack-infra/nodepool master: Use hostname for launch exception https://review.openstack.org/585781 | 19:43 |
| Diabelko | corvus: clarkb: pabelanger: https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse_roles.html | 19:49 |
| Diabelko | "If roles/x/defaults/main.yml exists, variables listed therein will be added to the play." this otoh suggests that it will be added to the play, not only role scope | 19:49 |
| Diabelko | and if ansible builds them from the top, the default value from the last role in the play would kick in | 19:49 |
| openstackgerrit | Matthieu Huin proposed openstack-infra/zuul master: Add a dequeue command to zuul client https://review.openstack.org/95035 | 19:51 |
| corvus | Shrews, clarkb, pabelanger, mordred: we're looking at tagging 67824d8e64250ede0cb4a3ebe344412f5cba2218 as 3.2.0 ? | 20:01 |
| corvus | (nodepool) | 20:01 |
| * Shrews looks | 20:01 | |
| Shrews | corvus: correct | 20:02 |
| corvus | something like: http://paste.openstack.org/show/726648/ | 20:02 |
| Shrews | yeah. i think going from 3.1.0 -> 3.2.0 is warranted given the bigger changes (zk model, shade-to-sdk) | 20:04 |
| corvus | yeah, that's sorta what i was thinking | 20:04 |
| pabelanger | +1 for 3.2.0 and 67824d | 20:06 |
| Shrews | hrm... did we not document the shade-to-sdk change? | 20:06 |
| mordred | corvus: ++ | 20:06 |
| Shrews | https://zuul-ci.org/docs/nodepool/releasenotes.html#in-development | 20:06 |
| mordred | Shrews: I don't think we did | 20:07 |
| Shrews | should we? | 20:07 |
| pabelanger | might be helpful for packagers | 20:08 |
| corvus | i'm willing to be convinced either way; i don't have a feeling for how many folks might be using deployment mechanisms that wouldn't pick up on that automatically. | 20:08 |
| corvus | we did note the addition of re2 in zuul -- but that required a local c library install as well | 20:10 |
| Shrews | This doesn't change any functionality, so I guess I'm ok with no note since it should be invisible | 20:11 |
| mordred | yah - shade actually introduced a depend on openstacksdk about 6 months ago | 20:14 |
| mordred | so anyone running nodepool already has openstacksdk installed | 20:14 |
| corvus | oh that's even more invisible then | 20:14 |
| mordred | just as a transitive | 20:14 |
| mordred | yah | 20:14 |
| mordred | the main thing that might be worthy of note | 20:14 |
| Shrews | yeah. release the hounds then | 20:14 |
| corvus | only issue would be to let folks know they could clean up the old dep | 20:14 |
| mordred | is configuring logging to log openstack loggers instead of shade loggers | 20:14 |
| mordred | (and cleaning up the old dep) | 20:14 |
| pabelanger | Oh, hmm. maybe a note is good, to let people know they might need to update their logging.conf files | 20:16 |
| Shrews | ooh, logging. didn't consider that | 20:16 |
| pabelanger | is shade a handler still? | 20:16 |
| pabelanger | https://github.com/openstack-infra/puppet-nodepool/blob/master/templates/nodepool-launcher.logging.conf.erb | 20:16 |
| Shrews | pabelanger: no. i have a change up to remove that we should +W now | 20:16 |
| Shrews | https://review.openstack.org/583333 | 20:17 |
| Shrews | but it seems i missed that template | 20:17 |
| pabelanger | k, I'm now leaning towards a reno entry | 20:17 |
| pabelanger | due to logging.conf changes | 20:18 |
| Shrews | pabelanger: so are the config files in system-config unused at this point? | 20:19 |
| clarkb | the logging change isnt a change though right? that preserves old behavior | 20:20 |
| pabelanger | Shrews: which config is that? | 20:20 |
| mordred | clarkb: well, if a person had a logging config file (like we did) that has shade configured ... that logging config would cease logging things | 20:20 |
| Shrews | pabelanger: https://review.openstack.org/#/c/583333/1/modules/openstack_project/templates/nodepool/nodepool-builder.logging.conf.erb | 20:20 |
| pabelanger | Shrews: Hmm, let me check | 20:21 |
| pabelanger | I thought it was done in puppet-nodepool | 20:21 |
| Shrews | that same file is in puppet-nodepool | 20:21 |
| Shrews | we could be running nodepool with no logging for openstacksdk right now | 20:21 |
| clarkb | mordred: oh that logging config not the string munging | 20:22 |
| pabelanger | Shrews: seems like we overload it from system-config, not sure why | 20:22 |
| pabelanger | Shrews: possible we could change that and fall back to puppet-nodepool again, would need to read history | 20:22 |
| mordred | pabelanger: or - just leave it until we ansible it | 20:23 |
| mordred | :) | 20:23 |
| corvus | we don't actually recommend people set up local logging configs anymore, right? | 20:23 |
| Shrews | confirmed: we are not logging openstacksdk in prod | 20:23 |
| Shrews | dang it | 20:24 |
| mordred | Shrews: but I think we're also not missing anything in the logs that we want | 20:24 |
| mordred | or - maybe we are :) | 20:24 |
| Shrews | it's WARNING level | 20:24 |
| corvus | like, if you don't write a logging.conf, you get @info (and kazoo @warning). which is good. the situation for debug logging isn't as great, so those of us hacking on nodepool probably still need a local config. | 20:24 |
| mordred | yeah | 20:25 |
| Shrews | remote: https://review.openstack.org/585876 Update nodepool launcher logging | 20:25 |
| Shrews | is WARNING what we want for sdk? | 20:26 |
| corvus | i don't think we should see anything from sdk unless there's a problem | 20:27 |
| Shrews | pabelanger: ok, something is off. the builders get the logging config from system-config, but launchers puppet-nodepool | 20:27 |
| Shrews | (based on what i see in nl01 and nb01) | 20:28 |
| pabelanger | Shrews: yah, some of that is a mess due to zuulv3 migration I think. | 20:28 |
| pabelanger | due for a good clean up | 20:29 |
| mordred | pabelanger: ++ | 20:29 |
| Shrews | and multiple logging conf files for each, making it even more confusion | 20:30 |
| corvus | i don't feel compelled to write a release note for this because we don't require a logging config. so i'm still happy to go either way. | 20:30 |
| pabelanger | yes, we still support nodepoolv2 in puppet-nodepool for 3pci | 20:30 |
| mordred | corvus: same here | 20:30 |
| mordred | I would not vote against a release note, but nor would I suggest we hold off waiting for one | 20:31 |
| pabelanger | +1 | 20:31 |
| pabelanger | no objection to tagging | 20:31 |
| clarkb | ya Ithink its probably fine | 20:32 |
| corvus | Shrews: still ambivalent? | 20:32 |
| pabelanger | stepping away to do some meal prep | 20:33 |
| Shrews | corvus: yeah | 20:34 |
| Shrews | just trying to sort this logging craziness out | 20:34 |
| corvus | okay, the "meh"s have it. motion carries. i'll push the tag now. | 20:34 |
| * Shrews carries the convo over to #-infra | 20:34 | |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Add a python unit test framework https://review.openstack.org/585813 | 20:45 |
| openstackgerrit | Merged openstack-infra/zuul master: Default to 0o022 for python-daemon https://review.openstack.org/585068 | 20:52 |
| mordred | corvus: ^^ the "Add a python unit test framework" is super cool- and makes me think that load_tests method could/should go in a library somewhere | 20:54 |
| *** openstackgerrit has quit IRC | 21:04 | |
| *** samccann has quit IRC | 21:05 | |
| corvus | mordred: it's super groovy | 21:07 |
| corvus | tobiash: https://review.openstack.org/582794 do you have any thoughts on my comment there? | 21:07 |
| corvus | mordred: can you look at https://review.openstack.org/546474 when you get a chance? | 21:12 |
| clarkb | tristanC: reading through the openshift base job stack. One thing that stands out to me is that you are setting up the git webserver for basic http which is really slow aiui. Maybe using git:// or ssh:// or even configuring the cgi binary for smart http is a good idea? | 21:12 |
| clarkb | its probably fine for small repos but as you get into larger ones I expect you'll notice the difference | 21:13 |
| clarkb | not critical for a poc but might be easier to update before people start using it a bunch | 21:21 |
| *** openstackgerrit has joined #zuul | 21:26 | |
| openstackgerrit | Merged openstack-infra/zuul master: Add remote tests for synchronize https://review.openstack.org/584655 | 21:26 |
| clarkb | Diabelko: to rule out the defaults being the problem maybe add a defaults file for each of those roles and set contrail_version or use set fact on it early on? | 21:37 |
| clarkb | Diabelko: if it continues after doing that then something else is likely setting it in a more forceful manner | 21:38 |
| clarkb | tristanC: the other thing that is a bit odd is that the openshift stuff is way more openshift specific than I was expecting, but I guess this is where openshift is useful with the image management that k8s doesn't provide out of the box? | 21:51 |
| clarkb | if nothing else I think that confirms my concern that we need a generic k8s option too | 21:51 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Support complex data structures as secrets https://review.openstack.org/585892 | 21:58 |
| corvus | clarkb: well, if i recall, the spec is written to support bare k8s, so we shouldn't be seeing any openshift-specific stuff in our first pass. | 21:59 |
| clarkb | corvus: tristanC's first pass is openshift specific (not sure a bad thing, just that it confirms bare k8s is going to need to be a thing too) | 22:00 |
| corvus | clarkb: well, i mean, that's why we wrote a spec, right? to agree on what we're going to implement? and one of the feedback items which we accepted was that we should support k8s to start with. | 22:01 |
| clarkb | ++ | 22:01 |
| clarkb | I guess what I am saying is I expected openshift to be more like k8s and its well not :) | 22:01 |
| corvus | as i understand, it is k8s but with more stuff. what we write to support k8s should work in openshift. | 22:02 |
| clarkb | ya I think if you still to the k8s apis they are compatible or you can use the fancy openshift stuff like ImageStream and friends | 22:02 |
| corvus | clarkb, mordred, tobiash: https://review.openstack.org/585892 should make https://review.openstack.org/585830 possible | 22:05 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Support complex data structures as secrets https://review.openstack.org/585892 | 22:08 |
| clarkb | corvus: in the config schema update you changed {str: vs.Any(str, EncryptedPKCS1_OAEP)} to dict, does dict in voluptuous imply all the things? | 22:09 |
| corvus | (teensy doc update. turns out we did document that the values were either strings or encrypted values.) | 22:09 |
| corvus | clarkb: basically we're telling voluptuous to stop there -- as long as data is a dict (regardless of the contents of that dict) it's happy | 22:09 |
| corvus | so... yes. | 22:10 |
| clarkb | oh got it, the old data was in a dict just specific types | 22:10 |
| corvus | yep | 22:10 |
| clarkb | now its dict iwthout specific types | 22:10 |
| *** eandersson has quit IRC | 22:22 | |
| *** eandersson has joined #zuul | 22:24 | |
| *** sshnaidm is now known as sshnaidm|afk | 22:38 | |
| *** myoung|bbl is now known as myoung | 23:00 | |
| *** maeca has quit IRC | 23:14 | |
| *** rlandy has quit IRC | 23:48 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!