Report warning when config sections are not used

It's possible a variant is excluded via tree_variants option and the
section does not match anything. It can be confusing to users why
nothing is happening. This patch lets Pungi log all unmatched patterns.

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

pungi/phases/base.py

@@ -26,6 +26,10 @@ class PhaseBase(object):
         self.finished = False
         self._skipped = False
 
+        # A set of config patterns that were actually used. Starts as None, and
+        # when config is queried the variable turns into a set of patterns.
+        self.used_patterns = None
+
     def validate(self):
         pass
 
@@ -59,6 +63,41 @@ class PhaseBase(object):
         self.compose.notifier.send('phase-start', phase_name=self.name)
         self.run()
 
+    def get_config_block(self, variant, arch=None):
+        """In config for current phase, find a block corresponding to given
+        variant and arch. The arch should be given if and only if the config
+        uses variant/arch mapping.
+        """
+        self.used_patterns = self.used_patterns or set()
+        if arch is not None:
+            return util.get_arch_variant_data(self.compose.conf, self.name,
+                                              arch, variant, keys=self.used_patterns)
+        else:
+            return util.get_variant_data(self.compose.conf, self.name,
+                                         variant, keys=self.used_patterns)
+
+    def get_all_patterns(self):
+        """Get all variant patterns from config file for this phase."""
+        if isinstance(self.compose.conf.get(self.name), dict):
+            return set(self.compose.conf.get(self.name, {}).keys())
+        else:
+            return set(x[0] for x in self.compose.conf.get(self.name, []))
+
+    def report_unused_patterns(self):
+        """Log warning about unused parts of the config.
+
+        This is not technically an error, but can help debug when something
+        expected is missing.
+        """
+        all_patterns = self.get_all_patterns()
+        unused_patterns = all_patterns - self.used_patterns
+        if unused_patterns:
+            self.compose.log_warning(
+                '[%s] Patterns in config do not match any variant: %s'
+                % (self.name.upper(), ', '.join(sorted(unused_patterns))))
+            self.compose.log_info(
+                'Note that variants can be excluded in configuration file')
+
     def stop(self):
         if self.finished:
             return
@@ -66,6 +105,9 @@ class PhaseBase(object):
             self.pool.stop()
         self.finished = True
         self.compose.log_info("[DONE ] %s" % self.msg)
+        if self.used_patterns is not None:
+            # We only want to report this if the config was actually queried.
+            self.report_unused_patterns()
         self.compose.notifier.send('phase-stop', phase_name=self.name)
 
     def run(self):

pungi/phases/image_build.py

@@ -5,7 +5,7 @@ import os
 import time
 from kobo import shortcuts
 
-from pungi.util import get_variant_data, makedirs, get_mtime, get_file_size, failable
+from pungi.util import makedirs, get_mtime, get_file_size, failable
 from pungi.util import translate_path, get_repo_urls, version_generator
 from pungi.phases import base
 from pungi.linker import Linker
@@ -74,7 +74,7 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG
         for variant in self.compose.get_variants():
             arches = set([x for x in variant.arches if x != 'src'])
 
-            for image_conf in get_variant_data(self.compose.conf, self.name, variant):
+            for image_conf in self.get_config_block(variant):
                 # We will modify the data, so we need to make a copy to
                 # prevent problems in next iteration where the original
                 # value is needed.

pungi/phases/live_images.py

@@ -27,7 +27,7 @@ from productmd.images import Image
 from pungi.wrappers.kojiwrapper import KojiWrapper
 from pungi.wrappers import iso
 from pungi.phases import base
-from pungi.util import get_arch_variant_data, makedirs, get_mtime, get_file_size, failable
+from pungi.util import makedirs, get_mtime, get_file_size, failable
 from pungi.util import get_repo_urls
 
 
@@ -57,7 +57,7 @@ class LiveImagesPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG
 
         for variant in self.compose.all_variants.values():
             for arch in variant.arches + ["src"]:
-                for data in get_arch_variant_data(self.compose.conf, self.name, arch, variant):
+                for data in self.get_config_block(variant, arch):
                     subvariant = data.get('subvariant', variant.uid)
                     type = data.get('type', 'live')
 

pungi/phases/livemedia_phase.py

@@ -4,7 +4,7 @@ import os
 import time
 from kobo import shortcuts
 
-from pungi.util import get_variant_data, makedirs, get_mtime, get_file_size, failable
+from pungi.util import makedirs, get_mtime, get_file_size, failable
 from pungi.util import translate_path, get_repo_urls
 from pungi.phases.base import ConfigGuardedPhase, ImageConfigMixin, PhaseLoggerMixin
 from pungi.linker import Linker
@@ -56,7 +56,7 @@ class LiveMediaPhase(PhaseLoggerMixin, ImageConfigMixin, ConfigGuardedPhase):
     def run(self):
         for variant in self.compose.get_variants():
             arches = set([x for x in variant.arches if x != 'src'])
-            for image_conf in get_variant_data(self.compose.conf, self.name, variant):
+            for image_conf in self.get_config_block(variant):
                 subvariant = image_conf.get('subvariant', variant.uid)
                 name = image_conf.get(
                     'name', "%s-%s-Live" % (self.compose.ci_base.release.short, subvariant))

pungi/phases/osbs.py

@@ -20,7 +20,7 @@ class OSBSPhase(PhaseLoggerMixin, ConfigGuardedPhase):
 
     def run(self):
         for variant in self.compose.get_variants():
-            for conf in util.get_variant_data(self.compose.conf, self.name, variant):
+            for conf in self.get_config_block(variant):
                 self.pool.add(OSBSThread(self.pool))
                 self.pool.queue_put((self.compose, variant, conf))
 

pungi/phases/ostree.py

@@ -27,14 +27,14 @@ class OSTreePhase(ConfigGuardedPhase):
     def run(self):
         if isinstance(self.compose.conf.get(self.name), dict):
             for variant in self.compose.get_variants():
-                for conf in util.get_variant_data(self.compose.conf, self.name, variant):
+                for conf in self.get_config_block(variant):
                     for arch in conf.get('arches', []) or variant.arches:
                         self._enqueue(variant, arch, conf)
         else:
             # Legacy code path to support original configuration.
             for variant in self.compose.get_variants():
                 for arch in variant.arches:
-                    for conf in util.get_arch_variant_data(self.compose.conf, self.name, arch, variant):
+                    for conf in self.get_config_block(variant, arch):
                         self._enqueue(variant, arch, conf)
 
         self.pool.start()

pungi/phases/ostree_installer.py

@@ -23,7 +23,7 @@ class OstreeInstallerPhase(PhaseLoggerMixin, ConfigGuardedPhase):
     def run(self):
         for variant in self.compose.get_variants():
             for arch in variant.arches:
-                for conf in util.get_arch_variant_data(self.compose.conf, self.name, arch, variant):
+                for conf in self.get_config_block(variant, arch):
                     self.pool.add(OstreeInstallerThread(self.pool))
                     self.pool.queue_put((self.compose, variant, arch, conf))
 

pungi/util.py

@@ -208,7 +208,7 @@ def pkg_is_debug(pkg_obj):
 
 
 # fomat: [(variant_uid_regex, {arch|*: [data]})]
-def get_arch_variant_data(conf, var_name, arch, variant):
+def get_arch_variant_data(conf, var_name, arch, variant, keys=None):
     result = []
     for conf_variant, conf_data in conf.get(var_name, []):
         if variant is not None and not re.match(conf_variant, variant.uid):
@@ -219,6 +219,8 @@ def get_arch_variant_data(conf, var_name, arch, variant):
             if conf_arch == "*" and arch == "src":
                 # src is excluded from '*' and needs to be explicitly added to the mapping
                 continue
+            if keys is not None:
+                keys.add(conf_variant)
             if isinstance(conf_data[conf_arch], list):
                 result.extend(conf_data[conf_arch])
             else:
@@ -295,7 +297,7 @@ def get_arch_data(conf, var_name, arch):
     return result
 
 
-def get_variant_data(conf, var_name, variant):
+def get_variant_data(conf, var_name, variant, keys=None):
     """Get configuration for variant.
 
     Expected config format is a mapping from variant_uid regexes to lists of
@@ -303,12 +305,15 @@ def get_variant_data(conf, var_name, variant):
 
     :param var_name: name of configuration key with which to work
     :param variant: Variant object for which to get configuration
+    :param keys: A set to which a used pattern from config will be added (optional)
     :rtype: a list of values
     """
     result = []
     for conf_variant, conf_data in conf.get(var_name, {}).iteritems():
         if not re.match(conf_variant, variant.uid):
             continue
+        if keys is not None:
+            keys.add(conf_variant)
         if isinstance(conf_data, list):
             result.extend(conf_data)
         else:

tests/test_util.py

@@ -138,6 +138,18 @@ class TestGetVariantData(unittest.TestCase):
         result = util.get_variant_data({}, 'foo', mock.Mock(uid='Client'))
         self.assertItemsEqual(result, [])
 
+    def test_get_save_pattern(self):
+        conf = {
+            'foo': {
+                '^Client$': 1,
+                '^NotClient$': 2,
+            }
+        }
+        patterns = set()
+        result = util.get_variant_data(conf, 'foo', mock.Mock(uid='Client'), keys=patterns)
+        self.assertEqual(result, [1])
+        self.assertEqual(patterns, set(['^Client$']))
+
 
 class TestVolumeIdGenerator(unittest.TestCase):
     def setUp(self):