compose: Search all nested variants

The code to search for install tree and repo for image-build and
live-media was only looking at top-level variants. Addons, optional or
integrated layered products could not have been found. This would lead
to error messages such as "There is no variant Server-optional to get
repo from when building live image for Client" even though the variant
exists.

Various tests are updated to exercise this edge case.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář 2016-11-09 10:20:55 +01:00
parent 4427769f6a
commit bd00920c62
10 changed files with 32 additions and 16 deletions

View File

@ -97,7 +97,11 @@ class Compose(kobo.log.LoggingBase):
kobo.log.LoggingBase.__init__(self, logger) kobo.log.LoggingBase.__init__(self, logger)
# TODO: check if minimal conf values are set # TODO: check if minimal conf values are set
self.conf = conf self.conf = conf
# This is a dict mapping UID to Variant objects. It only contains top
# level variants.
self.variants = {} self.variants = {}
# This is a similar mapping, but contains even nested variants.
self.all_variants = {}
self.topdir = os.path.abspath(topdir) self.topdir = os.path.abspath(topdir)
self.skip_phases = skip_phases or [] self.skip_phases = skip_phases or []
self.just_phases = just_phases or [] self.just_phases = just_phases or []
@ -215,6 +219,10 @@ class Compose(kobo.log.LoggingBase):
parser = VariantsXmlParser(file_obj, tree_arches, tree_variants, logger=self._logger) parser = VariantsXmlParser(file_obj, tree_arches, tree_variants, logger=self._logger)
self.variants = parser.parse() self.variants = parser.parse()
self.all_variants = {}
for variant in self.get_variants():
self.all_variants[variant.uid] = variant
# populate ci_base with variants - needed for layered-products (compose_id) # populate ci_base with variants - needed for layered-products (compose_id)
####FIXME - compose_to_composeinfo is no longer needed and has been ####FIXME - compose_to_composeinfo is no longer needed and has been
#### removed, but I'm not entirely sure what this is needed for #### removed, but I'm not entirely sure what this is needed for

View File

@ -31,7 +31,7 @@ class ImageBuildPhase(base.ImageConfigMixin, base.ConfigGuardedPhase):
install_tree_from = image_conf.pop('install_tree_from', variant.uid) install_tree_from = image_conf.pop('install_tree_from', variant.uid)
if '://' in install_tree_from: if '://' in install_tree_from:
return install_tree_from return install_tree_from
install_tree_source = self.compose.variants.get(install_tree_from) install_tree_source = self.compose.all_variants.get(install_tree_from)
if not install_tree_source: if not install_tree_source:
raise RuntimeError( raise RuntimeError(
'There is no variant %s to get install tree from when building image for %s.' 'There is no variant %s to get install tree from when building image for %s.'
@ -56,7 +56,7 @@ class ImageBuildPhase(base.ImageConfigMixin, base.ConfigGuardedPhase):
extras.append(variant.uid) extras.append(variant.uid)
for extra in extras: for extra in extras:
v = self.compose.variants.get(extra) v = self.compose.all_variants.get(extra)
if not v: if not v:
raise RuntimeError( raise RuntimeError(
'There is no variant %s to get repo from when building image for %s.' 'There is no variant %s to get repo from when building image for %s.'

View File

@ -47,7 +47,7 @@ class LiveImagesPhase(base.ImageConfigMixin, base.ConfigGuardedPhase):
def _get_extra_repos(self, arch, variant, extras): def _get_extra_repos(self, arch, variant, extras):
repo = [] repo = []
for extra in extras: for extra in extras:
v = self.compose.variants.get(extra) v = self.compose.all_variants.get(extra)
if not v: if not v:
raise RuntimeError( raise RuntimeError(
'There is no variant %s to get repo from when building live image for %s.' 'There is no variant %s to get repo from when building live image for %s.'
@ -72,7 +72,7 @@ class LiveImagesPhase(base.ImageConfigMixin, base.ConfigGuardedPhase):
symlink_isos_to = self.compose.conf.get("symlink_isos_to") symlink_isos_to = self.compose.conf.get("symlink_isos_to")
commands = [] commands = []
for variant in self.compose.variants.values(): for variant in self.compose.all_variants.values():
for arch in variant.arches + ["src"]: for arch in variant.arches + ["src"]:
for data in get_arch_variant_data(self.compose.conf, self.name, arch, variant): for data in get_arch_variant_data(self.compose.conf, self.name, arch, variant):
subvariant = data.get('subvariant', variant.uid) subvariant = data.get('subvariant', variant.uid)

View File

@ -36,7 +36,7 @@ class LiveMediaPhase(ImageConfigMixin, ConfigGuardedPhase):
extras.append(variant.uid) extras.append(variant.uid)
for extra in extras: for extra in extras:
v = self.compose.variants.get(extra) v = self.compose.all_variants.get(extra)
if not v: if not v:
raise RuntimeError( raise RuntimeError(
'There is no variant %s to get repo from when building live media for %s.' 'There is no variant %s to get repo from when building live media for %s.'
@ -56,7 +56,7 @@ class LiveMediaPhase(ImageConfigMixin, ConfigGuardedPhase):
if 'install_tree_from' in image_conf: if 'install_tree_from' in image_conf:
variant_uid = image_conf['install_tree_from'] variant_uid = image_conf['install_tree_from']
try: try:
variant = self.compose.variants[variant_uid] variant = self.compose.all_variants[variant_uid]
except KeyError: except KeyError:
raise RuntimeError( raise RuntimeError(
'There is no variant %s to get repo from when building live media for %s.' 'There is no variant %s to get repo from when building live media for %s.'

View File

@ -45,7 +45,7 @@ class OSTreeThread(WorkerThread):
(arch, variant.uid, self.num)) (arch, variant.uid, self.num))
repodir = os.path.join(workdir, 'config_repo') repodir = os.path.join(workdir, 'config_repo')
source_variant = compose.variants[config['source_repo_from']] source_variant = compose.all_variants[config['source_repo_from']]
source_repo = translate_path(compose, source_repo = translate_path(compose,
compose.paths.compose.repository('$basearch', compose.paths.compose.repository('$basearch',
source_variant, source_variant,

View File

@ -69,7 +69,7 @@ class OstreeInstallerThread(WorkerThread):
""" """
if '://' in source: if '://' in source:
return source.replace('$arch', arch) return source.replace('$arch', arch)
source_variant = compose.variants[source] source_variant = compose.all_variants[source]
return translate_path( return translate_path(
compose, compose.paths.compose.repository(arch, source_variant, create_dir=False)) compose, compose.paths.compose.repository(arch, source_variant, create_dir=False))

View File

@ -59,6 +59,11 @@ class DummyCompose(object):
'Everything': mock.Mock(uid='Everything', arches=['x86_64', 'amd64'], 'Everything': mock.Mock(uid='Everything', arches=['x86_64', 'amd64'],
type='variant', is_empty=False), type='variant', is_empty=False),
} }
self.all_variants = self.variants.copy()
self.all_variants['Server-optional'] = mock.Mock(
uid='Server-optional', arches=['x86_64'], type='optional', is_empty=False,
parent=self.variants['Server'])
self.variants['Server'].variants = {'optional': self.all_variants['Server-optional']}
self.log_info = mock.Mock() self.log_info = mock.Mock()
self.log_error = mock.Mock() self.log_error = mock.Mock()
self.log_debug = mock.Mock() self.log_debug = mock.Mock()

View File

@ -277,7 +277,7 @@ class TestImageBuildPhase(PungiTestCase):
'distro': 'Fedora-20', 'distro': 'Fedora-20',
'disk_size': 3, 'disk_size': 3,
'arches': ['x86_64'], 'arches': ['x86_64'],
'install_tree_from': 'Everything', 'install_tree_from': 'Server-optional',
} }
} }
] ]
@ -302,7 +302,7 @@ class TestImageBuildPhase(PungiTestCase):
"format": [('docker', 'tar.xz')], "format": [('docker', 'tar.xz')],
"image_conf": { "image_conf": {
'image-build': { 'image-build': {
'install_tree': self.topdir + '/compose/Everything/$arch/os', 'install_tree': self.topdir + '/compose/Server-optional/$arch/os',
'kickstart': 'fedora-docker-base.ks', 'kickstart': 'fedora-docker-base.ks',
'format': 'docker', 'format': 'docker',
'repo': self.topdir + '/compose/Server/$arch/os', 'repo': self.topdir + '/compose/Server/$arch/os',
@ -340,7 +340,7 @@ class TestImageBuildPhase(PungiTestCase):
'distro': 'Fedora-20', 'distro': 'Fedora-20',
'disk_size': 3, 'disk_size': 3,
'arches': ['x86_64'], 'arches': ['x86_64'],
'repo_from': 'Everything', 'repo_from': ['Everything', 'Server-optional'],
} }
} }
] ]
@ -369,6 +369,7 @@ class TestImageBuildPhase(PungiTestCase):
'kickstart': 'fedora-docker-base.ks', 'kickstart': 'fedora-docker-base.ks',
'format': 'docker', 'format': 'docker',
'repo': ','.join([self.topdir + '/compose/Everything/$arch/os', 'repo': ','.join([self.topdir + '/compose/Everything/$arch/os',
self.topdir + '/compose/Server-optional/$arch/os',
self.topdir + '/compose/Server/$arch/os']), self.topdir + '/compose/Server/$arch/os']),
'variant': compose.variants['Server'], 'variant': compose.variants['Server'],
'target': 'f24', 'target': 'f24',

View File

@ -25,7 +25,7 @@ class TestLiveImagesPhase(PungiTestCase):
'amd64': { 'amd64': {
'kickstart': 'test.ks', 'kickstart': 'test.ks',
'additional_repos': ['http://example.com/repo/'], 'additional_repos': ['http://example.com/repo/'],
'repo_from': ['Everything'], 'repo_from': ['Everything', 'Server-optional'],
'release': None, 'release': None,
} }
}) })
@ -49,7 +49,8 @@ class TestLiveImagesPhase(PungiTestCase):
'scratch': False, 'scratch': False,
'repos': [self.topdir + '/compose/Client/amd64/os', 'repos': [self.topdir + '/compose/Client/amd64/os',
'http://example.com/repo/', 'http://example.com/repo/',
self.topdir + '/compose/Everything/amd64/os'], self.topdir + '/compose/Everything/amd64/os',
self.topdir + '/compose/Server-optional/amd64/os'],
'label': '', 'label': '',
'name': None, 'name': None,
'filename': 'image-name', 'filename': 'image-name',

View File

@ -326,12 +326,12 @@ class TestLiveMediaPhase(PungiTestCase):
'scratch': True, 'scratch': True,
'skip_tag': True, 'skip_tag': True,
'title': 'Custom Title', 'title': 'Custom Title',
'repo_from': ['Everything'], 'repo_from': ['Everything', 'Server-optional'],
'repo': ['http://example.com/extra_repo'], 'repo': ['http://example.com/extra_repo'],
'arches': ['x86_64'], 'arches': ['x86_64'],
'ksversion': '24', 'ksversion': '24',
'release': None, 'release': None,
'install_tree_from': 'Everything', 'install_tree_from': 'Server-optional',
'subvariant': 'Something', 'subvariant': 'Something',
'failable': ['*'], 'failable': ['*'],
} }
@ -359,12 +359,13 @@ class TestLiveMediaPhase(PungiTestCase):
'release': '20151203.t.0', 'release': '20151203.t.0',
'repo': ['http://example.com/extra_repo', 'repo': ['http://example.com/extra_repo',
self.topdir + '/compose/Everything/$basearch/os', self.topdir + '/compose/Everything/$basearch/os',
self.topdir + '/compose/Server-optional/$basearch/os',
self.topdir + '/compose/Server/$basearch/os'], self.topdir + '/compose/Server/$basearch/os'],
'scratch': True, 'scratch': True,
'skip_tag': True, 'skip_tag': True,
'target': 'f24', 'target': 'f24',
'title': 'Custom Title', 'title': 'Custom Title',
'install_tree': self.topdir + '/compose/Everything/$basearch/os', 'install_tree': self.topdir + '/compose/Server-optional/$basearch/os',
'version': '25', 'version': '25',
'subvariant': 'Something', 'subvariant': 'Something',
'failable_arches': ['*'], 'failable_arches': ['*'],