From 56a9978d560ca96c428429de6f153104adc05f6f Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 13 Oct 2016 12:44:13 -0400 Subject: [PATCH 01/10] Introduce workflow failure condition --- awx/api/serializers.py | 6 ++- .../0041_v310_workflow_failure_condition.py | 24 ++++++++++++ awx/main/models/workflow.py | 9 ++++- awx/main/scheduler/__init__.py | 8 ++-- .../tests/functional/models/test_workflow.py | 39 +++++++++++++++++++ 5 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 awx/main/migrations/0041_v310_workflow_failure_condition.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index d6aa39f706..4b5b411a08 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2249,11 +2249,15 @@ class WorkflowNodeBaseSerializer(BaseSerializer): success_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) failure_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) + fail_on_job_failure = serializers.BooleanField( + help_text=('If set to true, and if the job runs and fails, ' + 'the workflow will also be marked as failed.'), + default=True) class Meta: fields = ('*', '-name', '-description', 'id', 'url', 'related', 'unified_job_template', 'success_nodes', 'failure_nodes', 'always_nodes', - 'inventory', 'credential', 'job_type', 'job_tags', 'skip_tags', 'limit', 'skip_tags') + 'inventory', 'credential', 'job_type', 'job_tags', 'skip_tags', 'limit', 'skip_tags', 'fail_on_job_failure') def get_related(self, obj): res = super(WorkflowNodeBaseSerializer, self).get_related(obj) diff --git a/awx/main/migrations/0041_v310_workflow_failure_condition.py b/awx/main/migrations/0041_v310_workflow_failure_condition.py new file mode 100644 index 0000000000..8544f9884a --- /dev/null +++ b/awx/main/migrations/0041_v310_workflow_failure_condition.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0040_v310_artifacts'), + ] + + operations = [ + migrations.AddField( + model_name='workflowjobnode', + name='fail_on_job_failure', + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name='workflowjobtemplatenode', + name='fail_on_job_failure', + field=models.BooleanField(default=True), + ), + ] diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index 2848b38a4a..6ef5e88956 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -60,6 +60,10 @@ class WorkflowNodeBase(CreatedModifiedModel): default=None, on_delete=models.SET_NULL, ) + fail_on_job_failure = models.BooleanField( + blank=True, + default=True, + ) # Prompting-related fields inventory = models.ForeignKey( 'Inventory', @@ -137,7 +141,7 @@ class WorkflowNodeBase(CreatedModifiedModel): Return field names that should be copied from template node to job node. ''' return ['workflow_job', 'unified_job_template', - 'inventory', 'credential', 'char_prompts'] + 'inventory', 'credential', 'char_prompts', 'fail_on_job_failure'] class WorkflowJobTemplateNode(WorkflowNodeBase): # TODO: Ensure the API forces workflow_job_template being set @@ -383,6 +387,9 @@ class WorkflowJob(UnifiedJob, WorkflowJobOptions, JobNotificationMixin, Workflow from awx.main.tasks import RunWorkflowJob return RunWorkflowJob + def _has_failed(self): + return self.workflow_job_nodes.filter(job__status='failed', fail_on_job_failure=True).exists() + def socketio_emit_data(self): return {} diff --git a/awx/main/scheduler/__init__.py b/awx/main/scheduler/__init__.py index b93ce6956e..6ecdc09b37 100644 --- a/awx/main/scheduler/__init__.py +++ b/awx/main/scheduler/__init__.py @@ -73,10 +73,12 @@ def process_finished_workflow_jobs(workflow_jobs): dag = WorkflowDAG(workflow_job) if dag.is_workflow_done(): with transaction.atomic(): - # TODO: detect if wfj failed - workflow_job.status = 'completed' + if workflow_job._has_failed(): + workflow_job.status = 'failed' + else: + workflow_job.status = 'successful' workflow_job.save() - workflow_job.websocket_emit_status('completed') + workflow_job.websocket_emit_status(workflow_job.status) def rebuild_graph(): """Regenerate the task graph by refreshing known tasks from Tower, purging diff --git a/awx/main/tests/functional/models/test_workflow.py b/awx/main/tests/functional/models/test_workflow.py index cc61c34ed9..aa622d996a 100644 --- a/awx/main/tests/functional/models/test_workflow.py +++ b/awx/main/tests/functional/models/test_workflow.py @@ -90,3 +90,42 @@ class TestWorkflowJobTemplate: assert len(parent_qs) == 1 assert parent_qs[0] == wfjt.workflow_job_template_nodes.all()[1] +@pytest.mark.django_db +class TestWorkflowJobFailure: + @pytest.fixture + def wfj(self): + return WorkflowJob.objects.create(name='test-wf-job') + + def test_workflow_has_failed(self, wfj): + """ + Test that a single failed node with fail_on_job_failure = true + leads to the entire WF being marked as failed + """ + job = Job.objects.create(name='test-job', status='failed') + # Node has a failed job connected + WorkflowJobNode.objects.create(workflow_job=wfj, job=job) + assert wfj._has_failed() + + def test_workflow_not_failed_unran_job(self, wfj): + """ + Test that an un-ran node will not mark workflow job as failed + """ + WorkflowJobNode.objects.create(workflow_job=wfj) + assert not wfj._has_failed() + + def test_workflow_not_failed_successful_job(self, wfj): + """ + Test that a sucessful node will not mark workflow job as failed + """ + job = Job.objects.create(name='test-job', status='successful') + WorkflowJobNode.objects.create(workflow_job=wfj, job=job) + assert not wfj._has_failed() + + def test_workflow_not_failed_failed_job_but_okay(self, wfj): + """ + Test that a failed node will not mark workflow job as failed + if the fail_on_job_failure is set to false + """ + job = Job.objects.create(name='test-job', status='failed') + WorkflowJobNode.objects.create(workflow_job=wfj, job=job, fail_on_job_failure=False) + assert not wfj._has_failed() From 48fc9eb773663cbc9db445948fdc2cfd0d85bae4 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 14 Oct 2016 16:30:58 -0400 Subject: [PATCH 02/10] update workflow model unit tests --- awx/main/tests/unit/models/test_workflow_unit.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/awx/main/tests/unit/models/test_workflow_unit.py b/awx/main/tests/unit/models/test_workflow_unit.py index 9df61ffe97..bcbec85c81 100644 --- a/awx/main/tests/unit/models/test_workflow_unit.py +++ b/awx/main/tests/unit/models/test_workflow_unit.py @@ -139,6 +139,7 @@ class TestWorkflowJobCreate: char_prompts=wfjt_node_no_prompts.char_prompts, inventory=None, credential=None, unified_job_template=wfjt_node_no_prompts.unified_job_template, + fail_on_job_failure=True, workflow_job=workflow_job_unit) def test_create_with_prompts(self, wfjt_node_with_prompts, workflow_job_unit, mocker): @@ -150,6 +151,7 @@ class TestWorkflowJobCreate: inventory=wfjt_node_with_prompts.inventory, credential=wfjt_node_with_prompts.credential, unified_job_template=wfjt_node_with_prompts.unified_job_template, + fail_on_job_failure=True, workflow_job=workflow_job_unit) @mock.patch('awx.main.models.workflow.WorkflowNodeBase.get_parent_nodes', lambda self: []) From 10c847e8048055e37be2517a11425ea3425b02eb Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 17 Oct 2016 08:35:26 -0400 Subject: [PATCH 03/10] docs feedback for fojf incorporated --- awx/api/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 4b5b411a08..a463a06064 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2251,7 +2251,7 @@ class WorkflowNodeBaseSerializer(BaseSerializer): always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) fail_on_job_failure = serializers.BooleanField( help_text=('If set to true, and if the job runs and fails, ' - 'the workflow will also be marked as failed.'), + 'the workflow is marked as failed.'), default=True) class Meta: From e0173a5ca2174fc07bb07ba367b16e597da27048 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 25 Oct 2016 09:16:09 -0400 Subject: [PATCH 04/10] bump migration file number --- ...ilure_condition.py => 0045_v310_workflow_failure_condition.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename awx/main/migrations/{0041_v310_workflow_failure_condition.py => 0045_v310_workflow_failure_condition.py} (100%) diff --git a/awx/main/migrations/0041_v310_workflow_failure_condition.py b/awx/main/migrations/0045_v310_workflow_failure_condition.py similarity index 100% rename from awx/main/migrations/0041_v310_workflow_failure_condition.py rename to awx/main/migrations/0045_v310_workflow_failure_condition.py From 836c13f8571a3be69cfcc7b597eff31fb39097d9 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 25 Oct 2016 09:18:14 -0400 Subject: [PATCH 05/10] bump migration dependency --- awx/main/migrations/0045_v310_workflow_failure_condition.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/migrations/0045_v310_workflow_failure_condition.py b/awx/main/migrations/0045_v310_workflow_failure_condition.py index 8544f9884a..9bafa0feb6 100644 --- a/awx/main/migrations/0045_v310_workflow_failure_condition.py +++ b/awx/main/migrations/0045_v310_workflow_failure_condition.py @@ -7,7 +7,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('main', '0040_v310_artifacts'), + ('main', '0044_v310_project_playbook_files'), ] operations = [ From 04013e4d9f9daef86e0d82c33fdfae8f250d0a8c Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 25 Oct 2016 16:34:36 -0400 Subject: [PATCH 06/10] fix problems with OPTIONS for char_prompts --- awx/api/serializers.py | 26 +++++++++++--------------- awx/api/views.py | 5 +++++ awx/main/models/workflow.py | 16 ++++++++++++++++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index a463a06064..70e37b8220 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2242,10 +2242,10 @@ class WorkflowJobListSerializer(WorkflowJobSerializer, UnifiedJobListSerializer) pass class WorkflowNodeBaseSerializer(BaseSerializer): - job_type = serializers.SerializerMethodField() - job_tags = serializers.SerializerMethodField() - limit = serializers.SerializerMethodField() - skip_tags = serializers.SerializerMethodField() + job_type = serializers.CharField(allow_blank=True, required=False, default=None) + job_tags = serializers.CharField(allow_blank=True, required=False, default=None) + limit = serializers.CharField(allow_blank=True, required=False, default=None) + skip_tags = serializers.CharField(allow_blank=True, required=False, default=None) success_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) failure_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) @@ -2265,17 +2265,13 @@ class WorkflowNodeBaseSerializer(BaseSerializer): res['unified_job_template'] = obj.unified_job_template.get_absolute_url() return res - def get_job_type(self, obj): - return obj.char_prompts.get('job_type', None) - - def get_job_tags(self, obj): - return obj.char_prompts.get('job_tags', None) - - def get_skip_tags(self, obj): - return obj.char_prompts.get('skip_tags', None) - - def get_limit(self, obj): - return obj.char_prompts.get('limit', None) + def validate(self, attrs): + # char_prompts go through different validation, so remove them here + print ' attrs: ' + str(attrs) + for fd in ['job_type', 'job_tags', 'skip_tags', 'limit']: + if fd in attrs: + attrs.pop(fd) + return super(WorkflowNodeBaseSerializer, self).validate(attrs) class WorkflowJobTemplateNodeSerializer(WorkflowNodeBaseSerializer): diff --git a/awx/api/views.py b/awx/api/views.py index 66980e141d..2327f11b6a 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2751,6 +2751,11 @@ class WorkflowJobTemplateWorkflowNodesList(SubListCreateAPIView): relationship = 'workflow_job_template_nodes' parent_key = 'workflow_job_template' + def update_raw_data(self, data): + for fd in ['job_type', 'job_tags', 'skip_tags', 'limit', 'skip_tags']: + data[fd] = None + return super(WorkflowJobTemplateWorkflowNodesList, self).update_raw_data(data) + # TODO: class WorkflowJobTemplateJobsList(SubListAPIView): diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index 6ef5e88956..3bc0608cfc 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -97,6 +97,22 @@ class WorkflowNodeBase(CreatedModifiedModel): data[fd] = self.char_prompts[fd] return data + @property + def job_type(self): + return self.char_prompts.get('job_type', None) + + @property + def job_tags(self): + return self.char_prompts.get('job_tags', None) + + @property + def skip_tags(self): + return self.char_prompts.get('skip_tags', None) + + @property + def limit(self): + return self.char_prompts.get('limit', None) + def get_prompts_warnings(self): ujt_obj = self.unified_job_template if ujt_obj is None: From 8165141210c95a86a68e009a9f8f0432154b3f00 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 26 Oct 2016 09:28:20 -0400 Subject: [PATCH 07/10] expand documentation for WFJT node list --- .../workflow_job_template_workflow_nodes_list.md | 14 ++++++++++++++ awx/api/views.py | 9 +++++++++ 2 files changed, 23 insertions(+) create mode 100644 awx/api/templates/api/workflow_job_template_workflow_nodes_list.md diff --git a/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md b/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md new file mode 100644 index 0000000000..feb48373cb --- /dev/null +++ b/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md @@ -0,0 +1,14 @@ +# Workflow Job Template Workflow Node List + +Workflow nodes reference templates to execute and define the ordering +in which to execute them. After a job ran as part of a workflow finishes +the subsequent actions will be to. + + - run nodes contained in "failure_nodes" or "always_nodes" if job failed + - run nodes contained in "success_nodes" or "always_nodes" if job succeeded + +The workflow will be marked as failed if any jobs ran as a part of the workflow +fail and have the field `fail_on_job_failure` set to true. If not, the +workflow job will be marked as successful. + +{% include "api/sub_list_create_api_view.md" %} \ No newline at end of file diff --git a/awx/api/views.py b/awx/api/views.py index 2327f11b6a..d22c030965 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2404,6 +2404,7 @@ class JobTemplateLabelList(DeleteLastUnattachLabelMixin, SubListCreateAttachDeta serializer_class = LabelSerializer parent_model = JobTemplate relationship = 'labels' + new_in_300 = True def post(self, request, *args, **kwargs): # If a label already exists in the database, attach it instead of erroring out @@ -2697,6 +2698,7 @@ class WorkflowJobTemplateList(ListCreateAPIView): model = WorkflowJobTemplate serializer_class = WorkflowJobTemplateListSerializer always_allow_superuser = False + new_in_310 = True # TODO: RBAC ''' @@ -2714,10 +2716,12 @@ class WorkflowJobTemplateDetail(RetrieveUpdateDestroyAPIView): model = WorkflowJobTemplate serializer_class = WorkflowJobTemplateSerializer always_allow_superuser = False + new_in_310 = True class WorkflowJobTemplateLabelList(JobTemplateLabelList): parent_model = WorkflowJobTemplate + new_in_310 = True # TODO: @@ -2725,6 +2729,7 @@ class WorkflowJobTemplateLaunch(GenericAPIView): model = WorkflowJobTemplate serializer_class = EmptySerializer + new_in_310 = True def get(self, request, *args, **kwargs): data = {} @@ -2750,6 +2755,7 @@ class WorkflowJobTemplateWorkflowNodesList(SubListCreateAPIView): parent_model = WorkflowJobTemplate relationship = 'workflow_job_template_nodes' parent_key = 'workflow_job_template' + new_in_310 = True def update_raw_data(self, data): for fd in ['job_type', 'job_tags', 'skip_tags', 'limit', 'skip_tags']: @@ -2770,12 +2776,14 @@ class WorkflowJobList(ListCreateAPIView): model = WorkflowJob serializer_class = WorkflowJobListSerializer + new_in_310 = True # TODO: class WorkflowJobDetail(RetrieveDestroyAPIView): model = WorkflowJob serializer_class = WorkflowJobSerializer + new_in_310 = True class WorkflowJobWorkflowNodesList(SubListAPIView): @@ -2785,6 +2793,7 @@ class WorkflowJobWorkflowNodesList(SubListAPIView): parent_model = WorkflowJob relationship = 'workflow_job_nodes' parent_key = 'workflow_job' + new_in_310 = True class SystemJobTemplateList(ListAPIView): From 2794d62b4a274ceccc1f16b31ef1d8a1ec72b975 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 26 Oct 2016 09:35:00 -0400 Subject: [PATCH 08/10] fix null problem with char_prompts --- awx/api/serializers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 70e37b8220..26730745de 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2242,10 +2242,10 @@ class WorkflowJobListSerializer(WorkflowJobSerializer, UnifiedJobListSerializer) pass class WorkflowNodeBaseSerializer(BaseSerializer): - job_type = serializers.CharField(allow_blank=True, required=False, default=None) - job_tags = serializers.CharField(allow_blank=True, required=False, default=None) - limit = serializers.CharField(allow_blank=True, required=False, default=None) - skip_tags = serializers.CharField(allow_blank=True, required=False, default=None) + job_type = serializers.CharField(allow_blank=True, allow_null=True, required=False, default=None) + job_tags = serializers.CharField(allow_blank=True, allow_null=True, required=False, default=None) + limit = serializers.CharField(allow_blank=True, allow_null=True, required=False, default=None) + skip_tags = serializers.CharField(allow_blank=True, allow_null=True, required=False, default=None) success_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) failure_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) always_nodes = serializers.PrimaryKeyRelatedField(many=True, read_only=True) From 6b4737b5f0ded82f318638ab7ad484452e8e19d4 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 26 Oct 2016 09:53:54 -0400 Subject: [PATCH 09/10] more WFJT node docs feedback --- .../api/workflow_job_template_workflow_nodes_list.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md b/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md index feb48373cb..e6b078a23f 100644 --- a/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md +++ b/awx/api/templates/api/workflow_job_template_workflow_nodes_list.md @@ -1,14 +1,14 @@ # Workflow Job Template Workflow Node List Workflow nodes reference templates to execute and define the ordering -in which to execute them. After a job ran as part of a workflow finishes -the subsequent actions will be to. +in which to execute them. After a job in this workflow finishes, +the subsequent actions are to: - run nodes contained in "failure_nodes" or "always_nodes" if job failed - run nodes contained in "success_nodes" or "always_nodes" if job succeeded -The workflow will be marked as failed if any jobs ran as a part of the workflow -fail and have the field `fail_on_job_failure` set to true. If not, the -workflow job will be marked as successful. +The workflow is marked as failed if any jobs run as part of that workflow fail +and have the field `fail_on_job_failure` set to true. If not, the workflow +job is marked as successful. {% include "api/sub_list_create_api_view.md" %} \ No newline at end of file From 0e99d150bbc8d95e3856c2f5cad75c5209114b45 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 26 Oct 2016 09:57:57 -0400 Subject: [PATCH 10/10] correct error in workflow unit tests --- awx/api/serializers.py | 1 - awx/main/tests/unit/models/test_workflow_unit.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 26730745de..d681fac426 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2267,7 +2267,6 @@ class WorkflowNodeBaseSerializer(BaseSerializer): def validate(self, attrs): # char_prompts go through different validation, so remove them here - print ' attrs: ' + str(attrs) for fd in ['job_type', 'job_tags', 'skip_tags', 'limit']: if fd in attrs: attrs.pop(fd) diff --git a/awx/main/tests/unit/models/test_workflow_unit.py b/awx/main/tests/unit/models/test_workflow_unit.py index bcbec85c81..bc7e9b5bce 100644 --- a/awx/main/tests/unit/models/test_workflow_unit.py +++ b/awx/main/tests/unit/models/test_workflow_unit.py @@ -217,7 +217,7 @@ class TestWorkflowWarnings: def test_warn_scan_errors_node_prompts(self, job_node_with_prompts): job_node_with_prompts.unified_job_template.job_type = 'scan' - job_node_with_prompts.job_type = 'run' + job_node_with_prompts.char_prompts['job_type'] = 'run' job_node_with_prompts.inventory = Inventory(name='different-inventory', pk=23) assert 'ignored' in job_node_with_prompts.get_prompts_warnings() assert 'job_type' in job_node_with_prompts.get_prompts_warnings()['ignored']