Merge pull request #2869 from AlanCoding/syncs_do_not_update_project

Do not update project details after sync jobs

Reviewed-by: https://github.com/softwarefactory-project-zuul[bot]

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

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')

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

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.