Merge "Add delete-after-upload option"

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

nodepool/builder.py

@@ -56,6 +56,17 @@ STATUS_UPLOADING = 2
 STATUS_PAUSED = 3
 
 
+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
@@ -248,16 +259,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:
@@ -273,8 +274,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)
@@ -396,6 +397,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)
@@ -413,6 +416,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
 

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,

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

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

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'

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

nodepool/tests/unit/test_builder.py

@@ -593,3 +593,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)

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.