From 245364ece1689853bf33ac25d7319618d064d909 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 9 Apr 2018 16:12:19 -0400 Subject: [PATCH] Update port device_owner when unshelving When we shelve offload an instance, we unplug VIFs, delete the guest from the compute host, etc. The instance is still logically attached to its ports but they aren't connected on any host. When we unshelve an instance, it is scheduled and created on a potentially new host, in a potentially new availability zone. During unshelve, the compute manager will call the setup_instance_network_on_host() method to update the port host binding information for the new host, but was not updating the device_owner, which reflects the availability zone that the instance is in. Because of this, an instance can be created in az1, shelved, and then unshelved in az2 but the port device_owner still says az1 even though the port host binding is for a compute host in az2. This change simply updates the port device_owner when updating the port binding host during unshelve. A TODO is left in the cleanup_instance_network_on_host() method which is called during shelve offload but is currently not implemented. We should unbind ports when shelve offloading, but that is a bit of a bigger change and left for a separate patch since it is not technically needed for this bug fix. Change-Id: Ibd1cbe0e9b5cf3ede542dbf62b1a7d503ba7ea06 Closes-Bug: #1759924 (cherry picked from commit 93802619adde69bf84d26d7231480abb4da07c91) (cherry picked from commit 3e38d1cf16ca29948be499aa37e2494ffd001f12) --- nova/network/neutronv2/api.py | 11 +++++++++++ nova/tests/unit/network/test_neutronv2.py | 17 ++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 4692293cae40..4d1b2cce5c72 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -2571,6 +2571,13 @@ class API(base_api.NetworkAPI): def cleanup_instance_network_on_host(self, context, instance, host): """Cleanup network for specified instance on host.""" + # TODO(mriedem): This should likely be implemented at least for the + # shelve offload operation because the instance is being removed from + # a compute host, VIFs are unplugged, etc, so the ports should also + # be unbound, albeit still logically attached to the instance (for the + # shelve scenario). If _unbind_ports was going to be leveraged here, it + # would have to be adjusted a bit since it currently clears the + # device_id field on the port which is not what we'd want for shelve. pass def _get_pci_devices_from_migration_context(self, migration_context, @@ -2630,6 +2637,10 @@ class API(base_api.NetworkAPI): # same host, there is nothing to do. if p.get(BINDING_HOST_ID) != host: updates[BINDING_HOST_ID] = host + # If the host changed, the AZ could have also changed so we + # need to update the device_owner. + updates['device_owner'] = ( + 'compute:%s' % instance.availability_zone) # NOTE: Before updating the port binding make sure we # remove the pre-migration status from the binding profile if binding_profile.get(MIGRATING_ATTR): diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 30bc659749d9..9277a0b2110f 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -3922,6 +3922,9 @@ class TestNeutronv2WithMock(test.TestCase): # removed since it does not match with the current host. update_port_mock.assert_called_once_with( 'fake-port-1', {'port': {neutronapi.BINDING_HOST_ID: 'my-host', + 'device_owner': + 'compute:%s' % + instance.availability_zone, neutronapi.BINDING_PROFILE: { 'fake_profile': 'fake_data'}}}) @@ -3948,7 +3951,10 @@ class TestNeutronv2WithMock(test.TestCase): # Assert that update_port was called on the port with a # different host but with no binding profile. update_port_mock.assert_called_once_with( - uuids.portid, {'port': {neutronapi.BINDING_HOST_ID: 'my-host'}}) + uuids.portid, {'port': {neutronapi.BINDING_HOST_ID: 'my-host', + 'device_owner': + 'compute:%s' % + instance.availability_zone}}) @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_same_host(self, @@ -3971,7 +3977,9 @@ class TestNeutronv2WithMock(test.TestCase): # Assert that update_port was only called on the port without a host. update_port_mock.assert_called_once_with( 'fake-port-2', - {'port': {neutronapi.BINDING_HOST_ID: instance.host}}) + {'port': {neutronapi.BINDING_HOST_ID: instance.host, + 'device_owner': 'compute:%s' % + instance.availability_zone}}) @mock.patch.object(pci_whitelist.Whitelist, 'get_devspec') @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) @@ -4025,6 +4033,7 @@ class TestNeutronv2WithMock(test.TestCase): 'fake-port-1', {'port': {neutronapi.BINDING_HOST_ID: 'fake-host', + 'device_owner': 'compute:%s' % instance.availability_zone, neutronapi.BINDING_PROFILE: {'pci_slot': '0000:0b:00.1', 'physical_network': 'physnet1', @@ -5233,7 +5242,9 @@ class TestNeutronv2Portbinding(TestNeutronv2Base): ports = {'ports': [{'id': 'test1'}]} self.moxed_client.list_ports(**search_opts).AndReturn(ports) port_req_body = {'port': - {neutronapi.BINDING_HOST_ID: expected_bind_host}} + {neutronapi.BINDING_HOST_ID: expected_bind_host, + 'device_owner': 'compute:%s' % + self.instance['availability_zone']}} self.moxed_client.update_port('test1', port_req_body).AndReturn(None) self.mox.ReplayAll()