Updates for ostree-container phase

This patch connects the phase into the main script, and adds other
modifications:

* The archive is now stored in the images/ subdirectory in the compose.
* Documentation is updated to correctly mention that variant repos are
  not available.
* Configuration for path and name of the final archive is dropped. There
  are reasonable defaults for this and there's no point in having users
  configure it.
* The extra message for the archive is no longer sent.
* The pungi-make-ostree utility is no longer required in the buildroot.

The pungi-make-ostree utility doesn't do any significant work. It
modifies configuration files (which can happen on the compose host), and
it starts other processes.

This patch changes the ostree-container phase to no longer need the
script in the buildroot. Instead, the utility is called on the compose
host to do the config manipulation and output the needed commands. Those
are then passed into the runroot task.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
(cherry picked from commit 081c31238b)

doc/configuration.rst

@@ -1766,16 +1766,16 @@ another directory. Any new packages in the compose will be added to the
 repository with a new commit.
 
 **ostree**
-    (*dict*) -- a mapping of configuration for each. The format should be
+    (*dict*) -- a mapping of configuration for each variant. The format should
-    ``{variant_uid_regex: config_dict}``. It is possible to use a list of
+    be ``{variant_uid_regex: config_dict}``. It is possible to use a list of
     configuration dicts as well.
 
     The configuration dict for each variant arch pair must have these keys:
 
     * ``treefile`` -- (*str*) Filename of configuration for ``rpm-ostree``.
     * ``config_url`` -- (*str*) URL for Git repository with the ``treefile``.
-    * ``repo`` -- (*str|dict|[str|dict]*) repos specified by URL or variant UID
+    * ``repo`` -- (*str|dict|[str|dict]*) repos specified by URL or a dict of
-      or a dict of repo options, ``baseurl`` is required in the dict.
+      repo options, ``baseurl`` is required in the dict.
     * ``ostree_repo`` -- (*str*) Where to put the ostree repository
 
     These keys are optional:
@@ -1817,13 +1817,11 @@ Example config
         "^Atomic$": {
             "treefile": "fedora-atomic-docker-host.json",
             "config_url": "https://git.fedorahosted.org/git/fedora-atomic.git",
+            "keep_original_sources": True,
             "repo": [
-                "Server",
                 "http://example.com/repo/x86_64/os",
-                {"baseurl": "Everything"},
                 {"baseurl": "http://example.com/linux/repo", "exclude": "systemd-container"},
             ],
-            "keep_original_sources": True,
             "ostree_repo": "/mnt/koji/compose/atomic/Rawhide/",
             "update_summary": True,
             # Automatically generate a reasonable version
@@ -1852,23 +1850,19 @@ https://github.com/containers/skopeo/pull/2114 is resolved. Each invocation
 will thus create a new OCI archive image *from scratch*.
 
 **ostree_container**
-    (*dict*) -- a mapping of configuration for each. The format should be
+    (*dict*) -- a mapping of configuration for each variant. The format should
-    ``{variant_uid_regex: config_dict}``. It is possible to use a list of
+    be ``{variant_uid_regex: config_dict}``. It is possible to use a list of
     configuration dicts as well.
 
     The configuration dict for each variant arch pair must have these keys:
 
     * ``treefile`` -- (*str*) Filename of configuration for ``rpm-ostree``.
     * ``config_url`` -- (*str*) URL for Git repository with the ``treefile``.
-    * ``repo`` -- (*str|dict|[str|dict]*) repos specified by URL or variant UID
-      or a dict of repo options, ``baseurl`` is required in the dict.
-    * ``ociarchive_path`` -- (*str*) Where to put the OCI archive.
-    * ``ociarchive_name`` -- (*str*) Base name to use for the ociarchive file.
-      Final name will be ``{name}-{version}.ociarchive`` (ommitting the version
-      if it is not set).
 
     These keys are optional:
 
+    * ``repo`` -- (*str|dict|[str|dict]*) repos specified by URL or a dict of
+      repo options, ``baseurl`` is required in the dict.
     * ``keep_original_sources`` -- (*bool*) Keep the existing source repos in
       the tree config file. If not enabled, all the original source repos will
       be removed from the tree config file.
@@ -1900,14 +1894,9 @@ Example config
             "config_url": "https://gitlab.com/CentOS/cloud/sagano.git",
             "config_branch": "main",
             "repo": [
-                "Server",
                 "http://example.com/repo/x86_64/os",
-                {"baseurl": "Everything"},
                 {"baseurl": "http://example.com/linux/repo", "exclude": "systemd-container"},
             ],
-            "ociarchive_path": "/mnt/koji/compose/ostree_container/",
-            # Base name to use for the ociarchive file. Final name will be {name}-{version}.ociarchive
-            "ociarchive_name": "sagano",
             # Automatically generate a reasonable version
             "version": "!OSTREE_VERSION_FROM_LABEL_DATE_TYPE_RESPIN",
             # Only run this for x86_64 even if Sagano has more arches

doc/examples.rst

@@ -348,15 +348,8 @@ This is a shortened configuration for Fedora Radhide compose as of 2019-10-14.
             "treefile": "fedora-tier-0-38.yaml",
             "config_url": "https://gitlab.com/CentOS/cloud/sagano.git",
             "config_branch": "main",
-            "repo": [
+            # Consume packages from Everything
-                "Server",
+            "repo": "Everything",
-                "http://example.com/repo/x86_64/os",
-                {"baseurl": "Everything"},
-                {"baseurl": "http://example.com/linux/repo", "exclude": "systemd-container"},
-            ],
-            "ociarchive_path": "/mnt/koji/compose/ostree_container/",
-            # Base name to use for the ociarchive file. Final name will be {name}-{version}.ociarchive
-            "ociarchive_name": "sagano",
             # Automatically generate a reasonable version
             "version": "!OSTREE_VERSION_FROM_LABEL_DATE_TYPE_RESPIN",
             # Only run this for x86_64 even if Sagano has more arches

pungi/checks.py

@@ -1129,8 +1129,6 @@ def make_schema():
                             "properties": {
                                 "treefile": {"type": "string"},
                                 "config_url": {"type": "string"},
-                                "ociarchive_path": {"type": "string"},
-                                "ociarchive_name": {"type": "string"},
                                 "repo": {"$ref": "#/definitions/repos"},
                                 "keep_original_sources": {"type": "boolean"},
                                 "config_branch": {"type": "string"},
@@ -1145,9 +1143,6 @@ def make_schema():
                             "required": [
                                 "treefile",
                                 "config_url",
-                                "repo",
-                                "ociarchive_path",
-                                "ociarchive_name",
                             ],
                             "additionalProperties": False,
                         }

pungi/ostree/__init__.py

@@ -77,16 +77,15 @@ def main(args=None):
     )
     container.set_defaults(_class=Container, func="run")
     container.add_argument(
-        "--ociarchive-path",
+        "--name",
-        metavar="DIR",
-        required=True,
-        help="where to output the OCI archive (required)",
-    )
-    container.add_argument(
-        "--ociarchive-name",
         required=True,
         help="the name of the the OCI archive (required)",
     )
+    container.add_argument(
+        "--path",
+        required=True,
+        help="where to output the OCI archive (required)",
+    )
     container.add_argument(
         "--treefile",
         metavar="FILE",
@@ -102,11 +101,6 @@ def main(args=None):
     container.add_argument(
         "--extra-config", metavar="FILE", help="JSON file contains extra configurations"
     )
-    container.add_argument(
-        "--version",
-        metavar="VERSION",
-        help="version string to be used for OCI archive name",
-    )
 
     installerp = subparser.add_parser(
         "installer", help="Create an OSTree installer image"

pungi/ostree/container.py

@@ -16,20 +16,26 @@
 
 import os
 import json
-from kobo import shortcuts
+import six
+from six.moves import shlex_quote
+
 
 from .base import OSTree
-from .utils import (
+from .utils import tweak_treeconf
-    make_log_file,
+
-    tweak_treeconf,
+
-)
+def emit(cmd):
+    """Print line of shell code into the stream."""
+    if isinstance(cmd, six.string_types):
+        print(cmd)
+    else:
+        print(" ".join([shlex_quote(x) for x in cmd]))
 
 
 class Container(OSTree):
     def _make_container(self):
         """Compose OSTree Container Native image"""
-        log_file = make_log_file(self.logdir, "create-ostree-repo")
+        stamp_file = os.path.join(self.logdir, "%s.stamp" % self.name)
-        stamp_file = os.path.join(self.logdir, "%s.stamp" % self.ociarchive_name)
         cmd = [
             "rpm-ostree",
             "compose",
@@ -42,38 +48,20 @@ class Container(OSTree):
             "--touch-if-changed=%s" % stamp_file,
             self.treefile,
         ]
-        if self.version is None:
+        fullpath = os.path.join(self.path, "%s.ociarchive" % self.name)
-            fullpath = os.path.join(
-                self.ociarchive_path, "%s.ociarchive" % self.ociarchive_name
-            )
-        else:
-            fullpath = os.path.join(
-                self.ociarchive_path,
-                "%s-%s.ociarchive" % (self.ociarchive_name, self.version),
-            )
         cmd.append(fullpath)
 
         # Set the umask to be more permissive so directories get group write
         # permissions. See https://pagure.io/releng/issue/8811#comment-629051
-        oldumask = os.umask(0o0002)
+        emit("umask 0002")
-        try:
+        emit(cmd)
-            shortcuts.run(
-                cmd,
-                show_cmd=True,
-                stdout=True,
-                logfile=log_file,
-                universal_newlines=True,
-            )
-        finally:
-            os.umask(oldumask)
 
     def run(self):
+        self.name = self.args.name
+        self.path = self.args.path
         self.treefile = self.args.treefile
-        self.version = self.args.version
         self.logdir = self.args.log_dir
         self.extra_config = self.args.extra_config
-        self.ociarchive_path = self.args.ociarchive_path
-        self.ociarchive_name = self.args.ociarchive_name
 
         if self.extra_config:
             self.extra_config = json.load(open(self.extra_config, "r"))
@@ -86,13 +74,13 @@ class Container(OSTree):
             repos = []
             keep_original_sources = True
 
-            update_dict = {}
+        update_dict = {}
 
-            self.treefile = tweak_treeconf(
+        self.treefile = tweak_treeconf(
-                self.treefile,
+            self.treefile,
-                source_repos=repos,
+            source_repos=repos,
-                keep_original_sources=keep_original_sources,
+            keep_original_sources=keep_original_sources,
-                update_dict=update_dict,
+            update_dict=update_dict,
-            )
+        )
 
         self._make_container()

pungi/phases/__init__.py

@@ -35,6 +35,7 @@ from .image_checksum import ImageChecksumPhase  # noqa
 from .livemedia_phase import LiveMediaPhase  # noqa
 from .ostree import OSTreePhase  # noqa
 from .ostree_installer import OstreeInstallerPhase  # noqa
+from .ostree_container import OSTreeContainerPhase  # noqa
 from .osbs import OSBSPhase  # noqa
 from .phases_metadata import gather_phases_metadata  # noqa
 

pungi/phases/ostree_container.py

@@ -5,7 +5,6 @@ import json
 import os
 from kobo import shortcuts
 from kobo.threads import ThreadPool, WorkerThread
-from collections import OrderedDict
 
 from pungi.runroot import Runroot
 from .base import ConfigGuardedPhase
@@ -26,7 +25,9 @@ class OSTreeContainerPhase(ConfigGuardedPhase):
         return [
             translate_path(
                 self.compose,
-                self.compose.paths.work.pkgset_repo(pkgset.name, "$basearch"),
+                self.compose.paths.work.pkgset_repo(
+                    pkgset.name, "$basearch", create_dir=False
+                ),
             )
             for pkgset in self.pkgset_phase.package_sets
         ]
@@ -84,7 +85,7 @@ class OSTreeContainerThread(WorkerThread):
             config.get("config_branch", "main"),
         )
 
-        repos = shortcuts.force_list(config["repo"]) + self.repos
+        repos = shortcuts.force_list(config.get("repo", [])) + self.repos
         repos = get_repo_dicts(repos, logger=self.pool)
 
         # copy the original config and update before save to a json file
@@ -116,78 +117,50 @@ class OSTreeContainerThread(WorkerThread):
             compose, variant, arch, config, repodir, extra_config_file=extra_config_file
         )
 
-        if compose.notifier:
-            # 'pungi-make-ostree container' writes to {ociarchive_name}.stamp in
-            # logdir if the compose succeeded. If the compose failed, an exception
-            # will be raised.
-            os.path.exists(
-                os.path.join(self.logdir, "%s.stamp" % config["ociarchive_name"])
-            )
-            if config["version"] is None:
-                filename = "%s.ociarchive" % config["ociarchive_name"]
-            else:
-                filename = (
-                    "%s-%s.ociarchive" % (config["ociarchive_name"], config["version"]),
-                )
-            compose.notifier.send(
-                "ostree_container",
-                variant=variant.uid,
-                arch=arch,
-                filename=filename,
-                version=config["version"],
-                path=translate_path(compose, config["ociarchive_path"]),
-                local_path=config["ociarchive_path"],
-            )
-
         self.pool.log_info("[DONE ] %s" % (msg))
 
     def _run_ostree_container_cmd(
         self, compose, variant, arch, config, config_repo, extra_config_file=None
     ):
-        args = OrderedDict(
+        target_dir = compose.paths.compose.image_dir(variant) % {"arch": arch}
-            [
+        util.makedirs(target_dir)
-                ("log-dir", self.logdir),
+        archive_name = "%s-%s-%s" % (
-                ("treefile", os.path.join(config_repo, config["treefile"])),
+            compose.conf["release_short"],
-                ("version", util.version_generator(compose, config.get("version"))),
+            variant.uid,
-                ("extra-config", extra_config_file),
+            util.version_generator(compose, config.get("version")),
-                ("ociarchive-path", config.get("ociarchive_path")),
-                ("ociarchive-name", config.get("ociarchive_name")),
-            ]
         )
-        default_packages = ["pungi", "ostree", "rpm-ostree", "selinux-policy-targeted"]
+
+        # Run the pungi-make-ostree command locally to create a script to
+        # execute in runroot environment.
+        cmd = [
+            "pungi-make-ostree",
+            "container",
+            "--log-dir=%s" % self.logdir,
+            "--name=%s" % archive_name,
+            "--path=%s" % target_dir,
+            "--treefile=%s" % os.path.join(config_repo, config["treefile"]),
+            "--extra-config=%s" % extra_config_file,
+        ]
+
+        _, runroot_script = shortcuts.run(cmd, universal_newlines=True)
+
+        default_packages = ["ostree", "rpm-ostree", "selinux-policy-targeted"]
         additional_packages = config.get("runroot_packages", [])
         packages = default_packages + additional_packages
         log_file = os.path.join(self.logdir, "runroot.log")
         # TODO: Use to get previous build
-        mounts = [compose.topdir, config["ostree_repo"]]
+        mounts = [compose.topdir]
+
         runroot = Runroot(compose, phase="ostree_container")
-
+        runroot.run(
-        if compose.conf["ostree_container_use_koji_plugin"]:
+            " && ".join(runroot_script.splitlines()),
-            runroot.run_pungi_ostree(
+            log_file=log_file,
-                dict(args),
+            arch=arch,
-                log_file=log_file,
+            packages=packages,
-                arch=arch,
+            mounts=mounts,
-                packages=packages,
+            new_chroot=True,
-                mounts=mounts,
+            weight=compose.conf["runroot_weights"].get("ostree"),
-                weight=compose.conf["runroot_weights"].get("ostree"),
+        )
-            )
-        else:
-            cmd = ["pungi-make-ostree", "container"]
-            for key, value in args.items():
-                if value is True:
-                    cmd.append("--%s" % key)
-                elif value:
-                    cmd.append("--%s=%s" % (key, value))
-
-            runroot.run(
-                cmd,
-                log_file=log_file,
-                arch=arch,
-                packages=packages,
-                mounts=mounts,
-                new_chroot=True,
-                weight=compose.conf["runroot_weights"].get("ostree"),
-            )
 
     def _clone_repo(self, compose, repodir, url, branch):
         scm.get_dir_from_scm(

pungi/scripts/pungi_koji.py

@@ -423,6 +423,7 @@ def run_compose(
         compose, buildinstall_phase, pkgset_phase
     )
     ostree_phase = pungi.phases.OSTreePhase(compose, pkgset_phase)
+    ostree_container_phase = pungi.phases.OSTreeContainerPhase(compose, pkgset_phase)
     createiso_phase = pungi.phases.CreateisoPhase(compose, buildinstall_phase)
     extra_isos_phase = pungi.phases.ExtraIsosPhase(compose, buildinstall_phase)
     liveimages_phase = pungi.phases.LiveImagesPhase(compose)
@@ -451,6 +452,7 @@ def run_compose(
         test_phase,
         ostree_phase,
         ostree_installer_phase,
+        ostree_container_phase,
         extra_isos_phase,
         osbs_phase,
         osbuild_phase,
@@ -525,6 +527,7 @@ def run_compose(
         (gather_phase, createrepo_phase),
         extrafiles_phase,
         (ostree_phase, ostree_installer_phase),
+        ostree_container_phase,
     )
     essentials_phase = pungi.phases.WeaverPhase(compose, essentials_schema)
     essentials_phase.start()