diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 8ba0015c79..e6ce9ff2bb 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -2043,23 +2043,21 @@ class ConductorManager(base_manager.BaseConductorManager): :raises: NodeLocked if node is locked by another conductor. :raises: NodeNotFound if the node associated with the port does not exist. + :raises: InvalidState if a vif is still attached to the port. """ LOG.debug('RPC destroy_port called for port %(port)s', {'port': port.uuid}) with task_manager.acquire(context, port.node_id, purpose='port deletion') as task: - node = task.node - vif = task.driver.network.get_current_vif(task, port) - if ((node.provision_state == states.ACTIVE or node.instance_uuid) - and not node.maintenance and vif): - msg = _("Cannot delete the port %(port)s as node " - "%(node)s is active or has " - "instance UUID assigned or port is bound " - "to vif %(vif)s") - raise exception.InvalidState(msg % {'node': node.uuid, - 'port': port.uuid, - 'vif': vif}) + vif, vif_use = utils.get_attached_vif(port) + if vif: + msg = _("Cannot delete the port %(port)s as it is bound " + "to VIF %(vif)s for %(use)s use.") + raise exception.InvalidState( + msg % {'port': port.uuid, + 'vif': vif, + 'use': vif_use}) port.destroy() LOG.info('Successfully deleted port %(port)s. ' 'The node associated with the port was %(node)s', diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index e537219c63..75b90486ac 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1134,3 +1134,30 @@ def hash_password(password=''): :param value: Value to be hashed """ return crypt.crypt(password, make_salt()) + + +def get_attached_vif(port): + """Get any attached vif ID for the port + + :param port: The port object upon which to check for a vif + record. + :returns: Returns a tuple of the vif if found and the use of + the vif in the form of a string, 'tenant', 'cleaning' + 'provisioning', 'rescuing'. + :raises: InvalidState exception upon finding a port with a + transient state vif on the port. + """ + + tenant_vif = port.internal_info.get('tenant_vif_port_id') + if tenant_vif: + return (tenant_vif, 'tenant') + clean_vif = port.internal_info.get('cleaning_vif_port_id') + if clean_vif: + return (clean_vif, 'cleaning') + prov_vif = port.internal_info.get('provisioning_vif_port_id') + if prov_vif: + return (prov_vif, 'provisioning') + rescue_vif = port.internal_info.get('rescuing_vif_port_id') + if rescue_vif: + return (rescue_vif, 'rescuing') + return (None, None) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 7011b56e98..fd4b45fd44 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -6311,15 +6311,29 @@ class DestroyPortTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.context, port) self.assertEqual(exception.InvalidState, exc.exc_info[0]) - def test_destroy_port_node_active_and_maintenance(self): + def test_destroy_port_node_active_and_maintenance_vif_present(self): + instance_uuid = uuidutils.generate_uuid() + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + instance_uuid=instance_uuid, + provision_state='active', + maintenance=True) + port = obj_utils.create_test_port( + self.context, + node_id=node.id, + internal_info={'tenant_vif_port_id': 'fake-id'}) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.destroy_port, + self.context, port) + self.assertEqual(exception.InvalidState, exc.exc_info[0]) + + def test_destroy_port_node_active_and_maintenance_no_vif(self): instance_uuid = uuidutils.generate_uuid() node = obj_utils.create_test_node(self.context, driver='fake-hardware', instance_uuid=instance_uuid, provision_state='active', maintenance=True) port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra={'vif_port_id': 'fake-id'}) + node_id=node.id) self.service.destroy_port(self.context, port) self.assertRaises(exception.PortNotFound, self.dbapi.get_port_by_uuid, diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 2ec5649c6e..8a5f7a33c0 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -2051,3 +2051,42 @@ class AgentTokenUtilsTestCase(tests_base.TestCase): conductor_utils.is_agent_token_supported('6.2.1')) self.assertFalse( conductor_utils.is_agent_token_supported('6.0.0')) + + +class GetAttachedVifTestCase(db_base.DbTestCase): + + def setUp(self): + super(GetAttachedVifTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context, + driver='fake-hardware') + self.port = obj_utils.get_test_port(self.context, + node_id=self.node.id) + + def test_get_attached_vif_none(self): + vif, use = conductor_utils.get_attached_vif(self.port) + self.assertIsNone(vif) + self.assertIsNone(use) + + def test_get_attached_vif_tenant(self): + self.port.internal_info = {'tenant_vif_port_id': '1'} + vif, use = conductor_utils.get_attached_vif(self.port) + self.assertEqual('1', vif) + self.assertEqual('tenant', use) + + def test_get_attached_vif_provisioning(self): + self.port.internal_info = {'provisioning_vif_port_id': '1'} + vif, use = conductor_utils.get_attached_vif(self.port) + self.assertEqual('1', vif) + self.assertEqual('provisioning', use) + + def test_get_attached_vif_cleaning(self): + self.port.internal_info = {'cleaning_vif_port_id': '1'} + vif, use = conductor_utils.get_attached_vif(self.port) + self.assertEqual('1', vif) + self.assertEqual('cleaning', use) + + def test_get_attached_vif_rescuing(self): + self.port.internal_info = {'rescuing_vif_port_id': '1'} + vif, use = conductor_utils.get_attached_vif(self.port) + self.assertEqual('1', vif) + self.assertEqual('rescuing', use) diff --git a/releasenotes/notes/prevent-ports-with-vif-deletion-3edac3df5aa1becf.yaml b/releasenotes/notes/prevent-ports-with-vif-deletion-3edac3df5aa1becf.yaml new file mode 100644 index 0000000000..3d7fed1e23 --- /dev/null +++ b/releasenotes/notes/prevent-ports-with-vif-deletion-3edac3df5aa1becf.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes logic that is applied to port deletions to also consider the + presence of a VIF attachment record, which should be removed before + attempting to delete the node. Failure to do so can result in + erroneous records in the Networking Service.