launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/allow-hostname-recreation into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/maas/allow-hostname-recreation/+merge/123015

Pre-imped with Julian.  I ran a manual omshell to reproduce the error output.  This also revealed a problem where we weren't quoting the hostname we pass to omshell.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/allow-hostname-recreation/+merge/123015
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/allow-hostname-recreation into lp:maas.

=== modified file 'src/provisioningserver/omshell.py'
--- src/provisioningserver/omshell.py	2012-08-22 20:37:49 +0000
+++ src/provisioningserver/omshell.py	2012-09-06 06:42:34 +0000
@@ -130,7 +130,13 @@
         # that the 'create' command failed.  Unfortunately there's no
         # other output like "successful" to check so this is the best we
         # can do.
-        if "hardware-type" not in output:
+        if "hardware-type" in output:
+            # Success.
+            pass
+        elif "can't open object: I/O error" in output:
+            # Host map already existed.  Treat as success.
+            pass
+        else:
             raise CalledProcessError(returncode, "omshell", output)
 
     def remove(self, ip_address):

=== modified file 'src/provisioningserver/tests/test_omshell.py'
--- src/provisioningserver/tests/test_omshell.py	2012-08-31 15:43:12 +0000
+++ src/provisioningserver/tests/test_omshell.py	2012-09-06 06:42:34 +0000
@@ -21,6 +21,7 @@
 from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
 from maastesting.testcase import TestCase
+from mock import Mock
 from provisioningserver import omshell
 from provisioningserver.omshell import (
     generate_omapi_key,
@@ -98,6 +99,35 @@
             CalledProcessError, shell.create, ip_address, mac_address)
         self.assertEqual(random_output, exc.output)
 
+    def test_create_succeeds_when_host_map_already_exists(self):
+        # To omshell, creating the same host map twice is an error.  But
+        # Omshell.create swallows the error and makes it look like
+        # success.
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+            'hostname': factory.make_name('hostname')
+        }
+        shell = Omshell(factory.make_name('server'), factory.make_name('key'))
+        # This is the kind of error output we get if a host map has
+        # already been created.
+        error_output = dedent("""\
+            obj: host
+            ip-address = %(ip)s
+            hardware-address = %(mac)s
+            name = "%(hostname)s"
+            >
+            can't open object: I/O error
+            obj: host
+            ip-address = %(ip)s
+            hardware-address = %(mac)s
+            name = "%(hostname)s"
+            """) % params
+        shell._run = Mock(return_value=(0, error_output))
+        shell.create(params['ip'], params['mac'])
+        # The test is that we get here without error.
+        pass
+
     def test_remove_calls_omshell_correctly(self):
         server_address = factory.getRandomString()
         shared_key = factory.getRandomString()