Fix CI failures (#1)

* Fix linting * Fix unit tests * Remove py38 testing * Fix integration tests
ansible · Oct 13, 2022 · 8a767ce · 8a767ce
1 parent 6b26ccf
commit 8a767ce
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 39 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -113,9 +113,6 @@ jobs:
       fail-fast: false
       matrix:
         py_version:
-          - name: '3.8'
-            tox_env: integration-py38
-
           - name: '3.9'
             tox_env: integration-py39
 
@@ -167,9 +164,6 @@ jobs:
       fail-fast: false
       matrix:
         py_version:
-          - name: '3.8'
-            tox_env: unit-py38
-
           - name: '3.9'
             tox_env: unit-py39
 

diff --git a/ansible_builder/__main__.py b/ansible_builder/__main__.py
@@ -2,4 +2,3 @@
 
 if __name__ == '__main__':
     cli.run()
-
diff --git a/ansible_builder/_target_scripts/introspect.py b/ansible_builder/_target_scripts/introspect.py
@@ -295,6 +295,7 @@ def create_introspect_parser(parser):
 
     return introspect_parser
 
+
 EXCLUDE_REQUIREMENTS = frozenset((
     # obviously already satisfied or unwanted
     'ansible', 'ansible-base', 'python', 'ansible-core',
@@ -385,7 +386,6 @@ def write_file(filename: str, lines: list) -> bool:
     return True
 
 
-
 def main():
     args = parse_args()
 

diff --git a/ansible_builder/cli.py b/ansible_builder/cli.py
@@ -10,8 +10,8 @@
 from .exceptions import DefinitionError
 from .main import AnsibleBuilder
 from .policies import PolicyChoices
-from ._target_scripts.introspect import create_introspect_parser, run_introspect, process, simple_combine, base_collections_path
-from .utils import configure_logger, write_file
+from ._target_scripts.introspect import create_introspect_parser, run_introspect
+from .utils import configure_logger
 
 
 logger = logging.getLogger(__name__)
@@ -36,7 +36,7 @@ def run():
             sys.exit(1)
 
     elif args.action == 'introspect':
-        run_introspect()
+        run_introspect(args, logger)
 
     logger.error("An error has occured.")
     sys.exit(1)

diff --git a/ansible_builder/main.py b/ansible_builder/main.py
@@ -12,7 +12,6 @@
 from .utils import run_command, copy_file
 
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -276,8 +275,7 @@ def __init__(self, definition: UserDefinition,
         self.steps.append(f"ARG PYCMD={self.definition.python_path or '/usr/bin/python3'}")
 
         if ansible_refs := self.definition.ansible_ref_install_list:
-            self.steps.append(f"ARG ANSIBLE_INSTALL_REFS='{self.definition.ansible_ref_install_list}'")
-
+            self.steps.append(f"ARG ANSIBLE_INSTALL_REFS='{ansible_refs}'")
 
     def create_folder_copy_files(self):
         """Creates the build context file for this Containerfile
@@ -315,6 +313,9 @@ def create_folder_copy_files(self):
                 # FIXME: just use builtin copy?
                 copy_file(str(script_path), scripts_dir)
 
+        # later steps depend on base image containing these scripts
+        context_dir = pathlib.Path(self.build_outputs_dir).stem
+        self.steps.append(f'COPY {context_dir}/scripts/ /output/scripts/')
 
     def prepare_ansible_config_file(self):
         ansible_config_file_path = self.definition.ansible_config
@@ -354,15 +355,14 @@ def prepare_dynamic_base_and_builder(self):
 
         if not self.definition.builder_image:
             if python := self.definition.python_package_name:
-                self.steps.append(f'ARG PYPKG={self.definition.python_package_name}')
+                self.steps.append(f'ARG PYPKG={python}')
                 # FIXME: better dnf cleanup needed?
                 self.steps.append('RUN dnf install $PYPKG -y && dnf clean all')
 
-            if ansible_refs := self.definition.ansible_ref_install_list:
+            if self.definition.ansible_ref_install_list:
                 self.steps.append('ARG ANSIBLE_INSTALL_REFS')
                 self.steps.append('ARG PYCMD')
-                self.steps.append(f'RUN $PYCMD -m ensurepip && $PYCMD -m pip install --no-cache-dir $ANSIBLE_INSTALL_REFS')
-
+                self.steps.append('RUN $PYCMD -m ensurepip && $PYCMD -m pip install --no-cache-dir $ANSIBLE_INSTALL_REFS')
 
     def prepare_build_context(self):
         if any(self.definition.get_dep_abs_path(thing) for thing in ('galaxy', 'system', 'python')):
@@ -381,14 +381,7 @@ def prepare_introspect_assemble_steps(self):
         # The introspect/assemble block is valid if there are any form of requirements
         if any(self.definition.get_dep_abs_path(thing) for thing in ('galaxy', 'system', 'python')):
 
-            # copy in the various helper scripts
-            self.steps.append(f'ADD {constants.user_content_subfolder}/scripts/* .')
-
-            # FIXME: just fix it to run the scripts from the right place
-            self.steps.append('RUN mkdir -p /output && cp ./install-from-bindep /output')
-
-
-            introspect_cmd = "RUN $PYCMD introspect.py introspect --sanitize"
+            introspect_cmd = "RUN $PYCMD /output/scripts/introspect.py introspect --sanitize"
 
             requirements_file_exists = os.path.exists(os.path.join(
                 self.build_outputs_dir, constants.CONTEXT_FILES['python']
@@ -408,14 +401,14 @@ 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)
-            self.steps.append("RUN ./assemble")
+            self.steps.append("RUN /output/scripts/assemble")
 
         return self.steps
 
     def prepare_system_runtime_deps_steps(self):
         self.steps.extend([
             "COPY --from=builder /output/ /output/",
-            "RUN /output/install-from-bindep && rm -rf /output/wheels",
+            "RUN /output/scripts/install-from-bindep && rm -rf /output/wheels",
         ])
 
         return self.steps
@@ -443,7 +436,8 @@ def prepare_build_stage_steps(self):
                 "FROM $EE_BUILDER_IMAGE as builder"
                 "",
             ])
-        else:  # dynamic builder, create from customized base
+        else:
+            # dynamic builder, create from customized base
             self.steps.extend([
                 'FROM base as builder',
                 'ARG PYCMD',

diff --git a/ansible_builder/user_definition.py b/ansible_builder/user_definition.py
@@ -21,7 +21,8 @@
 
 
 # HACK: manage lifetimes more carefully
-_tempfiles: list[tempfile.TemporaryFile] = []
+_tempfiles: list[type[tempfile.TemporaryFile]] = []
+
 
 class ImageDescription:
     """
@@ -230,7 +231,6 @@ def _validate_v2(self):
             # Must set these values so that Containerfile uses the proper images
             self.build_arg_defaults['EE_BASE_IMAGE'] = self.base_image.name
 
-
     def _validate_v1(self):
         """
         Validate all execution environment file, version 1, keys.

diff --git a/demo/v3_demo/execution-environment.yml b/demo/v3_demo/execution-environment.yml
@@ -4,9 +4,9 @@ version: 3
 images:
   base_image:
     name: quay.io/centos/centos:stream8  # vanilla image!
-    #name: quay.io/centos/centos:stream9  # vanilla image!
-    #name: registry.fedoraproject.org/fedora:36  # vanilla image!
-    #name: registry.access.redhat.com/ubi9/ubi:latest  # vanilla image!
+    # name: quay.io/centos/centos:stream9  # vanilla image!
+    # name: registry.fedoraproject.org/fedora:36  # vanilla image!
+    # name: registry.access.redhat.com/ubi9/ubi:latest  # vanilla image!
 
 # no longer required, we do the needful inline, but should still work if someone sets it
 #  builder_image:
@@ -17,9 +17,9 @@ dependencies:
     package_name: python39
     python_path: /usr/bin/python3.9
 
-  ansible_core: https://github.com/ansible/ansible/archive/refs/tags/v2.13.2.tar.gz # install from a GH ref tarball
+  ansible_core: https://github.com/ansible/ansible/archive/refs/tags/v2.13.2.tar.gz   # install from a GH ref tarball
 
-  ansible_runner: ansible-runner==2.2.1 # install from PyPI
+  ansible_runner: ansible-runner==2.2.1   # install from PyPI
 
   # FIXME: inline splat-as-string sucks, make separate keys for collections and roles
   galaxy: |

diff --git a/test/unit/test_introspect.py b/test/unit/test_introspect.py
@@ -1,6 +1,6 @@
 import os
 
-from ansible_builder.target_scripts.introspect import process, process_collection, simple_combine
+from ansible_builder._target_scripts.introspect import process, process_collection, simple_combine
 from ansible_builder.requirements import sanitize_requirements
 
 

diff --git a/tox.ini b/tox.ini
@@ -17,7 +17,7 @@ commands =
     yamllint --version
     yamllint -s .
 
-[testenv:unit{,-py38,-py39,-py310}]
+[testenv:unit{,-py39,-py310}]
 description = Run unit tests
 commands = pytest {posargs:test/unit}
 
@@ -27,7 +27,7 @@ commands = pytest {posargs:test/unit}
 description = Run pulp integration tests
 commands = pytest -n 1 -m "serial" {posargs:test/pulp_integration}
 
-[testenv:integration{,-py38,-py39,-py310}]
+[testenv:integration{,-py39,-py310}]
 description = Run integration tests
 # rootless podman reads $HOME
 passenv =