From 8fc590ef87a72f848cd8b9fc3c37f5896a099385 Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Tue, 22 Nov 2016 10:57:55 -0500 Subject: [PATCH] Re-enable alien-image-list command and tests Changes the alien-image-list command to compare provider images and those recorded in ZooKeeper. Re-enables the test_alient_image_list_fail test and adds a new test (test_alien_image_list_empty) to verify normal operation. Add build and upload information to image metadata This way we can track back an image in the cloud to the image in zk, and also filter out images in alien list. NOTE: I'm unclear as to how to artificially create an alien image in FakeProvider to test the "alien image found" case, but the pathway is there to add a test for it if that is, indeed, possible. Change-Id: Ib68e05edccd0f7aac10593edae0a7098a6f30de0 --- nodepool/builder.py | 12 +++++-- nodepool/cmd/nodepoolcmd.py | 59 ++++++++++++++++++++++----------- nodepool/tests/test_commands.py | 16 +++++++-- 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/nodepool/builder.py b/nodepool/builder.py index c6d1282a1..aa7c1966e 100644 --- a/nodepool/builder.py +++ b/nodepool/builder.py @@ -694,11 +694,12 @@ class UploadWorker(BaseWorker): super(UploadWorker, self).__init__(config_path) self.log = logging.getLogger("nodepool.builder.UploadWorker.%s" % name) - def _uploadImage(self, build_id, image_name, images, provider): + def _uploadImage(self, build_id, upload_id, image_name, images, provider): ''' Upload a local DIB image build to a provider. :param str build_id: Unique ID of the image build to upload. + :param str upload_id: Unique ID of the upload. :param str image_name: Name of the diskimage. :param list images: A list of DibImageFile objects from this build that available for uploading. @@ -735,17 +736,22 @@ class UploadWorker(BaseWorker): (build_id, filename, provider.name)) manager = self._config.provider_managers[provider.name] + provider_image = provider.images.get(image_name) if provider_image is None: raise exceptions.BuilderInvalidCommandError( "Could not find matching provider image for %s" % image_name ) + meta = provider_image.meta.copy() + meta['nodepool_build_id'] = build_id + meta['nodepool_upload_id'] = upload_id + try: external_id = manager.uploadImage( ext_image_name, filename, image_type=image.extension, - meta=provider_image.meta + meta=meta ) except Exception: self.log.exception("Failed to upload image %s to provider %s" % @@ -844,7 +850,7 @@ class UploadWorker(BaseWorker): upnum = self._zk.storeImageUpload( image.name, build.id, provider.name, data) - data = self._uploadImage(build.id, image.name, + data = self._uploadImage(build.id, upnum, image.name, local_images, provider) # Set final state diff --git a/nodepool/cmd/nodepoolcmd.py b/nodepool/cmd/nodepoolcmd.py index fc10d97b2..8c558e7b4 100644 --- a/nodepool/cmd/nodepoolcmd.py +++ b/nodepool/cmd/nodepoolcmd.py @@ -204,26 +204,47 @@ class NodePoolCmd(NodepoolApp): t = PrettyTable(["Provider", "Name", "Image ID"]) t.align = 'l' - with self.pool.getDB().getSession() as session: - for provider in self.pool.config.providers.values(): - if (self.args.provider and - provider.name != self.args.provider): - continue - manager = self.pool.getProviderManager(provider) - images = [] - try: - images = manager.listImages() - except Exception as e: - log.warning("Exception listing alien images for %s: %s" - % (provider.name, str(e.message))) + for provider in self.pool.config.providers.values(): + if (self.args.provider and + provider.name != self.args.provider): + continue + manager = self.pool.getProviderManager(provider) + + # Build list of provider images as known by the provider + provider_images = [] + try: + # Only consider images marked as managed by nodepool. + # Prevent cloud-provider images from showing + # up in alien list since we can't do anything about them + # anyway. + provider_images = [ + image for image in manager.listImages() + if 'nodepool_build_id' in image['properties']] + except Exception as e: + log.warning("Exception listing alien images for %s: %s" + % (provider.name, str(e.message))) + + alien_ids = [] + uploads = [] + for image in provider_images: + # Build list of provider images as recorded in ZK + for bnum in self.zk.getBuildNumbers(image['name']): + uploads.extend( + self.zk.getUploads(image['name'], bnum, + provider.name, + states=[zk.READY]) + ) + + # Calculate image IDs present in the provider, but not in ZK + provider_image_ids = set([img['id'] for img in provider_images]) + zk_image_ids = set([img.external_id for img in uploads]) + alien_ids = provider_image_ids - zk_image_ids + + for image in provider_images: + if image['id'] in alien_ids: + t.add_row([provider.name, image['name'], image['id']]) - for image in images: - if image['metadata'].get('image_type') == 'snapshot': - if not session.getSnapshotImageByExternalID( - provider.name, image['id']): - t.add_row([provider.name, image['name'], - image['id']]) print t def hold(self): @@ -320,7 +341,7 @@ class NodePoolCmd(NodepoolApp): # commands needing ZooKeeper if self.args.command in ('image-build', 'dib-image-list', 'image-list', 'dib-image-delete', - 'image-delete'): + 'image-delete', 'alien-image-list'): self.zk = zk.ZooKeeper() self.zk.connect(config.zookeeper_servers.values()) else: diff --git a/nodepool/tests/test_commands.py b/nodepool/tests/test_commands.py index d79e00461..4df6cfd0c 100644 --- a/nodepool/tests/test_commands.py +++ b/nodepool/tests/test_commands.py @@ -16,7 +16,6 @@ import logging import os.path import sys # noqa making sure its available for monkey patching -from unittest import skip import fixtures import mock @@ -46,6 +45,12 @@ class TestNodepoolCMD(tests.DBTestCase): rows_with_val += 1 self.assertEquals(rows_with_val, count) + def assert_alien_images_listed(self, configfile, image_cnt, image_id): + self.assert_listed(configfile, ['alien-image-list'], 2, image_id, image_cnt) + + def assert_alien_images_empty(self, configfile): + self.assert_alien_images_listed(configfile, 0, 0) + def assert_images_listed(self, configfile, image_cnt, status="ready"): self.assert_listed(configfile, ['image-list'], 6, status, image_cnt) @@ -88,7 +93,14 @@ class TestNodepoolCMD(tests.DBTestCase): self.patch_argv("-c", configfile, "alien-list") nodepoolcmd.main() - @skip("Skipping until ZooKeeper is enabled") + def test_alien_image_list_empty(self): + configfile = self.setup_config("node.yaml") + self._useBuilder(configfile) + self.waitForImage('fake-provider', 'fake-image') + self.patch_argv("-c", configfile, "alien-image-list") + nodepoolcmd.main() + self.assert_alien_images_empty(configfile) + def test_alien_image_list_fail(self): def fail_list(self): raise RuntimeError('Fake list error')