From 1ad738e8799c1929134df105d37d5a042a739345 Mon Sep 17 00:00:00 2001
From: Jan Kaluža <jkaluza@redhat.com>
Date: Jan 29 2019 06:36:58 +0000
Subject: Merge #342 `Do not wait for the result of Pulp compose in busy loop.`


---

diff --git a/freshmaker/handlers/koji/rebuild_images_on_odcs_compose_done.py b/freshmaker/handlers/koji/rebuild_images_on_odcs_compose_done.py
index af8fffe..12b98ea 100644
--- a/freshmaker/handlers/koji/rebuild_images_on_odcs_compose_done.py
+++ b/freshmaker/handlers/koji/rebuild_images_on_odcs_compose_done.py
@@ -48,13 +48,22 @@ class RebuildImagesOnODCSComposeDone(ContainerBuildHandler):
             self.force_dry_run()
 
         query = db.session.query(ArtifactBuild).join('composes')
-        first_batch_builds = query.filter(
-            ArtifactBuild.dep_on == None,  # noqa
+        # Get all the builds waiting for this compose in PLANNED state ...
+        builds_ready_to_rebuild = query.filter(
             ArtifactBuild.state == ArtifactBuildState.PLANNED.value,
             Compose.odcs_compose_id == event.compose['id'])
-        if self.dry_run:
-            builds_ready_to_rebuild = first_batch_builds
-        else:
+        # ... and depending on DONE parent image or parent image which is
+        # not planned to be built in this Event (dep_on == None).
+        builds_ready_to_rebuild = [
+            b for b in builds_ready_to_rebuild if
+            b.dep_on is None or b.dep_on.state == ArtifactBuildState.DONE.value
+        ]
+
+        if not self.dry_run:
+            # In non-dry-run mode, check that all the composes are ready.
+            # In dry-run mode, the composes are fake, so they are always ready.
             builds_ready_to_rebuild = six.moves.filter(
-                lambda build: build.composes_ready, first_batch_builds)
+                lambda build: build.composes_ready, builds_ready_to_rebuild)
+
+        # Start the rebuild.
         self.start_to_build_images(builds_ready_to_rebuild)
diff --git a/freshmaker/odcsclient.py b/freshmaker/odcsclient.py
index 7486afa..8d6380c 100644
--- a/freshmaker/odcsclient.py
+++ b/freshmaker/odcsclient.py
@@ -44,7 +44,7 @@ from freshmaker.errata import Errata
 from freshmaker.kojiservice import koji_service
 from freshmaker.consumer import work_queue_put
 from freshmaker.types import ArtifactBuildState
-from freshmaker.utils import krb_context, retry
+from freshmaker.utils import krb_context
 from freshmaker.events import ODCSComposeStateChangeEvent
 
 
@@ -298,32 +298,6 @@ class FreshmakerODCSClient(object):
             with krb_context():
                 new_compose = odcs.new_compose(
                     ' '.join(content_sets), 'pulp')
-
-                # Pulp composes in ODCS takes just few seconds, because ODCS
-                # only generates the .repo file after single query to Pulp.
-                # TODO: Freshmaker is currently not designed to handle
-                # multiple ODCS composes per rebuild Event and since these
-                # composes are done in no-time normally, it is OK here to
-                # block. It would still be nice to redesign that part of
-                # Freshmaker to do things "right".
-                # This is tracked here: https://pagure.io/freshmaker/issue/114
-                @retry(timeout=60 * 30, interval=2)
-                def wait_for_compose(compose_id):
-                    ret = odcs.get_compose(compose_id)
-                    if ret["state_name"] == "done":
-                        return True
-                    elif ret["state_name"] == "failed":
-                        return False
-                    self.handler.log_info(
-                        "Waiting for Pulp compose to finish: %r", ret)
-                    raise Exception("ODCS compose not finished.")
-
-                done = wait_for_compose(new_compose["id"])
-                if not done:
-                    build.transition(
-                        ArtifactBuildState.FAILED.value, "Cannot generate "
-                        "ODCS PULP compose %s for content_sets %r"
-                        % (str(new_compose["id"]), content_sets))
         else:
             new_compose = self._fake_odcs_new_compose(
                 content_sets, 'pulp')
diff --git a/tests/handlers/koji/test_rebuild_images_on_odcs_compose_done.py b/tests/handlers/koji/test_rebuild_images_on_odcs_compose_done.py
index 8904efc..2c41f48 100644
--- a/tests/handlers/koji/test_rebuild_images_on_odcs_compose_done.py
+++ b/tests/handlers/koji/test_rebuild_images_on_odcs_compose_done.py
@@ -119,3 +119,21 @@ class TestRebuildImagesOnODCSComposeDone(helpers.ModelsTestCase):
         args, kwargs = start_to_build_images.call_args
         passed_builds = sorted(args[0], key=lambda build: build.id)
         self.assertEqual([self.build_1, self.build_3], passed_builds)
+
+    @patch('freshmaker.models.ArtifactBuild.composes_ready',
+           new_callable=PropertyMock)
+    @patch('freshmaker.handlers.ContainerBuildHandler.start_to_build_images')
+    def test_start_to_build_parent_image_done(self, start_to_build_images, composes_ready):
+        composes_ready.return_value = True
+        self.build_1.state = ArtifactBuildState.DONE.value
+
+        event = ODCSComposeStateChangeEvent(
+            'msg-id', {'id': self.compose_1.id, 'state': 'done'}
+        )
+
+        handler = RebuildImagesOnODCSComposeDone()
+        handler.handle(event)
+
+        args, kwargs = start_to_build_images.call_args
+        passed_builds = sorted(args[0], key=lambda build: build.id)
+        self.assertEqual([self.build_3, self.build_2], passed_builds)