From 6f7b3aca9bb0ca865ec6a3efb24dfc5979769edf Mon Sep 17 00:00:00 2001 From: Duc Truong Date: Wed, 10 Jul 2019 21:46:40 +0000 Subject: [PATCH] Fix senlin cluster create Calling openstacksdk's create_cluster no longer returns the action id in the location. Instead the caller has to use the cluster id to query the cluster status and determine the execution status based on that. Change-Id: Ia09915dc0529aaec21d0826f28e29d435354e5e0 Task: 35786 --- heat/engine/clients/os/senlin.py | 29 ++++++++++++++----- .../resources/openstack/senlin/cluster.py | 3 +- heat/tests/openstack/senlin/test_cluster.py | 18 +++++++----- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/heat/engine/clients/os/senlin.py b/heat/engine/clients/os/senlin.py index 408cabe296..6b62d01c7f 100644 --- a/heat/engine/clients/os/senlin.py +++ b/heat/engine/clients/os/senlin.py @@ -50,6 +50,17 @@ class SenlinClientPlugin(sdk_plugin.OpenStackSDKPlugin): ) return False + def cluster_is_active(self, cluster_id): + cluster = self.client().get_cluster(cluster_id) + if cluster.status == 'ACTIVE': + return True + elif cluster.status == 'ERROR': + raise exception.ResourceInError( + status_reason=cluster.status_reason, + resource_status=cluster.status, + ) + return False + def get_profile_id(self, profile_name): profile = self.client().get_profile(profile_name) return profile.id @@ -72,15 +83,19 @@ class SenlinClientPlugin(sdk_plugin.OpenStackSDKPlugin): if action['done']: continue all_executed = False - if action['action_id'] is None: - func = getattr(self.client(), action['func']) - ret = func(**action['params']) - if isinstance(ret, dict): - action['action_id'] = ret['action'] + if 'action_id' in action: + if action['action_id'] is None: + func = getattr(self.client(), action['func']) + ret = func(**action['params']) + if isinstance(ret, dict): + action['action_id'] = ret['action'] + else: + action['action_id'] = ret.location.split('/')[-1] else: - action['action_id'] = ret.location.split('/')[-1] + ret = self.check_action_status(action['action_id']) + action['done'] = ret else: - ret = self.check_action_status(action['action_id']) + ret = self.cluster_is_active(action['cluster_id']) action['done'] = ret # Execute these actions one by one. break diff --git a/heat/engine/resources/openstack/senlin/cluster.py b/heat/engine/resources/openstack/senlin/cluster.py index 371e12a999..b8e187abe5 100644 --- a/heat/engine/resources/openstack/senlin/cluster.py +++ b/heat/engine/resources/openstack/senlin/cluster.py @@ -218,12 +218,11 @@ class Cluster(res_base.BaseSenlinResource): } cluster = self.client().create_cluster(**params) - action_id = cluster.location.split('/')[-1] self.resource_id_set(cluster.id) # for cluster creation, we just to check the action status # the action is executed above action = { - 'action_id': action_id, + 'cluster_id': cluster.id, 'done': False, } actions.append(action) diff --git a/heat/tests/openstack/senlin/test_cluster.py b/heat/tests/openstack/senlin/test_cluster.py index 88656df447..2b59a20231 100644 --- a/heat/tests/openstack/senlin/test_cluster.py +++ b/heat/tests/openstack/senlin/test_cluster.py @@ -122,6 +122,8 @@ class SenlinClusterTest(common.HeatTestCase): self.assertEqual((cluster.CREATE, cluster.COMPLETE), cluster.state) self.assertEqual(self.fake_cl.id, cluster.resource_id) + self.assertEqual(1, self.senlin_mock.get_action.call_count) + self.assertEqual(1, self.senlin_mock.get_cluster.call_count) return cluster def test_cluster_create_success(self): @@ -149,17 +151,17 @@ class SenlinClusterTest(common.HeatTestCase): cfg.CONF.set_override('action_retry_limit', 0) cluster = self._init_cluster(self.t) self.senlin_mock.create_cluster.return_value = self.fake_cl - mock_action = mock.MagicMock() - mock_action.status = 'FAILED' - mock_action.status_reason = 'oops' + mock_cluster = mock.MagicMock() + mock_cluster.status = 'ERROR' + mock_cluster.status_reason = 'oops' self.senlin_mock.get_policy.return_value = mock.Mock( id='fake_policy_id' ) - self.senlin_mock.get_action.return_value = mock_action + self.senlin_mock.get_cluster.return_value = mock_cluster create_task = scheduler.TaskRunner(cluster.create) ex = self.assertRaises(exception.ResourceFailure, create_task) expected = ('ResourceInError: resources.senlin-cluster: ' - 'Went to status FAILED due to "oops"') + 'Went to status ERROR due to "oops"') self.assertEqual(expected, six.text_type(ex)) def test_cluster_delete_success(self): @@ -205,7 +207,7 @@ class SenlinClusterTest(common.HeatTestCase): } self.senlin_mock.update_cluster.assert_called_once_with( cluster=self.fake_cl, **cluster_update_kwargs) - self.assertEqual(3, self.senlin_mock.get_action.call_count) + self.assertEqual(2, self.senlin_mock.get_action.call_count) def test_cluster_update_desire_capacity(self): cluster = self._create_cluster(self.t) @@ -226,7 +228,7 @@ class SenlinClusterTest(common.HeatTestCase): } self.senlin_mock.cluster_resize.assert_called_once_with( cluster=cluster.resource_id, **cluster_resize_kwargs) - self.assertEqual(3, self.senlin_mock.get_action.call_count) + self.assertEqual(2, self.senlin_mock.get_action.call_count) def test_cluster_update_policy_add_remove(self): cluster = self._create_cluster(self.t) @@ -259,7 +261,7 @@ class SenlinClusterTest(common.HeatTestCase): self.senlin_mock.cluster_detach_policy.assert_called_once_with( **detach_policy_kwargs) self.assertEqual(0, self.senlin_mock.cluster_update_policy.call_count) - self.assertEqual(4, self.senlin_mock.get_action.call_count) + self.assertEqual(3, self.senlin_mock.get_action.call_count) def test_cluster_update_policy_exists(self): cluster = self._create_cluster(self.t)