From 5af954d4d0103eab3fe0544e3ecdbaec5bafffa0 Mon Sep 17 00:00:00 2001
From: Alessandro Pilotti <apilotti@cloudbasesolutions.com>
Date: Thu, 10 Apr 2014 15:01:45 +0300
Subject: [PATCH] Fixes regression: reboot only if hostname is set

---
 cloudbaseinit/plugins/windows/sethostname.py  | 11 ++++-
 .../tests/plugins/windows/test_sethostname.py | 41 ++++++++++++++-----
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/cloudbaseinit/plugins/windows/sethostname.py b/cloudbaseinit/plugins/windows/sethostname.py
index 47f97bcb..11fade57 100644
--- a/cloudbaseinit/plugins/windows/sethostname.py
+++ b/cloudbaseinit/plugins/windows/sethostname.py
@@ -14,6 +14,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import platform
+
 from oslo.config import cfg
 
 from cloudbaseinit.openstack.common import log as logging
@@ -56,7 +58,12 @@ class SetHostNamePlugin(base.BasePlugin):
         else:
             new_host_name = metadata_host_name
 
-        LOG.info("Setting hostname: %s" % new_host_name)
-        reboot_required = osutils.set_host_name(new_host_name)
+        if platform.node().lower() == new_host_name.lower():
+            LOG.debug("Hostname already set to: %s" % new_host_name)
+            reboot_required = False
+        else:
+            LOG.info("Setting hostname: %s" % new_host_name)
+            osutils.set_host_name(new_host_name)
+            reboot_required = True
 
         return (base.PLUGIN_EXECUTION_DONE, reboot_required)
diff --git a/cloudbaseinit/tests/plugins/windows/test_sethostname.py b/cloudbaseinit/tests/plugins/windows/test_sethostname.py
index eb10176e..b02e2d60 100644
--- a/cloudbaseinit/tests/plugins/windows/test_sethostname.py
+++ b/cloudbaseinit/tests/plugins/windows/test_sethostname.py
@@ -19,6 +19,7 @@ import unittest
 
 from oslo.config import cfg
 
+from cloudbaseinit.plugins import base
 from cloudbaseinit.plugins.windows import sethostname
 from cloudbaseinit.tests.metadata import fake_json_response
 
@@ -32,41 +33,59 @@ class SetHostNamePluginPluginTests(unittest.TestCase):
         self.fake_data = fake_json_response.get_fake_metadata_json(
             '2013-04-04')
 
+    @mock.patch('platform.node')
     @mock.patch('cloudbaseinit.osutils.factory.get_os_utils')
-    def _test_execute(self, mock_get_os_utils, hostname_exists,
-                      new_hostname_length=1):
+    def _test_execute(self, mock_get_os_utils, mock_node, hostname_exists=True,
+                      hostname_already_set=False, new_hostname_length=1):
         mock_service = mock.MagicMock()
         mock_osutils = mock.MagicMock()
         fake_shared_data = 'fake data'
         new_hostname = 'x' * new_hostname_length
+
         if hostname_exists:
             mock_service.get_host_name.return_value = new_hostname
         else:
             mock_service.get_host_name.return_value = None
+
         CONF.set_override('netbios_host_name_compatibility', True)
         mock_get_os_utils.return_value = mock_osutils
-        mock_osutils.set_host_name.return_value = False
-        response = self._sethostname_plugin.execute(mock_service,
-                                                    fake_shared_data)
-        mock_service.get_host_name.assert_caled_once_with()
+
         if hostname_exists is True:
             length = sethostname.NETBIOS_HOST_NAME_MAX_LEN
-            mock_get_os_utils.assert_called_once_with()
             hostname = new_hostname.split('.', 1)[0]
             if len(new_hostname) > length:
                 hostname = hostname[:length]
-            mock_osutils.set_host_name.assert_called_once_with(hostname)
 
-        self.assertEqual(response, (1, False))
+            if hostname_already_set:
+                mock_node.return_value = hostname
+            else:
+                mock_node.return_value = 'fake_old_value'
+
+        response = self._sethostname_plugin.execute(mock_service,
+                                                    fake_shared_data)
+
+        mock_service.get_host_name.assert_called_once_with()
+
+        if hostname_exists is True:
+            mock_get_os_utils.assert_called_once_with()
+            if hostname_already_set:
+                self.assertFalse(mock_osutils.set_host_name.called)
+            else:
+                mock_osutils.set_host_name.assert_called_once_with(hostname)
+
+        self.assertEqual(
+            response, (base.PLUGIN_EXECUTION_DONE,
+                       hostname_exists and not hostname_already_set))
+
+    def test_execute_hostname_already_set(self):
+        self._test_execute(hostname_already_set=True)
 
     def test_execute_hostname_to_be_truncated(self):
         self._test_execute(
-            hostname_exists=True,
             new_hostname_length=sethostname.NETBIOS_HOST_NAME_MAX_LEN + 1)
 
     def test_execute_no_truncate_needed(self):
         self._test_execute(
-            hostname_exists=True,
             new_hostname_length=sethostname.NETBIOS_HOST_NAME_MAX_LEN)
 
     def test_execute_no_hostname(self):