Prevents deletion of ports for active nodes

Prevents deletion of ironic port of the node whose provision
state is active. User can delete the port of such node's when
the node's are in the maintenance mode.

Co-Authored-By: Kaifeng Wang <kaifeng.w@gmail.com>
Change-Id: I3c2cc8ec5ddb04d7ab829d1341a045da6dee07e3
Closes-Bug: #1694693

ironic/conductor/manager.py

@@ -2143,7 +2143,8 @@ class ConductorManager(base_manager.BaseConductorManager):
 
     @METRICS.timer('ConductorManager.destroy_port')
     @messaging.expected_exceptions(exception.NodeLocked,
-                                   exception.NodeNotFound)
+                                   exception.NodeNotFound,
+                                   exception.InvalidState)
     def destroy_port(self, context, port):
         """Delete a port.
 
@@ -2158,6 +2159,14 @@ class ConductorManager(base_manager.BaseConductorManager):
                   {'port': port.uuid})
         with task_manager.acquire(context, port.node_id,
                                   purpose='port deletion') as task:
+            node = task.node
+            if ((node.provision_state == states.ACTIVE or node.instance_uuid)
+                and not node.maintenance):
+                    msg = _("Cannot delete the port %(port)s as node "
+                            "%(node)s is active or has "
+                            "instance UUID assigned")
+                    raise exception.InvalidState(msg % {'node': node.uuid,
+                                                        'port': port.uuid})
             port.destroy()
             LOG.info('Successfully deleted port %(port)s. '
                      'The node associated with the port was %(node)s',

ironic/tests/unit/conductor/test_manager.py

@@ -7180,6 +7180,45 @@ class DestroyPortTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         # Compare true exception hidden by @messaging.expected_exceptions
         self.assertEqual(exception.NodeLocked, exc.exc_info[0])
 
+    def test_destroy_port_node_active_state(self):
+        instance_uuid = uuidutils.generate_uuid()
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          instance_uuid=instance_uuid,
+                                          provision_state='active')
+        port = obj_utils.create_test_port(self.context,
+                                          node_id=node.id,
+                                          extra={'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(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'})
+        self.service.destroy_port(self.context, port)
+        self.assertRaises(exception.PortNotFound,
+                          self.dbapi.get_port_by_uuid,
+                          port.uuid)
+
+    def test_destroy_port_with_instance_not_in_active(self):
+        instance_uuid = uuidutils.generate_uuid()
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          instance_uuid=instance_uuid,
+                                          provision_state='deploy failed')
+        port = obj_utils.create_test_port(self.context,
+                                          node_id=node.id)
+        exc = self.assertRaises(messaging.rpc.ExpectedException,
+                                self.service.destroy_port,
+                                self.context, port)
+        self.assertEqual(exception.InvalidState, exc.exc_info[0])
+
 
 @mgr_utils.mock_record_keepalive
 class DestroyPortgroupTestCase(mgr_utils.ServiceSetUpMixin,

releasenotes/notes/port_delete-6628b736a1b556f6.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Prevents deletion of ports for active nodes. It is still possible to
+    delete them after putting the node in the maintenance mode.