Be explicit about generating release for images

The config now uses similar logic what previous commit did for OSTree.
Also we should report error when an unknown generator is used.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

doc/configuration.rst

@@ -929,8 +929,9 @@ will be provided according to :ref:`automatic versioning <auto-version>`.
  * ``image_build_version``
  * ``live_images_version``
 
-Release is specified by these options. If set explicitly to ``None``, a value
+Release is specified by these options. If set to a magic value to
-will be generated according to :ref:`automatic versioning <auto-version>`.
+``!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN``, a value will be generated according
+to :ref:`automatic versioning <auto-version>`.
 
  * ``global_release`` -- global fallback setting
  * ``live_media_release``
@@ -1001,9 +1002,10 @@ Live Media Settings
       * ``ksurl`` (*str*)
       * ``ksversion`` (*str*)
       * ``scratch`` (*bool*)
-      * ``release`` (*str*) -- a string with the release, or explicit ``None``
+      * ``release`` (*str*) -- a string with the release, or
-        for automatically generating one. See :ref:`automatic versioning
+        ``!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN`` to automatically generate a
-        <auto-version>` for details.
+        suitable value. See :ref:`automatic versioning <auto-version>` for
+        details.
       * ``skip_tag`` (*bool*)
       * ``repo`` (*str|[str]*) -- repos specified by URL or variant UID
       * ``title`` (*str*)
@@ -1013,29 +1015,6 @@ Live Media Settings
 
       * ``repo_from`` -- deprecated, use ``repo`` instead
 
-If many of your media use the same value for one of ``ksurl``, ``release``,
-``target`` or ``version``, consider using these options to set the value in one
-place and have all media inherit it.
-
-**live_media_ksurl**
-    (*str*) -- Provides a fallback for media that do not specify ``ksurl`` in
-    the ``live_media`` block.
-
-**live_media_release**
-    (*str*) -- Provides a fallback for media that do not specify ``release`` in
-    the ``live_media`` block. Please note that if you set this, there is no way
-    to unset it for a particular media. This is important if you want the
-    release generated by Koji.
-
-**live_media_target**
-    (*str*) -- Provides a fallback for media that do not specify ``target`` in
-    the ``live_media`` block.
-
-**live_media_version**
-    (*str*) -- Provides a fallback for media that do not specify ``version`` in
-    the ``live_media`` block.
-
-
 
 Image Build Settings
 ====================
@@ -1054,8 +1033,9 @@ Image Build Settings
     automatically transformed into format suitable for ``koji``. A repo for the
     currently built variant will be added as well.
 
-    If you explicitly set ``release`` to ``None``, it will be replaced with
+    If you explicitly set ``release`` to
-    a value generated as described in :ref:`automatic versioning <auto-version>`.
+    ``!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN``, it will be replaced with a value
+    generated as described in :ref:`automatic versioning <auto-version>`.
 
     Please don't set ``install_tree``. This gets automatically set by *pungi*
     based on current variant. You can use ``install_tree_from`` key to use
@@ -1136,7 +1116,7 @@ Example
                     'repo': ['Everything'],
 
                     # Set release automatically.
-                    'release': None,
+                    'release': '!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN',
                 }
             }
         ]
@@ -1231,7 +1211,8 @@ an OSTree repository. This always runs in Koji as a ``runroot`` task.
 
     * ``repo`` -- (*str|[str]*) repos specified by URL or variant UID
     * ``release`` -- (*str*) Release value to set for the installer image. Set
-      to ``None`` to generate the value :ref:`automatically <auto-version>`.
+      to ``!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN`` to generate the value
+      :ref:`automatically <auto-version>`.
     * ``failable`` -- (*[str]*) List of architectures for which this
       deliverable is not release blocking.
 
@@ -1268,7 +1249,7 @@ Example config
                     "https://example.com/extra-repo1.repo",
                     "https://example.com/extra-repo2.repo",
                 ],
-                "release": None,
+                "release": "!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN",
                 "installpkgs": ["fedora-productimg-atomic"],
                 "add_template": ["atomic-installer/lorax-configure-repo.tmpl"],
                 "add_template_var": [

pungi/phases/base.py

@@ -112,15 +112,15 @@ class ImageConfigMixin(object):
 
     def get_release(self, cfg):
         """
-        If release is set explicitly to None, replace it with date and respin.
+        If release is set to a magic string (or explicitly to None -
-        Uses configuration passed as argument, phase specific settings and
+        deprecated), replace it with a generated value. Uses configuration
-        global settings.
+        passed as argument, phase specific settings and global settings.
         """
         for key, conf in [('release', cfg),
                           ('%s_release' % self.name, self.compose.conf),
                           ('global_release', self.compose.conf)]:
             if key in conf:
-                return conf[key] or self.compose.image_release
+                return util.version_generator(self.compose, conf[key]) or self.compose.image_release
         return None
 
     def get_ksurl(self, cfg):

pungi/phases/image_build.py

@@ -6,7 +6,7 @@ import time
 from kobo import shortcuts
 
 from pungi.util import get_variant_data, makedirs, get_mtime, get_file_size, failable
-from pungi.util import translate_path, get_repo_urls
+from pungi.util import translate_path, get_repo_urls, version_generator
 from pungi.phases import base
 from pungi.linker import Linker
 from pungi.wrappers.kojiwrapper import KojiWrapper
@@ -66,8 +66,9 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG
 
     def _set_release(self, image_conf):
         """If release is set explicitly to None, replace it with date and respin."""
-        if 'release' in image_conf and image_conf['release'] is None:
+        if 'release' in image_conf:
-            image_conf['release'] = self.compose.image_release
+            image_conf['release'] = (version_generator(self.compose, image_conf['release']) or
+                                     self.compose.image_release)
 
     def run(self):
         for variant in self.compose.get_variants():

pungi/phases/ostree_installer.py

@@ -9,7 +9,7 @@ from kobo import shortcuts
 
 from .base import ConfigGuardedPhase, PhaseLoggerMixin
 from .. import util
-from ..util import get_volid, get_repo_urls
+from ..util import get_volid, get_repo_urls, version_generator
 from ..wrappers import kojiwrapper, iso, lorax, scm
 
 
@@ -70,8 +70,8 @@ class OstreeInstallerThread(WorkerThread):
                              self.template_dir, logger=self.pool._logger)
 
     def _get_release(self, compose, config):
-        if 'release' in config and config['release'] is None:
+        if 'release' in config:
-            return compose.image_release
+            return version_generator(compose, config['release']) or compose.image_release
         return config.get('release', None)
 
     def _copy_image(self, compose, variant, arch, filename, output_dir):

pungi/util.py

@@ -769,4 +769,8 @@ def version_generator(compose, gen):
     """
     if gen == '!OSTREE_VERSION_FROM_LABEL_DATE_TYPE_RESPIN':
         return '%s.%s' % (compose.image_version, compose.image_release)
+    if gen == '!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN':
+        return compose.image_release
+    if gen and gen[0] == '!':
+        raise RuntimeError("Unknown version generator '%s'" % gen)
     return gen

tests/test_imagebuildphase.py

@@ -112,7 +112,7 @@ class TestImageBuildPhase(PungiTestCase):
     def test_image_build_phase_global_options(self, ThreadPool):
         compose = DummyCompose(self.topdir, {
             'image_build_ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git',
-            'image_build_release': None,
+            'image_build_release': '!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN',
             'image_build_target': 'f24',
             'image_build_version': 'Rawhide',
             'image_build': {
@@ -171,7 +171,7 @@ class TestImageBuildPhase(PungiTestCase):
     def test_image_build_phase_missing_version(self, ThreadPool):
         compose = DummyCompose(self.topdir, {
             'image_build_ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git',
-            'image_build_release': None,
+            'image_build_release': '!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN',
             'image_build_target': 'f24',
             'image_build': {
                 '^Server$': [
@@ -481,6 +481,44 @@ class TestImageBuildPhase(PungiTestCase):
         self.assertEqual(args[0][1].get('image_conf', {}).get('image-build', {}).get('release'),
                          '20151203.t.0')
 
+    @mock.patch('pungi.phases.image_build.ThreadPool')
+    def test_image_build_create_release_with_explicit_config(self, ThreadPool):
+        compose = DummyCompose(self.topdir, {
+            'image_build': {
+                '^Server$': [
+                    {
+                        'image-build': {
+                            'format': [('docker', 'tar.xz')],
+                            'name': 'Fedora-Docker-Base',
+                            'target': 'f24',
+                            'version': 'Rawhide',
+                            'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git',
+                            'kickstart': "fedora-docker-base.ks",
+                            'distro': 'Fedora-20',
+                            'disk_size': 3,
+                            'arches': ['x86_64'],
+                            'release': '!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN',
+                        }
+                    }
+                ]
+            },
+            'koji_profile': 'koji',
+        })
+
+        self.assertValidConfig(compose.conf)
+
+        phase = ImageBuildPhase(compose)
+
+        phase.run()
+
+        # assert at least one thread was started
+        self.assertTrue(phase.pool.add.called)
+
+        self.assertTrue(phase.pool.queue_put.called_once)
+        args, kwargs = phase.pool.queue_put.call_args
+        self.assertEqual(args[0][1].get('image_conf', {}).get('image-build', {}).get('release'),
+                         '20151203.t.0')
+
     @mock.patch('pungi.phases.image_build.ThreadPool')
     def test_image_build_scratch_build(self, ThreadPool):
         compose = DummyCompose(self.topdir, {

tests/test_ostree_installer_phase.py

@@ -355,6 +355,67 @@ class OstreeThreadTest(helpers.PungiTestCase):
         self.assertImageAdded(self.compose, ImageCls, iso)
         self.assertAllCopied(run)
 
+    @mock.patch('kobo.shortcuts.run')
+    @mock.patch('productmd.images.Image')
+    @mock.patch('pungi.util.get_mtime')
+    @mock.patch('pungi.util.get_file_size')
+    @mock.patch('pungi.phases.ostree_installer.iso')
+    @mock.patch('os.link')
+    @mock.patch('pungi.wrappers.kojiwrapper.KojiWrapper')
+    def test_run_with_explicitly_generated_release(self, KojiWrapper, link, iso,
+                                                   get_file_size, get_mtime, ImageCls, run):
+        pool = mock.Mock()
+        cfg = {
+            'repo': 'Everything',
+            'release': '!RELEASE_FROM_LABEL_DATE_TYPE_RESPIN',
+            "installpkgs": ["fedora-productimg-atomic"],
+            "add_template": ["/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl"],
+            "add_template_var": [
+                "ostree_osname=fedora-atomic",
+                "ostree_ref=fedora-atomic/Rawhide/x86_64/docker-host",
+            ],
+            "add_arch_template": ["/spin-kickstarts/atomic-installer/lorax-embed-repo.tmpl"],
+            "add_arch_template_var": [
+                "ostree_repo=https://kojipkgs.fedoraproject.org/compose/atomic/Rawhide/",
+                "ostree_osname=fedora-atomic",
+                "ostree_ref=fedora-atomic/Rawhide/x86_64/docker-host",
+            ],
+        }
+        self.compose.conf['runroot_weights'] = {'ostree_installer': 123}
+        koji = KojiWrapper.return_value
+        koji.run_runroot_cmd.return_value = {
+            'task_id': 1234,
+            'retcode': 0,
+            'output': 'Foo bar\n',
+        }
+        get_file_size.return_value = 1024
+        get_mtime.return_value = 13579
+        final_iso_path = self.topdir + '/compose/Everything/x86_64/iso/image-name'
+
+        t = ostree.OstreeInstallerThread(pool)
+
+        t.process((self.compose, self.compose.variants['Everything'], 'x86_64', cfg), 1)
+
+        self.assertRunrootCall(
+            koji,
+            'file://%s/compose/Everything/x86_64/os' % self.topdir,
+            '20151203.t.0',
+            isfinal=True,
+            extra=['--installpkgs=fedora-productimg-atomic',
+                   '--add-template=/spin-kickstarts/atomic-installer/lorax-configure-repo.tmpl',
+                   '--add-arch-template=/spin-kickstarts/atomic-installer/lorax-embed-repo.tmpl',
+                   '--add-template-var=ostree_osname=fedora-atomic',
+                   '--add-template-var=ostree_ref=fedora-atomic/Rawhide/x86_64/docker-host',
+                   '--add-arch-template-var=ostree_repo=https://kojipkgs.fedoraproject.org/compose/atomic/Rawhide/',
+                   '--add-arch-template-var=ostree_osname=fedora-atomic',
+                   '--add-arch-template-var=ostree_ref=fedora-atomic/Rawhide/x86_64/docker-host',
+                   '--logfile=%s/%s/lorax.log' % (self.topdir, LOG_PATH)],
+            weight=123,
+        )
+        self.assertIsoLinked(link, get_file_size, get_mtime, final_iso_path)
+        self.assertImageAdded(self.compose, ImageCls, iso)
+        self.assertAllCopied(run)
+
     @mock.patch('kobo.shortcuts.run')
     @mock.patch('productmd.images.Image')
     @mock.patch('pungi.util.get_mtime')

tests/test_util.py

@@ -642,5 +642,23 @@ class GetRepoFuncsTestCase(unittest.TestCase):
         self.assertEqual(repos, expect)
 
 
+class TestVersionGenerator(unittest.TestCase):
+    def test_unknown_generator(self):
+        compose = mock.Mock()
+        with self.assertRaises(RuntimeError) as ctx:
+            util.version_generator(compose, '!GIMME_VERSION')
+
+        self.assertEqual(str(ctx.exception),
+                         "Unknown version generator '!GIMME_VERSION'")
+
+    def test_passthrough_value(self):
+        compose = mock.Mock()
+        self.assertEqual(util.version_generator(compose, '1.2.3'), '1.2.3')
+
+    def test_passthrough_none(self):
+        compose = mock.Mock()
+        self.assertEqual(util.version_generator(compose, None), None)
+
+
 if __name__ == "__main__":
     unittest.main()