| openstackgerrit | Merged openstack-infra/nodepool: Add missing pause fields to config-validate https://review.openstack.org/410992 | 00:34 |
|---|---|---|
| openstackgerrit | Merged openstack-infra/nodepool: Add missing pause fields to config-validate https://review.openstack.org/411004 | 00:52 |
| openstackgerrit | Merged openstack-infra/nodepool: Validate configs when used by tests https://review.openstack.org/410995 | 00:52 |
| *** jamielennox is now known as jamielennox|away | 00:58 | |
| *** jamielennox|away is now known as jamielennox | 01:00 | |
| *** jamielennox is now known as jamielennox|away | 02:36 | |
| *** jamielennox|away is now known as jamielennox | 02:51 | |
| *** harlowja has quit IRC | 03:03 | |
| *** saneax-_-|AFK is now known as saneax | 04:46 | |
| *** saneax is now known as saneax-_-|AFK | 05:12 | |
| *** saneax-_-|AFK is now known as saneax | 05:17 | |
| *** abregman has joined #zuul | 06:10 | |
| *** hashar has joined #zuul | 07:40 | |
| *** hashar has quit IRC | 08:04 | |
| *** hashar has joined #zuul | 09:35 | |
| *** openstackgerrit has quit IRC | 10:18 | |
| *** abregman is now known as abregman|afk | 12:37 | |
| *** openstackgerrit has joined #zuul | 12:49 | |
| openstackgerrit | Merged openstack-infra/zuul: Fix retry accounting off-by-one bug https://review.openstack.org/408814 | 12:49 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_nonvoting_job https://review.openstack.org/410376 | 13:01 |
| openstackgerrit | Merged openstack-infra/zuul: Re-enable test_no_job_project https://review.openstack.org/410378 | 13:03 |
| *** abregman|afk is now known as abregman | 13:16 | |
| *** saneax is now known as saneax-_-|AFK | 13:23 | |
| rcarrillocruz | afk for a while, picking my son | 14:37 |
| rcarrillocruz | laterz | 14:37 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Remove ZK lock variables https://review.openstack.org/410853 | 14:56 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 14:56 |
| Shrews | let's see what ^^^ breaks | 14:57 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 15:40 |
| mordred | Shrews: woot! | 15:46 |
| *** abregman has quit IRC | 16:01 | |
| Shrews | well that change is failing spectacularly | 16:12 |
| *** Cibo_ has quit IRC | 16:18 | |
| *** Cibo_ has joined #zuul | 16:23 | |
| mordred | Shrews: that means you did something right | 16:34 |
| *** hashar has quit IRC | 16:35 | |
| Shrews | they pass individually on my local machine, so... weird | 16:43 |
| clarkb | resource contention between parallel tests? | 16:44 |
| Shrews | gotta be something with the test setup that i missed. all of the test_builder tests passed, but test_commands.py and test_nodepool.py (both needing ZK connections, too) failed. | 16:49 |
| Shrews | oh, i think i see it | 16:57 |
| *** ajafo has quit IRC | 17:09 | |
| *** ajafo has joined #zuul | 17:10 | |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 17:10 |
| openstackgerrit | James E. Blair proposed openstack-infra/zuul: Fix test_rerun_on_abort https://review.openstack.org/411426 | 17:25 |
| jeblair | SpamapS, jhesketh, mordred: would you please all review 411426? and in the future, please assume that the tests are correct and carefully evaluate behavior to the contrary. | 17:27 |
| Shrews | pabelanger: jeblair: Can you guys think of a way we could test 411360 w/o disrupting production? | 17:49 |
| clarkb | Shrews: maybe apply it to one of the two builders | 17:51 |
| clarkb | at least that way if it breaks we just turn that one off for a bit | 17:51 |
| Shrews | clarkb: oh, that's a good idea | 17:52 |
| pabelanger | Ya, we could do that | 17:52 |
| clarkb | but also, lean on the integration test. Its pretty good at catching the high level things | 17:52 |
| clarkb | update as necessary to exercise the important bits | 17:52 |
| pabelanger | maybe we should add a 2nd provider to our devstack test | 17:52 |
| Shrews | i still worry about possible ZK data corruption by just upgrading one of the builders. maybe i could super stress my 12G RAM laptop by running devstack and zookeeper and a builder | 17:57 |
| Shrews | ugh | 17:57 |
| pabelanger | I could give it a go in my local env | 18:08 |
| pabelanger | once I get centos-7 working again | 18:08 |
| greghaynes | Shrews: it shouldnt be hard to write some multithreaded zk client unit tests if youre really worried. ISTM that as long as kazooclient is reentrant it shouldnt be an issue | 18:10 |
| greghaynes | left a doc comment, though | 18:10 |
| Shrews | greghaynes: oh, good eye | 18:11 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 18:12 |
| jeblair | Shrews: it looks like kazoo set_hosts updates the host list but does not change the connection status. it has 'undefined' behavior if you set it to a completely disparate zk cluster. i think that's all fine, as it should let an operator gracefully (or as gracefully as possible) rotate a zk configuration (eg, add or drop a server). and we can just note in the docs that if you want to do something different, like switch to a new cluster, you ... | 18:25 |
| jeblair | ... should restart nodepool. | 18:25 |
| jeblair | Shrews: does that sound right to you? | 18:26 |
| *** harlowja has joined #zuul | 18:31 | |
| jeblair | Shrews: minor point in 411360 -- why create the zk for the builder in the test? why not just let the builder create its own client, and have the tests use their own client? that would let us automatically exercise the client creation code in the tests; and i'm not really worried about the tests having an extra connection open. | 18:33 |
| Shrews | jeblair: i *think* what the client will do with a totally *new* set of hosts is remain connected to the old cluster, unless the old cluster goes away, in which case it should automatically use the new cluster | 18:44 |
| Shrews | jeblair: so, if indeed you do switch hosts, and the old cluster remains active, then yes, a restart will be needed | 18:44 |
| jeblair | Shrews: that makes sense. caveat operator. :) | 18:44 |
| Shrews | jeblair: as for your minor point, it seemed like keeping all of the kazoo specifics contained to the ZooKeeper class made most sense to me (builder doesn't need to know how to create a kazoo client, and the fact that it is kazoo should remain hidden as best as possible IMO) | 18:46 |
| Shrews | plus, this seemed like shortest-path-to-success | 18:47 |
| Shrews | :) | 18:47 |
| jeblair | Shrews: er, i may have not communicated that well -- see my note on the change where i think i phrased it slightly differently | 18:49 |
| jeblair | Shrews: it's more about not special-casing the test path | 18:49 |
| Shrews | jeblair: oh! (words words) | 18:49 |
| Shrews | jeblair: so, the builder's zk is based on the config values only. we need the per-test chroot for the tests | 18:50 |
| Shrews | that's why the tests pass a zk object in | 18:50 |
| jeblair | Shrews: don't we interpolate the chroot into the config in the tests? | 18:50 |
| Shrews | jeblair: the chroot is randomized for each test | 18:50 |
| jeblair | (i think that's how the workers got their zk client before, iirc) | 18:50 |
| Shrews | ChrootedZKFixture or something | 18:51 |
| jeblair | Shrews: yeah, but we splat the chroot into the text fixture config before using it | 18:51 |
| Shrews | do we? | 18:51 |
| Shrews | i don't see that | 18:52 |
| jeblair | Shrews: http://git.openstack.org/cgit/openstack-infra/nodepool/tree/nodepool/tests/__init__.py#n415 | 18:52 |
| jeblair | so anything with setup_config will do that | 18:52 |
| *** harlowja has quit IRC | 18:52 | |
| Shrews | jeblair: oh, well that changes e'erthang | 18:53 |
| pabelanger | so, alien-image-list is broken | 18:58 |
| pabelanger | we're not properly resolving our diskimage names | 18:58 |
| jeblair | whoopsie | 18:58 |
| pabelanger | were also duplicating data too | 18:58 |
| pabelanger | http://paste.openstack.org/raw/592533/ | 18:58 |
| clarkb | pabelanger: what do you mean by resolving names? | 18:58 |
| pabelanger | clarkb: were trying to use template-ubuntu-xenial-1481225253 for example to search data in zookeeper | 18:59 |
| pabelanger | when it should be just ubuntu-xenial | 18:59 |
| clarkb | (also keep in mind that osic is a single cloud with 3 providers in nodepool so that particular one will be funny) | 18:59 |
| jeblair | pabelanger: remember that we actually should have alien images atm since we haven't (iirc) deleted the mysql builder images | 19:00 |
| jeblair | (alien images that *look like* nodepool images, even) | 19:00 |
| pabelanger | jeblair: I think http://git.openstack.org/cgit/openstack-infra/nodepool/tree/nodepool/cmd/nodepoolcmd.py#n227 is the reason we don't actually see the old iamges right now | 19:01 |
| pabelanger | if you use alien-image-list on nodepool.o.o, you just see the current images we have uploaded with nb01 and nb02 | 19:02 |
| clarkb | ah yes we needed a way to filter out the cloud images and we did ^ | 19:02 |
| clarkb | but if old code didn't do that then ya | 19:02 |
| pabelanger | yup | 19:03 |
| * clarkb adds better distinction between cloud images and user images in the cloud grumps etherpad | 19:03 | |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 19:04 |
| pabelanger | we might need to also upload the diskimage name into meta data too | 19:04 |
| openstackgerrit | Merged openstack-infra/zuul: Add update storyboard script https://review.openstack.org/407229 | 19:08 |
| pabelanger | yup test_alien_image_list_empty is failing, but passing | 19:17 |
| openstackgerrit | Adam Gandelman proposed openstack-infra/zuul: Add new v3 test_dyanmic_config_conflict (WIP) https://review.openstack.org/411467 | 19:19 |
| openstackgerrit | Adam Gandelman proposed openstack-infra/zuul: Add new v3 test_dyanmic_config_conflict (WIP) https://review.openstack.org/411467 | 19:24 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Clean up alien-(image-)list tests https://review.openstack.org/411470 | 19:28 |
| pabelanger | clarkb: ^ okay, fixes our tests but doesn't fix alien-image-list yet | 19:29 |
| mordred | clarkb: to be fair - there IS a distinction between images uploaded by the cloud and images uploaded by us - visibility=Public | is_public=True will only be set for provider provided images, and is_public=False|visiblity=Private will be for ours | 19:41 |
| clarkb | oh then maybe we should filter on that? | 19:42 |
| mordred | pabelanger, clarkb: it's possible we should combine an amount of filtering on those attributes with what we're doing now | 19:42 |
| mordred | yah | 19:42 |
| clarkb | I don't know that we want to combine thenm | 19:42 |
| * mordred goes to make patch | 19:42 | |
| clarkb | what we want is the list of all images that were not uploaded by the cloud provider | 19:42 |
| mordred | yah | 19:42 |
| clarkb | then remove what nodepool is using from that list and the rsult is the aliens | 19:42 |
| openstackgerrit | Monty Taylor proposed openstack-infra/nodepool: Use is_public to determine vendor images https://review.openstack.org/411473 | 19:46 |
| mordred | clarkb: something like that ^^ | 19:46 |
| pabelanger | Oh, haha. I see another issue | 19:49 |
| Shrews | So, according to testr FAQ (https://wiki.openstack.org/wiki/Testr#How_can_I_run_just_one_test.3F_Just_some_tests.3F), I should be able to run multiple tests through tox like so: tox -epy27 -- '(TestNodepoolCMD|TestZooKeeper)' | 19:49 |
| clarkb | mordred: yup | 19:49 |
| mordred | pabelanger: fwiw, properties and metadata are the same, but metadata is a backwards compat field and properties is in the data model contract | 19:49 |
| clarkb | Shrews: yes | 19:49 |
| Shrews | but that only runs the first matching tests (TestZooKeeper class tests are not run) | 19:49 |
| mordred | pabelanger: did you hit something not being able to find image['properties'] ? | 19:50 |
| * clarkb attempts to run tests via regex locally | 19:51 | |
| Shrews | where's robert collins when you need him... | 19:52 |
| clarkb | Shrews: it ran both for me... | 19:52 |
| clarkb | Shrews: http://paste.openstack.org/show/592538/ | 19:53 |
| pabelanger | mordred: yes, our fakes: https://review.openstack.org/#/c/411470/ | 19:53 |
| Shrews | clarkb: oh, nm. i am teh dumb | 19:53 |
| clarkb | tox -e py27 -- '(TestNodepoolCMD|TestZooKeeper)' | 19:53 |
| clarkb | is what I ran | 19:53 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 19:53 |
| mordred | pabelanger: yah - we should fix the fakes :) | 19:53 |
| Shrews | clarkb: all the slowest tests reported were from one class. silly me didn't check logs | 19:54 |
| pabelanger | my, haha, was a mistake. Thought I see an issue someplace else | 19:55 |
| mordred | oh - well - | 19:55 |
| mordred | I see an issue we should fix | 19:55 |
| mordred | let me send up a quick patch - we're still returning objects from our fakes in our fake provider | 19:55 |
| pabelanger | ya, was just going to ask if shade had something we could use | 19:56 |
| mordred | not anything marked as public - but maybe it's time to do that next | 19:56 |
| openstackgerrit | David Shrewsbury proposed openstack-infra/nodepool: Share single ZooKeeper object among workers https://review.openstack.org/411360 | 19:56 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Use provider images from yaml for alien_image_list https://review.openstack.org/411487 | 20:28 |
| pabelanger | jeblair: clarkb: mordred: Shrews: ^ I think is part of our fix for alien-image-list | 20:28 |
| pabelanger | tested it, and we did leak an image in infracloud-vanilla for xenial | 20:30 |
| jeblair | PSA i just changed 410853 411360 411470 411473 411487 to topic:zuulv3 | 20:34 |
| pabelanger | sorry about that | 20:36 |
| clarkb | pabelanger: do you not need to pass provider.images[name] or .name in the build lookup? | 20:36 |
| * clarkb digs into zk.py | 20:37 | |
| mordred | ok. nothing is set up in the tests to provide the ability for alien_image_list to throw a failure | 20:37 |
| clarkb | pabelanger: ya getBuildNumbers takes a string of the image name | 20:37 |
| mordred | the monkeypath in that test isn't going to work becau ethe FakeOpenStackCloud is already instantiated in setupFakes | 20:37 |
| mordred | we need to be able to pass in a should-fail attribute somewhere | 20:38 |
| mordred | but since it's alien-image-list, there isn't a great place from which to do that | 20:38 |
| clarkb | mordred: why would alien_image_list throw a failure? | 20:39 |
| mordred | clarkb: no idea | 20:39 |
| mordred | but there is a test: test_alien_image_list_fail | 20:39 |
| mordred | which pabelanger has a patch up to try to fix - since it just always passes | 20:39 |
| clarkb | huh | 20:39 |
| mordred | I think we could make it fail | 20:40 |
| clarkb | I mean it either lists images or it doesn't | 20:40 |
| mordred | but I'm not sure what that'll get us | 20:40 |
| clarkb | mordred: right | 20:40 |
| mordred | other than testing the test suite | 20:40 |
| openstackgerrit | Monty Taylor proposed openstack-infra/nodepool: Use is_public to determine vendor images https://review.openstack.org/411473 | 20:41 |
| clarkb | mordred: I think the tests we want are no aliens and some aliens | 20:41 |
| clarkb | double points if you can set them up to come from mars | 20:41 |
| mordred | hahahah | 20:42 |
| pabelanger | just tested is_public, that worked as expected | 20:42 |
| mordred | pabelanger: yay! | 20:42 |
| mordred | clarkb: any thoughts on how to get an alien image into the FakeOpenStackCloud? | 20:47 |
| pabelanger | jeblair: clarkb: left a comment on 411487, can you advise | 20:47 |
| clarkb | mordred: I think you just edit the images list directly in the test | 20:47 |
| mordred | clarkb: how? there's not a handle to the cloud object ... | 20:48 |
| clarkb | mordred: fakecloud.images.add(whateveralienlookslike) | 20:48 |
| mordred | oh - wait - I've got an idea | 20:48 |
| jeblair | mordred, clarkb, pabelanger: i believe that test was originally testing that it *did not* raise an exception on failure. of course, changes to the fake cloud stuff probably caused that to silently stop working, so a fix should probably make sure it actually runs :) | 20:48 |
| jeblair | pabelanger: oh, derp. i neglected to realize you were iterating over a dict, so you *are* getting name strings there. cc clarkb | 20:49 |
| clarkb | pabelanger: I think using image.name is more explicit than side effecting .values | 20:49 |
| clarkb | oh now I grok | 20:49 |
| clarkb | p.images[i.name] = i is why it works ok | 20:50 |
| * clarkb updates vote | 20:50 | |
| jeblair | yeah, so 3 choices: a) as-is, b) .keys() with 'image' to be more specific, or c) .values() and 'image.name' | 20:50 |
| jeblair | any work for me, so +2 | 20:51 |
| clarkb | ya I have approved | 20:51 |
| pabelanger | neat :) | 20:51 |
| clarkb | mordred: the fake cloud takes an images list in its constructor | 20:52 |
| mordred | right. but I don't operate the fake cloud | 20:52 |
| clarkb | mordred: if you look in test_commands there is an example of patching things to do stuff with server listing | 20:53 |
| clarkb | you can also patch out the thing that returns the fake cloud in __init__.py or similar | 20:54 |
| clarkb | or possibly update setUpFakes to pass in an images list somehow | 20:54 |
| clarkb | I am not sure what the least bad choice is :) | 20:54 |
| mordred | that's probably what I need to do -the setUpFakes thing | 20:55 |
| mordred | I do not see anywhere in test_commands where we manipulate the data returned from FakeOpenStackCloud | 20:55 |
| clarkb | mordred: oh its these alient list tests | 20:56 |
| clarkb | so maybe my version is old and yours has been updated? | 20:56 |
| mordred | clarkb: which test you looking at? | 20:56 |
| jeblair | perhaps it would be safe for a fakeopenstackcloud to always have some fake leaked images? | 20:56 |
| jeblair | also some public ones | 20:56 |
| mordred | jeblair: I think the problem I'm running in to is that I don't know how to , in a test, get a hold of the fake cloud instance so it can be manipulated | 20:56 |
| clarkb | mordred: test_alien_list_fail has the patching | 20:57 |
| mordred | clarkb: that doesn't work | 20:57 |
| clarkb | mordred: ya because object is already made? | 20:57 |
| mordred | clarkb: ++ | 20:57 |
| jeblair | mordred: yeah, i was saying maybe we can have it always have the images -- like it creates them in the constructor. but i guess that really only inverts the problem (we would not be able to have a test with 0 leaked images) | 20:57 |
| clarkb | I think it would work if you changed it to patch the object itself | 20:57 |
| mordred | jeblair: so we can have it always have leaked instances, but then we can't test the negative case | 20:57 |
| clarkb | but digging that out may be unfun | 20:57 |
| jeblair | mordred: right :) | 20:57 |
| mordred | exactly | 20:57 |
| mordred | clarkb: the object is returned from a closure | 20:57 |
| mordred | clarkb: so there is no handle to the object - it's not a self property anywhere :) | 20:58 |
| * mordred pokes some more | 20:58 | |
| mordred | I have a whole new very stupid idea | 20:58 |
| clarkb | mordred: right what you would do is replace what that closure replaces | 20:59 |
| clarkb | mordred: you'd repatch nodepool.provider_manager.ProviderManager._getClient with your own closure or whatever | 20:59 |
| jeblair | mordred: test_image_upload_fail swaps out the fakeopenstackcloud class | 20:59 |
| jeblair | clarkb: i *think* that may be what that test does | 20:59 |
| openstackgerrit | Merged openstack-infra/nodepool: Use provider images from yaml for alien_image_list https://review.openstack.org/411487 | 21:00 |
| mordred | jeblair: it does not actually do that though | 21:00 |
| mordred | jeblair: I mean, it does - but it does it after the fake_client has been instantiated | 21:00 |
| mordred | I havea solution though - one sec | 21:00 |
| jeblair | mordred: gotcha | 21:00 |
| clarkb | mordred: that should be fine though | 21:00 |
| clarkb | you have to do it bfore the useNodepool stuff | 21:01 |
| jeblair | mordred: yeah, you could instantiate the fake client in the get_fake_client method | 21:01 |
| clarkb | as that is what instantiates all the things outside the tests | 21:01 |
| mordred | http://paste.openstack.org/show/592550/ | 21:01 |
| mordred | that ^^ | 21:01 |
| mordred | gives me a handle I can manipulate | 21:01 |
| jeblair | mordred: that's nice | 21:01 |
| mordred | jeblair: I'm sad it took me so long to realize I can just do that :) | 21:02 |
| mordred | you should have seen the mental gymnastics I was doing | 21:02 |
| jeblair | i wonder if test_image_upload_fail could use that too? | 21:02 |
| mordred | yup | 21:02 |
| Shrews | mordred: hah, you are (were?) doing the gymnastics i was doing a few weeks back when i was considering that problem | 21:04 |
| Shrews | never got around back to it, but glad you did | 21:05 |
| *** Kitty- has joined #zuul | 21:05 | |
| clarkb | both methods are roughly equivalent, they both change the return value of get one cloud | 21:05 |
| clarkb | this is definitely nicer to read though | 21:06 |
| mordred | HAHAHAHAHAHAHHAHAHA | 21:17 |
| mordred | ugh | 21:17 |
| mordred | *HEADDESK* | 21:17 |
| pabelanger | clarkb: also, is there any reason not to have alien-image-delete/purge? | 21:19 |
| clarkb | pabelanger: yes, the osic situation | 21:21 |
| pabelanger | Ah, ya. Keep forgetting about that | 21:21 |
| clarkb | its really easy to delete in bulk with the alien output as is too so not sure we want the real foot gun | 21:22 |
| clarkb | pabelanger: alient-image-list > file ; review file ; for image in cat file ; do openstack image delete $file ; done | 21:22 |
| pabelanger | ya | 21:22 |
| jeblair | perhaps we should have it include all known images when removing images from the set, not just ones in that provider | 21:23 |
| pabelanger | interesting | 21:24 |
| clarkb | that is one option, though it potentially makes alien listing less accurate | 21:24 |
| clarkb | depending on how you look at the output | 21:24 |
| mordred | clarkb, jeblair: I have things a little further, but it's eluding me | 21:24 |
| mordred | I'm going to push up what I've got | 21:24 |
| mordred | well - two local things first | 21:26 |
| pabelanger | clarkb: jeblair: would it be a lot of work to update nodepool-builder now to handle the osic-cloud1 use case properly now? EG: 1 ubuntu-xenial upload over 3 | 21:26 |
| pabelanger | something like provider groups | 21:27 |
| mordred | maybe when we rework launchers | 21:27 |
| mordred | we can express "this is a cloud that should have an image uploaded to it" and "this is a profile on that cloud with different flavor/az information in it that we should launch nodes in" | 21:28 |
| mordred | the same issue came up with hpcloud back in the day | 21:28 |
| clarkb | pabelanger: mordred ya I think its a launcher thing more so than a builder thing | 21:28 |
| pabelanger | k, something we should keep in mind | 21:30 |
| clarkb | mordred: I like the idea of profiles | 21:30 |
| clarkb | the tricky bit to represent is going to be flavor to image stuff potentially | 21:31 |
| clarkb | without ending up copying image lists to represent all the things | 21:31 |
| mordred | yah | 21:31 |
| mordred | well, we haven't ever had any specific relationship between flavors and images before have we? | 21:32 |
| mordred | we've had "use these 3 different flavors" on osic and "use these three different azs" on hpcloud - both of which are just different parameters to pass to create_server | 21:32 |
| clarkb | mordred: we do since the image list is where you list your flavor details | 21:32 |
| Shrews | pabelanger: jeblair: on the single ZK connection change... I won't be around most of the day tomorrow (traveling to snowy WV) if you happen to test the change then. | 21:33 |
| Shrews | but i'm almost 10% sure it will work perfectly and without issues | 21:33 |
| Shrews | maybe 11 | 21:33 |
| jeblair | Shrews changes go to 11 | 21:36 |
| openstackgerrit | Monty Taylor proposed openstack-infra/nodepool: Use is_public to determine vendor images https://review.openstack.org/411473 | 21:36 |
| openstackgerrit | Monty Taylor proposed openstack-infra/nodepool: Remove FakeProviderManager https://review.openstack.org/411512 | 21:36 |
| mordred | jeblair, clarkb: ^^ so 411473 - if you run nodepool.tests.test_commands.TestNodepoolCMD.test_alien_image_list_alien | 21:37 |
| mordred | in the stdout it _looks_ to my eyeballs like it's showing images in the alien list output | 21:37 |
| mordred | but I also may just be silly | 21:37 |
| mordred | and missing something really obvious | 21:38 |
| jeblair | mordred: you're going to like this | 21:39 |
| jeblair | mordred: the fact that it looks like it's working means it's not working | 21:39 |
| mordred | jeblair: _excellent_ | 21:39 |
| mordred | jeblair: how silly of me :) | 21:39 |
| jeblair | mordred: the assert list methods monkepatch prettytable | 21:39 |
| jeblair | mordred: and as part of that, it means that nothing should show up in the table | 21:40 |
| mordred | ah | 21:40 |
| jeblair | mordred: (but i added a debug statement so you can see what *would* have been in the table) | 21:40 |
| jeblair | mordred: when i run it i see 2 table outputs | 21:40 |
| jeblair | mordred: http://paste.openstack.org/show/592554/ | 21:41 |
| mordred | jeblair: yes. that is what I also see | 21:41 |
| mordred | jeblair: so that's good at least | 21:41 |
| jeblair | mordred: the second is what happens when the prettytable monkepatch thing happens, the first must not be monkepatched | 21:41 |
| mordred | it gives me a new place to go put a bunch fo swear words in print statements | 21:41 |
| mordred | well - the _first_ thing does not show me the extra image I put in to the image list either | 21:42 |
| mordred | oh. duh. of course it doesn't | 21:42 |
| mordred | one sec - lemme see if I can at least get that to show up | 21:42 |
| mordred | I added a public alien image which means I did _not_ add an alien image | 21:42 |
| jeblair | mordred: it looks like your test is running the list command twice | 21:43 |
| mordred | and things approrpiately did not show the provider image | 21:43 |
| jeblair | mordred: assert_alien_images runs the command (with the prettytable mock) | 21:43 |
| jeblair | mordred: so that's the second output. the invocation of the command in the test isn't necessary (that's the first output) | 21:43 |
| mordred | the nodepoolcmd.main() ? | 21:44 |
| jeblair | yep | 21:44 |
| jeblair | and patch argv | 21:44 |
| jeblair | assert_alien_images does all that | 21:45 |
| mordred | DEBUG:tests.PrettyTableMock:['fake-provider', 'Fake Alien', 'alien-image'] | 21:45 |
| mordred | DEBUG:tests.PrettyTableMock:['fake-provider', 'template-fake-image-1481838296', '17f62c9c281d4ae3a2f9b8300a0f6b8f'] | 21:45 |
| jeblair | via assert_alien_images_listed via assert_listed | 21:45 |
| mordred | woot. - I mean - at least a thing I made showed p in the thing | 21:45 |
| jeblair | ++ | 21:45 |
| jeblair | we can drop assert_alien_images; that seems pretty specific to a single test | 21:46 |
| mordred | OH! I need to pass in the id I'm looing for | 21:47 |
| * adam_g emerges from from a rabbit hole | 21:49 | |
| openstackgerrit | Adam Gandelman proposed openstack-infra/zuul: Fix conflict detection and reporting for in-repo configs https://review.openstack.org/411467 | 21:49 |
| openstackgerrit | Monty Taylor proposed openstack-infra/nodepool: Use is_public to determine vendor images https://review.openstack.org/411473 | 21:53 |
| mordred | jeblair: BOOM ^^ | 21:53 |
| mordred | jeblair: thanks for the help- I'm sure I would have gotten there _eventually_ with enough curse words in print statements, but it likelu would have taken quite a while and I would have thrown something | 21:53 |
| openstackgerrit | Paul Belanger proposed openstack-infra/nodepool: Drop 'template' from default value of template-hostname https://review.openstack.org/411515 | 21:59 |
| jeblair | adam_g: just finished looking over https://review.openstack.org/409376 -- one comment inline -- let me know if that makes sense (cc mordred SpamapS) | 21:59 |
| jeblair | mordred: throw me a brisket? | 22:00 |
| mordred | jeblair: yes. I agree with your comment | 22:01 |
| mordred | jeblair: I don't have a brisket today - but I do have a nice looking picanha | 22:01 |
| adam_g | jeblair: yeah, that makes sense. i might actually rebase that on top of 411467 | 22:01 |
| mordred | jeblair: that's this one: https://en.wikipedia.org/wiki/Picanha | 22:02 |
| pabelanger | now I want a picanha | 22:09 |
| SpamapS | jeblair: ACK | 22:17 |
| *** Kitty- has quit IRC | 22:37 | |
| *** Kitty- has joined #zuul | 22:37 | |
| *** saneax-_-|AFK is now known as saneax | 22:40 | |
| *** saneax is now known as saneax-_-|AFK | 22:48 | |
| Shrews | mordred: that does not look like it would be the tastiest cut of meat, but it's still cow, so i want some | 22:58 |
| jeblair | adam_g: left some comments on 411467 | 22:58 |
| *** saneax-_-|AFK is now known as saneax | 23:03 | |
| SpamapS | mmmmmmmmmm fatty Picanha | 23:05 |
| SpamapS | they have that at the brazilian steakhouse in Tarzana.. mm | 23:06 |
| mordred | Shrews: oh - it's actually the tastiest cut of meat | 23:20 |
| *** Kitty- has left #zuul | 23:28 | |
| adam_g | jeblair: thanks | 23:33 |
| adam_g | jeblair: so yeah, im definitely conflating the speculative merges with landing merges. | 23:38 |
| adam_g | will need to dig in a bit further | 23:38 |
| adam_g | but AFAICS the gist of 411467 is gonna be needed if the schedulers going to be catching and reporting conflicts for all changes | 23:39 |
| *** jamielennox is now known as jamielennox|away | 23:46 | |
| *** jamielennox|away is now known as jamielennox | 23:50 | |
| *** harlowja has joined #zuul | 23:58 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!