Merge "Block port deletions where vif is present"

4 files changed, 92 insertions, 14 deletions

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index b96033641..b63d5412e 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -2052,23 +2052,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 836409dad..35e2c94fc 100644
--- a/ironic/conductor/utils.py
+++ b/ironic/conductor/utils.py
@@ -1144,3 +1144,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 fe51497f7..28a250254 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -6348,15 +6348,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 e72935e54..7e3b8ce4a 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)