[live-images] Don't tweak kickstarts

Instead of downloading the kickstart file in Pungi and modifying it,
just pass it to Koji.

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

doc/configuration.rst

@@ -638,7 +638,8 @@ Live Images Settings
     list should be tuples ``(variant_uid_regex, {arch|*: config})``. The config
     should be a dict with these keys:
 
-      * ``kickstart`` (*str|dict*)
+      * ``kickstart`` (*str*)
+      * ``ksurl`` (*str*) [optional] -- where to get the kickstart from
       * ``name`` (*str*)
       * ``version`` (*str*)
       * ``additional_repos`` (*list*) -- external repos specified by URL

pungi/phases/live_images.py

@@ -17,20 +17,17 @@
 
 import os
 import sys
-import copy
 import time
 import pipes
 import shutil
-import tempfile
 
 from kobo.threads import ThreadPool, WorkerThread
 from kobo.shortcuts import run
 
 from pungi.wrappers.kojiwrapper import KojiWrapper
 from pungi.wrappers.iso import IsoWrapper
-from pungi.wrappers.scm import get_file_from_scm
 from pungi.phases.base import PhaseBase
-from pungi.util import get_arch_variant_data
+from pungi.util import get_arch_variant_data, resolve_git_url
 from pungi.paths import translate_path
 
 
@@ -86,11 +83,10 @@ class LiveImagesPhase(PhaseBase):
 
         for variant in self.compose.variants.values():
             for arch in variant.arches + ["src"]:
-                ks_in = get_ks_in(self.compose, arch, variant)
+                data = get_arch_variant_data(self.compose.conf, "live_images", arch, variant)
-                if not ks_in:
+                if not data:
                     continue
-
+                data = data[0]
-                ks_file = tweak_ks(self.compose, arch, variant, ks_in)
 
                 iso_dir = self.compose.paths.compose.iso_dir(arch, variant, symlink_to=symlink_isos_to)
                 if not iso_dir:
@@ -102,32 +98,36 @@ class LiveImagesPhase(PhaseBase):
                     "iso_path": None,
                     "wrapped_rpms_path": iso_dir,
                     "build_arch": arch,
-                    "ks_file": ks_file,
+                    "ks_file": data['kickstart'],
+                    "ksurl": None,
                     "specfile": None,
                     "scratch": False,
                     "label": "",  # currently not used
                 }
+
+                if 'ksurl' in data:
+                    cmd['ksurl'] = resolve_git_url(data['ksurl'])
+
                 cmd["repos"] = [translate_path(
                     self.compose, self.compose.paths.compose.repository(arch, variant, create_dir=False))]
 
                 # additional repos
-                data = get_arch_variant_data(self.compose.conf, "live_images", arch, variant)
+                cmd["repos"].extend(data.get("additional_repos", []))
-                cmd["repos"].extend(data[0].get("additional_repos", []))
+                cmd['repos'].extend(self._get_extra_repos(arch, variant, data.get('repos_from', [])))
-                cmd['repos'].extend(self._get_extra_repos(arch, variant, data[0].get('repos_from', [])))
 
                 # Explicit name and version
-                cmd["name"] = data[0].get("name", None)
+                cmd["name"] = data.get("name", None)
-                cmd["version"] = data[0].get("version", None)
+                cmd["version"] = data.get("version", None)
 
-                cmd['type'] = data[0].get('type', 'live')
+                cmd['type'] = data.get('type', 'live')
 
                 # Specfile (for images wrapped in rpm)
-                cmd["specfile"] = data[0].get("specfile", None)
+                cmd["specfile"] = data.get("specfile", None)
 
                 # Scratch (only taken in consideration if specfile specified)
                 # For images wrapped in rpm is scratch disabled by default
                 # For other images is scratch always on
-                cmd["scratch"] = data[0].get("scratch", False)
+                cmd["scratch"] = data.get("scratch", False)
 
                 format = "%(compose_id)s-%(variant)s-%(arch)s-%(disc_type)s%(disc_num)s%(suffix)s"
                 # Custom name (prefix)
@@ -204,7 +204,8 @@ class CreateLiveImageThread(WorkerThread):
                                                      image_type=cmd['type'],
                                                      wait=True,
                                                      archive=archive,
-                                                     specfile=cmd["specfile"])
+                                                     specfile=cmd["specfile"],
+                                                     ksurl=cmd['ksurl'])
 
         # avoid race conditions?
         # Kerberos authentication failed: Permission denied in replay cache code (-1765328215)
@@ -239,47 +240,3 @@ class CreateLiveImageThread(WorkerThread):
         dir, filename = os.path.split(iso_path)
         iso = IsoWrapper()
         run("cd %s && %s" % (pipes.quote(dir), iso.get_manifest_cmd(filename)))
-
-
-def get_ks_in(compose, arch, variant):
-    data = get_arch_variant_data(compose.conf, "live_images", arch, variant)
-    if not data:
-        return
-    scm_dict = data[0]["kickstart"]
-
-    if isinstance(scm_dict, dict):
-        file_name = os.path.basename(os.path.basename(scm_dict["file"]))
-        if scm_dict["scm"] == "file":
-            scm_dict["file"] = os.path.join(compose.config_dir, os.path.basename(scm_dict["file"]))
-    else:
-        file_name = os.path.basename(os.path.basename(scm_dict))
-        scm_dict = os.path.join(compose.config_dir, os.path.basename(scm_dict))
-
-    tmp_dir = tempfile.mkdtemp(prefix="ks_in_")
-    get_file_from_scm(scm_dict, tmp_dir, logger=compose._logger)
-    ks_in = os.path.join(compose.paths.work.topdir(arch), "liveimage-%s.%s.ks.in" % (variant.uid, arch))
-    shutil.copy2(os.path.join(tmp_dir, file_name), ks_in)
-    shutil.rmtree(tmp_dir)
-    return ks_in
-
-
-def tweak_ks(compose, arch, variant, ks_in):
-    if variant.environments:
-        # get groups from default environment (with lowest display_order)
-        envs = copy.deepcopy(variant.environments)
-        envs.sort(lambda x, y: cmp(x["display_order"], y["display_order"]))
-        env = envs[0]
-        groups = sorted(env["groups"])
-    else:
-        # no environments -> get default groups
-        groups = []
-        for i in variant.groups:
-            if i["default"]:
-                groups.append(i["name"])
-        groups.sort()
-
-    ks_file = os.path.join(compose.paths.work.topdir(arch), "liveimage-%s.%s.ks" % (variant.uid, arch))
-    contents = open(ks_in, "r").read()
-    contents = contents.replace("__GROUPS__", "\n".join(["@%s" % i for i in groups]))
-    open(ks_file, "w").write(contents)
-    return ks_file

pungi/wrappers/kojiwrapper.py

@@ -162,7 +162,7 @@ class KojiWrapper(object):
 
         return cmd
 
-    def get_create_image_cmd(self, name, version, target, arch, ks_file, repos, image_type="live", image_format=None, release=None, wait=True, archive=False, specfile=None):
+    def get_create_image_cmd(self, name, version, target, arch, ks_file, repos, image_type="live", image_format=None, release=None, wait=True, archive=False, specfile=None, ksurl=None):
         # Usage: koji spin-livecd [options] <name> <version> <target> <arch> <kickstart-file>
         # Usage: koji spin-appliance [options] <name> <version> <target> <arch> <kickstart-file>
         # Examples:
@@ -194,6 +194,9 @@ class KojiWrapper(object):
         if specfile:
             cmd.append("--specfile=%s" % specfile)
 
+        if ksurl:
+            cmd.append("--ksurl=%s" % ksurl)
+
         if isinstance(repos, list):
             for repo in repos:
                 cmd.append("--repo=%s" % repo)

tests/test_koji_wrapper.py

@@ -297,12 +297,13 @@ class LiveImageKojiWrapperTest(KojiWrapperBaseTestCase):
     def test_get_create_image_cmd_full(self):
         cmd = self.koji.get_create_image_cmd('my_name', '1.0', 'f24-candidate',
                                              'x86_64', '/path/to/ks', ['/repo/1', '/repo/2'],
-                                             release='1', wait=False, archive=True, specfile='foo.spec')
+                                             release='1', wait=False, archive=True, specfile='foo.spec',
+                                             ksurl='https://git.example.com/')
         self.assertEqual(cmd[0:2], ['koji', 'spin-livecd'])
-        self.assertItemsEqual(cmd[2:8],
+        self.assertEqual(cmd[-5:], ['my_name', '1.0', 'f24-candidate', 'x86_64', '/path/to/ks'])
+        self.assertItemsEqual(cmd[2:-5],
                               ['--noprogress', '--nowait', '--repo=/repo/1', '--repo=/repo/2',
-                               '--release=1', '--specfile=foo.spec'])
+                               '--release=1', '--specfile=foo.spec', '--ksurl=https://git.example.com/'])
-        self.assertEqual(cmd[8:], ['my_name', '1.0', 'f24-candidate', 'x86_64', '/path/to/ks'])
 
     def test_spin_livecd_with_format(self):
         with self.assertRaises(ValueError):

tests/test_liveimagesphase.py

@@ -61,13 +61,12 @@ class _DummyCompose(object):
 class TestLiveImagesPhase(unittest.TestCase):
 
     @mock.patch('pungi.phases.live_images.ThreadPool')
-    @mock.patch('pungi.phases.live_images.get_ks_in')
+    def test_live_image_build(self, ThreadPool):
-    @mock.patch('pungi.phases.live_images.tweak_ks')
-    def test_live_image_build(self, tweak_ks, get_ks_in, ThreadPool):
         compose = _DummyCompose({
             'live_images': [
                 ('^Client$', {
                     'amd64': {
+                        'kickstart': 'test.ks',
                         'additional_repos': ['http://example.com/repo/'],
                         'repos_from': ['Everything'],
                     }
@@ -75,10 +74,6 @@ class TestLiveImagesPhase(unittest.TestCase):
             ],
         })
 
-        get_ks_in.side_effect = (lambda compose, arch, variant:
-                                 None if variant.uid != 'Client' or arch != 'amd64' else '/path/to/ks_in')
-        tweak_ks.return_value = '/path/to/ks_file'
-
         phase = LiveImagesPhase(compose)
 
         phase.run()
@@ -88,7 +83,7 @@ class TestLiveImagesPhase(unittest.TestCase):
         self.maxDiff = None
         self.assertItemsEqual(phase.pool.queue_put.mock_calls,
                               [mock.call((compose,
-                                          {'ks_file': '/path/to/ks_file',
+                                          {'ks_file': 'test.ks',
                                            'build_arch': 'amd64',
                                            'wrapped_rpms_path': '/iso_dir/amd64/Client',
                                            'scratch': False,
@@ -100,18 +95,20 @@ class TestLiveImagesPhase(unittest.TestCase):
                                            'iso_path': '/iso_dir/amd64/Client/image-name',
                                            'version': None,
                                            'specfile': None,
-                                           'type': 'live'},
+                                           'type': 'live',
+                                           'ksurl': None},
                                           compose.variants['Client'],
                                           'amd64'))])
 
     @mock.patch('pungi.phases.live_images.ThreadPool')
-    @mock.patch('pungi.phases.live_images.get_ks_in')
+    @mock.patch('pungi.phases.live_images.resolve_git_url')
-    @mock.patch('pungi.phases.live_images.tweak_ks')
+    def test_spin_appliance(self, resolve_git_url, ThreadPool):
-    def test_spin_appliance(self, tweak_ks, get_ks_in, ThreadPool):
         compose = _DummyCompose({
             'live_images': [
                 ('^Client$', {
                     'amd64': {
+                        'kickstart': 'test.ks',
+                        'ksurl': 'https://git.example.com/kickstarts.git?#HEAD',
                         'additional_repos': ['http://example.com/repo/'],
                         'repos_from': ['Everything'],
                         'type': 'appliance',
@@ -120,9 +117,7 @@ class TestLiveImagesPhase(unittest.TestCase):
             ],
         })
 
-        get_ks_in.side_effect = (lambda compose, arch, variant:
+        resolve_git_url.return_value = 'https://git.example.com/kickstarts.git?#CAFEBABE'
-                                 None if variant.uid != 'Client' or arch != 'amd64' else '/path/to/ks_in')
-        tweak_ks.return_value = '/path/to/ks_file'
 
         phase = LiveImagesPhase(compose)
 
@@ -133,7 +128,7 @@ class TestLiveImagesPhase(unittest.TestCase):
         self.maxDiff = None
         self.assertItemsEqual(phase.pool.queue_put.mock_calls,
                               [mock.call((compose,
-                                          {'ks_file': '/path/to/ks_file',
+                                          {'ks_file': 'test.ks',
                                            'build_arch': 'amd64',
                                            'wrapped_rpms_path': '/iso_dir/amd64/Client',
                                            'scratch': False,
@@ -145,9 +140,12 @@ class TestLiveImagesPhase(unittest.TestCase):
                                            'iso_path': '/iso_dir/amd64/Client/image-name',
                                            'version': None,
                                            'specfile': None,
-                                           'type': 'appliance'},
+                                           'type': 'appliance',
+                                           'ksurl': 'https://git.example.com/kickstarts.git?#CAFEBABE'},
                                           compose.variants['Client'],
                                           'amd64'))])
+        self.assertEqual(resolve_git_url.mock_calls,
+                         [mock.call('https://git.example.com/kickstarts.git?#HEAD')])
 
 
 class TestCreateLiveImageThread(unittest.TestCase):
@@ -172,6 +170,7 @@ class TestCreateLiveImageThread(unittest.TestCase):
             'version': None,
             'specfile': None,
             'type': 'live',
+            'ksurl': 'https://git.example.com/kickstarts.git?#CAFEBABE',
         }
 
         koji_wrapper = KojiWrapper.return_value
@@ -207,7 +206,8 @@ class TestCreateLiveImageThread(unittest.TestCase):
                                     image_type='live',
                                     archive=False,
                                     specfile=None,
-                                    wait=True)])
+                                    wait=True,
+                                    ksurl='https://git.example.com/kickstarts.git?#CAFEBABE')])
 
     @mock.patch('shutil.copy2')
     @mock.patch('pungi.phases.live_images.run')
@@ -229,6 +229,7 @@ class TestCreateLiveImageThread(unittest.TestCase):
             'version': None,
             'specfile': None,
             'type': 'appliance',
+            'ksurl': None,
         }
 
         koji_wrapper = KojiWrapper.return_value
@@ -264,7 +265,8 @@ class TestCreateLiveImageThread(unittest.TestCase):
                                     image_type='appliance',
                                     archive=False,
                                     specfile=None,
-                                    wait=True)])
+                                    wait=True,
+                                    ksurl=None)])
 
     @mock.patch('shutil.copy2')
     @mock.patch('pungi.phases.live_images.run')
@@ -287,7 +289,8 @@ class TestCreateLiveImageThread(unittest.TestCase):
             'name': None,
             'iso_path': '/iso_dir/amd64/Client/image-name',
             'version': None,
-            'specfile': None
+            'specfile': None,
+            'ksurl': None,
         }
 
         koji_wrapper = KojiWrapper.return_value
@@ -323,7 +326,8 @@ class TestCreateLiveImageThread(unittest.TestCase):
             'name': None,
             'iso_path': '/iso_dir/amd64/Client/image-name',
             'version': None,
-            'specfile': None
+            'specfile': None,
+            'ksurl': None,
         }
 
         def boom(*args, **kwargs):