From 4db4f1e904fe12d794df0e3542ba4ba3a39e9f41 Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Tue, 20 Dec 2016 20:29:59 +0800 Subject: [PATCH] To reset metadata for resources when mark unhealthy Some resources do not work if their metadata is in a wrong state, .e.g the metadata 'scaling_in_progress' of scaling group/policy might be always True if engine restarts while scaling. This patch adds an interface 'handle_metadata_reset' for resource, then the plugins can override it if needed. We reset the metadata while marking resource healthy. Closes-Bug: #1651084 (cherry picked from commit 5b04acb4e25efbea58de998278e390fb64d7c575) Conflicts: heat/tests/engine/service/test_stack_resources.py Change-Id: Ibd6c18acf6f3f24cf9bf16a524127850968062bc --- heat/engine/resource.py | 10 ++ heat/engine/service.py | 1 + heat/scaling/cooldown.py | 6 + .../engine/service/test_stack_resources.py | 108 ++++++++---------- 4 files changed, 63 insertions(+), 62 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index c5f2b08bfd..0b7f36c1b8 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -431,6 +431,16 @@ class Resource(object): db_res.update_metadata(metadata) self._rsrc_metadata = metadata + def handle_metadata_reset(self): + """Default implementation; should be overridden by resources. + + Now we override this method to reset the metadata for scale-policy + and scale-group resources, because their metadata might hang in a + wrong state ('scaling_in_progress' is always True) if engine restarts + while scaling. + """ + pass + @classmethod def set_needed_by(cls, db_rsrc, needed_by, expected_engine_id=None): if db_rsrc: diff --git a/heat/engine/service.py b/heat/engine/service.py index 8b98d13ddd..e69526faf4 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1878,6 +1878,7 @@ class EngineService(service.Service): if mark_unhealthy: rsrc.state_set(rsrc.CHECK, rsrc.FAILED, reason=reason) elif rsrc.state == (rsrc.CHECK, rsrc.FAILED): + rsrc.handle_metadata_reset() rsrc.state_set(rsrc.CHECK, rsrc.COMPLETE, reason=reason) except exception.UpdateInProgress: diff --git a/heat/scaling/cooldown.py b/heat/scaling/cooldown.py index 6d1784145c..315b543c3a 100644 --- a/heat/scaling/cooldown.py +++ b/heat/scaling/cooldown.py @@ -69,3 +69,9 @@ class CooldownMixin(object): self.metadata_set(metadata) except exception.NotFound: pass + + def handle_metadata_reset(self): + metadata = self.metadata_get() + if 'scaling_in_progress' in metadata: + metadata['scaling_in_progress'] = False + self.metadata_set(metadata) diff --git a/heat/tests/engine/service/test_stack_resources.py b/heat/tests/engine/service/test_stack_resources.py index c75295c8eb..d9a87ddc38 100644 --- a/heat/tests/engine/service/test_stack_resources.py +++ b/heat/tests/engine/service/test_stack_resources.py @@ -20,6 +20,7 @@ from heat.common import identifier from heat.engine.clients.os import keystone from heat.engine import dependencies from heat.engine import resource as res +from heat.engine.resources.aws.ec2 import instance as ins from heat.engine import service from heat.engine import stack from heat.engine import stack_lock @@ -536,82 +537,77 @@ class StackResourcesServiceTest(common.HeatTestCase): self.assertIsInstance(stack_dependencies, dependencies.Dependencies) self.assertEqual(2, len(stack_dependencies.graph())) + def _test_mark_healthy_asserts(self, action='CHECK', status='FAILED', + reason='state changed', meta=None): + rs = self.eng.describe_stack_resource( + self.ctx, self.stack.identifier(), + 'WebServer', with_attr=None) + self.assertIn('resource_action', rs) + self.assertIn('resource_status', rs) + self.assertIn('resource_status_reason', rs) + + self.assertEqual(action, rs['resource_action']) + self.assertEqual(status, rs['resource_status']) + self.assertEqual(reason, rs['resource_status_reason']) + if meta is not None: + self.assertIn('metadata', rs) + self.assertEqual(meta, rs['metadata']) + @tools.stack_context('service_mark_healthy_create_complete_test_stk') def test_mark_healthy_in_create_complete(self): self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', False, resource_status_reason='noop') - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - self.assertIn('resource_action', r) - self.assertIn('resource_status', r) - self.assertIn('resource_status_reason', r) - - self.assertEqual(r['resource_action'], 'CREATE') - self.assertEqual(r['resource_status'], 'COMPLETE') - self.assertEqual(r['resource_status_reason'], 'state changed') + self._test_mark_healthy_asserts(action='CREATE', + status='COMPLETE') @tools.stack_context('service_mark_unhealthy_create_complete_test_stk') def test_mark_unhealthy_in_create_complete(self): + + reason = 'Some Reason' self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', True, - resource_status_reason='Some Reason') + resource_status_reason=reason) - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'FAILED') - self.assertEqual(r['resource_status_reason'], 'Some Reason') + self._test_mark_healthy_asserts(reason=reason) @tools.stack_context('service_mark_healthy_check_failed_test_stk') def test_mark_healthy_check_failed(self): + reason = 'Some Reason' self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', True, - resource_status_reason='Some Reason') + resource_status_reason=reason) + self._test_mark_healthy_asserts(reason=reason) - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) + meta = {'for_test': True} - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'FAILED') - self.assertEqual(r['resource_status_reason'], 'Some Reason') + def override_metadata_reset(rsrc): + rsrc.metadata_set(meta) + ins.Instance.handle_metadata_reset = override_metadata_reset + + reason = 'Good Reason' self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', False, - resource_status_reason='Good Reason') - - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'COMPLETE') - self.assertEqual(r['resource_status_reason'], 'Good Reason') + resource_status_reason=reason) + self._test_mark_healthy_asserts(status='COMPLETE', + reason=reason, + meta=meta) @tools.stack_context('service_mark_unhealthy_check_failed_test_stack') def test_mark_unhealthy_check_failed(self): + reason = 'Some Reason' self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', True, - resource_status_reason='Some Reason') - - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'FAILED') - self.assertEqual(r['resource_status_reason'], 'Some Reason') + resource_status_reason=reason) + self._test_mark_healthy_asserts(reason=reason) + new_reason = 'New Reason' self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', True, - resource_status_reason='New Reason') - - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'FAILED') - self.assertEqual(r['resource_status_reason'], 'New Reason') + resource_status_reason=new_reason) + self._test_mark_healthy_asserts(reason=new_reason) @tools.stack_context('service_mark_unhealthy_invalid_value_test_stk') def test_mark_unhealthy_invalid_value(self): @@ -627,28 +623,16 @@ class StackResourcesServiceTest(common.HeatTestCase): def test_mark_unhealthy_none_reason(self): self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', True) - - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'FAILED') - self.assertEqual(r['resource_status_reason'], - 'state changed by resource_mark_unhealthy api') + default_reason = 'state changed by resource_mark_unhealthy api' + self._test_mark_healthy_asserts(reason=default_reason) @tools.stack_context('service_mark_unhealthy_empty_reason_test_stk') def test_mark_unhealthy_empty_reason(self): self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), 'WebServer', True, resource_status_reason="") - - r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(), - 'WebServer', with_attr=None) - - self.assertEqual(r['resource_action'], 'CHECK') - self.assertEqual(r['resource_status'], 'FAILED') - self.assertEqual(r['resource_status_reason'], - 'state changed by resource_mark_unhealthy api') + default_reason = 'state changed by resource_mark_unhealthy api' + self._test_mark_healthy_asserts(reason=default_reason) @tools.stack_context('service_mark_unhealthy_lock_no_converge_test_stk') def test_mark_unhealthy_lock_no_convergence(self):