From 0d306d4964ee53abbd809beefce222154378bf57 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Fri, 8 Mar 2024 13:55:48 -0800
Subject: [PATCH] Fix 'failable' handling for kiwibuild phase

The mechanisms here are a bit subtle and the kiwibuild phase
didn't quite get them right. The arg passed to `util.failable`
is supposed to be a boolean, but kiwibuild was passing it the
list of failable arches (which will always evaluate True).

How this is meant to work is that we only make *the Koji task
as a whole* failable (by passing `True` to `util.failable`) if
*all* the arches in it are failable. If *any* arch in the task
is not failable, the task should not be failable.

We allow a subset of arches to fail by passing the Koji task a
list of `optional_arches`, later. If an arch is 'optional', that
arch failing won't cause the Koji task itself to be considered
failed.

This commit fixes the logic (I hope), renames all the variables
and adds a couple of comments to make it clearer what's going on,
and does a bit of making the code simpler.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
---
 pungi/phases/kiwibuild.py | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/pungi/phases/kiwibuild.py b/pungi/phases/kiwibuild.py
index 75201c08..ff08a491 100644
--- a/pungi/phases/kiwibuild.py
+++ b/pungi/phases/kiwibuild.py
@@ -84,11 +84,9 @@ class KiwiBuildPhase(
 
                 repo = self._get_repo(image_conf, variant)
 
-                can_fail = image_conf.pop("failable", [])
-                if can_fail == ["*"]:
-                    can_fail = image_conf["arches"]
-                if can_fail:
-                    can_fail = sorted(can_fail)
+                failable_arches = image_conf.pop("failable", [])
+                if failable_arches == ["*"]:
+                    failable_arches = image_conf["arches"]
 
                 self.pool.add(RunKiwiBuildThread(self.pool))
                 self.pool.queue_put(
@@ -100,7 +98,7 @@ class KiwiBuildPhase(
                         release,
                         target,
                         repo,
-                        can_fail,
+                        failable_arches,
                     )
                 )
 
@@ -117,13 +115,15 @@ class RunKiwiBuildThread(WorkerThread):
             release,
             target,
             repo,
-            can_fail,
+            failable_arches,
         ) = item
-        self.can_fail = can_fail
+        self.failable_arches = failable_arches
+        # the Koji task as a whole can only fail if *all* arches are failable
+        can_task_fail = set(failable_arches).issuperset(set(arches))
         self.num = num
         with util.failable(
             compose,
-            can_fail,
+            can_task_fail,
             variant,
             "*",
             "kiwibuild",
@@ -145,7 +145,8 @@ class RunKiwiBuildThread(WorkerThread):
             profile=config["kiwi_profile"],
             release=release,
             repos=repo,
-            optional_arches=self.can_fail,
+            # this ensures the task won't fail if only failable arches fail
+            optional_arches=self.failable_arches,
         )
 
         koji.save_task_id(task_id)