| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-04 | |||
| 17:03:56 | sean-k-mooney | neutron sees the port on the sorce because we have not clean it up yet so when the rollback of the migration happens and we set the host back to the source neturon send the vif-plugged event right away | |
| 17:04:51 | dansmith | oh, it's the rollback case only? | |
| 17:05:28 | artom | dansmith, revert resize is the downstream bug | |
| 17:05:38 | artom | Hrmm, do we actually have 2 problems here? | |
| 17:05:46 | bauzas | gibi: left yet another comment https://review.openstack.org/#/c/623543/39 | |
| 17:05:46 | sean-k-mooney | dansmith: artom is fixing the rollback case. there is another race with sdn controler that send vif-plugged when you bind the port that this will also fix | |
| 17:05:46 | dansmith | okay, I'm even less concerned then :) | |
| 17:05:57 | bauzas | gibi: can't we do all of this on the libvirt driver anyway? | |
| 17:06:12 | dansmith | the rollback case could just not wait for events since it's already on the source, I would think.. that would be a band-aid worth doing I would think | |
| 17:06:39 | bauzas | gibi: but I guess it's not possible because you wanna claim() this | |
| 17:06:59 | gibi | bauzas: looking | |
| 17:07:39 | artom | dansmith, that'd be a weird bit of into to pass down to the virt driver, no? | |
| 17:07:47 | artom | ... cuz we just call spawn there | |
| 17:08:03 | dansmith | we already pass a similar flag in places don't we? | |
| 17:08:04 | mriedem | mdbooth: i don't really remember that, but i'm assuming that timestamp stuff was copied from other scenario tests, and the commit message doesn't really explain the problem with the timestamp stuff | |
| 17:08:12 | mriedem | just "it's bad" isn't very good | |
| 17:08:23 | artom | dansmith, I'd have to check | |
| 17:08:44 | dansmith | maybe I'm thinking about within the driver, but still | |
| 17:08:47 | mriedem | dansmith: upstream it's at least known to be happening in evacuate | |
| 17:08:58 | mriedem | http://status.openstack.org/elastic-recheck/#1813789 | |
| 17:09:01 | gibi | bauzas: yeah we need the info to drive the pci claim | |
| 17:09:08 | dansmith | mriedem: on evacuate, not evacuate and then restart-on-original-host right? | |
| 17:09:13 | sean-k-mooney | ya i was going to say the same. but dan is right we could skip waiting in this case but i kindo of like not have another flag to pass down | |
| 17:09:18 | gibi | bauzas: does compute/manager have access to the ProvideTree? | |
| 17:09:19 | bauzas | gibi: but you understand my concern, right? | |
| 17:09:32 | dansmith | mriedem: the evac case seems ripe for just moving where we wait, but I haven't gone digging | |
| 17:09:39 | sean-k-mooney | the flag would only fix the rollback case. artom change would fix the cisco aci/old odl case | |
| 17:09:39 | bauzas | gibi: that's the problem, I don't think so | |
| 17:09:41 | gibi | bauzas: I understand that we call placement a lot | |
| 17:09:47 | sean-k-mooney | in addtion to rollbacks | |
| 17:10:12 | dansmith | artom: thinking of this: vifs_already_plugged=False, | |
| 17:10:18 | bauzas | gibi: and say you have two pci requests for the same interface... | |
| 17:10:22 | mriedem | gibi: bauzas: pretty sure the compute manager has a handle to the provider tree, | |
| 17:10:26 | bauzas | it's a bit sad to query it twice | |
| 17:10:27 | mriedem | because it has to clear it on sighup | |
| 17:10:41 | mriedem | dansmith: i don't know the answer to that offhand | |
| 17:10:41 | gibi | mriedem: ooh | |
| 17:10:47 | artom | dansmith, ah, OK. It's still only a bandaid tho... | |
| 17:10:59 | mriedem | gibi: bauzas: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L551 | |
| 17:11:02 | artom | dansmith, thanks, that'll allow me to eat :P | |
| 17:11:10 | gibi | bauzas: sure your concern is valid either I would like to use a ProviderTree or add a local cache | |
| 17:12:00 | gibi | mriedem: looking | |
| 17:12:13 | mdbooth | mriedem: The purpose of the timestamp stuff IIUC is to assert that the contents of the volume are preserved across the migration. I personally think it's an important test, but if it's problematic in practise we'd need to be pragmatic, e.g. removing or guarding it. NP if you don't have a strong opinion, lyarwood and I can fling fedoras at each other internally :) | |
| 17:13:26 | gibi | bauzas, mriedem: so here in the compute/manager we only need the name of the RP and we know the UUID of it. So I guess I should be able to use the ProvideTree cache for that | |
| 17:13:26 | mriedem | well, give a justifiable reason in the commit message to remove it | |
| 17:14:27 | mriedem | need another core on the 2.71 novaclient change https://review.openstack.org/#/c/640657/ so we can get that into the final release | |
| 17:14:43 | bauzas | gibi: the cache is a non-public information from the reportclient | |
| 17:14:51 | bauzas | gibi: at the moment, I mean | |
| 17:15:10 | bauzas | mriedem: on it | |
| 17:15:27 | gibi | bauzas: but I call reportclient anyhow so I can call something else on the reportclient that uses the internal cache | |
| 17:16:20 | gibi | bauzas: it I fail then I will add a local cache to my code to query each RP once | |
| 17:16:27 | bauzas | gibi: sure, I'm just saying that if you wanna use its cache, you have to make its attribute accessible | |
| 17:16:44 | bauzas | either thru a getter or whatever | |
| 17:17:08 | gibi | bauzas: I will go with a new method (if not exists already) in reportclient that reads an RP from the cache | |
| 17:17:09 | bauzas | I dislike at most accessing internal attributes in python | |
| 17:17:17 | bauzas | even if that's allowed | |
| 17:17:37 | bauzas | gibi: fine with me | |
| 17:18:01 | bauzas | my point being "please don't just access self.reportclient.provider_tree" or whatever it's named | |
| 17:20:17 | gibi | bauzas: understood | |
| 17:20:25 | bauzas | sorry but I have to bail out | |
| 17:20:28 | bauzas | ++ | |
| 17:24:54 | gibi | bauzas: thanks a lot | |
| 17:24:59 | gibi | bauzas: I will bail soon as well | |
| 17:25:09 | gibi | bauzas: I will finish updating the patch tomorrow morning | |
| 17:29:20 | openstackgerrit | Stephen Finucane proposed openstack/nova master: fakelibvirt: Add ability to generate fake PCI devices https://review.openstack.org/640409 | |
| 17:29:21 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Create PCI tests for NUMA'y tests https://review.openstack.org/640826 | |
| 17:31:41 | openstackgerrit | Chris Friesen proposed openstack/nova master: Flavor extra spec and image properties validation https://review.openstack.org/620706 | |
| 17:32:09 | cfriesen | mriedem: this addresses almost all your comments. will split out the hardware.py stuff as you asked in the next revision | |
| 17:56:59 | openstackgerrit | Chris Dent proposed openstack/nova master: Correct lower-constraints.txt and the related tox job https://review.openstack.org/622972 | |
| 17:57:19 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: qemu: Make disk image conversion dramatically faster https://review.openstack.org/640781 | |
| 17:58:52 | mriedem | cfriesen: wouldn't a deactivate image (even if it doesn't change) on resize fail the resize if the dest host can't download the image (and it's not cached)? | |
| 18:00:08 | cfriesen | mriedem: good question. I seem to remember some scenarios where it copies it from the cache on the source...but let me check. | |
| 18:04:52 | mriedem | maybe if you're using shared storage? | |
| 18:04:55 | mriedem | but not everyone does | |
| 18:05:13 | mriedem | i believe the libvirt driver code falls back to pull from the source if it's not in the dest image cache | |
| 18:05:16 | mriedem | or tries to | |
| 18:05:22 | mriedem | mdbooth probably knows better | |
| 18:18:04 | aspiers | gah, looks like functional tests for https://review.openstack.org/#/c/538498/ failed in the gate | |
| 18:18:23 | aspiers | http://logs.openstack.org/98/538498/22/gate/nova-tox-functional/d76478b/testr_results.html.gz has a bunch of errors like "oslo_config.cfg.NoSuchOptError: no such option sync_on_startup in group [placement_database]" | |
| 18:18:28 | aspiers | does that make sense to anyone | |
| 18:18:30 | aspiers | ? | |
| 18:24:47 | cdent | aspiers: that's interesting, it should default to false | |
| 18:25:07 | cdent | except that's not what the error says, it says the conf option doesn't exist | |
| 18:25:11 | aspiers | right | |
| 18:25:19 | aspiers | totally weird that this should appear in the gate but not in the check | |
| 18:25:41 | cdent | which means the gate doesn't have the right code... | |
| 18:25:56 | sean-k-mooney | aspiers ist it posble that it was usign the inrepo placement? | |
| 18:26:43 | sean-k-mooney | it does not exist here https://github.com/openstack/nova/blob/master/nova/conf/placement.py | |
| 18:27:11 | cdent | sean-k-mooney: the fix should be using an indepdent conf | |
| 18:27:34 | cdent | and according to the failures it's using the external placement | |
| 18:27:47 | cdent | however | |
| 18:27:57 | cdent | it might be that the fixture needs to be tweakd. | |
| 18:28:15 | cdent | aspiers: if you have the time can you try to run the functional tests locally on a clean tox? | |
| 18:28:32 | aspiers | cdent: sure | |
| 18:28:51 | sean-k-mooney | well looking at code search it only exists in placement http://codesearch.openstack.org/?q=sync_on_startup&i=nope&files=&repos= | |
| 18:28:58 | aspiers | right | |
| 18:29:24 | aspiers | cdent: but the zuul check already did that and passed ... | |
| 18:30:10 | cdent | aspiers: yes, but that change (to add that config) merged to placement several minutes ago so maybe it wasn't there in check | |
| 18:31:40 | sean-k-mooney | cdent: if it only merged a few minuts ago its not in pypi | |
| 18:31:56 | sean-k-mooney | the last relase was feb 13th https://pypi.org/project/osc-placement/#history | |