From 4eef0fe6354304b4639a3b635e4955457188e4ce Mon Sep 17 00:00:00 2001 From: Arnaud Morin Date: Thu, 18 Aug 2022 17:52:58 +0200 Subject: Unbind port when offloading a shelved instance When offloading a shelved instance, the compute needs to remove the binding so the port will appear as "unbound" in neutron. Closes-Bug: 1983471 Change-Id: Ia49271b126870c7936c84527a4c39ab96b6c5ea7 Signed-off-by: Arnaud Morin --- nova/compute/manager.py | 3 ++ nova/network/neutron.py | 21 +++++++++--- .../functional/libvirt/test_pci_sriov_servers.py | 25 +++----------- nova/tests/functional/test_servers.py | 38 ++++++++++++++++++++++ nova/tests/unit/compute/test_shelve.py | 6 +++- ...removed-shelved-offloaded-f1772a64be007b24.yaml | 6 ++++ 6 files changed, 73 insertions(+), 26 deletions(-) create mode 100644 releasenotes/notes/port-binding-removed-shelved-offloaded-f1772a64be007b24.yaml diff --git a/nova/compute/manager.py b/nova/compute/manager.py index f25d037c50..8de5422c6d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -6768,6 +6768,9 @@ class ComputeManager(manager.Manager): current_power_state = self._get_power_state(instance) network_info = self.network_api.get_instance_nw_info(context, instance) + ports_id = [vif['id'] for vif in network_info] + self.network_api.unbind_ports(context, ports_id, detach=False) + block_device_info = self._get_instance_block_device_info(context, instance, bdms=bdms) diff --git a/nova/network/neutron.py b/nova/network/neutron.py index 27e7d06455..6c9f19f010 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -612,10 +612,22 @@ class API: raise exception.ExternalNetworkAttachForbidden( network_uuid=net['id']) + def unbind_ports(self, context, ports, detach=True): + """Unbind and detach the given ports by clearing their + device_owner and dns_name. + The device_id will also be cleaned if detach=True. + + :param context: The request context. + :param ports: list of port IDs. + """ + neutron = get_client(context) + self._unbind_ports(context, ports, neutron, detach=detach) + def _unbind_ports(self, context, ports, - neutron, port_client=None): - """Unbind the given ports by clearing their device_id, + neutron, port_client=None, detach=True): + """Unbind and detach the given ports by clearing their device_owner and dns_name. + The device_id will also be cleaned if detach=True. :param context: The request context. :param ports: list of port IDs. @@ -638,11 +650,12 @@ class API: port_req_body: ty.Dict[str, ty.Any] = { 'port': { - 'device_id': '', - 'device_owner': '', constants.BINDING_HOST_ID: None, } } + if detach: + port_req_body['port']['device_id'] = '' + port_req_body['port']['device_owner'] = '' try: port = self._show_port( context, port_id, neutron_client=neutron, diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py index b32d165e10..072eb2c212 100644 --- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py +++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py @@ -1483,10 +1483,7 @@ class VDPAServersTest(_PCIServersWithMigrationTestBase): # to any host but should still be owned by the vm port = self.neutron.show_port(vdpa_port['id'])['port'] self.assertEqual(server['id'], port['device_id']) - # FIXME(sean-k-mooney): we should be unbinding the port from - # the host when we shelve offload but we don't today. - # This is unrelated to vdpa port and is a general issue. - self.assertEqual(hostname, port['binding:host_id']) + self.assertIsNone(port['binding:host_id']) self.assertIn('binding:profile', port) self.assertIsNone(server['OS-EXT-SRV-ATTR:hypervisor_hostname']) self.assertIsNone(server['OS-EXT-SRV-ATTR:host']) @@ -1508,9 +1505,7 @@ class VDPAServersTest(_PCIServersWithMigrationTestBase): self.assertPCIDeviceCounts(hostname, total=num_pci, free=num_pci) self.assertIsNone(server['OS-EXT-SRV-ATTR:hypervisor_hostname']) port = self.neutron.show_port(vdpa_port['id'])['port'] - # FIXME(sean-k-mooney): shelve offload should unbind the port - # self.assertEqual('', port['binding:host_id']) - self.assertEqual(hostname, port['binding:host_id']) + self.assertIsNone(port['binding:host_id']) server = self._unshelve_server(server) self.assertPCIDeviceCounts(hostname, total=num_pci, free=num_pci - 2) @@ -1541,9 +1536,7 @@ class VDPAServersTest(_PCIServersWithMigrationTestBase): self.assertPCIDeviceCounts(source, total=num_pci, free=num_pci) self.assertIsNone(server['OS-EXT-SRV-ATTR:hypervisor_hostname']) port = self.neutron.show_port(vdpa_port['id'])['port'] - # FIXME(sean-k-mooney): shelve should unbind the port - # self.assertEqual('', port['binding:host_id']) - self.assertEqual(source, port['binding:host_id']) + self.assertIsNone(port['binding:host_id']) # force the unshelve to the other host self.api.put_service( @@ -2889,17 +2882,7 @@ class RemoteManagedServersTest(_PCIServersWithMigrationTestBase): port = self.neutron.show_port(uuids.dpu_tunnel_port)['port'] self.assertIn('binding:profile', port) - self.assertEqual( - { - 'pci_vendor_info': '15b3:101e', - 'pci_slot': '0000:82:00.4', - 'physical_network': None, - 'pf_mac_address': '52:54:00:1e:59:02', - 'vf_num': 3, - 'card_serial_number': 'MT0000X00002', - }, - port['binding:profile'], - ) + self.assertEqual({}, port['binding:profile']) def test_suspend(self): self.start_compute() diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index d1ab84aa7b..4c9ab739d4 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -6522,3 +6522,41 @@ class PortAndFlavorAccelsServerCreateTest(AcceleratorServerBase): binding_profile = neutronapi.get_binding_profile(updated_port) self.assertNotIn('arq_uuid', binding_profile) self.assertNotIn('pci_slot', binding_profile) + + +class PortBindingShelvedServerTest(integrated_helpers._IntegratedTestBase): + """Tests for servers with ports.""" + + compute_driver = 'fake.SmallFakeDriver' + + def setUp(self): + super(PortBindingShelvedServerTest, self).setUp() + self.flavor_id = self._create_flavor( + disk=10, ephemeral=20, swap=5 * 1024) + + def test_shelve_offload_with_port(self): + # Do not wait before offloading + self.flags(shelved_offload_time=0) + + server = self._create_server( + flavor_id=self.flavor_id, + networks=[{'port': self.neutron.port_1['id']}]) + + port = self.neutron.show_port(self.neutron.port_1['id'])['port'] + + # Assert that the port is actually associated to the instance + self.assertEqual(port['device_id'], server['id']) + self.assertEqual(port['binding:host_id'], 'compute') + self.assertEqual(port['binding:status'], 'ACTIVE') + + # Do shelve + server = self._shelve_server(server, 'SHELVED_OFFLOADED') + + # Retrieve the updated port + port = self.neutron.show_port(self.neutron.port_1['id'])['port'] + + # Assert that the port is still associated to the instance + # but the binding is not on the compute anymore + self.assertEqual(port['device_id'], server['id']) + self.assertIsNone(port['binding:host_id']) + self.assertNotIn('binding:status', port) diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index c939b927f1..705ca2f34f 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -209,6 +209,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): instance = self._shelve_offload(clean_shutdown=False) mock_power_off.assert_called_once_with(instance, 0, 0) + @mock.patch.object(neutron_api.API, 'unbind_ports') @mock.patch.object(compute_utils, 'EventReporter') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @mock.patch.object(nova.compute.manager.ComputeManager, @@ -225,7 +226,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): def _shelve_offload(self, mock_notify, mock_notify_instance_usage, mock_get_power_state, mock_update_resource_tracker, mock_delete_alloc, mock_terminate, mock_get_bdms, - mock_event, clean_shutdown=True): + mock_event, mock_unbind_ports, clean_shutdown=True): host = 'fake-mini' instance = self._create_fake_instance_obj(params={'host': host}) instance.task_state = task_states.SHELVING @@ -278,6 +279,9 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): instance.uuid, graceful_exit=False) + mock_unbind_ports.assert_called_once_with( + self.context, mock.ANY, detach=False) + return instance @mock.patch('nova.compute.utils.' diff --git a/releasenotes/notes/port-binding-removed-shelved-offloaded-f1772a64be007b24.yaml b/releasenotes/notes/port-binding-removed-shelved-offloaded-f1772a64be007b24.yaml new file mode 100644 index 0000000000..7e2dccbbf4 --- /dev/null +++ b/releasenotes/notes/port-binding-removed-shelved-offloaded-f1772a64be007b24.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + [`bug 1983471 `_] + When offloading a shelved instance, the compute will now remove the + binding so instance ports will appear as "unbound" in neutron. -- cgit v1.2.1