Block port deletions where vif is present

We must not allow users to be able to delete
ports when a VIF record is present. If they are
able to, then the user risks orphaning ports in
neutron which could result in address depletion
prevent the port from being able to be used again.

Co-Authored-By: Madhuri Kumari <madhuri.kumari@intel.com>
Story: 2006710
Task: 37074
Change-Id: I9f8eaf94a375984ea2f567f0a87c2ed1ed5cb3b7

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',

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)

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,

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)

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.