Filter variants and architectures
There already were config options tree_arches and tree_variants, but the filtering was done at the compose level and there was not interaction between the two options. This led to problems when a variant would have all its arches filtered out. This patch moves the filtering to the variant loader. It adds better logging, so whenever a variant is filtered (for any reason), it will be explicitly stated in the logs. Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář
parent
652987f2cc
commit
d383e6c5c0
@ -202,9 +202,11 @@ class Compose(kobo.log.LoggingBase):
|
||||
shutil.copy2(os.path.join(tmp_dir, file_name), variants_file)
|
||||
shutil.rmtree(tmp_dir)
|
||||
|
||||
file_obj = open(variants_file, "r")
|
||||
tree_arches = self.conf.get("tree_arches", None)
|
||||
self.variants = VariantsXmlParser(file_obj, tree_arches).parse()
|
||||
tree_variants = self.conf.get("tree_variants", None)
|
||||
with open(variants_file, "r") as file_obj:
|
||||
parser = VariantsXmlParser(file_obj, tree_arches, tree_variants, logger=self._logger)
|
||||
self.variants = parser.parse()
|
||||
|
||||
# populate ci_base with variants - needed for layered-products (compose_id)
|
||||
####FIXME - compose_to_composeinfo is no longer needed and has been
|
||||
@ -215,26 +217,16 @@ class Compose(kobo.log.LoggingBase):
|
||||
def get_variants(self, types=None, arch=None, recursive=False):
|
||||
result = []
|
||||
types = types or ["variant", "optional", "addon", "layered-product"]
|
||||
tree_variants = self.conf.get("tree_variants", None)
|
||||
for i in self.variants.values():
|
||||
if tree_variants and i.name not in tree_variants:
|
||||
continue
|
||||
if i.type in types:
|
||||
if arch and arch not in i.arches:
|
||||
continue
|
||||
for i in self.variants.itervalues():
|
||||
if i.type in types and (not arch or arch in i.arches):
|
||||
result.append(i)
|
||||
result.extend(i.get_variants(types=types, arch=arch, recursive=recursive))
|
||||
return sorted(set(result))
|
||||
|
||||
def get_arches(self):
|
||||
result = set()
|
||||
tree_arches = self.conf.get("tree_arches", None)
|
||||
for variant in self.get_variants():
|
||||
for arch in variant.arches:
|
||||
if tree_arches:
|
||||
if arch in tree_arches:
|
||||
result.add(arch)
|
||||
else:
|
||||
result.add(arch)
|
||||
return sorted(result)
|
||||
|
||||
|
||||
@ -42,12 +42,14 @@ if not os.path.isfile(VARIANTS_DTD):
|
||||
|
||||
|
||||
class VariantsXmlParser(object):
|
||||
def __init__(self, file_obj, tree_arches=None):
|
||||
def __init__(self, file_obj, tree_arches=None, tree_variants=None, logger=None):
|
||||
self.tree = lxml.etree.parse(file_obj)
|
||||
self.dtd = lxml.etree.DTD(open(VARIANTS_DTD, "r"))
|
||||
self.addons = {}
|
||||
self.layered_products = {}
|
||||
self.tree_arches = tree_arches
|
||||
self.tree_variants = tree_variants
|
||||
self.logger = logger
|
||||
self.validate()
|
||||
|
||||
def _is_true(self, value):
|
||||
@ -62,7 +64,7 @@ class VariantsXmlParser(object):
|
||||
errors = [str(i) for i in self.dtd.error_log.filter_from_errors()]
|
||||
raise ValueError("Variants XML doesn't validate:\n%s" % "\n".join(errors))
|
||||
|
||||
def parse_variant_node(self, variant_node):
|
||||
def parse_variant_node(self, variant_node, parent=None):
|
||||
variant_dict = {
|
||||
"id": str(variant_node.attrib["id"]),
|
||||
"name": str(variant_node.attrib["name"]),
|
||||
@ -72,9 +74,14 @@ class VariantsXmlParser(object):
|
||||
"environments": [],
|
||||
"buildinstallpackages": [],
|
||||
"is_empty": bool(variant_node.attrib.get("is_empty", False)),
|
||||
"parent": parent,
|
||||
}
|
||||
if self.tree_arches:
|
||||
variant_dict["arches"] = [i for i in variant_dict["arches"] if i in self.tree_arches]
|
||||
if not variant_dict["arches"]:
|
||||
if self.logger:
|
||||
self.logger.info('Excluding variant %s: all its arches are filtered.' % variant_dict['id'])
|
||||
return None
|
||||
|
||||
for grouplist_node in variant_node.xpath("groups"):
|
||||
for group_node in grouplist_node.xpath("group"):
|
||||
@ -121,19 +128,21 @@ class VariantsXmlParser(object):
|
||||
|
||||
contains_optional = False
|
||||
for child_node in variant_node.xpath("variants/variant"):
|
||||
child_variant = self.parse_variant_node(child_node)
|
||||
variant.add_variant(child_variant)
|
||||
child_variant = self.parse_variant_node(child_node, variant)
|
||||
if not self.add_child(child_variant, variant):
|
||||
continue
|
||||
if child_variant.type == "optional":
|
||||
contains_optional = True
|
||||
|
||||
has_optional = self._is_true(variant_node.attrib.get("has_optional", "false"))
|
||||
if has_optional and not contains_optional:
|
||||
optional = Variant(id="optional", name="optional", type="optional", arches=variant.arches, groups=[])
|
||||
variant.add_variant(optional)
|
||||
optional = Variant(id="optional", name="optional", type="optional",
|
||||
arches=variant.arches, groups=[], parent=variant)
|
||||
self.add_child(optional, variant)
|
||||
|
||||
for ref in variant_node.xpath("variants/ref/@id"):
|
||||
child_variant = self.parse_variant_node(self.addons[ref])
|
||||
variant.add_variant(child_variant)
|
||||
child_variant = self.parse_variant_node(self.addons[ref], variant)
|
||||
self.add_child(child_variant, variant)
|
||||
|
||||
# XXX: top-level optional
|
||||
# for ref in variant_node.xpath("variants/ref/@id"):
|
||||
@ -141,6 +150,19 @@ class VariantsXmlParser(object):
|
||||
|
||||
return variant
|
||||
|
||||
def _is_excluded(self, variant):
|
||||
if self.tree_variants and variant.uid not in self.tree_variants:
|
||||
if self.logger:
|
||||
self.logger.info('Excluding variant %s: filtered by configuration.' % variant)
|
||||
return True
|
||||
return False
|
||||
|
||||
def add_child(self, child, parent):
|
||||
if not child or self._is_excluded(child):
|
||||
return None
|
||||
parent.add_variant(child)
|
||||
return child
|
||||
|
||||
def parse(self):
|
||||
# we allow top-level addon definitions which can be referenced in variants
|
||||
for variant_node in self.tree.xpath("/variants/variant[@type='addon']"):
|
||||
@ -154,6 +176,8 @@ class VariantsXmlParser(object):
|
||||
result = {}
|
||||
for variant_node in self.tree.xpath("/variants/variant[@type='variant']"):
|
||||
variant = self.parse_variant_node(variant_node)
|
||||
if not variant or self._is_excluded(variant):
|
||||
continue
|
||||
result[variant.id] = variant
|
||||
|
||||
for variant_node in self.tree.xpath("/variants/variant[not(@type='variant' or @type='addon' or @type='layered-product')]"):
|
||||
@ -163,7 +187,8 @@ class VariantsXmlParser(object):
|
||||
|
||||
|
||||
class Variant(object):
|
||||
def __init__(self, id, name, type, arches, groups, environments=None, buildinstallpackages=None, is_empty=False):
|
||||
def __init__(self, id, name, type, arches, groups, environments=None,
|
||||
buildinstallpackages=None, is_empty=False, parent=None):
|
||||
if not id.isalnum():
|
||||
raise ValueError("Variant ID must contain only alphanumeric characters: %s" % id)
|
||||
|
||||
@ -178,7 +203,7 @@ class Variant(object):
|
||||
self.environments = sorted(copy.deepcopy(environments), lambda x, y: cmp(x["name"], y["name"]))
|
||||
self.buildinstallpackages = sorted(buildinstallpackages)
|
||||
self.variants = {}
|
||||
self.parent = None
|
||||
self.parent = parent
|
||||
self.is_empty = is_empty
|
||||
|
||||
def __getitem__(self, name):
|
||||
|
||||
82
tests/fixtures/variants.xml
vendored
Normal file
82
tests/fixtures/variants.xml
vendored
Normal file
@ -0,0 +1,82 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE variants PUBLIC "-//Red Hat, Inc.//DTD Variants info//EN" "variants.dtd">
|
||||
|
||||
<variants>
|
||||
<variant id="ResilientStorage" name="Resilient Storage" type="addon">
|
||||
<arches>
|
||||
<arch>x86_64</arch>
|
||||
</arches>
|
||||
<groups>
|
||||
<group default="true">resilient-storage</group>
|
||||
</groups>
|
||||
</variant>
|
||||
|
||||
<variant id="Live" name="Live" type="variant" is_empty="true">
|
||||
<arches>
|
||||
<arch>x86_64</arch>
|
||||
</arches>
|
||||
</variant>
|
||||
|
||||
<variant id="Crashy" name="Crashy" type="variant">
|
||||
<arches>
|
||||
<arch>ppc64le</arch>
|
||||
</arches>
|
||||
</variant>
|
||||
|
||||
<variant id="Gluster" name="Gluster Layered Product" type="layered-product">
|
||||
<release name="Gluster" version="2.3" short="Gluster" />
|
||||
<arches>
|
||||
<arch>x86_64</arch>
|
||||
</arches>
|
||||
<groups>
|
||||
<group default="true">gluster</group>
|
||||
</groups>
|
||||
</variant>
|
||||
|
||||
<variant id="Client" name="Client" type="variant">
|
||||
<arches>
|
||||
<arch>i386</arch>
|
||||
<arch>x86_64</arch>
|
||||
</arches>
|
||||
<groups>
|
||||
<group default="true">core</group>
|
||||
<group default="true">standard</group>
|
||||
<group default="false">text-internet</group>
|
||||
<group default="true" uservisible="false">firefox</group>
|
||||
<group>skype</group>
|
||||
</groups>
|
||||
<environments>
|
||||
<environment>minimal</environment>
|
||||
<environment display_order="1000">desktop</environment>
|
||||
</environments>
|
||||
</variant>
|
||||
|
||||
<variant id="Server" name="Server" type="variant" has_optional="true">
|
||||
<arches>
|
||||
<arch>x86_64</arch>
|
||||
<arch>s390x</arch>
|
||||
</arches>
|
||||
<groups>
|
||||
<group default="true" uservisible="true">core</group>
|
||||
<group default="true">standard</group>
|
||||
<group default="true">text-internet</group>
|
||||
</groups>
|
||||
<environments>
|
||||
<environment>minimal</environment>
|
||||
</environments>
|
||||
<variants>
|
||||
<ref id="ResilientStorage"/>
|
||||
<ref id="Gluster"/>
|
||||
<variant id="optional" name="optional" type="optional">
|
||||
<arches>
|
||||
<arch>x86_64</arch>
|
||||
<arch>s390x</arch>
|
||||
</arches>
|
||||
<groups>
|
||||
<group default="false">firefox</group>
|
||||
</groups>
|
||||
</variant>
|
||||
</variants>
|
||||
</variant>
|
||||
|
||||
</variants>
|
||||
@ -13,6 +13,12 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
from pungi.compose import Compose
|
||||
|
||||
|
||||
class ConfigWrapper(dict):
|
||||
def __init__(self, *args, **kwargs):
|
||||
super(ConfigWrapper, self).__init__(*args, **kwargs)
|
||||
self._open_file = '%s/fixtures/config.conf' % os.path.abspath(os.path.dirname(__file__))
|
||||
|
||||
|
||||
class ComposeTestCase(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tmp_dir = tempfile.mkdtemp()
|
||||
@ -123,6 +129,158 @@ class ComposeTestCase(unittest.TestCase):
|
||||
|
||||
self.assertEqual(compose.image_release, '20160107.n.2')
|
||||
|
||||
@mock.patch('pungi.compose.ComposeInfo')
|
||||
def test_get_variant_arches_without_filter(self, ci):
|
||||
conf = ConfigWrapper(
|
||||
variants_file={'scm': 'file',
|
||||
'repo': None,
|
||||
'file': 'variants.xml'},
|
||||
release_name='Test',
|
||||
release_version='1.0',
|
||||
release_short='test',
|
||||
)
|
||||
|
||||
compose = Compose(conf, self.tmp_dir)
|
||||
compose.read_variants()
|
||||
|
||||
self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]),
|
||||
['Client', 'Crashy', 'Live', 'Server'])
|
||||
self.assertEqual(sorted([v.uid for v in compose.variants['Server'].variants.itervalues()]),
|
||||
['Server-Gluster', 'Server-ResilientStorage', 'Server-optional'])
|
||||
self.assertItemsEqual(compose.variants['Client'].arches,
|
||||
['i386', 'x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Crashy'].arches,
|
||||
['ppc64le'])
|
||||
self.assertItemsEqual(compose.variants['Live'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].arches,
|
||||
['s390x', 'x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['Gluster'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['ResilientStorage'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['optional'].arches,
|
||||
['s390x', 'x86_64'])
|
||||
|
||||
self.assertEqual([v.uid for v in compose.get_variants()],
|
||||
['Client', 'Crashy', 'Live', 'Server', 'Server-Gluster',
|
||||
'Server-ResilientStorage', 'Server-optional'])
|
||||
self.assertEqual(compose.get_arches(), ['i386', 'ppc64le', 's390x', 'x86_64'])
|
||||
|
||||
@mock.patch('pungi.compose.ComposeInfo')
|
||||
def test_get_variant_arches_with_arch_filter(self, ci):
|
||||
conf = ConfigWrapper(
|
||||
variants_file={'scm': 'file',
|
||||
'repo': None,
|
||||
'file': 'variants.xml'},
|
||||
release_name='Test',
|
||||
release_version='1.0',
|
||||
release_short='test',
|
||||
tree_arches=['x86_64'],
|
||||
)
|
||||
|
||||
compose = Compose(conf, self.tmp_dir)
|
||||
compose.read_variants()
|
||||
|
||||
self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]),
|
||||
['Client', 'Live', 'Server'])
|
||||
self.assertEqual(sorted([v.uid for v in compose.variants['Server'].variants.itervalues()]),
|
||||
['Server-Gluster', 'Server-ResilientStorage', 'Server-optional'])
|
||||
self.assertItemsEqual(compose.variants['Client'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Live'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['Gluster'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['ResilientStorage'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['optional'].arches,
|
||||
['x86_64'])
|
||||
|
||||
self.assertEqual(compose.get_arches(), ['x86_64'])
|
||||
self.assertEqual([v.uid for v in compose.get_variants()],
|
||||
['Client', 'Live', 'Server', 'Server-Gluster',
|
||||
'Server-ResilientStorage', 'Server-optional'])
|
||||
|
||||
@mock.patch('pungi.compose.ComposeInfo')
|
||||
def test_get_variant_arches_with_variant_filter(self, ci):
|
||||
ci.return_value.compose.respin = 2
|
||||
ci.return_value.compose.date = '20160107'
|
||||
ci.return_value.compose.type = 'production'
|
||||
ci.return_value.compose.type_suffix = '.n'
|
||||
|
||||
conf = ConfigWrapper(
|
||||
variants_file={'scm': 'file',
|
||||
'repo': None,
|
||||
'file': 'variants.xml'},
|
||||
release_name='Test',
|
||||
release_version='1.0',
|
||||
release_short='test',
|
||||
tree_variants=['Server', 'Client', 'Server-Gluster'],
|
||||
)
|
||||
|
||||
compose = Compose(conf, self.tmp_dir)
|
||||
compose.read_variants()
|
||||
|
||||
self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]),
|
||||
['Client', 'Server'])
|
||||
self.assertItemsEqual(compose.variants['Client'].arches,
|
||||
['i386', 'x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].arches,
|
||||
['s390x', 'x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['Gluster'].arches,
|
||||
['x86_64'])
|
||||
|
||||
self.assertEqual(compose.get_arches(), ['i386', 's390x', 'x86_64'])
|
||||
self.assertEqual([v.uid for v in compose.get_variants()],
|
||||
['Client', 'Server', 'Server-Gluster'])
|
||||
|
||||
@mock.patch('pungi.compose.ComposeInfo')
|
||||
def test_get_variant_arches_with_both_filters(self, ci):
|
||||
ci.return_value.compose.respin = 2
|
||||
ci.return_value.compose.date = '20160107'
|
||||
ci.return_value.compose.type = 'production'
|
||||
ci.return_value.compose.type_suffix = '.n'
|
||||
|
||||
logger = mock.Mock()
|
||||
|
||||
conf = ConfigWrapper(
|
||||
variants_file={'scm': 'file',
|
||||
'repo': None,
|
||||
'file': 'variants.xml'},
|
||||
release_name='Test',
|
||||
release_version='1.0',
|
||||
release_short='test',
|
||||
tree_variants=['Server', 'Client', 'Server-optional'],
|
||||
tree_arches=['x86_64'],
|
||||
)
|
||||
|
||||
compose = Compose(conf, self.tmp_dir, logger=logger)
|
||||
compose.read_variants()
|
||||
|
||||
self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]),
|
||||
['Client', 'Server'])
|
||||
self.assertItemsEqual(compose.variants['Client'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].arches,
|
||||
['x86_64'])
|
||||
self.assertItemsEqual(compose.variants['Server'].variants['optional'].arches,
|
||||
['x86_64'])
|
||||
|
||||
self.assertEqual(compose.get_arches(), ['x86_64'])
|
||||
self.assertEqual([v.uid for v in compose.get_variants()],
|
||||
['Client', 'Server', 'Server-optional'])
|
||||
|
||||
self.assertItemsEqual(
|
||||
logger.info.call_args_list,
|
||||
[mock.call('Excluding variant Live: filtered by configuration.'),
|
||||
mock.call('Excluding variant Crashy: all its arches are filtered.'),
|
||||
mock.call('Excluding variant Server-ResilientStorage: filtered by configuration.'),
|
||||
mock.call('Excluding variant Server-Gluster: filtered by configuration.')]
|
||||
)
|
||||
|
||||
|
||||
class StatusTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user