repoclosure: Allow aborting compose when repoclosure fails

Alternatively the call to repoclosure can be turned off. This is
customizable per variant and architecture.

Fixes: https://pagure.io/pungi/issue/676
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

doc/configuration.rst

@@ -152,6 +152,18 @@ Options
     ([*str*]) -- list of variants which should be included; if undefined, all
     variants from variants.xml will be included
 
+**repoclosure_strictness**
+    (*list*) -- variant/arch mapping describing how repoclosure should run.
+    Possible values are
+
+     * ``off`` -- do not run repoclosure
+     * ``lenient`` -- (default) run repoclosure and write results to logs, but
+       detected errors are only reported in logs
+     * ``fatal`` -- abort compose when any issue is detected
+
+    When multiple blocks in the mapping match a variant/arch combination, the
+    last value will win.
+
 **repoclosure_backend**
     (*str*) -- Select which tool should be used to run repoclosure over created
     repositories. By default ``yum`` is used, but you can switch to ``dnf``.
@@ -190,6 +202,13 @@ Example
     tree_arches = ["x86_64"]
     tree_variants = ["Server"]
 
+    repoclosure_strictness = [
+        # Make repoclosure failures fatal for compose on all variants …
+        ('^.*$', {'*': 'fatal'}),
+        # … except for Everything where it should not run at all.
+        ('^Everything$', {'*': 'off'})
+    ]
+
 
 Image Naming
 ============

doc/phases.rst

@@ -141,9 +141,10 @@ Test
 
 This phase is supposed to run some sanity checks on the finished compose.
 
-The first test is to run ``repoclosure`` on each repository. However, even if
+The first test is to run ``repoclosure`` on each repository. By default errors
-it fails, the compose will still be considered a success. The actual error has
+are only reported in the log, the compose will still be considered a success.
-to be looked up in the compose logs directory.
+The actual error has to be looked up in the compose logs directory.
+Configuration allows customizing this.
 
 The other test is to check all images listed the metadata and verify that they
 look sane. For ISO files headers are checked to verify the format is correct,

pungi/checks.py

@@ -622,6 +622,11 @@ def _make_schema():
                 "type": "boolean",
                 "default": False,
             },
+            "repoclosure_strictness": _variant_arch_mapping({
+                "type": "string",
+                "default": "lenient",
+                "enum": ["off", "lenient", "fatal"],
+            }),
             "repoclosure_backend": {
                 "type": "string",
                 "default": "yum",

pungi/phases/test.py

@@ -22,7 +22,7 @@ from pungi.wrappers import repoclosure
 from pungi.arch import get_valid_arches
 from pungi.phases.base import PhaseBase
 from pungi.phases.gather import get_lookaside_repos
-from pungi.util import is_arch_multilib, failable, temp_dir
+from pungi.util import is_arch_multilib, failable, temp_dir, get_arch_variant_data
 
 
 class TestPhase(PhaseBase):
@@ -44,6 +44,11 @@ def run_repoclosure(compose):
         for variant in compose.get_variants(arch=arch):
             if variant.is_empty:
                 continue
+
+            conf = get_arch_variant_data(compose.conf, 'repoclosure_strictness', arch, variant)
+            if conf and conf[-1] == 'off':
+                continue
+
             lookaside = {}
             if variant.parent:
                 repo_id = "repoclosure-%s.%s" % (variant.parent.uid, arch)
@@ -72,7 +77,11 @@ def run_repoclosure(compose):
                     run(cmd, logfile=compose.paths.log.log_file(arch, "repoclosure-%s" % variant),
                         workdir=tmp_dir)
                 except RuntimeError as exc:
-                    compose.log_warning('Repoclosure failed for %s.%s\n%s' % (variant.uid, arch, exc))
+                    if conf and conf[-1] == 'fatal':
+                        raise
+                    else:
+                        compose.log_warning('Repoclosure failed for %s.%s\n%s'
+                                            % (variant.uid, arch, exc))
 
     compose.log_info("[DONE ] %s" % msg)
 

tests/helpers.py

@@ -159,6 +159,12 @@ def boom(*args, **kwargs):
     raise Exception('BOOM')
 
 
+def mk_boom(cls=Exception, msg='BOOM'):
+    def b(*args, **kwargs):
+        raise cls(msg)
+    return b
+
+
 PKGSET_REPOS = dict(
     pkgset_source='repos',
     pkgset_repos={},

tests/test_test_phase.py

@@ -14,7 +14,7 @@ import sys
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 import pungi.phases.test as test_phase
-from tests.helpers import DummyCompose, PungiTestCase, touch
+from tests.helpers import DummyCompose, PungiTestCase, touch, mk_boom
 
 
 PAD = '\0' * 100
@@ -169,23 +169,31 @@ class TestCheckImageSanity(PungiTestCase):
 
 class TestRepoclosure(PungiTestCase):
 
+    def setUp(self):
+        super(TestRepoclosure, self).setUp()
+        self.maxDiff = None
+
     def _get_repo(self, variant, arch, path=None):
         path = path or arch + '/os'
         return {
             'repoclosure-%s.%s' % (variant, arch): self.topdir + '/compose/%s/%s' % (variant, path)
         }
 
+    @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
+    @mock.patch('pungi.phases.test.run')
+    def test_repoclosure_skip_if_disabled(self, mock_run, mock_grc):
+        compose = DummyCompose(self.topdir, {
+            'repoclosure_strictness': [('^.*$', {'*': 'off'})]
+        })
+        test_phase.run_repoclosure(compose)
+
+        self.assertItemsEqual(mock_grc.call_args_list, [])
+
     @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
     @mock.patch('pungi.phases.test.run')
     def test_repoclosure_default_backend(self, mock_run, mock_grc):
         compose = DummyCompose(self.topdir, {})
         test_phase.run_repoclosure(compose)
-        self.maxDiff = None
-        all_repos = {}
-        for variant in compose.variants.itervalues():
-            for arch in variant.arches:
-                all_repos.update(self._get_repo(variant.uid, arch))
-            all_repos.update(self._get_repo(variant.uid, 'src', 'source/tree'))
 
         self.assertItemsEqual(
             mock_grc.call_args_list,
@@ -205,12 +213,6 @@ class TestRepoclosure(PungiTestCase):
     def test_repoclosure_dnf_backend(self, mock_run, mock_grc):
         compose = DummyCompose(self.topdir, {'repoclosure_backend': 'dnf'})
         test_phase.run_repoclosure(compose)
-        self.maxDiff = None
-        all_repos = {}
-        for variant in compose.variants.itervalues():
-            for arch in variant.arches:
-                all_repos.update(self._get_repo(variant.uid, arch))
-            all_repos.update(self._get_repo(variant.uid, 'src', 'source/tree'))
 
         self.assertItemsEqual(
             mock_grc.call_args_list,
@@ -225,6 +227,52 @@ class TestRepoclosure(PungiTestCase):
              mock.call(backend='dnf', arch=['x86_64', 'noarch'], lookaside={},
                        repos=self._get_repo('Everything', 'x86_64'))])
 
+    @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
+    @mock.patch('pungi.phases.test.run')
+    def test_repoclosure_report_error(self, mock_run, mock_grc):
+        compose = DummyCompose(self.topdir, {
+            'repoclosure_strictness': [('^.*$', {'*': 'fatal'})]
+        })
+        mock_run.side_effect = mk_boom(cls=RuntimeError)
+
+        with self.assertRaises(RuntimeError):
+            test_phase.run_repoclosure(compose)
+
+    @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
+    @mock.patch('pungi.phases.test.run')
+    def test_repoclosure_overwrite_options_creates_correct_commands(self, mock_run, mock_grc):
+        compose = DummyCompose(self.topdir, {
+            'repoclosure_backend': 'dnf',
+            'repoclosure_strictness': [
+                ('^.*$', {'*': 'off'}),
+                ('^Server$', {'*': 'fatal'}),
+            ]
+        })
+        test_phase.run_repoclosure(compose)
+
+        self.assertItemsEqual(
+            mock_grc.call_args_list,
+            [mock.call(backend='dnf', arch=['amd64', 'x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Server', 'amd64')),
+             mock.call(backend='dnf', arch=['x86_64', 'noarch'], lookaside={},
+                       repos=self._get_repo('Server', 'x86_64')),
+             ])
+
+    @mock.patch('pungi.wrappers.repoclosure.get_repoclosure_cmd')
+    @mock.patch('pungi.phases.test.run')
+    def test_repoclosure_uses_correct_behaviour(self, mock_run, mock_grc):
+        compose = DummyCompose(self.topdir, {
+            'repoclosure_backend': 'dnf',
+            'repoclosure_strictness': [
+                ('^.*$', {'*': 'off'}),
+                ('^Server$', {'*': 'fatal'}),
+            ]
+        })
+        mock_run.side_effect = mk_boom(cls=RuntimeError)
+
+        with self.assertRaises(RuntimeError):
+            test_phase.run_repoclosure(compose)
+
 
 if __name__ == "__main__":
     unittest.main()