From 3d8b25569269c94cc046a9f257a91b54b9d6bb72 Mon Sep 17 00:00:00 2001 From: W Chan Date: Sat, 23 May 2020 20:45:21 +0000 Subject: [PATCH] Fix empty with items list resulting in parent workflow stuck in running Fix a bug where passing an empty list to a with items task in a subworkflow causes the parent workflow to be stuck in running status. The workflow engine usually skips execution of the with items task because the list is empty. The issue here is that when the task is marked complete, the status change in the task wasn't published which will triggered additional logic to handle completion and move the parent workflow forward. This patch publishes the completeion status of the task now. --- CHANGELOG.rst | 2 ++ ...sta-test-subworkflow-empty-with-items.yaml | 6 ++++ ...sta-test-subworkflow-empty-with-items.yaml | 12 ++++++++ .../tests/unit/test_with_items.py | 28 +++++++++++++++++++ st2common/st2common/services/workflows.py | 2 +- .../orquesta/test_wiring_with_items.py | 7 +++++ .../actions/with-items-empty-parent.yaml | 12 ++++++++ .../actions/with-items-empty.yaml | 12 ++++++++ .../workflows/with-items-empty-parent.yaml | 10 +++++++ .../actions/workflows/with-items-empty.yaml | 11 ++++++++ 10 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 contrib/examples/actions/orquesta-test-subworkflow-empty-with-items.yaml create mode 100644 contrib/examples/actions/workflows/tests/orquesta-test-subworkflow-empty-with-items.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty-parent.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty-parent.yaml create mode 100644 st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty.yaml diff --git a/CHANGELOG.rst b/CHANGELOG.rst index eb47978e58..12e75aa25f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -24,6 +24,8 @@ Fixed their state to the MongoDB database. (bug fix) #4932 Contributed by Nick Maludy (@nmaludy Encore Technologies) +* Fix a bug where passing an empty list to a with items task in a subworkflow causes + the parent workflow to be stuck in running status. (bug fix) #4954 3.2.0 - April 27, 2020 diff --git a/contrib/examples/actions/orquesta-test-subworkflow-empty-with-items.yaml b/contrib/examples/actions/orquesta-test-subworkflow-empty-with-items.yaml new file mode 100644 index 0000000000..e22cf7bd45 --- /dev/null +++ b/contrib/examples/actions/orquesta-test-subworkflow-empty-with-items.yaml @@ -0,0 +1,6 @@ +--- +name: orquesta-test-subworkflow-empty-with-items +description: A workflow for testing passing an emtpy list to with items task in a subworkflow. +runner_type: orquesta +entry_point: workflows/tests/orquesta-test-subworkflow-empty-with-items.yaml +enabled: true diff --git a/contrib/examples/actions/workflows/tests/orquesta-test-subworkflow-empty-with-items.yaml b/contrib/examples/actions/workflows/tests/orquesta-test-subworkflow-empty-with-items.yaml new file mode 100644 index 0000000000..99508ec08e --- /dev/null +++ b/contrib/examples/actions/workflows/tests/orquesta-test-subworkflow-empty-with-items.yaml @@ -0,0 +1,12 @@ +version: 1.0 + +tasks: + task1: + action: examples.orquesta-test-with-items + input: + tempfiles: [] + next: + - when: <% succeeded() %> + do: task2 + task2: + action: core.noop diff --git a/contrib/runners/orquesta_runner/tests/unit/test_with_items.py b/contrib/runners/orquesta_runner/tests/unit/test_with_items.py index bbc10f0035..31c0a10470 100644 --- a/contrib/runners/orquesta_runner/tests/unit/test_with_items.py +++ b/contrib/runners/orquesta_runner/tests/unit/test_with_items.py @@ -582,3 +582,31 @@ def test_with_items_concurrency_pause_and_resume(self): self.assertEqual(wf_ex_db.status, wf_statuses.SUCCEEDED) lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) self.assertEqual(lv_ac_db.status, action_constants.LIVEACTION_STATUS_SUCCEEDED) + + def test_subworkflow_with_items_empty_list(self): + wf_input = {'members': []} + wf_meta = base.get_wf_fixture_meta_data(TEST_PACK_PATH, 'with-items-empty-parent.yaml') + lv_ac_db = lv_db_models.LiveActionDB(action=wf_meta['name'], parameters=wf_input) + lv_ac_db, ac_ex_db = action_service.request(lv_ac_db) + + # Identify the records for the main workflow. + wf_ex_db = wf_db_access.WorkflowExecution.query(action_execution=str(ac_ex_db.id))[0] + tk_ex_dbs = wf_db_access.TaskExecution.query(workflow_execution=str(wf_ex_db.id)) + self.assertEqual(len(tk_ex_dbs), 1) + + # Identify the records for the tasks. + t1_ac_ex_db = ex_db_access.ActionExecution.query(task_execution=str(tk_ex_dbs[0].id))[0] + t1_wf_ex_db = wf_db_access.WorkflowExecution.query(action_execution=str(t1_ac_ex_db.id))[0] + self.assertEqual(t1_ac_ex_db.status, action_constants.LIVEACTION_STATUS_SUCCEEDED) + self.assertEqual(t1_wf_ex_db.status, wf_statuses.SUCCEEDED) + + # Manually processing completion of the subworkflow in task1. + workflows.get_engine().process(t1_ac_ex_db) + t1_ex_db = wf_db_access.TaskExecution.get_by_id(tk_ex_dbs[0].id) + self.assertEqual(t1_ex_db.status, wf_statuses.SUCCEEDED) + + # Check that the workflow execution is completed. + wf_ex_db = wf_db_access.WorkflowExecution.get_by_id(wf_ex_db.id) + self.assertEqual(wf_ex_db.status, wf_statuses.SUCCEEDED) + lv_ac_db = lv_db_access.LiveAction.get_by_id(str(lv_ac_db.id)) + self.assertEqual(lv_ac_db.status, action_constants.LIVEACTION_STATUS_SUCCEEDED) diff --git a/st2common/st2common/services/workflows.py b/st2common/st2common/services/workflows.py index 9a075a5f80..ce4b4e31a8 100644 --- a/st2common/st2common/services/workflows.py +++ b/st2common/st2common/services/workflows.py @@ -595,7 +595,7 @@ def request_task_execution(wf_ex_db, st2_ctx, task_ex_req): # Fast forward task execution to completion. update_task_execution(str(task_ex_db.id), statuses.SUCCEEDED) - update_task_state(str(task_ex_db.id), statuses.SUCCEEDED, publish=False) + update_task_state(str(task_ex_db.id), statuses.SUCCEEDED) # Refresh and return the task execution return wf_db_access.TaskExecution.get_by_id(str(task_ex_db.id)) diff --git a/st2tests/integration/orquesta/test_wiring_with_items.py b/st2tests/integration/orquesta/test_wiring_with_items.py index ebfc4860dc..b527ec5a61 100644 --- a/st2tests/integration/orquesta/test_wiring_with_items.py +++ b/st2tests/integration/orquesta/test_wiring_with_items.py @@ -287,3 +287,10 @@ def test_with_items_concurrency_pause_and_resume(self): # Wait for completion. ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) + + def test_subworkflow_empty_with_items(self): + wf_name = 'examples.orquesta-test-subworkflow-empty-with-items' + ex = self._execute_workflow(wf_name) + ex = self._wait_for_completion(ex) + + self.assertEqual(ex.status, ac_const.LIVEACTION_STATUS_SUCCEEDED) diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty-parent.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty-parent.yaml new file mode 100644 index 0000000000..2a02dd5063 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty-parent.yaml @@ -0,0 +1,12 @@ +--- +name: with-items-empty-parent +description: A workflow to test passing of empty array to subworkflow that has a with items task. +pack: orquesta_tests +runner_type: orquesta +entry_point: workflows/with-items-empty-parent.yaml +enabled: true +parameters: + members: + required: true + type: array + default: [] diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty.yaml new file mode 100644 index 0000000000..111a8f2432 --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/with-items-empty.yaml @@ -0,0 +1,12 @@ +--- +name: with-items-empty +description: A workflow to test passing of empty array to with items task. +pack: orquesta_tests +runner_type: orquesta +entry_point: workflows/with-items-empty.yaml +enabled: true +parameters: + members: + required: true + type: array + default: [] diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty-parent.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty-parent.yaml new file mode 100644 index 0000000000..62f7af3f5f --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty-parent.yaml @@ -0,0 +1,10 @@ +version: 1.0 + +description: A workflow to test passing of empty array to subworkflow that has a with items task. + +input: + - members + +tasks: + task1: + action: orquesta_tests.with-items-empty members=<% ctx().members %> diff --git a/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty.yaml b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty.yaml new file mode 100644 index 0000000000..5c66e06e0e --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/orquesta_tests/actions/workflows/with-items-empty.yaml @@ -0,0 +1,11 @@ +version: 1.0 + +description: A workflow to test passing of empty array to with items task. + +input: + - members + +tasks: + task1: + with: <% ctx(members) %> + action: core.echo message="<% item() %>, resistance is futile!"