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 e9410af.
rnc · Aug 2, 2022 · f7626b7 · f7626b7
1 parent e663492
commit f7626b7
Show file tree

Hide file tree

Showing 8 changed files with 176 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,75 @@
+Image Descriptor and Modules
+=========================
+
+.. contents::
+    :backlinks: none
+
+.. note::
+    This chapter applies to :doc:`builder engines </handbook/building/builder-engines>` that use Dockerfile as the input.
+
+
+While :doc:`module processing </handbook/modules/merging>` 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
@@ -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
@@ -7,6 +7,7 @@ Module processing
 .. note::
     This chapter applies to :doc:`builder engines </handbook/building/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
@@ -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
@@ -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
Original file line number	Diff line number	Diff line change
Expand Up		@@ -9,3 +9,4 @@ to succeed with CEKit.

		merging
		versioning
		image_order