Take care of return codes when executing scripts

As userdata plugin is doing when executing userdata scripts,
do the same for the scripts executed under the localscripts plugin.
    * move the `get_plugin_return_value` to a common place
    * properly test the method above
    * use it with the localscripts plugin and make sure to choose
      the most comprehensive plugin status and reboot requirement

Change-Id: Id3aeab079be39e2c4d3d8bf51ef10ef1c7be68e2

cloudbaseinit/plugins/common/execcmd.py

@@ -21,6 +21,7 @@ import uuid
 
 from cloudbaseinit.openstack.common import log as logging
 from cloudbaseinit.osutils import factory as osutils_factory
+from cloudbaseinit.plugins.common import base
 
 
 __all__ = (
@@ -51,6 +52,10 @@ TAG_REGEX = {
     )
 }
 
+# important return values range
+RET_START = 1001
+RET_END = 1003
+
 
 def _ec2_find_sections(data):
     """An intuitive script generator.
@@ -85,6 +90,23 @@ def _split_sections(multicmd):
         yield command
 
 
+def get_plugin_return_value(ret_val):
+    plugin_status = base.PLUGIN_EXECUTION_DONE
+    reboot = False
+
+    try:
+        ret_val = int(ret_val)
+    except (ValueError, TypeError):
+        ret_val = 0
+
+    if ret_val and RET_START <= ret_val <= RET_END:
+        reboot = bool(ret_val & 1)
+        if ret_val & 2:
+            plugin_status = base.PLUGIN_EXECUTE_ON_NEXT_BOOT
+
+    return plugin_status, reboot
+
+
 class BaseCommand(object):
     """Implements logic for executing an user command.
 

cloudbaseinit/plugins/common/localscripts.py

@@ -17,6 +17,7 @@ import os
 from oslo.config import cfg
 
 from cloudbaseinit.plugins.common import base
+from cloudbaseinit.plugins.common import execcmd
 from cloudbaseinit.plugins.common import fileexecutils
 
 opts = [
@@ -36,8 +37,17 @@ class LocalScriptsPlugin(base.BasePlugin):
                        if os.path.isfile(os.path.join(path, f))])
 
     def execute(self, service, shared_data):
+        plugin_status = base.PLUGIN_EXECUTION_DONE
+        reboot = False
+
         if CONF.local_scripts_path:
             for file_path in self._get_files_in_dir(CONF.local_scripts_path):
-                fileexecutils.exec_file(file_path)
+                ret_val = fileexecutils.exec_file(file_path)
+                new_plugin_status, new_reboot = \
+                    execcmd.get_plugin_return_value(ret_val)
+                plugin_status = max(plugin_status, new_plugin_status)
+                reboot = reboot or new_reboot
+                if reboot:
+                    break
 
-        return (base.PLUGIN_EXECUTION_DONE, False)
+        return plugin_status, reboot

cloudbaseinit/plugins/common/userdata.py

@@ -19,6 +19,7 @@ import io
 from cloudbaseinit.metadata.services import base as metadata_services_base
 from cloudbaseinit.openstack.common import log as logging
 from cloudbaseinit.plugins.common import base
+from cloudbaseinit.plugins.common import execcmd
 from cloudbaseinit.plugins.common.userdataplugins import factory
 from cloudbaseinit.plugins.common import userdatautils
 from cloudbaseinit.utils import encoding
@@ -114,7 +115,7 @@ class UserDataPlugin(base.BasePlugin):
                        'filename': part.get_filename()})
             LOG.exception(ex)
 
-        return self._get_plugin_return_value(ret_val)
+        return execcmd.get_plugin_return_value(ret_val)
 
     def _add_part_handlers(self, user_data_plugins, user_handlers,
                            new_user_handlers):
@@ -142,22 +143,6 @@ class UserDataPlugin(base.BasePlugin):
         LOG.debug("Calling part handler \"__end__\" event")
         handler_func(None, "__end__", None, None)
 
-    def _get_plugin_return_value(self, ret_val):
-        plugin_status = base.PLUGIN_EXECUTION_DONE
-        reboot = False
-
-        try:
-            ret_val = int(ret_val)
-        except (ValueError, TypeError):
-            ret_val = 0
-
-        if ret_val and 1001 <= ret_val <= 1003:
-            reboot = bool(ret_val & 1)
-            if ret_val & 2:
-                plugin_status = base.PLUGIN_EXECUTE_ON_NEXT_BOOT
-
-        return (plugin_status, reboot)
-
     def _process_non_multi_part(self, user_data):
         if user_data.startswith(b'#cloud-config'):
             user_data_plugins = factory.load_plugins()
@@ -166,4 +151,4 @@ class UserDataPlugin(base.BasePlugin):
         else:
             ret_val = userdatautils.execute_user_data_script(user_data)
 
-        return self._get_plugin_return_value(ret_val)
+        return execcmd.get_plugin_return_value(ret_val)

cloudbaseinit/tests/plugins/common/test_execcmd.py

@@ -22,6 +22,7 @@ try:
 except ImportError:
     import mock
 
+from cloudbaseinit.plugins.common import base
 from cloudbaseinit.plugins.common import execcmd
 from cloudbaseinit.tests import testutils
 
@@ -163,3 +164,15 @@ class TestExecCmd(unittest.TestCase):
 
     def test_process_ec2_order(self, _):
         self._test_process_ec2()
+
+    def test_get_plugin_return_value(self, _):
+        ret_val_map = {
+            0: (base.PLUGIN_EXECUTION_DONE, False),
+            1: (base.PLUGIN_EXECUTION_DONE, False),
+            "invalid": (base.PLUGIN_EXECUTION_DONE, False),
+            1001: (base.PLUGIN_EXECUTION_DONE, True),
+            1002: (base.PLUGIN_EXECUTE_ON_NEXT_BOOT, False),
+            1003: (base.PLUGIN_EXECUTE_ON_NEXT_BOOT, True),
+        }
+        for ret_val, expect in ret_val_map.items():
+            self.assertEqual(expect, execcmd.get_plugin_return_value(ret_val))

cloudbaseinit/tests/plugins/common/test_localscripts.py

@@ -43,20 +43,30 @@ class LocalScriptsPluginTests(unittest.TestCase):
             sorted(os.path.join(fake_path, f) for f in fake_file_list),
             response)
 
+    @mock.patch('cloudbaseinit.plugins.common.execcmd'
+                '.get_plugin_return_value')
     @testutils.ConfPatcher('local_scripts_path',
                            mock.sentinel.mock_local_scripts_path)
     @mock.patch('cloudbaseinit.plugins.common.localscripts'
                 '.LocalScriptsPlugin._get_files_in_dir')
     @mock.patch('cloudbaseinit.plugins.common.fileexecutils.exec_file')
-    def test_execute(self, mock_exec_file, mock_get_files_in_dir):
+    def test_execute(self, mock_exec_file, mock_get_files_in_dir,
+                     mock_get_plugin_return_value):
         mock_service = mock.MagicMock()
         fake_path = os.path.join('fake', 'path')
 
-        mock_get_files_in_dir.return_value = [fake_path]
+        mock_get_files_in_dir.return_value = [fake_path] * 2
+        mock_exec_file.side_effect = [1001, 1002]
+        mock_get_plugin_return_value.side_effect = [
+            (base.PLUGIN_EXECUTE_ON_NEXT_BOOT, False),
+            (base.PLUGIN_EXECUTION_DONE, True),
+        ]
 
         response = self._localscripts.execute(mock_service, shared_data=None)
 
         mock_get_files_in_dir.assert_called_once_with(
             mock.sentinel.mock_local_scripts_path)
-        mock_exec_file.assert_called_once_with(fake_path)
+        mock_exec_file.assert_called_with(fake_path)
-        self.assertEqual((base.PLUGIN_EXECUTION_DONE, False), response)
+        mock_get_plugin_return_value.assert_any_call(1001)
+        mock_get_plugin_return_value.assert_any_call(1002)
+        self.assertEqual((base.PLUGIN_EXECUTE_ON_NEXT_BOOT, True), response)

cloudbaseinit/tests/plugins/common/test_userdata.py

@@ -156,8 +156,8 @@ class UserDataPluginTest(unittest.TestCase):
 
     @mock.patch('cloudbaseinit.plugins.common.userdata.UserDataPlugin'
                 '._add_part_handlers')
-    @mock.patch('cloudbaseinit.plugins.common.userdata.UserDataPlugin'
+    @mock.patch('cloudbaseinit.plugins.common.execcmd'
-                '._get_plugin_return_value')
+                '.get_plugin_return_value')
     def _test_process_part(self, mock_get_plugin_return_value,
                            mock_add_part_handlers,
                            handler_func, user_data_plugin, content_type):
@@ -267,8 +267,8 @@ class UserDataPluginTest(unittest.TestCase):
 
     @mock.patch('cloudbaseinit.plugins.common.userdatautils'
                 '.execute_user_data_script')
-    @mock.patch('cloudbaseinit.plugins.common.userdata.UserDataPlugin'
+    @mock.patch('cloudbaseinit.plugins.common.execcmd'
-                '._get_plugin_return_value')
+                '.get_plugin_return_value')
     def test_process_non_multi_part(self, mock_get_plugin_return_value,
                                     mock_execute_user_data_script):
         user_data = b'fake'
@@ -280,8 +280,8 @@ class UserDataPluginTest(unittest.TestCase):
 
     @mock.patch('cloudbaseinit.plugins.common.userdataplugins.factory.'
                 'load_plugins')
-    @mock.patch('cloudbaseinit.plugins.common.userdata.UserDataPlugin'
+    @mock.patch('cloudbaseinit.plugins.common.execcmd'
-                '._get_plugin_return_value')
+                '.get_plugin_return_value')
     def test_process_non_multi_part_cloud_config(
             self, mock_get_plugin_return_value, mock_load_plugins):
         user_data = b'#cloud-config'