From 32122e68222f6e988af621d6338c575f8f8c1837 Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Tue, 10 Dec 2024 15:26:17 -0500 Subject: [PATCH 1/2] Fix misused project cache identifier (#15690) Fix project cache identifiers for new updates Finish test and discover viable solution Add comment on related task code --- awx/main/models/projects.py | 26 ++++++++- awx/main/tasks/jobs.py | 1 + awx/main/tests/live/tests/conftest.py | 16 +++++- .../live/tests/projects/test_requirements.py | 56 +++++++++++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 awx/main/tests/live/tests/projects/test_requirements.py diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index 8c8fcd52baf3..d0b22830ee0d 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -5,6 +5,8 @@ import datetime import os import urllib.parse as urlparse +from uuid import uuid4 +import logging # Django from django.conf import settings @@ -39,6 +41,8 @@ ROLE_SINGLETON_SYSTEM_AUDITOR, ) +logger = logging.getLogger('awx.main.models.projects') + __all__ = ['Project', 'ProjectUpdate'] @@ -447,7 +451,25 @@ def needs_update_on_launch(self): @property def cache_id(self): - return str(self.last_job_id) + """This gives the folder name where collections and roles will be saved to so it does not re-download + + 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 self.current_job_id: + return str(self.current_job_id) + elif self.last_job_id: + return str(self.last_job_id) + elif self.last_job_run: + return self.last_job_run.isoformat() + else: + logger.warning(f'No info about last update for project {self.id}, content cache may misbehave') + if self.modified: + return self.modified.isoformat() + else: + return str(uuid4()) @property def notification_templates(self): @@ -618,7 +640,7 @@ def branch_override(self): @property def cache_id(self): if self.branch_override or self.job_type == 'check' or (not self.project): - return str(self.id) + return str(self.id) # causes it to not use the cache, basically return self.project.cache_id def result_stdout_raw_limited(self, start_line=0, end_line=None, redact_sensitive=True): diff --git a/awx/main/tasks/jobs.py b/awx/main/tasks/jobs.py index 80b639e2a74a..36493c4e217d 100644 --- a/awx/main/tasks/jobs.py +++ b/awx/main/tasks/jobs.py @@ -700,6 +700,7 @@ def get_sync_needs(self, project, scm_branch=None): logger.debug(f'Project not available locally, {self.instance.id} will sync with remote') sync_needs.append(source_update_tag) + # 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): diff --git a/awx/main/tests/live/tests/conftest.py b/awx/main/tests/live/tests/conftest.py index 848e8fd7d21d..0fae03918971 100644 --- a/awx/main/tests/live/tests/conftest.py +++ b/awx/main/tests/live/tests/conftest.py @@ -1,14 +1,18 @@ import time +import pytest + # These tests are invoked from the awx/main/tests/live/ subfolder # so any fixtures from higher-up conftest files must be explicitly included from awx.main.tests.functional.conftest import * # noqa +from awx.main.models import Organization + -def wait_to_leave_status(job, status, timeout=25, sleep_time=0.1): +def wait_to_leave_status(job, status, timeout=30, sleep_time=0.1): """Wait until the job does NOT have the specified status with some timeout - the default timeout of 25 if chosen because the task manager runs on a 20 second + the default timeout is based on the task manager running a 20 second schedule, and the API does not guarentee working jobs faster than this """ start = time.time() @@ -26,3 +30,11 @@ def wait_for_job(job, final_status='successful', running_timeout=800): wait_to_leave_status(job, 'running', timeout=running_timeout) assert job.status == final_status, f'Job was not successful id={job.id} status={job.status}' + + +@pytest.fixture(scope='session') +def default_org(): + org = Organization.objects.filter(name='Default').first() + if org is None: + raise Exception('Tests expect Default org to already be created and it is not') + return org diff --git a/awx/main/tests/live/tests/projects/test_requirements.py b/awx/main/tests/live/tests/projects/test_requirements.py new file mode 100644 index 000000000000..c82ccbec80fa --- /dev/null +++ b/awx/main/tests/live/tests/projects/test_requirements.py @@ -0,0 +1,56 @@ +import os +import time + +import pytest + +from django.conf import settings + +from awx.main.tests.live.tests.conftest import wait_for_job + +from awx.main.models import Project, SystemJobTemplate + + +@pytest.fixture(scope='session') +def project_with_requirements(default_org): + project, _ = Project.objects.get_or_create( + name='project-with-requirements', + scm_url='https://github.com/ansible/test-playbooks.git', + scm_branch="with_requirements", + scm_type='git', + organization=default_org, + ) + start = time.time() + while time.time() - start < 3.0: + if project.current_job or project.last_job or project.last_job_run: + break + assert project.current_job or project.last_job or project.last_job_run, f'Project never updated id={project.id}' + update = project.current_job or project.last_job + if update: + wait_for_job(update) + return project + + +def project_cache_is_populated(project): + proj_cache = os.path.join(project.get_cache_path(), project.cache_id) + return os.path.exists(proj_cache) + + +def test_cache_is_populated_after_cleanup_job(project_with_requirements): + assert project_with_requirements.cache_id is not None # already updated, should be something + cache_path = os.path.join(settings.PROJECTS_ROOT, '.__awx_cache') + assert os.path.exists(cache_path) + + assert project_cache_is_populated(project_with_requirements) + + cleanup_sjt = SystemJobTemplate.objects.get(name='Cleanup Job Details') + cleanup_job = cleanup_sjt.create_unified_job(extra_vars={'days': 0}) + cleanup_job.signal_start() + wait_for_job(cleanup_job) + + project_with_requirements.refresh_from_db() + assert project_with_requirements.cache_id is not None + update = project_with_requirements.update() + wait_for_job(update) + + # Now, we still have a populated cache + assert project_cache_is_populated(project_with_requirements) From efbe729c42322a215007dc0dda1ec39449ab8f8a Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Tue, 10 Dec 2024 18:20:14 -0500 Subject: [PATCH 2/2] bump sqlparse to meet DAB requirement (#15697) Signed-off-by: Seth Foster --- requirements/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 6fda858d65c0..eb255c1e9d6c 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -470,7 +470,7 @@ slack-sdk==3.27.0 # via -r /awx_devel/requirements/requirements.in smmap==5.0.1 # via gitdb -sqlparse==0.5.1 +sqlparse==0.5.3 # via # -r /awx_devel/requirements/requirements.in # django