From fd454706caade41b4863b3b5beee0c696a358248 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 26 Feb 2024 16:14:59 -0800 Subject: [PATCH] Add delete-after-upload option This allows operators to delete large diskimage files after uploads are complete, in order to save space. A setting is also provided to keep certain formats, so that if operators would like to delete large formats such as "raw" while retaining a qcow2 copy (which, in an emergency, could be used to inspect the image, or manually converted and uploaded for use), that is possible. Change-Id: I97ca3422044174f956d6c5c3c35c2dbba9b4cadf --- doc/source/configuration.rst | 23 ++++++ nodepool/builder.py | 67 +++++++++++++--- nodepool/cmd/config_validator.py | 2 + nodepool/config.py | 8 +- nodepool/driver/fake/adapter.py | 6 +- nodepool/tests/fixtures/clouds.yaml | 16 ++++ .../node_diskimage_cleanup_formats.yaml | 77 +++++++++++++++++++ nodepool/tests/unit/test_builder.py | 29 +++++++ .../delete-after-upload-0d6ec8691437ce9f.yaml | 8 ++ 9 files changed, 222 insertions(+), 14 deletions(-) create mode 100644 nodepool/tests/fixtures/node_diskimage_cleanup_formats.yaml create mode 100644 releasenotes/notes/delete-after-upload-0d6ec8691437ce9f.yaml diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 8723e4002..a24a27b41 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -358,6 +358,29 @@ Options In case the diskimage is not used by any provider and no formats are configured, the image won't be built. + .. attr:: delete-after-upload + :type: bool + :default: False + + When set to ``True``, the builder will delete the disk image + file from disk after all uploads are complete. If more than one + format is built, this is performed individually for each format + (so that, for example, if all `vhd` uploads are complete but not + `qcow2`, then the `qcow2` files will remain while `vhd` are + deleted). + + The diskimage manifest directories are retained as long as any + uploaded image is present. + + .. attr:: keep-formats + :type: list + + If :attr:`diskimages.delete-after-upload` is set, this setting + may be used to retain images of certain formats even after their + uploads are complete. For example, setting this value to + `["qcow2"]` will retain `qcow2` images while deleting all other + formats. + .. attr:: rebuild-age :type: int :default: 86400 diff --git a/nodepool/builder.py b/nodepool/builder.py index 0aca542f9..bf21f9188 100644 --- a/nodepool/builder.py +++ b/nodepool/builder.py @@ -50,6 +50,17 @@ DEFAULT_QEMU_IMAGE_COMPAT_OPTIONS = "--qemu-img-options 'compat=0.10'" BUILD_PROCESS_POLL_TIMEOUT = 30 * 1000 +def removeDibItem(filename, log): + if filename is None: + return + try: + os.remove(filename) + log.info("Removed DIB file %s" % filename) + except OSError as e: + if e.errno != 2: # No such file or directory + raise e + + class DibImageFile(object): ''' Class used as an API to finding locally built DIB image files, and @@ -239,16 +250,6 @@ class CleanupWorker(BaseWorker): :param ImageBuild build: The build we want to delete. :param Logger log: A logging object for log output. ''' - def removeDibItem(filename): - if filename is None: - return - try: - os.remove(filename) - log.info("Removed DIB file %s" % filename) - except OSError as e: - if e.errno != 2: # No such file or directory - raise e - base = "-".join([image_name, build_id]) files = DibImageFile.from_image_id(images_dir, base) if not files: @@ -264,8 +265,8 @@ class CleanupWorker(BaseWorker): path, ext = filename.rsplit('.', 1) manifest_dir = path + ".d" items = [filename, f.md5_file, f.sha256_file] - list(map(removeDibItem, items)) - + for item in items: + removeDibItem(item, log) try: shutil.rmtree(manifest_dir) log.info("Removed DIB manifest %s" % manifest_dir) @@ -387,6 +388,8 @@ class CleanupWorker(BaseWorker): self.log.info("Deleting on-disk build: " "%s-%s", image_name, build_id) self._deleteLocalBuild(image_name, build_id) + self._pruneLocalBuildFormats( + known_providers, image_name, build_id) except Exception: self.log.exception("Exception cleaning up local build %s:", local_build) @@ -404,6 +407,46 @@ class CleanupWorker(BaseWorker): seen.add(name_and_id) yield name_and_id + def _pruneLocalBuildFormats(self, providers, image_name, build_id): + '''If the diskimage is configured to delete local builds afetr + upload, this method will do so. + + For this to happen, the diskimage must have the + delete-after-upload option set. We then check each format and + delete it only if the upload of that format is complete for + all providers and we are not configuret to keep that format + with keep-formats. + + ''' + images_dir = self._config.images_dir + diskimage = self._config.diskimages.get(image_name) + if not diskimage.delete_after_upload: + return + to_keep = set(diskimage.keep_image_types) + # Examine the currently uploaded images and determine which + # formats need to be kept for future uploads. + for provider in providers: + if not provider.manage_images: + continue + if image_name not in provider.diskimages: + continue + upload = self._zk.getMostRecentBuildImageUploads( + 1, image_name, build_id, provider.name, zk.READY) + if not upload: + to_keep.add(provider.image_type) + + base = "-".join([image_name, build_id]) + files = DibImageFile.from_image_id(images_dir, base) + if not files: + return + + for f in files: + if f.extension in to_keep: + continue + image_path = f.to_path(images_dir) + for item in [image_path, f.md5_file, f.sha256_file]: + removeDibItem(item, self.log) + def _emitBuildRequestStats(self): '''Emit general build request stats diff --git a/nodepool/cmd/config_validator.py b/nodepool/cmd/config_validator.py index 866e861b5..fd61ab862 100644 --- a/nodepool/cmd/config_validator.py +++ b/nodepool/cmd/config_validator.py @@ -42,6 +42,8 @@ class ConfigValidator: 'pause': bool, 'elements': [str], 'formats': [str], + 'keep-formats': [str], + 'delete-after-upload': bool, 'parent': str, 'release': v.Any(str, int), 'rebuild-age': int, diff --git a/nodepool/config.py b/nodepool/config.py index 29838f0cf..b0e0f0be9 100644 --- a/nodepool/config.py +++ b/nodepool/config.py @@ -305,7 +305,9 @@ class DiskImage(ConfigValue): self.dib_cmd = 'disk-image-create' self.elements = [] self.env_vars = {} - self.image_types = set([]) + self.image_types = set() + self.delete_after_upload = False + self.keep_image_types = set() self.pause = False self.python_path = 'auto' self.shell_type = None @@ -343,6 +345,10 @@ class DiskImage(ConfigValue): image_types = config.get('formats', None) if image_types: self.image_types = set(image_types) + keep_image_types = config.get('keep-formats', None) + if keep_image_types: + self.keep_image_types = set(keep_image_types) + self.delete_after_upload = config.get('delete-after-upload', False) pause = config.get('pause', None) if pause: self.pause = pause diff --git a/nodepool/driver/fake/adapter.py b/nodepool/driver/fake/adapter.py index 78bfa70c2..040559471 100644 --- a/nodepool/driver/fake/adapter.py +++ b/nodepool/driver/fake/adapter.py @@ -410,12 +410,16 @@ class FakeOpenStackCloud(object): class FakeUploadFailCloud(FakeOpenStackCloud): log = logging.getLogger("nodepool.FakeUploadFailCloud") - def __init__(self, *args, times_to_fail=None, **kw): + def __init__(self, *args, times_to_fail=None, fail_callback=None, **kw): super(FakeUploadFailCloud, self).__init__(*args, **kw) self.times_to_fail = times_to_fail self.times_failed = 0 + self.fail_callback = fail_callback def create_image(self, **kwargs): + if self.fail_callback: + if not self.fail_callback(kwargs): + return super(FakeUploadFailCloud, self).create_image(**kwargs) if self.times_to_fail is None: raise exceptions.BuilderError("Test fail image upload.") self.times_failed += 1 diff --git a/nodepool/tests/fixtures/clouds.yaml b/nodepool/tests/fixtures/clouds.yaml index faebc88d8..848dd34d9 100644 --- a/nodepool/tests/fixtures/clouds.yaml +++ b/nodepool/tests/fixtures/clouds.yaml @@ -13,3 +13,19 @@ clouds: project_id: 'fake' auth_url: 'fake' image_format: 'vhd' + + fake-qcow2: + auth: + username: 'fake' + password: 'fake' + project_id: 'fake' + auth_url: 'fake' + image_format: 'qcow2' + + fake-raw: + auth: + username: 'fake' + password: 'fake' + project_id: 'fake' + auth_url: 'fake' + image_format: 'raw' diff --git a/nodepool/tests/fixtures/node_diskimage_cleanup_formats.yaml b/nodepool/tests/fixtures/node_diskimage_cleanup_formats.yaml new file mode 100644 index 000000000..f266774af --- /dev/null +++ b/nodepool/tests/fixtures/node_diskimage_cleanup_formats.yaml @@ -0,0 +1,77 @@ +elements-dir: . +images-dir: '{images_dir}' +build-log-dir: '{build_log_dir}' + +zookeeper-servers: + - host: {zookeeper_host} + port: {zookeeper_port} + chroot: {zookeeper_chroot} + +zookeeper-tls: + ca: {zookeeper_ca} + cert: {zookeeper_cert} + key: {zookeeper_key} + +labels: + - name: fake-label + min-ready: 1 + +providers: + - name: fake-provider-qcow2 + cloud: fake-qcow2 + driver: fake + region-name: fake-region + rate: 0.0001 + diskimages: + - name: fake-image + pools: + - name: main + max-servers: 96 + labels: + - name: fake-label + diskimage: fake-image + min-ram: 8192 + - name: fake-provider-raw + cloud: fake-raw + driver: fake + region-name: fake-region + rate: 0.0001 + diskimages: + - name: fake-image + pools: + - name: main + max-servers: 96 + labels: + - name: fake-label + diskimage: fake-image + min-ram: 8192 + - name: fake-provider-vhd-fail + cloud: fake-vhd + driver: fake + region-name: fake-region + rate: 0.0001 + diskimages: + - name: fake-image + pools: + - name: main + max-servers: 96 + labels: + - name: fake-label + diskimage: fake-image + min-ram: 8192 + +diskimages: + - name: fake-image + delete-after-upload: true + keep-formats: + - qcow2 + elements: + - fedora + - vm + release: 21 + dib-cmd: nodepool/tests/fake-image-create + env-vars: + TMPDIR: /opt/dib_tmp + DIB_IMAGE_CACHE: /opt/dib_cache + DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/ + BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2 diff --git a/nodepool/tests/unit/test_builder.py b/nodepool/tests/unit/test_builder.py index 25f88e3e2..b13b54658 100644 --- a/nodepool/tests/unit/test_builder.py +++ b/nodepool/tests/unit/test_builder.py @@ -584,3 +584,32 @@ class TestNodePoolBuilder(tests.DBTestCase): post_file = os.path.join( images_dir, f'fake-image-{image.build_id}.qcow2.post') self.assertTrue(os.path.exists(post_file), 'Post hook file exists') + + def test_cleanup_image_format(self): + def fail_callback(args): + return 'fail' in args['nodepool_provider_name'] + + fake_client = fakeadapter.FakeUploadFailCloud( + fail_callback=fail_callback) + + def get_fake_client(*args, **kwargs): + return fake_client + + self.useFixture(fixtures.MockPatchObject( + fakeadapter.FakeAdapter, '_getClient', + get_fake_client)) + + configfile = self.setup_config('node_diskimage_cleanup_formats.yaml') + self.useBuilder(configfile) + build = self.waitForBuild('fake-image', check_files=False) + self.waitForImage('fake-provider-qcow2', 'fake-image') + self.waitForImage('fake-provider-raw', 'fake-image') + + self.assertEqual(set(build._formats), set(['qcow2', 'raw', 'vhd'])) + base = "-".join(['fake-image', build.id]) + files = builder.DibImageFile.from_image_id( + self._config_images_dir.path, base) + self.assertEqual(2, len(files)) + files.sort(key=lambda x: x.extension) + self.assertEqual('qcow2', files[0].extension) + self.assertEqual('vhd', files[1].extension) diff --git a/releasenotes/notes/delete-after-upload-0d6ec8691437ce9f.yaml b/releasenotes/notes/delete-after-upload-0d6ec8691437ce9f.yaml new file mode 100644 index 000000000..45da1c70a --- /dev/null +++ b/releasenotes/notes/delete-after-upload-0d6ec8691437ce9f.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Nodepool-builder may now be configured to delete disk image files + after uploads are complete, but while the corresponding images are + still in service. The :attr:`diskimages.delete-after-upload` and + :attr:`diskimages.keep-formats` configuration options may be used + to configure this behavior.