From 0dc9ebe38e16988b4b2f0725be07b06650c4383c Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 09:21:01 +0100 Subject: [PATCH 01/29] Fix a regression in notifier service which would cause generic trigger to only be dispatched on completed states, but not on custom states if action_sensor.emit_when config option was configured. Issue was inadverently introduced in https://github.com/StackStorm/st2/pull/4536, Issue reported by Hiroyasu OHYAMA (@userlocalhost). --- st2actions/st2actions/notifier/notifier.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/st2actions/st2actions/notifier/notifier.py b/st2actions/st2actions/notifier/notifier.py index b626bb5d13..f93d3634d7 100644 --- a/st2actions/st2actions/notifier/notifier.py +++ b/st2actions/st2actions/notifier/notifier.py @@ -97,10 +97,10 @@ def process(self, execution_db): self._post_notify_triggers(liveaction_db=liveaction_db, execution_db=execution_db) - if cfg.CONF.action_sensor.enable: - with CounterWithTimer(key='notifier.generic_trigger.post'): - self._post_generic_trigger(liveaction_db=liveaction_db, - execution_db=execution_db) + if cfg.CONF.action_sensor.enable: + with CounterWithTimer(key='notifier.generic_trigger.post'): + self._post_generic_trigger(liveaction_db=liveaction_db, + execution_db=execution_db) def _get_execution_for_liveaction(self, liveaction): execution = ActionExecution.get(liveaction__id=str(liveaction.id)) From 30228241f5b236c0004ef5cca8acb80f5434610a Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 09:23:53 +0100 Subject: [PATCH 02/29] Add tests for regression - make sure we also test process() code path. --- st2actions/tests/unit/test_notifier.py | 80 ++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/st2actions/tests/unit/test_notifier.py b/st2actions/tests/unit/test_notifier.py index f82ecd0bf2..10246ef534 100644 --- a/st2actions/tests/unit/test_notifier.py +++ b/st2actions/tests/unit/test_notifier.py @@ -263,3 +263,83 @@ def test_post_generic_trigger_with_emit_condition(self, dispatch): payload=exp, trace_context={}) self.assertEqual(dispatch.call_count, 3) + + @mock.patch('oslo_config.cfg.CONF.action_sensor.enable', mock.MagicMock( + return_value=True)) + @mock.patch.object(Notifier, '_get_runner_ref', mock.MagicMock( + return_value='local-shell-cmd')) + @mock.patch.object(Notifier, '_get_trace_context', mock.MagicMock( + return_value={})) + @mock.patch('st2common.transport.reactor.TriggerDispatcher.dispatch') + @mock.patch('st2actions.notifier.notifier.LiveAction') + @mock.patch('st2actions.notifier.notifier.policy_service.apply_post_run_policies', mock.Mock()) + def test_process_post_generic_notify_trigger_on_completed_state_default(self, + mock_LiveAction, mock_dispatch): + # Verify that generic action trigger is posted on all completed states when action sensor + # is enabled + for status in LIVEACTION_STATUSES: + notifier = Notifier(connection=None, queues=[]) + + liveaction_db = LiveActionDB(id=bson.ObjectId(), action='core.local') + liveaction_db.status = status + execution = MOCK_EXECUTION + execution.liveaction = vars(LiveActionAPI.from_model(liveaction_db)) + execution.status = liveaction_db.status + + mock_LiveAction.get_by_id.return_value = liveaction_db + + notifier = Notifier(connection=None, queues=[]) + notifier.process(execution) + + if status in LIVEACTION_COMPLETED_STATES: + exp = {'status': status, + 'start_timestamp': str(liveaction_db.start_timestamp), + 'result': {}, 'parameters': {}, + 'action_ref': u'core.local', + 'runner_ref': 'local-shell-cmd', + 'execution_id': str(MOCK_EXECUTION.id), + 'action_name': u'core.local'} + mock_dispatch.assert_called_with('core.st2.generic.actiontrigger', + payload=exp, trace_context={}) + + self.assertEqual(mock_dispatch.call_count, len(LIVEACTION_COMPLETED_STATES)) + + @mock.patch('oslo_config.cfg.CONF.action_sensor', mock.MagicMock( + enable=True, emit_when=['scheduled', 'pending', 'abandoned'])) + @mock.patch.object(Notifier, '_get_runner_ref', mock.MagicMock( + return_value='local-shell-cmd')) + @mock.patch.object(Notifier, '_get_trace_context', mock.MagicMock( + return_value={})) + @mock.patch('st2common.transport.reactor.TriggerDispatcher.dispatch') + @mock.patch('st2actions.notifier.notifier.LiveAction') + @mock.patch('st2actions.notifier.notifier.policy_service.apply_post_run_policies', mock.Mock()) + def test_process_post_generic_notify_trigger_on_custom_emit_when_states(self, + mock_LiveAction, mock_dispatch): + # Verify that generic action trigger is posted on all completed states when action sensor + # is enabled + for status in LIVEACTION_STATUSES: + notifier = Notifier(connection=None, queues=[]) + + liveaction_db = LiveActionDB(id=bson.ObjectId(), action='core.local') + liveaction_db.status = status + execution = MOCK_EXECUTION + execution.liveaction = vars(LiveActionAPI.from_model(liveaction_db)) + execution.status = liveaction_db.status + + mock_LiveAction.get_by_id.return_value = liveaction_db + + notifier = Notifier(connection=None, queues=[]) + notifier.process(execution) + + if status in ['scheduled', 'pending', 'abandoned']: + exp = {'status': status, + 'start_timestamp': str(liveaction_db.start_timestamp), + 'result': {}, 'parameters': {}, + 'action_ref': u'core.local', + 'runner_ref': 'local-shell-cmd', + 'execution_id': str(MOCK_EXECUTION.id), + 'action_name': u'core.local'} + mock_dispatch.assert_called_with('core.st2.generic.actiontrigger', + payload=exp, trace_context={}) + + self.assertEqual(mock_dispatch.call_count, 3) From 5b164c0b25d202a05279eccee2fb5dcfb5ca21f9 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 10:15:34 +0100 Subject: [PATCH 03/29] Make sure we don't log API key inside st2api log file if API key is provided using "?st2-api-key" query parameter. --- st2common/st2common/middleware/logging.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/middleware/logging.py b/st2common/st2common/middleware/logging.py index e4d30a1e7d..2a51a3edf9 100644 --- a/st2common/st2common/middleware/logging.py +++ b/st2common/st2common/middleware/logging.py @@ -19,11 +19,19 @@ import itertools from st2common.constants.api import REQUEST_ID_HEADER +from st2common.constants.auth import QUERY_PARAM_ATTRIBUTE_NAME +from st2common.constants.auth import QUERY_PARAM_API_KEY_ATTRIBUTE_NAME +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common import log as logging from st2common.router import Request, NotFoundException LOG = logging.getLogger(__name__) +SECRET_QUERY_PARAMS = [ + QUERY_PARAM_ATTRIBUTE_NAME, + QUERY_PARAM_API_KEY_ATTRIBUTE_NAME +] + try: clock = time.perf_counter except AttributeError: @@ -46,12 +54,19 @@ def __call__(self, environ, start_response): request = Request(environ) + query_params = request.GET.dict_of_lists() + + # Mask secret / sensitive query params + for param_name in SECRET_QUERY_PARAMS: + if param_name in query_params: + query_params[param_name] = MASKED_ATTRIBUTE_VALUE + # Log the incoming request values = { 'method': request.method, 'path': request.path, 'remote_addr': request.remote_addr, - 'query': request.GET.dict_of_lists(), + 'query': query_params, 'request_id': request.headers.get(REQUEST_ID_HEADER, None) } From 7feb8987aa73d27d3cc21f35408815cf6b0b589c Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 10:28:09 +0100 Subject: [PATCH 04/29] Also mask query parameters which are defined in MASKED_ATTRIBUTES_BLACKLIST constant and log.mask_secrets_blacklist config option. --- st2common/st2common/middleware/logging.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/middleware/logging.py b/st2common/st2common/middleware/logging.py index 2a51a3edf9..9eeaa4cfeb 100644 --- a/st2common/st2common/middleware/logging.py +++ b/st2common/st2common/middleware/logging.py @@ -14,14 +14,18 @@ # limitations under the License. from __future__ import absolute_import + import time import types import itertools +from oslo_config import cfg + from st2common.constants.api import REQUEST_ID_HEADER from st2common.constants.auth import QUERY_PARAM_ATTRIBUTE_NAME from st2common.constants.auth import QUERY_PARAM_API_KEY_ATTRIBUTE_NAME from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE +from st2common.constants.secrets import MASKED_ATTRIBUTES_BLACKLIST from st2common import log as logging from st2common.router import Request, NotFoundException @@ -30,7 +34,7 @@ SECRET_QUERY_PARAMS = [ QUERY_PARAM_ATTRIBUTE_NAME, QUERY_PARAM_API_KEY_ATTRIBUTE_NAME -] +] + MASKED_ATTRIBUTES_BLACKLIST try: clock = time.perf_counter @@ -57,7 +61,8 @@ def __call__(self, environ, start_response): query_params = request.GET.dict_of_lists() # Mask secret / sensitive query params - for param_name in SECRET_QUERY_PARAMS: + secret_query_params = SECRET_QUERY_PARAMS + cfg.CONF.log.mask_secrets_blacklist + for param_name in secret_query_params: if param_name in query_params: query_params[param_name] = MASKED_ATTRIBUTE_VALUE From 81d5766de23ec1d204e6c6f1ff1653fb7f5dc3eb Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 12:30:25 +0100 Subject: [PATCH 05/29] Add a test case for masking secret values in API log messages. --- .../tests/unit/test_logging_middleware.py | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 st2common/tests/unit/test_logging_middleware.py diff --git a/st2common/tests/unit/test_logging_middleware.py b/st2common/tests/unit/test_logging_middleware.py new file mode 100644 index 0000000000..885fea95b3 --- /dev/null +++ b/st2common/tests/unit/test_logging_middleware.py @@ -0,0 +1,65 @@ +# Licensed to the StackStorm, Inc ('StackStorm') under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import mock +import unittest2 + +from st2common.middleware.logging import LoggingMiddleware +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE + +__all__ = [ + 'LoggingMiddlewareTestCase' +] + + +class LoggingMiddlewareTestCase(unittest2.TestCase): + @mock.patch('st2common.middleware.logging.LOG') + @mock.patch('st2common.middleware.logging.Request') + def test_secret_parameters_are_masked_in_log_message(self, mock_request, mock_log): + + def app(environ, custom_start_response): + custom_start_response(status='200 OK', headers=[('Content-Length', 100)]) + return [None] + + router = mock.Mock() + endpoint = mock.Mock() + router.match.return_value = (endpoint, None) + middleware = LoggingMiddleware(app=app, router=router) + + environ = {} + mock_request.return_value.GET.dict_of_lists.return_value = { + 'foo': 'bar', + 'bar': 'baz', + 'x-auth-token': 'secret', + 'st2-api-key': 'secret', + 'password': 'secret', + 'st2_auth_token': 'secret', + 'token': 'secret' + } + middleware(environ=environ, start_response=mock.Mock()) + + expected_query = { + 'foo': 'bar', + 'bar': 'baz', + 'x-auth-token': MASKED_ATTRIBUTE_VALUE, + 'st2-api-key': MASKED_ATTRIBUTE_VALUE, + 'password': MASKED_ATTRIBUTE_VALUE, + 'token': MASKED_ATTRIBUTE_VALUE, + 'st2_auth_token': MASKED_ATTRIBUTE_VALUE + } + + call_kwargs = mock_log.info.call_args_list[0][1] + query = call_kwargs['extra']['query'] + self.assertEqual(query, expected_query) From e04d86efd410917e48c30e974d493209bd075b7c Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 14:09:58 +0100 Subject: [PATCH 06/29] Update a test case to verify that custom attributes which are defined in the config are also masked. --- st2common/tests/unit/test_logging_middleware.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_logging_middleware.py b/st2common/tests/unit/test_logging_middleware.py index 885fea95b3..014634e414 100644 --- a/st2common/tests/unit/test_logging_middleware.py +++ b/st2common/tests/unit/test_logging_middleware.py @@ -16,6 +16,8 @@ import mock import unittest2 +from oslo_config import cfg + from st2common.middleware.logging import LoggingMiddleware from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE @@ -38,6 +40,9 @@ def app(environ, custom_start_response): router.match.return_value = (endpoint, None) middleware = LoggingMiddleware(app=app, router=router) + cfg.CONF.set_override(group='log', name='mask_secrets_blacklist', + override=['blacklisted_4', 'blacklisted_5']) + environ = {} mock_request.return_value.GET.dict_of_lists.return_value = { 'foo': 'bar', @@ -46,7 +51,9 @@ def app(environ, custom_start_response): 'st2-api-key': 'secret', 'password': 'secret', 'st2_auth_token': 'secret', - 'token': 'secret' + 'token': 'secret', + 'blacklisted_4': 'super secret', + 'blacklisted_5': 'super secret', } middleware(environ=environ, start_response=mock.Mock()) @@ -57,7 +64,9 @@ def app(environ, custom_start_response): 'st2-api-key': MASKED_ATTRIBUTE_VALUE, 'password': MASKED_ATTRIBUTE_VALUE, 'token': MASKED_ATTRIBUTE_VALUE, - 'st2_auth_token': MASKED_ATTRIBUTE_VALUE + 'st2_auth_token': MASKED_ATTRIBUTE_VALUE, + 'blacklisted_4': MASKED_ATTRIBUTE_VALUE, + 'blacklisted_5': MASKED_ATTRIBUTE_VALUE, } call_kwargs = mock_log.info.call_args_list[0][1] From 364523e524a4d055a1fe37e23717775ebf09c2a2 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 16:37:16 +0100 Subject: [PATCH 07/29] Move metric instrumentation inside the method call. This way we avoid outliers when method returns early aka immediately. --- st2actions/st2actions/notifier/notifier.py | 44 +++++++++++----------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/st2actions/st2actions/notifier/notifier.py b/st2actions/st2actions/notifier/notifier.py index f93d3634d7..9921167dc2 100644 --- a/st2actions/st2actions/notifier/notifier.py +++ b/st2actions/st2actions/notifier/notifier.py @@ -97,10 +97,7 @@ def process(self, execution_db): self._post_notify_triggers(liveaction_db=liveaction_db, execution_db=execution_db) - if cfg.CONF.action_sensor.enable: - with CounterWithTimer(key='notifier.generic_trigger.post'): - self._post_generic_trigger(liveaction_db=liveaction_db, - execution_db=execution_db) + self._post_generic_trigger(liveaction_db=liveaction_db, execution_db=execution_db) def _get_execution_for_liveaction(self, liveaction): execution = ActionExecution.get(liveaction__id=str(liveaction.id)) @@ -252,25 +249,26 @@ def _post_generic_trigger(self, liveaction_db=None, execution_db=None): LOG.debug(msg % (execution_id, execution_db.status, target_statuses), extra=extra) return - payload = {'execution_id': execution_id, - 'status': liveaction_db.status, - 'start_timestamp': str(liveaction_db.start_timestamp), - # deprecate 'action_name' at some point and switch to 'action_ref' - 'action_name': liveaction_db.action, - 'action_ref': liveaction_db.action, - 'runner_ref': self._get_runner_ref(liveaction_db.action), - 'parameters': liveaction_db.get_masked_parameters(), - 'result': liveaction_db.result} - # Use execution_id to extract trace rather than liveaction. execution_id - # will look-up an exact TraceDB while liveaction depending on context - # may not end up going to the DB. - trace_context = self._get_trace_context(execution_id=execution_id) - LOG.debug('POSTing %s for %s. Payload - %s. TraceContext - %s', - ACTION_TRIGGER_TYPE['name'], liveaction_db.id, payload, trace_context) - - with CounterWithTimer(key='notifier.generic_trigger.dispatch'): - self._trigger_dispatcher.dispatch(self._action_trigger, payload=payload, - trace_context=trace_context) + with CounterWithTimer(key='notifier.generic_trigger.post'): + payload = {'execution_id': execution_id, + 'status': liveaction_db.status, + 'start_timestamp': str(liveaction_db.start_timestamp), + # deprecate 'action_name' at some point and switch to 'action_ref' + 'action_name': liveaction_db.action, + 'action_ref': liveaction_db.action, + 'runner_ref': self._get_runner_ref(liveaction_db.action), + 'parameters': liveaction_db.get_masked_parameters(), + 'result': liveaction_db.result} + # Use execution_id to extract trace rather than liveaction. execution_id + # will look-up an exact TraceDB while liveaction depending on context + # may not end up going to the DB. + trace_context = self._get_trace_context(execution_id=execution_id) + LOG.debug('POSTing %s for %s. Payload - %s. TraceContext - %s', + ACTION_TRIGGER_TYPE['name'], liveaction_db.id, payload, trace_context) + + with CounterWithTimer(key='notifier.generic_trigger.dispatch'): + self._trigger_dispatcher.dispatch(self._action_trigger, payload=payload, + trace_context=trace_context) def _get_runner_ref(self, action_ref): """ From 56a67052d641cf6c7b8415c9f6cd1c8eeba84685 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 17:04:17 +0100 Subject: [PATCH 08/29] Add changelog entries. --- CHANGELOG.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6447c237ef..3ae23e37cf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,16 @@ Changelog In development -------------- +Fixed +~~~~~ + +* Fix inadvertent regression in notifier service which would cause generic action trigger to only + be dispatched for completed states even if custom states were specified using + ``action_sensor.emit_when`` config option. (bug fix) + Reported by Shu Sugimoto (@shusugmt). #4591 +* Make sure we don't log auth token and api key inside st2api log file if those values are provided + via query parameter and not header (``?x-auth-token=foo``, ``?st2-api-key=bar``). (bug fix) #4592 + #4589 2.10.3 - March 06, 2019 ----------------------- From addf9f64b3cb43030af0af1bc25833b9dd7bdcb0 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 28 Feb 2019 10:43:37 -0800 Subject: [PATCH 09/29] Fixes https://github.com/StackStorm/st2/issues/4567: config_context renders against incorrect pack Root cause: When a action calls other action which belongs to the different pack, during `render_live_params`, only parent pack configuration is retrieved. Fixes: Passes action reference to `render_live_params`, if action parent pack is different with child action pack, the configuration for child pack is retired and rendered. --- st2common/st2common/services/workflows.py | 6 +++++- st2common/st2common/util/param.py | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/services/workflows.py b/st2common/st2common/services/workflows.py index 65a2a21040..ba237cdfce 100644 --- a/st2common/st2common/services/workflows.py +++ b/st2common/st2common/services/workflows.py @@ -535,6 +535,9 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non # Identify the runner for the action. runner_type_db = action_utils.get_runnertype_by_name(action_db.runner_type['name']) + # Identify action reference + action_ref = task_ex_db.task_spec.get('spec').get('action') + # Set context for the action execution. ac_ex_ctx = { 'pack': st2_ctx.get('pack'), @@ -545,7 +548,8 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non 'task_execution_id': str(task_ex_db.id), 'task_name': task_ex_db.task_name, 'task_id': task_ex_db.task_id - } + }, + 'ref': action_ref } if st2_ctx.get('api_user'): diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 302d959da3..6cd7c4277a 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -276,6 +276,7 @@ def render_live_params(runner_parameters, action_parameters, params, action_cont pack = action_context.get('pack') user = action_context.get('user') + action_ref = action_context.get('ref') try: config = get_config(pack, user) @@ -285,6 +286,17 @@ def render_live_params(runner_parameters, action_parameters, params, action_cont G = _create_graph(action_context, config) + # Retrieve config for action pack if it is different with parent + if action_ref: + action_pack = action_ref.split('.')[0] + if pack != action_pack: + try: + action_config = get_config(action_pack, user) + G = _create_graph(action_context, action_config) + except Exception as e: + LOG.info('Failed to retrieve action config for pack %s and user %s: %s' % ( + action_pack, user, str(e))) + # Additional contexts are applied after all other contexts (see _create_graph), but before any # of the dependencies have been resolved. for name, value in six.iteritems(additional_contexts): From eae01e4643d0a8bda7640fc6b6440c2cac98523e Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 28 Feb 2019 13:31:02 -0800 Subject: [PATCH 10/29] Remove render for parent pack configuration. Googed Stackstorm documentation and did not see anywhere mentioned that this is supported. Fix typo for `cancellation` --- st2common/st2common/services/workflows.py | 15 +++++++++------ st2common/st2common/util/param.py | 12 ------------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/st2common/st2common/services/workflows.py b/st2common/st2common/services/workflows.py index ba237cdfce..08267422e1 100644 --- a/st2common/st2common/services/workflows.py +++ b/st2common/st2common/services/workflows.py @@ -357,7 +357,7 @@ def request_resume(ac_ex_db): @retrying.retry(retry_on_exception=wf_exc.retry_on_exceptions) def request_cancellation(ac_ex_db): wf_ac_ex_id = str(ac_ex_db.id) - LOG.info('[%s] Processing cancelation request for workflow.', wf_ac_ex_id) + LOG.info('[%s] Processing cancellation request for workflow.', wf_ac_ex_id) wf_ex_dbs = wf_db_access.WorkflowExecution.query(action_execution=str(ac_ex_db.id)) @@ -388,12 +388,12 @@ def request_cancellation(ac_ex_db): root_ac_ex_db = ac_svc.get_root_execution(ac_ex_db) if root_ac_ex_db != ac_ex_db and root_ac_ex_db.status not in ac_const.LIVEACTION_CANCEL_STATES: - LOG.info('[%s] Cascading cancelation request to parent workflow.', wf_ac_ex_id) + LOG.info('[%s] Cascading cancellation request to parent workflow.', wf_ac_ex_id) root_lv_ac_db = lv_db_access.LiveAction.get(id=root_ac_ex_db.liveaction['id']) ac_svc.request_cancellation(root_lv_ac_db, None) LOG.debug('[%s] %s', wf_ac_ex_id, conductor.serialize()) - LOG.info('[%s] Completed processing cancelation request for workflow.', wf_ac_ex_id) + LOG.info('[%s] Completed processing cancellation request for workflow.', wf_ac_ex_id) return wf_ex_db @@ -535,12 +535,16 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non # Identify the runner for the action. runner_type_db = action_utils.get_runnertype_by_name(action_db.runner_type['name']) - # Identify action reference + # Identify action pack name action_ref = task_ex_db.task_spec.get('spec').get('action') + if action_ref: + pack = action_ref.split('.')[0] + else: + pack = st2_ctx.get('pack') # Set context for the action execution. ac_ex_ctx = { - 'pack': st2_ctx.get('pack'), + 'pack': pack, 'user': st2_ctx.get('user'), 'parent': st2_ctx, 'orquesta': { @@ -549,7 +553,6 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non 'task_name': task_ex_db.task_name, 'task_id': task_ex_db.task_id }, - 'ref': action_ref } if st2_ctx.get('api_user'): diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 6cd7c4277a..302d959da3 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -276,7 +276,6 @@ def render_live_params(runner_parameters, action_parameters, params, action_cont pack = action_context.get('pack') user = action_context.get('user') - action_ref = action_context.get('ref') try: config = get_config(pack, user) @@ -286,17 +285,6 @@ def render_live_params(runner_parameters, action_parameters, params, action_cont G = _create_graph(action_context, config) - # Retrieve config for action pack if it is different with parent - if action_ref: - action_pack = action_ref.split('.')[0] - if pack != action_pack: - try: - action_config = get_config(action_pack, user) - G = _create_graph(action_context, action_config) - except Exception as e: - LOG.info('Failed to retrieve action config for pack %s and user %s: %s' % ( - action_pack, user, str(e))) - # Additional contexts are applied after all other contexts (see _create_graph), but before any # of the dependencies have been resolved. for name, value in six.iteritems(additional_contexts): From 916a22551fb8c7056930ea0111d80c32fdaddf15 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 7 Mar 2019 11:30:18 -0800 Subject: [PATCH 11/29] New unit test for config_context renders against incorrect pack Added new unit test case for the fix. Modified two dummy packs that unit test can run. Fix code review comments. Updated CHANGELOG.txt --- st2common/st2common/services/workflows.py | 11 +-- .../tests/unit/services/test_workflow.py | 74 +++++++++++++++++++ st2tests/st2tests/base.py | 3 +- .../fixtures/packs/configs/dummy_pack_7.yaml | 2 + .../actions/render_config_context.yaml | 8 ++ .../workflows/render_config_context.yaml | 7 ++ .../packs/dummy_pack_7/actions/render.py | 7 ++ .../packs/dummy_pack_7/actions/render.yaml | 12 +++ .../packs/dummy_pack_7/config.schema.yaml | 5 ++ 9 files changed, 121 insertions(+), 8 deletions(-) create mode 100644 st2tests/st2tests/fixtures/packs/configs/dummy_pack_7.yaml create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/workflows/render_config_context.yaml create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.py create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_7/config.schema.yaml diff --git a/st2common/st2common/services/workflows.py b/st2common/st2common/services/workflows.py index 08267422e1..645b2301da 100644 --- a/st2common/st2common/services/workflows.py +++ b/st2common/st2common/services/workflows.py @@ -357,7 +357,7 @@ def request_resume(ac_ex_db): @retrying.retry(retry_on_exception=wf_exc.retry_on_exceptions) def request_cancellation(ac_ex_db): wf_ac_ex_id = str(ac_ex_db.id) - LOG.info('[%s] Processing cancellation request for workflow.', wf_ac_ex_id) + LOG.info('[%s] Processing cancelation request for workflow.', wf_ac_ex_id) wf_ex_dbs = wf_db_access.WorkflowExecution.query(action_execution=str(ac_ex_db.id)) @@ -388,12 +388,12 @@ def request_cancellation(ac_ex_db): root_ac_ex_db = ac_svc.get_root_execution(ac_ex_db) if root_ac_ex_db != ac_ex_db and root_ac_ex_db.status not in ac_const.LIVEACTION_CANCEL_STATES: - LOG.info('[%s] Cascading cancellation request to parent workflow.', wf_ac_ex_id) + LOG.info('[%s] Cascading cancelation request to parent workflow.', wf_ac_ex_id) root_lv_ac_db = lv_db_access.LiveAction.get(id=root_ac_ex_db.liveaction['id']) ac_svc.request_cancellation(root_lv_ac_db, None) LOG.debug('[%s] %s', wf_ac_ex_id, conductor.serialize()) - LOG.info('[%s] Completed processing cancellation request for workflow.', wf_ac_ex_id) + LOG.info('[%s] Completed processing cancelation request for workflow.', wf_ac_ex_id) return wf_ex_db @@ -537,10 +537,7 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non # Identify action pack name action_ref = task_ex_db.task_spec.get('spec').get('action') - if action_ref: - pack = action_ref.split('.')[0] - else: - pack = st2_ctx.get('pack') + pack = action_ref.split('.')[0] if action_ref else st2_ctx.get('pack') # Set context for the action execution. ac_ex_ctx = { diff --git a/st2common/tests/unit/services/test_workflow.py b/st2common/tests/unit/services/test_workflow.py index dc2354a9fb..ef5453dfdd 100644 --- a/st2common/tests/unit/services/test_workflow.py +++ b/st2common/tests/unit/services/test_workflow.py @@ -28,8 +28,10 @@ from st2common.exceptions import action as action_exc from st2common.models.db import liveaction as lv_db_models from st2common.models.db import execution as ex_db_models +from st2common.models.db.pack import ConfigDB from st2common.persistence import execution as ex_db_access from st2common.persistence import workflow as wf_db_access +from st2common.persistence.pack import Config from st2common.services import action as action_service from st2common.services import workflows as workflow_service from st2common.transport import liveaction as lv_ac_xport @@ -40,8 +42,16 @@ TEST_PACK = 'orquesta_tests' TEST_PACK_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + '/' + TEST_PACK +PACK_7 = 'dummy_pack_7' +PACK_7_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + '/' + PACK_7 + +PACK_20 = 'dummy_pack_20' +PACK_20_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + '/' + PACK_20 + PACKS = [ TEST_PACK_PATH, + PACK_7_PATH, + PACK_20_PATH, st2tests.fixturesloader.get_fixtures_packs_base_path() + '/core' ] @@ -346,3 +356,67 @@ def test_evaluate_action_execution_delay(self): ac_ex_req = {'action': 'core.noop', 'input': None, 'item_id': 1} actual_delay = workflow_service.eval_action_execution_delay(task_ex_req, ac_ex_req, True) self.assertIsNone(actual_delay) + + def test_request_action_execution_render(self): + # Manually create ConfigDB + output = 'Testing' + value = { + "config_item_one": output + } + config_db = ConfigDB(pack=PACK_7, values=value) + config = Config.add_or_update(config_db) + self.assertEqual(len(config), 3) + + wf_meta = self.get_wf_fixture_meta_data(PACK_20_PATH, 'render_config_context.yaml') + + # Manually create the liveaction and action execution objects without publishing. + lv_ac_db = lv_db_models.LiveActionDB(action=wf_meta['name']) + lv_ac_db, ac_ex_db = action_service.create_request(lv_ac_db) + + # Request the workflow execution. + wf_def = self.get_wf_def(PACK_20_PATH, wf_meta) + st2_ctx = self.mock_st2_context(ac_ex_db) + wf_ex_db = workflow_service.request(wf_def, ac_ex_db, st2_ctx) + spec_module = specs_loader.get_spec_module(wf_ex_db.spec['catalog']) + wf_spec = spec_module.WorkflowSpec.deserialize(wf_ex_db.spec) + + # Pass down appropriate st2 context to the task and action execution(s). + root_st2_ctx = wf_ex_db.context.get('st2', {}) + st2_ctx = { + 'execution_id': wf_ex_db.action_execution, + 'user': root_st2_ctx.get('user'), + 'pack': root_st2_ctx.get('pack') + } + + # Manually request task execution. + task_route = 0 + task_id = 'task1' + task_spec = wf_spec.tasks.get_task(task_id) + task_ctx = {'foo': 'bar'} + + task_ex_req = { + 'id': task_id, + 'route': task_route, + 'spec': task_spec, + 'ctx': task_ctx, + 'actions': [ + {'action': 'dummy_pack_7.render', 'input': None} + ] + } + workflow_service.request_task_execution(wf_ex_db, st2_ctx, task_ex_req) + + # Check task execution is saved to the database. + task_ex_dbs = wf_db_access.TaskExecution.query(workflow_execution=str(wf_ex_db.id)) + self.assertEqual(len(task_ex_dbs), 1) + workflow_service.request_task_execution(wf_ex_db, st2_ctx, task_ex_req) + + # Manually request action execution + task_ex_db = task_ex_dbs[0] + action_ex_db = workflow_service.request_action_execution(wf_ex_db, task_ex_db, st2_ctx, + task_ex_req['actions'][0]) + + # Check required attributes. + self.assertIsNotNone(str(action_ex_db.id)) + self.assertEqual(task_ex_db.workflow_execution, str(wf_ex_db.id)) + expected_parameters = {'value1': output} + self.assertEqual(expected_parameters, action_ex_db.parameters) diff --git a/st2tests/st2tests/base.py b/st2tests/st2tests/base.py index b0c7e8eddb..3fdc91816b 100644 --- a/st2tests/st2tests/base.py +++ b/st2tests/st2tests/base.py @@ -618,7 +618,8 @@ def mock_st2_context(self, ac_ex_db, context=None): st2_ctx = { 'st2': { 'api_url': api_util.get_full_public_api_url(), - 'action_execution_id': str(ac_ex_db.id) + 'action_execution_id': str(ac_ex_db.id), + 'user': 'stanley' } } diff --git a/st2tests/st2tests/fixtures/packs/configs/dummy_pack_7.yaml b/st2tests/st2tests/fixtures/packs/configs/dummy_pack_7.yaml new file mode 100644 index 0000000000..6b01d06715 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/configs/dummy_pack_7.yaml @@ -0,0 +1,2 @@ +--- +config_item_one: "testing" diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml new file mode 100644 index 0000000000..0f1c6582d1 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml @@ -0,0 +1,8 @@ +--- +name: render_config_context +pack: dummy_pack_20 +description: Run render config context workflow +runner_type: orquesta +entry_point: workflows/render_config_context.yaml +enabled: true +parameters: {} diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/workflows/render_config_context.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/workflows/render_config_context.yaml new file mode 100644 index 0000000000..2d05f1dc51 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/workflows/render_config_context.yaml @@ -0,0 +1,7 @@ +version: 1.0 +description: Testing config context render". +tasks: + task1: + action: dummy_pack_7.render +output: + - context_value: <% task(task1).result.result.context_value %> diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.py b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.py new file mode 100644 index 0000000000..97ab48fea0 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.py @@ -0,0 +1,7 @@ +from st2common.runners.base_action import Action + + +class PrintPythonVersionAction(Action): + + def run(self, value1): + return {"context_value": value1} diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml new file mode 100644 index 0000000000..3f11fc264b --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml @@ -0,0 +1,12 @@ +--- +name: render +runner_type: python-script +description: Action that uses config context +enabled: true +entry_point: render.py +parameters: + value1: + description: Input for action1. Defaults to config_context value. + required: false + type: "string" + default: "{{ config_context.config_item_one }}" diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_7/config.schema.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_7/config.schema.yaml new file mode 100644 index 0000000000..731f42d98c --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_7/config.schema.yaml @@ -0,0 +1,5 @@ +--- +config_item_one: + description: "Item use to test config context." + type: "string" + required: true From 84034276a52cb6acb179540494c84c005d216245 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 7 Mar 2019 14:59:05 -0800 Subject: [PATCH 12/29] Fix review comments for unit test. Remove action and workflow from dummy_pack_20 to orquesta_tests Rename action_ref to spec_action Rename import names for test_workflow.py and reflect changes for dummy_pack_20 --- st2common/st2common/services/workflows.py | 6 +++--- st2common/tests/unit/services/test_workflow.py | 16 ++++++---------- .../actions/render_config_context.yaml | 2 +- .../actions/workflows/render_config_context.yaml | 0 4 files changed, 10 insertions(+), 14 deletions(-) rename st2tests/st2tests/fixtures/packs/{dummy_pack_20 => orquesta_tests}/actions/render_config_context.yaml (89%) rename st2tests/st2tests/fixtures/packs/{dummy_pack_20 => orquesta_tests}/actions/workflows/render_config_context.yaml (100%) diff --git a/st2common/st2common/services/workflows.py b/st2common/st2common/services/workflows.py index 645b2301da..cc29d96d5b 100644 --- a/st2common/st2common/services/workflows.py +++ b/st2common/st2common/services/workflows.py @@ -536,12 +536,12 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non runner_type_db = action_utils.get_runnertype_by_name(action_db.runner_type['name']) # Identify action pack name - action_ref = task_ex_db.task_spec.get('spec').get('action') - pack = action_ref.split('.')[0] if action_ref else st2_ctx.get('pack') + spec_action = task_ex_db.task_spec.get('spec').get('action') + pack_name = spec_action.split('.')[0] if spec_action else st2_ctx.get('pack') # Set context for the action execution. ac_ex_ctx = { - 'pack': pack, + 'pack': pack_name, 'user': st2_ctx.get('user'), 'parent': st2_ctx, 'orquesta': { diff --git a/st2common/tests/unit/services/test_workflow.py b/st2common/tests/unit/services/test_workflow.py index ef5453dfdd..b1021fae41 100644 --- a/st2common/tests/unit/services/test_workflow.py +++ b/st2common/tests/unit/services/test_workflow.py @@ -28,10 +28,10 @@ from st2common.exceptions import action as action_exc from st2common.models.db import liveaction as lv_db_models from st2common.models.db import execution as ex_db_models -from st2common.models.db.pack import ConfigDB +from st2common.models.db import pack as pk_db_models from st2common.persistence import execution as ex_db_access +from st2common.persistence import pack as pk_db_access from st2common.persistence import workflow as wf_db_access -from st2common.persistence.pack import Config from st2common.services import action as action_service from st2common.services import workflows as workflow_service from st2common.transport import liveaction as lv_ac_xport @@ -45,13 +45,9 @@ PACK_7 = 'dummy_pack_7' PACK_7_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + '/' + PACK_7 -PACK_20 = 'dummy_pack_20' -PACK_20_PATH = st2tests.fixturesloader.get_fixtures_packs_base_path() + '/' + PACK_20 - PACKS = [ TEST_PACK_PATH, PACK_7_PATH, - PACK_20_PATH, st2tests.fixturesloader.get_fixtures_packs_base_path() + '/core' ] @@ -363,18 +359,18 @@ def test_request_action_execution_render(self): value = { "config_item_one": output } - config_db = ConfigDB(pack=PACK_7, values=value) - config = Config.add_or_update(config_db) + config_db = pk_db_models.ConfigDB(pack=PACK_7, values=value) + config = pk_db_access.Config.add_or_update(config_db) self.assertEqual(len(config), 3) - wf_meta = self.get_wf_fixture_meta_data(PACK_20_PATH, 'render_config_context.yaml') + wf_meta = self.get_wf_fixture_meta_data(TEST_PACK_PATH, 'render_config_context.yaml') # Manually create the liveaction and action execution objects without publishing. lv_ac_db = lv_db_models.LiveActionDB(action=wf_meta['name']) lv_ac_db, ac_ex_db = action_service.create_request(lv_ac_db) # Request the workflow execution. - wf_def = self.get_wf_def(PACK_20_PATH, wf_meta) + wf_def = self.get_wf_def(TEST_PACK_PATH, wf_meta) st2_ctx = self.mock_st2_context(ac_ex_db) wf_ex_db = workflow_service.request(wf_def, ac_ex_db, st2_ctx) spec_module = specs_loader.get_spec_module(wf_ex_db.spec['catalog']) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/render_config_context.yaml similarity index 89% rename from st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml rename to st2tests/st2tests/fixtures/packs/orquesta_tests/actions/render_config_context.yaml index 0f1c6582d1..2f720cfd25 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/render_config_context.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/render_config_context.yaml @@ -1,6 +1,6 @@ --- name: render_config_context -pack: dummy_pack_20 +pack: orquesta_tests description: Run render config context workflow runner_type: orquesta entry_point: workflows/render_config_context.yaml diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/workflows/render_config_context.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/render_config_context.yaml similarity index 100% rename from st2tests/st2tests/fixtures/packs/dummy_pack_20/actions/workflows/render_config_context.yaml rename to st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/render_config_context.yaml From c2d0e9e15b2b3cbbdadf066b80a6e9f2874b9816 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 7 Mar 2019 15:14:58 -0800 Subject: [PATCH 13/29] Remove code for getting action reference from spc.action --- st2common/st2common/services/workflows.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/st2common/st2common/services/workflows.py b/st2common/st2common/services/workflows.py index cc29d96d5b..db5bc3c6f0 100644 --- a/st2common/st2common/services/workflows.py +++ b/st2common/st2common/services/workflows.py @@ -536,8 +536,7 @@ def request_action_execution(wf_ex_db, task_ex_db, st2_ctx, ac_ex_req, delay=Non runner_type_db = action_utils.get_runnertype_by_name(action_db.runner_type['name']) # Identify action pack name - spec_action = task_ex_db.task_spec.get('spec').get('action') - pack_name = spec_action.split('.')[0] if spec_action else st2_ctx.get('pack') + pack_name = action_ref.split('.')[0] if action_ref else st2_ctx.get('pack') # Set context for the action execution. ac_ex_ctx = { From cb8851beba88b63f4d765ef94d3ead5da306fa93 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Thu, 7 Mar 2019 18:02:41 -0800 Subject: [PATCH 14/29] Integration test for config_context renders against incorrect pack In order to run integration test, test used packs has to be installed first. Install `orquesta_tests` and `dummy_pack_7` packs in `` DB Create `virtualenv` for packs Add new testcase in test_wiring.py --- st2tests/integration/orquesta/base.py | 11 +++- st2tests/integration/orquesta/test_wiring.py | 19 ++++++ st2tests/st2tests/base.py | 67 ++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/st2tests/integration/orquesta/base.py b/st2tests/integration/orquesta/base.py index 4d06a9eb4e..6e5a3cc38b 100644 --- a/st2tests/integration/orquesta/base.py +++ b/st2tests/integration/orquesta/base.py @@ -25,6 +25,7 @@ from st2client import client as st2 from st2client import models from st2common.constants import action as action_constants +from st2tests.base import InstallFixturesPacks LIVEACTION_LAUNCHED_STATUSES = [ @@ -53,12 +54,20 @@ def _delete_temp_file(self, temp_file_path): os.remove(temp_file_path) -class TestWorkflowExecution(unittest2.TestCase): +class TestWorkflowExecution(InstallFixturesPacks): @classmethod def setUpClass(cls): + super(TestWorkflowExecution, cls).setUpClass() cls.st2client = st2.Client(base_url='http://127.0.0.1') + def tearDown(self): + super(TestWorkflowExecution, self).tearDown() + super(TestWorkflowExecution, self).delete_files() + + def install_packs(self, packs): + super(TestWorkflowExecution, self).install_packs(packs) + def _execute_workflow(self, action, parameters=None, execute_async=True, expected_status=None, expected_result=None): diff --git a/st2tests/integration/orquesta/test_wiring.py b/st2tests/integration/orquesta/test_wiring.py index 0306897810..70dff54b02 100644 --- a/st2tests/integration/orquesta/test_wiring.py +++ b/st2tests/integration/orquesta/test_wiring.py @@ -17,11 +17,14 @@ from __future__ import absolute_import +import mock + from integration.orquesta import base from st2common.constants import action as ac_const +@mock.patch('st2common.util.virtualenvs.BASE_PACK_REQUIREMENTS', []) class WiringTest(base.TestWorkflowExecution): def test_sequential(self): @@ -144,3 +147,19 @@ def test_output_on_error(self): self.assertEqual(ex.status, ac_const.LIVEACTION_STATUS_FAILED) self.assertDictEqual(ex.result, expected_result) + + def test_config_context_renders(self): + packs = ["orquesta_tests", "dummy_pack_7"] + + self.install_packs(packs) + config_value = "testing" + wf_name = 'orquesta_tests.render_config_context' + + expected_output = {'context_value': config_value} + expected_result = {'output': expected_output} + + ex = self._execute_workflow(wf_name) + ex = self._wait_for_completion(ex) + + self.assertEqual(ex.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) + self.assertDictEqual(ex.result, expected_result) diff --git a/st2tests/st2tests/base.py b/st2tests/st2tests/base.py index 3fdc91816b..75d6650d29 100644 --- a/st2tests/st2tests/base.py +++ b/st2tests/st2tests/base.py @@ -47,8 +47,10 @@ from st2common.exceptions.db import StackStormDBObjectConflictError from st2common.models.db import db_setup, db_teardown, db_ensure_indexes from st2common.models.db.execution_queue import ActionExecutionSchedulingQueueItemDB +from st2common.bootstrap.actionsregistrar import ActionsRegistrar from st2common.bootstrap.base import ResourceRegistrar from st2common.bootstrap.configsregistrar import ConfigsRegistrar +from st2common.bootstrap import runnersregistrar from st2common.content.utils import get_packs_base_paths from st2common.content.loader import MetaLoader from st2common.exceptions.db import StackStormDBObjectNotFoundError @@ -58,6 +60,7 @@ from st2common.services import workflows as wf_svc from st2common.util import api as api_util from st2common.util import loader +from st2common.util.virtualenvs import setup_pack_virtualenv import st2common.models.db.rule as rule_model import st2common.models.db.rule_enforcement as rule_enforcement_model import st2common.models.db.sensor as sensor_model @@ -83,6 +86,7 @@ 'DbTestCase', 'DbModelTestCase', 'CleanDbTestCase', + 'InstallFixturesPacks', 'CleanFilesTestCase', 'IntegrationTestCase', 'RunnerTestCase', @@ -465,6 +469,69 @@ def setUp(self): self._register_pack_configs() +class InstallFixturesPacks(DbTestCase): + """ + Class installs Fixtures packs according to passed pack list + """ + def install_packs(self, packs): + self._setup_env() + self._create_pack_virtualenv(packs) + self._register_packs(packs) + self._register_pack_configs(packs) + + def delete_files(self): + try: + shutil.rmtree(self.virtualenvs_path) + except Exception: + pass + + def _setup_env(self): + self.pack_path = get_packs_base_paths()[0] + self.virtualenvs_path = os.path.join(self.pack_path, 'virtualenvs/') + + def _create_pack_virtualenv(self, packs): + self._create_virtual_path() + + for pack_name in packs: + # Create virtualenv + setup_pack_virtualenv(pack_name=pack_name, update=False, include_pip=False, + include_setuptools=False, include_wheel=False) + + def _register_packs(self, packs, validate_configs=False): + """ + Register all the packs inside the fixtures directory. + """ + # Register runners. + runnersregistrar.register_runners() + + # Register actions. + actions_registrar = ActionsRegistrar(use_pack_cache=False, fail_on_failure=True) + for pack_name in packs: + actions_registrar.register_from_pack(self.pack_path + '/' + pack_name) + + # Register packs. + registrar = ResourceRegistrar(use_pack_cache=False, use_runners_cache=True) + for pack_name in packs: + registrar._register_pack_db(pack_name=pack_name, pack_dir=self.pack_path + '/' + + pack_name) + + def _register_pack_configs(self, packs, validate_configs=False): + """ + Register all the packs inside the fixtures directory. + """ + registrar = ConfigsRegistrar(use_pack_cache=False, validate_configs=validate_configs) + for pack_name in packs: + config_path = registrar._get_config_path_for_pack(pack_name) + if os.path.exists(config_path): + registrar._register_config_for_pack(pack=pack_name, config_path=config_path) + + def _create_virtual_path(self): + if os.path.isdir(self.virtualenvs_path): + self.delete_files() + + os.mkdir(self.virtualenvs_path) + + class CleanFilesTestCase(TestCase): """ Base test class which deletes specified files and directories on setUp and `tearDown. From 75a6f9f6270a6aa09925f83d8f71dbd94ec32bd1 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Fri, 8 Mar 2019 16:17:46 -0800 Subject: [PATCH 15/29] Remove integration test code and setup test environment from `./tools/launchdev.sh Remove integration task code made yesterday. Add code to setup integration test for render: During integration setup on cicd, `launchdev.sh start -x` will be called to install `examples` pack in order to run integration test. Add other packs are installed too, includes `core`, `default` and `pack`. Add new orquesta workflow action in examples. Still don't know which packs can be used for render action other than dummy_pack_7. --- .../actions/render_config_context.yaml | 8 +++ .../workflows/render_config_context.yaml | 7 ++ st2tests/integration/orquesta/base.py | 11 +-- st2tests/integration/orquesta/test_wiring.py | 6 +- st2tests/st2tests/base.py | 67 ------------------- tools/launchdev.sh | 6 ++ 6 files changed, 23 insertions(+), 82 deletions(-) create mode 100644 contrib/examples/actions/render_config_context.yaml create mode 100644 contrib/examples/actions/workflows/render_config_context.yaml diff --git a/contrib/examples/actions/render_config_context.yaml b/contrib/examples/actions/render_config_context.yaml new file mode 100644 index 0000000000..f9c307f130 --- /dev/null +++ b/contrib/examples/actions/render_config_context.yaml @@ -0,0 +1,8 @@ +--- +name: render_config_context +pack: examples +description: Run render config context workflow +runner_type: orquesta +entry_point: workflows/render_config_context.yaml +enabled: true +parameters: {} diff --git a/contrib/examples/actions/workflows/render_config_context.yaml b/contrib/examples/actions/workflows/render_config_context.yaml new file mode 100644 index 0000000000..2d05f1dc51 --- /dev/null +++ b/contrib/examples/actions/workflows/render_config_context.yaml @@ -0,0 +1,7 @@ +version: 1.0 +description: Testing config context render". +tasks: + task1: + action: dummy_pack_7.render +output: + - context_value: <% task(task1).result.result.context_value %> diff --git a/st2tests/integration/orquesta/base.py b/st2tests/integration/orquesta/base.py index 6e5a3cc38b..4d06a9eb4e 100644 --- a/st2tests/integration/orquesta/base.py +++ b/st2tests/integration/orquesta/base.py @@ -25,7 +25,6 @@ from st2client import client as st2 from st2client import models from st2common.constants import action as action_constants -from st2tests.base import InstallFixturesPacks LIVEACTION_LAUNCHED_STATUSES = [ @@ -54,20 +53,12 @@ def _delete_temp_file(self, temp_file_path): os.remove(temp_file_path) -class TestWorkflowExecution(InstallFixturesPacks): +class TestWorkflowExecution(unittest2.TestCase): @classmethod def setUpClass(cls): - super(TestWorkflowExecution, cls).setUpClass() cls.st2client = st2.Client(base_url='http://127.0.0.1') - def tearDown(self): - super(TestWorkflowExecution, self).tearDown() - super(TestWorkflowExecution, self).delete_files() - - def install_packs(self, packs): - super(TestWorkflowExecution, self).install_packs(packs) - def _execute_workflow(self, action, parameters=None, execute_async=True, expected_status=None, expected_result=None): diff --git a/st2tests/integration/orquesta/test_wiring.py b/st2tests/integration/orquesta/test_wiring.py index 70dff54b02..04eacbef20 100644 --- a/st2tests/integration/orquesta/test_wiring.py +++ b/st2tests/integration/orquesta/test_wiring.py @@ -24,7 +24,6 @@ from st2common.constants import action as ac_const -@mock.patch('st2common.util.virtualenvs.BASE_PACK_REQUIREMENTS', []) class WiringTest(base.TestWorkflowExecution): def test_sequential(self): @@ -149,11 +148,8 @@ def test_output_on_error(self): self.assertDictEqual(ex.result, expected_result) def test_config_context_renders(self): - packs = ["orquesta_tests", "dummy_pack_7"] - - self.install_packs(packs) config_value = "testing" - wf_name = 'orquesta_tests.render_config_context' + wf_name = 'examples.render_config_context' expected_output = {'context_value': config_value} expected_result = {'output': expected_output} diff --git a/st2tests/st2tests/base.py b/st2tests/st2tests/base.py index 75d6650d29..3fdc91816b 100644 --- a/st2tests/st2tests/base.py +++ b/st2tests/st2tests/base.py @@ -47,10 +47,8 @@ from st2common.exceptions.db import StackStormDBObjectConflictError from st2common.models.db import db_setup, db_teardown, db_ensure_indexes from st2common.models.db.execution_queue import ActionExecutionSchedulingQueueItemDB -from st2common.bootstrap.actionsregistrar import ActionsRegistrar from st2common.bootstrap.base import ResourceRegistrar from st2common.bootstrap.configsregistrar import ConfigsRegistrar -from st2common.bootstrap import runnersregistrar from st2common.content.utils import get_packs_base_paths from st2common.content.loader import MetaLoader from st2common.exceptions.db import StackStormDBObjectNotFoundError @@ -60,7 +58,6 @@ from st2common.services import workflows as wf_svc from st2common.util import api as api_util from st2common.util import loader -from st2common.util.virtualenvs import setup_pack_virtualenv import st2common.models.db.rule as rule_model import st2common.models.db.rule_enforcement as rule_enforcement_model import st2common.models.db.sensor as sensor_model @@ -86,7 +83,6 @@ 'DbTestCase', 'DbModelTestCase', 'CleanDbTestCase', - 'InstallFixturesPacks', 'CleanFilesTestCase', 'IntegrationTestCase', 'RunnerTestCase', @@ -469,69 +465,6 @@ def setUp(self): self._register_pack_configs() -class InstallFixturesPacks(DbTestCase): - """ - Class installs Fixtures packs according to passed pack list - """ - def install_packs(self, packs): - self._setup_env() - self._create_pack_virtualenv(packs) - self._register_packs(packs) - self._register_pack_configs(packs) - - def delete_files(self): - try: - shutil.rmtree(self.virtualenvs_path) - except Exception: - pass - - def _setup_env(self): - self.pack_path = get_packs_base_paths()[0] - self.virtualenvs_path = os.path.join(self.pack_path, 'virtualenvs/') - - def _create_pack_virtualenv(self, packs): - self._create_virtual_path() - - for pack_name in packs: - # Create virtualenv - setup_pack_virtualenv(pack_name=pack_name, update=False, include_pip=False, - include_setuptools=False, include_wheel=False) - - def _register_packs(self, packs, validate_configs=False): - """ - Register all the packs inside the fixtures directory. - """ - # Register runners. - runnersregistrar.register_runners() - - # Register actions. - actions_registrar = ActionsRegistrar(use_pack_cache=False, fail_on_failure=True) - for pack_name in packs: - actions_registrar.register_from_pack(self.pack_path + '/' + pack_name) - - # Register packs. - registrar = ResourceRegistrar(use_pack_cache=False, use_runners_cache=True) - for pack_name in packs: - registrar._register_pack_db(pack_name=pack_name, pack_dir=self.pack_path + '/' + - pack_name) - - def _register_pack_configs(self, packs, validate_configs=False): - """ - Register all the packs inside the fixtures directory. - """ - registrar = ConfigsRegistrar(use_pack_cache=False, validate_configs=validate_configs) - for pack_name in packs: - config_path = registrar._get_config_path_for_pack(pack_name) - if os.path.exists(config_path): - registrar._register_config_for_pack(pack=pack_name, config_path=config_path) - - def _create_virtual_path(self): - if os.path.isdir(self.virtualenvs_path): - self.delete_files() - - os.mkdir(self.virtualenvs_path) - - class CleanFilesTestCase(TestCase): """ Base test class which deletes specified files and directories on setUp and `tearDown. diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 764bcc129c..3951d105b4 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -204,6 +204,8 @@ function st2start(){ if [ "$copy_examples" = true ]; then echo "Copying examples from ./contrib/examples to $PACKS_BASE_DIR" cp -Rp ./contrib/examples $PACKS_BASE_DIR + cp -Rp ./st2tests/st2tests/fixtures/packs/dummy_pack_7 $PACKS_BASE_DIR + cp -p ./st2tests/st2tests/fixtures/packs/configs/dummy_pack_7.yaml $CONFIG_BASE_DIR fi # activate virtualenv to set PYTHONPATH @@ -354,6 +356,10 @@ function st2start(){ --log-file "$LOGDIR/mistral-api.log" fi + if [ "$copy_examples" = true ]; then + st2 run packs.setup_virtualenv packs=dummy_pack_7 + fi + # Check whether screen sessions are started SCREENS=( "st2-api" From 98eaf8e863c75b621a5536bc837376412adba25b Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Sat, 9 Mar 2019 17:20:45 -0800 Subject: [PATCH 16/29] Change integration test pack from `dummy_pack_7` to `tests`. Copy dummy_pack_7 to tests pack Move setting up pack `virtualenv` to the end to make sure st2client and st2api are up running --- .../actions/workflows/render_config_context.yaml | 2 +- st2tests/tests/actions/render.py | 7 +++++++ st2tests/tests/actions/render.yaml | 12 ++++++++++++ st2tests/tests/config.schema.yaml | 5 +++++ st2tests/tests/configs/tests.yaml.yaml | 2 ++ st2tests/tests/pack.yaml | 6 ++++++ tools/launchdev.sh | 12 ++++++------ 7 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 st2tests/tests/actions/render.py create mode 100644 st2tests/tests/actions/render.yaml create mode 100644 st2tests/tests/config.schema.yaml create mode 100644 st2tests/tests/configs/tests.yaml.yaml create mode 100644 st2tests/tests/pack.yaml diff --git a/contrib/examples/actions/workflows/render_config_context.yaml b/contrib/examples/actions/workflows/render_config_context.yaml index 2d05f1dc51..3ff64f6cf2 100644 --- a/contrib/examples/actions/workflows/render_config_context.yaml +++ b/contrib/examples/actions/workflows/render_config_context.yaml @@ -2,6 +2,6 @@ version: 1.0 description: Testing config context render". tasks: task1: - action: dummy_pack_7.render + action: tests.render output: - context_value: <% task(task1).result.result.context_value %> diff --git a/st2tests/tests/actions/render.py b/st2tests/tests/actions/render.py new file mode 100644 index 0000000000..97ab48fea0 --- /dev/null +++ b/st2tests/tests/actions/render.py @@ -0,0 +1,7 @@ +from st2common.runners.base_action import Action + + +class PrintPythonVersionAction(Action): + + def run(self, value1): + return {"context_value": value1} diff --git a/st2tests/tests/actions/render.yaml b/st2tests/tests/actions/render.yaml new file mode 100644 index 0000000000..3f11fc264b --- /dev/null +++ b/st2tests/tests/actions/render.yaml @@ -0,0 +1,12 @@ +--- +name: render +runner_type: python-script +description: Action that uses config context +enabled: true +entry_point: render.py +parameters: + value1: + description: Input for action1. Defaults to config_context value. + required: false + type: "string" + default: "{{ config_context.config_item_one }}" diff --git a/st2tests/tests/config.schema.yaml b/st2tests/tests/config.schema.yaml new file mode 100644 index 0000000000..731f42d98c --- /dev/null +++ b/st2tests/tests/config.schema.yaml @@ -0,0 +1,5 @@ +--- +config_item_one: + description: "Item use to test config context." + type: "string" + required: true diff --git a/st2tests/tests/configs/tests.yaml.yaml b/st2tests/tests/configs/tests.yaml.yaml new file mode 100644 index 0000000000..6b01d06715 --- /dev/null +++ b/st2tests/tests/configs/tests.yaml.yaml @@ -0,0 +1,2 @@ +--- +config_item_one: "testing" diff --git a/st2tests/tests/pack.yaml b/st2tests/tests/pack.yaml new file mode 100644 index 0000000000..9ca7158cd0 --- /dev/null +++ b/st2tests/tests/pack.yaml @@ -0,0 +1,6 @@ +--- +name : tests +description : dummy pack +version : 0.1.0 +author : st2-dev +email : info@stackstorm.com diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 3951d105b4..233d225961 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -204,8 +204,8 @@ function st2start(){ if [ "$copy_examples" = true ]; then echo "Copying examples from ./contrib/examples to $PACKS_BASE_DIR" cp -Rp ./contrib/examples $PACKS_BASE_DIR - cp -Rp ./st2tests/st2tests/fixtures/packs/dummy_pack_7 $PACKS_BASE_DIR - cp -p ./st2tests/st2tests/fixtures/packs/configs/dummy_pack_7.yaml $CONFIG_BASE_DIR + cp -Rp ./st2tests/tests $PACKS_BASE_DIR + cp -p ./st2tests/tests/configs/tests.yaml $CONFIG_BASE_DIR fi # activate virtualenv to set PYTHONPATH @@ -356,10 +356,6 @@ function st2start(){ --log-file "$LOGDIR/mistral-api.log" fi - if [ "$copy_examples" = true ]; then - st2 run packs.setup_virtualenv packs=dummy_pack_7 - fi - # Check whether screen sessions are started SCREENS=( "st2-api" @@ -397,6 +393,10 @@ function st2start(){ --config-file $ST2_CONF --register-all fi + if [ "$copy_examples" = true ]; then + st2 run packs.setup_virtualenv packs=tests + fi + # List screen sessions screen -ls || exit 0 } From c5883a80cae60a451243c63ca07950f987458311 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Tue, 12 Mar 2019 14:43:52 -0700 Subject: [PATCH 17/29] Move tests pack from st2 repo to st2test repo. 1. Move tests pack action to https://github.com/StackStorm/st2tests/tree/master/packs/tests Add a default value to config.schema.yaml and renamed action `render` to `render_config_context` Please see https://github.com/StackStorm/st2tests/pull/150 2. Made change in CHANGELOG.rst 3. Rename copy_examples to copy_test_packs in `tools/launchdev.sh` 4. Handle gracefully when authentication is on for setting up virtualenv. 5. Renamed action `render` to `render_config_context` for unit test dummy_pack_7 --- .../workflows/render_config_context.yaml | 2 +- .../tests/unit/services/test_workflow.py | 2 +- st2tests/integration/orquesta/test_wiring.py | 2 +- .../{render.py => render_config_context.py} | 0 ...render.yaml => render_config_context.yaml} | 6 ++-- .../workflows/render_config_context.yaml | 2 +- st2tests/tests/actions/render.py | 7 ----- st2tests/tests/actions/render.yaml | 12 ------- st2tests/tests/config.schema.yaml | 5 --- st2tests/tests/configs/tests.yaml.yaml | 2 -- st2tests/tests/pack.yaml | 6 ---- tools/launchdev.sh | 31 ++++++++++++++----- 12 files changed, 30 insertions(+), 47 deletions(-) rename st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/{render.py => render_config_context.py} (100%) rename st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/{render.yaml => render_config_context.yaml} (57%) delete mode 100644 st2tests/tests/actions/render.py delete mode 100644 st2tests/tests/actions/render.yaml delete mode 100644 st2tests/tests/config.schema.yaml delete mode 100644 st2tests/tests/configs/tests.yaml.yaml delete mode 100644 st2tests/tests/pack.yaml diff --git a/contrib/examples/actions/workflows/render_config_context.yaml b/contrib/examples/actions/workflows/render_config_context.yaml index 3ff64f6cf2..24f2d5394d 100644 --- a/contrib/examples/actions/workflows/render_config_context.yaml +++ b/contrib/examples/actions/workflows/render_config_context.yaml @@ -2,6 +2,6 @@ version: 1.0 description: Testing config context render". tasks: task1: - action: tests.render + action: tests.render_config_context output: - context_value: <% task(task1).result.result.context_value %> diff --git a/st2common/tests/unit/services/test_workflow.py b/st2common/tests/unit/services/test_workflow.py index b1021fae41..735b56788c 100644 --- a/st2common/tests/unit/services/test_workflow.py +++ b/st2common/tests/unit/services/test_workflow.py @@ -396,7 +396,7 @@ def test_request_action_execution_render(self): 'spec': task_spec, 'ctx': task_ctx, 'actions': [ - {'action': 'dummy_pack_7.render', 'input': None} + {'action': 'dummy_pack_7.render_config_context', 'input': None} ] } workflow_service.request_task_execution(wf_ex_db, st2_ctx, task_ex_req) diff --git a/st2tests/integration/orquesta/test_wiring.py b/st2tests/integration/orquesta/test_wiring.py index 04eacbef20..297a171bb0 100644 --- a/st2tests/integration/orquesta/test_wiring.py +++ b/st2tests/integration/orquesta/test_wiring.py @@ -148,7 +148,7 @@ def test_output_on_error(self): self.assertDictEqual(ex.result, expected_result) def test_config_context_renders(self): - config_value = "testing" + config_value = "Testing" wf_name = 'examples.render_config_context' expected_output = {'context_value': config_value} diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.py b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render_config_context.py similarity index 100% rename from st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.py rename to st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render_config_context.py diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render_config_context.yaml similarity index 57% rename from st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml rename to st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render_config_context.yaml index 3f11fc264b..e67d323c65 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render.yaml +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_7/actions/render_config_context.yaml @@ -1,12 +1,12 @@ --- -name: render +name: render_config_context runner_type: python-script description: Action that uses config context enabled: true -entry_point: render.py +entry_point: render_config_context.py parameters: value1: - description: Input for action1. Defaults to config_context value. + description: Input for render_config_context. Defaults to config_context value. required: false type: "string" default: "{{ config_context.config_item_one }}" diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/render_config_context.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/render_config_context.yaml index 2d05f1dc51..5683cbe98f 100644 --- a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/render_config_context.yaml +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/render_config_context.yaml @@ -2,6 +2,6 @@ version: 1.0 description: Testing config context render". tasks: task1: - action: dummy_pack_7.render + action: dummy_pack_7.render_config_context output: - context_value: <% task(task1).result.result.context_value %> diff --git a/st2tests/tests/actions/render.py b/st2tests/tests/actions/render.py deleted file mode 100644 index 97ab48fea0..0000000000 --- a/st2tests/tests/actions/render.py +++ /dev/null @@ -1,7 +0,0 @@ -from st2common.runners.base_action import Action - - -class PrintPythonVersionAction(Action): - - def run(self, value1): - return {"context_value": value1} diff --git a/st2tests/tests/actions/render.yaml b/st2tests/tests/actions/render.yaml deleted file mode 100644 index 3f11fc264b..0000000000 --- a/st2tests/tests/actions/render.yaml +++ /dev/null @@ -1,12 +0,0 @@ ---- -name: render -runner_type: python-script -description: Action that uses config context -enabled: true -entry_point: render.py -parameters: - value1: - description: Input for action1. Defaults to config_context value. - required: false - type: "string" - default: "{{ config_context.config_item_one }}" diff --git a/st2tests/tests/config.schema.yaml b/st2tests/tests/config.schema.yaml deleted file mode 100644 index 731f42d98c..0000000000 --- a/st2tests/tests/config.schema.yaml +++ /dev/null @@ -1,5 +0,0 @@ ---- -config_item_one: - description: "Item use to test config context." - type: "string" - required: true diff --git a/st2tests/tests/configs/tests.yaml.yaml b/st2tests/tests/configs/tests.yaml.yaml deleted file mode 100644 index 6b01d06715..0000000000 --- a/st2tests/tests/configs/tests.yaml.yaml +++ /dev/null @@ -1,2 +0,0 @@ ---- -config_item_one: "testing" diff --git a/st2tests/tests/pack.yaml b/st2tests/tests/pack.yaml deleted file mode 100644 index 9ca7158cd0..0000000000 --- a/st2tests/tests/pack.yaml +++ /dev/null @@ -1,6 +0,0 @@ ---- -name : tests -description : dummy pack -version : 0.1.0 -author : st2-dev -email : info@stackstorm.com diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 233d225961..a045b67659 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -7,7 +7,7 @@ function usage() { subcommand=$1; shift runner_count=1 use_gunicorn=true -copy_examples=false +copy_test_packs=false load_content=true use_ipv6=false include_mistral=false @@ -21,7 +21,7 @@ while getopts ":r:gxcu6m" o; do use_gunicorn=false ;; x) - copy_examples=true + copy_test_packs=true ;; c) load_content=false @@ -201,11 +201,10 @@ function st2start(){ cp -Rp ./contrib/core/ $PACKS_BASE_DIR cp -Rp ./contrib/packs/ $PACKS_BASE_DIR - if [ "$copy_examples" = true ]; then - echo "Copying examples from ./contrib/examples to $PACKS_BASE_DIR" + if [ "$copy_test_packs" = true ]; then + echo "Copying test packs examples and tests to $PACKS_BASE_DIR" cp -Rp ./contrib/examples $PACKS_BASE_DIR - cp -Rp ./st2tests/tests $PACKS_BASE_DIR - cp -p ./st2tests/tests/configs/tests.yaml $CONFIG_BASE_DIR + cp -Rp ./st2tests/packs/tests $PACKS_BASE_DIR fi # activate virtualenv to set PYTHONPATH @@ -393,8 +392,24 @@ function st2start(){ --config-file $ST2_CONF --register-all fi - if [ "$copy_examples" = true ]; then - st2 run packs.setup_virtualenv packs=tests + if [ "$copy_test_packs" = true ]; then + # Check if authentication is enabled + while read -r line; do + if [ "$line" == "[auth]" ]; then + found_auth=true + fi + + if [ "$found_auth" = true ]; then + typeset -l line + if [ "$line" = "enable = true" ]; then + echo "Warning: Please setup virtualenv for pack \"tests\" before run integration test" + break + elif [ "$line" = "enable = false" ]; then + st2 run packs.setup_virtualenv packs=tests + break + fi + fi + done < "$ST2_CONF" fi # List screen sessions From 65c2f2d640db422ec849aa0dbacb544eeb586cb1 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Tue, 12 Mar 2019 16:07:15 -0700 Subject: [PATCH 18/29] Add step to clone st2tests repo and copy tests pack Clone st2tests repo to /tmp directory Copy st2tests/tests pack to /opt/stackstorm/packs Remove /tmp/st2tests directory --- tools/launchdev.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tools/launchdev.sh b/tools/launchdev.sh index a045b67659..69d9b22781 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -204,7 +204,17 @@ function st2start(){ if [ "$copy_test_packs" = true ]; then echo "Copying test packs examples and tests to $PACKS_BASE_DIR" cp -Rp ./contrib/examples $PACKS_BASE_DIR - cp -Rp ./st2tests/packs/tests $PACKS_BASE_DIR + # Clone st2tests in /tmp directory. + pushd /tmp + git clone https://github.com/StackStorm/st2tests.git + ret=$? + if [ ${ret} -eq 0 ]; then + cp -Rp ./st2tests/packs/tests $PACKS_BASE_DIR + rm -R st2tests/ + else + echo "Failed to clone st2tests repo" + fi + popd fi # activate virtualenv to set PYTHONPATH From 095edf46ba0f49833567e0fada6094dc0b0d7a15 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Tue, 12 Mar 2019 16:16:31 -0700 Subject: [PATCH 19/29] Remove unused import: `mock` --- st2tests/integration/orquesta/test_wiring.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/st2tests/integration/orquesta/test_wiring.py b/st2tests/integration/orquesta/test_wiring.py index 297a171bb0..e62e957377 100644 --- a/st2tests/integration/orquesta/test_wiring.py +++ b/st2tests/integration/orquesta/test_wiring.py @@ -17,8 +17,6 @@ from __future__ import absolute_import -import mock - from integration.orquesta import base from st2common.constants import action as ac_const From 07251f5d9ccc17338f6d9c01f3ce2e07cfa46051 Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Wed, 13 Mar 2019 11:30:59 -0700 Subject: [PATCH 20/29] Remove authentication setup checking for simplicity If `st2 run packs.setup_virtualenv packs=tests` returns an error, then warn users --- tools/launchdev.sh | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 69d9b22781..9d699ba224 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -403,23 +403,10 @@ function st2start(){ fi if [ "$copy_test_packs" = true ]; then - # Check if authentication is enabled - while read -r line; do - if [ "$line" == "[auth]" ]; then - found_auth=true - fi - - if [ "$found_auth" = true ]; then - typeset -l line - if [ "$line" = "enable = true" ]; then - echo "Warning: Please setup virtualenv for pack \"tests\" before run integration test" - break - elif [ "$line" = "enable = false" ]; then - st2 run packs.setup_virtualenv packs=tests - break - fi - fi - done < "$ST2_CONF" + st2 run packs.setup_virtualenv packs=tests + if [ $? != 0 ]; then + echo "Warning: Please setup virtualenv for pack \"tests\" before run integration test" + fi fi # List screen sessions From 1e945930bdc9802b7f2e6bdf58334effcdd44a0b Mon Sep 17 00:00:00 2001 From: Jinping Han Date: Wed, 13 Mar 2019 11:43:11 -0700 Subject: [PATCH 21/29] Change warning message. Change the message to Unable to setup virtualenv for the \"tests\" pack. Please setup virtualenv for the \"tests\" pack before running integration tests. --- tools/launchdev.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 9d699ba224..63debe2466 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -405,7 +405,7 @@ function st2start(){ if [ "$copy_test_packs" = true ]; then st2 run packs.setup_virtualenv packs=tests if [ $? != 0 ]; then - echo "Warning: Please setup virtualenv for pack \"tests\" before run integration test" + echo "Warning: Unable to setup virtualenv for the \"tests\" pack. Please setup virtualenv for the \"tests\" pack before running integration tests" fi fi From 95b6058bd528ce106f49084c6e32f7b6f7103e7d Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 08:58:55 +0100 Subject: [PATCH 22/29] Add changelog entry. --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3ae23e37cf..7a0615db66 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,8 @@ Fixed * Make sure we don't log auth token and api key inside st2api log file if those values are provided via query parameter and not header (``?x-auth-token=foo``, ``?st2-api-key=bar``). (bug fix) #4592 #4589 +* Fix rendering of ``{{ config_context. }}`` in orquesta task that references action from a + different pack (bug fix) #4570 #4567 2.10.3 - March 06, 2019 ----------------------- From 8ee02a71d3a4b4fe4df2ad560cb7bc3771bb3e9e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 19:57:38 +0100 Subject: [PATCH 23/29] Fix some services (st2actionrunner, st2scheduler, st2workflowengine) to correctly use default config file from /etc/st2/st2.conf if one is not explicitly provided using --config-file command line argument. --- st2actions/st2actions/config.py | 4 +++- st2actions/st2actions/scheduler/config.py | 4 +++- st2actions/st2actions/workflows/config.py | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/st2actions/st2actions/config.py b/st2actions/st2actions/config.py index 4e03772e37..fbd661a59d 100644 --- a/st2actions/st2actions/config.py +++ b/st2actions/st2actions/config.py @@ -22,12 +22,14 @@ import st2common.config as common_config from st2common.constants.system import VERSION_STRING +from st2common.constants.system import DEFAULT_CONFIG_FILE_PATH CONF = cfg.CONF def parse_args(args=None): - CONF(args=args, version=VERSION_STRING) + CONF(args=args, version=VERSION_STRING, + default_config_files=[DEFAULT_CONFIG_FILE_PATH]) def register_opts(): diff --git a/st2actions/st2actions/scheduler/config.py b/st2actions/st2actions/scheduler/config.py index 27edfd6634..54addb77fb 100644 --- a/st2actions/st2actions/scheduler/config.py +++ b/st2actions/st2actions/scheduler/config.py @@ -19,6 +19,7 @@ from st2common import config as common_config from st2common.constants import system as sys_constants +from st2common.constants.system import DEFAULT_CONFIG_FILE_PATH from st2common import log as logging @@ -26,7 +27,8 @@ def parse_args(args=None): - cfg.CONF(args=args, version=sys_constants.VERSION_STRING) + cfg.CONF(args=args, version=sys_constants.VERSION_STRING, + default_config_files=[DEFAULT_CONFIG_FILE_PATH]) def register_opts(): diff --git a/st2actions/st2actions/workflows/config.py b/st2actions/st2actions/workflows/config.py index ac659ef29c..b5fcfb9e23 100644 --- a/st2actions/st2actions/workflows/config.py +++ b/st2actions/st2actions/workflows/config.py @@ -19,10 +19,12 @@ from st2common import config as common_config from st2common.constants import system as sys_constants +from st2common.constants.system import DEFAULT_CONFIG_FILE_PATH def parse_args(args=None): - cfg.CONF(args=args, version=sys_constants.VERSION_STRING) + cfg.CONF(args=args, version=sys_constants.VERSION_STRING, + default_config_files=[DEFAULT_CONFIG_FILE_PATH]) def register_opts(): From a2e56180cc81c3008d50a628d5e2e7e643d23f0f Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 10:23:15 +0100 Subject: [PATCH 24/29] Add changelog entry for #4596. --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7a0615db66..29827dec3d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,8 @@ Fixed #4589 * Fix rendering of ``{{ config_context. }}`` in orquesta task that references action from a different pack (bug fix) #4570 #4567 +* Add missing default config location (``/etc/st2/st2.conf``) to the following services: + ``st2actionrunner``, ``st2scheduler``, ``st2workflowengine``. (bug fix) #4596 2.10.3 - March 06, 2019 ----------------------- From 75cef96415bb55b13afc2295021a873daab6bbcc Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 09:53:28 +0100 Subject: [PATCH 25/29] Treat any statsd metric driver related errors as non-fatal. Underneath, statsd uses UDP protocol which is "fire and forget" which means all the errors are non-fatal and don't propagate back to the application. There is an edge case with how statsd library establishes connection though. If hostname is used instead of an IP address and this hostname can't be resolved, this DNS resolution error will propagate all the way up and break the application. This behavior is of course not desired (all metric related operation errors should be non-fatal) and this PR fixes that. We simply ignore those errors and log them. --- .../metrics/drivers/statsd_driver.py | 20 ++++++++++++++++ st2common/st2common/util/misc.py | 24 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/metrics/drivers/statsd_driver.py b/st2common/st2common/metrics/drivers/statsd_driver.py index 163938f291..1d0823d3bb 100644 --- a/st2common/st2common/metrics/drivers/statsd_driver.py +++ b/st2common/st2common/metrics/drivers/statsd_driver.py @@ -13,14 +13,20 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging as stdlib_logging from numbers import Number import statsd from oslo_config import cfg +from st2common import log as logging from st2common.metrics.base import BaseMetricsDriver from st2common.metrics.utils import check_key from st2common.metrics.utils import get_full_key_name +from st2common.util.misc import ignore_and_log_exception + + +LOG = logging.getLogger(__name__) __all__ = [ 'StatsdDriver' @@ -30,11 +36,20 @@ class StatsdDriver(BaseMetricsDriver): """ StatsD Implementation of the metrics driver + + NOTE: Statsd uses UDP which is "fire and forget" and any kind of send error is not fatal. There + is an issue with python-statsd library though which doesn't ignore DNS resolution related error + and bubbles them all the way up. + + This of course breaks the application. Any kind of metric related errors should be considered + as non-fatal and not degrade application in any way if an error occurs and that's why we wrap + all the statsd library calls here to ignore the errors and just log them. """ def __init__(self): statsd.Connection.set_defaults(host=cfg.CONF.metrics.host, port=cfg.CONF.metrics.port, sample_rate=cfg.CONF.metrics.sample_rate) + @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) def time(self, key, time): """ Timer metric @@ -46,6 +61,7 @@ def time(self, key, time): timer = statsd.Timer('') timer.send(key, time) + @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) def inc_counter(self, key, amount=1): """ Increment counter @@ -57,6 +73,7 @@ def inc_counter(self, key, amount=1): counter = statsd.Counter(key) counter.increment(delta=amount) + @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) def dec_counter(self, key, amount=1): """ Decrement metric @@ -68,6 +85,7 @@ def dec_counter(self, key, amount=1): counter = statsd.Counter(key) counter.decrement(delta=amount) + @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) def set_gauge(self, key, value): """ Set gauge value. @@ -79,6 +97,7 @@ def set_gauge(self, key, value): gauge = statsd.Gauge(key) gauge.send(None, value) + @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) def inc_gauge(self, key, amount=1): """ Increment gauge value. @@ -90,6 +109,7 @@ def inc_gauge(self, key, amount=1): gauge = statsd.Gauge(key) gauge.increment(None, amount) + @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) def dec_gauge(self, key, amount=1): """ Decrement gauge value. diff --git a/st2common/st2common/util/misc.py b/st2common/st2common/util/misc.py index 0768e2cb35..60b6862eb9 100644 --- a/st2common/st2common/util/misc.py +++ b/st2common/st2common/util/misc.py @@ -15,10 +15,13 @@ from __future__ import absolute_import +import logging + import os import re import sys import collections +import functools import six @@ -26,7 +29,8 @@ 'prefix_dict_keys', 'compare_path_file_name', 'lowercase_value', - 'get_field_name_from_mongoengine_error' + 'get_field_name_from_mongoengine_error', + ] @@ -168,3 +172,21 @@ def get_field_name_from_mongoengine_error(exc): return match.groups()[0] return msg + + +def ignore_and_log_exception(exc_classes=(Exception,), logger=None, level=logging.WARNING): + """ + Decorator which catches the provided exception classes and logs them instead of letting them + bubble all the way up. + """ + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except exc_classes as e: + message = ('Exception in fuction "%s": %s' % (func.__name__, str(e))) + logger.log(level, message) + + return wrapper + return decorator From 406aa49f8a1b85e2483bb39b37a6142876eb9eae Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 10:27:25 +0100 Subject: [PATCH 26/29] Only treat socket errors as non-fatal. We still want AssertionErrors to propagate all the way up since those indicate programmer / developer error and should be caught during development. --- .../metrics/drivers/statsd_driver.py | 31 +++++++++++++------ st2common/st2common/util/misc.py | 2 ++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/st2common/st2common/metrics/drivers/statsd_driver.py b/st2common/st2common/metrics/drivers/statsd_driver.py index 1d0823d3bb..83a51275af 100644 --- a/st2common/st2common/metrics/drivers/statsd_driver.py +++ b/st2common/st2common/metrics/drivers/statsd_driver.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import socket import logging as stdlib_logging from numbers import Number @@ -28,6 +29,11 @@ LOG = logging.getLogger(__name__) +# Which exceptions thrown by statsd library should be considered as non-fatal +NON_FATAL_EXC_CLASSES = [ + socket.error, +] + __all__ = [ 'StatsdDriver' ] @@ -38,18 +44,20 @@ class StatsdDriver(BaseMetricsDriver): StatsD Implementation of the metrics driver NOTE: Statsd uses UDP which is "fire and forget" and any kind of send error is not fatal. There - is an issue with python-statsd library though which doesn't ignore DNS resolution related error + is an issue with python-statsd library though which doesn't ignore DNS resolution related errors and bubbles them all the way up. This of course breaks the application. Any kind of metric related errors should be considered - as non-fatal and not degrade application in any way if an error occurs and that's why we wrap - all the statsd library calls here to ignore the errors and just log them. + as non-fatal and not degrade application in any way if an error occurs. That's why we wrap all + the statsd library calls here to ignore the errors and just log them. """ + def __init__(self): statsd.Connection.set_defaults(host=cfg.CONF.metrics.host, port=cfg.CONF.metrics.port, sample_rate=cfg.CONF.metrics.sample_rate) - @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) + @ignore_and_log_exception(exc_classes=NON_FATAL_EXC_CLASSES, logger=LOG, + level=stdlib_logging.WARNING) def time(self, key, time): """ Timer metric @@ -61,7 +69,8 @@ def time(self, key, time): timer = statsd.Timer('') timer.send(key, time) - @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) + @ignore_and_log_exception(exc_classes=NON_FATAL_EXC_CLASSES, logger=LOG, + level=stdlib_logging.WARNING) def inc_counter(self, key, amount=1): """ Increment counter @@ -73,7 +82,8 @@ def inc_counter(self, key, amount=1): counter = statsd.Counter(key) counter.increment(delta=amount) - @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) + @ignore_and_log_exception(exc_classes=NON_FATAL_EXC_CLASSES, logger=LOG, + level=stdlib_logging.WARNING) def dec_counter(self, key, amount=1): """ Decrement metric @@ -85,7 +95,8 @@ def dec_counter(self, key, amount=1): counter = statsd.Counter(key) counter.decrement(delta=amount) - @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) + @ignore_and_log_exception(exc_classes=NON_FATAL_EXC_CLASSES, logger=LOG, + level=stdlib_logging.WARNING) def set_gauge(self, key, value): """ Set gauge value. @@ -97,7 +108,8 @@ def set_gauge(self, key, value): gauge = statsd.Gauge(key) gauge.send(None, value) - @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) + @ignore_and_log_exception(exc_classes=NON_FATAL_EXC_CLASSES, logger=LOG, + level=stdlib_logging.WARNING) def inc_gauge(self, key, amount=1): """ Increment gauge value. @@ -109,7 +121,8 @@ def inc_gauge(self, key, amount=1): gauge = statsd.Gauge(key) gauge.increment(None, amount) - @ignore_and_log_exception(logger=LOG, level=stdlib_logging.WARNING) + @ignore_and_log_exception(exc_classes=NON_FATAL_EXC_CLASSES, logger=LOG, + level=stdlib_logging.WARNING) def dec_gauge(self, key, amount=1): """ Decrement gauge value. diff --git a/st2common/st2common/util/misc.py b/st2common/st2common/util/misc.py index 60b6862eb9..44d9d14d17 100644 --- a/st2common/st2common/util/misc.py +++ b/st2common/st2common/util/misc.py @@ -179,6 +179,8 @@ def ignore_and_log_exception(exc_classes=(Exception,), logger=None, level=loggin Decorator which catches the provided exception classes and logs them instead of letting them bubble all the way up. """ + exc_classes = tuple(exc_classes) + def decorator(func): @functools.wraps(func) def wrapper(*args, **kwargs): From f69f6c5275901faece82595000a6ee593b51c87f Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 10:46:55 +0100 Subject: [PATCH 27/29] Add test cases for it. --- .../metrics/drivers/statsd_driver.py | 2 + st2common/tests/unit/test_metrics.py | 52 ++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/metrics/drivers/statsd_driver.py b/st2common/st2common/metrics/drivers/statsd_driver.py index 83a51275af..2324ab4b71 100644 --- a/st2common/st2common/metrics/drivers/statsd_driver.py +++ b/st2common/st2common/metrics/drivers/statsd_driver.py @@ -32,6 +32,8 @@ # Which exceptions thrown by statsd library should be considered as non-fatal NON_FATAL_EXC_CLASSES = [ socket.error, + IOError, + OSError ] __all__ = [ diff --git a/st2common/tests/unit/test_metrics.py b/st2common/tests/unit/test_metrics.py index dbd2750432..1c7543ca7b 100644 --- a/st2common/tests/unit/test_metrics.py +++ b/st2common/tests/unit/test_metrics.py @@ -12,9 +12,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from datetime import datetime, timedelta + +import socket +from datetime import datetime +from datetime import timedelta import unittest2 +import mock from mock import patch, MagicMock from oslo_config import cfg @@ -200,6 +204,52 @@ def test_get_full_key_name(self): result = get_full_key_name('api.requests') self.assertEqual(result, 'st2.prod.api.requests') + @patch('st2common.metrics.drivers.statsd_driver.LOG') + @patch('st2common.metrics.drivers.statsd_driver.statsd') + def test_driver_socket_exceptions_are_not_fatal(self, statsd, mock_log): + # Socket errors such as DNS resolution errors should be considered non fatal and ignored + mock_logger = mock.Mock() + StatsdDriver.logger = mock_logger + + # 1. timer + mock_timer = MagicMock(side_effect=socket.error('error 1')) + statsd.Timer('').send.side_effect = mock_timer + params = ('test', 10) + self._driver.time(*params) + statsd.Timer('').send.assert_called_with('st2.test', 10) + + # 2. counter + key = 'test' + mock_counter = MagicMock(side_effect=socket.error('error 2')) + statsd.Counter(key).increment.side_effect = mock_counter + self._driver.inc_counter(key) + mock_counter.assert_called_once_with(delta=1) + + key = 'test' + mock_counter = MagicMock(side_effect=socket.error('error 3')) + statsd.Counter(key).decrement.side_effect = mock_counter + self._driver.dec_counter(key) + mock_counter.assert_called_once_with(delta=1) + + # 3. gauge + params = ('key', 100) + mock_gauge = MagicMock(side_effect=socket.error('error 4')) + statsd.Gauge().send.side_effect = mock_gauge + self._driver.set_gauge(*params) + mock_gauge.assert_called_once_with(None, params[1]) + + params = ('key1',) + mock_gauge = MagicMock(side_effect=socket.error('error 5')) + statsd.Gauge().increment.side_effect = mock_gauge + self._driver.inc_gauge(*params) + mock_gauge.assert_called_once_with(None, 1) + + params = ('key1',) + mock_gauge = MagicMock(side_effect=socket.error('error 6')) + statsd.Gauge().decrement.side_effect = mock_gauge + self._driver.dec_gauge(*params) + mock_gauge.assert_called_once_with(None, 1) + class TestCounterContextManager(unittest2.TestCase): @patch('st2common.metrics.base.METRICS') From bd4caca0d298d3dedec0b2defcf1ea45ab6eba47 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 10:58:40 +0100 Subject: [PATCH 28/29] Also include class name in the log message (if available). --- st2common/st2common/util/misc.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/util/misc.py b/st2common/st2common/util/misc.py index 44d9d14d17..02d8f31513 100644 --- a/st2common/st2common/util/misc.py +++ b/st2common/st2common/util/misc.py @@ -187,7 +187,12 @@ def wrapper(*args, **kwargs): try: return func(*args, **kwargs) except exc_classes as e: - message = ('Exception in fuction "%s": %s' % (func.__name__, str(e))) + if len(args) >= 1 and getattr(args[0], '__class__', None): + func_name = '%s.%s' % (args[0].__class__.__name__, func.__name__) + else: + func_name = func.__name__ + + message = ('Exception in fuction "%s": %s' % (func_name, str(e))) logger.log(level, message) return wrapper From 0a7f0d3be3afb968fb0138c6d18d2ded2dc3aba1 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 14 Mar 2019 15:17:00 +0100 Subject: [PATCH 29/29] Cherry pick changelog entry. --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 29827dec3d..b503471209 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,6 +18,13 @@ Fixed different pack (bug fix) #4570 #4567 * Add missing default config location (``/etc/st2/st2.conf``) to the following services: ``st2actionrunner``, ``st2scheduler``, ``st2workflowengine``. (bug fix) #4596 +* Update statsd metrics driver so any exception thrown by statsd library is treated as non fatal. + + Previously there was an edge case if user used a hostname instead of an IP address for metrics + backend server address. In such scenario, if hostname DNS resolution failed, statsd driver would + throw the exception which would propagate all the way up and break the application. (bug fix) #4597 + + Reported by Chris McKenzie. 2.10.3 - March 06, 2019 -----------------------