Fix race in dib_image_list_json test

Change Ibff0a9016936b461eccb1b48dcf42f5ad8d8434e had an error that
was not caught by testing due to a race in the test code -- the
build might finish before the webapp starts.  This corrects the
test (and a few sibling tests) by starting the webapp first, then
starting the builder.

The actual error is also corrected: ImageBuild.updateFromDict is
implemented.  Further, duplicate fromDict method calls are removed
from several classes and renamed to updateFromDict so that the
APIs match.

Change-Id: I918d1badf838a23d6b0813b5b31dac4888d04ce0

nodepool/tests/unit/test_webapp.py

@@ -34,12 +34,15 @@ class TestWebApp(tests.DBTestCase):
     def test_image_list(self):
         configfile = self.setup_config('node.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
-        self.useBuilder(configfile)
         pool.start()
         webapp = self.useWebApp(pool, port=0)
         webapp.start()
         port = webapp.server.socket.getsockname()[1]
 
+        # Start the builder after the pool + webapp so they see the
+        # cache update
+        self.useBuilder(configfile)
+
         self.waitForImage('fake-provider', 'fake-image')
         self.waitForNodes('fake-label')
 
@@ -68,12 +71,15 @@ class TestWebApp(tests.DBTestCase):
     def test_image_list_filtered(self):
         configfile = self.setup_config('node.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
-        self.useBuilder(configfile)
         pool.start()
         webapp = self.useWebApp(pool, port=0)
         webapp.start()
         port = webapp.server.socket.getsockname()[1]
 
+        # Start the builder after the pool + webapp so they see the
+        # cache update
+        self.useBuilder(configfile)
+
         self.waitForImage('fake-provider', 'fake-image')
         self.waitForNodes('fake-label')
 
@@ -90,12 +96,15 @@ class TestWebApp(tests.DBTestCase):
     def test_image_list_json(self):
         configfile = self.setup_config('node.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
-        self.useBuilder(configfile)
         pool.start()
         webapp = self.useWebApp(pool, port=0)
         webapp.start()
         port = webapp.server.socket.getsockname()[1]
 
+        # Start the builder after the pool + webapp so they see the
+        # cache update
+        self.useBuilder(configfile)
+
         self.waitForImage('fake-provider', 'fake-image')
         self.waitForNodes('fake-label')
 
@@ -115,12 +124,15 @@ class TestWebApp(tests.DBTestCase):
     def test_dib_image_list_json(self):
         configfile = self.setup_config('node.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
-        self.useBuilder(configfile)
         pool.start()
         webapp = self.useWebApp(pool, port=0)
         webapp.start()
         port = webapp.server.socket.getsockname()[1]
 
+        # Start the builder after the pool + webapp so they see the
+        # cache update
+        self.useBuilder(configfile)
+
         self.waitForImage('fake-provider', 'fake-image')
         self.waitForNodes('fake-label')
 
@@ -141,17 +153,18 @@ class TestWebApp(tests.DBTestCase):
     def test_image_status_json(self):
         configfile = self.setup_config("node.yaml")
         pool = self.useNodepool(configfile, watermark_sleep=1)
-        builder = self.useBuilder(configfile)
-        # Make sure we have enough time to test for the build request
-        # before it's processed by the build worker.
-        for worker in builder._build_workers:
-            worker._interval = 60
 
         pool.start()
         webapp = self.useWebApp(pool, port=0)
         webapp.start()
         port = webapp.server.socket.getsockname()[1]
 
+        builder = self.useBuilder(configfile)
+        # Make sure we have enough time to test for the build request
+        # before it's processed by the build worker.
+        for worker in builder._build_workers:
+            worker._interval = 60
+
         self.waitForImage("fake-provider", "fake-image")
         self.waitForNodes('fake-label')
 

nodepool/zk/zookeeper.py

@@ -159,12 +159,9 @@ class BaseModel(Serializable):
         d['state_time'] = self.state_time
         return d
 
-    def fromDict(self, d):
+    def updateFromDict(self, d):
         '''
         Set base attributes based on the given dict.
-
-        Unlike the derived classes, this should NOT return an object as it
-        assumes self has already been instantiated.
         '''
         if 'state' in d:
             self.state = d['state']
@@ -241,16 +238,19 @@ class ImageBuild(BaseModel):
         :returns: An initialized ImageBuild object.
         '''
         o = ImageBuild(image_name, o_id)
-        super(ImageBuild, o).fromDict(d)
+        o.updateFromDict(d)
-        o.builder = d.get('builder')
+        return o
-        o.builder_id = d.get('builder_id')
+
-        o.username = d.get('username', 'zuul')
+    def updateFromDict(self, d):
-        o.python_path = d.get('python_path', '/usr/bin/python2')
+        super().updateFromDict(d)
-        o.shell_type = d.get('shell_type')
+        self.builder = d.get('builder')
+        self.builder_id = d.get('builder_id')
+        self.username = d.get('username', 'zuul')
+        self.python_path = d.get('python_path', '/usr/bin/python2')
+        self.shell_type = d.get('shell_type')
         # Only attempt the split on non-empty string
         if d.get('formats', ''):
-            o.formats = d.get('formats', '').split(',')
+            self.formats = d.get('formats', '').split(',')
-        return o
 
 
 class ImageBuildRequest(object):
@@ -351,12 +351,11 @@ class ImageUpload(BaseModel):
         :returns: An initialized ImageUpload object.
         '''
         o = ImageUpload(build_id, provider_name, image_name, upload_id)
-        super(ImageUpload, o).fromDict(d)
         o.updateFromDict(d)
         return o
 
     def updateFromDict(self, d):
-        super().fromDict(d)
+        super().updateFromDict(d)
         self.external_id = d.get('external_id')
         self.external_name = d.get('external_name')
         self.format = d.get('format')
@@ -468,12 +467,11 @@ class NodeRequest(BaseModel):
         :returns: An initialized NodeRequest object.
         '''
         o = NodeRequest(o_id)
-        super(NodeRequest, o).fromDict(d)
         o.updateFromDict(d)
         return o
 
     def updateFromDict(self, d):
-        super().fromDict(d)
+        super().updateFromDict(d)
         self.declined_by = d.get('declined_by', [])
         self.node_types = d.get('node_types', [])
         self.nodes = d.get('nodes', [])
@@ -661,8 +659,6 @@ class Node(BaseModel):
         :returns: An initialized Node object.
         '''
         o = Node(o_id)
-        super(Node, o).fromDict(d)
-
         o.updateFromDict(d)
         return o
 
@@ -672,7 +668,7 @@ class Node(BaseModel):
 
         :param dict d: The dictionary
         '''
-        super().fromDict(d)
+        super().updateFromDict(d)
         self.cloud = d.get('cloud')
         self.provider = d.get('provider')
         self.pool = d.get('pool')