From f7626b7d16a57adbd33931b01fa5b007606929d7 Mon Sep 17 00:00:00 2001 From: Nick Cross Date: Wed, 27 Jul 2022 11:12:24 +0100 Subject: [PATCH] Revert changes for module processing order. Fix handling of package managers in included repositories. Improve documentation. Revert "Install modules (including executions) *after* packages are installed, not before." This reverts commit e9410afcadd13a13a7d8a57e23ce6666ec038ecc. --- cekit/descriptor/image.py | 10 ++- cekit/templates/template.jinja | 2 +- docs/descriptor/includes/packages/manager.rst | 4 + docs/handbook/modules/image_order.rst | 75 +++++++++++++++++++ docs/handbook/modules/index.rst | 1 + docs/handbook/modules/merging.rst | 6 ++ tests/test_integ_builder_osbs.py | 52 ++++++------- tests/test_integ_install_ordering.py | 57 +++++++++++++- 8 files changed, 176 insertions(+), 31 deletions(-) create mode 100644 docs/handbook/modules/image_order.rst diff --git a/cekit/descriptor/image.py b/cekit/descriptor/image.py index b7f405e5..e3da366d 100644 --- a/cekit/descriptor/image.py +++ b/cekit/descriptor/image.py @@ -388,6 +388,7 @@ def apply_module_overrides(self, module_registry): self.packages._descriptor["repositories"] = list( self._package_repositories.values() ) + # final 'run' value if self.run: self.run = self.run.merge(self._module_run) @@ -396,7 +397,7 @@ def apply_module_overrides(self, module_registry): def process_install_list( self, source, to_install_list, install_list, module_registry - ): + ) -> None: module_overrides = self._image_overrides["modules"] artifact_overrides = self._image_overrides["artifacts"] for to_install in to_install_list: @@ -457,6 +458,13 @@ def process_install_list( if name not in self._package_repositories: self._package_repositories[name] = repo + # collect package manager + if not self.packages.manager and module.packages.manager: + logger.debug( + f"Applying module package manager of {module.packages.manager} to image" + ) + self.packages._descriptor["manager"] = module.packages.manager + # incorporate run specification contributed by module if module.run: # we're looping in order of install, so we want the current module to override whatever we have diff --git a/cekit/templates/template.jinja b/cekit/templates/template.jinja index c451ddc8..74ddc43b 100644 --- a/cekit/templates/template.jinja +++ b/cekit/templates/template.jinja @@ -184,10 +184,10 @@ rm{% for repo in repositories %} /etc/yum.repos.d/{{ repo.filename }}{% endfor % {% endfor %} {% endif %} -{{ process_module(animage) }} {% for to_install in animage.modules.install %} {{ process_module(helper.module(to_install), animage) }} {% endfor %} +{{ process_module(animage) }} {%if helper.cachito(animage) %} RUN rm -rf $REMOTE_SOURCE_DIR {% endif %} diff --git a/docs/descriptor/includes/packages/manager.rst b/docs/descriptor/includes/packages/manager.rst index a7ee5efb..81b0db02 100644 --- a/docs/descriptor/includes/packages/manager.rst +++ b/docs/descriptor/includes/packages/manager.rst @@ -15,6 +15,10 @@ Currently available options are ``yum``, ``dnf``, ``microdnf``, ``apt-get`` and If you do not specify this key the default value is ``yum``. If your image requires different package manager you need to specify it. + It is only possible to define a single package manager for an image (although multi-stage images may have + different package managers). A package manager may be defined in a module or in an image (the latter takes + precedence). + The default ``yum`` value will work fine on Fedora and RHEL images because OS maintains a symlink to the proper package manager. diff --git a/docs/handbook/modules/image_order.rst b/docs/handbook/modules/image_order.rst new file mode 100644 index 00000000..cf85cf73 --- /dev/null +++ b/docs/handbook/modules/image_order.rst @@ -0,0 +1,75 @@ +Image Descriptor and Modules +========================= + +.. contents:: + :backlinks: none + +.. note:: + This chapter applies to :doc:`builder engines ` that use Dockerfile as the input. + + +While :doc:`module processing ` chapter covered the template processing modules this section +describes how the image processing interacts with the module processing. + + +.. graphviz:: + :align: center + :alt: Module installation + :name: module-installation-diagram + + digraph module_installation { + graph [fontsize="11", fontname="Open Sans", compound="true", splines=ortho, nodesep=0.5, ranksep=0.75]; + node [shape="box", fontname="Open Sans", fontsize="10"]; + + // main rendering + subgraph cluster_0 { + label="Main Rendering Generation"; + builder [label="Builder image handling"]; + from [label="FROM generation"]; + extra [label="Extra directory copying"]; + image [label="Image Processing"]; + cleanup [label="Cleanup"]; + final [label="Final stages", href="#final-stages"]; + } + + // process_image + subgraph cluster_1 { + label="Image Rendering"; + cachito [label="Cachito Support", rank=same]; + repo [label="Repository Management"]; + module [label="Included Module Processing"]; + complete_image [label="Final Image stages", href="#final-image-stages"]; + } + + // process_module + subgraph cluster_2 { + artifact [label="Artifact copying", rank=same]; + pkg_install [label="Package installation"]; + ports [label="Expose Ports"]; + run [label="Run scripts"]; + volumes [label="Configure volumes"]; + } + + // graph control + builder -> from -> extra -> image -> cleanup -> final; + cachito -> repo -> module -> complete_image; + artifact -> pkg_install -> ports -> run -> volumes; + + // subgraph links + builder -> cachito[constraint=false]; + image -> cachito[constraint=false]; + module -> artifact[constraint=false]; + complete_image -> artifact[constraint=false]; + } + + +Final Stages +""""""""""""""""""""""" + +This encompasses defining the ``USER``, the ``WORKDIR``, and ``ENTRYPOINT``. Finally the ``RUN`` command is generated. + +Final Image Stages +""""""""""""""""""""""" + +This encompasses the final part of the generation for the image descriptor which may include e.g. package installation. +Note that this happens **after** modules have been included and processed. diff --git a/docs/handbook/modules/index.rst b/docs/handbook/modules/index.rst index 41b46b89..9b4839b8 100644 --- a/docs/handbook/modules/index.rst +++ b/docs/handbook/modules/index.rst @@ -9,3 +9,4 @@ to succeed with CEKit. merging versioning + image_order diff --git a/docs/handbook/modules/merging.rst b/docs/handbook/modules/merging.rst index 834dcf46..8ba6bd08 100644 --- a/docs/handbook/modules/merging.rst +++ b/docs/handbook/modules/merging.rst @@ -7,6 +7,7 @@ Module processing .. note:: This chapter applies to :doc:`builder engines ` that use Dockerfile as the input. + Understanding how modules are merged together is important. This knowledge will let you introduce modules that work better together and make rebuilds faster which is an important aspect of the image and module development. @@ -94,6 +95,11 @@ because every image is squashed (depends on builder though). Package installation is executed as ``root`` user. +.. note:: + It is only possible to define a single package manager for an image (although multi-stage images may have + different package managers). A package manager may be defined in a module or in an image (the latter takes + precedence). + Environment variables ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/test_integ_builder_osbs.py b/tests/test_integ_builder_osbs.py index 422884e8..a13e3b84 100644 --- a/tests/test_integ_builder_osbs.py +++ b/tests/test_integ_builder_osbs.py @@ -1932,19 +1932,6 @@ def test_osbs_builder_with_rhpam_1(tmpdir, caplog): COPY $REMOTE_SOURCE $REMOTE_SOURCE_DIR WORKDIR $REMOTE_SOURCE_DIR/app -###### START image 'operator-builder:7.11' -###### \\ - # Set 'operator-builder' image defined environment variables - ENV \\ - JBOSS_IMAGE_NAME="rhpam-7/rhpam-kogito-operator" \\ - JBOSS_IMAGE_VERSION="7.11" - # Set 'operator-builder' image defined labels - LABEL \\ - name="rhpam-7/rhpam-kogito-operator" \\ - version="7.11" -###### / -###### END image 'operator-builder:7.11' - ###### START module 'org.kie.kogito.builder:7.11' ###### \\ # Copy 'org.kie.kogito.builder' module general artifacts to '/workspace/' destination @@ -1959,6 +1946,19 @@ def test_osbs_builder_with_rhpam_1(tmpdir, caplog): ###### / ###### END module 'org.kie.kogito.builder:7.11' +###### START image 'operator-builder:7.11' +###### \\ + # Set 'operator-builder' image defined environment variables + ENV \\ + JBOSS_IMAGE_NAME="rhpam-7/rhpam-kogito-operator" \\ + JBOSS_IMAGE_VERSION="7.11" + # Set 'operator-builder' image defined labels + LABEL \\ + name="rhpam-7/rhpam-kogito-operator" \\ + version="7.11" +###### / +###### END image 'operator-builder:7.11' + RUN rm -rf $REMOTE_SOURCE_DIR ## / @@ -2125,19 +2125,6 @@ def test_osbs_builder_with_rhpam_3(tmpdir, caplog): FROM registry.access.redhat.com/ubi8/go-toolset:1.14.12 AS operator-builder-one USER root -###### START image 'operator-builder-one:7.11' -###### \\ - # Set 'operator-builder-one' image defined environment variables - ENV \\ - JBOSS_IMAGE_NAME="rhpam-7/rhpam-kogito-operator" \\ - JBOSS_IMAGE_VERSION="7.11" - # Set 'operator-builder-one' image defined labels - LABEL \\ - name="rhpam-7/rhpam-kogito-operator" \\ - version="7.11" -###### / -###### END image 'operator-builder-one:7.11' - ###### START module 'org.kie.kogito.builder:7.11' ###### \\ # Copy 'org.kie.kogito.builder' module general artifacts to '/workspace/' destination @@ -2152,6 +2139,19 @@ def test_osbs_builder_with_rhpam_3(tmpdir, caplog): ###### / ###### END module 'org.kie.kogito.builder:7.11' +###### START image 'operator-builder-one:7.11' +###### \\ + # Set 'operator-builder-one' image defined environment variables + ENV \\ + JBOSS_IMAGE_NAME="rhpam-7/rhpam-kogito-operator" \\ + JBOSS_IMAGE_VERSION="7.11" + # Set 'operator-builder-one' image defined labels + LABEL \\ + name="rhpam-7/rhpam-kogito-operator" \\ + version="7.11" +###### / +###### END image 'operator-builder-one:7.11' + ## / ## END builder image diff --git a/tests/test_integ_install_ordering.py b/tests/test_integ_install_ordering.py index 483cb112..8c18bc0d 100644 --- a/tests/test_integ_install_ordering.py +++ b/tests/test_integ_install_ordering.py @@ -28,7 +28,7 @@ def run_cekit(image_dir, descriptor, args=None): @pytest.mark.skipif( platform.system() == "Darwin", reason="Disabled on macOS, cannot run Docker" ) -def test_module_uses_installed_package_in_execute_script(tmp_path): +def test_module_uses_installed_package_in_execute_script(tmp_path, caplog): """Check that when a module has an install script that depends on packages that should be installed in an image, the install script succeeds. """ @@ -51,6 +51,7 @@ def test_module_uses_installed_package_in_execute_script(tmp_path): "name": "test_module", "version": "1.0", "execute": [{"script": str(install_script.name)}], + "packages": {"manager": "apk", "install": ["zip"]}, } ) ) @@ -63,7 +64,57 @@ def test_module_uses_installed_package_in_execute_script(tmp_path): "repositories": [{"name": "modules", "path": "modules/"}], "install": [{"name": "test_module"}], }, - "packages": {"manager": "apk", "install": ["zip"]}, } - run_cekit(image_dir, image_descriptor, args=["build", "docker"]) + run_cekit(image_dir, image_descriptor, args=["-v", "build", "docker"]) + + assert "Applying module package manager of apk to image" in caplog.text + + +@pytest.mark.skipif( + platform.system() == "Darwin", reason="Disabled on macOS, cannot run Docker" +) +def test_module_uses_installed_package_in_execute_script_manager_precedence( + tmp_path, caplog +): + """Check that when a module has an install script that depends on packages that should be installed in an image, + the install script succeeds and that the image manager takes precedence over modules + """ + + image_dir = tmp_path / "image" + module_dir = image_dir / "modules" + + image_dir.mkdir(exist_ok=True) + + # Build the module + module_dir.mkdir(exist_ok=True) + + install_script = module_dir / "install.sh" + install_script.write_text("\n".join(["#!/bin/bash", "zip --version"])) + + module_descriptor_file = module_dir / "module.yaml" + module_descriptor_file.write_text( + yaml.dump( + { + "name": "test_module", + "version": "1.0", + "execute": [{"script": str(install_script.name)}], + "packages": {"manager": "dnf", "install": ["zip"]}, + } + ) + ) + + image_descriptor = { + "name": "test_image", + "version": "1.0", + "from": "alpine:latest", + "modules": { + "repositories": [{"name": "modules", "path": "modules/"}], + "install": [{"name": "test_module"}], + }, + "packages": {"manager": "apk"}, + } + + run_cekit(image_dir, image_descriptor, args=["-v", "build", "docker"]) + + assert "Applying module package manager of apk to image" not in caplog.text