| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-01 | |||
| 00:06:20 | jaypipes | nope. | |
| 00:06:32 | dansmith | um why? | |
| 00:06:35 | jaypipes | dansmith: delete doesn't clean up placement at all. | |
| 00:06:46 | dansmith | delete doesn't call self.update() or whatever in compute manager? | |
| 00:06:58 | dansmith | that would mean you can't boot instances on a compute node until after the periodic runs to clean up | |
| 00:06:58 | jaypipes | update is only inventory. | |
| 00:07:53 | jaypipes | dansmith: welcome to my hell. | |
| 00:08:01 | dansmith | no, I'm serious | |
| 00:08:21 | dansmith | that makes zero sense to me.. surely we'd have people beating down our door if deleting an instance didn't free up space for another one | |
| 00:08:44 | dansmith | jaypipes: but again, the tests pass for me, so what are you saying is failing? | |
| 00:09:09 | jaypipes | dansmith: nm, I'm wrong. | |
| 00:09:27 | jaypipes | I think... | |
| 00:09:28 | dansmith | self._update_resource_tracker() is in _complete_deletion() | |
| 00:09:32 | dansmith | surely that updates things | |
| 00:09:44 | dansmith | yeah, calls update_usage() | |
| 00:09:44 | jaypipes | only if the uuid is in self.tracked_instances. | |
| 00:10:35 | dansmith | which it will be if it was just running | |
| 00:41:16 | mriedem | dansmith: jaypipes: did you guys sort out the delete allocations on delete thing? b/c i'm pretty sure melwitt has a functional test up for that. | |
| 00:41:27 | jaypipes | mriedem: yeah, I was wrong. | |
| 00:42:12 | mriedem | oh https://review.openstack.org/#/c/470578/ is for local delete | |
| 00:43:11 | mriedem | alright well someone just ping me when the pep8 thing is fixed and i'll +2 | |
| 00:45:51 | jaypipes | mriedem: I'm working on it. | |
| 00:48:27 | openstackgerrit | Michael Still proposed openstack/nova master: Move execs of tee to privsep. https://review.openstack.org/489438 | |
| 00:53:08 | mikal | melwitt / sdague: the +W fell off https://review.openstack.org/#/c/486831 if you're feeling charitable | |
| 00:53:22 | mikal | It would make tonyb's day | |
| 00:53:53 | tonyb | it would at that | |
| 00:54:48 | mriedem | there was no +W on that | |
| 00:55:01 | mriedem | nice try aussies | |
| 00:55:06 | jaypipes | dansmith: why are we setting the availability zone on the build request in these tests? | |
| 00:55:15 | mriedem | jaypipes: to force the host | |
| 00:55:17 | mriedem | at boot time | |
| 00:55:34 | jaypipes | ugh | |
| 00:55:35 | mriedem | it uses the forced_host thing in the api | |
| 00:55:36 | mriedem | yeah | |
| 00:55:41 | mikal | mriedem: curse you meddling kids | |
| 00:55:43 | mriedem | but it makes the order deterministic | |
| 00:56:12 | mriedem | jaypipes: plus we lost all the docs on that forced_host thing with the docs migration i think | |
| 00:58:01 | tonyb | ZOMG I was lied to! | |
| 00:58:44 | mriedem | https://github.com/openstack/openstack-manuals/commit/c21f7bb13ccec63ecf96f5d9d0a9d30f1057e4d5#diff-320133d1ccbd2ba417cd580760c18606 | |
| 00:59:10 | mriedem | stephenfin: ^ do we have plans to import that into the nova docs? | |
| 00:59:19 | mriedem | because this forced_host thing isn't in the compute API reference | |
| 01:00:51 | jaypipes | really wish we used the names source and destination instead of "host" and "other" | |
| 01:01:09 | mikal | jaypipes: that's defeatist | |
| 01:01:24 | mikal | jaypipes: next you'll be complaining that we convey "hairpin enable failed" with a ProcessException | |
| 01:01:36 | jaypipes | heh | |
| 01:02:08 | mriedem | jaypipes: agree on the naming there, we could rename / refactor later too | |
| 01:02:25 | mriedem | plus there is a lot of redundant code in there before we get to the confirm/revert action which could be put into a private common method | |
| 01:02:51 | jaypipes | mriedem: honestly, I just don't understand why there's a need for the whole "do this test in the opposite order" thing. | |
| 01:04:59 | dansmith | jaypipes: because you fail it differently in each case | |
| 01:05:09 | dansmith | jaypipes: keep it on master and disable the skips and you'll see | |
| 01:05:26 | jaypipes | dansmith: but if the whole "availability_zone": "nova:" + initial thing is so deterministic, why the need? | |
| 01:05:38 | dansmith | because going from one compute node to the other, and running their periodics in that order end up with an updated allocation | |
| 01:05:45 | dansmith | in the reverse order, one of them deletes it last | |
| 01:06:00 | dansmith | i.e. one updates, then the other deletes | |
| 01:06:06 | dansmith | vs one deletes then the other updates (creates) | |
| 01:06:45 | jaypipes | with all the stuff we discussed today about ocata computes, there's going to need to be so many friggin conditionals in these code paths that we're introducing more risk than anything else :( | |
| 01:07:00 | dansmith | you mean in order to pass the tests? | |
| 01:07:07 | dansmith | that's exactly why we must have it tested both ways | |
| 01:07:20 | dansmith | ocata to pike _has_ to work, and pike to pike _has_ to work, with no leaks | |
| 01:07:35 | jaypipes | well these tests don't test ocata to pike, only pike to pike | |
| 01:07:38 | dansmith | pike to ocata I can kind of get on board with allowing to be leaky, but it's still not awesome | |
| 01:07:50 | dansmith | jaypipes: exactly, we must have _at least_ this much coverage, | |
| 01:07:59 | dansmith | and we really need one that simulates the pike/ocata split | |
| 01:08:38 | dansmith | we have the grenade multinode case, which helps, but it's not deterministic | |
| 01:08:49 | dansmith | and I think we've shown thus far, that if we don't lay down a test-driven approach to what we expect, | |
| 01:08:55 | jaypipes | I'm in a fix one thing, break ten others place right now :( | |
| 01:08:58 | dansmith | we're going to keep bouncing around between partial solutions | |
| 01:10:23 | dansmith | jaypipes: I'm not sure what the alternative is.. if I can write a test that shows that the code is broken if we end up running things in a particular sequence, that's a problem right? | |
| 01:11:21 | mriedem | we can possibly build on this to make one of the computes' service version be ocata | |
| 01:11:31 | mriedem | to test that code path in the change that checks the min service version | |
| 01:11:32 | jaypipes | dansmith: I'm not disagreeing with you that shit is broken. I just don't know how to fix this without breaking the happy path | |
| 01:11:44 | dansmith | mriedem: we have to simulate the ocata way of managing allocations too | |
| 01:11:46 | mriedem | plus we can do the single node resize to same host testing building on this | |
| 01:12:15 | dansmith | mriedem: i.e. the "always blow away everything without looking first" way | |
| 01:12:18 | mriedem | happy path is all computes are pike isn't it? | |
| 01:12:22 | dansmith | yes | |
| 01:12:27 | mriedem | which this tests | |
| 01:12:30 | dansmith | right | |
| 01:12:42 | jaypipes | no. happy path is non-move operations. | |
| 01:12:49 | dansmith | hah | |
| 01:12:56 | dansmith | that's the beer path | |
| 01:13:10 | mriedem | so, | |
| 01:13:26 | mriedem | what about bfv with shared storage resize to another az? | |
| 01:13:29 | mriedem | will THAT work?! | |
| 01:13:45 | jaypipes | not funny | |
| 01:13:48 | mriedem | uh mixed compute too | |
| 01:14:16 | mriedem | fine fine fine | |
| 01:14:19 | mriedem | s/resize/evacuate/ | |
| 01:14:23 | mriedem | happy? | |
| 01:15:35 | mriedem | so i think the tests as written are a same-version compute good representation of what we expect for confirm and revert, and before dan moved them from the top of the series of fixes, they were passing | |
| 01:15:45 | mriedem | so i think we get that baseline going, and then starting adding in test wrinkles | |
| 01:16:00 | mriedem | i think this is all easier once we have the tests written | |
| 01:16:21 | dansmith | mriedem: meaning they worked on top of the series, which means the series can work atop the tests, right? | |
| 01:16:30 | mriedem | yeah | |
| 01:16:39 | dansmith | that was my reason for doing that before rebasing, to make sure they could work with jay's set | |
| 01:16:49 | mriedem | just like how we do the functional regression bug testaroos | |
| 01:17:05 | dansmith | agreed, we *have* to get this baseline working, and then we can expand the base a bit | |
| 01:17:12 | dansmith | for things like single compute at least | |
| 01:17:31 | jaypipes | I don't disagree with you. | |
| 01:17:38 | jaypipes | just raging that's all. | |