Fixes a key comparison bug for slivertools (#239)

* Extracts name from key for comparison with sliver_tool_id.

* Format change to make yapf happy.

* Changes to st.key().name() from st.key.name, since they are apparently bound functions, not properties.

* Temporarily sets sandbox to use v1 hostnames.json to be sure that the IPUpdateHandler() behaves as expected.

* Undoes temporary use of v1 hostnames.json. The test was successfull.

* Fixes a small bug where the ipv6 address might be initialized based on an IPv4 no existing.

* Improves and expands the IPUpdateHandler() unit testing.

* Fixes to make yapf happy with the build.

* Fixes to make yapf happy with the build.

* Removes stray vim swap file that somehow made it into the repo.

* For the test_initialize test, don't return an empty mock object, but just an empty list.

server/mlabns/handlers/update.py

@@ -220,8 +220,8 @@ def update(self):
                 # See if this sliver_tool already exists in the datastore.
                 sliver_tool_id = model.get_sliver_tool_id(
                     slice_tool.tool_id, slice_id, server_id, site_id)
-                slivertool = list(filter(lambda st: st.key == sliver_tool_id,
-                                         slivertools))
+                slivertool = list(filter(
+                    lambda st: st.key().name() == sliver_tool_id, slivertools))
 
                 # If the sliver_tool already exists in the datastore, edit it.
                 # If not, add it to the datastore.
@@ -253,7 +253,7 @@ def set_sliver_tool(self, sliver_tool, ipv4, ipv6, rr, fqdn):
         updated = False
         if not ipv4:
             ipv4 = message.NO_IP_ADDRESS
-        if not ipv4:
+        if not ipv6:
             ipv6 = message.NO_IP_ADDRESS
 
         if not sliver_tool.sliver_ipv4 == ipv4:

server/mlabns/tests/test_update.py

@@ -123,31 +123,106 @@ def setUp(self):
         environ_patch.start()
 
     @mock.patch.object(urllib2, 'urlopen')
+    @mock.patch.object(update.IPUpdateHandler, 'initialize_sliver_tool')
     @mock.patch.object(update.IPUpdateHandler, 'put_sliver_tool')
-    def test_update(self, mock_put_sliver_tool, mock_urlopen):
+    def test_update(self, mock_put, mock_initialize, mock_urlopen):
         mock_urlopen.return_value = StringIO.StringIO("""[
 {
-    "hostname": "ndt.iupui.mlab4.xyz01.measurement-lab.org",
+    "hostname": "ndt-iupui-mlab4-xyz01.mlab-staging.measurement-lab.org",
+    "ipv4": "192.168.0.99",
+    "ipv6": "2002:AB:1234::99"
+}
+]""")
+        model.Site.all.return_value.fetch.return_value = [
+            mock.Mock(site_id='xyz01', roundrobin=True)
+        ]
+        model.Tool.all.return_value.fetch.return_value = [
+            mock.Mock(slice_id='iupui_ndt', tool_id='ndt')
+        ]
+
+        mock_slivertools = mock.Mock(
+            slice_id='iupui_ndt',
+            tool_id='ndt',
+            fqdn='ndt.iupui.mlab4.xyz01.measurement-lab.org',
+            sliver_ipv4='192.168.0.1',
+            sliver_ipv6='2002:AB:1234::1',
+            roundrobin=True)
+        sliver_tool_id = model.get_sliver_tool_id('ndt', 'iupui_ndt', 'mlab4',
+                                                  'xyz01')
+        mock_slivertools.key.return_value.name.return_value = sliver_tool_id
+        model.SliverTool.all.return_value.fetch.return_value = [
+            mock_slivertools
+        ]
+
+        handler = update.IPUpdateHandler()
+        handler.update()
+
+        self.assertFalse(mock_initialize.called)
+        self.assertTrue(mock_put.called)
+
+    @mock.patch.object(urllib2, 'urlopen')
+    @mock.patch.object(update.IPUpdateHandler, 'initialize_sliver_tool')
+    @mock.patch.object(update.IPUpdateHandler, 'put_sliver_tool')
+    def test_initialize(self, mock_put, mock_initialize, mock_urlopen):
+        mock_urlopen.return_value = StringIO.StringIO("""[
+{
+    "hostname": "ndt-iupui-mlab4-abc02.mlab-staging.measurement-lab.org",
     "ipv4": "192.168.0.1",
     "ipv6": "2002:AB:1234::1"
 }
 ]""")
         model.Site.all.return_value.fetch.return_value = [
-            mock.Mock(site_id='xyz01')
+            mock.Mock(site_id='abc02', roundrobin=True)
         ]
         model.Tool.all.return_value.fetch.return_value = [
             mock.Mock(slice_id='iupui_ndt', tool_id='ndt')
         ]
+        model.SliverTool.all.return_value.fetch.return_value = []
+
+        handler = update.IPUpdateHandler()
+        handler.update()
+
+        self.assertTrue(mock_initialize.called)
+        self.assertTrue(mock_put.called)
+
+    @mock.patch.object(urllib2, 'urlopen')
+    @mock.patch.object(update.IPUpdateHandler, 'initialize_sliver_tool')
+    @mock.patch.object(update.IPUpdateHandler, 'put_sliver_tool')
+    def test_no_update(self, mock_put, mock_initialize, mock_urlopen):
+
+        mock_urlopen.return_value = StringIO.StringIO("""[
+{
+    "hostname": "ndt-iupui-mlab4-abc02.mlab-staging.measurement-lab.org",
+    "ipv4": "192.168.0.1",
+    "ipv6": "2002:AB:1234::1"
+}
+]""")
+        model.Site.all.return_value.fetch.return_value = [
+            mock.Mock(site_id='abc02', roundrobin=True)
+        ]
+        model.Tool.all.return_value.fetch.return_value = [
+            mock.Mock(slice_id='iupui_ndt', tool_id='ndt')
+        ]
+
+        mock_slivertools = mock.Mock(
+            slice_id='iupui_ndt',
+            tool_id='ndt',
+            fqdn='ndt-iupui-mlab4-abc02.mlab-staging.measurement-lab.org',
+            sliver_ipv4='192.168.0.1',
+            sliver_ipv6='2002:AB:1234::1',
+            roundrobin=True)
+        sliver_tool_id = model.get_sliver_tool_id('ndt', 'iupui_ndt', 'mlab4',
+                                                  'abc02')
+        mock_slivertools.key.return_value.name.return_value = sliver_tool_id
         model.SliverTool.all.return_value.fetch.return_value = [
-            mock.Mock(slice_id='iupui_ndt',
-                      tool_id='ndt',
-                      fqdn='ndt.iupui.mlab4.xyz01.measurement-lab.org')
+            mock_slivertools
         ]
 
         handler = update.IPUpdateHandler()
         handler.update()
 
-        self.assertTrue(mock_put_sliver_tool.called)
+        self.assertFalse(mock_initialize.called)
+        self.assertFalse(mock_put.called)
 
 
 if __name__ == '__main__':