Remove provisioning network ports during tear down

Previously, provisioning network ports were only deleted during a
successful deployment, when the node's network is flipped from the
provisioning network to the tenant network. In many failure scenarios
this code path would not be reached, as well as the case of aborting
deployment. This is not normally an issue, because during instance
termination, nova deletes all ports with a device ID matching the
instance's UUID, including any remaining provisioning ports.

This change removes the provisioning network ports when tearing down a
node to ensure that they are deleted in all scenarios.

Change-Id: I5656802ee04b44247f4a81bb311b5e306a737a4e
Closes-Bug: #1732412
Related-Bug: #1607394

ironic/drivers/modules/agent.py

@@ -418,6 +418,9 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
         task.driver.storage.detach_volumes(task)
         deploy_utils.tear_down_storage_configuration(task)
         task.driver.network.unconfigure_tenant_networks(task)
+        # NOTE(mgoddard): If the deployment was unsuccessful the node may have
+        # ports on the provisioning network which were not deleted.
+        task.driver.network.remove_provisioning_network(task)
 
         return states.DELETED
 

ironic/drivers/modules/iscsi_deploy.py

@@ -492,6 +492,9 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface):
         task.driver.storage.detach_volumes(task)
         deploy_utils.tear_down_storage_configuration(task)
         task.driver.network.unconfigure_tenant_networks(task)
+        # NOTE(mgoddard): If the deployment was unsuccessful the node may have
+        # ports on the provisioning network which were not deleted.
+        task.driver.network.remove_provisioning_network(task)
         return states.DELETED
 
     @METRICS.timer('ISCSIDeploy.prepare')

ironic/tests/unit/drivers/modules/test_agent.py

@@ -301,12 +301,16 @@ class TestAgentDeploy(db_base.DbTestCase):
 
     @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes',
                        autospec=True)
+    @mock.patch.object(flat_network.FlatNetwork,
+                       'remove_provisioning_network',
+                       spec_set=True, autospec=True)
     @mock.patch.object(flat_network.FlatNetwork,
                        'unconfigure_tenant_networks',
                        spec_set=True, autospec=True)
     @mock.patch('ironic.conductor.utils.node_power_action', autospec=True)
     def test_tear_down(self, power_mock,
                        unconfigure_tenant_nets_mock,
+                       remove_provisioning_net_mock,
                        storage_detach_volumes_mock):
         object_utils.create_test_volume_target(
             self.context, node_id=self.node.id)
@@ -317,6 +321,8 @@ class TestAgentDeploy(db_base.DbTestCase):
             self.assertEqual(driver_return, states.DELETED)
             unconfigure_tenant_nets_mock.assert_called_once_with(mock.ANY,
                                                                  task)
+            remove_provisioning_net_mock.assert_called_once_with(mock.ANY,
+                                                                 task)
             storage_detach_volumes_mock.assert_called_once_with(
                 task.driver.storage, task)
         # Verify no volumes exist for new task instances.

ironic/tests/unit/drivers/modules/test_iscsi_deploy.py

@@ -744,12 +744,16 @@ class ISCSIDeployTestCase(db_base.DbTestCase):
 
     @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes',
                        autospec=True)
+    @mock.patch.object(flat_network.FlatNetwork,
+                       'remove_provisioning_network',
+                       spec_set=True, autospec=True)
     @mock.patch.object(flat_network.FlatNetwork,
                        'unconfigure_tenant_networks',
                        spec_set=True, autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
     def test_tear_down(self, node_power_action_mock,
                        unconfigure_tenant_nets_mock,
+                       remove_provisioning_net_mock,
                        storage_detach_volumes_mock):
         obj_utils.create_test_volume_target(
             self.context, node_id=self.node.id)
@@ -761,6 +765,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase):
                                                            states.POWER_OFF)
             unconfigure_tenant_nets_mock.assert_called_once_with(mock.ANY,
                                                                  task)
+            remove_provisioning_net_mock.assert_called_once_with(mock.ANY,
+                                                                 task)
             storage_detach_volumes_mock.assert_called_once_with(
                 task.driver.storage, task)
         # Verify no volumes exist for new task instances.

releasenotes/notes/fix-provisioning-port-cleanup-79ee7930ca206c42.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes an issue whereby in certain deployment failure scenarios a node's
+    provisioning ports are not deleted.  The issue would typically have been
+    masked by nova, which deletes all ports with a device ID matching the
+    instance's UUID during instance termination.  See `bug 1732412
+    <https://bugs.launchpad.net/ironic/+bug/1732412>`_
+    for details.