Merge branch 'devel' into AAP-84138

awx/main/models/projects.py

@@ -450,13 +450,14 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin, CustomVirtualEn
 
     @property
     def cache_id(self):
-        """This gives the folder name where collections and roles will be saved to so it does not re-download
+        # Prefer scm_revision as the cache key if available. This guarantees that project changes are tracked correctly
+        # even over multiple nodes. The scm_revision is a hex string and thus safe to be used as a directory name.
+        if self.scm_revision:
+            return self.scm_revision
 
-        Normally we want this to track with the last update, because every update should pull new content.
-        This does not count sync jobs, but sync jobs do not update last_job or current_job anyway.
-        If cleanup_jobs deletes the last jobs, then we can fallback to using any given heuristic related
-        to the last job ran.
-        """
+        # If no scm_revision is available (e.g. non-scm projects), use these. current_job_id and last_job_id are global
+        # IDs in the database. This means that when a project sync runs on one node, all other nodes become outdated,
+        # resulting in unnecessary re-syncs.
         if self.current_job_id:
             return str(self.current_job_id)
         elif self.last_job_id:
@@ -638,7 +639,7 @@ class ProjectUpdate(UnifiedJob, ProjectOptions, JobNotificationMixin, TaskManage
 
     @property
     def cache_id(self):
-        if self.branch_override or self.job_type == 'check' or (not self.project):
+        if self.branch_override or (not self.project):
             return str(self.id)  # causes it to not use the cache, basically
         return self.project.cache_id
 

awx/main/tasks/jobs.py

@@ -885,7 +885,9 @@ class SourceControlMixin(BaseTask):
         # Determine whether or not this project sync needs to populate the cache for Ansible content, roles and collections
         has_cache = os.path.exists(os.path.join(project.get_cache_path(), project.cache_id))
         # Galaxy requirements are not supported for manual projects
-        if project.scm_type and ((not has_cache) or branch_override):
+        # If a source update is scheduled, always include roles/collections because
+        # the new revision may have different requirements.
+        if project.scm_type and ((not has_cache) or branch_override or source_update_tag in sync_needs):
             sync_needs.extend(['install_roles', 'install_collections'])
 
         return sync_needs

awx/main/tests/unit/models/test_project.py

@@ -12,3 +12,35 @@ def test_clean_credential_insights():
         proj.clean_credential()
 
     assert json.dumps(str(e.value)) == json.dumps(str([u'Insights Credential is required for an Insights Project.']))
+
+
+class TestProjectCacheId:
+    def test_cache_id_prefers_scm_revision(self):
+        proj = Project(scm_revision='a6e2e68042e58cccfda35614ef84537d39690fcd')
+        proj.current_job_id = 100
+        proj.last_job_id = 99
+        assert proj.cache_id == 'a6e2e68042e58cccfda35614ef84537d39690fcd'
+
+    def test_cache_id_falls_back_to_current_job_id(self):
+        proj = Project(scm_revision='')
+        proj.current_job_id = 100
+        proj.last_job_id = 99
+        assert proj.cache_id == '100'
+
+    def test_cache_id_falls_back_to_last_job_id(self):
+        proj = Project(scm_revision='')
+        proj.current_job_id = None
+        proj.last_job_id = 99
+        assert proj.cache_id == '99'
+
+    def test_cache_id_stable_across_different_last_job_ids(self):
+        """Simulates the multi-controller scenario: scm_revision stays the same
+        even when last_job_id changes due to a sync on another node."""
+        proj = Project(scm_revision='a6e2e68042e58cccfda35614ef84537d39690fcd')
+        proj.last_job_id = 100
+        cache_id_before = proj.cache_id
+
+        proj.last_job_id = 200
+        cache_id_after = proj.cache_id
+
+        assert cache_id_before == cache_id_after

awx/main/tests/unit/tasks/test_jobs.py

@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+import os
 import pytest
 from unittest import mock
 
@@ -661,3 +662,82 @@ class TestRunInventoryUpdatePopulateWorkloadIdentityTokens:
 
         # The instance's get_cloud_credential should now return the same object with context
         assert task.instance.get_cloud_credential() is cloud_cred
+
+
+class TestGetSyncNeeds:
+    """Tests for SourceControlMixin.get_sync_needs cache behavior."""
+
+    def _make_task_and_project(self, tmp_path, scm_revision='abc123', local_head='abc123', cache_exists=True):
+        """Create a SourceControlMixin instance and a mock project for testing get_sync_needs."""
+        project_path = str(tmp_path / 'project')
+        os.makedirs(project_path, exist_ok=True)
+
+        cache_base = str(tmp_path / 'cache')
+        os.makedirs(cache_base, exist_ok=True)
+        if cache_exists and scm_revision:
+            os.makedirs(os.path.join(cache_base, scm_revision), exist_ok=True)
+
+        project = mock.MagicMock()
+        project.scm_type = 'git'
+        project.scm_revision = scm_revision
+        project.scm_branch = 'main'
+        project.get_project_path.return_value = project_path
+        project.get_cache_path.return_value = cache_base
+        project.cache_id = scm_revision if scm_revision else 'fallback'
+
+        git_repo = mock.MagicMock()
+        git_repo.head.commit.hexsha = local_head
+
+        task = jobs.SourceControlMixin()
+        task.instance = mock.MagicMock(spec=Job)
+
+        return task, project, git_repo
+
+    def test_source_update_forces_role_install(self, tmp_path):
+        """When a source update is needed (local HEAD differs from project revision),
+        install_roles and install_collections must be included even if cache exists.
+        A new revision may have changed requirements.yml."""
+        task, project, git_repo = self._make_task_and_project(
+            tmp_path,
+            scm_revision='abc123',
+            local_head='def456',
+            cache_exists=True,
+        )
+
+        with mock.patch('awx.main.tasks.jobs.git.Repo', return_value=git_repo):
+            sync_needs = task.get_sync_needs(project)
+
+        assert 'update_git' in sync_needs
+        assert 'install_roles' in sync_needs
+        assert 'install_collections' in sync_needs
+
+    def test_no_sync_when_revision_matches_and_cache_exists(self, tmp_path):
+        """When local HEAD matches project revision and cache exists, no sync needed."""
+        task, project, git_repo = self._make_task_and_project(
+            tmp_path,
+            scm_revision='abc123',
+            local_head='abc123',
+            cache_exists=True,
+        )
+
+        with mock.patch('awx.main.tasks.jobs.git.Repo', return_value=git_repo):
+            sync_needs = task.get_sync_needs(project)
+
+        assert sync_needs == []
+
+    def test_cache_miss_triggers_role_install(self, tmp_path):
+        """When cache doesn't exist, install_roles and install_collections are needed
+        even if the git revision matches."""
+        task, project, git_repo = self._make_task_and_project(
+            tmp_path,
+            scm_revision='abc123',
+            local_head='abc123',
+            cache_exists=False,
+        )
+
+        with mock.patch('awx.main.tasks.jobs.git.Repo', return_value=git_repo):
+            sync_needs = task.get_sync_needs(project)
+
+        assert 'update_git' not in sync_needs
+        assert 'install_roles' in sync_needs
+        assert 'install_collections' in sync_needs