diff --git a/nodepool/builder.py b/nodepool/builder.py index c654271cb..e1f92faed 100644 --- a/nodepool/builder.py +++ b/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' diff --git a/nodepool/status.py b/nodepool/status.py index 92cfe7b84..c99b731f4 100644 --- a/nodepool/status.py +++ b/nodepool/status.py @@ -190,14 +190,15 @@ 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) - state = paused and 'paused' or build.state - objs.append({'id': '-'.join([image_name, build_no]), - 'image': image_name, - 'builder': build.builder, - 'formats': build.formats, - 'state': state, - 'age': int(build.state_time) - }) + if build: + state = paused and 'paused' or build.state + objs.append({'id': '-'.join([image_name, build_no]), + 'image': image_name, + 'builder': build.builder, + 'formats': build.formats, + 'state': state, + 'age': int(build.state_time) + }) return (objs, headers_table) diff --git a/nodepool/tests/unit/test_zk.py b/nodepool/tests/unit/test_zk.py index fc7316919..39930cd5c 100644 --- a/nodepool/tests/unit/test_zk.py +++ b/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())} diff --git a/nodepool/zk.py b/nodepool/zk.py index 8f5704319..5fb358a10 100644 --- a/nodepool/zk.py +++ b/nodepool/zk.py @@ -1274,7 +1274,12 @@ class ZooKeeper(object): except kze.NoNodeError: return None - d = ImageBuild.fromDict(self._bytesToDict(data), build_number) + 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