Skip to content

Commit

Permalink
Fix misused project cache identifier (ansible#15690)
Browse files Browse the repository at this point in the history
Fix project cache identifiers for new updates

Finish test and discover viable solution

Add comment on related task code
  • Loading branch information
AlanCoding authored Dec 10, 2024
1 parent a129bc8 commit 32122e6
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 4 deletions.
26 changes: 24 additions & 2 deletions awx/main/models/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -39,6 +41,8 @@
ROLE_SINGLETON_SYSTEM_AUDITOR,
)

logger = logging.getLogger('awx.main.models.projects')

__all__ = ['Project', 'ProjectUpdate']


Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions awx/main/tasks/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
16 changes: 14 additions & 2 deletions awx/main/tests/live/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -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()
Expand All @@ -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
56 changes: 56 additions & 0 deletions awx/main/tests/live/tests/projects/test_requirements.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 32122e6

Please sign in to comment.