Repository: cekit/cekit

Issue:
Make sure we execute cleanup after installing any packages
We need to ensure that YUM/DNF cleanup is executed when we install any packages.

Related to #83 and probably to #343 too.

Discussion:

Patch:
diff --git a/cekit/template_helper.py b/cekit/template_helper.py
index 9e61cd0..d0eccc9 100644
--- a/cekit/template_helper.py
+++ b/cekit/template_helper.py
@@ -10,6 +10,25 @@ class TemplateHelper(object):
     def module(self, to_install):
         return self._module_registry.get_module(to_install.name, to_install.version)
 
+    def packages_to_install(self, image):
+        """
+        Method that returns list of packages to be installed by any of
+        modules or directly in the image
+        """
+        all_modules = []
+        packages = []
+
+        if 'modules' in image and 'install' in image.modules:
+            all_modules += [self.module(m) for m in image.modules.install]
+
+        all_modules.append(image)
+
+        for module in all_modules:
+            if 'packages' in module and 'install' in module.packages:
+                packages += module.packages.install
+
+        return packages
+
     def filename(self, source):
         """Simple helper to return the file specified name"""
 
diff --git a/cekit/templates/template.jinja b/cekit/templates/template.jinja
index 1da28a0..dd5870f 100644
--- a/cekit/templates/template.jinja
+++ b/cekit/templates/template.jinja
@@ -22,11 +22,65 @@ rm{% for repo in repositories %} /etc/yum.repos.d/{{ repo.filename }}{% endfor %
     && rpm -q{% for package in packages %} {{ package }}{% endfor %}
     {% endif %}
 {%- endmacro %}
+{% macro repo_make_cache(pkg_manager) -%}
+    {% if pkg_manager in ['microdnf', 'yum'] %}
+{{ pkg_manager }} makecache
+    {% endif %}
+{%- endmacro %}
 {% macro repo_clear_cache(pkg_manager) -%}
     {% if pkg_manager in ['microdnf', 'yum'] %}
 {{ pkg_manager }} clean all && [ ! -d /var/cache/yum ] || rm -rf /var/cache/yum
     {% endif %}
 {%- endmacro %}
+{% macro process_module(module) %}
+# begin {{ module.name }}:{{ module.version }}
+{% if module.packages and module.packages.install %}
+
+# Install required RPMs and ensure that the packages were installed
+USER root
+RUN {{ pkg_install(pkg_manager, module.packages.install) }}
+{% endif %}
+{% if helper.envs(module.envs)|length > 0 %}
+
+# Environment variables
+ENV \
+    {% for env in helper.envs(module.envs)|sort(attribute='name') %}
+    {{ env.name }}="{{ env.value }}" {% if loop.index < loop.length %}\{% endif %}
+
+    {% endfor %}
+{% endif %}
+{% if module.labels|length > 0 %}
+
+# Labels
+LABEL \
+      {% for label in module.labels|sort(attribute='name') %}
+      {{ label.name }}="{{ label.value }}" {% if loop.index < loop.length %} \{% endif %}
+
+      {% endfor %}
+{% endif %}
+{% if helper.ports(module.ports)|length > 0 %}
+
+# Exposed ports
+EXPOSE {%- for port in helper.ports(module.ports) %} {{ port }}{% endfor %}
+{% endif %}
+{%- if module.execute|length > 0 %}
+
+# Custom scripts
+{% for exec in module.execute %}
+USER {{ exec.user }}
+RUN [ "bash", "-x", "/tmp/scripts/{{ exec.directory }}/{{ exec.script }}" ]
+{% endfor %}
+{% endif %}
+{%- if module.volumes|length > 0 %}
+
+# Volumes
+{% for volume in module.volumes %}
+VOLUME ["{{ volume['path'] }}"]
+{% endfor %}
+{% endif %}
+
+# end {{ module.name }}:{{ module.version }}
+{%- endmacro %}
 
 # Copyright 2017 Red Hat
 #
@@ -62,9 +116,9 @@ RUN {{ repo_install(pkg_manager, repo.rpm) }}
 {% endif %}
 {% endfor %}
 {% endif %}
-{% if packages.repositories_injected or packages.repositories %}
 
-RUN yum makecache
+{% if helper.packages_to_install(image) %}
+RUN {{ repo_make_cache(pkg_manager) }}
 {% endif %}
 
 # Add scripts used to configure the image
@@ -78,55 +132,7 @@ COPY \
     {% endfor %}
     /tmp/artifacts/
 {% endif %}
-{% macro process_module(module) %}
-# begin {{ module.name }}:{{ module.version }}
-{% if module.packages and module.packages.install %}
-
-# Install required RPMs and ensure that the packages were installed
-USER root
-RUN {{ pkg_install(pkg_manager, module.packages.install) }}
-{% endif %}
-{% if helper.envs(module.envs)|length > 0 %}
-
-# Environment variables
-ENV \
-    {% for env in helper.envs(module.envs)|sort(attribute='name') %}
-    {{ env.name }}="{{ env.value }}" {% if loop.index < loop.length %}\{% endif %}
-
-    {% endfor %}
-{% endif %}
-{% if module.labels|length > 0 %}
-
-# Labels
-LABEL \
-      {% for label in module.labels|sort(attribute='name') %}
-      {{ label.name }}="{{ label.value }}" {% if loop.index < loop.length %} \{% endif %}
-
-      {% endfor %}
-{% endif %}
-{% if helper.ports(module.ports)|length > 0 %}
-
-# Exposed ports
-EXPOSE {%- for port in helper.ports(module.ports) %} {{ port }}{% endfor %}
-{% endif %}
-{%- if module.execute|length > 0 %}
-
-# Custom scripts
-{% for exec in module.execute %}
-USER {{ exec.user }}
-RUN [ "bash", "-x", "/tmp/scripts/{{ exec.directory }}/{{ exec.script }}" ]
-{% endfor %}
-{% endif %}
-{%- if module.volumes|length > 0 %}
-
-# Volumes
-{% for volume in module.volumes %}
-VOLUME ["{{ volume['path'] }}"]
-{% endfor %}
-{% endif %}
 
-# end {{ module.name }}:{{ module.version }}
-{% endmacro %}
 {% for to_install in image.modules.install %}
 {{ process_module(helper.module(to_install)) }}
 {% endfor %}
@@ -135,16 +141,18 @@ VOLUME ["{{ volume['path'] }}"]
 USER root
 RUN [ ! -d /tmp/scripts ] || rm -rf /tmp/scripts
 RUN [ ! -d /tmp/artifacts ] || rm -rf /tmp/artifacts
-{% if packages.repositories_injected or packages.repositories %}
+
+{% if helper.packages_to_install(image) %}
+# Clear package manager metadata
 RUN {{ repo_clear_cache(pkg_manager) }}
-{% if packages.repositories_injected %}
+{% endif %}
 
+{% if packages.repositories_injected %}
 # Remove custom repo files
 RUN {{ repo_remove(pkg_manager, packages.repositories_injected) }}
 {% endif %}
-{% endif %}
 
-# run user
+# Run user
 USER {{ run['user'] }}
 {% if 'workdir' in run %}
 


Test patch:
diff --git a/tests/modules/repo_packages/packages_module/module.yaml b/tests/modules/repo_packages/packages_module/module.yaml
new file mode 100644
index 0000000..2bc0502
--- /dev/null
+++ b/tests/modules/repo_packages/packages_module/module.yaml
@@ -0,0 +1,6 @@
+schema_version: 1
+name: packages_module
+packages:
+  install:
+    - kernel
+    - java
\ No newline at end of file
diff --git a/tests/modules/repo_packages/packages_module_1/module.yaml b/tests/modules/repo_packages/packages_module_1/module.yaml
new file mode 100644
index 0000000..7739e5b
--- /dev/null
+++ b/tests/modules/repo_packages/packages_module_1/module.yaml
@@ -0,0 +1,6 @@
+schema_version: 1
+name: packages_module_1
+packages:
+  install:
+    - wget
+    - mc
\ No newline at end of file
diff --git a/tests/test_validate.py b/tests/test_validate.py
index 1a03a4c..5ba28b5 100644
--- a/tests/test_validate.py
+++ b/tests/test_validate.py
@@ -614,7 +614,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child_of_child/script_d" ]
 
 # end child_of_child:None
-
 # begin child2_of_child:None
 
 # Custom scripts
@@ -622,7 +621,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child2_of_child/scripti_e" ]
 
 # end child2_of_child:None
-
 # begin child3_of_child:None
 
 # Custom scripts
@@ -630,7 +628,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child3_of_child/script_f" ]
 
 # end child3_of_child:None
-
 # begin child:None
 
 # Environment variables
@@ -642,7 +639,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child/script_b" ]
 
 # end child:None
-
 # begin child_2:None
 
 # Custom scripts
@@ -650,7 +646,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child_2/script_c" ]
 
 # end child_2:None
-
 # begin child_of_child3:None
 
 # Custom scripts
@@ -658,7 +653,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child_of_child3/script_g" ]
 
 # end child_of_child3:None
-
 # begin child2_of_child3:None
 
 # Custom scripts
@@ -666,11 +660,9 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/child2_of_child3/script_h" ]
 
 # end child2_of_child3:None
-
 # begin child_3:None
 
 # end child_3:None
-
 # begin master:None
 
 # Environment variables
@@ -726,8 +718,10 @@ def test_override_modules_flat(tmpdir, mocker):
         yaml.dump(img_desc, fd, default_flow_style=False)
 
     run_cekit(image_dir)
+    
     assert check_dockerfile_text(image_dir, 'foo="mod_2"')
-
+    assert not check_dockerfile_text(image_dir, "RUN yum makecache")
+    assert not check_dockerfile_text(image_dir, "RUN yum clean all")
 
 def test_execution_order_flat(tmpdir, mocker):
     mocker.patch.object(sys, 'argv', ['cekit',
@@ -766,7 +760,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/mod_1/c" ]
 
 # end mod_1:None
-
 # begin mod_2:None
 
 # Environment variables
@@ -782,7 +775,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/mod_2/c" ]
 
 # end mod_2:None
-
 # begin mod_3:None
 
 # Custom scripts
@@ -794,7 +786,6 @@ USER root
 RUN [ "bash", "-x", "/tmp/scripts/mod_3/c" ]
 
 # end mod_3:None
-
 # begin mod_4:None
 
 # Custom scripts
@@ -808,7 +799,75 @@ RUN [ "bash", "-x", "/tmp/scripts/mod_4/c" ]
 # end mod_4:None
 """
     assert check_dockerfile_text(image_dir, expected_modules_order)
+    assert not check_dockerfile_text(image_dir, "RUN yum makecache")
+    assert not check_dockerfile_text(image_dir, "RUN yum clean all")
+
+def test_package_related_commands_packages_in_module(tmpdir, mocker):
+    mocker.patch.object(sys, 'argv', ['cekit',
+                                      '-v',
+                                      'generate'])
+
+    image_dir = str(tmpdir.mkdir('source'))
+    copy_repos(image_dir)
+
+    img_desc = image_descriptor.copy()
+    img_desc['modules']['install'] = [{'name': 'packages_module'}, {'name': 'packages_module_1'}]
+    img_desc['modules']['repositories'] = [{'name': 'modules',
+                                            'path': 'tests/modules/repo_packages'}]
+
+    with open(os.path.join(image_dir, 'image.yaml'), 'w') as fd:
+        yaml.dump(img_desc, fd, default_flow_style=False)
+
+    run_cekit(image_dir)
+
+    expected_packages_order_install = """
+# begin packages_module:None
+
+# Install required RPMs and ensure that the packages were installed
+USER root
+RUN yum --setopt=tsflags=nodocs install -y kernel java \\
+    && rpm -q kernel java
+
+# end packages_module:None
+# begin packages_module_1:None
+
+# Install required RPMs and ensure that the packages were installed
+USER root
+RUN yum --setopt=tsflags=nodocs install -y wget mc \\
+    && rpm -q wget mc
+
+# end packages_module_1:None
+"""
+
+    assert check_dockerfile_text(image_dir, "RUN yum makecache")
+    assert check_dockerfile_text(image_dir, expected_packages_order_install)
+    assert check_dockerfile_text(image_dir, "RUN yum clean all && [ ! -d /var/cache/yum ] || rm -rf /var/cache/yum")
+
+def test_package_related_commands_packages_in_image(tmpdir, mocker):
+    mocker.patch.object(sys, 'argv', ['cekit',
+                                      '-v',
+                                      'generate'])
+
+    image_dir = str(tmpdir.mkdir('source'))
+    copy_repos(image_dir)
+
+    img_desc = image_descriptor.copy()
+    img_desc['packages'] = {'install': ['wget', 'mc']}
+
+    with open(os.path.join(image_dir, 'image.yaml'), 'w') as fd:
+        yaml.dump(img_desc, fd, default_flow_style=False)
+
+    run_cekit(image_dir)
+
+    expected_packages_install = """
+# Install required RPMs and ensure that the packages were installed
+USER root
+RUN yum --setopt=tsflags=nodocs install -y wget mc \\
+    && rpm -q wget mc
+"""
 
+    assert check_dockerfile_text(image_dir, "RUN yum makecache")
+    assert check_dockerfile_text(image_dir, expected_packages_install)
 
 def test_nonexisting_image_descriptor(mocker, tmpdir, caplog):
     mocker.patch.object(sys, 'argv', ['cekit',