| mordred | ++ | 00:02 |
|---|---|---|
| openstackgerrit | Merged openstack-infra/nodepool: Separate image upload logs into separate logger https://review.openstack.org/394285 | 00:15 |
| jeblair | jhesketh: i approved pabelanger's test changes that you +2d; feel free to -1 or +3 the rest as you review them. :) | 00:25 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_repo_deleted test https://review.openstack.org/396703 | 00:25 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_check_smtp_pool test https://review.openstack.org/396707 | 00:26 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_disable_at test https://review.openstack.org/396785 | 00:26 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_live_reconfiguration test https://review.openstack.org/393488 | 00:26 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_crd_check_reconfiguration test https://review.openstack.org/396788 | 00:26 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_crd_check_unknown test https://review.openstack.org/396798 | 00:27 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_crd_gate_unknown / test_crd_undefined_project https://review.openstack.org/396799 | 00:27 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_head_is_dequeued_once test https://review.openstack.org/396803 | 00:27 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_merger_repack_large_change test https://review.openstack.org/396805 | 00:27 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_noop_job test https://review.openstack.org/396807 | 00:27 |
| jhesketh | jeblair: sure, will do | 00:31 |
| *** jamielennox is now known as jamielennox|away | 03:07 | |
| *** jamielennox|away is now known as jamielennox | 03:21 | |
| openstackgerrit | Ian Wienand proposed openstack-infra/nodepool: Separate image upload logs into separate logger https://review.openstack.org/397516 | 03:50 |
| *** jamielennox is now known as jamielennox|away | 04:15 | |
| *** jamielennox|away is now known as jamielennox | 04:17 | |
| *** bcoca has quit IRC | 04:23 | |
| *** jamielennox is now known as jamielennox|away | 04:40 | |
| *** jamielennox|away is now known as jamielennox | 05:03 | |
| *** abregman has joined #zuul | 06:15 | |
| tobiash | jeblair, greghaynes, clarkb: thanks for your feedback | 06:53 |
| tobiash | I will try out the docker in chroot method (docker load on every slave boot would be too io consuming) | 06:54 |
| tobiash | for the windows use case I will look into wgetting the image during dib build | 06:54 |
| *** hashar has joined #zuul | 07:17 | |
| *** abregman has quit IRC | 08:17 | |
| *** abregman has joined #zuul | 08:53 | |
| *** abregman has quit IRC | 09:04 | |
| *** abregman has joined #zuul | 09:12 | |
| *** openstackgerrit has quit IRC | 09:18 | |
| *** openstackgerrit has joined #zuul | 09:19 | |
| * Zara reads zuul meeting logs-- fungi: using storyboard for agendas one day, 1) worklist-users currently need to be named explicitly (though support for teams in worklists is going ahead steadily), so they're less easily editable right now. 2) there's API support for worklist events but we don't yet expose anything in the UI, so while you can find the history, it's not easy to find it yet. so those are the s | 10:00 | |
| * Zara pecific changes we should make. | 10:00 | |
| Zara | (and looks like the meeting is too late for me, but I can read logs for now!) | 10:03 |
| openstackgerrit | Jan Hruban proposed openstack-infra/zuul: Do not share the gerrit queue dbs in tests https://review.openstack.org/397677 | 11:02 |
| openstackgerrit | Jan Hruban proposed openstack-infra/zuul: Query changes only via relevant sources https://review.openstack.org/397678 | 11:02 |
| openstackgerrit | Jan Hruban proposed openstack-infra/zuul: Fix the file matcher for github https://review.openstack.org/397679 | 11:02 |
| *** abregman is now known as abregman|mtg | 13:06 | |
| *** abregman|mtg is now known as abregman | 13:23 | |
| pabelanger | morning | 13:38 |
| *** yolanda has quit IRC | 13:39 | |
| *** bcoca has joined #zuul | 13:48 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Initial support for bindep https://review.openstack.org/397754 | 13:50 |
| pabelanger | Shrews: ^ bindep will help us get zookeeper onto our worker nodes. Going to work on landing that today | 13:51 |
| pabelanger | clarkb: ^ I'm on the fence about adding diskimage-builder dependencies into nodepools bindep.txt file, maybe we also land it for diskimage-builder, then update our job to also iterate over its bindep.txt file? | 13:52 |
| *** bcoca has left #zuul | 13:52 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Initial support for bindep https://review.openstack.org/339200 | 13:53 |
| pabelanger | clarkb: Shrews: ^ better review, recycles original change-id | 13:53 |
| *** yolanda has joined #zuul | 13:55 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Initial support for bindep https://review.openstack.org/339200 | 14:04 |
| pabelanger | okay, I stand corrected. zookeeperd is already in bindep-fallback.txt :) | 14:04 |
| pabelanger | so, this is just an optimization | 14:05 |
| *** yolanda has quit IRC | 14:07 | |
| *** yolanda has joined #zuul | 14:23 | |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul: Move code out of zuul/manager/__init__.py https://review.openstack.org/349037 | 14:27 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul: Remove v3 project template test https://review.openstack.org/395722 | 14:30 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul: Remove v3 project template test https://review.openstack.org/395722 | 14:34 |
| openstackgerrit | Paul Belanger proposed openstack-infra/zuul: Move code out of zuul/manager/__init__.py https://review.openstack.org/349037 | 14:34 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Do not delete DIB images that are building. https://review.openstack.org/397766 | 14:36 |
| Shrews | jeblair: pabelanger: ^^^ that fixes a pretty big issue where we were marking in progress builds as 'deleted' before they could finish building | 14:37 |
| mordred | Shrews: they should build quicker | 14:37 |
| Shrews | fo shizzle | 14:38 |
| pabelanger | Shrews: we have some good test coverage around that today, curious why our testing didn't catch that. Possible they are currently disabled? | 14:41 |
| Shrews | pabelanger: i'm pretty darn certain we don't currently have any tests that check ZK data :) i saw it change in zookeeper while testing jeblair's changes | 14:42 |
| pabelanger | Ya, looks like it | 14:42 |
| Shrews | we should, ya know, enable tests and stuff | 14:44 |
| pabelanger | Ya | 14:44 |
| pabelanger | trying to get unit tests working on fedora | 14:44 |
| pabelanger | but, that is going to be a lot of churn | 14:44 |
| pabelanger | zookeeper things :) | 14:44 |
| Shrews | pabelanger: i'm going to start with test_commands.py | 14:45 |
| pabelanger | Shrews: ack | 14:45 |
| *** hashar is now known as hasharAway | 14:50 | |
| *** abregman is now known as abregman|brb | 15:13 | |
| Shrews | pabelanger: jeblair: so, the current dib-image-delete command takes just an 'id' param. but the ids across images are no longer unique, so we need to require the image name, too. should we just add a new param for the image name? or re-purpose the id field to be combined image name and id (e.g., trusty-0000001)? | 15:18 |
| Shrews | and ids are no longer int values | 15:18 |
| pabelanger | I'm leaning towards single args | 15:22 |
| pabelanger | especially, if we render trusty-0000001 is any output | 15:22 |
| Shrews | well, dib-image-list renders somewhat misleading info now: http://paste.openstack.org/show/589298/ | 15:23 |
| Shrews | because ID is not really unique | 15:24 |
| Shrews | perhaps we should change that to combine Image and ID columns | 15:24 |
| pabelanger | ya | 15:24 |
| mordred | Shrews: it's devstack-trusty 0000000001 is the unique key right? | 15:24 |
| Shrews | mordred: right | 15:24 |
| pabelanger | I don't mind combining them | 15:25 |
| Shrews | i feel like that's the best option... but you all have the ops experience | 15:26 |
| Shrews | so i defer to you | 15:26 |
| pabelanger | Personally, it is easier to write bash script, if I only need to cut 1 item, them 2 :) | 15:27 |
| Shrews | lol, ++ | 15:27 |
| Shrews | mordred: combining them sound reasonable to you too? | 15:28 |
| Shrews | nodepool dib-image-delete -id devstack-trusty-0000000001 | 15:29 |
| Shrews | for example | 15:29 |
| mordred | Shrews: I think that's fine | 15:30 |
| Shrews | wow, my irc terminal window just _totally_ disappeared | 15:31 |
| Shrews | fedora, you so cray cray | 15:31 |
| fungi | Zara: thanks for spotting and responding to my half-baked musing on non-wiki agenda options | 15:44 |
| Zara | heh | 15:45 |
| *** openstackgerrit has quit IRC | 15:48 | |
| *** openstackgerrit has joined #zuul | 15:49 | |
| clarkb | I'm not sure combining them with an arbitrary separator that you have to parse out makes sense (seems potentially error prone) | 15:53 |
| clarkb | why not just nodepool image-delete devstack-trusty 000000001 ? | 15:54 |
| Shrews | clarkb: it's not arbitrary, it's what is used in the ondisk name | 15:54 |
| clarkb | Shrews: ah ok if its already used somewhere then not as bad | 15:55 |
| openstackgerrit | Merged openstack-infra/zuul: Add attempts logic for jobs https://review.openstack.org/395056 | 16:00 |
| timrc | Hey sad I missed the meeting, I was tied up in another meeting, but.. I'd just like to put this out there: I think it'd be cool if, as work completes, and someone thinks its sufficiently cool, to maybe do a demo? I know that's a departure from how these meetings traditionally look. Demoing zuul things will also help us towards the goal of making it simple to deploy :P | 16:08 |
| fungi | for people who actually enjoy our q&a site... https://ask.openstack.org/question/98988 | 16:10 |
| fungi | if i leave an answer there i'll more likely just direct them to irc or an ml | 16:11 |
| pabelanger | oh neat | 16:12 |
| mordred | that's amazing. | 16:15 |
| mordred | timrc: I don't think that's a terrible idea at all - although depending on the thing in question a demo may be tricky - or may be a thing we want to schedule for a later time if it would take a while to get going | 16:16 |
| timrc | fungi: I'm sort of against doing that, personally. It's nice to have those responses cataloged on the q&a site and searchable from Google. | 16:17 |
| pabelanger | Shrews: okay, starting in test_nodepool.py. Have my ubuntu box setup | 16:17 |
| timrc | fungi: Really don't like consigning that sort of thing to irc logs (even if they are indexed by Google) it might be hard to get all the context | 16:17 |
| Shrews | i'm not exactly sure how we'd demo some of these things, especially via irc. | 16:19 |
| timrc | mordred: Fair point. Maybe just signing up / scheduling demos during the meeting itself, then. | 16:19 |
| timrc | Shrews: Yeah, it may require G+ hangout or some other technology that is jeblair-approved :) | 16:19 |
| timrc | screen? | 16:19 |
| timrc | jk | 16:19 |
| mordred | well actually - we've done screencasts over telnet port thing before | 16:20 |
| mordred | so you can netcat a port and see the terminal session on the other side | 16:20 |
| mordred | did it for the AFS walkthrough | 16:20 |
| pabelanger | if we could have some sort of CLI trigger for zuul, in place of gerrit. That would take a large piece of out of play for demoing | 16:20 |
| timrc | mordred: That's amazing. | 16:21 |
| fungi | timrc: well, the traditional way to handle q&a is to add the questions and answers to an faq list in the project's docs | 16:21 |
| fungi | which provides them in a durable, visible place | 16:21 |
| timrc | fungi: That's cool. Yeah something is durable, has the question and answer together, and is indexable by your favorite search engine would be my criteria. But it makes me wonder why not just use the q&a site itself and just point to it from project docs? | 16:22 |
| fungi | mainly because it's a web forum? | 16:23 |
| fungi | i'm probably just biased against the quality of discussions that happen on web forums in general | 16:23 |
| timrc | And? Ah | 16:24 |
| timrc | All I know is that I've relied on googling and finding answers on q&a sites in the past. I've found it pretty useful to be able to do that | 16:25 |
| fungi | i tend to cringe at most of the answers i find to things on places like stackexchange and ubuntuforums | 16:26 |
| fungi | usually there's more noise (in the form of uninformed/incorrect answers) than signal | 16:26 |
| timrc | Yeah, I tend to read the accepted answer and the comments below it but... I get what you're saying. | 16:26 |
| *** abregman|brb is now known as abregman | 16:27 | |
| timrc | It is nice to have authorative answers mapped to specific questions and not have to dig or be uncertain about the information being provided. | 16:27 |
| fungi | the sheer number of actively dangerous "solutions" that i see proposed by well-meaning respondents who don't actually know what they're talking about is vast | 16:28 |
| timrc | Kind of makes it incumbent of you to provide answers to zuul questions on those forums since we're not going to change people's behavior :) | 16:28 |
| timrc | I kid! | 16:28 |
| fungi | and the real challenge is figuring out what "authoritative" means when it's not curated by authorities on the topic (or more to the point, how can you tell that those curating the answers are authoritative?) | 16:29 |
| clarkb | re demo, not sure that meeting is great forum for that but we have done similar demo type things in the past successfully just scheduling around subset of individual's free time. And ya the screen or pipe script to socket type solutions work pretty well | 16:29 |
| clarkb | (you can also record with script and playback at a later date :) ) | 16:29 |
| timrc | fungi: We just turn mordred into a dread pirate roberts of sorts and then anyone of us can answer? I don't know how the openstack forum works but other forums, including reddit and the ubuntu forum let you associate credentials with the user. | 16:30 |
| timrc | clarkb: ++ | 16:31 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Use image name in DIB image IDs https://review.openstack.org/397817 | 16:31 |
| Shrews | pabelanger: mordred: jeblair: ^^^ I left the ID and Image fields separate, but changed ID to be the combined form | 16:31 |
| fungi | timrc: askbot has a karma system that's sort of supposed to indicate how many friends have liked your post, or whatever the term is | 16:32 |
| Shrews | now it looks like this: http://paste.openstack.org/show/589305/ | 16:32 |
| Shrews | my thinking was that someone may be interested in image name only in the output, and that saves them from parsing it out | 16:34 |
| mordred | that makes sense to me | 16:35 |
| pabelanger | same | 16:36 |
| *** abregman has quit IRC | 16:40 | |
| jeblair | Shrews, clarkb, pabelanger: the combined id lgtm as well. having said that, if we wanted to make a shorter unique ids for images, i think there are a couple of options for doing that, but they involve a small to moderate amount of coding. i'm inclined to go with the solution Shrews has proposed. | 16:45 |
| jeblair | pabelanger, clarkb: can you take a look at https://review.openstack.org/396749 ? | 16:48 |
| clarkb | jeblair: ya I will add it to this morning's list. (I am currently recovering from monitor going crazy again, is apparently related to displayport bug and X being silly) | 16:52 |
| pabelanger | jeblair: +2 | 16:54 |
| openstackgerrit | Merged openstack-infra/nodepool: Do not delete DIB images that are building. https://review.openstack.org/397766 | 17:03 |
| pabelanger | jeblair: Shrews: I guess, nodepool-builder is no longer option and we'll need to update our BaseTestCase to start it | 17:17 |
| pabelanger | optional* | 17:17 |
| pabelanger | for now, I'm just manually launching it | 17:17 |
| jeblair | pabelanger: oh, i meant to reply to an earlier thing -- for running tests on fedora, i would recommend just getting a zk running locally, then do "NODEPOOL_ZK_HOST=localhost ..." to run tests | 17:22 |
| pabelanger | jeblair: Ah, cool. I'll give that a go | 17:22 |
| jeblair | because i would like to make the unit tests use that and then just go ahead and remove the per-test zk fixture | 17:23 |
| Shrews | pabelanger: i prefer to use a docker image for zookeeper: https://hub.docker.com/r/jplock/zookeeper/ | 17:34 |
| Shrews | works great for me. and easy to start/stop | 17:34 |
| Shrews | pabelanger: also, learn about zk-shell | 17:34 |
| pabelanger | jeblair: okay, that works as expected, thanks | 17:35 |
| pabelanger | Shrews: Ya, I haven't tried docker yet; some day soon. I'll look into zk-shell shortly | 17:35 |
| pabelanger | clarkb: Shrews: jeblair: thoughts about making zuul / nodepool directory structure the same? EG: nodepool/nodepool/tests vs zuul/tests | 17:37 |
| jeblair | pabelanger: the reason to put the tests in the package is if the tests are part of the package and expected to be consumable downstream. they are not in the case of zuul. maybe someday. :) | 17:38 |
| Shrews | jeblair: oh, this is interesting: http://logs.openstack.org/17/397817/1/check/nodepool-coverage-db-ubuntu-xenial/a57f23b/console.html#_2016-11-15_16_36_50_774972 | 17:39 |
| jeblair | i love kazoo logs | 17:40 |
| mordred | Shrews: it looks like the file doesn't exist | 17:41 |
| jeblair | http://logs.openstack.org/17/397817/1/check/nodepool-coverage-db-ubuntu-xenial/a57f23b/console.html#_2016-11-15_16_36_40_849444 | 17:42 |
| Shrews | mordred: DIAF. KTHXBAI | 17:42 |
| jeblair | that's where it is deleted | 17:42 |
| * Shrews hugs mordred | 17:42 | |
| jeblair | Shrews: is that with your 'don't delete building' change? | 17:43 |
| Shrews | jeblair: yeah, so now i'm all puzzled | 17:43 |
| mordred | I love that the next line after "Removed" is "is ready" | 17:43 |
| pabelanger | jeblair: Shrews: okay, first issue with localhost zookeeper, looks like nodepool-builder is persisting data across testtools.run: http://paste.openstack.org/show/589313/ | 17:43 |
| mordred | or is built, rather | 17:43 |
| pabelanger | guess I need to see how to make said path unique | 17:43 |
| mordred | I mean, I know it's the issue being investigated - but as 2 lines back to back in a log, it's lovely reading | 17:44 |
| jeblair | Shrews: i don't think it is | 17:44 |
| Shrews | pabelanger: test runs are in nodepool_tests | 17:44 |
| jeblair | Shrews: that change wasn't based on 397766 and 397766 merged after those tests started | 17:44 |
| pabelanger | Shrews: Ah, so nodepool-builder is not using it it seems | 17:44 |
| Shrews | jeblair: oh! you are correct, sir | 17:44 |
| Shrews | pabelanger: did you start nodepool by hand? the tests chroot to /nodepool_test... normally everything goes in /nodepool | 17:45 |
| pabelanger | Shrews: .tox/py27/bin/python -m testtools.run nodepool.tests.test_nodepool.TestNodepool.test_node is how I ran my test | 17:46 |
| pabelanger | however | 17:46 |
| pabelanger | I needed to add self._useBuilder() to start nodepool-builder | 17:46 |
| pabelanger | I suspect, we need to update that to use the zk fixture | 17:46 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Use image name in DIB image IDs https://review.openstack.org/397817 | 17:47 |
| Shrews | jeblair: i rebased it | 17:47 |
| Shrews | pabelanger: oh, if it's not using the fixture, then yeah | 17:50 |
| clarkb | jeblair: reviewed 396749, I +2'd because it mostly does what it says it does, but reading through the surrounding code I don't know that it does all the cleanup it could as a result. Also comments in https://review.openstack.org/#/c/396749/1/nodepool/builder.py may point out bugs in builder code? (not new bugs in this change so I didn't -1) | 17:57 |
| clarkb | Shrews: you may be interested in comments in builder.py there ^ | 17:58 |
| Shrews | clarkb: responded. your noticing of things is outpacing of doing the things | 18:10 |
| Shrews | :) | 18:10 |
| pabelanger | Shrews: jeblair are you okay with exposing zookeeper as an argument to builder.NodePoolBuilder()? Then we can pass in the fixture at test time | 18:10 |
| Shrews | s/of/our/ | 18:11 |
| Shrews | pabelanger: As opposed to getting it from the config file? | 18:13 |
| Shrews | pabelanger: Being able to re-read the config file to get ZK changes is one of the features jeblair explicitly wanted. It's a pretty nifty one. | 18:14 |
| Shrews | so passing it in negates that ability | 18:15 |
| pabelanger | okay | 18:15 |
| pabelanger | Hmm, let me see how to support that | 18:15 |
| clarkb | Shrews: responded to the builder.py comments, hopefully its a bit more clear? let me know if you want higher bandwidth communication. re the other things yes I figured its just really weird to em to spend effort on updating all that dead code | 18:17 |
| Shrews | clarkb: why would you need/want to run a second manual build request if there is already an existing build request? | 18:18 |
| Shrews | not following | 18:18 |
| clarkb | Shrews: because that first build requset has gone into a state where no subsequent action will be taken | 18:19 |
| clarkb | Shrews: if it fails, there will be an open request and you cannot submit another manual request if I am reading that right | 18:19 |
| Shrews | clarkb: oh, the state doesn't matter (at least, it shouldn't) | 18:19 |
| clarkb | (and if it fails without updating the data due to an exception or similar you will think the work is still in progress unless you introspect the host system) | 18:20 |
| clarkb | Shrews: why not? | 18:20 |
| Shrews | the request will remain open until it succeeds | 18:20 |
| clarkb | I would assume any request for a manual build intends for it to succeed | 18:20 |
| clarkb | Shrews: yes, but if I read the locking code correctly it will never try a second time | 18:20 |
| clarkb | so yes it will remain open until it succeeds, however I don't think it will ever succeed if I am reading that correctly | 18:21 |
| Shrews | it WILL try a second time, because the build request still exists | 18:22 |
| Shrews | the checks on 390 and 396 will see the request is still open | 18:22 |
| Shrews | the request itself is a zookeeper node that is not removed until there is a successful build | 18:23 |
| clarkb | oh ok, I was reading the build and the buildrequest as being the same node | 18:24 |
| Shrews | so build 00003 may fail for reason X, but the next build 00004 might succeed | 18:24 |
| clarkb | and got confuzzled, so the case to handle there is to properly update the state on exception | 18:24 |
| Shrews | that state should exist within the build (00003) node | 18:25 |
| Shrews | 00003 should have state='failed' | 18:25 |
| clarkb | it won't if an exception short circuits that code path (more likely an issue with the upload side than the build side) | 18:25 |
| Shrews | clarkb: yeah, the ZK stuff is a bit confusing at first without the full picture | 18:25 |
| clarkb | setting state to failed depends on the return value for both of those operations so anything that raises past it will result in a non updated node | 18:26 |
| clarkb | (and you want an updated node so you can see 00003 failed and 00004 is now building and so on) | 18:26 |
| Shrews | clarkb: the uploading code is definitely going to have bugs (but most of it is copy-pasta from the old code). but even if we don't mark it failed, it will remain 'uploading', which is a state we handle and recognize too | 18:27 |
| Shrews | but, let's have this conversation as we test that part of it | 18:27 |
| clarkb | Shrews: yes, but humans won't grok it | 18:27 |
| clarkb | they will say "why am I uploading 100 copies of the same image" | 18:27 |
| clarkb | absically you want the data to reflect reality for humans debugging | 18:28 |
| Shrews | clarkb: i agree | 18:28 |
| clarkb | even if you handle the things more gracefully in the code | 18:28 |
| clarkb | Shrews: the difference with the old code is that https://git.openstack.org/cgit/openstack-infra/nodepool/tree/nodepool/builder.py#n147 sends a work failure to nodepoold which then updates the database | 18:30 |
| clarkb | hwoever new code is interacting with the db directly so must handle that error checking locally | 18:30 |
| Shrews | clarkb: we have no disagreement here. if _uploadImage() does not set the failure state properly, we should fix that | 18:31 |
| clarkb | cool | 18:31 |
| clarkb | I think build needs it too fwiw (just less likely to have problems with it beacusedib is more reliable than glance) | 18:31 |
| Shrews | build *SHOULD* set failure state | 18:31 |
| Shrews | as i've been able to test that | 18:31 |
| Shrews | i haven't been able to test the upload code as well | 18:31 |
| clarkb | Shrews: it does as long as no exceptions are raised | 18:32 |
| Shrews | clarkb: i tried not to change exception handling for the initial iteration. if it bubbled up before, it bubbles up now | 18:32 |
| clarkb | it looks like subprocess' OSError is caught not sure if statsd or the zk things can raise | 18:33 |
| clarkb | Shrews: sort of https://git.openstack.org/cgit/openstack-infra/nodepool/tree/nodepool/builder.py#n114 is the same as with upload, so I think we may need that there too | 18:34 |
| clarkb | (but its possible further reorganization plus preexisting OSError catching is sufficient now | 18:35 |
| clarkb | in any case being able to write tests to cover what happens when things fail will be helpful here | 18:36 |
| clarkb | (and that work is in progress) | 18:36 |
| *** harlowja has quit IRC | 18:38 | |
| pabelanger | Shrews: now that we are using zookeeper, does that mean DibImage in nodedb will be removed? | 18:46 |
| clarkb | pabelanger: yes, as well as all the build image and upload image and check for missing image stuff in nodepool.py | 18:47 |
| clarkb | pabelanger: the snapshot image db table will also go away | 18:47 |
| pabelanger | okay, that is what I am looking at now. And noticed we're still looking to the database for state info | 18:47 |
| Shrews | pabelanger: yeah. we need to integrate the new builder into nodepoold still | 18:48 |
| pabelanger | k | 18:48 |
| Shrews | pabelanger: can you reapply the +A here? https://review.openstack.org/397817 | 18:53 |
| pabelanger | sure | 18:53 |
| openstackgerrit | Merged openstack-infra/nodepool: Use image name in DIB image IDs https://review.openstack.org/397817 | 18:56 |
| jhesketh | Morning | 19:00 |
| *** harlowja has joined #zuul | 19:06 | |
| pabelanger | clarkb: Shrews: so, if I'm looking at nodepool-builder now, is it safe to say both the builder / uploader are now responsible to check for missing images themself | 19:33 |
| clarkb | pabelanger: yes | 19:34 |
| pabelanger | clarkb: Shrews: meaning, we can remove that logic from nodepool.py | 19:34 |
| pabelanger | okay, that's what I suspected | 19:34 |
| clarkb | if you look at the existing code there is a check that says for every image do we have all the image types and is the image new enough | 19:34 |
| clarkb | and builds accordingly if things don't line up as expected | 19:34 |
| jeblair | right. now nodepoold is powerless in that respect. if it doesn't have an image that it needs ready, it just needs to punt on the request and wait until a builder satisfies it. much simpler. :) | 20:07 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Remove snapshot support https://review.openstack.org/396719 | 20:09 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Remove diskimage parameter from config https://review.openstack.org/396749 | 20:09 |
| jeblair | pabelanger: ^ fixed your catch on 719 | 20:10 |
| pabelanger | k | 20:11 |
| pabelanger | Yay | 20:37 |
| pabelanger | .tox/py27/bin/python -m testtools.run nodepool.tests.test_nodepool.TestNodepool.test_node | 20:37 |
| pabelanger | Ran 1 test in 5.265s | 20:38 |
| pabelanger | OK | 20:38 |
| pabelanger | :) | 20:38 |
| pabelanger | some ugly hacking however | 20:38 |
| pabelanger | let me clean up the patch and upload for some eyes | 20:38 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: [WIP] Re-enable TestNodepool.test_node https://review.openstack.org/397944 | 20:45 |
| pabelanger | okay, that is pretty ugly | 20:45 |
| jeblair | pabelanger: the issue is that you need to run a NodePool and have it talk to the zookeeper used by the test, yeah? | 20:47 |
| pabelanger | jeblair: Yes, along with nodepool-builder too | 20:48 |
| jeblair | pabelanger: pabelanger the setup_config method should write out the zookeeper connection information to the config file used by the test | 20:49 |
| jeblair | that should mean that you don't need to pass in a zookeeper object. instead, nodepool can instantiate one on its own by using that config | 20:50 |
| pabelanger | jeblair: okay, let me try that | 20:50 |
| mordred | pabelanger: have I mentioned ttrun ? | 20:54 |
| mordred | pabelanger: "ttrun -epy27 nodepool.tests.test_nodepool.TestNodepool.test_node" is the same as what you typed above, fwiw - in case that's helpful to you in any way | 20:54 |
| pabelanger | ah, cool | 20:54 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Remove Zookeeper per-test fixture https://review.openstack.org/397948 | 20:59 |
| mordred | pabelanger: (you have to pip install it - but still) | 21:04 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Reduce kazoo logging in tests https://review.openstack.org/397952 | 21:08 |
| jeblair | pabelanger, Shrews: i think/hope you're going to love that one ^. i do. | 21:08 |
| pabelanger | jeblair: danke | 21:09 |
| openstackgerrit | Merged openstack-infra/nodepool: Remove snapshot support https://review.openstack.org/396719 | 21:18 |
| openstackgerrit | Merged openstack-infra/nodepool: Remove diskimage parameter from config https://review.openstack.org/396749 | 21:19 |
| pabelanger | jeblair: cool! I see how it works now. Thanks for the info | 21:22 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: [WIP] Re-enable TestNodepool.test_node https://review.openstack.org/397944 | 21:29 |
| pabelanger | Shrews: jeblair: I noticed a trivial thing when converting to zk.getMostRecentImageUpload(), existing code usually is provider then image for args, but new zk functions are look to be image, then provider. Do we care that changed? | 21:33 |
| jeblair | pabelanger: i noticed that too. :) | 21:34 |
| jeblair | pabelanger: the new thing matches the rest of the zk api, i'd leave the signature as is (and make the change in the calling functions) for now | 21:34 |
| pabelanger | wfm | 21:35 |
| jeblair | (and, if we want to bikeshed on the best order for zk api methods, probably do that when Shrews is on vacation so he doesn't kill us) | 21:35 |
| pabelanger | hehe | 21:35 |
| jeblair | pabelanger: i think you should remove the rest of the image related stuff from nodepool.py before you make the change in 397944 | 21:35 |
| jeblair | pabelanger: (i left some quick comments) | 21:35 |
| pabelanger | jeblair: sure, also thoughts on line 1407? I guess we don't need an image.id anymore | 21:37 |
| jeblair | pabelanger: that entire function should go away. problem solved. :) | 21:37 |
| pabelanger | neat | 21:37 |
| jeblair | pabelanger: basically, nodepoold doesn't have any image building responsibility. all it can do is just wait for the right images to show up. | 21:37 |
| clarkb | and it should already do that for the most part in the rest of the scheduler | 21:38 |
| clarkb | allocation etc all know not to do things with a falvor if the image isn't there | 21:38 |
| pabelanger | jeblair: okay, let me check up the left over bits, see if the schedule launches something | 21:39 |
| jeblair | yep, these were just mostly the vestigal check for missing images bits | 21:39 |
| pabelanger | jeblair: what about periodic clean ups for dib images? | 21:45 |
| jeblair | pabelanger: builder handles that too | 21:47 |
| pabelanger | ack | 21:47 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Remove image building responsibility from nodepool.py https://review.openstack.org/397965 | 21:53 |
| *** harlowja has quit IRC | 21:55 | |
| jeblair | pabelanger: one more thing you can remove in that patch (see comment) | 21:57 |
| pabelanger | jeblair: k | 21:58 |
| pabelanger | jeblair: ImageBuildJob too in nodepool/jobs.py> | 21:59 |
| pabelanger | is that no longer needed? | 21:59 |
| jeblair | pabelanger: correct | 22:02 |
| jeblair | pabelanger: imageuploadjob as well | 22:02 |
| Shrews | re: param order, i tried to keep them in the order they are in the zk path | 22:10 |
| Shrews | did i mess up somewhere? | 22:11 |
| Shrews | oh, you're comparing to the old code | 22:12 |
| Shrews | old code is old :-P | 22:12 |
| Shrews | but i don't care if someone wants to change the order if it's more logical | 22:13 |
| *** harlowja has joined #zuul | 22:13 | |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Remove Zookeeper per-test fixture https://review.openstack.org/397948 | 22:17 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Reduce kazoo logging in tests https://review.openstack.org/397952 | 22:17 |
| openstackgerrit | James E. Blair proposed openstack-infra/nodepool: Fix race condition in build cleanup https://review.openstack.org/397975 | 22:17 |
| jeblair | Shrews: i found another sneaky cleanup bug ^ | 22:18 |
| Shrews | jeblair: ah, yeah. i'd say we're going to find a lot of those | 22:20 |
| jeblair | Shrews: yeah, if it ends up being a problem, maybe we can think about either doing some batch processing (like, get all the nodes we're going to work with, then do the filtering on that same data set so we have a (more) consistent view), or different locking. but for now, maybe just make sure they are well annotated as they come up and we can see how big of a problem it is. | 22:22 |
| jeblair | (this is us learning about zk!) | 22:22 |
| jeblair | that, btw, was the only error in the 'remove zk test fixture' runs, so that's cool. :) | 22:23 |
| Shrews | i've been distracted by something for the last couple of hours, and now must go make dinner. will try to catch up on stuff later tonight | 22:24 |
| jeblair | Shrews: cool, i think i've landed everything that was ready today. no rush. enjoy your evening. :) | 22:25 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Remove image building responsibility from nodepool.py https://review.openstack.org/397965 | 23:16 |
| pabelanger | jeblair: re: https://review.openstack.org/#/c/397944/2/nodepool/tests/fixtures/node.yaml yes, we need that code, fake-image-create needs it. I'm sure we can make it more user friendly | 23:22 |
| jeblair | pabelanger: huh. ok. yeah. :) | 23:42 |
| * SpamapS is wading into test_zuul_refs | 23:55 | |
| SpamapS | jeblair: so I'm poking at test_zuul_refs as part of the work to look at removing ZuulTestCase.ref_has_change ... | 23:57 |
| SpamapS | jeblair: I dropped a rock down that well and not sure I heard a splash. Before I dive in.. any advice? | 23:58 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!