From 0007edcfaaff9833d4c51bc28500f58e161f7b84 Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Jul 20 2018 10:48:43 +0000
Subject: Pass ODCS composes to Koji using 'compose_ids' instead of 'yum_repourls' when possible.


---

diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py
index 5c2f5d7..b5381ad 100644
--- a/freshmaker/handlers/__init__.py
+++ b/freshmaker/handlers/__init__.py
@@ -392,7 +392,7 @@ class ContainerBuildHandler(BaseHandler):
     def build_container(self, scm_url, branch, target,
                         repo_urls=None, isolated=False,
                         release=None, koji_parent_build=None,
-                        arch_override=None):
+                        arch_override=None, compose_ids=None):
         """
         Build a container in Koji.
 
@@ -408,8 +408,9 @@ class ContainerBuildHandler(BaseHandler):
                 profile=conf.koji_profile, logger=log,
                 dry_run=self.dry_run) as service:
             log.info('Building container from source: %s, '
-                     'release=%r, parent=%r, target=%r, arch=%r',
-                     scm_url, release, koji_parent_build, target, arch_override)
+                     'release=%r, parent=%r, target=%r, arch=%r, compose_ids=%r',
+                     scm_url, release, koji_parent_build, target, arch_override,
+                     compose_ids)
 
             return service.build_container(scm_url,
                                            branch,
@@ -465,10 +466,24 @@ class ContainerBuildHandler(BaseHandler):
 
         release = parse_NVR(build.rebuilt_nvr)["release"]
 
+        # OSBS cannot handle both repo_urls and compose_ids in the same time.
+        # We use repo_urls only in special cases to build base images. In this
+        # cases, we want to convert compose_ids to repository URLs. Otherwise,
+        # just pass compose_ids to OSBS via Koji.
+        compose_ids = []
+        if repo_urls:
+            repo_urls += [self.odcs_get_compose(
+                rel.compose.odcs_compose_id)['result_repofile']
+                for rel in build.composes]
+        else:
+            compose_ids = []
+            for relation in build.composes:
+                compose_ids.append(relation.compose.odcs_compose_id)
+
         return self.build_container(
             scm_url, branch, target, repo_urls=repo_urls,
             isolated=True, release=release, koji_parent_build=parent,
-            arch_override=arches)
+            arch_override=arches, compose_ids=compose_ids)
 
     def odcs_get_compose(self, compose_id):
         """
@@ -499,16 +514,12 @@ class ContainerBuildHandler(BaseHandler):
         """
         repo_urls = []
 
-        # At first include image_extra_repos if any for this name-version.
+        # Include image_extra_repos if any for this name-version.
         if build.original_nvr:
             parsed_nvr = parse_NVR(build.original_nvr)
             name_version = "%s-%s" % (parsed_nvr["name"], parsed_nvr["version"])
             if name_version in conf.image_extra_repo:
                 repo_urls.append(conf.image_extra_repo[name_version])
-
-        repo_urls += [self.odcs_get_compose(rel.compose.odcs_compose_id)['result_repofile']
-                      for rel in build.composes]
-
         return repo_urls
 
     def start_to_build_images(self, builds):
diff --git a/freshmaker/kojiservice.py b/freshmaker/kojiservice.py
index 2d17d01..c1dd650 100644
--- a/freshmaker/kojiservice.py
+++ b/freshmaker/kojiservice.py
@@ -140,7 +140,7 @@ class KojiService(object):
     def build_container(self, source_url, branch, target,
                         scratch=None, repo_urls=None, isolated=False,
                         release=None, koji_parent_build=None,
-                        arch_override=None):
+                        arch_override=None, compose_ids=None):
         """Build container by buildContainer"""
 
         build_target = target
@@ -151,6 +151,8 @@ class KojiService(object):
 
         if repo_urls:
             build_opts['yum_repourls'] = repo_urls
+        if compose_ids:
+            build_opts['compose_ids'] = compose_ids
         if isolated:
             build_opts['isolated'] = True
         if koji_parent_build:
diff --git a/tests/test_handler.py b/tests/test_handler.py
index a0a22a5..22b4959 100644
--- a/tests/test_handler.py
+++ b/tests/test_handler.py
@@ -23,6 +23,7 @@
 # Written by Chenxiong Qi <cqi@redhat.com>
 
 from mock import patch
+import json
 
 import freshmaker
 
@@ -111,12 +112,24 @@ class TestGetRepoURLs(helpers.ModelsTestCase):
             state=EventState.BUILDING,
             released=False)
 
+        build_args = {}
+        build_args["repository"] = "repo"
+        build_args["commit"] = "hash"
+        build_args["parent"] = None
+        build_args["target"] = "target"
+        build_args["branch"] = "branch"
+        build_args["arches"] = "x86_64"
+
         self.build_1 = ArtifactBuild.create(
             db.session, self.event, 'build-1', ArtifactType.IMAGE,
             state=ArtifactBuildState.PLANNED)
+        self.build_1.rebuilt_nvr = "foo-1-2"
+        self.build_1.build_args = json.dumps(build_args)
         self.build_2 = ArtifactBuild.create(
             db.session, self.event, 'build-2', ArtifactType.IMAGE,
             state=ArtifactBuildState.PLANNED)
+        self.build_2.rebuilt_nvr = "foo-2-2"
+        self.build_2.build_args = json.dumps(build_args)
 
         db.session.commit()
 
@@ -154,17 +167,10 @@ class TestGetRepoURLs(helpers.ModelsTestCase):
         repos = handler.get_repo_urls(self.build_2)
         self.assertEqual(repos, [])
 
-    def test_get_repo_urls_both_pulp_and_main_compose(self):
+    def test_get_repo_urls_only_odcs_composes(self):
         handler = MyHandler()
         repos = handler.get_repo_urls(self.build_1)
-        self.assertEqual(
-            [
-                'http://localhost/5.repo',
-                'http://localhost/6.repo',
-                'http://localhost/7.repo',
-                'http://localhost/8.repo',
-            ],
-            sorted(repos))
+        self.assertEqual(repos, [])
 
     @patch.object(freshmaker.conf, 'image_extra_repo', new={
         'build-3': "http://localhost/test.repo"
@@ -178,6 +184,31 @@ class TestGetRepoURLs(helpers.ModelsTestCase):
         repos = handler.get_repo_urls(build_3)
         self.assertEqual(repos, ["http://localhost/test.repo"])
 
+    @patch("freshmaker.handlers.ContainerBuildHandler.build_container")
+    def test_build_image_artifact_build_only_odcs_composes(
+            self, build_container):
+        handler = MyHandler()
+        handler.build_image_artifact_build(self.build_1)
+        build_container.assert_called_once_with(
+            'git://pkgs.fedoraproject.org/repo#hash', 'branch', 'target',
+            arch_override='x86_64', compose_ids=[5, 6, 7, 8], isolated=True,
+            koji_parent_build=None, release='2', repo_urls=[])
+
+    @patch("freshmaker.handlers.ContainerBuildHandler.build_container")
+    def test_build_image_artifact_build_repo_urls(
+            self, build_container):
+        handler = MyHandler()
+        handler.build_image_artifact_build(self.build_1, ["http://localhost/x.repo"])
+
+        repo_urls = [
+            'http://localhost/x.repo', 'http://localhost/5.repo',
+            'http://localhost/6.repo', 'http://localhost/7.repo',
+            'http://localhost/8.repo']
+        build_container.assert_called_once_with(
+            'git://pkgs.fedoraproject.org/repo#hash', 'branch', 'target',
+            arch_override='x86_64', compose_ids=[], isolated=True,
+            koji_parent_build=None, release='2', repo_urls=repo_urls)
+
 
 class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase):
     """Test BaseHandler.allow_build"""

