Merge "Ignore unparsable/empty image build ZNode data"

nodepool/builder.py

@@ -368,7 +368,7 @@ class CleanupWorker(BaseWorker):
                 # still in progress so that it is checked again later with
                 # its new build state.
                 b = self._zk.getBuild(image, build.id)
-                if b.state != zk.BUILDING:
+                if b and b.state != zk.BUILDING:
                     return True
                 pass
         except exceptions.ZKLockException:
@@ -780,7 +780,7 @@ class BuildWorker(BaseWorker):
                 data = self._buildWrapper(diskimage)
 
                 # Remove request on a successful build
-                if data.state == zk.READY:
+                if data and data.state == zk.READY:
                     self._zk.removeBuildRequest(diskimage.name)
 
         except exceptions.ZKLockException:
@@ -1256,7 +1256,7 @@ class UploadWorker(BaseWorker):
                 # that another thread isn't trying to delete this build just
                 # before we upload.
                 b = self._zk.getBuild(image.name, build.id)
-                if b.state == zk.DELETING:
+                if not b or b.state == zk.DELETING:
                     return False
 
                 # New upload number with initial state 'uploading'

nodepool/status.py

@@ -190,6 +190,7 @@ def dib_image_list(zk):
         paused = zk.getImagePaused(image_name)
         for build_no in zk.getBuildNumbers(image_name):
             build = zk.getBuild(image_name, build_no)
+            if build:
                 state = paused and 'paused' or build.state
                 objs.append({'id': '-'.join([image_name, build_no]),
                              'image': image_name,

nodepool/tests/unit/test_zk.py

@@ -326,6 +326,23 @@ class TestZooKeeper(tests.DBTestCase):
         self.zk.removeBuildRequest(image)
         self.assertFalse(self.zk.hasBuildRequest(image))
 
+    def test_buildLock_orphan(self):
+        image = "ubuntu-trusty"
+        build_number = "0000000003"
+
+        path = self.zk._imageBuildNumberLockPath(image, build_number)
+
+        # Pretend we still think the image build exists
+        # (e.g. multiple cleanup workers itertating over builds)
+        with self.zk.imageBuildNumberLock(image, build_number, blocking=False):
+            # We now created an empty build number node
+            pass
+
+        self.assertIsNotNone(self.zk.client.exists(path))
+
+        # Should not throw an exception because of the empty upload
+        self.assertIsNone(self.zk.getBuild(image, build_number))
+
     def test_getMostRecentBuilds(self):
         image = "ubuntu-trusty"
         v1 = {'state': zk.READY, 'state_time': int(time.time())}

nodepool/zk.py

@@ -1274,7 +1274,12 @@ class ZooKeeper(object):
         except kze.NoNodeError:
             return None
 
+        try:
             d = ImageBuild.fromDict(self._bytesToDict(data), build_number)
+        except json.decoder.JSONDecodeError:
+            self.log.exception('Error loading json data from image build %s',
+                               path)
+            return None
         d.stat = stat
         return d