From 5631305bada250f7ba545bc35f26506b94064375 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 09:21:01 +0100 Subject: [PATCH 1/3] 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 1015e437da9f77612d024c3b66bae13f438dc1aa Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 09:23:53 +0100 Subject: [PATCH 2/3] 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 00781e1ffffefd5c091da424f6e128c55017a216 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 13 Mar 2019 09:28:40 +0100 Subject: [PATCH 3/3] Add changelog entry for it. --- CHANGELOG.rst | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 499a76d654..325bddb103 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -30,13 +30,13 @@ Changed to make it easier for developers to understand. (improvement) * Update Python runner code so it prioritizes libraries from pack virtual environment over StackStorm system dependencies. - + For example, if pack depends on ``six==1.11.0`` in pack ``requirements.txt``, but StackStorm depends on ``six==1.10.0``, ``six==1.11.0`` will be used when running Python actions from that pack. - + Keep in mind that will not work correctly if pack depends on a library which brakes functionality used by Python action wrapper code. - + Contributed by Hiroyasu OHYAMA (@userlocalhost). #4571 Fixed @@ -49,6 +49,11 @@ Fixed with items task. (bug fix) #4523 * Fix orquesta workflow bug where context variables are being overwritten on task join. (bug fix) StackStorm/orquesta#112 +* 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 2.10.3 - March 06, 2019 ----------------------- @@ -60,7 +65,7 @@ Fixed with ``null`` for the ``Access-Control-Allow-Origin`` header. The fix returns the first of our allowed origins if the requesting origin is not a supported origin. Reported by Barak Tawily. (bug fix) - + 2.9.3 - March 06, 2019 ----------------------- @@ -134,7 +139,7 @@ Fixed Reported by @johandahlberg (bug fix) #4533 * Fix ``core.sendmail`` action so it specifies ``charset=UTF-8`` in the ``Content-Type`` email header. This way it works correctly when an email subject and / or body contains unicode data. - + Reported by @johandahlberg (bug fix) #4533 4534 * Fix CLI ``st2 apikey load`` not being idempotent and API endpoint ``/api/v1/apikeys`` not