| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-27 | |||
| 14:11:56 | dansmith | hmm, yeah, I didn't think this was like this | |
| 14:12:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass source vifs to driver.cleanup in _post_live_migration https://review.openstack.org/586568 | |
| 14:12:14 | mriedem | giblet: ^ | |
| 14:12:21 | giblet | mriedem: looking | |
| 14:13:03 | dansmith | I thought if we were running a virt that could do real reboot, we did that and only fell back to the shutdown/restart if not | |
| 14:13:06 | dansmith | but I don't see that | |
| 14:13:56 | melwitt | mriedem: why were you thinking not to use the source vifs throughout the entire method? just wondering | |
| 14:14:26 | mriedem | no particular reason, just wanted to minimize the amount of change, | |
| 14:14:42 | mriedem | but we could just do that at the top rather than get the refreshed nw info cache | |
| 14:15:20 | mriedem | i.e. here https://review.openstack.org/#/c/586568/1/nova/compute/manager.py@6555 | |
| 14:17:00 | mriedem | i can definitely make that change if it makes more sense | |
| 14:17:39 | melwitt | yeah, I'm not 100% sure but it feels like it should be consistent throughout. but I guess that's never guaranteed anyway because neutron events in flight could change the network_info as it goes through the method anyway? | |
| 14:18:01 | mriedem | shouldn't | |
| 14:18:23 | mriedem | an event would be processed separately and shouldn't be able to modify that network_info variable by reference | |
| 14:18:31 | melwitt | oh, yeah, okay | |
| 14:18:32 | mriedem | the instance.info_cache might be updated concurrently, sure | |
| 14:18:42 | mriedem | but we're using the local variable in most places | |
| 14:18:47 | melwitt | yeah | |
| 14:18:49 | mriedem | the versioned notifications will still use instance.info_cache | |
| 14:20:10 | mriedem | left that as a comment so giblet can also ponder it | |
| 14:20:35 | mriedem | i didn't do it in https://review.openstack.org/#/c/586402/ because (1) it was late and (2) i just wanted to get the immediate fire put out | |
| 14:20:48 | melwitt | I guess I could see the rationale in only using the source vifs for the relevant actions because like I think you mentioned, maybe the notifications should reflect the state of the network info cache at the time it was queried | |
| 14:21:03 | melwitt | that's the only other thing network info is used for in that method, I assume? | |
| 14:21:20 | pooja_jadhav | mriedem: hello | |
| 14:22:37 | pooja_jadhav | sean-k-mooney : hello | |
| 14:22:59 | mriedem | and unfilter_instance in the firewall driver, | |
| 14:23:13 | mriedem | i looked at how it was used in the various drivers and it was just getting the mac address off the vifs in one case | |
| 14:23:31 | mriedem | which i don't think should change | |
| 14:23:34 | mriedem | but, | |
| 14:23:57 | mriedem | admittedly, only passing the source vifs from migrate_data to 2 spots indicates tight coupling into knowing exactly what those methods are doing with network_info | |
| 14:23:59 | giblet | mriedem, melwitt: I think having the current network infor send in the notification is the valid thing as we are notifying about current state | |
| 14:24:00 | pooja_jadhav | sean-k-mooney, mriedem: I am trying live migrate and using nfs storage, its failing for "Binding failed for port e973dde6-d68c-4aec-a70d-86dcd81fa11b and host Neha-VirtualBox." | |
| 14:24:21 | mriedem | pooja_jadhav: i can't really help you debug that right now | |
| 14:24:30 | melwitt | giblet, mriedem: I think that makes sense too, the more I think about it | |
| 14:24:40 | mriedem | pooja_jadhav: i'd suggest using something besides devstack if you want a more sophisticated deployment tool for multi-node with live migration, like openstack-ansible | |
| 14:24:57 | pooja_jadhav | mriedem: ok | |
| 14:25:00 | mriedem | melwitt: i'm totally fine with making the generic switch at the top of the method | |
| 14:25:07 | mriedem | i don't like the tight coupling that's in here really | |
| 14:25:16 | mriedem | i just wanted to reduce any exposure to regression | |
| 14:25:40 | mriedem | pooja_jadhav: or look at a nova-live-migration job config and see how it set things up | |
| 14:25:46 | mriedem | but those don't use nfs | |
| 14:25:56 | mriedem | http://logs.openstack.org/02/586402/2/check/nova-live-migration/2db7a54/ | |
| 14:26:19 | mriedem | pooja_jadhav: binding failed means something failed in neutron | |
| 14:26:28 | mriedem | so network is messed up | |
| 14:26:43 | pooja_jadhav | mriedem: Hmm | |
| 14:27:26 | melwitt | mriedem: yeah, I'm thinking I agree with giblet though, that we should leave it the way you have it. let the notifications use the fresh network info and not artificially send source vif. I think the only reason to use source vifs there is if somehow a notifications listener might want to know which vif is actually being acted upon during the actions in the method. hmm. | |
| 14:28:13 | pooja_jadhav | mriedem: But I am not able to see any error logs at neutron side.. thats the problem | |
| 14:28:39 | mriedem | melwitt: giblet: well, only the versioned notifications will use the instance.info_cache, | |
| 14:28:45 | mriedem | the legacy ones would end up using the source vifs | |
| 14:29:15 | melwitt | oh | |
| 14:29:59 | mriedem | anyway, we could always change this later i guess if it causes some other unanticipated problem | |
| 14:30:16 | melwitt | yeah | |
| 14:30:22 | mriedem | let me check to make sure the mac address on the vif is the same between source and dest | |
| 14:30:35 | mriedem | since that's used in the firewall driver to unfilter | |
| 14:31:16 | giblet | mriedem: in the current code the legacy notification uses the local network_info and I guess that is the same as what the versioned gets from instance.info_cache | |
| 14:31:17 | mriedem | source vif "address": "fa:16:3e:cc:ff:66" | |
| 14:31:32 | mriedem | from the cache: "address": "fa:16:3e:cc:ff:66" | |
| 14:31:37 | mriedem | so yeah the mac doesn't change | |
| 14:31:44 | mriedem | giblet: yes | |
| 14:32:24 | giblet | mriedem: then I still think that the current code in your patch is good | |
| 14:34:46 | s10 | What could be done with unrescue/soft reboot QoS issue? Should we use _create_domain_and_network() in that functions instead of simple _create_domain()? Or call plug_vifs()? | |
| 14:42:35 | mriedem | woot ceph shared storage change got through stack.sh and is now running tempest | |
| 14:42:49 | cdent | huzzah | |
| 14:42:58 | dansmith | cdent: did you see my question to efried earlier? | |
| 14:43:07 | cdent | dansmith: no sir, what's up? | |
| 14:43:26 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Retry allocation writes server side https://review.openstack.org/586048 | |
| 14:43:33 | dansmith | [06:36:22] <dansmith>efried: what should happen if I have compute nodes with MISC_SHARES (and thus no DISK_GB inventory)? Should the scheduler receive split allocations from placement with disk on the sharing provider? | |
| 14:43:39 | dansmith | [06:36:46] <dansmith>because I have yet to convince it to do that in a functional test | |
| 14:43:46 | dansmith | cdent: ^ | |
| 14:44:09 | cdent | one sec, let me find something | |
| 14:45:04 | cdent | dansmith: this is current passing: https://github.com/cdent/placecat/blob/master/gabbits/fridge.yaml#L204-L213 | |
| 14:45:34 | cdent | which is an example of some allocations with sharing providers | |
| 14:45:56 | cdent | so in theory it should work, but I'm not clear on what needs to happen on compute-node side to set things up | |
| 14:46:19 | dansmith | cdent: that is asserting what? that one of the providers only has a part of the whole? | |
| 14:46:34 | dansmith | or, two providers in the request | |
| 14:46:37 | mriedem | # but there are two resource providers in that one allocations block | |
| 14:46:44 | cdent | ^ | |
| 14:46:51 | dansmith | yeah | |
| 14:47:05 | dansmith | so, that tells me that a single non-fancy request to placement should return a split allocation | |
| 14:47:07 | mriedem | dansmith: we should know shortly from this ceph patch i have | |
| 14:47:32 | cdent | If we need a specific functional test for something, I'm semi idle right now, so could make something if someone tells me what it needs to be | |
| 14:47:33 | dansmith | and the scheduler is doing a non-fancy request, so it should be getting back a split allocation I guess | |
| 14:47:40 | melwitt | mriedem: in https://review.openstack.org/586568 is that taking care of the live migration rollback scenario? or is that still an open question | |
| 14:47:53 | dansmith | cdent: well, I've tried writing a very hacky one and placement is returning no allocation requests | |
| 14:47:53 | mriedem | melwitt: i looked at rollback and didn't see anything that needed this type of thing | |
| 14:47:59 | melwitt | mriedem: ack | |
| 14:48:13 | cdent | dansmith: do you want to push it up and I'll tune it and you can go review something? | |
| 14:48:29 | mriedem | melwitt: i'd say if we ever go the generic route in _post_live_migration, we'd want to do the same in _rollback_live_migration | |
| 14:48:48 | mriedem | rollback is likely less of an issue b/c if we failed live migration, we won't activate the dest host port bindings and get into this mess | |
| 14:49:21 | [fcandido] | http://eavesdrop.openstack.org/meetings | |
| 14:49:36 | melwitt | ack | |
| 14:49:45 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: funtional test with sharing providers https://review.openstack.org/586589 | |
| 14:49:51 | dansmith | cdent: ^ | |
| 14:49:52 | cdent | on it | |
| 14:50:00 | dansmith | cdent: warning, it's very, uh, forced | |
| 14:50:10 | cdent | ha, noted | |
| 14:50:42 | fried_rice | dansmith/superdan: I haven't caught up on the whole conversation, but you're asking about a compute node that's marked as a sharing provider? | |
| 14:50:45 | dansmith | cdent: attempts to create a provider with disk, associate with the compute host providers, nuke the disk inventory from one and then try to boot and see if we got the shared bit | |
| 14:51:04 | cdent | ✔ | |
| 14:51:07 | dansmith | fried_rice: no, not a compute node marked as sharing, just a compute with no disk because it's associated to a shared disk provider | |