| *** jamesmcarthur has quit IRC | 00:22 | |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Fix build-docker-image when using buildset_registry https://review.openstack.org/637650 | 00:25 |
|---|---|---|
| *** sdake has joined #zuul | 00:25 | |
| *** chandankumar has quit IRC | 00:28 | |
| *** chandankumar has joined #zuul | 00:29 | |
| clarkb | in cleaning up the issue queries I notice that we search all queries to find the PR depends on | 00:30 |
| clarkb | any idea if we can search PRs instead (would simplify the test suite as we could delete some fakes I think) | 00:30 |
| clarkb | hrm reading the api docs search for PRs is search for issues | 00:35 |
| *** jamesmcarthur has joined #zuul | 00:35 | |
| *** jamesmcarthur has quit IRC | 00:40 | |
| *** sdake has quit IRC | 00:58 | |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Don't request PR issue data https://review.openstack.org/636728 | 00:59 |
| clarkb | I think ^ will pass tests now. I was waiting for testsuite to finish locally but its far too slow for that and I need to make dinner | 01:00 |
| clarkb | I had to update the test suite/fakes to deal with the changes. Also we should have someone with better knowledge of github api and github3 confirm that should work probably | 01:01 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-preview master: WIP: test docker registry https://review.openstack.org/637037 | 01:01 |
| *** sdake has joined #zuul | 01:15 | |
| *** sdake has quit IRC | 01:16 | |
| *** rlandy has quit IRC | 01:18 | |
| *** sdake has joined #zuul | 01:21 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: amqp: add basic trigger https://review.openstack.org/637458 | 01:32 |
| *** sdake has quit IRC | 01:42 | |
| *** sdake has joined #zuul | 01:43 | |
| *** jamesmcarthur has joined #zuul | 01:46 | |
| *** sdake has quit IRC | 01:54 | |
| *** jamesmcarthur has quit IRC | 01:56 | |
| *** jamesmcarthur has joined #zuul | 01:58 | |
| *** sdake has joined #zuul | 02:01 | |
| *** sdake has quit IRC | 02:22 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: amqp: add message informations to the job variables https://review.openstack.org/637666 | 02:27 |
| *** jamesmcarthur has quit IRC | 02:41 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: amqp: add message informations to the job variables https://review.openstack.org/637666 | 03:08 |
| openstackgerrit | Ian Wienand proposed openstack-infra/nodepool master: [dnm] testing devstack fix https://review.openstack.org/637669 | 03:11 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: web: add triggers information to pipeline list https://review.openstack.org/637670 | 03:13 |
| *** cognifloyd has joined #zuul | 03:33 | |
| *** rfolco has quit IRC | 03:36 | |
| fungi | clarkb: 637218 seems to be suffering from a merge conflict | 03:45 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: webtrigger: add initial driver and event https://review.openstack.org/555153 | 04:01 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: webtrigger: add web route and rpclistener https://review.openstack.org/554839 | 04:01 |
| *** andreaf has quit IRC | 04:08 | |
| *** andreaf has joined #zuul | 04:09 | |
| *** jlvillal has quit IRC | 04:14 | |
| *** jlvillal has joined #zuul | 04:14 | |
| *** Shrews has quit IRC | 04:17 | |
| *** Shrews has joined #zuul | 04:19 | |
| *** Shrews has quit IRC | 04:24 | |
| *** Shrews has joined #zuul | 04:25 | |
| *** jamesmcarthur has joined #zuul | 04:26 | |
| *** jamesmcarthur has quit IRC | 04:31 | |
| *** chandankumar is now known as chkumar|ruck | 04:32 | |
| *** irclogbot_3 has quit IRC | 04:55 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: amqp: add message informations to the job variables https://review.openstack.org/637666 | 04:57 |
| *** jamesmcarthur has joined #zuul | 04:58 | |
| *** bhavikdbavishi has joined #zuul | 05:00 | |
| *** jamesmcarthur has quit IRC | 05:03 | |
| *** jamesmcarthur has joined #zuul | 05:38 | |
| *** jamesmcarthur has quit IRC | 05:43 | |
| *** bjackman has joined #zuul | 05:46 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul-jobs master: install-kubernetes: fix kube config permission https://review.openstack.org/637682 | 05:48 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool master: DNM: test install-kubernetes fix https://review.openstack.org/637683 | 05:49 |
| tristanC | zuul-maint, looking for review on https://review.openstack.org/632620 to be able to use add-build-sshkey on static node. thanks :) | 05:50 |
| *** jamesmcarthur has joined #zuul | 06:10 | |
| *** jamesmcarthur has quit IRC | 06:15 | |
| *** badboy has joined #zuul | 06:19 | |
| *** quiquell|off is now known as quiquell|rover | 06:29 | |
| *** sanjayu__ has joined #zuul | 06:33 | |
| *** sanjayu__ has quit IRC | 06:38 | |
| *** saneax has joined #zuul | 06:59 | |
| *** jamesmcarthur has joined #zuul | 07:11 | |
| *** jamesmcarthur has quit IRC | 07:15 | |
| *** quiquell|rover is now known as quique|rover|r-- | 07:26 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul-jobs master: install-kubernetes: fix minikube config permission https://review.openstack.org/637682 | 07:32 |
| *** jamesmcarthur has joined #zuul | 07:48 | |
| *** jamesmcarthur has quit IRC | 07:53 | |
| zbr | is it true that having a files section in job definition would prevent zuul from running periodic jobs? | 08:07 |
| zbr | i think it was raised https://storyboard.openstack.org/#!/story/2005040 -- is there a workaround for this? we still want to avoid duplicating files section at all child jobs. | 08:09 |
| *** pcaruana has joined #zuul | 08:11 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul-jobs master: install-kubernetes: fix minikube config permission https://review.openstack.org/637682 | 08:19 |
| *** bhavikdbavishi has quit IRC | 08:23 | |
| *** saneax has quit IRC | 08:23 | |
| *** saneax has joined #zuul | 08:23 | |
| *** gtema has joined #zuul | 08:23 | |
| *** sdake has joined #zuul | 08:27 | |
| *** themroc has joined #zuul | 08:30 | |
| *** sdake has quit IRC | 08:37 | |
| *** electrofelix has joined #zuul | 08:38 | |
| *** sdake has joined #zuul | 08:45 | |
| *** quique|rover|r-- is now known as quiquell|rover | 08:46 | |
| *** jamesmcarthur has joined #zuul | 08:50 | |
| *** jamesmcarthur has quit IRC | 08:55 | |
| *** hashar has joined #zuul | 08:55 | |
| *** bhavikdbavishi has joined #zuul | 08:57 | |
| *** jpena|off is now known as jpena | 08:59 | |
| *** panda|off is now known as panda | 09:11 | |
| *** hashar has quit IRC | 09:20 | |
| *** hashar has joined #zuul | 09:20 | |
| *** sdake has quit IRC | 09:25 | |
| *** sdake has joined #zuul | 09:28 | |
| *** hashar has quit IRC | 09:47 | |
| *** hashar has joined #zuul | 09:48 | |
| *** sdake has quit IRC | 09:49 | |
| *** jamesmcarthur has joined #zuul | 09:51 | |
| *** jamesmcarthur has quit IRC | 09:55 | |
| openstackgerrit | Matthieu Huin proposed openstack-infra/zuul master: CLI: fail if trying to enqueue/dequeue a change for the wrong project https://review.openstack.org/636662 | 09:57 |
| openstackgerrit | Simon Westphahl proposed openstack-infra/zuul master: Show animated progress bar in preparation phase https://review.openstack.org/637810 | 10:11 |
| *** hashar has quit IRC | 10:15 | |
| openstackgerrit | Simon Westphahl proposed openstack-infra/zuul master: Log to job output when running Ansible setup https://review.openstack.org/637813 | 10:24 |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/zuul master: web: add jobs list filter https://review.openstack.org/633652 | 10:25 |
| *** jamesmcarthur has joined #zuul | 10:27 | |
| *** jamesmcarthur has quit IRC | 10:32 | |
| openstackgerrit | Matthieu Huin proposed openstack-infra/zuul master: CLI: fail if trying to enqueue/dequeue a change for the wrong project https://review.openstack.org/636662 | 10:42 |
| iurygregory | Morning everyone o/, does anyone has idea how to set a configuration file to be used by tox when running zuulv3? We have functional tests in python-ironicclient https://github.com/openstack/python-ironicclient/blob/master/tools/run_functional.sh but i didnt figure out how could i made it run using zuulv3, any tips? | 10:45 |
| iurygregory | My zuulv3 patch is https://review.openstack.org/#/c/633010/ | 10:45 |
| *** hashar has joined #zuul | 10:50 | |
| openstackgerrit | Tristan Cacqueray proposed openstack-infra/nodepool master: config: add statsd-server config parameter https://review.openstack.org/535560 | 10:53 |
| openstackgerrit | Matthieu Huin proposed openstack-infra/zuul master: CLI: fail if trying to enqueue/dequeue a change for the wrong project https://review.openstack.org/636662 | 11:01 |
| mordred | tobiash: https://review.openstack.org/#/c/636728 - is that ok for github enterprise? | 11:17 |
| *** jamesmcarthur has joined #zuul | 11:29 | |
| *** jamesmcarthur has quit IRC | 11:33 | |
| *** bhavikdbavishi has quit IRC | 11:36 | |
| *** bhavikdbavishi has joined #zuul | 11:37 | |
| tobiash | mordred: looking | 11:49 |
| tobiash | mordred: looks like this was introduced in ghe 2.13 | 11:53 |
| tobiash | mordred: and ghe 2.12 is unsupported since Decenber 12, 2018 | 11:53 |
| badboy | how do I delete ~/src/project dir after finishing the job? | 11:53 |
| tobiash | so that would be ok from my point of fuew | 11:54 |
| tobiash | SpamapS: I hope you're not on 2.12 and earlier? | 11:54 |
| tobiash | mordred: commented on https://review.openstack.org/636728 | 11:56 |
| tobiash | SpamapS: you definitely want to review ^ | 11:57 |
| mordred | tobiash: cool | 11:57 |
| mordred | tobiash: perhaps we should have a policy to only support the same versions of ghe that github supports? | 11:58 |
| tobiash | and I've verified that the labels are there on our 2.14 | 11:58 |
| mordred | (although I agree, let's make sure SpamapS is not on old ghe) | 11:59 |
| mordred | tobiash: \o/ | 11:59 |
| tobiash | probably | 11:59 |
| tobiash | and if we want to support new gh features we should either wait for all supported versions to support or use that feature optional | 11:59 |
| tobiash | (e.g. if we want to use graphql optimizations) | 12:00 |
| mordred | ++ | 12:00 |
| tobiash | e.g I think we might be able to optimize some things with graphql but that's supported for apps only since 2.14 (while 2.13 is still supported) | 12:01 |
| mordred | yeah - so in order to continue supporting 2.13, we'd have to have a "detect app graphql support, if so, use it, if not, use the unoptimized version" type of logic | 12:02 |
| tobiash | yea | 12:02 |
| tobiash | or we wait with graphql until 2.13 is unsupported :) | 12:03 |
| mordred | yeah | 12:05 |
| mordred | I tried learning some graphql the other day ... and I think I still have a lot to learn | 12:05 |
| tobiash | \o/ for merging the multi ansible spec | 12:05 |
| tobiash | btw, I have a working proof of concept of multi-ansible: https://review.openstack.org/631931 | 12:05 |
| tobiash | it's still wip as there is something missing like more docs, release notes, more tests and some further minor things | 12:06 |
| tobiash | but at least ansible 2.6 is already fully working with zuul-stream functional and also tox remote tests | 12:07 |
| mordred | \o/ | 12:07 |
| tobiash | e.g. http://logs.openstack.org/31/631931/8/check/zuul-tox-remote/da9d854/testr_results.html.gz | 12:07 |
| *** bhavikdbavishi has quit IRC | 12:08 | |
| mordred | tobiash: I like how the multi-ansible support works for the container images at container build time | 12:10 |
| tobiash | :) | 12:10 |
| openstackgerrit | Brendan proposed openstack-infra/zuul-jobs master: Use zuul_workspace_root variable for Git workspace prep https://review.openstack.org/636870 | 12:10 |
| *** jpena is now known as jpena|lunch | 12:30 | |
| *** rfolco has joined #zuul | 12:33 | |
| *** bjackman has quit IRC | 12:48 | |
| *** bhavikdbavishi has joined #zuul | 12:57 | |
| *** jamesmcarthur has joined #zuul | 13:06 | |
| *** jamesmcarthur has quit IRC | 13:11 | |
| *** jamesmcarthur has joined #zuul | 13:12 | |
| *** bhavikdbavishi has quit IRC | 13:14 | |
| *** gtema has quit IRC | 13:15 | |
| *** hashar has quit IRC | 13:18 | |
| *** sdake has joined #zuul | 13:25 | |
| *** jamesmcarthur has quit IRC | 13:32 | |
| *** rlandy has joined #zuul | 13:34 | |
| *** jpena|lunch is now known as jpena | 13:35 | |
| *** chkumar|ruck is now known as chandankumar | 13:45 | |
| *** jamesmcarthur has joined #zuul | 13:49 | |
| *** jamesmcarthur has quit IRC | 13:56 | |
| *** gtema has joined #zuul | 13:59 | |
| *** sdake has quit IRC | 14:05 | |
| *** rfolco has quit IRC | 14:07 | |
| *** rfolco has joined #zuul | 14:08 | |
| *** bjackman has joined #zuul | 14:12 | |
| *** sdake has joined #zuul | 14:13 | |
| *** cognifloyd has quit IRC | 14:18 | |
| *** sdake has quit IRC | 14:19 | |
| *** hashar has joined #zuul | 14:23 | |
| openstackgerrit | Matthieu Huin proposed openstack-infra/zuul master: CLI: fail if trying to enqueue/dequeue a change for the wrong project https://review.openstack.org/636662 | 14:32 |
| *** goern has quit IRC | 14:33 | |
| *** [GNU] has joined #zuul | 14:34 | |
| *** sdake has joined #zuul | 14:38 | |
| fungi | zbr: yes, adding a "files" list tells zuul you only want the job to be run if the triggering change alters at least one file in the list. since a periodic trigger involves no file changes, it won't trigger a job which limits its runs based on a files list | 14:40 |
| fungi | zbr: the argument has been made that this behavior, while consistent, is counter-intuitive and could be improved | 14:41 |
| zbr | fungi: this means that if we want to reuse a job definition in some periodical and in some file triggered jobs, we can do it only if we create two layered base jobs, one with files and one without files. A bit inconvenient I would say, but I can understand why it would be confused. Alternative would be to add an option to make it ignore patterns when triggered on schedule. | 14:42 |
| *** saneax has quit IRC | 14:42 | |
| fungi | i think quiquell|rover mentioned creating a story about ignoring files lists if the trigger isn't relevant for files (timer, ref-updated, et cetera) | 14:43 |
| jkt | I'm really struggling with this not-yet-merged "runc" nodepool launcher. Is there any black magic in provider's config tracking, especially its pools? | 14:47 |
| jkt | that code is trying to store something for each running container in a "pool", but that "pool" is actually getting reinitialized periodically in the background from a combination of regular nodepool provider config file and on-disk JSON | 14:48 |
| jkt | were there any changes in this in the past two years? Perhaps tristanC wrote that code in that way before these auto-reinits got introduced, or something? | 14:49 |
| quiquell|rover | fungi: yep is samething, I am puting a review in place to possible fix, ignoring files matchine at timer event type | 14:52 |
| quiquell|rover | fungi: if this is ok | 14:52 |
| fungi | since it's a behavior change, it likely warrants a discussion on the zuul-discuss ml | 14:53 |
| *** sdake has quit IRC | 14:53 | |
| Shrews | jkt: i have no idea what a "auto-reinit" is wrt nodepool. that seems unique to the runc driver perhaps? i haven't looked at that driver yet | 14:53 |
| jkt | Shrews: I meant that periodic background thread activity at https://git.openstack.org/cgit/openstack-infra/nodepool/tree/nodepool/launcher.py#n1070 | 14:56 |
| jkt | Shrews: that thing calls into nodepool/driver/runc/config.py, apparently re-reading config from /etc/nodepool/nodepool.yaml | 14:57 |
| pabelanger | Shrews: wb! do you mind adding https://review.openstack.org/636393/ to your review queue, fixes an issue we are seeing downstream with nodepool and large retries numbers | 14:57 |
| jkt | and then there's RuncProvider and RuncLauncher, and these two try to store the info about running containers in each "pool" (pool being each hypervisor) | 14:58 |
| jkt | except that no matter when I try to debug-log len(pool.containers), it's always 0 even though there are some containers launched by nodepool on that hypervisor | 14:58 |
| *** sdake has joined #zuul | 14:59 | |
| quiquell|rover | fungi: I send an email to zuul-discussoin (I think) | 15:00 |
| Shrews | jkt: reconfiguring of pools should only happen if there were actual changes to the config. if it's happening all the time, that's a runc driver bug | 15:00 |
| Shrews | jkt: that's been there since the beginning | 15:01 |
| Shrews | pabelanger: will try. i'm still feeling like poo, but also bored | 15:01 |
| pabelanger | Shrews: Oh, no rush then. Get healthy! | 15:02 |
| jkt | Shrews: I am not necessarily saying that it's destroying some handlers, just that it's apparently sticking data which are supposed to be strictly runtime into a data structure for which there's a background thread to update based on on-disk config | 15:02 |
| jkt | that sounds fishy | 15:02 |
| quiquell|rover | fungi: nop I don't find the email :-( | 15:03 |
| Shrews | jkt: that's vague enough that i have no idea what you are referring to. :) you should probably discuss with tristanC | 15:04 |
| fungi | quiquell|rover: could it be stuck in the moderation queue? | 15:05 |
| Shrews | pabelanger: seems simple enough | 15:05 |
| quiquell|rover | fungi: have resend it | 15:05 |
| quiquell|rover | fungi: now s there | 15:06 |
| dmsimard | tobiash: that multi-ansible patch poc is awesome but reminds me how much we need a better way to properly secure plugins/modules/etc | 15:06 |
| jkt | Shrews: is this OK? https://review.openstack.org/#/c/535556/22/nodepool/driver/runc/config.py@52 | 15:06 |
| tristanC | jkt: it's entirely possible that the quota code for the runc provider doesn't work. it seems like using config.pool.containers isn't correct since it's a foreign data struct | 15:06 |
| jkt | given that the rest of the code is trying to update this pool.containers as it launches and terminates containers? | 15:06 |
| Shrews | jkt: i do not know. not familiar with the driver | 15:07 |
| tristanC | jkt: that containers set should be moved to the provider object indeed | 15:07 |
| jkt | tristanC: I've seen other providers actually iterating the list of currently running nodes from zookeeper, is that the pattern to follow now? | 15:07 |
| tristanC | jkt: that would be another way to check for current quota indeed | 15:08 |
| jkt | I'm also confused a bit by the difference between hasProviderQuota and hasRemainingQuota. When does it happen that a pool to use gets chosen? | 15:09 |
| jkt | because I see that the openstack driver can, perhaps temporarily, raise an exception to indicate that it cannot launch an already assigned VM right now because the openstack tenant is over quota | 15:10 |
| jkt | and it *seems* to me that in runc, this allocation happens right when a node is intially requested, and therefore the request will be left waiting once the quotas actually start working, even if other pools become available | 15:11 |
| tristanC | jkt: i don't remember what is the difference betwen hasProviderQuota and hasRemainingQuota. Perhaps tobiash know? | 15:12 |
| jkt | the docs say that hasProviderQuota should somehow disregard the currently allocated nodes | 15:14 |
| jkt | perhaps meaning "can this pool ever accommodate this request?" | 15:14 |
| tobiash | jkt, tristanC: the different is that hasProviderQuota tells you if this provider can ever satisfy this request (e.g. in case of a multinode request that is larger than the provider quota) and hasRemainingQuota tells nodepool if there is enough quota at the moment | 15:15 |
| tristanC | Shrews: tobiash: btw, https://review.openstack.org/637682 should fix nodepool-functional-k8s failure | 15:20 |
| tristanC | pabelanger: could you please review on https://review.openstack.org/632620, we need this to use add-build-sshkey with static node | 15:21 |
| openstackgerrit | Merged openstack-infra/nodepool master: Properly handle TaskManagerStopped exception https://review.openstack.org/636393 | 15:26 |
| *** sdake has quit IRC | 15:28 | |
| tobiash | clarkb: +2 with question on https://review.openstack.org/637615 | 15:33 |
| tobiash | clarkb: do you want to rebase that stack to fix the merge conflict? | 15:33 |
| clarkb | tobiash: that chould cache merged shas too aiui | 15:35 |
| *** sdake has joined #zuul | 15:35 | |
| clarkb | and ya I need to rebase once properly awake | 15:35 |
| tobiash | clarkb: the merged sha's were on a different key in the json | 15:35 |
| tobiash | 'merge_commit_sha' | 15:36 |
| clarkb | hrm right was the sha field not the same as merged sha in that case? I should make that update. I also realized we should flip the order of addition to the cache in getPRBySha so tjat older PRs are evicted first | 15:37 |
| pabelanger | tristanC: why doesn't remove-build-sshkey role work? Isn't that what it does, cleans up the authorized_keys file? | 15:37 |
| clarkb | asis weput nwere shas in first I think so they are least recently used | 15:37 |
| tobiash | depends on the order in which they come from github | 15:38 |
| tobiash | we should check that first | 15:38 |
| tobiash | ok, according to the docs the default sort order is 'created' | 15:39 |
| tobiash | we might want 'updated' instead if that can be specified in github3.py | 15:39 |
| clarkb | I think we change it to descending, in the search query | 15:39 |
| tobiash | descending is default | 15:41 |
| tobiash | but ++ to make that explicit | 15:41 |
| tobiash | we can also request 'updated' as sort order here: https://git.zuul-ci.org/cgit/zuul/tree/zuul/driver/github/githubconnection.py#n1204 | 15:42 |
| tobiash | and make desc explicit | 15:43 |
| corvus | pabelanger: if i understand tristanC's change correctly, it's to handle the case where the post playbook doesn't run | 15:43 |
| pabelanger | tobiash: tristanC: left questions on 632620 | 15:43 |
| pabelanger | corvus: ah, maybe we if grow the cleanup playbook, this is a good use case | 15:44 |
| pabelanger | however, a base post-run job that doesn't run, likely means something else is broken | 15:44 |
| corvus | pabelanger: yes, though there will always be a very small chance even that won't run, if an executor crashes, for example. | 15:45 |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: install-kubernetes: fix minikube config permission https://review.openstack.org/637682 | 15:45 |
| clarkb | tobiash: for the merged sha should it be if not open and merge_commit_sha in pr: use merge_commit_sha? | 15:46 |
| pabelanger | k, then suggestion with uuid might not work, if we are trying to cover post-run never running | 15:46 |
| clarkb | or do we always want to cache both if present to be able to lookup either? | 15:47 |
| corvus | clarkb: there's no harm to being able to look up either | 15:47 |
| corvus | clarkb: and we need both | 15:47 |
| corvus | clarkb: the ansible post-merge shippable run used the merge sha | 15:47 |
| tobiash | clarkb: I'd just add it to the cache if that key exists | 15:47 |
| clarkb | ya can add both and bump size of the cache to accomodate | 15:48 |
| openstackgerrit | Quique Llorente proposed openstack-infra/zuul master: Ignore files at timer trigger https://review.openstack.org/637916 | 15:51 |
| *** sdake has quit IRC | 15:52 | |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Rename project to project_name in getPullBySha https://review.openstack.org/637218 | 15:55 |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Test GithubShaCache https://review.openstack.org/637228 | 15:55 |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Switch to LRU based sha to PR cache https://review.openstack.org/637615 | 15:55 |
| clarkb | that is just the rebase to make review easier | 15:55 |
| clarkb | now to make those changes suggested above | 15:55 |
| quiquell|rover | fungi: Have put a review with a unit test and fix for the periodic + files just to illustrate | 15:56 |
| tobiash | k | 15:56 |
| *** sdake has joined #zuul | 15:59 | |
| *** ianychoi has quit IRC | 16:00 | |
| *** jamesmcarthur has joined #zuul | 16:01 | |
| *** sdake has quit IRC | 16:03 | |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Switch to LRU based sha to PR cache https://review.openstack.org/637615 | 16:04 |
| clarkb | tobiash: ^ that has both suggestions | 16:04 |
| *** ParsectiX has joined #zuul | 16:09 | |
| *** ianychoi has joined #zuul | 16:12 | |
| jkt | tristanC: I think I've fixed it. There's indeed also a TOCTOU race, in addition to what we said earlier; there's plenty of opportunity for race between node allocation and time the the container actually begins starting | 16:13 |
| openstackgerrit | Jan Kundrát proposed openstack-infra/nodepool master: Implement a Runc driver https://review.openstack.org/535556 | 16:16 |
| *** bjackman has quit IRC | 16:18 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Fix typo in build api endpoint https://review.openstack.org/636227 | 16:28 |
| corvus | i'm going to self-approve that ^ it's a rebase of a one-line patch to fix a merge conflict | 16:29 |
| *** themroc has quit IRC | 16:29 | |
| *** bjackman has joined #zuul | 16:32 | |
| *** cognifloyd has joined #zuul | 16:34 | |
| corvus | mordred, tobiash, clarkb: i have a tricky problem. i changed the zuul_return data type for zuul.artifacts from a list to a dictionary keyed by the image name, so that the dictionary merge that zuul_return performs would automatically handle returning multiple artifacts from different playbooks (without the user having to read in the old values, append to the list, and write it out again). so now the | 16:37 |
| corvus | ansible task looks like: http://paste.openstack.org/show/745381/ | 16:37 |
| corvus | but apparently ansible doesn't jinja template dict keys like that | 16:38 |
| *** quiquell|rover is now known as quiquell|off | 16:38 | |
| corvus | should i purse trying to find a way to make that work, or should we switch back to expecting zuul.artifacts to be a list, and modify zuul_return to treat the artifacts list specially and automatically append? | 16:38 |
| corvus | or some other option? | 16:38 |
| mordred | corvus: oy | 16:39 |
| mordred | corvus: the list form *was* nice - and zuul_return is a thing specifically to return data to zuul, so it doesn't seem like a layer violation for it to grok zuul.artifacts | 16:41 |
| mordred | corvus: so I think I'm in favor of that | 16:41 |
| corvus | yeah, everything about using it felt better, except for the fact that it would be an exception to the general rule of how data is merged in zuul. but maybe it's worth it. | 16:42 |
| clarkb | one terrible way around it would be to use another variable and append two vars together in a jinja block? | 16:42 |
| corvus | clarkb: you mean if we switched to list, or is that a way to keep using dict? | 16:43 |
| clarkb | that is a way to keep using a dict | 16:43 |
| fungi | corvus: just a heads up, kendall nelson is sending you a survey link to use for indicating whether or not we want zuul onboarding and/or project update session slots during the summit in denver | 16:43 |
| corvus | hey everyone ^ do we? :) | 16:43 |
| clarkb | you set fact imagestr = 'image_' then {{ imagestr + image.repository }}:{{image_tag}} | 16:43 |
| clarkb | that should work because {{}} is an explicit jinja block | 16:44 |
| corvus | clarkb: i'm not following | 16:44 |
| corvus | clarkb: maybe you could mock it up in paste/etherpad | 16:44 |
| mordred | oh - I grok | 16:44 |
| clarkb | corvus: http://paste.openstack.org/show/745381/ I'm looking at that and you are saying the image_ prepend doesn't work beacuse that isn't treated as jinja? | 16:44 |
| clarkb | I'm saying put that all in {{}} which is explicitly jinja | 16:45 |
| corvus | clarkb: no i'm saying that yaml dict keys don't run jinja | 16:45 |
| openstackgerrit | Jan Kundrát proposed openstack-infra/nodepool master: Implement a Runc driver https://review.openstack.org/535556 | 16:45 |
| fungi | corvus: i thought the onboarding session in berlin was pretty awesome, even if i did miss a chunk of it running around looking for brochures. i didn't see the project update session though | 16:45 |
| clarkb | corvus: oh at all got it | 16:45 |
| corvus | yeah, what zuul gets back is literally "image_{{ image.repository }}:{{ image_tag }}" in the dict key | 16:45 |
| corvus | so it seems ansible runs jinja on values but not keys | 16:46 |
| corvus | there's probably some way to build the artifacts dictionary as a string via a json filter or something | 16:46 |
| corvus | but that almost certainly does defeat the point of having made this change so it's "easier to use" :) | 16:47 |
| corvus | if there were a simple solution like "prefix the dict key with !" or something, maybe... | 16:47 |
| corvus | but if the vanguard user of this feature can't use it in a comprehensible way, we may have gone off path :) | 16:48 |
| pabelanger | https://github.com/ansible/ansible/issues/17324 might be the issue you are having, seems to include some suggestions, if you are wanting to try | 16:48 |
| corvus | pabelanger: nice google foo :) | 16:48 |
| clarkb | https://github.com/ansible/ansible/issues/17324#issuecomment-449108361 is the workaround? its not a great one imo | 16:53 |
| clarkb | (because its magic you need to know is necessary) | 16:53 |
| clarkb | but it is also reasonably straightfoward once you have that knowledge | 16:54 |
| *** pcaruana has quit IRC | 16:57 | |
| tobiash | corvus: maybe you could use the from_yaml filter to set a more complex variable like this. https://docs.ansible.com/ansible/latest/user_guide/playbooks_filters.html#filters-for-formatting-data | 17:05 |
| tobiash | to me that reads like it might solve your problem | 17:05 |
| tobiash | corvus: I'm thinking about something like this: http://paste.openstack.org/show/745382/ | 17:07 |
| SpamapS | tobiash: no, we aren't actually even live on GHE yet | 17:11 |
| SpamapS | just deployed it | 17:11 |
| SpamapS | And I'm pretty sure GoDaddy was on 2.13 when I left | 17:12 |
| tobiash | SpamapS: ok, so this is ok then | 17:12 |
| *** chandankumar is now known as kmrchdn | 17:12 | |
| SpamapS | corvus: I've had this problem in the past too. tobiash's method is legit, but I usually just drop to python and write a little module. | 17:25 |
| mordred | SpamapS, corvus: which I think sounds like "handle the list merge in zuul_return" | 17:30 |
| openstackgerrit | Merged openstack-infra/zuul master: Fix typo in build api endpoint https://review.openstack.org/636227 | 17:30 |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Switch to LRU based sha to PR cache https://review.openstack.org/637615 | 17:32 |
| SpamapS | mordred: yep | 17:34 |
| tobiash | clarkb: +2 with comment on https://review.openstack.org/637218 | 17:42 |
| openstackgerrit | Merged openstack-infra/zuul master: Don't request PR issue data https://review.openstack.org/636728 | 17:42 |
| clarkb | tobiash: why don't I do a followup to implement that | 17:43 |
| tobiash | clarkb: it's still +2, just wanted to make us think about it | 17:44 |
| clarkb | yup | 17:45 |
| *** ParsectiX has quit IRC | 17:49 | |
| tobiash | SpamapS: do you want to re-review the dockerized test setup (636424)? | 17:49 |
| *** hashar has quit IRC | 17:50 | |
| corvus | yeah, i'm 80% through the patch to switch back to list :) | 17:52 |
| *** bjackman has quit IRC | 17:54 | |
| openstackgerrit | Clark Boylan proposed openstack-infra/zuul master: Clarify project vs repository in getPullBySha https://review.openstack.org/637956 | 17:54 |
| clarkb | tobiash: ^ hows that? | 17:55 |
| tobiash | clarkb: gread (and sorry if that's too much bikeshedding) | 17:55 |
| SpamapS | tobiash: +3'd, :-D | 17:56 |
| tobiash | s/gread/great/ | 17:56 |
| tobiash | SpamapS: \o/ | 17:56 |
| clarkb | tobiash: no I think part of the problem we had with this code was that it was slightly confusing due to name choices and on top of that was trying to accomodate weird api behavior so simplifying both as much as possible is a win | 17:57 |
| tobiash | :) | 17:57 |
| SpamapS | tobiash: it will please me very much to apt remove zookeeperd from my laptop. | 17:58 |
| tobiash | SpamapS: I never installed it and had this off-tree for too long | 17:58 |
| mordred | corvus: https://review.openstack.org/#/c/637615 has 2x+2 (the lru patch) - do you want to look at it agai before it goes in? | 18:00 |
| corvus | mordred: yeah, i'll give it a quick once-over and +3 | 18:00 |
| mordred | kk. the rest of teh stack is +3 | 18:01 |
| corvus | clarkb, mordred, tobiash: i bet 2048 per project still would have been fine even with the merge sha -- remember, we're just aiming for active shas, even with ansible, there's probably at most a few hundred of those. | 18:02 |
| clarkb | corvus: ya though at 1700 active PRs we'd invalidate when we updated the lru | 18:03 |
| clarkb | one way we could address that is to map in the pr updated timestamp to the cache rather than relying on when we last saw it | 18:03 |
| clarkb | (I believe this is possible with cachetools) | 18:03 |
| tobiash | with the now correct sorting we'll have the newest pr's in the cache on any cache miss so I'm fine with either | 18:05 |
| corvus | clarkb: good point, we don't want to do that... in fact, "more PR sha's than cache size" is currently a potentially fatal situation, yeah? we may never succeed in the lookup if we page out by the end of our search... | 18:05 |
| corvus | tobiash: oh right | 18:05 |
| corvus | so that should address that case | 18:05 |
| tobiash | yes | 18:05 |
| clarkb | ya the sort order is the current mitigation against that | 18:06 |
| corvus | so yeah, we can probably dial the size back down. | 18:06 |
| clarkb | so I think it is correct and we can dial the size back if we like | 18:06 |
| corvus | i'm going to +3 and we can do that later | 18:06 |
| clarkb | ok | 18:06 |
| *** jamesmcarthur has quit IRC | 18:17 | |
| *** jamesmcarthur has joined #zuul | 18:18 | |
| openstackgerrit | Merged openstack-infra/zuul master: Add dockerized test setup https://review.openstack.org/636424 | 18:21 |
| *** jpena is now known as jpena|off | 18:22 | |
| *** kmrchdn is now known as chandankumar | 18:26 | |
| *** ParsectiX has joined #zuul | 18:31 | |
| mnaser | so i've been working on a small poc of an api (authenticated via openstack credentials) that allows you to create 'connections' to github "targets" (i.e. users or orgs) | 18:36 |
| mnaser | when you try to create a connection, it checks if its installed or not, and if it is *not*, it stops, if it is, it adds it with a "verification" link which the user has to open to authenticate their account via github | 18:36 |
| mnaser | that way, i dont have arbitrary people who have access to the app | 18:37 |
| mnaser | that's as far as i got so far. the last moving piece is an executable that runs against that db and retrieves all of those and groups them based on openstack tenants | 18:37 |
| mnaser | is anyone doing something similar in terms of self-serve management of projects? or maybe if there's any zuul targets about auth soon so i dont duplicate stuff | 18:38 |
| mordred | mnaser: mhu is working on support for JWT auth: https://review.openstack.org/#/c/576907/ | 18:39 |
| mnaser | mordred: ou thats interesting but seems to be pretty hard wired for jwt based auth | 18:41 |
| clarkb | mnaser: is the concern that anyone in github could start sending events to your zuul? | 18:43 |
| *** gtema has quit IRC | 18:43 | |
| clarkb | (I think anyone can already add the app as is? I know i didn't do anything in kata for them to start sending us events) | 18:43 |
| mnaser | clarkb: well anyone can add the app but i dont want someone to be able to start adding an arbitrary project to their account without knowing they control it | 18:44 |
| mnaser | i.e. if the zuul app is added across 400 orgs, i do a add project to kata-containers .. how can i know that openstack user X 'controls' github org 'kata-containers' | 18:44 |
| *** cognifloyd has quit IRC | 18:44 | |
| mnaser | hosted CI solutions solve this by doing login via github and call it a day | 18:45 |
| clarkb | I see you are verifying the addition on the zuul side not the github side | 18:45 |
| clarkb | (since github side is already arbitrary) | 18:45 |
| mnaser | yeah | 18:45 |
| mnaser | and not letting you add to zuul if the app isnt installed either | 18:45 |
| mnaser | so the verificaiton gets the user token which then goes ahead and checks that the user is allowed to install that app (or did indeed install it) | 18:46 |
| mnaser | that way if someone installs zuul on kata-containers, i cant just 'snipe' it and start doing ci on it or something like that | 18:46 |
| *** gtema has joined #zuul | 18:47 | |
| openstackgerrit | Merged openstack-infra/zuul master: Rename project to project_name in getPullBySha https://review.openstack.org/637218 | 18:51 |
| openstackgerrit | Merged openstack-infra/zuul master: Test GithubShaCache https://review.openstack.org/637228 | 18:51 |
| openstackgerrit | Merged openstack-infra/zuul master: Switch to LRU based sha to PR cache https://review.openstack.org/637615 | 18:51 |
| tobiash | jhesketh: do you want to have a third review on https://review.openstack.org/636146 ? (I'm asking because you gave the second +2) | 18:52 |
| *** jamesmcarthur has quit IRC | 18:53 | |
| SpamapS | mnaser: isn't the tenant configuration enough for this? Would you add a project to your tenant if you didn't know who was going to send events to you? | 18:54 |
| openstackgerrit | Merged openstack-infra/zuul master: Clarify project vs repository in getPullBySha https://review.openstack.org/637956 | 18:55 |
| *** gtema has quit IRC | 18:56 | |
| *** electrofelix has quit IRC | 19:03 | |
| openstackgerrit | Tobias Henkel proposed openstack-infra/zuul master: Remove reference to localhost in zuul_return docs https://review.openstack.org/637974 | 19:09 |
| *** panda is now known as panda|off | 19:17 | |
| mordred | SpamapS: what if you don't know the people, and someone comes and signs up for your service and says "I want to have project X be in my tenant" - bur project X is also already in another tenant - how do you know that the requestor is making a valid request? | 19:22 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Switch artifact return back to list https://review.openstack.org/637979 | 19:23 |
| *** jamesmcarthur has joined #zuul | 19:24 | |
| corvus | mordred, SpamapS, mnaser: we should make a distinction between adding a project to a tenant for read-only (ie, required-projects, depends-on) purposes, vs read-write (ie project stanzas). at least until we figure out how to do implied foreign projects in required-projects. the underlying difference is whether configuration objects (especially "projects") are loaded from the repo. | 19:25 |
| SpamapS | mordred: you can see who is in the org that owns the repo or is in the list of write-access-enabled users fo rthe repo. | 19:25 |
| corvus | that's just a clarification. the underlying problem of trust still stands. just that i think the more precise question is "is it permitted to load configuration objects from this project in this tenant?" rather than "is it permitted to add this project to this tenant?" | 19:26 |
| mordred | corvus: ++ | 19:26 |
| mordred | SpamapS: yah - but that would involve a human going and looking at an account... and would assume you can map a requestor to a github account in the first place | 19:27 |
| SpamapS | Also I believe you have to have admin on a repo to add an app to it. | 19:28 |
| mordred | SpamapS: so I *think* mnaser is trying to have a computer be able to map a requstor to a github account and then to check to see what permissions that requestor's github account has on the repository in general | 19:28 |
| SpamapS | Yeah I just wonder.. if you can't actually add Zuul to a repo without being an admin, what's the danger? | 19:29 |
| mordred | what if someone has already added the zuul app to a repo for the purposes of a different tenant in this shared zuul | 19:29 |
| mordred | the adding of the app to the repo doesn't necesarily contain any information about what zuul tenant(s) that app might want to be used by - nor which of them, to corvus point, are valid tenants to load zuul config objects from | 19:30 |
| *** sdake has joined #zuul | 19:33 | |
| SpamapS | Oh, you can use one app, for multiple tenants? | 19:34 |
| SpamapS | That seems like a bad idea. | 19:34 |
| SpamapS | Perhaps we should add tenant name to the app config. | 19:34 |
| mordred | there is only one app instance | 19:34 |
| mordred | SpamapS: it's like travis - there is only one travis app - but when you click to install the app into your repo, it takes you to the post-install url which then does a transaction to tie your gh account info to your travis account info | 19:36 |
| SpamapS | But I'm pretty sure Travis isn't multi-tenant. | 19:39 |
| SpamapS | since we have cross-repo concerns, we might have to think harder about it. | 19:39 |
| mordred | well - I'm 100% certain travis is multi-tenant | 19:40 |
| mordred | they don't run a new travis for each of the million users | 19:40 |
| mordred | but I agree - since we have cross-repo concerns, I think we likely need to think harder about it | 19:41 |
| SpamapS | It's multi-tenant at a different level then maybe? | 19:41 |
| mnaser | an app is meant to be used across several projects | 19:41 |
| SpamapS | The point is that there aren't two ways to do travis in one repo. | 19:41 |
| mordred | yeah | 19:41 |
| mordred | one cannot have one github repo be connected to 2 different travis users | 19:41 |
| mnaser | i.e. the same travis-ci app gets added to anyone using it | 19:41 |
| mnaser | travis does auth via github though | 19:41 |
| mnaser | so they can very easily know who added what | 19:42 |
| SpamapS | Yeah | 19:42 |
| mordred | SpamapS: so - yeah - that's definitely a thing we have that they don't that we need to ponder | 19:42 |
| mnaser | so for me the verification portion is to make sure that the person who installed the app is a github admin that is *allowed* to install the app | 19:43 |
| mnaser | also i totally used it as a hack project to try out golang which is fun and all | 19:44 |
| mnaser | but i think ill just rewrite it in python+flask | 19:44 |
| mnaser | ..boring i know | 19:44 |
| mordred | mnaser: I think to SpamapS's point - I believe one must be an admin to install an app, no? | 19:45 |
| mnaser | mordred: thats correct | 19:45 |
| SpamapS | Right so if the app is installed, it was at least authorized by an admin at that time. | 19:46 |
| mnaser | so the flow here: 1) github user mordred installs foobar-ci on `my-cool-org` in github ... 2) openstack user mordred hits $some_api saying that he'd like to add `my-cool-org` to zuul | 19:46 |
| mnaser | at this state, i dont know that github user mordred is the same person as openstack user mordred | 19:46 |
| mnaser | because said api i'm talking about authenticates using openstack credentials | 19:46 |
| mnaser | *IF* this api authenticated using github credentials, i can easily "approve" that because i know its mordred, but mordred in openstack could be mordred1234 as a user and i dont have the link between both | 19:47 |
| SpamapS | mnaser: ahh, the github app should be the one pinging $some_api, which it will do, there's a signup API endpoint that can be different than the hook API. | 19:47 |
| SpamapS | And it might even make sense for us to just add that to zuul-web. | 19:47 |
| mnaser | and this is more of an administrative reason.. if mordred adds the app to a project, but SpamapS adds the app in $some_api .. now SpamapS has control over the org in the zuul sense | 19:48 |
| mnaser | and they can disable/enable projects from being tested, etc | 19:48 |
| SpamapS | We actually did that with BonnyCI. jamielennox did the code.. it's somewhere under https://github.com/BonnyCI | 19:48 |
| corvus | SpamapS, mnaser: oh, so maybe the app registration api endpoint could then do some openstack stuff to get the mapping? | 19:49 |
| mnaser | right, github app can ping $some_api, but said api will say that github user "mordred" did this request, and .. i dont know who mordred is in my api | 19:49 |
| mnaser | yes, thats the whole reason behind the "verification" step, i send you off to a custom link which does oauth at github | 19:49 |
| mnaser | and i get a token back to verify that you have identified correctly | 19:49 |
| corvus | mnaser: right, but if that regestration link did *openstack* auth, then you'd have your mapping | 19:49 |
| mnaser | corvus: yep indeed, but the concern here is i wanted to make it api based so it limits my choices | 19:50 |
| mnaser | that way a user can easily make the request to add a project via cli, get a url to do a manual verification with and move on, it also helps not keeping long lived associations of users | 19:51 |
| mnaser | it's really a tricky problem | 19:51 |
| *** sdake has quit IRC | 19:52 | |
| *** sdake has joined #zuul | 19:55 | |
| *** sdake has quit IRC | 19:58 | |
| *** hashar has joined #zuul | 20:00 | |
| *** openstackgerrit has quit IRC | 20:09 | |
| *** gtema has joined #zuul | 20:19 | |
| *** gtema has quit IRC | 20:35 | |
| *** openstackgerrit has joined #zuul | 20:37 | |
| openstackgerrit | Jan Kundrát proposed openstack-infra/zuul master: More parallelism for git clones and checkouts https://review.openstack.org/637996 | 20:37 |
| jkt | ^^^ 1.5 minutes off of a 6min job :) | 20:43 |
| SpamapS | mmmm paralell | 20:50 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Combine zuul.artifacts lists in zuul_return https://review.openstack.org/638005 | 20:55 |
| corvus | tobiash, clarkb, mordred, fungi: ^ part 2 of that fix | 20:55 |
| *** hashar has quit IRC | 20:55 | |
| *** hashar has joined #zuul | 20:56 | |
| *** badboy has quit IRC | 20:57 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul-jobs master: Use list form of zuul artifact return https://review.openstack.org/638007 | 21:00 |
| fungi | tobiash: mordred: is there a reason we're holding approval of 637979? | 21:13 |
| fungi | or just raced each other reviewing? | 21:14 |
| * fungi isn't sure whether to +3 or only +2 that one | 21:14 | |
| tobiash | I think it was a race ;) | 21:14 |
| fungi | approved in that case | 21:19 |
| fungi | looking at the other two patches related to that now | 21:19 |
| *** takamatsu has joined #zuul | 21:21 | |
| fungi | approved all three | 21:33 |
| openstackgerrit | Merged openstack-infra/zuul master: Switch artifact return back to list https://review.openstack.org/637979 | 21:49 |
| openstackgerrit | Merged openstack-infra/zuul-jobs master: Use list form of zuul artifact return https://review.openstack.org/638007 | 21:52 |
| mordred | fungi: yes. just a race | 21:56 |
| openstackgerrit | Merged openstack-infra/zuul master: Combine zuul.artifacts lists in zuul_return https://review.openstack.org/638005 | 21:56 |
| *** sdake has joined #zuul | 21:58 | |
| *** hashar has quit IRC | 21:59 | |
| *** sdake has quit IRC | 22:09 | |
| *** rlandy is now known as rlandy|brb | 22:18 | |
| *** rlandy|brb is now known as rlandy | 22:27 | |
| *** sdake has joined #zuul | 22:52 | |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul master: Allow extra data in artifact schema validation https://review.openstack.org/638038 | 22:59 |
| openstackgerrit | Merged openstack-infra/zuul master: Allow extra data in artifact schema validation https://review.openstack.org/638038 | 23:33 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!