| *** avass has quit IRC | 00:26 | |
| *** hamalq has quit IRC | 01:53 | |
| *** evrardjp has quit IRC | 02:33 | |
| *** evrardjp has joined #zuul | 02:33 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Revert "Revert "Make repo state buildset global"" https://review.opendev.org/c/zuul/zuul/+/785535 | 03:18 |
|---|---|---|
| openstackgerrit | James E. Blair proposed zuul/zuul master: Fix missing repo state restore https://review.opendev.org/c/zuul/zuul/+/785310 | 03:18 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Keep jobgraphs frozen across reconfiguration https://review.opendev.org/c/zuul/zuul/+/785536 | 03:18 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Add a fast-forward test https://review.opendev.org/c/zuul/zuul/+/786521 | 03:18 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Correct repo_state format in isUpdateNeeded https://review.opendev.org/c/zuul/zuul/+/786522 | 03:18 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Restore repo state in checkoutBranch https://review.opendev.org/c/zuul/zuul/+/786523 | 03:18 |
| corvus | clarkb, tobiash: ^ erm, that opened a can of worms. i believe the isUpdateNeeded method is not currently doing anything and may not have for some time. it may never have. i left that as a series of patches to try to untangle this. start at 786512 and go down the stack from there. i *think* that by the end everything should work, but i haven't run the whole test suite yet; there could still be more work | 03:26 |
| corvus | to do. when we are done, we'll still need to squash. | 03:26 |
| *** rlandy|rover|bbl has quit IRC | 03:47 | |
| *** ykarel has joined #zuul | 04:15 | |
| *** jfoufas1 has joined #zuul | 04:24 | |
| *** vishalmanchanda has joined #zuul | 04:30 | |
| *** bhavikdbavishi has joined #zuul | 04:30 | |
| *** bhavikdbavishi has quit IRC | 04:32 | |
| *** saneax has joined #zuul | 05:42 | |
| lyr | Hi there. FYI there's a broken link in this doc, between 2nd & 3rd code bloc https://zuul-ci.org/docs/zuul/discussion/encryption.html#encryption | 06:35 |
| *** jcapitao has joined #zuul | 06:38 | |
| *** zbr has quit IRC | 06:56 | |
| *** zbr has joined #zuul | 06:58 | |
| *** avass has joined #zuul | 07:00 | |
| *** ykarel_ has joined #zuul | 07:00 | |
| *** ykarel has quit IRC | 07:02 | |
| *** holser has quit IRC | 07:13 | |
| *** ykarel_ has quit IRC | 07:15 | |
| *** ykarel has joined #zuul | 07:17 | |
| *** bhavikdbavishi has joined #zuul | 07:30 | |
| *** jpena|off is now known as jpena | 07:30 | |
| *** rpittau|afk is now known as rpittau | 07:32 | |
| *** harrymichal has joined #zuul | 07:34 | |
| *** bhavikdbavishi has quit IRC | 07:37 | |
| *** harrymichal has quit IRC | 07:39 | |
| *** tosky has joined #zuul | 07:39 | |
| *** harrymichal has joined #zuul | 07:39 | |
| *** harrymichal_ has joined #zuul | 07:41 | |
| *** harrymichal has quit IRC | 07:44 | |
| *** harrymichal_ is now known as harrymichal | 07:44 | |
| *** harrymichal has quit IRC | 08:22 | |
| *** harrymichal has joined #zuul | 08:22 | |
| *** travier has joined #zuul | 08:40 | |
| * harrymichal waves at travier | 08:40 | |
| *** ykarel is now known as ykarel|lunch | 08:44 | |
| *** ykarel|lunch has quit IRC | 08:48 | |
| *** bhavikdbavishi has joined #zuul | 08:53 | |
| *** bhavikdbavishi1 has joined #zuul | 08:55 | |
| *** bhavikdbavishi has quit IRC | 08:57 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 08:57 | |
| *** harrymichal has quit IRC | 08:59 | |
| *** harrymichal has joined #zuul | 08:59 | |
| *** harrymichal has quit IRC | 09:17 | |
| *** harrymichal has joined #zuul | 09:17 | |
| *** harrymichal has quit IRC | 09:19 | |
| *** harrymichal has joined #zuul | 09:19 | |
| *** ykarel has joined #zuul | 09:38 | |
| *** jcapitao has quit IRC | 09:45 | |
| *** jcapitao has joined #zuul | 09:46 | |
| *** holser has joined #zuul | 10:10 | |
| *** bhavikdbavishi has quit IRC | 10:25 | |
| *** jcapitao is now known as jcapitao_lunch | 10:26 | |
| *** cloudnull has quit IRC | 11:09 | |
| *** cloudnull7 has joined #zuul | 11:09 | |
| *** jpena is now known as jpena|lunch | 11:32 | |
| *** rlandy has joined #zuul | 11:50 | |
| *** rlandy is now known as rlandy|rover | 11:50 | |
| *** jcapitao_lunch is now known as jcapitao | 12:09 | |
| *** bhavikdbavishi has joined #zuul | 12:24 | |
| *** bhavikdbavishi1 has joined #zuul | 12:27 | |
| *** bhavikdbavishi has quit IRC | 12:28 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 12:28 | |
| *** jpena|lunch is now known as jpena | 12:30 | |
| *** bhavikdbavishi has quit IRC | 12:39 | |
| *** VinayJain212 has joined #zuul | 13:22 | |
| VinayJain212 | Hi, what is best way to access log_url of current build in POST phase...I have script which needs log_url as input after CI is finished. | 13:24 |
| avass | VinayJain212: I think there either is a change on the way to expose that or it was recently. I can look it up in a couple of minutes | 13:25 |
| VinayJain212 | avass - yes that would be great. So just to confirm its not possible in current version of zuul? | 13:27 |
| *** sean-k-mooney has joined #zuul | 13:30 | |
| travier | Hi there. I have some questions related to https://github.com/containers/toolbox/issues/714. We're looking into adding Fedora CoreOS as a host for testing the containers/toolbox project. I gathered that everything is running on OpenStack and apparently we can pass userdata to test instances: https://zuul-ci.org/docs/nodepool/openstack.html#attr-providers.[openstack].pools.labels.userdata. Could this userdata be customized per test job triggered on Zuul or does it | 13:30 |
| travier | has to be fixed? Do we have to use Ansible for test jobs scripting or can we use shell scripts or something else? Thanks! | 13:30 |
| sean-k-mooney | the vms are created by nodepool not zuul | 13:31 |
| sean-k-mooney | and you cant pass custimised data to nodepool form zuul as part of the job defintion | 13:32 |
| sean-k-mooney | travier: so you cant pass per job user data no | 13:33 |
| travier | ok, so we can set the userdata, but it will be the same for all jobs? | 13:33 |
| travier | ok | 13:33 |
| sean-k-mooney | it has to be the same for all uses of that lable | 13:33 |
| sean-k-mooney | really the way its intended to work is nodepool does what is nessalry to provde an env where ansible can connect | 13:33 |
| travier | we can work with that | 13:33 |
| sean-k-mooney | then you use ansibel for the rest of the customisation | 13:33 |
| sean-k-mooney | travier: you can also reboot the nodes provide by nodepool so you could have an ansible srcipt coonect and write a file to the node and reboot it if that helps | 13:34 |
| sean-k-mooney | i dont know if core os will pick that up and rerun ignition/cloud-init or not | 13:35 |
| travier | it won't we only run Ignition on first boot. And we don't have Python 3 | 13:36 |
| sean-k-mooney | in 2021? | 13:36 |
| travier | I was also pointed to raw ansible modules | 13:36 |
| travier | We don't have any Python ;) | 13:36 |
| sean-k-mooney | ah | 13:36 |
| sean-k-mooney | ya so you can use the raw ansible module to prep the env for ansible | 13:36 |
| sean-k-mooney | i think i did that in kolla ansible | 13:37 |
| sean-k-mooney | they remvoed it or moved it but like this | 13:39 |
| sean-k-mooney | https://github.com/openstack/kolla-ansible/blob/stable/pike/ansible/roles/baremetal/tasks/pre-install.yml#L3-L8 | 13:39 |
| sean-k-mooney | you have to disable gathering facts untill python is avaiable | 13:39 |
| sean-k-mooney | so i think you would need to do this in your base job | 13:39 |
| sean-k-mooney | clarkb: do you know off the top of your head if we can list a different file for zuul to read when we list a repo | 13:41 |
| travier | Hum, I'm sorry I'm asking weird questions, but do we really need to install Python? Can't we directly send the job commands as shell and gather output? | 13:42 |
| sean-k-mooney | usecase is in downstream ci it woudl be nice if i could tell zuul to ignroe the .zuul.yaml form upstream and read .zuul-downstream.yaml isntead | 13:42 |
| avass | VinayJain212: it looks like upload_results.url is available if used in the same playbook as the log upload: https://opendev.org/opendev/base-jobs/src/branch/master/playbooks/base/post-logs.yaml#L39 | 13:42 |
| sean-k-mooney | travier: ansible requires python to work | 13:42 |
| avass | but I'm pretty sure that is going to be deprecated for a different variable, or the same variable but set differently | 13:43 |
| sean-k-mooney | travier: they shell and command module require it | 13:43 |
| sean-k-mooney | travier:and simple json | 13:43 |
| sean-k-mooney | travier: so unless you are only going to use the raw modules then yes you need python | 13:43 |
| sean-k-mooney | travier: this is not a requiremnt of zuul its a requirement for using ansible | 13:44 |
| travier | yes, so the question is: can we use zuul without ansible? | 13:44 |
| sean-k-mooney | no | 13:44 |
| travier | ok | 13:45 |
| travier | what if I do everything in a raw module? | 13:45 |
| sean-k-mooney | if you do everyting in the raw module then you dont obviously | 13:46 |
| mordred | There's still base job operations, like getting the source code to the remote machine | 13:46 |
| sean-k-mooney | but that is very limiting | 13:46 |
| mordred | So you'd have to reimplement fundamental parts of the base jobs | 13:46 |
| sean-k-mooney | mordred: yep doing everything with raw would also have to mean rewriting that to use raw | 13:46 |
| sean-k-mooney | yep | 13:47 |
| mordred | Yup. | 13:47 |
| avass | VinayJain212: here: https://review.opendev.org/c/zuul/zuul-jobs/+/776677/6/roles/upload-logs-swift/tasks/main.yaml | 13:47 |
| sean-k-mooney | which kill the reusablity | 13:47 |
| sean-k-mooney | travier: what is the objection to having python? | 13:47 |
| mordred | I mean, it's a computer, you CAN do anything :) | 13:47 |
| travier | We have to do custom things anyway as we can not just install a pile of package on Fedore CoreOS, it's easier to just run everything inside of a container | 13:48 |
| corvus | VinayJain212, avass: don't rely on that variable; it will be removed; https://review.opendev.org/c/zuul/zuul-jobs/+/776677 should let you do what you want when it merges. | 13:48 |
| avass | corvus: yeah that's what I was thinking of | 13:48 |
| sean-k-mooney | but you can add some packages to core os | 13:49 |
| sean-k-mooney | travier: you can use a python virtual env too by the way | 13:49 |
| travier | Nothing against Python, it's just not installed by default on Fedora CoreOS | 13:49 |
| corvus | travier, mordred: wait, let's back up a bit about this "can i use zuul without ansible" thing... | 13:49 |
| *** bhagyashris is now known as bhagyashris|away | 13:49 | |
| sean-k-mooney | travier: right but you could use the user-data to install it | 13:50 |
| sean-k-mooney | i.e. have ignition prep the coreos instance so that you can use ansible | 13:50 |
| *** saneax has quit IRC | 13:50 | |
| VinayJain212 | avass, corvus - how do I access this variable in build config. I tried using zuul_log_url but it fails. | 13:51 |
| corvus | travier, mordred: oh, nm, mordred covered the base jobs bit :) so it's possible but you'd have to re-do a bnuch of work | 13:51 |
| VinayJain212 | is there any example you can point me where log_url is used in config file. | 13:51 |
| sean-k-mooney | travier: you could try binding mounting python form a contianer to the host and setting the python virtul enve dir for ansible to use that host mount | 13:51 |
| sean-k-mooney | travier: really until https://github.com/coreos/fedora-coreos-tracker/issues/592 is resovle i dont think zuul can supprot it as a target distro | 13:53 |
| avass | VinayJain212: it's not yet merged so itäs not available yet. The only current option is to use upload_result.url but like corvus said it's going to be removed and it's only possible to use it in the same playbook that you upload logs in and only after the logs have been uploaded | 13:53 |
| avass | VinayJain212: so waiting for https://review.opendev.org/c/zuul/zuul-jobs/+/776677 to merge is preferable right now | 13:54 |
| travier | sean-k-mooney: yeah, it's ugly but I can do the python setup on first boot. Another question: you say Zuul is fine if the node reboots. Is it fine if it reboots in the middle of a playbook? | 13:56 |
| travier | between two commands | 13:56 |
| sean-k-mooney | travier: you can use the playbook to reboot the node and then have the next command wait | 13:57 |
| sean-k-mooney | and try an reconnect | 13:57 |
| sean-k-mooney | then continue on | 13:57 |
| sean-k-mooney | so like if you wanted to do a kernel update you could | 13:57 |
| travier | ok, thanks | 13:58 |
| *** hashar has joined #zuul | 14:01 | |
| corvus | tristanC: fyi see convo with travier ^ | 14:03 |
| *** ykarel has quit IRC | 14:03 | |
| travier | What I'm planning to do based on this conversation: | 14:07 |
| travier | 1. Add Fedora CoreOS QCOW2 images for all streams (stable, testing, next) | 14:07 |
| travier | 2. Setup OpenStack userdata with Zuul SSH key for those images | 14:07 |
| travier | 3. Use Raw Ansible module to install what's need for Ansible (Python 3, etc.) and update to latest version and reboot the node | 14:07 |
| travier | 4. Use regular Ansible playbooks to run shell commands to run the job | 14:07 |
| travier | Do that looks reasonable to you? | 14:07 |
| *** nils has joined #zuul | 14:08 | |
| sean-k-mooney | travier: i think so | 14:13 |
| corvus | travier: yep; note that #3 has to happen as the first thing in a pre-run playbook in a base job, which must be created in a trusted repo. | 14:14 |
| sean-k-mooney | depending on what you are tryign to do it might make sense to spawn a container with a seconnd ssh server and have ansible connect to data by useing a different invenoty | 14:14 |
| sean-k-mooney | then you could use other modules too | 14:14 |
| travier | we need to be on the host to test container tools (toolbox) | 14:15 |
| sean-k-mooney | i see ok | 14:15 |
| sean-k-mooney | travier: why not just have python installe dvia user-data | 14:16 |
| sean-k-mooney | for that lable | 14:17 |
| sean-k-mooney | in nodepool | 14:17 |
| sean-k-mooney | then you would not need to take special stpes in the zuul jobs | 14:17 |
| mordred | yeah - that would save you from having to have a whole different ... yeah that ^^ | 14:17 |
| corvus | ++ | 14:17 |
| mordred | then you could still write a normal pre-playbook to do the update-and-reboot step if you wanted | 14:18 |
| *** rpittau is now known as rpittau|afk | 14:18 | |
| mordred | but it could come after the normal base job pre playbook heirarchy | 14:18 |
| sean-k-mooney | travier: you can have multiple lables refer to the same image by the way to | 14:18 |
| travier | I could indeed, maybe delaying starting ssh before that is completed | 14:19 |
| sean-k-mooney | if you wanted a vanila lable and a fedora-coreOS-zuul lable | 14:19 |
| tristanC | travier: that should work, you can find toolbox's zuul ssh key here: https://softwarefactory-project.io/keys/zuul_rsa.pub | 14:19 |
| travier | Thanks everyone! Will give this a try first and we'll see if it works :) | 14:23 |
| corvus | travier: let us know if it works, or if we should brainstorm other ideas :) | 14:24 |
| corvus | travier: also thanks for toolbox | 14:24 |
| travier | ^^ that's for harrymichal :) I'm a Fedora CoreOS dev :) | 14:25 |
| corvus | harrymichal: thanks for toolbox! travier thanks for making toolbox better on coreos! :) | 14:27 |
| travier | :) | 14:33 |
| clarkb | sean-k-mooney: there is a lot of scrollback. Did you get an answer to your question? I don't know what the answer is myself but I suspect that isn't currently supported | 14:37 |
| sean-k-mooney | clarkb: no i didnt but i assume that is the case too | 14:37 |
| harrymichal | corvus: :) | 14:37 |
| sean-k-mooney | clarkb: currently we have a seperate config repoo that creates jobs on other pojects | 14:37 |
| sean-k-mooney | and we just dont import jobs form the source code repos | 14:38 |
| sean-k-mooney | but it would be nice to do it in tree | 14:38 |
| sean-k-mooney | clarkb: i dont think it would be partically hard to add | 14:38 |
| corvus | sean-k-mooney: you can add files, but not subtract them | 14:39 |
| sean-k-mooney | yep | 14:39 |
| sean-k-mooney | i have seen that | 14:39 |
| sean-k-mooney | https://zuul-ci.org/docs/zuul/reference/tenants.html#attr-tenant.untrusted-projects.%3Cproject%3E.extra-config-paths | 14:40 |
| sean-k-mooney | it would be nice if we could add a flag like disable_default_config_paths=true|false | 14:41 |
| sean-k-mooney | if we had that we could set it to false and then use extra-config-path to point at the new one | 14:41 |
| corvus | sean-k-mooney: so you want to manage two different zuul systems from the same repo? | 14:41 |
| sean-k-mooney | corvus: kind of but no | 14:42 |
| sean-k-mooney | its for our downstream copy of the nova repo | 14:42 |
| sean-k-mooney | im trying to avoid touching the upstream .zuul.yaml file to avoid conflcits on rebases | 14:42 |
| sean-k-mooney | when we import chanage form stable branches | 14:43 |
| sean-k-mooney | it woudl be nice if we coudl say in our donw stream config look at this additonal file | 14:43 |
| sean-k-mooney | and ignore the default one | 14:43 |
| sean-k-mooney | the curren approch is to use a configure repor to defien the jobs out of repo but that has the disadvatage of no speculative job execution | 14:44 |
| corvus | sean-k-mooney: ok. fwiw, when we designed zuul for that use case, we designed it to support that ^ | 14:44 |
| sean-k-mooney | to support 2 different zuuls form the same repo? | 14:45 |
| corvus | understandable if priorities are different; just wanted to mention that it was a design decision, not an omission | 14:45 |
| corvus | oh no | 14:45 |
| *** jfoufas1 has quit IRC | 14:45 | |
| sean-k-mooney | oh i know why its the way it is | 14:45 |
| corvus | sean-k-mooney: zuul is a gating system, so the idea of two zuuls being responsible for the same repo does not compute :) | 14:45 |
| sean-k-mooney | it just make doing downstream ci harder | 14:45 |
| sean-k-mooney | well we have 2 repos | 14:45 |
| sean-k-mooney | and we would like zuul to be gating dowstream | 14:46 |
| sean-k-mooney | the issue here is we periodicl rebase our downstream copy to pickup the stable fixes | 14:46 |
| sean-k-mooney | and if we modifed the zuul file downstream it would confilt with upstream changes | 14:46 |
| sean-k-mooney | that is why we defien jobs out fo tree downstream | 14:47 |
| corvus | sean-k-mooney: right, i'm just saying at the time, we chose to solve that via using other repos. note that you don't have to define those jobs in config repos | 14:47 |
| sean-k-mooney | and since only config repos can define jobs for other repos we losse the ablity to test jobs before they merge without doing depens on and other hacks | 14:47 |
| corvus | sean-k-mooney: you could have a "downstream-jobs" untrusted repo with your job definitions, and you only need a config project to tell zuul to run those jobs. | 14:47 |
| corvus | depends-on is not a hack | 14:47 |
| sean-k-mooney | its not a hack but we would have to defien the job in an untrusted repo and the piplene in a truesed one | 14:48 |
| sean-k-mooney | so we go form 1 repo upstream to 3 downstream | 14:48 |
| corvus | right, but the project-pipeline doesn't change as often | 14:48 |
| sean-k-mooney | ture it s just an issue when adding new jobs | 14:49 |
| corvus | and yes, that's 2 more repos, but those 2 repos can serve all of your projects | 14:49 |
| sean-k-mooney | which is what im contimplating doing currently | 14:49 |
| corvus | (so if you have N openstack projects in this system, you'd only need N+2 repos downstream) | 14:49 |
| sean-k-mooney | ok i can do this but i was hoping i could just use one repo and override it in my tenatns file som how | 14:50 |
| sean-k-mooney | corvus: ya the ux is still worse then if we had a per repo override though | 14:50 |
| sean-k-mooney | i dont think it conflicts with zuuls design goals but just a lower priortiy nice to have feature | 14:51 |
| sean-k-mooney | i mean if we are bing strtict about it we dont need extra-config-paths either today but it improves the ux in some cases | 14:52 |
| corvus | sean-k-mooney: yep, i get where you're coming from; the main question is how much downstream patches do you want in the repo; zuul was originally designed to support zero for that case, you're willing to accept more. | 14:55 |
| corvus | sean-k-mooney: note that by locating your downstream jobs in a separate repo, you can still use/inherit from upstream jobs because you can limit the load objects to only jobs. that becomes more difficult if you exclude the entire "upstream" config file. | 14:56 |
| sean-k-mooney | corvus: oh yep i know basicaly if it was not for the restiction on defining the pipelines i would not have raised it | 14:58 |
| sean-k-mooney | corvus: and ya i know i can filter what gets loaded and how the inheritiance works. | 14:59 |
| sean-k-mooney | tl;dr our ci and qe folks woudl like the upstream devs to work more on downstream ci. we dont want to use jenkisn and want to try and reuse the upstream job from tiple o to test nova downstream with package | 15:00 |
| sean-k-mooney | corvus: today that is done by defining the jobs and piplien in a config repo but i was trying to see if i could make it feel like working on an upstream job | 15:01 |
| sean-k-mooney | if i have 2 addtional repos the answer is kind of with thw 1 repo we hav righ now less so but it can work | 15:02 |
| clarkb | corvus: https://review.opendev.org/c/zuul/zuul/+/786521 lgtm but I did leave a coupel of notes. I'll work through the stack too | 15:11 |
| corvus | sean-k-mooney: ack; to be clear i'm not irc -2ing or anything, just trying to add my perspective / experience and make sure you're aware of all the options and implications :) | 15:12 |
| corvus | clarkb: it does look like the tests are good on that stack (the failures are either expected or random; i don't see any "real" test failures | 15:16 |
| clarkb | corvus: ya there is a linter catch that I'm trying to understand (its a variable not being used and I'm trying to decide if it is meant to be used or can just be cleaned up) | 15:18 |
| corvus | tobiash: if you could look at that stack (despite the test failures) and if it looks okay, i think we can squash and merge soon | 15:18 |
| clarkb | I think it may just need to be cleaned up | 15:19 |
| corvus | clarkb: yeah, my change to that test was pretty mechanistic | 15:20 |
| clarkb | corvus: "The tests did not catch this because they pass in the format that is expected." I'll admit I used the test you fixed up as a guide :) | 15:25 |
| clarkb | but also the conditions in isUpdateNeeded :/ | 15:25 |
| clarkb | anyway good catch on that | 15:25 |
| clarkb | corvus: also left a question/comment on https://review.opendev.org/c/zuul/zuul/+/786522 to help better understand this myself | 15:28 |
| corvus | clarkb: good q, replied. (i did test that yesterday, and my reply is my recollection from that) | 15:32 |
| *** ykarel has joined #zuul | 15:35 | |
| clarkb | ok ya that makes sense | 15:35 |
| sean-k-mooney | corvus: yep apriciate that. baiscaly if im not willing to spend the time to and try to scratch my own itch im not going to be upset if you are going to volenteer to do it either :) | 15:38 |
| sean-k-mooney | clarkb: we have 3 workable solutions without actully changeing zuul | 15:38 |
| sean-k-mooney | sorry that was for corvus ^ | 15:39 |
| *** rlandy|rover is now known as rlandy|rover|tra | 15:40 | |
| *** rlandy|rover|tra is now known as rlandy|rvr|train | 15:40 | |
| *** hashar has quit IRC | 15:41 | |
| *** ikhan has joined #zuul | 15:53 | |
| tobiash | corvus, clarkb: commented on 786522 | 15:58 |
| corvus | tobiash: ++ | 16:00 |
| *** ykarel has quit IRC | 16:01 | |
| *** ykarel has joined #zuul | 16:02 | |
| clarkb | I think the end result is the same but much clearer in tobiash's suggested state | 16:02 |
| clarkb | (in both cases we would skip right?) | 16:03 |
| tobiash | clarkb: if the repo is not part of the repo state (should not happen anymore with global repo state) we need always an update | 16:04 |
| clarkb | I see so the check for project_repo_state is None would cause an update to happen (I read it as skip in taht case) | 16:05 |
| tobiash | corvus: there is something I don't yet understand in 786523 | 16:09 |
| tobiash | rest of the stack lgtm | 16:09 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: WIP: Digitalocean driver https://review.opendev.org/c/zuul/nodepool/+/759559 | 16:09 |
| corvus | tobiash: your question is actually very similar/related to clarkb's -- in that the key to both is what Repo._reset() does or doesn't do with repo_state | 16:11 |
| corvus | tobiash: in 785310 we do a restore on add_project, but otherwise getRepo doesn't do a restore for an existing project | 16:14 |
| tobiash | ah yes | 16:14 |
| corvus | i'm totally open to making the repo_state restore more low level (so we do it in reset or getrepo instead of mergeChangess or other high-level methods) | 16:15 |
| corvus | though i'd suggest if we want to do that, we at least fix up the current changes as-is, then make that change on top (since it looks like we more or less understand the current series and it just needs some tidying up) | 16:17 |
| *** rlandy|rvr|train is now known as rlandy|rover | 16:23 | |
| *** hamalq has joined #zuul | 16:28 | |
| *** hamalq has quit IRC | 16:28 | |
| *** hamalq has joined #zuul | 16:29 | |
| *** jpena is now known as jpena|off | 16:31 | |
| *** ykarel has quit IRC | 16:42 | |
| clarkb | ya I think we can fix this stack up but it should be pretty close to mergeable? | 16:44 |
| clarkb | by fixup I mean fix the linting issues and clean up some of the extra lines | 16:44 |
| corvus | ya | 16:46 |
| corvus | tobiash, clarkb: if we do move repo_state to lower-level, we might be able to drop this https://opendev.org/zuul/zuul/src/branch/master/zuul/executor/server.py#L1108-L1112 | 16:47 |
| corvus | (because presumably the getRepo call above would have set the state) | 16:48 |
| *** jcapitao has quit IRC | 16:55 | |
| *** nils has quit IRC | 16:56 | |
| *** VinayJain212 has quit IRC | 17:15 | |
| *** ajitha has joined #zuul | 17:40 | |
| *** vishalmanchanda has quit IRC | 18:04 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Add a fast-forward test https://review.opendev.org/c/zuul/zuul/+/786521 | 18:46 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Correct repo_state format in isUpdateNeeded https://review.opendev.org/c/zuul/zuul/+/786522 | 18:46 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Revert "Revert "Make repo state buildset global"" https://review.opendev.org/c/zuul/zuul/+/785535 | 18:46 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Fix missing repo state restore https://review.opendev.org/c/zuul/zuul/+/785310 | 18:46 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Keep jobgraphs frozen across reconfiguration https://review.opendev.org/c/zuul/zuul/+/785536 | 18:46 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Restore repo state in checkoutBranch https://review.opendev.org/c/zuul/zuul/+/786523 | 18:47 |
| corvus | that's updates from comments; i'm going to squash the 2 that need to go together now | 18:47 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Fix repo state restore / Keep jobgraphs frozen https://review.opendev.org/c/zuul/zuul/+/785536 | 18:51 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Restore repo state in checkoutBranch https://review.opendev.org/c/zuul/zuul/+/786523 | 18:51 |
| corvus | I expect that to pass tests and be ready for final review/merge; we'll see what the tests say | 18:52 |
| clarkb | for the question that just arrived to the zuul-discuss list I think the answer is that job level vars are passed as inventory vars (not passed on the command line). Secrets are passed on the command line. Is that correct? | 18:54 |
| corvus | clarkb: i have a response in progress | 18:55 |
| clarkb | ah ok I'll wait to read it then :) | 18:55 |
| clarkb | https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/ensure-nodejs/tasks/main.yaml#L26-L27 node_version is the var to set fwiw | 18:55 |
| clarkb | and it is set as a default, https://opendev.org/zuul/zuul-jobs/src/branch/master/roles/ensure-nodejs/defaults/main.yaml, which should make it easy to override | 18:56 |
| corvus | yeah, i think they've got it, i think we basically just need to point to which zuul thing to use to set it | 18:56 |
| clarkb | I also think there may be a recent openstack jobs example of fixing this there | 18:56 |
| corvus | i will suggest both the job but also the project level | 18:56 |
| clarkb | corvus: let me find a link to the openstack chagne as that may be illustrative | 18:56 |
| corvus | clarkb: ok | 18:56 |
| corvus | clarkb: zuul itself does this which i think is super clever: https://opendev.org/zuul/zuul/src/branch/master/.zuul.yaml#L271 | 18:56 |
| *** rlandy|rover is now known as rlandy|rvr|train | 18:56 | |
| clarkb | ya and openstack does version specific examples | 18:57 |
| clarkb | https://opendev.org/openstack/openstack-zuul-jobs/src/branch/master/zuul.d/project-templates.yaml#L2164-L2187 | 18:58 |
| clarkb | both may be helpful? | 18:58 |
| corvus | clarkb: thx, replied | 19:09 |
| openstackgerrit | Guillaume Chauvel proposed zuul/zuul master: BLOB Restore repo state in checkoutBranch https://review.opendev.org/c/zuul/zuul/+/786710 | 19:31 |
| guillaumec | corvus, oops | 19:31 |
| corvus | guillaumec: no problem; i thought about doing that actually :) | 19:32 |
| guillaumec | corvus, wrong copy/paste when i uploaded a change on top of your during my tests | 19:32 |
| openstackgerrit | Guillaume Chauvel proposed zuul/zuul master: Fix FakeGerritRefWatcher and FakeGerritPoller events https://review.opendev.org/c/zuul/zuul/+/786711 | 19:33 |
| openstackgerrit | Guillaume Chauvel proposed zuul/zuul master: Fix FakeGerritRefWatcher and FakeGerritPoller events https://review.opendev.org/c/zuul/zuul/+/786712 | 19:36 |
| corvus | guillaumec: thanks! a comment in _poll would be good so we know why it's set up like that. otherwise i'll probably want to put it back in _run every time i see it :) | 19:36 |
| *** ajitha has quit IRC | 19:43 | |
| openstackgerrit | Guillaume Chauvel proposed zuul/zuul master: Fix FakeGerritRefWatcher and FakeGerritPoller events https://review.opendev.org/c/zuul/zuul/+/786712 | 19:48 |
| *** rlandy|rvr|train is now known as rlandy|rover | 20:04 | |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: digitalocean: add image filter config https://review.opendev.org/c/zuul/nodepool/+/786723 | 20:05 |
| corvus | guillaumec: +2 thanks! | 20:08 |
| clarkb | corvus: that conflicts with your stack do we want to get one or the other in first? | 20:09 |
| corvus | clarkb: looks like it's not a real conflict; i can cherry-pick it | 20:10 |
| corvus | just a gerrit-conflict :) | 20:10 |
| clarkb | ah | 20:10 |
| corvus | and looks like i have test failures, so if you're going to +w it i'd say go for it :) | 20:11 |
| clarkb | oh I was just going through your stack again | 20:12 |
| clarkb | let me review guillaumec's | 20:12 |
| clarkb | 786712 has been approved | 20:15 |
| corvus | there are 2 legit test failures that are caused by the change tobiash requested to check for the project_repo_state being None in updateRepo; i'm digging into it. | 20:15 |
| corvus | okay, so there *is* an invocation of merger methods that pass in a repo state that is really a project repo state; so it's possible that in some cases isUpdateNeeded was working as intended | 20:23 |
| corvus | but i think the right solution to this is still to just have only one kind of repo state as part of the merger api | 20:23 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Revert "Revert "Make repo state buildset global"" https://review.opendev.org/c/zuul/zuul/+/785535 | 20:28 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Fix repo state restore / Keep jobgraphs frozen https://review.opendev.org/c/zuul/zuul/+/785536 | 20:28 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Restore repo state in checkoutBranch https://review.opendev.org/c/zuul/zuul/+/786523 | 20:28 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: digitalocean: add image filter config https://review.opendev.org/c/zuul/nodepool/+/786723 | 20:30 |
| clarkb | corvus: is https://review.opendev.org/c/zuul/zuul/+/785535/5/zuul/executor/server.py where you had to modify the revert? | 21:00 |
| corvus | clarkb: it's easy to see on a ps4->ps5 diff | 21:11 |
| corvus | clarkb: yes specifically around line 3029 in that file. | 21:12 |
| clarkb | yup with the update task | 21:12 |
| corvus | clarkb: line 1317 and 1319 i modified to match variable names to increase likelihood of future greps being helpful | 21:12 |
| corvus | but was not required | 21:12 |
| corvus | basically normalizing on "project_repo_state" as the name of a part of a repo state | 21:13 |
| clarkb | ++ | 21:13 |
| *** harrymichal has quit IRC | 21:19 | |
| *** harrymichal has joined #zuul | 21:19 | |
| clarkb | corvus: the diff between https://review.opendev.org/c/zuul/zuul/+/785536/3..7 patchsets 3 and 7 there isn't quite what I expected. It shows the suggested fixups from review but not what I expected from the squashing | 21:25 |
| clarkb | was this a case of largely noop squashing because 785536 ended up rewriting what was squashed into it prior? | 21:25 |
| corvus | clarkb: yeah, i think the way to look at it is that both patchsets have the other change incorporated in them (one via another commit, one via this one); nothing about it changed, so it doesn't show up in the diff. | 21:28 |
| corvus | an interdiff of the patchsets would probably show the other change | 21:28 |
| clarkb | the stack lgtm, assuming tests pass I think this is probably ready | 21:28 |
| clarkb | ya, the diff and squashing just melted my brain a bit | 21:29 |
| corvus | meta git | 21:29 |
| clarkb | also, I'm glad I pushed for the extra test case as I think that helped shake out a bunch of related issues | 21:34 |
| corvus | yep. it's still doing so :) | 21:36 |
| corvus | tobiash: i'm looking at https://review.opendev.org/648229 to try to make sure i understand everything | 21:36 |
| corvus | tobiash: Repo.reset() does Repo.update() [which is a git fetch] and then Repo._reset() which is basically a very complicated git reset --hard | 21:38 |
| corvus | 648229 made it so we skip Repo.reset() on the Merger.updateRepo() call. but we still call Repo.reset() on Merger.mergeChanges() and Merger.setRepoState(). that means that in the executor, most of the repos are still going to have Repo.reset() called on them (the only high-level method that doesn't call Repo.reset() is Merger.checkoutBranch() -- so basically repos like zuul-jobs) | 21:41 |
| corvus | tobiash: so was the benefit of 648229 that it sometimes reduced 2 reset() calls to one? | 21:41 |
| openstackgerrit | Merged zuul/zuul master: Fix FakeGerritRefWatcher and FakeGerritPoller events https://review.opendev.org/c/zuul/zuul/+/786712 | 21:42 |
| corvus | clarkb: ^ also if you have any thoughts | 21:43 |
| clarkb | sure, let me finish reviewing this gerrit acl change | 21:43 |
| corvus | i'm kind of struggling with figuring out what it is we're trying to optimize | 21:43 |
| corvus | i *think* what we really want to do is eliminate unecessary fetches, but in all cases we should still do a git reset for any of the 3 high level methods (mergeChanges, checkoutBranch, setRepoState) because that would either clean things up or be a no-op | 21:45 |
| *** rlandy|rover has quit IRC | 21:49 | |
| avass | corvus: I'm working on getting image builds for digital ocean zuul by bootstrapping from a cloud provider image, installing things in zuul+ansible like a normal job, stopping the instance and snapshotting it. but I realized the instance id is not part of the nodepool data in the inventory | 21:50 |
| clarkb | corvus: when I was reading the code trying to figure out the original issue fungi and I found the rough set of operations seemed to be: update/fetch if necessary, merge changes, set refs. Reading your comments above and the commit message of that change I think your understanding is what the intent was | 21:50 |
| clarkb | corvus: basically make that first update/fetch step as cheap as possible | 21:50 |
| avass | is it possible to add data that zookeeper should pass to zuul with the statemachine framework? | 21:50 |
| avass | oh, maybe external_id should just be added to the zk data | 21:51 |
| clarkb | avass: makes sense to have the instance uuid in zk data, but if you can't make that work for some reaosn or miss some other data DO does support the metadata service https://docs.digitalocean.com/products/droplets/how-to/retrieve-droplet-metadata/ | 21:51 |
| avass | there's also a 'host_id' field that is null. not sure what that is for | 21:51 |
| clarkb | you should be able to get the instances uuid and other info via metadata | 21:52 |
| avass | clarkb: ah yeah that could work | 21:52 |
| clarkb | avass: host_id is the identity of the host platform running the test instance | 21:52 |
| clarkb | avass: its useful for helping providers track down issues if you have jobs that always fail on a host_id | 21:52 |
| clarkb | not all clouds provide it though | 21:52 |
| corvus | avass: including the external_id in the inventory sounds reasonable | 21:52 |
| corvus | i think that would just be a zuul change, not nodepool | 21:53 |
| clarkb | host_id == hypervisor id roughly | 21:53 |
| clarkb | but it may be a k8s node id etc | 21:54 |
| avass | clarkb, corvus: droplet_id is part of the metadata but it could be useful for other providers (or if the metadata service is disabled) | 21:54 |
| avass | so I'll see if I can add it | 21:54 |
| corvus | avass: should just be in executor/server.py | 21:54 |
| avass | oh right that's already stored in zk so it should be enough to add it to the inventory? | 21:55 |
| corvus | ya | 21:56 |
| corvus | avass: in fact, just search for host_id to find the spot :) | 21:57 |
| openstackgerrit | Albin Vass proposed zuul/zuul master: Add nodepool.external_id to inventory https://review.opendev.org/c/zuul/zuul/+/786738 | 21:58 |
| avass | corvus: already did :) | 21:58 |
| corvus | in the merger server, why does updateRepo happen outside of the repo lock? it can reset the repository, including overwriting branch heads | 22:05 |
| corvus | it seems like if one process is running mergeChanges, it could be writing branch heads that get overwritten by another process handling a cat job | 22:06 |
| corvus | (obvs this only applies to the executor-as-merger, not actual mergers since they are single threads/processes) | 22:06 |
| *** ikhan has quit IRC | 22:07 | |
| *** ikhan has joined #zuul | 22:16 | |
| *** ikhan has quit IRC | 22:20 | |
| *** sduthil has quit IRC | 22:21 | |
| *** harrymichal has quit IRC | 23:00 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Clarify merger updates and resets https://review.opendev.org/c/zuul/zuul/+/786744 | 23:34 |
| corvus | clarkb, tobiash: ^ unsure if that passes tests, but that's sort of my thinking on how to make this more clear. i think it's actually easier to limit repo_state to the high-level methods rather than low-level. | 23:35 |
| *** tosky has quit IRC | 23:38 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!