Merge pull request #10359 from AlanCoding/stop_setting_venv

Remove uses of ansible_virtualenv_path

Run a job with recent AWX, then inspect:
In [2]: Job.objects.order_by('-created').first().custom_virtualenv
Out[2]: '/var/lib/awx/venv/ansible'

We shouldn't do this. The job was ran after the custom_virtualenv field has become entirely informative / non-functional. It is very arguable whether jobs should keep this field. For pre-migration jobs, it is an accurate record of fact, so I'm not going out of my way to argue for removal.
With this change, new jobs will record a blank value for custom_virtualenv, which is more correct, because we don't do this.
I have carefully looked over @rebeccahhh's #10090, and I don't see any way that ansible_virtualenv_path is used. Model use is reported for templates, so that doesn't block removing the field on jobs either.

Reviewed-by: Alan Rominger <arominge@redhat.com>
Reviewed-by: Rebeccah Hunter <rhunter@redhat.com>
Reviewed-by: Bill Nottingham <None>
Reviewed-by: Shane McDonald <me@shanemcd.com>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-06-08 12:26:38 +00:00 committed by GitHub
commit 7c442f3f50
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 2 additions and 79 deletions

View File

@ -66,13 +66,9 @@ class AnsibleInventoryLoader(object):
/usr/bin/ansible/ansible-inventory -i hosts --list
"""
def __init__(self, source, venv_path=None, verbosity=0):
def __init__(self, source, verbosity=0):
self.source = source
self.verbosity = verbosity
if venv_path:
self.venv_path = venv_path
else:
self.venv_path = settings.ANSIBLE_VENV_PATH
def get_base_args(self):
bargs = ['podman', 'run', '--user=root', '--quiet']
@ -131,7 +127,6 @@ class Command(BaseCommand):
def add_arguments(self, parser):
parser.add_argument('--inventory-name', dest='inventory_name', type=str, default=None, metavar='n', help='name of inventory to sync')
parser.add_argument('--inventory-id', dest='inventory_id', type=int, default=None, metavar='i', help='id of inventory to sync')
parser.add_argument('--venv', dest='venv', type=str, default=None, help='absolute path to the AWX custom virtualenv to use')
parser.add_argument('--overwrite', dest='overwrite', action='store_true', default=False, help='overwrite the destination hosts and groups')
parser.add_argument('--overwrite-vars', dest='overwrite_vars', action='store_true', default=False, help='overwrite (rather than merge) variables')
parser.add_argument('--keep-vars', dest='keep_vars', action='store_true', default=False, help='DEPRECATED legacy option, has no effect')
@ -824,7 +819,6 @@ class Command(BaseCommand):
raise CommandError('--source is required')
verbosity = int(options.get('verbosity', 1))
self.set_logging_level(verbosity)
venv_path = options.get('venv', None)
# Load inventory object based on name or ID.
if inventory_id:
@ -854,7 +848,7 @@ class Command(BaseCommand):
_eager_fields=dict(job_args=json.dumps(sys.argv), job_env=dict(os.environ.items()), job_cwd=os.getcwd())
)
data = AnsibleInventoryLoader(source=source, venv_path=venv_path, verbosity=verbosity).load()
data = AnsibleInventoryLoader(source=source, verbosity=verbosity).load()
logger.debug('Finished loading from source: %s', source)

View File

@ -1308,16 +1308,6 @@ class InventoryUpdate(UnifiedJob, InventorySourceOptions, JobNotificationMixin,
return self.global_instance_groups
return selected_groups
@property
def ansible_virtualenv_path(self):
if self.inventory_source and self.inventory_source.custom_virtualenv:
return self.inventory_source.custom_virtualenv
if self.inventory_source and self.inventory_source.source_project:
project = self.inventory_source.source_project
if project and project.custom_virtualenv:
return project.custom_virtualenv
return settings.ANSIBLE_VENV_PATH
def cancel(self, job_explanation=None, is_chain=False):
res = super(InventoryUpdate, self).cancel(job_explanation=job_explanation, is_chain=is_chain)
if res:

View File

@ -600,18 +600,6 @@ class Job(UnifiedJob, JobOptions, SurveyJobMixin, JobNotificationMixin, TaskMana
def get_ui_url(self):
return urljoin(settings.TOWER_URL_BASE, "/#/jobs/playbook/{}".format(self.pk))
@property
def ansible_virtualenv_path(self):
# the order here enforces precedence (it matters)
for virtualenv in (
self.job_template.custom_virtualenv if self.job_template else None,
self.project.custom_virtualenv,
self.organization.custom_virtualenv if self.organization else None,
):
if virtualenv:
return virtualenv
return settings.ANSIBLE_VENV_PATH
@property
def event_class(self):
if self.has_unpartitioned_events:

View File

@ -1262,10 +1262,6 @@ class BaseTask(object):
if not os.path.exists(settings.AWX_ISOLATION_BASE_PATH):
raise RuntimeError('AWX_ISOLATION_BASE_PATH=%s does not exist' % settings.AWX_ISOLATION_BASE_PATH)
# store a record of the venv used at runtime
if hasattr(self.instance, 'custom_virtualenv'):
self.update_model(pk, custom_virtualenv=getattr(self.instance, 'ansible_virtualenv_path', settings.ANSIBLE_VENV_PATH))
# Fetch "cached" fact data from prior runs and put on the disk
# where ansible expects to find it
if getattr(self.instance, 'use_fact_cache', False):

View File

@ -3,14 +3,6 @@ import pytest
from awx.main.models import JobTemplate, Job, JobHostSummary, WorkflowJob, Inventory, Project, Organization
@pytest.mark.django_db
def test_awx_virtualenv_from_settings(inventory, project, machine_credential):
jt = JobTemplate.objects.create(name='my-jt', inventory=inventory, project=project, playbook='helloworld.yml')
jt.credentials.add(machine_credential)
job = jt.create_unified_job()
assert job.ansible_virtualenv_path == '/var/lib/awx/venv/ansible'
@pytest.mark.django_db
def test_prevent_slicing():
jt = JobTemplate.objects.create(name='foo', job_slice_count=4)
@ -20,36 +12,6 @@ def test_prevent_slicing():
assert isinstance(job, Job)
@pytest.mark.django_db
def test_awx_custom_virtualenv(inventory, project, machine_credential, organization):
jt = JobTemplate.objects.create(name='my-jt', inventory=inventory, project=project, playbook='helloworld.yml', organization=organization)
jt.credentials.add(machine_credential)
job = jt.create_unified_job()
job.organization.custom_virtualenv = '/var/lib/awx/venv/fancy-org'
job.organization.save()
assert job.ansible_virtualenv_path == '/var/lib/awx/venv/fancy-org'
job.project.custom_virtualenv = '/var/lib/awx/venv/fancy-proj'
job.project.save()
assert job.ansible_virtualenv_path == '/var/lib/awx/venv/fancy-proj'
job.job_template.custom_virtualenv = '/var/lib/awx/venv/fancy-jt'
job.job_template.save()
assert job.ansible_virtualenv_path == '/var/lib/awx/venv/fancy-jt'
@pytest.mark.django_db
def test_awx_custom_virtualenv_without_jt(project):
project.custom_virtualenv = '/var/lib/awx/venv/fancy-proj'
project.save()
job = Job(project=project)
job.save()
job = Job.objects.get(pk=job.id)
assert job.ansible_virtualenv_path == '/var/lib/awx/venv/fancy-proj'
@pytest.mark.django_db
def test_job_host_summary_representation(host):
job = Job.objects.create(name='foo')

View File

@ -75,7 +75,6 @@ def job(mocker):
'launch_type': 'manual',
'verbosity': 1,
'awx_meta_vars.return_value': {},
'ansible_virtualenv_path': '',
'inventory.get_script_data.return_value': {},
}
)

View File

@ -187,7 +187,6 @@ def test_openstack_client_config_generation(mocker, source, expected, private_da
'source_vars_dict': {},
'get_cloud_credential': mocker.Mock(return_value=credential),
'get_extra_credentials': lambda x: [],
'ansible_virtualenv_path': '/var/lib/awx/venv/foo',
}
)
cloud_config = update.build_private_data(inventory_update, private_data_dir)
@ -229,7 +228,6 @@ def test_openstack_client_config_generation_with_project_domain_name(mocker, sou
'source_vars_dict': {},
'get_cloud_credential': mocker.Mock(return_value=credential),
'get_extra_credentials': lambda x: [],
'ansible_virtualenv_path': '/var/lib/awx/venv/foo',
}
)
cloud_config = update.build_private_data(inventory_update, private_data_dir)
@ -273,7 +271,6 @@ def test_openstack_client_config_generation_with_region(mocker, source, expected
'source_vars_dict': {},
'get_cloud_credential': mocker.Mock(return_value=credential),
'get_extra_credentials': lambda x: [],
'ansible_virtualenv_path': '/venv/foo',
}
)
cloud_config = update.build_private_data(inventory_update, private_data_dir)
@ -315,7 +312,6 @@ def test_openstack_client_config_generation_with_private_source_vars(mocker, sou
'source_vars_dict': {'private': source},
'get_cloud_credential': mocker.Mock(return_value=credential),
'get_extra_credentials': lambda x: [],
'ansible_virtualenv_path': '/var/lib/awx/venv/foo',
}
)
cloud_config = update.build_private_data(inventory_update, private_data_dir)

View File

@ -95,7 +95,6 @@ include(optional('/etc/tower/settings.py'), scope=locals())
include(optional('/etc/tower/conf.d/*.py'), scope=locals())
BASE_VENV_PATH = "/var/lib/awx/venv/"
ANSIBLE_VENV_PATH = os.path.join(BASE_VENV_PATH, "ansible")
AWX_VENV_PATH = os.path.join(BASE_VENV_PATH, "awx")
# If any local_*.py files are present in awx/settings/, use them to override

View File

@ -35,7 +35,6 @@ SCHEDULE_METADATA_LOCATION = '/var/lib/awx/.tower_cycle'
# Ansible base virtualenv paths and enablement
BASE_VENV_PATH = os.path.realpath("/var/lib/awx/venv")
ANSIBLE_VENV_PATH = os.path.join(BASE_VENV_PATH, "ansible")
# Base virtualenv paths and enablement
AWX_VENV_PATH = os.path.join(BASE_VENV_PATH, "awx")