From 4920cc26a3581664e15e239ce8f8d75334cf760e Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Thu, 16 May 2019 13:01:24 +0200
Subject: [PATCH] Allow disabling clean up on deployment failure

Change-Id: Iaa9e7ec1c2cd2b71047388e51255ec69d52d6a83
---
 metalsmith/_cmd.py                               |  5 ++++-
 metalsmith/_provisioner.py                       | 16 ++++++++++------
 metalsmith/test/test_cmd.py                      |  8 +++++++-
 metalsmith/test/test_provisioner.py              | 16 ++++++++++++++++
 .../notes/no-clean-up-03bfaee6bbb112fb.yaml      |  5 +++++
 5 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/no-clean-up-03bfaee6bbb112fb.yaml

diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py
index e567149..da46454 100644
--- a/metalsmith/_cmd.py
+++ b/metalsmith/_cmd.py
@@ -80,7 +80,8 @@ def _do_deploy(api, args, formatter):
                                   swap_size_mb=args.swap_size,
                                   config=config,
                                   netboot=args.netboot,
-                                  wait=wait)
+                                  wait=wait,
+                                  clean_up_on_failure=not args.no_clean_up)
     formatter.deploy(instance)
 
 
@@ -174,6 +175,8 @@ def _parse_args(args, config):
     deploy.add_argument('--user-name', help='Name of the admin user to create')
     deploy.add_argument('--passwordless-sudo', action='store_true',
                         help='allow password-less sudo for the user')
+    deploy.add_argument('--no-clean-up', help='Prevent clean up on failure',
+                        action='store_true')
 
     undeploy = subparsers.add_parser('undeploy')
     undeploy.set_defaults(func=_do_undeploy)
diff --git a/metalsmith/_provisioner.py b/metalsmith/_provisioner.py
index 2888861..5adda64 100644
--- a/metalsmith/_provisioner.py
+++ b/metalsmith/_provisioner.py
@@ -319,6 +319,9 @@ class Provisioner(_utils.GetNodeMixin):
             :meth:`reserve_node` for that.
         :param wait: How many seconds to wait for the deployment to finish,
             None to return immediately.
+        :param clean_up_on_failure: If True, then on failure the node is
+            cleared of instance information, VIFs are detached, created ports
+            and allocations are deleted.
         :return: :py:class:`metalsmith.Instance` object with the current
             status of provisioning. If ``wait`` is not ``None``, provisioning
             is already finished.
@@ -386,12 +389,13 @@ class Provisioner(_utils.GetNodeMixin):
         except Exception:
             exc_info = sys.exc_info()
 
-            try:
-                LOG.error('Deploy attempt failed on node %s, cleaning up',
-                          _utils.log_res(node))
-                self._clean_up(node, nics=nics)
-            except Exception:
-                LOG.exception('Clean up failed')
+            if clean_up_on_failure:
+                try:
+                    LOG.error('Deploy attempt failed on node %s, cleaning up',
+                              _utils.log_res(node))
+                    self._clean_up(node, nics=nics)
+                except Exception:
+                    LOG.exception('Clean up failed')
 
             six.reraise(*exc_info)
 
diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py
index 5dd9a77..2b4fc38 100644
--- a/metalsmith/test/test_cmd.py
+++ b/metalsmith/test/test_cmd.py
@@ -56,7 +56,8 @@ class TestDeploy(testtools.TestCase):
                                   swap_size_mb=None,
                                   config=mock.ANY,
                                   netboot=False,
-                                  wait=1800)
+                                  wait=1800,
+                                  clean_up_on_failure=True)
         provision_defaults.update(provision_args)
 
         _cmd.main(args)
@@ -502,6 +503,11 @@ class TestDeploy(testtools.TestCase):
                 '--swap-size', '4096', '--resource-class', 'compute']
         self._check(mock_pr, args, {}, {'swap_size_mb': 4096})
 
+    def test_no_clean_up(self, mock_pr):
+        args = ['deploy', '--network', 'mynet', '--image', 'myimg',
+                '--resource-class', 'compute', '--no-clean-up']
+        self._check(mock_pr, args, {}, {'clean_up_on_failure': False})
+
 
 @mock.patch.object(_provisioner, 'Provisioner', autospec=True)
 @mock.patch.object(_cmd.os_config, 'OpenStackConfig', autospec=True)
diff --git a/metalsmith/test/test_provisioner.py b/metalsmith/test/test_provisioner.py
index 716825a..e8d9ca4 100644
--- a/metalsmith/test/test_provisioner.py
+++ b/metalsmith/test/test_provisioner.py
@@ -1074,6 +1074,22 @@ abcd  image
             calls, any_order=True)
         self.api.baremetal.delete_allocation.assert_called_once_with('id2')
 
+    def test_deploy_failure_no_cleanup(self):
+        self.node.allocation_id = 'id2'
+        self.api.baremetal.set_node_provision_state.side_effect = (
+            RuntimeError('boom'))
+        self.assertRaisesRegex(RuntimeError, 'boom',
+                               self.pr.provision_node, self.node,
+                               'image', [{'network': 'n1'}, {'port': 'p1'}],
+                               wait=3600, clean_up_on_failure=False)
+
+        self.assertEqual(1, self.api.baremetal.update_node.call_count)
+        self.assertFalse(
+            self.api.baremetal.wait_for_nodes_provision_state.called)
+        self.assertFalse(self.api.network.delete_port.called)
+        self.assertFalse(self.api.baremetal.detach_vif_from_node.called)
+        self.assertFalse(self.api.baremetal.delete_allocation.called)
+
     def test_port_creation_failure(self):
         self.api.network.create_port.side_effect = RuntimeError('boom')
         self.assertRaisesRegex(RuntimeError, 'boom',
diff --git a/releasenotes/notes/no-clean-up-03bfaee6bbb112fb.yaml b/releasenotes/notes/no-clean-up-03bfaee6bbb112fb.yaml
new file mode 100644
index 0000000..4c82a67
--- /dev/null
+++ b/releasenotes/notes/no-clean-up-03bfaee6bbb112fb.yaml
@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Allows disabling clean up on failure via the new ``clean_up_on_failure``
+    argument and ``--no-clean-up`` flag.