| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-31 | |||
| 15:05:41 | tbachman | to a unique NIC | |
| 15:05:53 | tbachman | (e.g.. it has vendor/device IDs, but not bus ID) | |
| 15:06:10 | jangutter | sahid: yeah, I last looked at this code during the Pike cycle, and back then, you didn't have the fine-grained ability to pick a specific PCI device aside from the physical_network tag... I think. | |
| 15:07:04 | jangutter | dansmith: Ah, serendipity! But similar code! | |
| 15:13:15 | openstackgerrit | Merged openstack/nova master: Fix formatting for vcpu_pin_set and reserved_huge_pages https://review.openstack.org/587206 | |
| 15:13:23 | openstackgerrit | Merged openstack/nova master: [placement] api-ref: add traits parameter https://review.openstack.org/578048 | |
| 15:13:24 | sean-k-mooney | jangutter: the ablitiy to pass a vendor id/product id via the binding profile was implemented but never merged | |
| 15:14:08 | mriedem | i don't know where we store off that binding_failed vif type, unless it's coming from neutron? | |
| 15:14:20 | dansmith | it's coming from neutron, | |
| 15:14:21 | sean-k-mooney | mriedem: it is | |
| 15:14:24 | dansmith | I didn't think we persist it | |
| 15:14:36 | dansmith | that's why I suggested on the bug to check those logs, | |
| 15:14:49 | dansmith | because I imagine the compute restart thing is just failing to re-bind or plug or whatever | |
| 15:14:53 | mriedem | i'm adding an except handler to _init_instance so we don't block compute restart | |
| 15:14:54 | sean-k-mooney | mriedem: this is coming form the port status update notification | |
| 15:15:07 | mriedem | sean-k-mooney: oh the external event? | |
| 15:15:12 | sean-k-mooney | yep | |
| 15:15:18 | dansmith | mriedem: well, what do we do? start it up without networking? | |
| 15:15:24 | mriedem | no, | |
| 15:15:26 | sean-k-mooney | the same one we were workign around for the multiple port bindings | |
| 15:15:35 | mriedem | we do what we used to do when we'd get VirtualInterfacePlugException for the same thing before the os-vif conversion | |
| 15:15:48 | mriedem | which is not block compute restart and handle the error | |
| 15:15:56 | mriedem | i'll have it up shortly | |
| 15:16:36 | sean-k-mooney | mriedem: we should be binding the vm back to the source host when the livemigration fails. | |
| 15:17:31 | sean-k-mooney | on nova compute restart we could try to fix the vif bindings for the instance | |
| 15:17:43 | mriedem | we don't do anything like that in _rollback_live_migration | |
| 15:18:22 | mriedem | if neutron is telling us that the port's vif type is 'binding_failed', how do we recover to get the proper vif type? | |
| 15:18:54 | dansmith | do we need it? | |
| 15:18:56 | sean-k-mooney | mriedem: it told us the vif type was binding failed for the dest node | |
| 15:18:59 | dansmith | if binding_failed, we stop doing whatever we're doing | |
| 15:19:52 | sean-k-mooney | mriedem: we can rebind for the current node and see if neutron returns a valid vif. | |
| 15:20:27 | sean-k-mooney | dansmith: well we cant generate the xml for an instance with vif_type binding-faild. | |
| 15:20:36 | dansmith | sean-k-mooney: right.. | |
| 15:21:04 | sean-k-mooney | the fact that updating the db with old values and hard rebooting works indicate that neutron was able to still bindng the vif for the current host | |
| 15:22:12 | dansmith | so we _are_ persisting binding_failed in info_cache? | |
| 15:22:43 | sean-k-mooney | dansmith: looking at the bug report yes | |
| 15:22:53 | dansmith | we really shouldn't be doing that | |
| 15:23:35 | sean-k-mooney | well its technically a valid vif type but we could filter the update processing to drop it | |
| 15:23:40 | dansmith | oh yeah I see I skimmed over that paragraph | |
| 15:23:59 | dansmith | sean-k-mooney: it's not a valid vif_type.. you mean it's a valid value for that field or something :) | |
| 15:24:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle binding_failed vif plug errors on compute restart https://review.openstack.org/587498 | |
| 15:24:26 | mriedem | ^ adds the wrinkle for the same way we handled this before but now with the os-vif conversion way this fails | |
| 15:24:31 | sean-k-mooney | dansmith: i mean the neutron port binding extention definds it at a valid value for vif_type | |
| 15:24:38 | mriedem | figuring out how to deal with the actual binding_failed vif type is a follow up i think | |
| 15:24:52 | sean-k-mooney | dansmith: https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/portbindings.py#L86 | |
| 15:25:01 | dansmith | sean-k-mooney: yeah, that's what I mean | |
| 15:25:08 | dansmith | but it's not an actual type of vif | |
| 15:25:16 | mriedem | unbound could also screw us | |
| 15:25:27 | dansmith | it also seems really wrong that they ever used that field as a signal for failure | |
| 15:25:46 | sean-k-mooney | dansmith: yes like unbound it indicate that its not valid for this port to be used with a guest in its current state | |
| 15:25:54 | mriedem | sean-k-mooney: so i'm not sure what external event we're getting for this | |
| 15:26:14 | mriedem | because in queens we wouldn't be waiting for a vif plugged event so we'd just log "Received unexpected event %(event)s for ..." | |
| 15:26:17 | mriedem | and not update our cache | |
| 15:26:25 | dansmith | network-changed? | |
| 15:26:38 | mriedem | maybe, but not sure why | |
| 15:26:44 | mriedem | unless the port status changes? | |
| 15:26:51 | mriedem | if so, then yeah i think it's network-changed | |
| 15:26:58 | dansmith | LOG.debug('Refreshing instance network info cache due to ' | |
| 15:26:58 | dansmith | 'event %s.', event.key, instance=instance) | |
| 15:27:02 | dansmith | should see that if so | |
| 15:27:34 | mriedem | right, so in that case, we are getting fresh port info from neutron, | |
| 15:27:42 | mriedem | and if it's telling us the vif type is 'binding_failed' then i'm not sure what we can do | |
| 15:27:45 | sean-k-mooney | mriedem: the port status changed because we changed the host_id when we tried to migrate it | |
| 15:28:22 | mriedem | sean-k-mooney: i don't think that happens in pre_live_migration | |
| 15:28:25 | mriedem | that happens in post | |
| 15:29:05 | mriedem | pre_live_migration would, if we got past vif plugging, set the migrating_to field in the port binding profile | |
| 15:29:07 | sean-k-mooney | mriedem: we call plug vifs on the dest in pre livemigrate no? pre_live_migration | |
| 15:29:10 | mriedem | but that doesn't change the port's host binding | |
| 15:29:13 | mriedem | yes | |
| 15:29:17 | openstackgerrit | Merged openstack/nova stable/queens: Disable limits if force_hosts or force_nodes is set https://review.openstack.org/584616 | |
| 15:29:17 | dansmith | mriedem: yeah, I guess I was thinking more of a single bind failing meaning we shouldn't persist that, but I guess we're doing a wholesale update and it's telling us failed for that one | |
| 15:29:23 | openstackgerrit | Merged openstack/nova stable/queens: Add functional regressions tests for server_group_members OverQuota https://review.openstack.org/581845 | |
| 15:29:34 | dansmith | which I dunno, seems dumb for us to overwrite our data with that | |
| 15:29:43 | mriedem | dansmith: yeah that's what i said in the bug | |
| 15:29:58 | mriedem | if we can detect that type is changing from "ovs" to "binding_failed", don't save that n stuff | |
| 15:30:17 | mriedem | "here let me corrupt your internal state for you, you're welcome" | |
| 15:31:39 | sean-k-mooney | well the fact we are storing the vif_type persently in the cache is likely a bug in itself. | |
| 15:31:46 | mriedem | i don't have a setup for testing this, but it seems it would be easy enough to recreate by just stopping the neutron agent on a dest host and trying to live migrate to that host | |
| 15:32:03 | dansmith | sean-k-mooney: why? | |
| 15:32:17 | mriedem | that's the only way we know how to plug/unplug the thing, | |
| 15:32:26 | mriedem | similar to caching off the volume type in the bdm.connection_info | |
| 15:32:32 | dansmith | unless we always hit neutron always, but that's the point of the cache | |
| 15:32:37 | sean-k-mooney | well the fact that we dont ask neutron to bind the port again wen we start the instance and fail becasue we have a chagned bad value seams wrong | |
| 15:33:16 | dansmith | mriedem: will your "just reboot the instance" thing re-bind to the existing host? | |
| 15:33:30 | mriedem | if neutron gives us the proper information it should, | |
| 15:33:31 | sean-k-mooney | i dont think haveing the cache is a bad thing but the fact we dont retry with uncached data is what i thnk is weird | |
| 15:33:32 | mriedem | which leads me to https://review.openstack.org/#/c/587498/1/nova/compute/manager.py@956 | |
| 15:33:47 | mriedem | ^ is the cached info, but we could change that to get a fresh copy from neutron, | |
| 15:34:01 | mriedem | but that's why i'm asking if neutron is sending us binding_failed continually b/c if so, that won't help | |
| 15:35:28 | sean-k-mooney | mriedem: without the neutron logs its hard to tell but it seam more likely to me that the newly deoployed host is misconfigured then neutron is broken for all nodes | |
| 15:35:45 | dansmith | yes, obviously the root cause is in the neutron logs, | |
| 15:35:55 | mriedem | right, | |
| 15:36:03 | mriedem | but what gets us back to getting a real vif type from neutron? | |
| 15:36:05 | dansmith | but presumably it was bound before and just restoring the info makes it work on the original host again, | |
| 15:36:06 | dansmith | so presumably if we re-bind on the source it will work | |
| 15:36:07 | mriedem | re-plugging on the source? | |
| 15:36:17 | dansmith | mriedem: well, restarting the compute node for sure :) | |
| 15:36:26 | dansmith | just not positive about the reboot case | |
| 15:36:32 | dansmith | although I think it probably will | |
| 15:36:51 | mriedem | re-plug fails on the source when restarting the compute | |