From b79eecf5de0a89e15c4e326e9d433b9bf7047280 Mon Sep 17 00:00:00 2001
From: M V P Nitesh <m.nitesh@nectechnologies.in>
Date: Fri, 2 Jun 2017 18:10:26 +0530
Subject: [PATCH] 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                   | 11 +++++-
 ironic/tests/unit/conductor/test_manager.py   | 39 +++++++++++++++++++
 .../notes/port_delete-6628b736a1b556f6.yaml   |  5 +++
 3 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/port_delete-6628b736a1b556f6.yaml

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 5ddab21e4e..1610ca3986 100644
--- a/ironic/conductor/manager.py
+++ b/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',
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index a826e342d7..72152aef89 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/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,
diff --git a/releasenotes/notes/port_delete-6628b736a1b556f6.yaml b/releasenotes/notes/port_delete-6628b736a1b556f6.yaml
new file mode 100644
index 0000000000..757c97d476
--- /dev/null
+++ b/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.