Remove builds that are no longer in use

When a build is no longer used, make sure that all of its uploads
are deleted, then the build itself.

Change-Id: I56fc6677c27fa653889b0720a5d9b06aac32612f

nodepool/builder.py

@@ -310,16 +310,31 @@ class CleanupWorker(BaseWorker):
             except Exception:
                 self.log.exception("Exception cleaning up image %s:", image)
 
+    def _filterLocalBuilds(self, image, builds):
+        '''Return the subset of builds that are local'''
+        ret = []
+        for build in builds:
+            base = "-".join([image, build.id])
+            files = DibImageFile.from_image_id(self._config.imagesdir, base)
+            if files:
+                ret.append(build)
+        return ret
+
     def _cleanupImage(self, known_providers, image):
         '''
         Clean up one image.
         '''
-        # Get the list of all builds before we get the list of
-        # builds to keep.  That way, if a build transitions to
-        # ready between the two calls, it will show up in the list
-        # of builds to keep.
+        # Get the list of all builds, then work from that so that we
+        # have a consistent view of the data.
         all_builds = self._zk.getBuilds(image)
-        builds_to_keep = self._zk.getMostRecentBuilds(2, image, 'ready')
+        builds_to_keep = set([b for b in sorted(all_builds) if b.state=='ready'][:2])
+        local_builds = set(self._filterLocalBuilds(image, all_builds))
+        # remove any local builds that are not in use
+        if image not in self._config.images_in_use:
+            builds_to_keep -= local_builds
+            # TODO(jeblair): When all builds for an image which is not
+            # in use are deleted, the image znode should be deleted as
+            # well.
 
         for build in all_builds:
             # Start by deleting any uploads that are no longer needed
@@ -343,7 +358,7 @@ class CleanupWorker(BaseWorker):
                 # if it is older than the most recent two ready
                 # builds, or is in the building state but not actually
                 # building.
-                if build.id in [b.id for b in builds_to_keep]:
+                if build in builds_to_keep:
                     continue
                 elif self._inProgressBuild(build, image):
                     continue
@@ -437,6 +452,9 @@ class BuildWorker(BaseWorker):
         .. note:: It's important to lock the image build before we check
             the state time and then build to eliminate any race condition.
         '''
+        if diskimage.name not in self._config.images_in_use:
+            return
+
         now = int(time.time())
         builds = self._zk.getMostRecentBuilds(1, diskimage.name, 'ready')
 

nodepool/tests/fixtures/node_two_image_remove.yaml

@@ -0,0 +1,72 @@
+script-dir: .
+elements-dir: .
+images-dir: '{images_dir}'
+
+cron:
+  check: '*/15 * * * *'
+  cleanup: '*/1 * * * *'
+
+zmq-publishers:
+  - tcp://localhost:8881
+
+gearman-servers:
+  - host: localhost
+    port: {gearman_port}
+
+zookeeper-servers:
+  - host: {zookeeper_host}
+    port: {zookeeper_port}
+    chroot: {zookeeper_chroot}
+
+labels:
+  - name: fake-label
+    image: fake-image
+    min-ready: 1
+    providers:
+      - name: fake-provider
+
+providers:
+  - name: fake-provider
+    region-name: fake-region
+    keypair: 'if-present-use-this-keypair'
+    username: 'fake'
+    password: 'fake'
+    auth-url: 'fake'
+    project-id: 'fake'
+    max-servers: 96
+    pool: 'fake'
+    networks:
+      - net-id: 'some-uuid'
+    rate: 0.0001
+    images:
+      - name: fake-image
+        min-ram: 8192
+        name-filter: 'Fake'
+        meta:
+          key: value
+          key2: value
+
+targets:
+  - name: fake-target
+
+diskimages:
+  - name: fake-image
+    elements:
+      - fedora
+      - vm
+    release: 21
+    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
+  - name: fake-image2
+    elements:
+      - fedora
+      - vm
+    release: 21
+    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/test_builder.py

@@ -164,3 +164,12 @@ class TestNodePoolBuilder(tests.DBTestCase):
         self.waitForImage('fake-provider', 'fake-image')
         self.replace_config(configfile, 'node_two_image.yaml')
         self.waitForImage('fake-provider', 'fake-image2')
+
+    def test_image_removal(self):
+        configfile = self.setup_config('node_two_image.yaml')
+        self._useBuilder(configfile)
+        self.waitForImage('fake-provider', 'fake-image')
+        self.waitForImage('fake-provider', 'fake-image2')
+        self.replace_config(configfile, 'node_two_image_remove.yaml')
+        self.waitForImageDeletion('fake-provider', 'fake-image2')
+        self.waitForBuildDeletion('fake-image2', '0000000001')