From 2c7d5f2fca70bb5b37a6b294560c4fce8fc774fe Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Sep 12 2017 11:05:35 +0000
Subject: Submit container builds to Koji when RPMs are signed in Errata advisory.


---

diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py
index e2b378c..d7fc469 100644
--- a/freshmaker/handlers/__init__.py
+++ b/freshmaker/handlers/__init__.py
@@ -83,8 +83,9 @@ class BaseHandler(object):
         mbs = MBS(conf)
         return mbs.build_module(name, branch, rev)
 
-    def build_container(self, name, branch, rev,
-                        namespace='container', repo_urls=None):
+    def build_container(self, scm_url, branch, target,
+                        repo_urls=None, isolated=False,
+                        release=None, koji_parent_build=None):
         """
         Build a container in Koji.
 
@@ -107,15 +108,15 @@ class BaseHandler(object):
                 log.error('Could not login server %s', service.server)
                 return None
 
-            build_source = "{}/{}/{}.git?#{}".format(
-                conf.git_base_url, namespace, name, rev)
+            log.debug('Building container from source: %s', scm_url)
 
-            log.debug('Building container from source: %s', build_source)
-
-            return service.build_container(build_source,
+            return service.build_container(scm_url,
                                            branch,
+                                           target,
                                            repo_urls=repo_urls,
-                                           namespace=namespace,
+                                           isolated=isolated,
+                                           release=release,
+                                           koji_parent_build=koji_parent_build,
                                            scratch=conf.koji_container_scratch_build)
 
     def record_build(self, event, name, artifact_type,
diff --git a/freshmaker/handlers/bodhi/update_complete_stable.py b/freshmaker/handlers/bodhi/update_complete_stable.py
index f34f4f0..ad75080 100644
--- a/freshmaker/handlers/bodhi/update_complete_stable.py
+++ b/freshmaker/handlers/bodhi/update_complete_stable.py
@@ -60,7 +60,13 @@ class BodhiUpdateCompleteStableHandler(BaseHandler):
                 repo_url = '{}/{}/{}'.format(conf.git_base_url, 'container', name)
                 rev = utils.get_commit_hash(repo_url, branch=branch, logger=log)
 
-                task_id = self.build_container(name, branch, rev)
+                scm_url = "{}/{}/{}.git?#{}".format(
+                    conf.git_base_url, 'container', name, rev)
+
+                build_target = '{}-container-candidate'.format(
+                    'rawhide' if branch == 'master' else branch)
+
+                task_id = self.build_container(scm_url, branch, build_target)
                 if task_id is not None:
                     self.record_build(event, container['name'], ArtifactType.IMAGE, task_id)
             except:
diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py
index 8ee38a0..f763e58 100644
--- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py
+++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py
@@ -121,10 +121,62 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler):
         for url in repo_urls:
             log.info("   - %s", url)
 
-        # TODO: Rebuild first batch.
+        # Build first batch of images.
+        self._build_first_batch(db_event)
 
         return []
 
+    def _build_first_batch(self, db_event):
+        """
+        Rebuilds all the parents images - images in the first batch which don't
+        depend on other images.
+        """
+
+        rebuild_event = Event.get(db.session, db_event.message_id)
+        odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos,
+                    verify_ssl=conf.odcs_verify_ssl)
+        compose = odcs.get_compose(rebuild_event.compose_id)
+        # TODO: Add other repofiles from "extra events"
+        repo_urls = [compose["result_repofile"]]
+
+        for build in rebuild_event.builds:
+            if build.dep_on:
+                continue
+
+            if build.state != ArtifactBuildState.PLANNED.value:
+                log.error("Trying to build first batch of container images, "
+                          "but build %r is not in PLANNED state", build)
+                continue
+
+            if not build.build_args:
+                log.error("Cannot rebuild container image %r, build_args not "
+                          "defined", build)
+                continue
+
+            args = json.loads(build.build_args)
+
+            if not args["parent"]:
+                log.error("Base image %r should be rebuild, but this is not "
+                          "supported yet", build)
+                continue
+
+            parent = args["parent"]
+            scm_url = "%s/%s#%s" % (conf.git_base_url, args["repository"],
+                                    args["commit"])
+            release = build.name.split("-")[-1] + "." + str(int(time.time()))
+            # According to Luiz from OSBS team, it is OK to use "unknown" if
+            # we don't know the branch name. TODO: Get the branch name from
+            # Koji in lightblue.py.
+            branch = "unknown"
+            target = args["target"]
+
+            build.build_id = self.build_container(
+                scm_url, branch, target, repo_urls=repo_urls, isolated=True,
+                release=release, koji_parent_build=parent)
+            build.state = ArtifactBuildState.BUILD.value
+            db.session.add(build)
+            db.session.commit()
+
     def _prepare_yum_repo(self, db_event):
         """
         Prepare a yum repo for rebuild
@@ -328,6 +380,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler):
                 build_args["repository"] = image["repository"]
                 build_args["commit"] = image["commit"]
                 build_args["parent"] = parent_name
+                build_args["target"] = image["target"]
                 build.build_args = json.dumps(build_args)
                 db.session.commit()
 
diff --git a/freshmaker/handlers/git/dockerfile_change.py b/freshmaker/handlers/git/dockerfile_change.py
index 45f7e88..7495353 100644
--- a/freshmaker/handlers/git/dockerfile_change.py
+++ b/freshmaker/handlers/git/dockerfile_change.py
@@ -21,7 +21,7 @@
 #
 # Written by Chenxiong Qi <cqi@redhat.com>
 
-from freshmaker import log
+from freshmaker import log, conf
 from freshmaker.types import ArtifactType
 from freshmaker.handlers import BaseHandler
 from freshmaker.events import GitDockerfileChangeEvent
@@ -45,8 +45,16 @@ class GitDockerfileChangeHandler(BaseHandler):
             return []
 
         try:
-            task_id = self.build_container(event.container, event.branch, event.rev)
+            name = event.container
+            branch = event.branch
+            rev = event.rev
+            scm_url = "{}/{}/{}.git?#{}".format(
+                conf.git_base_url, 'container', name, rev)
 
+            build_target = '{}-container-candidate'.format(
+                'rawhide' if branch == 'master' else branch)
+
+            task_id = self.build_container(scm_url, branch, build_target)
             if task_id is not None:
                 self.record_build(event, event.container, ArtifactType.IMAGE, task_id)
 
diff --git a/freshmaker/kojiservice.py b/freshmaker/kojiservice.py
index 1f618c3..e1cc987 100644
--- a/freshmaker/kojiservice.py
+++ b/freshmaker/kojiservice.py
@@ -71,20 +71,25 @@ class KojiService(object):
     def logout(self):
         self.session.logout()
 
-    def build_container(self, source_url, branch,
-                        namespace=None, scratch=None, repo_urls=None):
+    def build_container(self, source_url, branch, target,
+                        scratch=None, repo_urls=None, isolated=False,
+                        release=None, koji_parent_build=None):
         """Build container by buildContainer"""
 
-        build_target = '{}-{}-candidate'.format(
-            'rawhide' if branch == 'master' else branch,
-            'container' if namespace is None else namespace)
-
+        build_target = target
         build_opts = {
             'scratch': False if scratch is None else True,
             'git_branch': branch,
         }
+
         if repo_urls:
             build_opts['yum_repourls'] = repo_urls
+        if isolated:
+            build_opts['isolated'] = True
+        if koji_parent_build:
+            build_opts['koji_parent_build'] = koji_parent_build
+        if release:
+            build_opts['release'] = release
 
         if self._logger:
             self._logger.debug('Build from target: %s', build_target)
@@ -106,8 +111,13 @@ class KojiService(object):
                                      arches=arches)
 
     def get_build(self, build_nvr):
+        log.info("get_build %r", build_nvr)
         return self.session.getBuild(build_nvr)
 
+    def get_task_request(self, task_id):
+        log.info("get_build %r", task_id)
+        return self.session.getTaskRequest(task_id)
+
 
 @contextlib.contextmanager
 def koji_service(profile=None, logger=None):
diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py
index 35a6e42..5981c15 100644
--- a/freshmaker/lightblue.py
+++ b/freshmaker/lightblue.py
@@ -123,13 +123,6 @@ class ContainerImage(dict):
         :param str srpm_name: Name of the package because of which the Docker
                               image is rebuilt.
         """
-        dockerfile_url = None
-        if "parsed_data" in self and "files" in self["parsed_data"]:
-            for f in self["parsed_data"]["files"]:
-                if f['key'] == 'buildfile':
-                    dockerfile_url = f['content_url']
-                    break
-
         srpm_nevra = None
         if "parsed_data" in self and "rpm_manifest" in self["parsed_data"]:
             for rpm in self["parsed_data"]["rpm_manifest"]:
@@ -139,35 +132,31 @@ class ContainerImage(dict):
 
         reponame = None
         commit = None
-        if dockerfile_url:
-            dockerfile, _, commit = dockerfile_url.partition("?id=")
-            _, _, reponame = dockerfile.partition("/cgit/")
-            reponame = reponame.replace("/plain/Dockerfile", "")
-
-        # If we cannot find reponame and commit in the Lightblue data,
-        # fallback to Koji to get a Koji build based on the ["brew"]["build"].
-        if not reponame or not commit:
-            nvr = self["brew"]["build"]
-            if nvr in ContainerImage.KOJI_BUILDS_CACHE:
-                reponame, commit = ContainerImage.KOJI_BUILDS_CACHE[nvr]
-            else:
-                with koji_service(conf.koji_profile, log) as session:
-                    build = session.get_build(nvr)
-                    if build:
-                        source = build["source"]
-                        if source is None:
-                            brew_task = session.getTaskRequest(
-                                build['task_id'])
-                            source = brew_task[0]
-
-                        m = re.match(r".*/(?P<namespace>.*)/(?P<container>.*)#(?P<commit>.*)", source)
+        target = None
+
+        # Find the repository name, commit id and koji target form the Koji
+        # build.
+        nvr = self["brew"]["build"]
+        if nvr in ContainerImage.KOJI_BUILDS_CACHE:
+            reponame, commit, target = ContainerImage.KOJI_BUILDS_CACHE[nvr]
+        else:
+            with koji_service(conf.koji_profile, log) as session:
+                build = session.get_build(nvr)
+                if build:
+                    brew_task = session.get_task_request(
+                        build['task_id'])
+                    source = brew_task[0]
+                    target = brew_task[1]
+
+                    m = re.match(r".*/(?P<namespace>.*)/(?P<container>.*)#(?P<commit>.*)", source)
+                    if m:
                         namespace = m.group("namespace")
                         container = m.group("container")
                         reponame = namespace + "/" + container
                         commit = m.group("commit")
-            ContainerImage.KOJI_BUILDS_CACHE[nvr] = (reponame, commit)
+        ContainerImage.KOJI_BUILDS_CACHE[nvr] = (reponame, commit, target)
 
-        data = {"repository": reponame, "commit": commit,
+        data = {"repository": reponame, "commit": commit, "target": target,
                 "srpm_nevra": srpm_nevra}
         self.update(data)
 
diff --git a/tests/test_bodhi_update_complete_stable_handler.py b/tests/test_bodhi_update_complete_stable_handler.py
index 70b899d..18080c4 100644
--- a/tests/test_bodhi_update_complete_stable_handler.py
+++ b/tests/test_bodhi_update_complete_stable_handler.py
@@ -143,9 +143,14 @@ class BodhiUpdateCompleteStableHandlerTest(helpers.FreshmakerTestCase):
         self.assertTrue(handler.can_handle(event))
         handler.handle(event)
 
-        self.assertEqual(handler.build_container.call_args_list,
-                         [mock.call('testimage1', 'f25', 'c123'),
-                          mock.call('testimage2', 'f25', 'c456')])
+        self.assertEqual(
+            handler.build_container.call_args_list,
+            [mock.call(
+                'git://pkgs.fedoraproject.org/container/testimage1.git?#c123',
+                'f25', 'f25-container-candidate'),
+             mock.call(
+                'git://pkgs.fedoraproject.org/container/testimage2.git?#c456',
+                'f25', 'f25-container-candidate')])
 
         events = models.Event.query.all()
         self.assertEqual(len(events), 1)
diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py
index 7fac6ab..adae8b5 100644
--- a/tests/test_errata_advisory_state_changed.py
+++ b/tests/test_errata_advisory_state_changed.py
@@ -203,7 +203,7 @@ class TestBatches(unittest.TestCase):
         if parent:
             parent = {"brew": {"build": parent}}
         return {'brew': {'build': build}, 'repository': build + '_repo',
-                'commit': build + '_123', 'parent': parent}
+                'commit': build + '_123', 'parent': parent, "target": "t1"}
 
     def test_batches_records(self):
         """
@@ -473,3 +473,81 @@ class TestFindEventsToInclude(unittest.TestCase):
 
         self.assertEqual(len(events), 1)
         self.assertEqual(events[0].search_key, "old_event_foo")
+
+
+class AnyStringWith(str):
+    def __eq__(self, other):
+        return self in other
+
+
+class TestBuildFirstBatch(unittest.TestCase):
+    """Test ErrataAdvisoryRPMsSignedHandler._build_first_batch"""
+
+    def setUp(self):
+        db.session.remove()
+        db.drop_all()
+        db.create_all()
+        db.session.commit()
+
+        build_args = '{"parent": "nvr", "repository": "repo", \
+            "target": "target", "commit": "hash"}'
+
+        self.db_event = Event.get_or_create(
+            db.session, "msg1", "current_event", ErrataAdvisoryRPMsSignedEvent,
+            released=False)
+        self.db_event.compose_id = 3
+        p1 = ArtifactBuild.create(db.session, self.db_event, "parent1-1-4",
+                                  "image",
+                                  state=ArtifactBuildState.PLANNED.value)
+        p1.build_args = build_args
+        b = ArtifactBuild.create(db.session, self.db_event, "parent1_child1", "image",
+                                 state=ArtifactBuildState.PLANNED.value,
+                                 dep_on=p1)
+        b.build_args = build_args
+        b = ArtifactBuild.create(db.session, self.db_event, "parent3", "image",
+                                 state=ArtifactBuildState.BUILD.value)
+        b.build_args = build_args
+        db.session.commit()
+
+    def tearDown(self):
+        db.session.remove()
+        db.drop_all()
+        db.session.commit()
+
+    @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS')
+    @patch('koji.ClientSession')
+    @patch('freshmaker.handlers.krbContext')
+    def test_build_first_batch(self, krb, ClientSession, ODCS):
+        """
+        Tests that only PLANNED images without a parent are submitted to
+        build system.
+        """
+        ODCS.return_value.get_compose.return_value = {
+            "id": 3,
+            "result_repo": "http://localhost/composes/latest-odcs-3-1/compose/Temporary",
+            "result_repofile": "http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo",
+            "source": "f26",
+            "source_type": 1,
+            "state": 2,
+            "state_name": "done",
+        }
+        mock_session = ClientSession.return_value
+        mock_session.buildContainer.return_value = 123
+
+        handler = ErrataAdvisoryRPMsSignedHandler()
+        handler._build_first_batch(self.db_event)
+
+        mock_session.buildContainer.assert_called_once_with(
+            'git://pkgs.fedoraproject.org/repo#hash',
+            'target',
+            {'scratch': True, 'isolated': True, 'koji_parent_build': u'nvr',
+             'git_branch': 'unknown', 'release': AnyStringWith('4.'),
+             'yum_repourls': [
+                 'http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo']})
+
+        db.session.refresh(self.db_event)
+        for build in self.db_event.builds:
+            if build.name == "parent1-1-4":
+                self.assertEqual(build.build_id, 123)
+            else:
+                self.assertEqual(build.build_id, None)
diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py
index a7a3d57..ea3731a 100644
--- a/tests/test_lightblue.py
+++ b/tests/test_lightblue.py
@@ -132,42 +132,9 @@ class TestContainerImageObject(unittest.TestCase):
         self.assertEqual('1233829', image['_id'])
         self.assertEqual('20151210T10:09:35.000-0500', image['brew']['completion_date'])
 
-    def test_resolve_commit(self):
-        image = ContainerImage.create({
-            '_id': '1233829',
-            'brew': {
-                'completion_date': u'20170421T04:27:51.000-0400',
-                'build': 'package-name-1-4-12.10',
-                'package': 'package-name-1'
-            },
-            'parsed_data': {
-                'files': [
-                    {
-                        'key': 'buildfile',
-                        'content_url': 'http://git.repo.com/cgit/rpms/repo-1/plain/Dockerfile?id=commit_hash1',
-                        'filename': u'Dockerfile'
-                    }
-                ],
-                'rpm_manifest': [
-                    {
-                        "srpm_name": "openssl",
-                        "srpm_nevra": "openssl-0:1.2.3-1.src"
-                    },
-                    {
-                        "srpm_name": "tespackage",
-                        "srpm_nevra": "testpackage-10:1.2.3-1.src"
-                    }
-                ]
-            }
-        })
-
-        image.resolve_commit("openssl")
-        self.assertEqual(image["repository"], "rpms/repo-1")
-        self.assertEqual(image["commit"], "commit_hash1")
-        self.assertEqual(image["srpm_nevra"], "openssl-0:1.2.3-1.src")
-
     @patch('freshmaker.kojiservice.KojiService.get_build')
-    def test_resolve_commit_koji_fallback(self, get_build):
+    @patch('freshmaker.kojiservice.KojiService.get_task_request')
+    def test_resolve_commit_koji_fallback(self, get_task_request, get_build):
         image = ContainerImage.create({
             '_id': '1233829',
             'brew': {
@@ -189,12 +156,14 @@ class TestContainerImageObject(unittest.TestCase):
             }
         })
 
-        get_build.return_value = {
-            "source": "git://example.com/rpms/repo-1#commit_hash1"}
+        get_build.return_value = {"task_id": 123456}
+        get_task_request.return_value = [
+            "git://example.com/rpms/repo-1#commit_hash1", "target1"]
 
         image.resolve_commit("openssl")
         self.assertEqual(image["repository"], "rpms/repo-1")
         self.assertEqual(image["commit"], "commit_hash1")
+        self.assertEqual(image["target"], "target1")
         self.assertEqual(image["srpm_nevra"], "openssl-0:1.2.3-1.src")
 
 
@@ -217,6 +186,9 @@ class TestContainerRepository(unittest.TestCase):
 class TestQueryEntityFromLightBlue(unittest.TestCase):
 
     def setUp(self):
+        # Clear the ContainerImage Koji cache.
+        ContainerImage.KOJI_BUILDS_CACHE = {}
+
         self.fake_server_url = 'lightblue.localhost'
         self.fake_cert_file = 'path/to/cert'
         self.fake_private_key = 'path/to/private-key'
@@ -269,7 +241,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
                     'files': [
                         {
                             'key': 'buildfile',
-                            'content_url': 'http://git.repo.com/cgit/ns/repo-2/plain/Dockerfile?id=commit_hash2',
+                            'content_url': 'http://git.repo.com/cgit/rpms/repo-2/plain/Dockerfile?id=commit_hash2',
                             'filename': 'Dockerfile'
                         },
                         {
@@ -291,10 +263,18 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
                 }
             },
         ]
+
         self.fake_container_images = [
             ContainerImage.create(data)
             for data in self.fake_images_with_parsed_data]
 
+        self.fake_koji_builds = [{"task_id": 123456}, {"task_id": 654321}]
+        self.fake_koji_task_requests = [
+            ["git://pkgs.devel.redhat.com/rpms/repo-1#commit_hash1",
+             "target1"],
+            ["git://pkgs.devel.redhat.com/rpms/repo-2#commit_hash2",
+             "target2"]]
+
     @patch('freshmaker.lightblue.requests.post')
     def test_find_container_images(self, post):
         post.return_value.status_code = http_client.OK
@@ -561,14 +541,18 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
 
     @patch('freshmaker.lightblue.LightBlue.find_container_repositories')
     @patch('freshmaker.lightblue.LightBlue.find_container_images')
+    @patch('freshmaker.kojiservice.KojiService.get_build')
+    @patch('freshmaker.kojiservice.KojiService.get_task_request')
     @patch('os.path.exists')
-    def test_images_with_content_set_packages(self, exists,
-                                              cont_images,
+    def test_images_with_content_set_packages(self, exists, koji_task_request,
+                                              koji_get_build, cont_images,
                                               cont_repos):
 
         exists.return_value = True
         cont_repos.return_value = self.fake_repositories_with_content_sets
         cont_images.return_value = self.fake_container_images
+        koji_task_request.side_effect = self.fake_koji_task_requests
+        koji_get_build.side_effect = self.fake_koji_builds
 
         lb = LightBlue(server_url=self.fake_server_url,
                        cert=self.fake_cert_file,
@@ -583,6 +567,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
                                  "repository": "rpms/repo-1",
                                  "commit": "commit_hash1",
                                  "srpm_nevra": "openssl-0:1.2.3-1.src",
+                                 "target": "target1",
                                  "brew": {
                                      "completion_date": u"20170421T04:27:51.000-0400",
                                      "build": "package-name-1-4-12.10",
@@ -609,9 +594,10 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
                                  }
                              },
                              {
-                                 "repository": "ns/repo-2",
+                                 "repository": "rpms/repo-2",
                                  "commit": "commit_hash2",
                                  "srpm_nevra": "openssl-1:1.2.3-1.src",
+                                 "target": "target2",
                                  "brew": {
                                      "completion_date": u"20170421T04:27:51.000-0400",
                                      "build": "package-name-2-4-12.10",
@@ -621,7 +607,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase):
                                      'files': [
                                          {
                                              'key': 'buildfile',
-                                             'content_url': 'http://git.repo.com/cgit/ns/repo-2/plain/Dockerfile?id=commit_hash2',
+                                             'content_url': 'http://git.repo.com/cgit/rpms/repo-2/plain/Dockerfile?id=commit_hash2',
                                              'filename': 'Dockerfile'
                                          },
                                          {

