| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-30 | |||
| 14:51:51 | efried | Realistically, we don't get to that point if there's a UUID "conflict" - we would have just returned that provider from the first GET in _ensure_resource_provider. | |
| 14:51:57 | mriedem | bauzas: ? | |
| 14:52:38 | cdent | brb | |
| 14:52:49 | mriedem | sdague: i think you will enjoy this https://review.openstack.org/#/c/507693/ | |
| 14:53:20 | efried | cdent jaypipes IMO the first thing we should do is the simplest: raise exceptions instead of returning None in those various paths. | |
| 14:54:03 | efried | Today that 404 will be indicating name conflict 99% of the time. | |
| 14:55:07 | efried | The only excepton would be concurrent create-followed-by-delete. So I'd even go so far as to say 99.99999% | |
| 14:55:09 | cdent | efried: yeah, I agree None is not right | |
| 14:55:13 | jaypipes | efried: go for it. | |
| 14:55:27 | cdent | you'll have to have the fun of figuring out what's up with the api_samples test :( | |
| 14:55:47 | efried | Exactly. jaypipes it looks like this: http://logs.openstack.org/32/523932/2/check/openstack-tox-functional/05f8149/testr_results.html.gz | |
| 14:56:24 | efried | It happens because those test cases are starting the fake compute host twice. Each one seems to be getting its own database. But still just one placement service. | |
| 14:56:57 | sdague | mriedem: nice, those test cleanups are especially nice | |
| 14:57:01 | efried | We create ComputeNode with a name and let the DB assign its UUID. Then we push that guy to placement. So the second time around, it has a different UUID. | |
| 14:57:13 | cdent | efried: i don't think "own database" is the right analysis. something else is going on, like the second node (not host), clobbering the first | |
| 14:57:51 | efried | cdent Okay. FWIW, I pdb'd it, and the compute host definitely ain't there the second time around. | |
| 14:58:42 | efried | I don't really have context on how these fixtures do their business. Who would be an expert there? | |
| 14:59:22 | cdent | sdague might have some insight into efried's problem? | |
| 15:01:55 | mriedem | efried: same cell? | |
| 15:02:00 | mriedem | i.e. it's a single-cell test right? | |
| 15:02:10 | takashin | nova-specs cores, would you take a look at a patch to create specs directory for Rocky https://review.openstack.org/#/c/514101/ ? | |
| 15:02:11 | mriedem | could be an issue with not using fake.set_nodes() and fake.restore_nodes | |
| 15:02:12 | efried | mriedem I don't know that answer. | |
| 15:02:28 | efried | mriedem http://logs.openstack.org/32/523932/2/check/openstack-tox-functional/05f8149/testr_results.html.gz | |
| 15:05:37 | openstackgerrit | Jackie Truong proposed openstack/python-novaclient master: Microversion 2.56 - Add trusted_image_certificates https://review.openstack.org/500396 | |
| 15:06:28 | takashin | a patch to create specs directory for Rocky https://review.openstack.org/#/c/514101/ | |
| 15:09:13 | openstackgerrit | Matthew Booth proposed openstack/nova master: Simplify BDM boot index checking https://review.openstack.org/524208 | |
| 15:10:17 | sdague | what's up? | |
| 15:10:32 | sdague | efried: can you summarize the issue, maybe I can help | |
| 15:11:25 | efried | sdague If report client tries to ensure a RP exists, and gets any error, it returns None (rather than raising an exception) but nobody is checking for that. | |
| 15:11:45 | efried | sdague In this test: http://logs.openstack.org/32/523932/2/check/openstack-tox-functional/05f8149/testr_results.html.gz we're hitting that code path | |
| 15:11:45 | mriedem | efried: these are a single cell test, using the SingleCellSimple | |
| 15:12:04 | mriedem | efried: looking at HypervisorsSampleJson233Tests, it's creating a 2nd service | |
| 15:12:10 | mriedem | but not setting unique nodes | |
| 15:12:42 | efried | sdague As long as we keep the `return None` in there, the test works; but if I raise an exception, the test fails because, as mriedem is saying, it's creating a second service, which creates a second ComputeNode with the same name but different UUID. | |
| 15:12:51 | mriedem | _IntegratedTestBase, the base class, creates a compute service, and the HypervisorsSampleJson test creates a compute service, but i think they have the same node name | |
| 15:13:01 | efried | sdague When we try to create that in placement, we get that 409 because the *name* conflicts. | |
| 15:13:10 | sdague | mriedem: yeh, that's probably something we should prevent on the test side | |
| 15:13:21 | efried | mriedem Yes, they're both called fake-mini. | |
| 15:13:22 | mriedem | so you need to put this in HypervisorsSampleJson when creating the 2nd service: | |
| 15:13:30 | mriedem | fake.set_nodes(['host1']) | |
| 15:13:35 | mriedem | self.addCleanup(fake.restore_nodes) | |
| 15:13:36 | efried | I'm confused as to why it's not `host1` | |
| 15:13:40 | mdbooth | stephenfin: How sure are you of assert(), btw? | |
| 15:13:53 | mriedem | efried: because the fake driver is pullin the node name from CONF.host | |
| 15:13:58 | mriedem | if you don't use fake.set_nodes | |
| 15:14:09 | stephenfin | mdbooth: Not hugely sure. I just recall being told not to use it at some point in the past | |
| 15:14:11 | efried | But as far as I can see, CONF.host *should* be `host1` and `host2` | |
| 15:14:55 | mdbooth | stephenfin: I think I'm using it correctly in that patch, but there may be practical considerations. e.g. if we disable it in CI. | |
| 15:14:57 | mriedem | efried: ala https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L1283 | |
| 15:15:18 | mdbooth | If it's disabled in production, we don't really care. | |
| 15:15:32 | efried | mriedem Okay, thanks, trying... | |
| 15:15:59 | cdent | efried: I can't remember, is the rp ending up named fake-mini or 'host?' ? | |
| 15:16:07 | efried | cdent fake-mini | |
| 15:16:17 | mdbooth | mriedem: Do you happen to remember, btw: all(el + 1 == l[i + 1] for i, el in enumerate(l[:-1])) ? | |
| 15:16:25 | mriedem | melwitt: so i've got this code now http://paste.openstack.org/show/627877/ | |
| 15:16:37 | mdbooth | You chose to add a comment to it for some reason :) | |
| 15:16:44 | mriedem | melwitt: which fails with this http://paste.openstack.org/show/627878/ | |
| 15:16:54 | mriedem | so i'm creating host2 in cell2 but when i try to pull host2 out of cell2, it's not found | |
| 15:17:06 | mriedem | i'm assuming that's because of the CellDatabase fixture defaulting to cell1 | |
| 15:17:15 | mriedem | which is weird since i'm targeting the context specifically to cell2 | |
| 15:17:26 | mriedem | but i can replay this on top of your fixture cleanup patch and see if that fixes it | |
| 15:18:04 | mriedem | mdbooth: ? | |
| 15:18:35 | mriedem | mdbooth: if i added a comment it's because i oculdn't understand ndipanov's code and had to figure it out at one point, and left a comment for posterity | |
| 15:19:02 | mdbooth | mriedem: Not important. I was doing something else and came across it. It made my head hurt :) I submitted this throw-away: https://review.openstack.org/#/c/524208/ | |
| 15:20:09 | mdbooth | I assume this was very X of this patch for some large value of X, and this complexity made sense at some version <X | |
| 15:20:16 | mdbooth | s/very/version/ | |
| 15:21:27 | mdbooth | I was actually trying to work out where we set the boot index of a root disk to 0 if it wasn't done explicitly in the api call | |
| 15:21:30 | mdbooth | still haven't found it | |
| 15:21:50 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add instance action record for shelve_offload instances https://review.openstack.org/523653 | |
| 15:23:31 | openstackgerrit | Merged openstack/nova stable/pike: Set regex flag on ostestr command for osprofiler tests https://review.openstack.org/510226 | |
| 15:23:45 | mriedem | melwitt: that didn't seem to fix it, still can't find host2 in cell2 | |
| 15:24:31 | tetsuro | Hi all, I'd like you to review https://review.openstack.org/#/c/465160/ | |
| 15:26:11 | tetsuro | Currently, VM with NUMA goes into a silent error if the virt_type doesn't support it. | |
| 15:26:21 | tetsuro | So, this patch is to raise an error explicitly if NUMA Topology not supported, | |
| 15:49:06 | mriedem | 2017-11-30 10:47:07,834 ERROR [nova.tests.functional.test_servers] Found host host2 in other cell cell1 | |
| 15:49:06 | mriedem | 2017-11-30 10:47:07,831 ERROR [nova.tests.functional.test_servers] Failed to find host host2 in cell cell2 | |
| 15:49:18 | mriedem | yeah, so for whatever reason, even if i create host2 in cell2, it only ever comes back from cell1 | |
| 15:50:29 | dansmith | mriedem: I commented on that migration sorting deal | |
| 15:51:13 | dansmith | I think I deserve a cookie and a nap now | |
| 15:52:30 | mriedem | heh, thanks, looking | |
| 15:52:34 | mriedem | i did lose some sleep over this last night | |
| 15:52:46 | mriedem | this and that god forsaken stray cat | |
| 15:53:31 | dansmith | hehe | |
| 16:00:12 | mriedem | dansmith: replied, | |
| 16:00:23 | mriedem | this doesn't deal with the global/local marker issue either, | |
| 16:00:24 | efried | mriedem Thanks for the pointer. After adding in the suggested code and finding & fixing the template files, I got the tests passing. <== cdent jaypipes sdague FYI | |
| 16:00:31 | mriedem | but i'm admittedly a bit lost on the local marker logic and how that all works | |
| 16:00:47 | mriedem | dansmith: i mentioned the cell-encoded marker idea too, but not entirely sure how we can use that to fix the local marker thing | |
| 16:01:05 | mriedem | if you look at PS21, he had a near exact copy of the instance list stuff with the global/local marker | |
| 16:01:48 | dansmith | mriedem: right, which is what I called out with him using the marker uuid in all the cells | |
| 16:02:05 | ildikov | mriedem: meeting on #openstack-meeting-cp if you have a few minutes | |
| 16:03:34 | mriedem | ildikov: you must have missed my o/ | |
| 16:03:54 | ildikov | mriedem: you must've done it while I was typing here :) | |
| 16:04:20 | dansmith | mriedem: the cell encoding in the marker could help in a few cases: | |
| 16:04:33 | dansmith | mriedem: if we encode the cell, then we can avoid looking in all cells for the marker uuid as the first step | |
| 16:04:47 | mriedem | yup, i at least understand that part so far | |
| 16:05:36 | dansmith | mriedem: if we encoded the cell *and* the value we're sorting on of the marker, then we could find the marker in each cell to restart the sort from without having to first look up the global marker in the cell it came from | |
| 16:05:51 | dansmith | but I think you said none of those are options without retooling the api stuff anyway, | |
| 16:05:55 | dansmith | so probably not worth it | |