From 5a42ef3b07bfd1047dcb7484bb280990a33d6c2c Mon Sep 17 00:00:00 2001
From: Alessandro Pilotti <apilotti@cloudbasesolutions.com>
Date: Sat, 11 Jul 2015 20:05:47 +0300
Subject: [PATCH] Improves logging when exiting the plugins loop

This helps in troubleshooting issues during plugin execution

Change-Id: I0ff584ee3179ee30f66de71f0591dc24c14f3596
Co-Authored-By: Claudiu Popa <cpopa@cloudbasesolutions.com>
---
 cloudbaseinit/init.py            |  8 +++--
 cloudbaseinit/tests/test_init.py | 61 ++++++++++++++++++++++++--------
 2 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/cloudbaseinit/init.py b/cloudbaseinit/init.py
index 3ed40e1c..e2ea788a 100644
--- a/cloudbaseinit/init.py
+++ b/cloudbaseinit/init.py
@@ -124,8 +124,12 @@ class InitManager(object):
 
         if reboot_required and CONF.allow_reboot:
             try:
+                LOG.info("Rebooting")
                 osutils.reboot()
             except Exception as ex:
                 LOG.error('reboot failed with error \'%s\'' % ex)
-        elif CONF.stop_service_on_exit:
-            osutils.terminate()
+        else:
+            LOG.info("Plugins execution done")
+            if CONF.stop_service_on_exit:
+                LOG.info("Stopping Cloudbase-Init service")
+                osutils.terminate()
diff --git a/cloudbaseinit/tests/test_init.py b/cloudbaseinit/tests/test_init.py
index 5b0c3371..dbfbed2c 100644
--- a/cloudbaseinit/tests/test_init.py
+++ b/cloudbaseinit/tests/test_init.py
@@ -143,14 +143,13 @@ class InitManagerTest(unittest.TestCase):
     @mock.patch('cloudbaseinit.plugins.common.factory.load_plugins')
     @mock.patch('cloudbaseinit.osutils.factory.get_os_utils')
     @mock.patch('cloudbaseinit.metadata.factory.get_metadata_service')
-    def test_configure_host(self, mock_get_metadata_service,
-                            mock_get_os_utils, mock_load_plugins,
-                            mock_exec_plugin,
-                            mock_check_os_requirements,
-                            mock_get_version):
-        instance_id = 'fake id'
-        name = 'fake name'
-        version = 'version'
+    def _test_configure_host(self, mock_get_metadata_service,
+                             mock_get_os_utils, mock_load_plugins,
+                             mock_exec_plugin,
+                             mock_check_os_requirements,
+                             mock_get_version, expected_logging,
+                             version, name, instance_id, reboot=True):
+
         mock_get_version.return_value = version
         fake_service = mock.MagicMock()
         fake_plugin = mock.MagicMock()
@@ -159,11 +158,6 @@ class InitManagerTest(unittest.TestCase):
         mock_get_metadata_service.return_value = fake_service
         fake_service.get_name.return_value = name
         fake_service.get_instance_id.return_value = instance_id
-        expected_logging = [
-            'Cloudbase-Init version: %s' % version,
-            'Metadata service loaded: %r' % name,
-            'Instance id: %s' % instance_id
-        ]
 
         with testutils.LogSnatcher('cloudbaseinit.init') as snatcher:
             self._init.configure_host()
@@ -175,6 +169,43 @@ class InitManagerTest(unittest.TestCase):
         mock_check_os_requirements.assert_called_once_with(self.osutils,
                                                            fake_plugin)
         mock_exec_plugin.assert_called_once_with(self.osutils, fake_service,
-                                                 fake_plugin, 'fake id', {})
+                                                 fake_plugin, instance_id, {})
         fake_service.cleanup.assert_called_once_with()
-        self.osutils.reboot.assert_called_once_with()
+        if reboot:
+            self.osutils.reboot.assert_called_once_with()
+        else:
+            self.assertFalse(self.osutils.reboot.called)
+
+    def _test_configure_host_with_logging(self, extra_logging, reboot=True):
+        instance_id = 'fake id'
+        name = 'fake name'
+        version = 'version'
+        expected_logging = [
+            'Cloudbase-Init version: %s' % version,
+            'Metadata service loaded: %r' % name,
+            'Instance id: %s' % instance_id,
+        ]
+        self._test_configure_host(
+            expected_logging=expected_logging + extra_logging,
+            version=version, name=name, instance_id=instance_id,
+            reboot=reboot)
+
+    @testutils.ConfPatcher('allow_reboot', False)
+    @testutils.ConfPatcher('stop_service_on_exit', False)
+    def test_configure_host_no_reboot_no_service_stopping(self):
+        self._test_configure_host_with_logging(
+            reboot=False,
+            extra_logging=['Plugins execution done'])
+
+    @testutils.ConfPatcher('allow_reboot', False)
+    @testutils.ConfPatcher('stop_service_on_exit', True)
+    def test_configure_host_no_reboot_allow_service_stopping(self):
+        self._test_configure_host_with_logging(
+            reboot=False,
+            extra_logging=['Plugins execution done',
+                           'Stopping Cloudbase-Init service'])
+
+    @testutils.ConfPatcher('allow_reboot', True)
+    def test_configure_host_reboot(self):
+        self._test_configure_host_with_logging(
+            extra_logging=['Rebooting'])