| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-26 | |||
| 17:05:49 | mriedem | i added some things which made it fail | |
| 17:05:52 | dansmith | because it's checking the straight line | |
| 17:05:56 | mriedem | right, so, | |
| 17:06:06 | dansmith | point being, do you want to just remove that like the other one I nuked, or fix it? | |
| 17:06:08 | melwitt | mriedem: need advice from a release perspective, is this considered critical enough to ask for a FFE to fix the install of the NoOp plugin that was the main feature released in 0.11.1? https://review.openstack.org/585530 | |
| 17:06:12 | mriedem | well, also because it's not changing the image during rebuild so it doesn't go through the scheduler | |
| 17:06:27 | mriedem | dansmith: i was thinking we could fix the test and add the negative wrinkle | |
| 17:06:34 | melwitt | *1.11.0 | |
| 17:06:38 | dansmith | mriedem: yup the other test did the image change | |
| 17:06:46 | mriedem | there were 2 rebuild tests? | |
| 17:06:49 | mriedem | i must have missed that | |
| 17:06:57 | dansmith | yup | |
| 17:06:59 | mriedem | dansmith: oh also, this brings up something i was going to talk with you about, | |
| 17:07:12 | dansmith | mriedem: this was supposed to be the "make sure the completely normal case still works | |
| 17:07:14 | dansmith | " | |
| 17:07:24 | mriedem | which is that if the user attaches a port in a network that doesn't fit the host, and rebuilds with a new image, it will fail | |
| 17:07:33 | mriedem | which is kind of a weird place to fail (on the rebuild but not the port attach) | |
| 17:08:03 | dansmith | yeah, but here are two alternatives: | |
| 17:08:10 | dansmith | 1. don't fail and they wonder why things suck all the sudden | |
| 17:08:12 | dansmith | 2. fail so they know | |
| 17:08:22 | mriedem | melwitt: i'd probably ask in the -release channel, and ask doug | |
| 17:08:28 | dansmith | 1. generates more support traffic I'd assume | |
| 17:09:17 | mriedem | i didn't worry too much about it because i figured we could do #2 later if we wanted | |
| 17:10:03 | melwitt | mriedem: okay thanks | |
| 17:10:58 | dansmith | mriedem: was that "didn't worry" statement aimed at me? | |
| 17:11:38 | mriedem | dansmith: yeah | |
| 17:12:29 | dansmith | mriedem: replies in there | |
| 17:16:42 | dansmith | mriedem: so, I might be out of my element here, but when I un-stub things, I get a failure to attach on the same network | |
| 17:16:58 | dansmith | because of a unique constraint on address.. i.e we can't have two on the same network (that aren't deleted) | |
| 17:17:08 | dansmith | you didn't hit thatbecause you were trying to break it with another network, | |
| 17:17:10 | dansmith | but.. | |
| 17:17:35 | dansmith | presumably there is more mock stuff to do to get me a different attachment on the second go or something | |
| 17:18:01 | dansmith | maybe I can work that out, hang oin | |
| 17:22:21 | dansmith | artom: | |
| 17:22:23 | dansmith | around? | |
| 17:22:39 | dansmith | did we or did we not skip the scheduler entirely when we rebuild to the same image? | |
| 17:22:59 | dansmith | now I'm confused because his tests all seem to assume we'll still hit the scheduler for a same-image rebuild | |
| 17:24:56 | cfriesen_ | that seems wrong | |
| 17:25:25 | dansmith | yeah | |
| 17:25:28 | dansmith | I think it is wrong | |
| 17:27:20 | cfriesen_ | I think the relevent code is rebuild_instance in the conductor | |
| 17:27:30 | cfriesen_ | only calls the scheduler if "host" is not set | |
| 17:30:15 | dansmith | ooh, I think I might've found a bug in the code with this | |
| 17:31:04 | dansmith | we get a 500 on NoValidHost in rebuild | |
| 17:31:45 | artom | dansmith, I thought we did | |
| 17:32:19 | dansmith | although we cast so we won't get that in the real world | |
| 17:32:24 | artom | I'd have to check the code I guess, but a thing that happened was if you rebuilt with the same new image after the first rebuild request with that image was refused, we'd let it pass | |
| 17:32:26 | dansmith | only because of CastAsCall | |
| 17:32:30 | artom | Until we added rolling back of the image_ref | |
| 17:32:51 | dansmith | mriedem: how do you handle that? Just check for 500 in the test or do more monkey work? | |
| 17:39:59 | dansmith | feels wrong to handle NoValidHost in the api when it will only happen in tests | |
| 17:43:37 | mriedem | assert the 500 in the test | |
| 17:43:45 | dansmith | okay | |
| 17:43:51 | mriedem | self.assertRaises(client.OpenStackAPIException, ...) or whatever | |
| 17:43:55 | mriedem | ex.response.status_code | |
| 17:43:59 | dansmith | yup | |
| 17:44:02 | mriedem | the server likely also has a fault on it | |
| 17:44:45 | dansmith | I don't see any other uses of asserting the 500.. did I miss them? | |
| 17:45:08 | mriedem | some tests assert the fault | |
| 17:45:38 | dansmith | I can't get the reason in the 500 unfortunately | |
| 17:45:55 | mriedem | self.assertIn('No valid host', six.text_type(ex)) | |
| 17:46:04 | dansmith | s'not in there | |
| 17:46:17 | dansmith | oh, the exception name is though | |
| 17:46:23 | dansmith | but not the string like normal | |
| 17:46:29 | mriedem | good enough for a test | |
| 17:46:36 | dansmith | ytup | |
| 17:47:27 | mriedem | fwiw i've tried to write new functional tests without CastAsCall as much as possible to avoid stuff like this, and force the functional tests to be written from a user perspective so we have to get faults and stuff like that | |
| 17:48:41 | mriedem | doesn't help you in these b/c of the layers of base class setup | |
| 17:49:11 | dansmith | yeah | |
| 17:49:35 | mriedem | it's like a bean dip after the sour cream at the bottom has gone bad | |
| 17:49:38 | dansmith | that rebuild test is fairly useful now, fwiw | |
| 17:49:44 | openstackgerrit | Dan Smith proposed openstack/nova master: Add additional functional tests for NUMA networks https://review.openstack.org/585385 | |
| 17:49:45 | dansmith | heh | |
| 17:50:00 | dansmith | man I should get a C-A-B on this at this point | |
| 17:52:59 | mriedem | do it | |
| 17:55:19 | openstackgerrit | Dan Smith proposed openstack/nova master: Add additional functional tests for NUMA networks https://review.openstack.org/585385 | |
| 17:55:23 | dansmith | omg I feel so much better | |
| 18:04:34 | Multiple_ | Looking for further review from sometime , please have a look https://review.openstack.org/#/c/563418/ | |
| 18:19:46 | openstackgerrit | Merged openstack/nova master: [placement] Extract base functional test case from test_direct https://review.openstack.org/585777 | |
| 18:22:07 | mriedem | efried: commented on https://review.openstack.org/#/c/583994/1/nova/network/neutronv2/api.py@410 - just let me know which you prefer | |
| 18:28:24 | melwitt | efried: can you remind me please again, is NRP done or are there remaining bits that will go after reshaper? | |
| 18:29:52 | openstackgerrit | karim proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317 | |
| 18:32:18 | mriedem | dansmith: now that you co-own that, are you going to make the updates to both it and the nit cleanup patch below it? | |
| 18:32:25 | mriedem | dansmith: or shall i? | |
| 18:32:39 | efried | mriedem: Responded. | |
| 18:32:49 | dansmith | mriedem: to the functional test one? | |
| 18:32:56 | efried | melwitt: are you tracking a specific nrp blueprint? | |
| 18:32:58 | mriedem | dansmith: yeah | |
| 18:33:41 | dansmith | mriedem: are there comments on the nits one? | |
| 18:34:01 | mriedem | dansmith: yeah a few things that came from later patches in the series before he wrote the nit cleanup patch, | |
| 18:34:06 | mriedem | a couple of which are yours from the api change with the docs | |
| 18:34:20 | mriedem | i can update both if you are busy | |
| 18:34:27 | melwitt | efried: no, was just getting confused about where NRP is as a feature, in general. I think my understanding has been, it's not complete yet, and we're focusing on reshaper first to make sure the path to migration is clear and complete, then finishing NRP after that | |
| 18:34:58 | mriedem | as long as allocation candidates handles NRP it's done isn't it? | |
| 18:35:05 | mriedem | https://blueprints.launchpad.net/nova/+spec/nested-resource-providers-allocation-candidates | |
| 18:35:41 | mriedem | and https://blueprints.launchpad.net/nova/+spec/placement-return-all-resources i guess | |
| 18:36:55 | efried | melwitt: In the sense that reshaper (which is a bp) is part of nrp, we're not done with nrp yet. There's also client-side work to be done on nrp that we haven't really even figured out the scope of. | |
| 18:37:24 | efried | But yes, besides reshaper, the actual placement API ought to be complete as far as nrp goes. | |
| 18:39:21 | mriedem | and with granular requests, | |
| 18:39:35 | mriedem | you should be able to create flavors that request resources which could be found on nested resource providers in the tree right? | |