From cebaf9a7d440e10cf5cc1c976032a2fd8267b2bb Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Thu, 30 Mar 2023 14:30:26 -0400 Subject: [PATCH] Add option for package manager - Fixes bug where we incorrecty assume dnf for python interpreter install - Add pulp tests that verify check_ansible and new pkg manager option --- .github/test-scripts/setup_pulp.sh | 2 +- ansible_builder/_target_scripts/assemble | 16 ++++--- ansible_builder/containerfile.py | 8 +++- ansible_builder/ee_schema.py | 11 ++++- ansible_builder/user_definition.py | 3 +- docs/definition.rst | 7 +++ .../v3/check_ansible/ee-missing-ansible.yml | 14 ++++++ .../v3/check_ansible/ee-missing-runner.yml | 14 ++++++ test/data/v3/check_ansible/ee-skip.yml | 14 ++++++ test/pulp_integration/test_v3.py | 48 +++++++++++++++++++ tox.ini | 4 +- 11 files changed, 128 insertions(+), 13 deletions(-) create mode 100644 test/data/v3/check_ansible/ee-missing-ansible.yml create mode 100644 test/data/v3/check_ansible/ee-missing-runner.yml create mode 100644 test/data/v3/check_ansible/ee-skip.yml create mode 100644 test/pulp_integration/test_v3.py diff --git a/.github/test-scripts/setup_pulp.sh b/.github/test-scripts/setup_pulp.sh index 17c617f7..6a8839ab 100755 --- a/.github/test-scripts/setup_pulp.sh +++ b/.github/test-scripts/setup_pulp.sh @@ -111,7 +111,7 @@ export XDG_RUNTIME_DIR=/tmp/pulptests mkdir $XDG_RUNTIME_DIR skopeo login --username admin --password password localhost:8080 --tls-verify=false -skopeo copy docker://registry.access.redhat.com/ubi9/ubi-micro:latest docker://localhost:8080/testrepo/ubi-micro --dest-tls-verify=false +skopeo copy docker://registry.access.redhat.com/ubi9/ubi-minimal:latest docker://localhost:8080/testrepo/ubi-minimal --dest-tls-verify=false podman login --username "$1" --password "$2" registry.redhat.io skopeo copy docker://registry.redhat.io/ansible-automation-platform-21/ansible-builder-rhel8:latest docker://localhost:8080/testrepo/ansible-builder-rhel8 --dest-tls-verify=false diff --git a/ansible_builder/_target_scripts/assemble b/ansible_builder/_target_scripts/assemble index a4a30ae6..955dcd88 100755 --- a/ansible_builder/_target_scripts/assemble +++ b/ansible_builder/_target_scripts/assemble @@ -37,12 +37,16 @@ if [ -z $PKGMGR ]; then PKGMGR=/usr/bin/dnf if [ -f "/usr/bin/microdnf" ]; then PKGMGR=/usr/bin/microdnf - if [ -z $PKGMGR_OPTS ]; then - # NOTE(pabelanger): skip install docs and weak dependencies to - # make smaller images. Sadly, setting these in dnf.conf don't - # appear to work. - PKGMGR_OPTS="--nodocs --setopt install_weak_deps=0" - fi + fi +fi + +if [ "$PKGMGR" = "/usr/bin/microdnf" ] +then + if [ -z $PKGMGR_OPTS ]; then + # NOTE(pabelanger): skip install docs and weak dependencies to + # make smaller images. Sadly, setting these in dnf.conf don't + # appear to work. + PKGMGR_OPTS="--nodocs --setopt install_weak_deps=0" fi fi diff --git a/ansible_builder/containerfile.py b/ansible_builder/containerfile.py index fb0b9e02..0ba6da56 100644 --- a/ansible_builder/containerfile.py +++ b/ansible_builder/containerfile.py @@ -72,8 +72,7 @@ def prepare(self): if not self.definition.builder_image: if self.definition.python_package_name: - # FIXME: better dnf cleanup needed? - self.steps.append('RUN dnf install $PYPKG -y && dnf clean all') + self.steps.append('RUN $PKGMGR install $PYPKG -y && $PKGMGR clean all') # We should always make sure pip is available for later stages. self.steps.append('RUN $PYCMD -m ensurepip') @@ -188,6 +187,9 @@ def _insert_global_args(self, include_values: bool = False): 'ANSIBLE_INSTALL_REFS': self.definition.ansible_ref_install_list, } + if self.definition.version >= 3: + global_args['PKGMGR'] = self.definition.options['package_manager_path'] + for arg, value in global_args.items(): if include_values and value: # quote the value in case it includes spaces @@ -362,6 +364,8 @@ def _prepare_introspect_assemble_steps(self): introspect_cmd += " --write-bindep=/tmp/src/bindep.txt --write-pip=/tmp/src/requirements.txt" self.steps.append(introspect_cmd) + if self.definition.version >= 3: + self.steps.append("ENV PKGMGR=$PKGMGR") self.steps.append("RUN /output/scripts/assemble") def _prepare_system_runtime_deps_steps(self): diff --git a/ansible_builder/ee_schema.py b/ansible_builder/ee_schema.py index d27373d1..2393e6c9 100644 --- a/ansible_builder/ee_schema.py +++ b/ansible_builder/ee_schema.py @@ -300,6 +300,10 @@ "description": "Disables the check for Ansible/Runner in final image", "type": "boolean", }, + "package_manager_path": { + "description": "Path to the system package manager to use", + "type": "string", + } }, }, }, @@ -328,7 +332,9 @@ def validate_schema(ee_def: dict): raise DefinitionError(msg=e.message, path=e.absolute_schema_path) _handle_aliasing(ee_def) - _handle_options_defaults(ee_def) + + if schema_version >= 3: + _handle_options_defaults(ee_def) def _handle_aliasing(ee_def: dict): @@ -361,3 +367,6 @@ def _handle_options_defaults(ee_def: dict): if ee_def['options'].get('skip_ansible_check') is None: ee_def['options']['skip_ansible_check'] = False + + if ee_def['options'].get('package_manager_path') is None: + ee_def['options']['package_manager_path'] = '/usr/bin/dnf' diff --git a/ansible_builder/user_definition.py b/ansible_builder/user_definition.py index 740703ae..f6e8552d 100644 --- a/ansible_builder/user_definition.py +++ b/ansible_builder/user_definition.py @@ -153,8 +153,7 @@ def additional_build_files(self): @property def options(self): - # Since some options have default values, the 'options' key is always available - return self.raw['options'] + return self.raw.get('options', {}) def get_dep_abs_path(self, entry): """Unique to the user EE definition, files can be referenced by either diff --git a/docs/definition.rst b/docs/definition.rst index 036da6d7..6bde99a6 100644 --- a/docs/definition.rst +++ b/docs/definition.rst @@ -267,6 +267,12 @@ options This section is a dictionary that contains keywords/options that can affect builder runtime functionality. Valid keys for this section are: + ``package_manager_path`` + A string with the path to the package manager (dnf or microdnf) to use. + The default is ``/usr/bin/dnf``. This value will be used to install a + python interpreter, if specified in ``dependencies``, and during the + build phase by the ``assemble`` script. + ``skip_ansible_check`` This boolean value controls whether or not the check for an installation of Ansible and Ansible Runner is performed on the final image. Set this @@ -277,6 +283,7 @@ Example ``options`` section: .. code:: yaml options: + package_manager_path: /usr/bin/microdnf skip_ansible_check: True version diff --git a/test/data/v3/check_ansible/ee-missing-ansible.yml b/test/data/v3/check_ansible/ee-missing-ansible.yml new file mode 100644 index 00000000..de672e87 --- /dev/null +++ b/test/data/v3/check_ansible/ee-missing-ansible.yml @@ -0,0 +1,14 @@ +--- +version: 3 + +images: + base_image: + name: localhost:8080/testrepo/ubi-minimal:latest + +dependencies: + ansible_runner: ansible_runner + python_interpreter: + package_name: python3 + +options: + package_manager_path: /usr/bin/microdnf diff --git a/test/data/v3/check_ansible/ee-missing-runner.yml b/test/data/v3/check_ansible/ee-missing-runner.yml new file mode 100644 index 00000000..95e9486c --- /dev/null +++ b/test/data/v3/check_ansible/ee-missing-runner.yml @@ -0,0 +1,14 @@ +--- +version: 3 + +images: + base_image: + name: localhost:8080/testrepo/ubi-minimal:latest + +dependencies: + ansible_core: ansible_core + python_interpreter: + package_name: python3 + +options: + package_manager_path: /usr/bin/microdnf diff --git a/test/data/v3/check_ansible/ee-skip.yml b/test/data/v3/check_ansible/ee-skip.yml new file mode 100644 index 00000000..7f11e426 --- /dev/null +++ b/test/data/v3/check_ansible/ee-skip.yml @@ -0,0 +1,14 @@ +--- +version: 3 + +images: + base_image: + name: localhost:8080/testrepo/ubi-minimal:latest + +dependencies: + python_interpreter: + package_name: python3 + +options: + skip_ansible_check: True + package_manager_path: /usr/bin/microdnf diff --git a/test/pulp_integration/test_v3.py b/test/pulp_integration/test_v3.py new file mode 100644 index 00000000..f5ca14c4 --- /dev/null +++ b/test/pulp_integration/test_v3.py @@ -0,0 +1,48 @@ +import pytest +import subprocess + + +class TestV3: + + def test_ansible_check_is_skipped(self, cli, tmp_path, data_dir, podman_ee_tag): + """ + Test that the check_ansible script is skipped will NOT cause build failure. + """ + ee_def = data_dir / 'v3' / 'check_ansible' / 'ee-skip.yml' + + result = cli( + f'ansible-builder build -c {tmp_path} -f {ee_def} -t {podman_ee_tag} ' + f'--container-runtime=podman -v3' + ) + + assert result.rc == 0 + + def test_missing_ansible(self, cli, tmp_path, data_dir, podman_ee_tag): + """ + Test that the check_ansible script will cause build failure if + ansible-core is not installed. + """ + ee_def = data_dir / 'v3' / 'check_ansible' / 'ee-missing-ansible.yml' + + with pytest.raises(subprocess.CalledProcessError) as einfo: + cli( + f'ansible-builder build -c {tmp_path} -f {ee_def} -t {podman_ee_tag} ' + f'--container-runtime=podman -v3' + ) + + assert "ERROR - Missing Ansible installation" in einfo.value.stdout + + def test_missing_runner(self, cli, tmp_path, data_dir, podman_ee_tag): + """ + Test that the check_ansible script will cause build failure if + ansible-runner is not installed. + """ + ee_def = data_dir / 'v3' / 'check_ansible' / 'ee-missing-runner.yml' + + with pytest.raises(subprocess.CalledProcessError) as einfo: + cli( + f'ansible-builder build -c {tmp_path} -f {ee_def} -t {podman_ee_tag} ' + f'--container-runtime=podman -v3' + ) + + assert "ERROR - Missing Ansible Runner installation" in einfo.value.stdout diff --git a/tox.ini b/tox.ini index 7a5d8b79..4ec09686 100644 --- a/tox.ini +++ b/tox.ini @@ -26,7 +26,9 @@ commands = pytest {posargs:test/unit} # Some of these tests must run serially because of a shared resource # (the system policy.json file). description = Run pulp integration tests -commands = pytest -n 1 -m "serial" {posargs:test/pulp_integration} +commands = + pytest -n 1 -m "serial" {posargs:test/pulp_integration} + pytest -m "not serial" {posargs:test/pulp_integration} [testenv:integration{,-py39,-py310}] description = Run integration tests