SpamapS | jeblair: should we make it automatically ignore /COMMIT_MSG ? | 00:00 |
---|---|---|
SpamapS | I say that, because that will always show up in every change. | 00:00 |
SpamapS | and would we even go so far as to ignore it in change_matchers.MatchAllFiles ? | 00:01 |
adam_g | hey. so digging into some tests, looking through test_success_pattern. it looks {success, failure}-pattern has been replaced by {success, failure}-url, is that correct? | 00:02 |
jeblair | SpamapS: i think there was some stuff to handle that at some point, though i may have broken it. git spelunking for when i created irrelevant-files may be in order. regardless, yeah, i think whatever is necessary to remove commit_msg from consideration is the right thing. i think the complexity may come with merge commits? in that case the *only* file is commit_msg... | 00:05 |
SpamapS | jeblair: with merge commits, I'd think we'd be applying file matchers to the children..? | 00:08 |
jeblair | adam_g: yes -- and i *think* that's just a name change | 00:09 |
jeblair | adam_g: ('cause what's a success-pattern? :) | 00:09 |
adam_g | jeblair: is there anything to be considered wrt backward compat of job config or is that out the window for v3? | 00:10 |
jeblair | adam_g: out the window | 00:10 |
adam_g | \o/ | 00:10 |
jeblair | (it's so different, the answer will involve phrases like 'conversion guide' and 'translation tool') | 00:11 |
jeblair | SpamapS: this is among the reasons skip-if is problematic. we don't really get that information from gerrit, and i think extracting it would be non-trivial. i think the existing solution just tried to get out of the way and let the job run on a merge commit, which is probably the safest simple thing. | 00:12 |
SpamapS | ahh, seems dbe6fab14f446b1929b2b30c31a18d489d16b272 had the logic I've been thinking of | 00:12 |
jeblair | SpamapS: yep. that's desirable. did i break that? | 00:12 |
SpamapS | jeblair: I believe so yes. | 00:13 |
jeblair | yay tests | 00:13 |
SpamapS | \o/ | 00:13 |
SpamapS | So I think there are really now 2 things to test | 00:13 |
SpamapS | 1) On a regular change with file changes, based on irrelevant-files, does the job run/not run. | 00:14 |
SpamapS | 2) On a merge commit, is irrelevant-files ignored entirely? | 00:14 |
SpamapS | Is that too simplistic? | 00:14 |
jeblair | SpamapS: that sounds right. as i continue to think about this, i wonder what would have happened before if a merge commit *did* have a file (i think this happens when it resolves a conflict). i wonder if in that case, the normal skip-if would apply and we might end up not running a job we should (because the merge commit fixed a conflict in an irrelevant file, but a child change touched a relevant file). that behavior sounds wrong and yours ... | 00:17 |
jeblair | ... sounds right. | 00:17 |
jeblair | SpamapS: (but i bet the old logic didn't check whether it was a merge commit, it just narrowly handled the case of commit with no files) | 00:18 |
jeblair | SpamapS: if you want to be bug-compatible with v2 on that and backlog improving it, that would be okay. :) | 00:19 |
SpamapS | jeblair: indeed. Well I think first I'll test that irrelevant-files works on regular changes.. and then add another commit that tests merges. | 00:19 |
SpamapS | and yeah, if the logic gets hairy, I'd be fine with an "expected fail" test. | 00:19 |
SpamapS | "We know this is broke.. we'll decide when we let it un-break" | 00:19 |
SpamapS | jeblair: actually you did not break the /COMMIT_MSG assumption | 00:22 |
SpamapS | jeblair: something else is still letting jobs run even when they match. :-/ | 00:22 |
jeblair | SpamapS: yay me? :) | 00:25 |
SpamapS | yes yay you | 00:26 |
* SpamapS is in pdb hell | 00:26 | |
jeblair | i get a momentary reprieve until you find out what i *did* break | 00:26 |
SpamapS | I'm still leaning toward me breaking things in my test harness | 00:32 |
SpamapS | but.. that's just my self deprecating side | 00:32 |
SpamapS | looking like maybe a pretty deep logic problem in variant inheritance... getting closer anyway | 00:37 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Remove builds that are no longer in use https://review.openstack.org/399307 | 00:38 |
jeblair | clarkb, ianw: ^ an actual tested process for removing an image (and provider)! | 00:39 |
jeblair | SpamapS: certain to be my fault if so; sic me on it as soon as your ready (though i have EOD brain now, so probably tomorrow at earliest). | 00:40 |
jeblair | *you're* ready (told you; eod brain) | 00:41 |
clarkb | jeblair: thanks will try to read in the morning.currently dealing with twins that have fevers do not recommend | 00:43 |
SpamapS | jeblair: running into similar trouble here. :-) | 00:44 |
adam_g | jeblair: if you've got anything left in that EOD brain.. any tips on whats happening at https://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/launcher/server.py?h=feature/zuulv3#n370 and why that url is stomping on the urls configured on the job? | 00:45 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Use constants for zk states https://review.openstack.org/399309 | 00:57 |
jeblair | adam_g: that should be the url that gets used if there is no success-url (or failure-url) defined. ie, the normal behavior is "use whatever url the launcher gave us". | 00:58 |
jeblair | adam_g: realistically, in v3, the launcher should not be responsible for that, so if you wanted to rip that part out, feel free. :) | 00:59 |
adam_g | jeblair: ok, yeah, it looks to be stomping on the defined success-url AFAICS | 00:59 |
jeblair | (ripping it out would probably be a distraction though, since apparently there's a logic reversal; you may just want to backlog that idea) | 01:00 |
adam_g | well, ill certainly be digging thru more tomorrow to figure out whats going on. the test seems to be legit failing now on the wrong message being reported. still dont fully grok what goes on once the job is launched there. | 01:01 |
jeblair | adam_g: that is strange. it looks like Item.formatJobResult dtrt. that should be what's called by Reporter._formatItemReportJobs to produce that part of the report | 01:04 |
jeblair | adam_g: oh! | 01:06 |
jeblair | adam_g: there is an exception handler in there that will hide an exception in formatting the url; i bet that's it | 01:06 |
* adam_g takes a look | 01:07 | |
jeblair | https://git.openstack.org/cgit/openstack-infra/zuul/tree/zuul/model.py?h=feature/zuulv3#n939 | 01:07 |
adam_g | hmm. the job seems not to have the message or url params configured at that point | 01:09 |
adam_g | back to the top | 01:09 |
ianw | "The repository 'http://mirror.regionone.osic-cloud1.openstack.org/ubuntu xenial Release' is not signed." | 01:14 |
*** mgagne has quit IRC | 06:18 | |
*** mgagne has joined #zuul | 06:21 | |
*** mgagne is now known as Guest52285 | 06:21 | |
*** jamielennox is now known as jamielennox|away | 06:36 | |
*** willthames has quit IRC | 07:02 | |
openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul: Refactor skip-if tests to use irrelevant-files https://review.openstack.org/399415 | 07:47 |
SpamapS | jeblair: so, when you're around... in writing the test above^ I had some weird variant stuff just by doing the configs a different way. If I defined the job in the 'jobs' section and then mentioned it in the projects section.. I got two variants, one of which did not have the irrelevant-files section, and thus, which would still run. | 07:48 |
*** jamielennox|away is now known as jamielennox | 08:03 | |
*** openstackgerrit has quit IRC | 08:03 | |
*** openstackgerrit has joined #zuul | 08:04 | |
*** hashar has joined #zuul | 08:10 | |
*** hogepodge has quit IRC | 08:15 | |
openstackgerrit | Joshua Hesketh proposed openstack-infra/zuul: Fix missing mutex release when aborting builds https://review.openstack.org/384980 | 09:15 |
*** willthames has joined #zuul | 09:55 | |
*** willthames has quit IRC | 10:34 | |
*** rcarrillocruz has joined #zuul | 10:53 | |
*** rcarrillocruz has quit IRC | 11:39 | |
*** rcarrillocruz has joined #zuul | 11:40 | |
*** abregman|afk has quit IRC | 12:16 | |
*** hogepodge has joined #zuul | 12:31 | |
*** abregman has joined #zuul | 12:42 | |
*** abregman is now known as abregman|afk | 12:57 | |
Shrews | jeblair: pabelanger: Still injecting caffeine into my system, but I don't think the test in 399230 is valid. | 13:13 |
Shrews | left a comment | 13:14 |
*** abregman|afk is now known as abregman | 13:49 | |
pabelanger | morning | 13:58 |
pabelanger | Shrews: looking | 13:58 |
*** lindycoder has joined #zuul | 14:00 | |
*** abregman has quit IRC | 14:03 | |
*** abregman has joined #zuul | 14:15 | |
pabelanger | Shrews: also left a comment | 14:20 |
*** rcarrillocruz has quit IRC | 14:30 | |
Zara | heh, I should probably write to the list in a bit-- if you're having problems with storyboard, and filing an issue about storyboard itself isn't possible, please feel free to ask us about stuff in #storyboard; it might be that we've done a bad job explaining something, or that something is still in-progress, but we can only improve things when we know what isn't working. | 14:43 |
Zara | api issues should be logged at: https://storyboard.openstack.org/#!/project/456 , ui issues at: https://storyboard.openstack.org/#!/project/457 | 14:46 |
*** rcarrillocruz has joined #zuul | 14:51 | |
*** abregman is now known as abregman|afk | 14:52 | |
Zara | oh, and now we have boartty (thanks, jeblair)! https://storyboard.openstack.org/#!/project/854 | 14:58 |
Zara | (so if the issue for the web ui would be 'everything', maybe try that, though it's still young.) | 14:59 |
Shrews | Zara: I assume that is posted in response to my email response to SpamapS. It's difficult to create an issue when it's the entire UX that is tripping me up. | 14:59 |
Shrews | As someone new to using it, it is not intuitive as to relationships between Dashboards, and Worklists, and Boards, and Stories and Projects (as a start to my issues). | 15:00 |
Shrews | but, happy to chat outside of #zuul if you want more details | 15:01 |
Zara | Shrews: yeah, it's to the whole thread really, and thank you! I've got to vanish for a meeting but I'll be back shortly :), generally #storyboard is the place to go if filing an issue via the ui itself isn't possible | 15:01 |
SpamapS | Shrews: what response btw? Haven't read my list mail lately | 15:08 |
Shrews | SpamapS: just replied a little bit ago | 15:08 |
SpamapS | Ah, I'll go peek | 15:09 |
SpamapS | Shrews: we definitely need people to assign themselves and in many instances add tasks (like on the test re-enablement story) | 15:12 |
SpamapS | But you can adjust just squawk at me and I can do it for you if the UI is truly annoying you. | 15:12 |
SpamapS | s/adjust/also/. DYAC | 15:12 |
*** abregman|afk has quit IRC | 15:22 | |
*** abregman has joined #zuul | 15:29 | |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add tests for enable / disabled labels https://review.openstack.org/399642 | 15:36 |
pabelanger | Shrews: jeblair: ^ that is going to fail, but wanted to first see if that is the correct way to disable a image from being built. min-ready was the mentioned in zuulv2 | 15:37 |
openstackgerrit | Ricardo Carrillo Cruz proposed openstack-infra/zuul: Re-model the job auth https://review.openstack.org/399645 | 15:38 |
rcarrillocruz | weee | 15:39 |
rcarrillocruz | i'm back to doing zuul things | 15:39 |
rcarrillocruz | slowly getting settled in Granada | 15:39 |
pabelanger | zuul all the things | 15:39 |
rcarrillocruz | (helo folks!) | 15:39 |
pabelanger | rcarrillocruz: how did your talk go? Was it recorded? | 15:39 |
rcarrillocruz | i hope not! | 15:40 |
rcarrillocruz | it was ok i guess | 15:40 |
rcarrillocruz | i mean | 15:40 |
rcarrillocruz | zuul is super dense | 15:40 |
rcarrillocruz | for people not from openstack land | 15:40 |
rcarrillocruz | can be hard to grasp concepts | 15:40 |
rcarrillocruz | i saw quite a few 'i understand nothing' faces | 15:40 |
rcarrillocruz | i heard you going to talk at openstack canada day ? | 15:41 |
mordred | rcarrillocruz: yup. I know those faces well | 15:41 |
rcarrillocruz | pabelanger: ^ | 15:41 |
rcarrillocruz | :D :D | 15:41 |
pabelanger | rcarrillocruz: yup, next Tuesday. Infra-cloud | 15:41 |
rcarrillocruz | awesome | 15:42 |
rcarrillocruz | yolanda told me she'll meet you there | 15:42 |
rcarrillocruz | she has some customer meeting i believe in montreal | 15:42 |
pabelanger | haha, ya. I think we'll have some snow for her too | 15:42 |
yolanda | pabelanger, neh... i looked at the weather reports and is not that bad | 15:43 |
yolanda | going to go shopping anyway, i really don't have clothes for cold weather, just some ski equipment | 15:43 |
pabelanger | Warm jacket an shoes for sure | 15:44 |
*** abregman is now known as abregman|afk | 15:44 | |
rcarrillocruz | it's going to take a while for me to get used to Granada | 15:44 |
rcarrillocruz | 0-1 degrees now | 15:44 |
rcarrillocruz | in Marbella is like 20 | 15:44 |
rcarrillocruz | sigh | 15:44 |
mordred | rcarrillocruz: oy | 15:45 |
mordred | rcarrillocruz: why did you move away from marbella? it's so purty there :) | 15:45 |
rcarrillocruz | inorite? my wife got an offer to open a new branch (she's an optics shop manager) , and life cost here is WAY CHEAPER to Marbella. Marbella is actually the first or second most expensive city, we need a new house and we'd have to go to very far outskirts to get something decent | 15:48 |
rcarrillocruz | good news is that we're just 1h:30 drive from there, so we can go whenever on weekends | 15:48 |
rcarrillocruz | mordred , SpamapS : just going a round over the zuul v3 tasks, https://storyboard.openstack.org/#!/story/2000772 is complete no? | 15:50 |
SpamapS | rcarrillocruz: no idea | 15:51 |
rcarrillocruz | or jeblair if you're around ^ | 15:51 |
rcarrillocruz | i don't see anything that would prevent a playbook from running, we already transform JJB to ansible playbook on-the-fly now | 15:52 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add tests for enable / disabled labels https://review.openstack.org/399642 | 15:54 |
pabelanger | Shrews: jeblair: ^ I believe that restores v2 functionality for disabling image-builds | 15:54 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add tests for enable / disabled labels https://review.openstack.org/399642 | 15:58 |
mordred | rcarrillocruz: nod. having recently moved to a much cheaper place from a much more expensive place - I can tell you it's worth it :) | 15:58 |
pabelanger | ++ | 15:59 |
rcarrillocruz | yeah, i will also literally pick my son with a 4min walk at kindergarten, whereas in Marbella you need a car for EVERYTHING | 15:59 |
pabelanger | plan on doing that too, once we sell our condo | 15:59 |
rcarrillocruz | here public transport is also a thing, Marbella is non-existant, buses are terrible | 15:59 |
mordred | rcarrillocruz: for https://storyboard.openstack.org/#!/story/2000772 | 16:02 |
mordred | rcarrillocruz: in v2.5 we do the jjb transform, but in v3 we need to find playbooks/roles in paths and run those | 16:02 |
rcarrillocruz | ah, gotcha | 16:02 |
mordred | so there will be some work to take what's in 2.5 and expand/improve it | 16:02 |
mordred | I think jeblair or jhesketh started the work of forward-porting the 2.5 code into v3 already | 16:03 |
jeblair | yeah, the forward porting is done. someone needs to actually write the "get a playbook and run it" code. | 16:06 |
jeblair | we also need to deal with roles, but that spec update is still in-progress. | 16:06 |
jeblair | Shrews, pabelanger: responded on 399230 | 16:08 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Use constants for zk states https://review.openstack.org/399309 | 16:09 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Remove builds that are no longer in use https://review.openstack.org/399307 | 16:09 |
Shrews | jeblair: but, getMostRecentImageUpload() only works per provider. You never actually verify anything about the removed provider | 16:13 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add tests for enable / disabled labels https://review.openstack.org/399642 | 16:13 |
Shrews | jeblair: "The main thing we're doing here is making sure that fake-image is gone from fake-provider2" ... there's nothing there to verify that other than the waitForImageDeletion | 16:14 |
Shrews | jeblair: oh, maybe that's enough. now i see the intent | 16:14 |
Shrews | didn't understand the assertEqual there, but the added explanation makes sense now | 16:15 |
Shrews | jeblair: +1'd | 16:16 |
jeblair | Shrews: w00t. maybe i should throw a comment around rando extra-credit test asserts like that. :) | 16:18 |
Shrews | jeblair: i suggested that in the +1, but not a reason to -1 for now | 16:19 |
Zara | Shrews: okay, back from meeting, though getting on a plane in a bit! As it happens, I've long been concerned about that issue; I started some work toward drafting a guide to how things relate; https://storyboard.openstack.org/#!/story/2000667 is the beginning of an overview. I planned to do more as we got more input on what was confusing, make it prettier, and move it somewhere more visible. *really* the ui lay | 16:22 |
Zara | out itself should convey how things relate, but that's a bigger task. | 16:22 |
Shrews | jeblair: 399307 ... we haven't yet defined __eq__ for ImageBuild. Need to add that | 16:23 |
Zara | (and I feel bad for being distracting in #zuul so suggest we continue the conversation in #storyboard) | 16:24 |
Shrews | Zara: let's pick it up after plane things | 16:25 |
Zara | np, and thanks. :) | 16:26 |
jeblair | Shrews: oh, i changed the construction of builds_to_keep to filter down from all builds, so that will actually be object identity matching | 16:27 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add tests for enable / disabled labels https://review.openstack.org/399642 | 16:27 |
Shrews | jeblair: hrm, and just realized... we added provider for equality in ImageUpload, but we also need image name. Same for ImageBuild. *sigh* | 16:27 |
Shrews | jeblair: ah | 16:27 |
jeblair | Shrews: (i was adding a third 'list of builds' so i thought i'd go ahead and do that to simplify things so we could do set operations, etc) | 16:27 |
jeblair | saves some zk traffic and nixes that race condition too | 16:28 |
*** hashar is now known as hasharAway | 16:29 | |
Shrews | jeblair: gotcha. i'll work up a change for that ImageUpload equality thing | 16:29 |
jeblair | cool | 16:31 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add test for provider max-servers enable / disable https://review.openstack.org/399681 | 16:39 |
pabelanger | jeblair: Shrews: clarkb: that should add coverage when we need to disable a provider (removal for nodepool) and only do uploads (max-servers 0) | 16:39 |
pabelanger | which is broken today :) | 16:40 |
clarkb | pabelanger: I thought that was working and its max-servers -1, don't do uploads which doesn't work | 16:41 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Use image name in ImageUpload equality https://review.openstack.org/399683 | 16:42 |
pabelanger | clarkb: Ya, I call that disabled provider, but max-server -1 is covered now | 16:43 |
jeblair | pabelanger: left a comment on that | 16:45 |
clarkb | pabelanger: ok I guess I don't understand what is broken about max-servers: 0 today | 16:46 |
pabelanger | jeblair: replied | 16:48 |
jeblair | clarkb, pabelanger: i think we have an option to perhaps have a clearer way to disable images -- simply remove the label section entirely, which is what i did in the test in https://review.openstack.org/399307 | 16:49 |
pabelanger | clarkb: I should have been more clear, sorry about that. max-servers: 0 works correctly today, max-servers: -1 is broken. 399681 adds coverage for both now | 16:49 |
jeblair | oh this is max-servers. | 16:49 |
jeblair | i don't think we ever did max-servers=-1 | 16:49 |
pabelanger | I thought we did | 16:49 |
pabelanger | let me double check | 16:49 |
clarkb | ya pretty sure that <0 max servers implied no image builds for a time | 16:55 |
jeblair | thought that was only min-ready | 16:56 |
pabelanger | 375448 was the change to master branch, but didn't get tests working | 16:56 |
clarkb | I want to say it could be toggled provider wide with max-servers, but maybe I crossed the wired there | 16:57 |
jeblair | clarkb, pabelanger: i'm leaning toward making this not about numbers but more about data structure. examples: https://etherpad.openstack.org/p/HCNcnSkBdj | 16:58 |
jeblair | (that works in the v3 branch) | 16:58 |
pabelanger | looking | 16:59 |
jeblair | basically, if you don't want an image in a provider, remove the 'image' stanza for that image from that provider. | 17:00 |
clarkb | ya that works | 17:00 |
jeblair | if you don't want an image *at all*, remove all traces of it except the diskimage section (which is needed to tell the builder that it's reponsible for it; once the image is removed, you can remove the diskimage section) | 17:00 |
jeblair | we might even be able to smooth that last bit out, actually, with a bit more work (a builder could check its local disk for an image and see that it is responsible for it even if it doesn't show up in its config) | 17:01 |
pabelanger | jeblair: clarkb: sure, we can do that | 17:02 |
openstackgerrit | Clint 'SpamapS' Byrum proposed openstack-infra/zuul: Refactor skip-if tests to use irrelevant-files https://review.openstack.org/399415 | 17:13 |
jeblair | pabelanger, clarkb: if we also want to support max-servers=-1, we can last 681 but should make a corresponding change to the delete obsolete uploads method so that it deletes any already uploaded images | 17:14 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Add test for provider uploads enable / disable https://review.openstack.org/399681 | 17:17 |
pabelanger | jeblair: clarkb: ^ wasn't had to implement the etherpad, it works out of box :) | 17:18 |
jeblair | pabelanger: well, that is what i spend yesterday doing :) | 17:19 |
Shrews | anyone care to +A 399230? already two +2's | 17:19 |
pabelanger | jeblair: I do like max-server: -1, as it would be a one line change to disable uploads, images: [] works, but more line of code change | 17:19 |
pabelanger | jeblair: Yay | 17:19 |
clarkb | jeblair: https://review.openstack.org/#/c/399307/ being the important one right? | 17:20 |
jeblair | clarkb: yeah, that's the delete-image-builds change | 17:20 |
clarkb | I will start reviewing that stack in a bit, then its off to making a new zanata server | 17:21 |
openstackgerrit | Merged openstack-infra/nodepool: Remove obsolete uploaded images https://review.openstack.org/399230 | 17:24 |
openstackgerrit | Merged openstack-infra/nodepool: Add check for uploads to deleteBuild() https://review.openstack.org/399213 | 17:25 |
openstackgerrit | Merged openstack-infra/nodepool: Update test_dib_image_delete to validate delete happened https://review.openstack.org/399196 | 17:25 |
openstackgerrit | Merged openstack-infra/nodepool: Remove image delete tests from test_nodepool https://review.openstack.org/398657 | 17:26 |
openstackgerrit | Merged openstack-infra/nodepool: Add image addition test to builder https://review.openstack.org/399294 | 17:26 |
SpamapS | jeblair: btw, in https://review.openstack.org/399415 .. .My trouble with variants may have been PBKAC.. at first, I had it like this: http://paste.openstack.org/show/589742/ | 17:31 |
SpamapS | jeblair: and what I ended up with was a variant that had no irrelevant-files section and ran every time | 17:32 |
SpamapS | Seems like anyone might make that mistake.. _OR_ it's a bug and I shouldn't have had that variant. I'm just not sure. | 17:32 |
jeblair | SpamapS: that's a bug :) | 17:33 |
*** Guest52285 is now known as mgagne | 17:33 | |
*** mgagne has quit IRC | 17:33 | |
*** mgagne has joined #zuul | 17:33 | |
jeblair | we shouldn't override things unless they are explicitly set | 17:33 |
SpamapS | jeblair: k, I'll write it up as a test | 17:33 |
jeblair | SpamapS: this should probably be a unit test in test_model | 17:38 |
jeblair | (they are lighter weight and don't run the scheduler) | 17:39 |
clarkb | jeblair: not sure if you noticed but looks like the table prefix for tests change is still unhappy | 17:39 |
SpamapS | jeblair: good idea.. should be easier to iterate on that too. | 17:39 |
jeblair | clarkb: blah | 17:39 |
jeblair | clarkb: that one is going to be tricky to solve -- it's basically because we're creating a second database object (because we have the main nodepool daemon and the command process running in the same python interpreter) | 17:43 |
jeblair | (so we're setting up the mappers twice) | 17:44 |
clarkb | maybe we should explore the common lock file appraoch? That should be straightforward /tmp/nodepooldbdroplockfile (or put it in the db itself)? | 17:44 |
*** rcarrillocruz has quit IRC | 17:47 | |
mordred | ok. I just found a thing that wins the prize for being the most absurd programming advice EVER: https://dev.to/dawranliou/never-write-for-loops-again | 17:50 |
jeblair | mordred: never write with vowels? only write with punctuation? (hi perl!) | 17:53 |
clarkb | mordred: if that was rewritten as "use tools that solve common iteration problems well" I would be on board | 17:53 |
clarkb | jeblair: I enjoyed the use of set differences in that one change btw :) | 17:54 |
jeblair | yeah, we're a stone's throw away from 'use a functional programming language' | 17:54 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Re-enable TestWebApp tests https://review.openstack.org/399716 | 17:54 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Use an external lockfile when dropping the test database https://review.openstack.org/399717 | 17:56 |
jeblair | clarkb: ^ | 17:56 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Re-enable TestWebApp tests https://review.openstack.org/399716 | 17:56 |
clarkb | jeblair: thanks | 17:57 |
jeblair | SpamapS: yeah, i think looking in Layout._createJobTree, it looks like we're assuming that we're going to run at least one variant, so we don't really have the idea that a more specific variant might end up telling us *not* to run the job. basically, this algorithm works for branch selection, but not deciding whether to run the job in the first place. | 18:01 |
jeblair | (the algorithm is roughly "find the first variant that matches, use that, but also mutate it with any subsequent variants which also match". so in this case, the variant without irrelevant-files matches after the one with irrelevant files failed to match) | 18:03 |
clarkb | jeblair: we might want to make it support TMPDIR, but +2 as is (I think we can make it support TMPDIR if that ever beomces a problem) | 18:04 |
jeblair | clarkb: let's put that on the roadmap for nodepool v12. ;) | 18:05 |
SpamapS | jeblair: yeah, might need to think about order of negative vs. positive filtering | 18:12 |
jeblair | SpamapS: yeah. also i'd like to amend my earlier comment -- i think this fails for branch selection in the same way (if you had a job that globally said only run on stable/foo, and thet added that to a project, it would probably run on all changes). | 18:13 |
jeblair | (which is nice! it means the problem is generalized!) | 18:13 |
SpamapS | yeah maybe not as corner case as I originally thought | 18:14 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Remove waitForBuiltImages() from nodepool.py https://review.openstack.org/399727 | 18:25 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Remove waitForBuiltImages() / JobTracker() from nodepool.py https://review.openstack.org/399727 | 18:29 |
pabelanger | Shrews: don't want to step on toes, but should I start helping in test_commands to enable tests? | 18:30 |
Shrews | pabelanger: working on image-delete now. feel free to tackle any others | 18:31 |
pabelanger | ack | 18:31 |
pabelanger | let me get a drink first | 18:31 |
Shrews | i think things have settled down enough to hit em again | 18:31 |
jeblair | SpamapS: this is curious -- in ProjectTemplateParser._parseJobTree if you instantiate a job on a project-pipeline and do not give it any arguments, it adds the first instance of the job directly to the tree. so in your exmaple, it should have added the main job definition, and *not* created a new variant without the matcher. | 18:32 |
Shrews | pabelanger: jeblair: image-delete is going to require provider, image name, and upload ID (only takes an id argument now). I plan to just have 3 required args for those | 18:33 |
Shrews | image-list already provides those values | 18:34 |
jeblair | SpamapS: so i don't actually know why that failed. even if we find and fix that, i think there might still be a problem with unwanted broadening of jobs in this manner (if you supply, say, an extra variable to a job in a project-pipeline jobtree, you would end up getting a blank variant instead of the main job. we might be able to solve that by inheriting from the main job in that case) | 18:35 |
jeblair | (where the 'main job' is 'the variant of the job that appeared first in the config') | 18:36 |
jeblair | Shrews: ++ | 18:36 |
SpamapS | jeblair: I know it's sort of fundamental to your vision of jobs, but for the record, complexity is why I shy from inheritance. I rather like composition and macros. It's the old crufty C programmer in me. ;-) | 18:36 |
SpamapS | That said | 18:37 |
SpamapS | this should not be complex | 18:37 |
SpamapS | Just need to feed it a good matrix of test scenarios. | 18:37 |
clarkb | fwiw composition and macros worked poorly for us in the jjb world | 18:37 |
jeblair | SpamapS: yes, this is designed to be very simple for users. it may require some brain bending on our parts to implement it, but we need to take the hit because of what clarkb said. :) | 18:38 |
jeblair | hopefully, the hit won't be that hard once we have, as you say, good test scenarios | 18:38 |
jeblair | if anyone hasn't seen it lately, this is what we're working to avoid: http://git.openstack.org/cgit/openstack-infra/project-config/tree/zuul/layout.yaml#n1246 | 18:40 |
jeblair | basically lines 1181 through 3560 of that file are completely incomprehensible by a human, and, at this point, likely never to be removed from the file even when no longer needed. | 18:41 |
jeblair | that's 2400 lines of ad-hoc regular expression based rules applied to jobs, most of which are on average about 7000 lines away from where they're used. | 18:42 |
SpamapS | yes, we are aligned on all points. now if i can just get more than 2 minutes at the keyboard ... #ParentLife | 18:43 |
jeblair | pabelanger, clarkb: so where are we on max-servers=-1? i think i argued it wasn't necessary because you can pull the image sections from the provider, and pabelanger argued that it could be useful if you want to temporarily disable a provider with a simple one-liner change. i think i buy that -- that both methods are useful (pabelangers for when you want to suspend service, mine for when you want to end service). | 18:47 |
jeblair | pabelanger, clarkb: do we want to support both things? | 18:47 |
jeblair | (part of me wants only one way to do this for simplicity, but i recognize we end up flipping these bits in production a lot) | 18:48 |
pabelanger | jeblair: clarkb: I don't mind trying it the new way, and skip max-server: -1 for now. Maybe revisit it in the future if images: [] is too much code churn. | 18:48 |
clarkb | git and git revert make it easy to do that | 18:52 |
clarkb | yes its more lines changed but logically its the same sort of change | 18:52 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Enable test_delete / test_delete_now tests https://review.openstack.org/399734 | 18:53 |
jeblair | okay, so no more -1s for now then | 18:54 |
pabelanger | ++ | 18:54 |
pabelanger | Shrews: jeblair: is it too early to talk about timeline for rolling nodepool zookeeper into production? Do you have something in mind? | 18:56 |
clarkb | I was thinking about ^ a bit and since uploads haev been so unreliable recently I think we may want some way of prepopulating the zk db with current data when we do that | 18:57 |
clarkb | and that way we don't spend 2 weeks trying to get our images rebuilt and uploaded | 18:57 |
jeblair | pabelanger: i think it can happen real soon now :) | 18:57 |
jeblair | clarkb: we could do that, or we could run a zk builder in parallel (maybe finally split it into its own machine like we've been threatening to) then make the prod switch when we have image quorum? | 18:59 |
pabelanger | mordred: do you want to review https://review.openstack.org/#/c/399717 and +3? Adds lockfile to the database drop issue | 18:59 |
pabelanger | Ya, I think the idea of the split | 19:00 |
pabelanger | I've started on puppet manifests for that | 19:00 |
pabelanger | and need to revist it | 19:00 |
pabelanger | revisit* | 19:00 |
jeblair | clarkb: (the db->zk migration idea isn't too hard; it would require not only the db migration, but also moving the diskimage files on disk since they are named differently) | 19:00 |
clarkb | jeblair: ya tahts another option but I think it will take ~2 weeks to reach that | 19:01 |
clarkb | whcih may not be the worst thing considering the long holiday coming up | 19:01 |
jeblair | clarkb: well, the new builder is far more agressive about uploading images | 19:01 |
pabelanger | that would actually, be really cool. first run zk nodepool-builder, before we upgrade nodepool.o.o, it should just upload our images across all providers | 19:01 |
clarkb | jeblair: ya, but the clouds are much more aggressive at failing recently too... :( | 19:01 |
jeblair | it totally rickrolls the old one | 19:01 |
clarkb | apparently rax just times out now | 19:02 |
jeblair | okay, maybe migration is the way to go then so we don't hate ourselves | 19:02 |
clarkb | but yes that would be a realtively easy way to transition, let it run on the side for a bit, once it is ready update nodepoold (well the 3 daemons) | 19:02 |
jeblair | clarkb: two daemons! | 19:03 |
jeblair | clarkb: (we get to drop the 'images' daemon) | 19:03 |
jeblair | (or, i guess we could keep it just for the webapp) | 19:03 |
clarkb | or attach webapp to one of the other two | 19:03 |
clarkb | but ya | 19:03 |
pabelanger | https://review.openstack.org/#/c/356484/ can be approved, for puppet-nodepool. moves ::diskimage_builder to builder.pp | 19:08 |
pabelanger | going to rebase the other changes | 19:08 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Re-enable test: test_image_delete_invalid https://review.openstack.org/399739 | 19:09 |
Shrews | i think it makes sense to run parallel since we don't really have experience with ZK in production. if we find problems, we don't break the world while we figure it out | 19:12 |
jeblair | i find that ^ fairly convincing. clarkb? | 19:16 |
clarkb | ya I think tahts a good reason to do it that way | 19:16 |
clarkb | and if we find the clouds are being annoying maybe thats a good time to dig into that more | 19:16 |
jeblair | yeah, we'll have some space to poke at that too | 19:17 |
Shrews | ugh, my 739 change is dependent on two different changes | 19:21 |
Shrews | can anyone +3 https://review.openstack.org/399683 ? | 19:23 |
jeblair | i'm going to approve 717 first | 19:23 |
jeblair | since the db drop just tanked another series of changes | 19:24 |
Shrews | jeblair: yeah. one of those has my 2nd dependency | 19:25 |
* Shrews takes a walk. bbiab | 19:25 | |
jeblair | Shrews: i approved 683 behind it; when they merge i'll recheck the others | 19:25 |
jeblair | Shrews: fun fact though, in the future -- while it's usually easy enough to just restack changes to have the right dependency graph, you can actually use Depends-On within the same repo if, for instance, you don't want to restack either of the two forks of the tree your change needs. | 19:27 |
jeblair | (i thought about doing that yesterday, but one of the series i needed was all -2 anyway so i just restacked) | 19:28 |
openstackgerrit | Merged openstack-infra/nodepool: Use an external lockfile when dropping the test database https://review.openstack.org/399717 | 19:29 |
openstackgerrit | Merged openstack-infra/nodepool: Use image name in ImageUpload equality https://review.openstack.org/399683 | 19:31 |
*** abregman|afk has quit IRC | 19:39 | |
Shrews | jeblair: oh, hah. didn't even consider that. | 19:53 |
Shrews | btw, smoke from forest fires travels a loooong way. almost difficult to be outside | 19:53 |
mordred | Shrews: you have forest fires in NC in November? | 19:57 |
Shrews | mordred: oh, there are several burning right now near Asheville | 19:58 |
Shrews | mordred: http://wildfiretoday.com/2016/11/17/wildfire-smoke-forecast-for-november-17-2016/ | 20:00 |
openstackgerrit | Merged openstack-infra/nodepool: Remove builds that are no longer in use https://review.openstack.org/399307 | 20:00 |
mordred | Shrews: wow | 20:00 |
Shrews | that area is at the worst of the drought scale | 20:01 |
Shrews | http://www.ncdrought.org/ | 20:01 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Remove waitForBuiltImages() / JobTracker() from nodepool.py https://review.openstack.org/399727 | 20:03 |
Shrews | pabelanger: btw, i'm changing test_image_delete_snapshot to test_image_delete, just waiting for some merges first | 20:03 |
pabelanger | Shrews: same, working on hold tests | 20:04 |
pabelanger | they are an easy enable | 20:04 |
* Shrews installs his new Red Hat branded laptop camera covers | 20:08 | |
openstackgerrit | Merged openstack-infra/nodepool: Use constants for zk states https://review.openstack.org/399309 | 20:10 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Re-enable test: test_image_delete_invalid https://review.openstack.org/399739 | 20:14 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Add test_image_delete test https://review.openstack.org/399757 | 20:15 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Re-enable test_hold test https://review.openstack.org/399758 | 20:15 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Add test_image_delete test https://review.openstack.org/399757 | 20:18 |
*** bhavik has joined #zuul | 20:21 | |
Shrews | So, image-update... is that the same as requesting a new DIB image build and re-upload? Or just to re-upload using the exiting image? | 20:21 |
Shrews | pabelanger: clarkb: ^^^ | 20:21 |
Shrews | s/exiting/existing/ | 20:22 |
pabelanger | rebuild the image and upload to provider | 20:22 |
pabelanger | http://docs.openstack.org/infra/nodepool/operation.html#command-line-tools | 20:22 |
pabelanger | I don't use it often | 20:22 |
pabelanger | I usually do, dib-image-build, image-upload as 2 steps | 20:22 |
Shrews | oh, the -h output did not include such detail | 20:22 |
clarkb | ya workflow tends to be ^ because you only need one build then upload to 20 regions | 20:23 |
pabelanger | but, think we should keep it | 20:23 |
Shrews | hrm, we are already covered by 'image-build' then, since the upload happens automatically. yeah? | 20:24 |
pabelanger | in fact, I'd likely use it more if image-update all ubuntu-trusty, didn't try to upload an image to tripleo-test-cloud-rh1, since it doesn't have the image today | 20:24 |
Shrews | well, current builder wouldn't try to upload to that if the image isn't in the provider config | 20:25 |
Shrews | i think we can remove the image-upload option then, if I understand correctly | 20:26 |
pabelanger | Right, I don't think an issue with zuulv3 | 20:26 |
Shrews | nodepool image-build <-- builds and uploads at the next scheduler interval | 20:27 |
*** bhavik has quit IRC | 20:27 | |
Shrews | well, builds immediately, then the upload will happen at the next interval (which is currently very quick) | 20:27 |
pabelanger | Ya, a little different workflow from today, since we do that manually (both steps) | 20:28 |
pabelanger | since we have 24 hour loop | 20:28 |
pabelanger | Shrews: what about image-upload, how are we using that? | 20:29 |
pabelanger | err | 20:30 |
pabelanger | 1 sec | 20:30 |
SpamapS | jeblair: I feel like the trouble is that when a job inherits from another one, we need to _not_ consider the parent as a potential match anymore. | 20:30 |
Shrews | pabelanger: image-upload is not needed now | 20:30 |
pabelanger | SpamapS: Ya, image-upload. | 20:30 |
pabelanger | SpamapS: sorry | 20:30 |
SpamapS | you got S'd | 20:30 |
* SpamapS fistpumps | 20:30 | |
SpamapS | take that | 20:30 |
SpamapS | ;) | 20:30 |
pabelanger | Shrews: yes, that | 20:30 |
pabelanger | so, image-upload / image-build would go away | 20:30 |
pabelanger | at the cost of not being able to manually trigger them | 20:31 |
Shrews | pabelanger: no, image-build stays, but it replaces the image-build/image-update combo | 20:31 |
pabelanger | ah | 20:31 |
pabelanger | what about dib-image-build? | 20:31 |
pabelanger | err | 20:32 |
pabelanger | brain is mush | 20:32 |
pabelanger | okay, I understand now | 20:32 |
Shrews | i don't see that one | 20:32 |
Shrews | hah | 20:32 |
pabelanger | Shrews: I'm happy to try things with out them in the new world order | 20:34 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Remove image-update based tests https://review.openstack.org/399764 | 20:37 |
Shrews | so, i guess image-upload is also unnecessary now | 20:39 |
Shrews | that would be the last of the gearman based commands | 20:40 |
pabelanger | right | 20:40 |
Shrews | w00t | 20:40 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Remove image-update based tests https://review.openstack.org/399764 | 20:41 |
pabelanger | Shrews: looking at 399757, that's a lot of args :) would it be possible to update that to use the image name? nodepool image-delete template-fake-image-1479501568 ? | 20:42 |
pabelanger | I guess that would be the Provider Image Name | 20:42 |
Shrews | pabelanger: nope | 20:42 |
pabelanger | l | 20:43 |
pabelanger | k | 20:43 |
Shrews | pabelanger: well, if we want to pull all uploads, then compare image.external_name | 20:43 |
Shrews | we'd still need build-id | 20:43 |
Shrews | and upload-id | 20:43 |
pabelanger | okay, will have to try it and see how it works | 20:44 |
jeblair | SpamapS: note that we have actual inheritance (where jobs with different names are related to each other -- job: name: foo; parent: bar) and also the things i've been calling variants (where one job out of several with the same name is chosen to run). it's potentially confusing because behind the scenes they both use the 'inheritFrom' method to copy attributes around. | 20:44 |
Shrews | though, i guess in reality, the same image for a build wouldn't be successfully upload more than once | 20:44 |
jeblair | pabelanger, Shrews, clarkb: the little voice in my head saying maybe we should make a unique id for uploads is getting louder. | 20:45 |
Shrews | jeblair: yeah | 20:46 |
clarkb | ++ | 20:46 |
* Shrews is starting to hear it too | 20:46 | |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Remove image-upload command and tests https://review.openstack.org/399766 | 20:46 |
* clarkb is hoping that latest commit to ansible PR will actually apss tests and we can get this silly bug fixed | 20:46 | |
Shrews | jeblair: i suggest allowing the current changes through, then considering how that would look afterwards | 20:48 |
jeblair | Shrews: agreed | 20:48 |
jeblair | brainstorming on that: a) redesign the zk storage hierarchy so it isn't deep (all uploads go into one container); b) have a global counter for upload ids (like an autoincrement table in postgres); c) generate uuids (they're longer but copy/pastable) | 20:48 |
jeblair | [that's in increasing order of preference for me] | 20:49 |
Shrews | (a) would touch quite a bit of code | 20:50 |
jeblair | yeah, and aside from this, i think it's a good schema design | 20:50 |
Shrews | (b) we could repurpose sequence znodes | 20:50 |
pabelanger | looking into the nodepool.tests.test_builder.TestNodePoolBuilder.test_image_removal failures | 20:51 |
Shrews | (c) we could keep the status quo, but add uuids to the znode data. but now some APIs would have to do more ZK queries | 20:52 |
Shrews | pabelanger: eh? where? | 20:52 |
pabelanger | http://logs.openstack.org/58/399758/1/check/nodepool-coverage-db-ubuntu-xenial/b8776e3/console.html | 20:52 |
clarkb | I like uuids because they are easy to reason about | 20:52 |
pabelanger | Shrews: it is the race you warned about | 20:53 |
clarkb | I think a) and b) would require extra brain processing around changing that code that c) wouldn't | 20:53 |
Shrews | pabelanger: ah, yeah. perhaps we need a loop around getBuild() | 20:55 |
pabelanger | Shrews: agreed, doing that now | 20:55 |
jeblair | Shrews: yeah, with (c) the lack of indexing would hurt, but if we only use it for the CLIs it probably wouldn't be that bad. | 20:58 |
Shrews | jeblair: oh, well that's a good point | 20:58 |
clarkb | woot tests pass now on ansible sync retry fix | 21:00 |
clarkb | now I can get lunch | 21:00 |
openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Better protect for race condition in waitForBuildDeletion() https://review.openstack.org/399770 | 21:03 |
pabelanger | Shrews: ^more protection | 21:03 |
Shrews | hrm, not quite sure why 399757 failed | 21:07 |
Shrews | looks like one of the threads abnormally aborted. if that's the cleanup thread, i could see the timeout | 21:09 |
Shrews | pabelanger: is that call to wait_for_threads() in that loop necessary there? looking at the code for that method, i'm not entirely certain we need that for testing the builder | 21:12 |
Shrews | like, don't need it in any of the tests | 21:12 |
pabelanger | let me check | 21:12 |
jeblair | Shrews: it should be harmless | 21:12 |
Shrews | ok. i'm not 100% certain exactly what that's doing anyway | 21:14 |
jeblair | Shrews: (it doesn't look like it's at issue in that test anyway, since it didn't time out inside it) | 21:14 |
Shrews | all of my changes have failed with that delete timeout, though the test passes locally. weird | 21:16 |
jeblair | Shrews: it makes sure that any threads that are running to create nodes, etc, have completed (since a thread running one of those will change the state). essentially it helps make sure the system is stable before we inspect it. it's generally not at issue for many builder tests, but any that incidentally wait for a node to be built would need it. easier to just have it. | 21:17 |
jeblair | Shrews: was that image uploaded twice? | 21:18 |
jeblair | with the same number? | 21:18 |
Shrews | jeblair: i don't see how | 21:20 |
jeblair | Shrews: i don't either, but i ask because i see two lines like 2016-11-18 20:27:56.026391 | INFO [nodepool.builder.UploadWorker] Image build 0000000001 in fake-provider is ready | 21:21 |
Shrews | well that is interesting | 21:21 |
Shrews | ! | 21:21 |
jeblair | (and if there were two uploads, it would cause the failure). first thing to eliminate is whether that's just log duplication. | 21:22 |
Shrews | this smells like a race | 21:22 |
Shrews | two threads uploading the same image | 21:22 |
jeblair | Shrews: we may need to turn zk logging back on and throw this at zuul a few times | 21:23 |
jeblair | Shrews: do you want to do that? i want to make some logging changes so we can discern the different upload threads. | 21:24 |
Shrews | jeblair: hold on a sec... | 21:24 |
Shrews | jeblair: i think we need a check inside the upload lock code to make sure some other thread hasn't already done the work | 21:25 |
Shrews | jeblair: b/c we check if it has been uploaded, if not, upload it (w/o doing the check in the critical section again) | 21:26 |
* Shrews codes the fix | 21:28 | |
jeblair | Shrews: oh! they're not doing it at the same time, they're doing it sequentially :) | 21:29 |
*** hasharAway is now known as hashar | 21:29 | |
Shrews | right | 21:29 |
jeblair | makes sense | 21:29 |
jeblair | Shrews: hrm, well why would they get the same upload id then? | 21:30 |
Shrews | jeblair: the build id is the same (that's what is in the log) | 21:31 |
hashar | hello :) | 21:32 |
hashar | Found a nice abuse of Depends-On on our setup a patch with 58 depends-on meta headers! | 21:34 |
jeblair | Shrews: doh. | 21:34 |
jeblair | hashar: do you mean someone was being mischevious, or they had a really complex thing? | 21:35 |
hashar | complex thing | 21:35 |
jeblair | cool. did it work? :) | 21:35 |
hashar | that is a breaking change in MediaWiki itself, it has been made to depend on all the ptch that updates the plugins | 21:35 |
hashar | think about Wordpress suddenly moving to python and depending on every single wordpress plugins that do the switch :D | 21:35 |
hashar | Zuul is still processing | 21:36 |
hashar | I am confident it will eventually reach the end! | 21:36 |
jeblair | yeah, that'll require some churn... how many mergers do you have? | 21:36 |
hashar | 1 :D | 21:36 |
hashar | it is merely doing Running query change:Ixxxxx | 21:36 |
jeblair | ah yeah. | 21:37 |
hashar | and yeah I havent thought about all the merge:merge jobs that are going to be send | 21:37 |
SpamapS | jeblair: ah yes that did confuse me | 21:37 |
SpamapS | but that may be because I had only 2 hours of sleep last night | 21:37 |
hashar | (I dont think too many depends-on need a fix. I reported it because I find it a creative use of the feature) | 21:38 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Add secondary upload check https://review.openstack.org/399774 | 21:38 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Log each worker thread separately https://review.openstack.org/399775 | 21:38 |
Shrews | jeblair: ^^^ | 21:38 |
jeblair | SpamapS: that is less than the surgeon general recommends before starting work on zuul | 21:38 |
jeblair | Shrews: ^^^ :) | 21:39 |
* SpamapS has always been a rebel | 21:39 | |
jeblair | Shrews: that's quite the race condition too, since that's a non-blocking lock. | 21:40 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Add test_image_delete test https://review.openstack.org/399757 | 21:42 |
Shrews | gah | 21:42 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Add test_image_delete test https://review.openstack.org/399757 | 21:44 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Remove image-update based tests https://review.openstack.org/399764 | 21:44 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Re-enable test: test_image_delete_invalid https://review.openstack.org/399739 | 21:44 |
openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Remove image-upload command and tests https://review.openstack.org/399766 | 21:44 |
Shrews | rebased those on the race fix | 21:45 |
jeblair | pabelanger, Shrews, clarkb: https://review.openstack.org/399770 failed with the db drop. it was running the lockfile code. :( | 21:45 |
jeblair | pabelanger, Shrews, clarkb: maybe we should lockfile the schema *creation* as well? | 21:45 |
Shrews | *sigh* | 21:46 |
Shrews | i hate databases | 21:46 |
openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Also use a lockfile when creating the schema https://review.openstack.org/399777 | 21:47 |
jeblair | almost gone | 21:47 |
*** rcarrillocruz has joined #zuul | 22:04 | |
pabelanger | jeblair: worth a try | 22:04 |
Shrews | pabelanger: that failure on 770 is all cray cray | 22:09 |
Shrews | Ok, I am ahead of schedule and already 2 beers into my weekend. I should neither review nor code anymore. C U all Monday. | 22:11 |
pabelanger | Shrews: ack, enjoy | 22:18 |
openstackgerrit | Merged openstack-infra/nodepool: Add secondary upload check https://review.openstack.org/399774 | 22:30 |
*** hashar has quit IRC | 22:31 | |
*** lindycoder has quit IRC | 22:31 | |
openstackgerrit | Merged openstack-infra/nodepool: Also use a lockfile when creating the schema https://review.openstack.org/399777 | 22:38 |
jeblair | 23:29 < openstackgerrit> James E. Blair proposed openstack-infra/infra-specs: Zuul v3: Add section on secrets https://review.openstack.org/386281 | 23:29 |
*** rcarrillocruz has quit IRC | 23:46 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!