From ed59cfef849f003d8b0bb9a5efb3b41fbc778c9c Mon Sep 17 00:00:00 2001 From: Jeremy Freudberg Date: Tue, 5 Jun 2018 11:45:18 -0400 Subject: [PATCH] Improve force delete * Drop use of stack abandon: just use a regular delete instead * Return stack name from force delete API call Change-Id: I33ee7323fade1b237957abb8f7c79b87eb20148f --- ...force-delete-changes-2e0881a99742c339.yaml | 6 +++++ sahara/api/v2/clusters.py | 8 +++++- sahara/service/heat/heat_engine.py | 13 ++++----- sahara/tests/unit/service/test_engine.py | 27 +++++-------------- .../tests/unit/utils/openstack/test_heat.py | 16 +++++++---- sahara/utils/openstack/heat.py | 9 +++---- 6 files changed, 40 insertions(+), 39 deletions(-) create mode 100644 releasenotes/notes/force-delete-changes-2e0881a99742c339.yaml diff --git a/releasenotes/notes/force-delete-changes-2e0881a99742c339.yaml b/releasenotes/notes/force-delete-changes-2e0881a99742c339.yaml new file mode 100644 index 0000000000..9cd4186f7d --- /dev/null +++ b/releasenotes/notes/force-delete-changes-2e0881a99742c339.yaml @@ -0,0 +1,6 @@ +--- +features: + - The behavior of force deletion of clusters (APIv2) has changed. + Stack-abandon is no longer used. The response from the force-delete + API call now includes the name of the stack which had underlain + that deleted cluster. diff --git a/sahara/api/v2/clusters.py b/sahara/api/v2/clusters.py index 1d10647c91..09467ecb3e 100644 --- a/sahara/api/v2/clusters.py +++ b/sahara/api/v2/clusters.py @@ -90,5 +90,11 @@ def clusters_update(cluster_id, data): def clusters_delete(cluster_id): data = u.request_data() force = data.get('force', False) + stack_name = api.get_cluster(cluster_id).get( + 'extra', {}).get( + 'heat_stack_name', None) api.terminate_cluster(cluster_id, force=force) - return u.render() + if force: + return u.render({"stack_name": stack_name}, status=200) + else: + return u.render(res=None, status=204) diff --git a/sahara/service/heat/heat_engine.py b/sahara/service/heat/heat_engine.py index de22e39660..d76ce4c43c 100644 --- a/sahara/service/heat/heat_engine.py +++ b/sahara/service/heat/heat_engine.py @@ -19,6 +19,7 @@ from oslo_log import log as logging from sahara import conductor as c from sahara import context +from sahara import exceptions as ex from sahara.i18n import _ from sahara.service import engine as e from sahara.service.heat import commons as heat_common @@ -194,7 +195,7 @@ class HeatEngine(e.Engine): def shutdown_cluster(self, cluster, force=False): """Shutdown specified cluster and all related resources.""" if force: - heat_shutdown = heat.abandon_stack + heat_shutdown = heat.lazy_delete_stack else: heat_shutdown = heat.delete_stack @@ -202,11 +203,11 @@ class HeatEngine(e.Engine): heat_shutdown(cluster) except heat_exc.HTTPNotFound: LOG.warning('Did not find stack for cluster.') - except heat_exc.BadRequest: - LOG.error("Can't force delete cluster.", exc_info=True) - finally: - self._clean_job_executions(cluster) - self._remove_db_objects(cluster) + except ex.HeatStackException: + raise + + self._clean_job_executions(cluster) + self._remove_db_objects(cluster) @cpo.event_wrapper( True, step=_('Create Heat stack'), param=('cluster', 1)) diff --git a/sahara/tests/unit/service/test_engine.py b/sahara/tests/unit/service/test_engine.py index 807099c7ca..72cb4ee788 100644 --- a/sahara/tests/unit/service/test_engine.py +++ b/sahara/tests/unit/service/test_engine.py @@ -127,42 +127,27 @@ class TestDeletion(base.SaharaTestCase): @mock.patch('sahara.service.heat.heat_engine.LOG.error') @mock.patch('sahara.utils.openstack.heat.delete_stack') - @mock.patch('sahara.utils.openstack.heat.abandon_stack') + @mock.patch('sahara.utils.openstack.heat.lazy_delete_stack') @mock.patch('sahara.service.engine.Engine._remove_db_objects') @mock.patch('sahara.service.engine.Engine._clean_job_executions') - def test_force_delete_calls(self, cj, rdb, abandon, delete, log_err): + def test_force_delete_calls(self, cj, rdb, lazy_delete, delete, log_err): engine = heat_engine.HeatEngine() - # Force delete when Heat service support abandon + # Force delete (lazy_delete) engine.shutdown_cluster(mock.Mock(), force=True) self.assertEqual(delete.call_count, 0) - self.assertEqual(abandon.call_count, 1) + self.assertEqual(lazy_delete.call_count, 1) self.assertEqual(cj.call_count, 1) self.assertEqual(rdb.call_count, 1) delete.reset_mock() - abandon.reset_mock() + lazy_delete.reset_mock() rdb.reset_mock() cj.reset_mock() # Regular delete engine.shutdown_cluster(mock.Mock(), force=False) self.assertEqual(delete.call_count, 1) - self.assertEqual(abandon.call_count, 0) + self.assertEqual(lazy_delete.call_count, 0) self.assertEqual(cj.call_count, 1) self.assertEqual(rdb.call_count, 1) - - delete.reset_mock() - abandon.reset_mock() - rdb.reset_mock() - cj.reset_mock() - - # Force delete when stack abandon unavailable - abandon.side_effect = heat_exc.BadRequest() - engine.shutdown_cluster(mock.Mock(), force=True) - self.assertEqual(delete.call_count, 0) - self.assertEqual(abandon.call_count, 1) - self.assertEqual(cj.call_count, 1) - self.assertEqual(rdb.call_count, 1) - log_err.assert_called_once_with( - "Can't force delete cluster.", exc_info=True) diff --git a/sahara/tests/unit/utils/openstack/test_heat.py b/sahara/tests/unit/utils/openstack/test_heat.py index ca2d789fca..53d858e27d 100644 --- a/sahara/tests/unit/utils/openstack/test_heat.py +++ b/sahara/tests/unit/utils/openstack/test_heat.py @@ -20,20 +20,26 @@ from sahara.utils.openstack import heat as heat_u class HeatClientTest(testtools.TestCase): + @mock.patch('sahara.utils.openstack.heat.get_stack') @mock.patch('heatclient.client.Client') @mock.patch('sahara.utils.openstack.base.url_for') @mock.patch('sahara.service.sessions.cache') @mock.patch('sahara.context.ctx') - def test_abandon(self, ctx, cache, url_for, heat): + def test_deleting(self, ctx, cache, url_for, heat, get_stack): class _FakeHeatStacks(object): def delete(self, stack): call_list.append("delete") - def abandon(self, stack): - call_list.append("abandon") + call_list = None + get_stack.return_value = None + get_stack.side_effect = lambda *args, **kwargs: call_list.append("get") heat.return_value.stacks = _FakeHeatStacks() call_list = [] - heat_u.abandon_stack(mock.Mock()) - self.assertEqual(call_list, ["delete", "abandon"]) + heat_u.lazy_delete_stack(mock.Mock()) + self.assertEqual(call_list, ["delete"]) + + call_list = [] + heat_u.delete_stack(mock.Mock()) + self.assertEqual(call_list, ["delete", "get"]) diff --git a/sahara/utils/openstack/heat.py b/sahara/utils/openstack/heat.py index 8e7d9f34ed..ff1ae405ef 100644 --- a/sahara/utils/openstack/heat.py +++ b/sahara/utils/openstack/heat.py @@ -84,13 +84,10 @@ def delete_stack(cluster): stack = get_stack(stack_name, raise_on_missing=False) -def abandon_stack(cluster): - '''Put stack in deleting state if not already, then abandon it.''' - heat_client = client() +def lazy_delete_stack(cluster): + '''Attempt to delete stack once, but do not await successful deletion''' stack_name = cluster.stack_name - base.execute_with_retries(heat_client.stacks.delete, stack_name) - # TODO(jfreud): sleep between calls? - base.execute_with_retries(heat_client.stacks.abandon, stack_name) + base.execute_with_retries(client().stacks.delete, stack_name) def get_stack_outputs(cluster):