From 4b3136794537c2756a94a948cdc8060ed3215ae3 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 4 Dec 2018 09:05:49 -0500 Subject: [PATCH] Do not update project details after sync jobs --- awx/main/models/projects.py | 15 ++++ awx/main/tests/functional/models/test_job.py | 16 ---- .../functional/models/test_unified_job.py | 73 +++++++++++++++++++ docs/clustering.md | 7 ++ 4 files changed, 95 insertions(+), 16 deletions(-) diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index f39b949f61..4974ad8c58 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -475,6 +475,21 @@ class ProjectUpdate(UnifiedJob, ProjectOptions, JobNotificationMixin, TaskManage def _get_parent_field_name(self): return 'project' + def _update_parent_instance(self): + if not self.project: + return # no parent instance to update + if self.job_type == PERM_INVENTORY_DEPLOY: + # Do not update project status if this is sync job + # unless no other updates have happened or started + first_update = False + if self.project.status == 'never updated' and self.status == 'running': + first_update = True + elif self.project.current_job == self: + first_update = True + if not first_update: + return + return super(ProjectUpdate, self)._update_parent_instance() + @classmethod def _get_task_class(cls): from awx.main.tasks import RunProjectUpdate diff --git a/awx/main/tests/functional/models/test_job.py b/awx/main/tests/functional/models/test_job.py index 2adea9cc19..39c7291c6d 100644 --- a/awx/main/tests/functional/models/test_job.py +++ b/awx/main/tests/functional/models/test_job.py @@ -2,7 +2,6 @@ import pytest import six from awx.main.models import JobTemplate, Job, JobHostSummary, WorkflowJob -from crum import impersonate @pytest.mark.django_db @@ -65,21 +64,6 @@ def test_awx_custom_virtualenv_without_jt(project): assert job.ansible_virtualenv_path == '/venv/fancy-proj' -@pytest.mark.django_db -def test_update_parent_instance(job_template, alice): - # jobs are launched as a particular user, user not saved as modified_by - with impersonate(alice): - assert job_template.current_job is None - assert job_template.status == 'never updated' - assert job_template.modified_by is None - job = job_template.jobs.create(status='new') - job.status = 'pending' - job.save() - assert job_template.current_job == job - assert job_template.status == 'pending' - assert job_template.modified_by is None - - @pytest.mark.django_db def test_job_host_summary_representation(host): job = Job.objects.create(name='foo') diff --git a/awx/main/tests/functional/models/test_unified_job.py b/awx/main/tests/functional/models/test_unified_job.py index 74e163c66e..18e80b63cc 100644 --- a/awx/main/tests/functional/models/test_unified_job.py +++ b/awx/main/tests/functional/models/test_unified_job.py @@ -2,6 +2,9 @@ import itertools import pytest import six +# CRUM +from crum import impersonate + # Django from django.contrib.contenttypes.models import ContentType @@ -154,6 +157,76 @@ def test_event_model_undefined(): assert wj.event_processing_finished +@pytest.mark.django_db +class TestUpdateParentInstance: + + def test_template_modified_by_not_changed_on_launch(self, job_template, alice): + # jobs are launched as a particular user, user not saved as JT modified_by + with impersonate(alice): + assert job_template.current_job is None + assert job_template.status == 'never updated' + assert job_template.modified_by is None + job = job_template.jobs.create(status='new') + job.status = 'pending' + job.save() + assert job_template.current_job == job + assert job_template.status == 'pending' + assert job_template.modified_by is None + + def check_update(self, project, status): + pu_check = project.project_updates.create( + job_type='check', status='new', launch_type='manual' + ) + pu_check.status = 'running' + pu_check.save() + # these should always be updated for a running check job + assert project.current_job == pu_check + assert project.status == 'running' + + pu_check.status = status + pu_check.save() + return pu_check + + def run_update(self, project, status): + pu_run = project.project_updates.create( + job_type='run', status='new', launch_type='sync' + ) + pu_run.status = 'running' + pu_run.save() + + pu_run.status = status + pu_run.save() + return pu_run + + def test_project_update_fails_project(self, project): + # This is the normal server auto-update on create behavior + assert project.status == 'never updated' + pu_check = self.check_update(project, status='failed') + + assert project.last_job == pu_check + assert project.status == 'failed' + + def test_project_sync_with_skip_update(self, project): + # syncs may be permitted to change project status + # only if prior status is "never updated" + assert project.status == 'never updated' + pu_run = self.run_update(project, status='successful') + + assert project.last_job == pu_run + assert project.status == 'successful' + + def test_project_sync_does_not_fail_project(self, project): + # Accurate normal server behavior, creating a project auto-updates + # have to create update, otherwise will fight with last_job logic + assert project.status == 'never updated' + pu_check = self.check_update(project, status='successful') + assert project.status == 'successful' + + self.run_update(project, status='failed') + assert project.last_job == pu_check + assert project.status == 'successful' + + @pytest.mark.django_db class TestTaskImpact: @pytest.fixture diff --git a/docs/clustering.md b/docs/clustering.md index a6693eb01b..d8b1217fdc 100644 --- a/docs/clustering.md +++ b/docs/clustering.md @@ -335,6 +335,13 @@ It's important to note that not all instances are required to be provisioned wit Project updates behave differently than they did before. Previously they were ordinary jobs that ran on a single instance. It's now important that they run successfully on any instance that could potentially run a job. Project's will now sync themselves to the correct version on the instance immediately prior to running the job. +When the sync happens, it is recorded in the database as a project update with a `launch_type` of "sync" +and a `job_type` of "run". Project syncs will not change the status or version of the project, +instead, they will update the source tree only on the instance where they run. +The only exception to this behavior is when the project is in the "never updated" state +(meaning that no project updates of any type have been ran), +in which case a sync should fill in the project's initial revision and status, and subsequent +syncs should not make such changes. If an Instance Group is configured but all instances in that group are offline or unavailable, any jobs that are launched targeting only that group will be stuck in a waiting state until instances become available. Fallback or backup resources should be provisioned to handle any work that might encounter this scenario.