From f52d77f0d291ee34f58276f45ac44f177883a31b Mon Sep 17 00:00:00 2001 From: Ahmed Elkhouly Date: Wed, 17 Feb 2016 12:13:05 -0500 Subject: [PATCH] Resource mark unhealthy RPC and ReST API There may exist resources that the user (or application) knows are unhealthy where Heat has no way of determining that. Add a PATCH handler to the Resource endpoint:: /stacks///resources/ The PATCH method will accept a JSON body of the form:: { 'mark_unhealthy': , 'resource_status_reason': } This patch Implements: - RPC API to mark resources as CHECK_FAILED in both the legacy and convergence architectures in heat-engine - ReST front end to the RPC API call in heat-api Change-Id: Ifa48b179723a2100fff548467db9e162bc669d13 Partially-implements: blueprint mark-unhealthy --- etc/heat/policy.json | 1 + heat/api/openstack/v1/__init__.py | 6 + heat/api/openstack/v1/resources.py | 31 +++ heat/engine/service.py | 46 ++++- heat/rpc/client.py | 20 ++ heat/tests/api/openstack_v1/test_resources.py | 137 +++++++++++++ .../engine/service/test_service_engine.py | 2 +- .../engine/service/test_stack_resources.py | 181 ++++++++++++++++++ heat/tests/test_rpc_client.py | 8 + 9 files changed, 430 insertions(+), 2 deletions(-) diff --git a/etc/heat/policy.json b/etc/heat/policy.json index b4980d9771..40c54d9c2b 100644 --- a/etc/heat/policy.json +++ b/etc/heat/policy.json @@ -36,6 +36,7 @@ "resource:index": "rule:deny_stack_user", "resource:metadata": "", "resource:signal": "", + "resource:mark_unhealthy": "rule:deny_stack_user", "resource:show": "rule:deny_stack_user", "stacks:abandon": "rule:deny_stack_user", "stacks:create": "rule:deny_stack_user", diff --git a/heat/api/openstack/v1/__init__.py b/heat/api/openstack/v1/__init__.py index f0920391cf..efd658b6b8 100644 --- a/heat/api/openstack/v1/__init__.py +++ b/heat/api/openstack/v1/__init__.py @@ -320,6 +320,12 @@ class API(wsgi.Router): 'url': '/resources/{resource_name}/signal', 'action': 'signal', 'method': 'POST' + }, + { + 'name': 'resource_mark_unhealthy', + 'url': '/resources/{resource_name}', + 'action': 'mark_unhealthy', + 'method': 'PATCH' } ]) diff --git a/heat/api/openstack/v1/resources.py b/heat/api/openstack/v1/resources.py index 87ffee39b7..5114acf391 100644 --- a/heat/api/openstack/v1/resources.py +++ b/heat/api/openstack/v1/resources.py @@ -17,6 +17,7 @@ import six from webob import exc from heat.api.openstack.v1 import util +from heat.common.i18n import _ from heat.common import identifier from heat.common import param_utils from heat.common import serializers @@ -155,6 +156,36 @@ class ResourceController(object): resource_name=resource_name, details=body) + @util.identified_stack + def mark_unhealthy(self, req, identity, resource_name, body): + """Mark a resource as healthy or unhealthy.""" + data = dict() + VALID_KEYS = (RES_UPDATE_MARK_UNHEALTHY, RES_UPDATE_STATUS_REASON) = ( + 'mark_unhealthy', rpc_api.RES_STATUS_DATA) + + invalid_keys = set(body) - set(VALID_KEYS) + if invalid_keys: + raise exc.HTTPBadRequest(_("Invalid keys in resource " + "mark unhealthy %s") % invalid_keys) + + if RES_UPDATE_MARK_UNHEALTHY not in body: + raise exc.HTTPBadRequest( + _("Missing mandatory (%s) key from mark unhealthy " + "request") % RES_UPDATE_MARK_UNHEALTHY) + + try: + data[RES_UPDATE_MARK_UNHEALTHY] = param_utils.extract_bool( + RES_UPDATE_MARK_UNHEALTHY, + body[RES_UPDATE_MARK_UNHEALTHY]) + except ValueError as e: + raise exc.HTTPBadRequest(six.text_type(e)) + + data[RES_UPDATE_STATUS_REASON] = body.get(RES_UPDATE_STATUS_REASON, "") + self.rpc_client.resource_mark_unhealthy(req.context, + stack_identity=identity, + resource_name=resource_name, + **data) + def create_resource(options): """Resources resource factory method.""" diff --git a/heat/engine/service.py b/heat/engine/service.py index f700f2ade9..73ce0d4225 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -294,7 +294,7 @@ class EngineService(service.Service): by the RPC caller. """ - RPC_API_VERSION = '1.25' + RPC_API_VERSION = '1.26' def __init__(self, host, topic): super(EngineService, self).__init__() @@ -1643,6 +1643,50 @@ class EngineService(service.Service): self.thread_group_mgr.start(stack.id, _resource_signal, stack, rsrc, details, False) + @context.request_context + def resource_mark_unhealthy(self, cnxt, stack_identity, resource_name, + mark_unhealthy, resource_status_reason=None): + """Mark the resource as healthy or unhealthy. + + Put the resource in CHECK_FAILED state if 'mark_unhealthy' + is true. Put the resource in CHECK_COMPLETE if 'mark_unhealthy' + is false and the resource is in CHECK_FAILED state. + Otherwise, make no change. + + :param mark_unhealthy: indicates whether the resource is unhealthy. + :param resource_status_reason: reason for health change. + """ + def lock(rsrc): + if rsrc.stack.convergence: + return rsrc.lock(self.engine_id) + else: + return stack_lock.StackLock(cnxt, + rsrc.stack.id, + self.engine_id) + + s = self._get_stack(cnxt, stack_identity) + stack = parser.Stack.load(cnxt, stack=s) + if resource_name not in stack: + raise exception.ResourceNotFound(resource_name=resource_name, + stack_name=stack.name) + + if not isinstance(mark_unhealthy, bool): + raise exception.Invalid(reason="mark_unhealthy is not a boolean") + + rsrc = stack[resource_name] + reason = (resource_status_reason or + "state changed by resource_mark_unhealthy api") + try: + with lock(rsrc): + if mark_unhealthy: + rsrc.state_set(rsrc.CHECK, rsrc.FAILED, reason=reason) + elif rsrc.state == (rsrc.CHECK, rsrc.FAILED): + rsrc.state_set(rsrc.CHECK, rsrc.COMPLETE, reason=reason) + + except exception.UpdateInProgress: + raise exception.ActionInProgress(stack_name=stack.name, + action=stack.action) + @context.request_context def find_physical_resource(self, cnxt, physical_resource_id): """Return an identifier for the specified resource. diff --git a/heat/rpc/client.py b/heat/rpc/client.py index d9b837f8a8..e21b9bf36c 100644 --- a/heat/rpc/client.py +++ b/heat/rpc/client.py @@ -44,6 +44,7 @@ class EngineClient(object): 1.23 - Add environment_files to create/update/preview/validate 1.24 - Adds ignorable_errors to validate_template 1.25 - list_stack_resource filter update + 1.26 - Add mark_unhealthy """ BASE_RPC_API_VERSION = '1.0' @@ -560,6 +561,25 @@ class EngineClient(object): version='1.3') + def resource_mark_unhealthy(self, ctxt, stack_identity, resource_name, + mark_unhealthy, resource_status_reason=None): + """Mark the resource as unhealthy or healthy. + + :param ctxt: RPC context. + :param stack_identity: Name of the stack. + :param resource_name: the Resource. + :param mark_unhealthy: indicates whether the resource is unhealthy. + :param resource_status_reason: reason for health change. + """ + return self.call( + ctxt, + self.make_msg('resource_mark_unhealthy', + stack_identity=stack_identity, + resource_name=resource_name, + mark_unhealthy=mark_unhealthy, + resource_status_reason=resource_status_reason), + version='1.26') + def create_watch_data(self, ctxt, watch_name, stats_data): """Creates data for CloudWatch and WaitConditions. diff --git a/heat/tests/api/openstack_v1/test_resources.py b/heat/tests/api/openstack_v1/test_resources.py index 015acd7343..bf9f925711 100644 --- a/heat/tests/api/openstack_v1/test_resources.py +++ b/heat/tests/api/openstack_v1/test_resources.py @@ -707,3 +707,140 @@ class ResourceControllerTest(tools.ControllerTest, common.HeatTestCase): self.assertIsNone(result) self.m.VerifyAll() + + def test_mark_unhealthy_valid_request(self, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'mark_unhealthy', True) + res_name = 'WebServer' + stack_identity = identifier.HeatIdentifier(self.tenant, + 'wordpress', '1') + + req = self._get(stack_identity._tenant_path()) + + self.m.StubOutWithMock(rpc_client.EngineClient, 'call') + body = {'mark_unhealthy': True, + rpc_api.RES_STATUS_DATA: 'Anything'} + params = {'stack_identity': stack_identity, + 'resource_name': res_name} + params.update(body) + + rpc_client.EngineClient.call( + req.context, + ('resource_mark_unhealthy', params), + version='1.26') + self.m.ReplayAll() + + result = self.controller.mark_unhealthy( + req, tenant_id=self.tenant, + stack_name=stack_identity.stack_name, + stack_id=stack_identity.stack_id, + resource_name=res_name, + body=body) + + self.assertIsNone(result) + self.m.VerifyAll() + + def test_mark_unhealthy_without_reason(self, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'mark_unhealthy', True) + res_name = 'WebServer' + stack_identity = identifier.HeatIdentifier(self.tenant, + 'wordpress', '1') + + req = self._get(stack_identity._tenant_path()) + + self.m.StubOutWithMock(rpc_client.EngineClient, 'call') + body = {'mark_unhealthy': True, rpc_api.RES_STATUS_DATA: ''} + params = {'stack_identity': stack_identity, + 'resource_name': res_name} + params.update(body) + + rpc_client.EngineClient.call( + req.context, + ('resource_mark_unhealthy', params), + version='1.26') + self.m.ReplayAll() + + del body[rpc_api.RES_STATUS_DATA] + + result = self.controller.mark_unhealthy( + req, tenant_id=self.tenant, + stack_name=stack_identity.stack_name, + stack_id=stack_identity.stack_id, + resource_name=res_name, + body=body) + + self.assertIsNone(result) + self.m.VerifyAll() + + def test_mark_unhealthy_with_invalid_keys(self, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'mark_unhealthy', True) + res_name = 'WebServer' + stack_identity = identifier.HeatIdentifier(self.tenant, + 'wordpress', '1') + + req = self._get(stack_identity._tenant_path()) + + self.m.StubOutWithMock(rpc_client.EngineClient, 'call') + body = {'mark_unhealthy': True, + rpc_api.RES_STATUS_DATA: 'Any', + 'invalid_key1': 1, 'invalid_key2': 2} + expected = "Invalid keys in resource mark unhealthy" + actual = self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.mark_unhealthy, req, + tenant_id=self.tenant, + stack_name=stack_identity.stack_name, + stack_id=stack_identity.stack_id, + resource_name=res_name, + body=body) + + self.assertIn(expected, six.text_type(actual)) + self.assertIn('invalid_key1', six.text_type(actual)) + self.assertIn('invalid_key2', six.text_type(actual)) + + def test_mark_unhealthy_with_invalid_value(self, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'mark_unhealthy', True) + res_name = 'WebServer' + stack_identity = identifier.HeatIdentifier(self.tenant, + 'wordpress', '1') + + req = self._get(stack_identity._tenant_path()) + + self.m.StubOutWithMock(rpc_client.EngineClient, 'call') + body = {'mark_unhealthy': 'XYZ', + rpc_api.RES_STATUS_DATA: 'Any'} + + expected = ('Unrecognized value "XYZ" for "mark_unhealthy", ' + 'acceptable values are: true, false') + + actual = self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.mark_unhealthy, req, + tenant_id=self.tenant, + stack_name=stack_identity.stack_name, + stack_id=stack_identity.stack_id, + resource_name=res_name, + body=body) + + self.assertIn(expected, six.text_type(actual)) + + def test_mark_unhealthy_without_mark_unhealthy_key(self, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'mark_unhealthy', True) + res_name = 'WebServer' + stack_identity = identifier.HeatIdentifier(self.tenant, + 'wordpress', '1') + + req = self._get(stack_identity._tenant_path()) + + self.m.StubOutWithMock(rpc_client.EngineClient, 'call') + body = {rpc_api.RES_STATUS_DATA: 'Any'} + + expected = ("Missing mandatory (%s) key from " + "mark unhealthy request" % 'mark_unhealthy') + + actual = self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.mark_unhealthy, req, + tenant_id=self.tenant, + stack_name=stack_identity.stack_name, + stack_id=stack_identity.stack_id, + resource_name=res_name, + body=body) + + self.assertIn(expected, six.text_type(actual)) diff --git a/heat/tests/engine/service/test_service_engine.py b/heat/tests/engine/service/test_service_engine.py index 8b387551ef..bdc736724f 100644 --- a/heat/tests/engine/service/test_service_engine.py +++ b/heat/tests/engine/service/test_service_engine.py @@ -40,7 +40,7 @@ class ServiceEngineTest(common.HeatTestCase): def test_make_sure_rpc_version(self): self.assertEqual( - '1.25', + '1.26', service.EngineService.RPC_API_VERSION, ('RPC version is changed, please update this test to new version ' 'and make sure additional test cases are added for RPC APIs ' diff --git a/heat/tests/engine/service/test_stack_resources.py b/heat/tests/engine/service/test_stack_resources.py index 5d0f8c60fb..49276b5bc0 100644 --- a/heat/tests/engine/service/test_stack_resources.py +++ b/heat/tests/engine/service/test_stack_resources.py @@ -22,6 +22,7 @@ from heat.engine import dependencies from heat.engine import resource as res from heat.engine import service from heat.engine import stack +from heat.engine import stack_lock from heat.engine import template as templatem from heat.objects import stack as stack_object from heat.tests import common @@ -497,3 +498,183 @@ class StackResourcesServiceTest(common.HeatTestCase): stack_dependencies = stk.dependencies self.assertIsInstance(stack_dependencies, dependencies.Dependencies) self.assertEqual(2, len(stack_dependencies.graph())) + + @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') + + @tools.stack_context('service_mark_unhealthy_create_complete_test_stk') + def test_mark_unhealthy_in_create_complete(self): + 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') + + @tools.stack_context('service_mark_healthy_check_failed_test_stk') + def test_mark_healthy_check_failed(self): + 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') + + 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') + + @tools.stack_context('service_mark_unhealthy_check_failed_test_stack') + def test_mark_unhealthy_check_failed(self): + 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') + + 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') + + @tools.stack_context('service_mark_unhealthy_invalid_value_test_stk') + def test_mark_unhealthy_invalid_value(self): + ex = self.assertRaises(dispatcher.ExpectedException, + self.eng.resource_mark_unhealthy, + self.ctx, + self.stack.identifier(), + 'WebServer', "This is wrong", + resource_status_reason="Some Reason") + self.assertEqual(exception.Invalid, ex.exc_info[0]) + + @tools.stack_context('service_mark_unhealthy_none_reason_test_stk') + 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') + + @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') + + @tools.stack_context('service_mark_unhealthy_lock_no_converge_test_stk') + def test_mark_unhealthy_lock_no_convergence(self): + mock_acquire = self.patchobject(stack_lock.StackLock, + 'acquire', + return_value=None) + + mock_release = self.patchobject(stack_lock.StackLock, + 'release', + return_value=None) + + self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), + 'WebServer', True, + resource_status_reason="") + + mock_acquire.assert_called_once_with() + mock_release.assert_called_once_with() + + @tools.stack_context('service_mark_unhealthy_lock_converge_test_stk', + convergence=True) + def test_mark_unhealthy_stack_lock_convergence(self): + mock_acquire = self.patchobject(res.Resource, + '_acquire', + return_value=None) + + self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(), + 'WebServer', True, + resource_status_reason="") + + mock_acquire.assert_called_once_with(self.eng.engine_id) + + @tools.stack_context('service_mark_unhealthy_lockexc_converge_test_stk', + convergence=True) + def test_mark_unhealthy_stack_lock_exc_convergence(self): + def _acquire(*args, **kwargs): + raise exception.UpdateInProgress(self.stack.name) + + self.patchobject( + res.Resource, + '_acquire', + return_value=None, + side_effect=exception.UpdateInProgress(self.stack.name)) + ex = self.assertRaises(dispatcher.ExpectedException, + self.eng.resource_mark_unhealthy, + self.ctx, + self.stack.identifier(), + 'WebServer', True, + resource_status_reason="") + self.assertEqual(exception.ActionInProgress, ex.exc_info[0]) + + @tools.stack_context('service_mark_unhealthy_lockexc_no_converge_test_stk') + def test_mark_unhealthy_stack_lock_exc_no_convergence(self): + self.patchobject( + stack_lock.StackLock, + 'acquire', + return_value=None, + side_effect=exception.ActionInProgress( + stack_name=self.stack.name, + action=self.stack.action)) + ex = self.assertRaises(dispatcher.ExpectedException, + self.eng.resource_mark_unhealthy, + self.ctx, + self.stack.identifier(), + 'WebServer', True, + resource_status_reason="") + self.assertEqual(exception.ActionInProgress, ex.exc_info[0]) diff --git a/heat/tests/test_rpc_client.py b/heat/tests/test_rpc_client.py index 78d6038909..262575825d 100644 --- a/heat/tests/test_rpc_client.py +++ b/heat/tests/test_rpc_client.py @@ -391,3 +391,11 @@ class EngineRpcAPITestCase(common.HeatTestCase): 'call', stack_identity=self.identity, version='1.22') + + def test_resource_mark_unhealthy(self): + self._test_engine_api('resource_mark_unhealthy', 'call', + stack_identity=self.identity, + resource_name='LogicalResourceId', + mark_unhealthy=True, + resource_status_reason="Any reason", + version='1.26')