| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-31 | |||
| 20:13:08 | sdague | the query string is out of bounds for that replacement | |
| 20:13:22 | efried | ahhhh, cool. | |
| 20:20:43 | dansmith | cdent: jaypipes: so the bottom patch in that series, | |
| 20:20:55 | dansmith | to fix the allocations thing by removing the where clause.. | |
| 20:20:59 | dansmith | doesn't seem to work for me | |
| 20:21:15 | dansmith | gibi's test on top of that still ends up with allocations for both computes after the confirm | |
| 20:22:18 | cdent | the way gibi_ changed it originally was less instrusive: it simply changed the existing and clause to one condition (the consumer uuid) | |
| 20:23:10 | cdent | which ought to be the same thing | |
| 20:23:33 | dansmith | isn't that way jay did? | |
| 20:24:19 | cdent | i’m looking up the discussion, one sec | |
| 20:24:30 | openstackgerrit | Jay Pipes proposed openstack/nova master: remove source provider allocs in confirm_resize() https://review.openstack.org/488510 | |
| 20:24:40 | jaypipes | dansmith: have a gander ^ | |
| 20:24:54 | jaypipes | dansmith: still needs tests but I want to get your early feedback. | |
| 20:25:25 | cdent | http://p.anticdent.org/4oEf | |
| 20:25:25 | dansmith | jaypipes: okay but see my question above? | |
| 20:25:47 | jaypipes | dansmith: about the bottom patch in the series? | |
| 20:26:03 | dansmith | yes | |
| 20:26:13 | jaypipes | dansmith: haven't looked into that yet. unrelated... | |
| 20:26:18 | jaypipes | will do so now | |
| 20:26:31 | cdent | dansmith: that ^ is gibi reporting on what he did, which may not be that all the tests pass but the particular situation was resolved | |
| 20:26:55 | dansmith | cdent: okay but the specific part of gibi's test that should be fixed by thebottom patch isn't | |
| 20:27:17 | dansmith | namely, after confirm, and after PUTing a singular allocation, we still pull the doubled allocation out of placement | |
| 20:28:14 | cdent | dansmith: yeah, and what gibi changed when he ran his local confirmation, was not exactly the same as what jaypipes did in the “bottom change" | |
| 20:28:58 | mriedem | i'm going to poke around and see if there is anything wrong with the test | |
| 20:29:18 | dansmith | cdent: his description seems identical to me, what did gibi do differently? | |
| 20:30:27 | cdent | i’m not certain | |
| 20:30:42 | cdent | i’m trying to get myself spun up | |
| 20:30:50 | dansmith | okay | |
| 20:31:09 | dansmith | I feel like this is one of those times where getting all of us in a room with a (big ass) whiteboard for a week would really help | |
| 20:31:40 | mriedem | sssshhhh | |
| 20:32:11 | dansmith | yeah, re-reading that whole discussion, it sure seems like jay's patch is what gibi did | |
| 20:33:27 | dansmith | oh, hmm | |
| 20:33:35 | dansmith | I think maybe he didn't update his allocations after confirming | |
| 20:34:24 | dansmith | ah hah, yep | |
| 20:34:52 | dansmith | mriedem: if you haven't already, I can fix this and rebase on jay's latest for everyone to see | |
| 20:34:57 | mriedem | go nuts | |
| 20:35:01 | jaypipes | go for it. | |
| 20:35:11 | cdent | dansmith: ? | |
| 20:36:02 | mriedem | yeah i see it | |
| 20:36:12 | mriedem | needs to do: allocations = self._get_allocations_by_server_uuid(server['id']) | |
| 20:36:14 | mriedem | after confirming the resize | |
| 20:36:22 | mriedem | before making assertions on that allocations variable | |
| 20:36:24 | mriedem | which is stale | |
| 20:37:00 | dansmith | cdent: check me: https://review.openstack.org/#/c/487958/7/nova/tests/functional/test_servers.py | |
| 20:37:01 | cdent | I think when he was confirming before, it was based on the logs, | |
| 20:37:05 | dansmith | mriedem: yeah | |
| 20:37:30 | cdent | line? | |
| 20:37:42 | cdent | found it | |
| 20:37:45 | mriedem | https://review.openstack.org/#/c/487958/7/nova/tests/functional/test_servers.py@1406 | |
| 20:38:27 | mriedem | the revert test seems ok in that regard | |
| 20:38:32 | cdent | yeah | |
| 20:38:46 | dansmith | I'm going to remove the resource_tracker.py change so we can get an expedited run on top of jay's stuff, | |
| 20:38:55 | dansmith | and then we can move this to the bottom, change the assertions and build on top | |
| 20:39:06 | mriedem | also | |
| 20:39:08 | jaypipes | ++ | |
| 20:39:09 | mriedem | have you noticed? ObjectActionError: Object action get_minimum_version failed because: Invalid binary prefix | |
| 20:39:30 | dansmith | where is that? | |
| 20:39:52 | jaypipes | must be on the latest | |
| 20:40:09 | dansmith | yeah he must not have done nova-compute | |
| 20:40:10 | jaypipes | since I *just* added the get_minimum_version_latest() thing | |
| 20:40:30 | mriedem | get_minimum_version_multi takes a list | |
| 20:40:45 | jaypipes | gah, ok | |
| 20:41:00 | jaypipes | sec... like I said, this was just so dansmith could take a lo9oksie :) | |
| 20:41:06 | mriedem | just use get_minimum_version | |
| 20:41:15 | jaypipes | k | |
| 20:41:43 | dansmith | damn I guess I probably need the virt/fake change too | |
| 20:42:13 | mriedem | the test will also likely need the AllServicesCurrent fixture now too | |
| 20:43:08 | openstackgerrit | Dan Smith proposed openstack/nova master: Test resize with placement api https://review.openstack.org/487958 | |
| 20:43:18 | dansmith | that's the minor fix | |
| 20:43:26 | dansmith | I can start working this on top of master | |
| 20:48:59 | cdent | I will try to catch up in the morning. I have no brains left. Good luck. Good night. | |
| 20:59:03 | dansmith | I'm just going to push this up rebased on master so we can move forward | |
| 20:59:17 | dansmith | jaypipes: you'll rebase on top of this and make sure this test keeps working as you make your changes, right? | |
| 20:59:45 | dansmith | and it'd be really good if we had a single-node version of these | |
| 21:00:10 | mriedem | i can help work on the single node one, | |
| 21:00:14 | mriedem | also digging into the revert case | |
| 21:00:21 | jaypipes | dansmith: yes, just ping me when you push. | |
| 21:00:34 | jaypipes | currentl fixing up unit tests for the service min version thing | |
| 21:00:48 | dansmith | mriedem: what revert case? | |
| 21:01:00 | mriedem | the revert resize test that fails | |
| 21:01:13 | dansmith | of gibi's? | |
| 21:01:15 | mriedem | http://logs.openstack.org/58/487958/7/check/gate-nova-tox-functional-ubuntu-xenial/7f7f332/console.html#_2017-07-31_17_04_11_513991 | |
| 21:01:16 | mriedem | yeah | |
| 21:01:21 | dansmith | he has self.fail() at the end | |
| 21:01:23 | dansmith | I think that's it | |
| 21:01:29 | dansmith | passes for me without that | |
| 21:02:04 | mriedem | hmm, ok the test was hitting this http://logs.openstack.org/58/487958/7/check/gate-nova-tox-functional-ubuntu-xenial/7f7f332/console.html#_2017-07-31_17_04_11_497592 but on the older patches in the series | |
| 21:02:28 | dansmith | jay hasn't fixed revert yet, AFAIK | |
| 21:02:29 | dansmith | only confirm | |
| 21:02:54 | mriedem | i think i just did locally | |
| 21:03:10 | dansmith | okay well I have it passing on master, so we'll iterate from there | |
| 21:03:22 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to instance_extra https://review.openstack.org/457711 | |
| 21:03:32 | openstackgerrit | Dan Smith proposed openstack/nova master: Test resize with placement api https://review.openstack.org/487958 | |
| 21:04:09 | dansmith | mriedem: jaypipes ^ | |
| 21:04:18 | dansmith | I can also try to clean up the sleep usage in there by just calling into the manager | |
| 21:04:59 | mriedem | dansmith: jaypipes: this fixed revert for me https://review.openstack.org/#/c/488510/6/nova/compute/resource_tracker.py | |
| 21:05:06 | mriedem | basically the same thing as confirm in the RT | |
| 21:07:16 | jaypipes | mriedem: sure, ok will add that. | |
| 21:08:20 | mriedem | dansmith: i think you just need to do: self.compute.manager.update_available_resource(ctxt) | |
| 21:08:30 | dansmith | mriedem: I know | |