From 9e2b3284ead4a548d383cdc3572b5662bd35274a Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Wed, 15 Nov 2017 12:20:32 +0000 Subject: [PATCH] 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 | 3 +++ ironic/drivers/modules/iscsi_deploy.py | 3 +++ ironic/tests/unit/drivers/modules/test_agent.py | 6 ++++++ ironic/tests/unit/drivers/modules/test_iscsi_deploy.py | 6 ++++++ .../fix-provisioning-port-cleanup-79ee7930ca206c42.yaml | 9 +++++++++ 5 files changed, 27 insertions(+) create mode 100644 releasenotes/notes/fix-provisioning-port-cleanup-79ee7930ca206c42.yaml diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 092333b3f9..e4eeb84a3b 100644 --- a/ironic/drivers/modules/agent.py +++ b/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 diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 03b5687085..4e255ad16d 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/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') diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index aecbec1a1b..b29e9e1d30 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/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. diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 72a4ecc489..e3d961e366 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/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. diff --git a/releasenotes/notes/fix-provisioning-port-cleanup-79ee7930ca206c42.yaml b/releasenotes/notes/fix-provisioning-port-cleanup-79ee7930ca206c42.yaml new file mode 100644 index 0000000000..0dc6d71255 --- /dev/null +++ b/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 + `_ + for details.