From 3a4b223d6a103c635ec5838060b077fe9638672a Mon Sep 17 00:00:00 2001 From: Marcin Juszkiewicz Date: Thu, 18 Jul 2019 15:54:36 +0200 Subject: [PATCH] Change how build process treats unbuildable images We have a list of images which are unbuildable for distro/buildtype/arch combos. This patch renames it from SKIPPED_IMAGES to UNBUILDABLE_IMAGES because 'skipped' images is something else in our code. Now all unbuildable images are marked as such and so are their children. At the end of build there is information provided about skipped images (due to --skip-parents or --skip-existing options) and then information about not buildable images. Both list should only contain entries related to requested build. Change-Id: If9b521339f564e483cba03d52e7c4eba271821a5 --- kolla/cmd/build.py | 2 +- kolla/image/build.py | 139 ++++++++++++++++++++++++++------------ kolla/tests/test_build.py | 15 ++-- tests/test_build.py | 4 +- 4 files changed, 111 insertions(+), 49 deletions(-) diff --git a/kolla/cmd/build.py b/kolla/cmd/build.py index e912a6d120..3ccc4475a0 100755 --- a/kolla/cmd/build.py +++ b/kolla/cmd/build.py @@ -33,7 +33,7 @@ def main(): statuses = build.run_build() if statuses: (bad_results, good_results, unmatched_results, - skipped_results) = statuses + skipped_results, unbuildable_results) = statuses if bad_results: return 1 return 0 diff --git a/kolla/image/build.py b/kolla/image/build.py index 71a93ebb04..2363764bad 100755 --- a/kolla/image/build.py +++ b/kolla/image/build.py @@ -71,16 +71,17 @@ STATUS_UNMATCHED = 'unmatched' STATUS_MATCHED = 'matched' STATUS_UNPROCESSED = 'unprocessed' STATUS_SKIPPED = 'skipped' +STATUS_UNBUILDABLE = 'unbuildable' # All error status constants. STATUS_ERRORS = (STATUS_CONNECTION_ERROR, STATUS_PUSH_ERROR, STATUS_ERROR, STATUS_PARENT_ERROR) -# The dictionary of skipped images supports keys in the format: +# The dictionary of unbuildable images supports keys in the format: # '++' where each component is optional # and can be omitted along with the + separator which means that component # is irrelevant. Otherwise all must match for skip to happen. -SKIPPED_IMAGES = { +UNBUILDABLE_IMAGES = { 'aarch64': { "cyborg-base", # no binary package "kibana", # no binary package @@ -505,7 +506,7 @@ class BuildTask(DockerTask): self.logger.debug('Processing') - if image.status == STATUS_SKIPPED: + if image.status in [STATUS_SKIPPED, STATUS_UNBUILDABLE]: self.logger.info('Skipping %s' % image.name) return @@ -741,6 +742,7 @@ class KollaWorker(object): self.image_statuses_good = dict() self.image_statuses_unmatched = dict() self.image_statuses_skipped = dict() + self.image_statuses_unbuildable = dict() self.maintainer = conf.maintainer self.distro_python_version = conf.distro_python_version @@ -1000,44 +1002,80 @@ class KollaWorker(object): else: filter_ += self.conf.profiles[profile] - if filter_: - patterns = re.compile(r"|".join(filter_).join('()')) - for image in self.images: - if image.status in (STATUS_MATCHED, STATUS_SKIPPED): - continue - if re.search(patterns, image.name): - image.status = STATUS_MATCHED - while (image.parent is not None and - image.parent.status not in (STATUS_MATCHED, - STATUS_SKIPPED)): - image = image.parent - if self.conf.skip_parents: - image.status = STATUS_SKIPPED - elif (self.conf.skip_existing and - image.in_docker_cache()): - image.status = STATUS_SKIPPED - else: - image.status = STATUS_MATCHED - LOG.debug('Image %s matched regex', image.name) - else: - image.status = STATUS_UNMATCHED - else: - for image in self.images: - image.status = STATUS_MATCHED - - # Unmatch (skip) unsupported images + # mark unbuildable images and their children tag_element = r'(%s|%s|%s)' % (self.base, self.install_type, self.base_arch) tag_re = re.compile(r'^%s(\+%s)*$' % (tag_element, tag_element)) - skipped_images = set() - for set_tag in SKIPPED_IMAGES: + unbuildable_images = set() + for set_tag in UNBUILDABLE_IMAGES: if tag_re.match(set_tag): - skipped_images.update(SKIPPED_IMAGES[set_tag]) - if skipped_images: + unbuildable_images.update(UNBUILDABLE_IMAGES[set_tag]) + + if unbuildable_images: for image in self.images: - if image.name in skipped_images: + if image.name in unbuildable_images: + image.status = STATUS_UNBUILDABLE + else: + # let's check ancestors + # if any of them is unbuildable then we mark it + # and then mark image + build_image = True + ancestor_image = image + while (ancestor_image.parent is not None): + ancestor_image = ancestor_image.parent + if ancestor_image.name in unbuildable_images or \ + ancestor_image.status == STATUS_UNBUILDABLE: + build_image = False + ancestor_image.status = STATUS_UNBUILDABLE + break + if not build_image: + image.status = STATUS_UNBUILDABLE + + # When we want to build a subset of images then filter_ part kicks in. + # Otherwise we just mark everything buildable as matched for build. + + if filter_: + patterns = re.compile(r"|".join(filter_).join('()')) + for image in self.images: + # as we now list not buildable/skipped images we need to + # process them otherwise list will contain also not requested + # entries + if image.status == STATUS_MATCHED: + continue + if re.search(patterns, image.name): + if image.status not in [STATUS_SKIPPED, + STATUS_UNBUILDABLE]: + image.status = STATUS_MATCHED + + # skip image if --skip-existing was given and image + # was already built + if (self.conf.skip_existing and image.in_docker_cache()): + image.status = STATUS_SKIPPED + + # handle image ancestors + ancestor_image = image + while (ancestor_image.parent is not None and + ancestor_image.parent.status not in + (STATUS_MATCHED, STATUS_SKIPPED)): + ancestor_image = ancestor_image.parent + if self.conf.skip_parents: + ancestor_image.status = STATUS_SKIPPED + elif (self.conf.skip_existing and + ancestor_image.in_docker_cache()): + ancestor_image.status = STATUS_SKIPPED + else: + if ancestor_image.status != STATUS_UNBUILDABLE: + ancestor_image.status = STATUS_MATCHED + LOG.debug('Image %s matched regex', image.name) + else: + # we do not care if it is skipped or not as we did not + # request it image.status = STATUS_UNMATCHED + else: + for image in self.images: + if image.status != STATUS_UNBUILDABLE: + image.status = STATUS_MATCHED def summary(self): """Walk the dictionary of images statuses and print results.""" @@ -1053,6 +1091,7 @@ class KollaWorker(object): 'failed': [], 'not_matched': [], 'skipped': [], + 'unbuildable': [], } if self.image_statuses_good: @@ -1097,26 +1136,38 @@ class KollaWorker(object): }) if self.image_statuses_skipped: - LOG.debug("================================") - LOG.debug("Images skipped due build options") - LOG.debug("================================") + LOG.info("===================================") + LOG.info("Images skipped due to build options") + LOG.info("===================================") for name in sorted(self.image_statuses_skipped.keys()): - LOG.debug(name) + LOG.info(name) results['skipped'].append({ 'name': name, }) + if self.image_statuses_unbuildable: + LOG.info("=========================================") + LOG.info("Images not buildable due to build options") + LOG.info("=========================================") + for name in sorted(self.image_statuses_unbuildable.keys()): + LOG.info(name) + results['unbuildable'].append({ + 'name': name, + }) + return results def get_image_statuses(self): if any([self.image_statuses_bad, self.image_statuses_good, self.image_statuses_unmatched, - self.image_statuses_skipped]): + self.image_statuses_skipped, + self.image_statuses_unbuildable]): return (self.image_statuses_bad, self.image_statuses_good, self.image_statuses_unmatched, - self.image_statuses_skipped) + self.image_statuses_skipped, + self.image_statuses_unbuildable) for image in self.images: if image.status == STATUS_BUILT: self.image_statuses_good[image.name] = image.status @@ -1124,12 +1175,15 @@ class KollaWorker(object): self.image_statuses_unmatched[image.name] = image.status elif image.status == STATUS_SKIPPED: self.image_statuses_skipped[image.name] = image.status + elif image.status == STATUS_UNBUILDABLE: + self.image_statuses_unbuildable[image.name] = image.status else: self.image_statuses_bad[image.name] = image.status return (self.image_statuses_bad, self.image_statuses_good, self.image_statuses_unmatched, - self.image_statuses_skipped) + self.image_statuses_skipped, + self.image_statuses_unbuildable) def build_image_list(self): def process_source_installation(image, section): @@ -1285,7 +1339,8 @@ class KollaWorker(object): queue = six.moves.queue.Queue() for image in self.images: - if image.status in (STATUS_UNMATCHED, STATUS_SKIPPED): + if image.status in (STATUS_UNMATCHED, STATUS_SKIPPED, + STATUS_UNBUILDABLE): # Don't bother queuing up build tasks for things that # were not matched in the first place... (not worth the # effort to run them, if they won't be used anyway). diff --git a/kolla/tests/test_build.py b/kolla/tests/test_build.py index ec4ab8f54e..72a35029f2 100644 --- a/kolla/tests/test_build.py +++ b/kolla/tests/test_build.py @@ -541,14 +541,14 @@ class MainTest(base.TestCase): @mock.patch.object(build, 'run_build') def test_images_built(self, mock_run_build): - image_statuses = ({}, {'img': 'built'}, {}, {}) + image_statuses = ({}, {'img': 'built'}, {}, {}, {}) mock_run_build.return_value = image_statuses result = build_cmd.main() self.assertEqual(0, result) @mock.patch.object(build, 'run_build') def test_images_unmatched(self, mock_run_build): - image_statuses = ({}, {}, {'img': 'unmatched'}, {}) + image_statuses = ({}, {}, {'img': 'unmatched'}, {}, {}) mock_run_build.return_value = image_statuses result = build_cmd.main() self.assertEqual(0, result) @@ -561,7 +561,7 @@ class MainTest(base.TestCase): @mock.patch.object(build, 'run_build') def test_bad_images(self, mock_run_build): - image_statuses = ({'img': 'error'}, {}, {}, {}) + image_statuses = ({'img': 'error'}, {}, {}, {}, {}) mock_run_build.return_value = image_statuses result = build_cmd.main() self.assertEqual(1, result) @@ -574,7 +574,14 @@ class MainTest(base.TestCase): @mock.patch.object(build, 'run_build') def test_skipped_images(self, mock_run_build): - image_statuses = ({}, {}, {}, {'img': 'skipped'}) + image_statuses = ({}, {}, {}, {'img': 'skipped'}, {}) + mock_run_build.return_value = image_statuses + result = build_cmd.main() + self.assertEqual(0, result) + + @mock.patch.object(build, 'run_build') + def test_unbuildable_images(self, mock_run_build): + image_statuses = ({}, {}, {}, {}, {'img': 'unbuildable'}) mock_run_build.return_value = image_statuses result = build_cmd.main() self.assertEqual(0, result) diff --git a/tests/test_build.py b/tests/test_build.py index d32188b55d..d952089688 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -48,11 +48,11 @@ class BuildTest(object): with patch.object(sys, 'argv', self.build_args): LOG.info("Running with args %s", self.build_args) (bad_results, good_results, unmatched_results, - skipped_results) = build.run_build() + skipped_results, unbuildable_results) = build.run_build() failures = 0 for image, result in bad_results.items(): - if result is not 'error': + if result != 'error': continue failures = failures + 1 LOG.critical(">>> Expected image '%s' to succeed!", image)