| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-31 | |||
| 23:06:33 | dansmith | ack | |
| 23:27:04 | mikal | Does anyone here understand what causes hairpins to fail to enable? | |
| 23:27:14 | mikal | I'm trying to unravel that code to be less ... processy | |
| 23:35:18 | jaypipes | mikal: sorry, no :( | |
| 23:36:00 | jaypipes | dansmith: sorry, was dinnering. what change did you make to make that test stable? | |
| 23:36:21 | dansmith | jaypipes: made sure to boot the instance on a specific node consistently | |
| 23:36:33 | dansmith | jaypipes: since the allocations are screwed up by one running the periodic before the other, | |
| 23:36:52 | dansmith | the order of boot, migration, and which node runs the periodic last affect the outcome | |
| 23:37:18 | dansmith | jaypipes: so now it runs each test twice, starting and finishing on a different node each time | |
| 23:37:25 | dansmith | since it always runs the periodic in the same order, | |
| 23:37:38 | dansmith | that will cover us against ordering issues if you pass all four tests | |
| 23:37:51 | dansmith | and mriedem is going to work on a single-node variant I think | |
| 23:43:07 | jaypipes | dansmith: k | |
| 23:43:15 | jaypipes | dansmith: so I'm good to pull and rebase? | |
| 23:44:01 | mikal | jaypipes: its ok, this is all nova-net code and might go to heaven soon anyways | |
| 23:44:14 | jaypipes | mikal: heaven? | |
| 23:44:25 | dansmith | jaypipes: cha | |
| 23:44:41 | mikal | jaypipes: would you prefer "is put out to pasture"? | |
| 23:44:42 | dansmith | jaypipes: remember things are upside down for mikal | |
| 23:44:51 | jaypipes | heh | |
| 23:44:54 | mikal | "kicks the bucket" | |
| 23:44:59 | mikal | "goes for a dirt nap" | |
| 23:46:54 | dansmith | jaypipes: oh looks like there is a pep8 error in that test patch, maybe you can fix when you rebase? | |
| 23:47:02 | jaypipes | dansmith: yuppers. | |
| 23:47:22 | dansmith | jaypipes: and note that two of those tests are self.skipTest()ed so you'll want to unskip them as soon as you can | |
| 23:47:31 | jaypipes | yup, got it. | |
| #openstack-nova - 2017-08-01 | |||
| 00:04:29 | jaypipes | dansmith: the reason it's failing is because you're forgetting to run that _run_perioidics() after calling delete on the server. | |
| 00:05:03 | jaypipes | dansmith: on the revert one. | |
| 00:05:32 | dansmith | jaypipes: the reason what is failing? | |
| 00:06:00 | dansmith | tests pass locally for me, and delete should clean up all the allocations without a periodic run, no? | |
| 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 | jaypipes | update is only inventory. | |
| 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: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 | jaypipes | only if the uuid is in self.tracked_instances. | |
| 00:09:44 | dansmith | yeah, calls update_usage() | |
| 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 | |