Fix needs_restart for non-bootstrapped instance (#131)

Before this patch needs_restart function didn't check if box.cfg was
called or not. But if box.cfg wasn't called we have to restart instance
if config was changed.

Author: dokshina

library/cartridge_instance.py

@@ -4,6 +4,7 @@
 from ansible.module_utils.helpers import ModuleRes, CartridgeException, cartridge_errcodes
 from ansible.module_utils.helpers import get_control_console
 from ansible.module_utils.helpers import dynamic_box_cfg_params, memory_size_box_cfg_params
+from ansible.module_utils.helpers import box_cfg_was_called
 
 import os
 
@@ -37,12 +38,6 @@ def serialize_dict(d):
     return '{ %s }' % ', '.join(parts)
 
 
-def box_cfg_was_called(control_console):
-    return control_console.eval('''
-        return type(box.cfg) ~= 'function'
-    ''')
-
-
 def get_box_cfg(control_console):
     return control_console.eval('''
         return type(box.cfg) == 'table' and box.cfg or box.NULL

library/cartridge_needs_restart.py

@@ -4,6 +4,7 @@
 from ansible.module_utils.helpers import ModuleRes, CartridgeException, cartridge_errcodes
 from ansible.module_utils.helpers import get_control_console
 from ansible.module_utils.helpers import dynamic_box_cfg_params
+from ansible.module_utils.helpers import box_cfg_was_called
 
 import os
 
@@ -120,7 +121,7 @@ def needs_restart(params):
     if last_restart_time < package_update_time:
         return ModuleRes(success=True, changed=True)
 
-    # check if instance config was changed (except memtx_memory)
+    # check if instance config was changed (except dynamic params)
     current_instance_conf = read_yaml_file_section(
         instance_conf_file,
         control_console,
@@ -130,7 +131,7 @@ def needs_restart(params):
         return ModuleRes(success=True, changed=True)
 
     if not stateboard:
-        # check if default config was changed (except memtx_memory)
+        # check if default config was changed (except dynamic params)
         current_default_conf = read_yaml_file_section(
             default_conf_path,
             control_console,
@@ -140,7 +141,13 @@ def needs_restart(params):
         if check_conf_updated(new_default_conf, current_default_conf, dynamic_box_cfg_params):
             return ModuleRes(success=True, changed=True)
 
+    # if box.cfg wasn't called,
+    if not box_cfg_was_called(control_console):
+        return ModuleRes(success=True, changed=True)
+
     current_cfg = get_current_cfg(control_console)
+    if current_cfg is None:
+        return ModuleRes(success=True, changed=True)
 
     for param_name in dynamic_box_cfg_params:
         new_value = None
@@ -153,7 +160,7 @@ def needs_restart(params):
         # If current parameter wasn't changed to the new value,
         # it mean that instance should be restarted to apply change
         if new_value is not None:
-            if current_cfg[param_name] != new_value:
+            if current_cfg.get(param_name) != new_value:
                 return ModuleRes(success=True, changed=True)
 
     return ModuleRes(success=True, changed=False)

module_utils/helpers.py

@@ -198,3 +198,9 @@ def is_expelled(host_vars):
 
 def is_stateboard(host_vars):
     return host_vars.get('stateboard') is True
+
+
+def box_cfg_was_called(control_console):
+    return control_console.eval('''
+        return type(box.cfg) ~= 'function'
+    ''')

unit/test_needs_restart.py

@@ -85,6 +85,39 @@ def test_instance_not_started(self):
         self.assertTrue(res.success, msg=res.msg)
         self.assertTrue(res.changed)
 
+    def test_box_cfg_is_function(self):
+        param_name = 'some-param'
+        old_value = 'old-value'
+        new_value = 'new-value'
+
+        self.instance.set_box_cfg_function()
+
+        self.instance.set_instance_config({
+            param_name: old_value,
+        })
+
+        # nothing changed
+        res = call_needs_restart(
+            control_sock=self.console_sock,
+            config={
+                param_name: old_value,
+            },
+        )
+
+        self.assertTrue(res.success, msg=res.msg)
+        self.assertTrue(res.changed)
+
+        # param was changed
+        res = call_needs_restart(
+            control_sock=self.console_sock,
+            config={
+                param_name: new_value,
+            },
+        )
+
+        self.assertTrue(res.success, msg=res.msg)
+        self.assertTrue(res.changed)
+
     def test_code_was_updated(self):
         # code was updated today, socket yesterday - needs restart
         self.instance.set_path_mtime(self.instance.APP_CODE_PATH, self.instance.DATE_TODAY)
@@ -290,7 +323,7 @@ def test_app_config_changed(self, instance_type, memory_param_name):
         ["memtx_memory"],
         ["vinyl_memory"],
     ])
-    def test_both_app_and_instance_memtx_memory_changed(self, memory_param_name):
+    def test_memory_size_changed(self, memory_param_name):
         current_memory_size = 100
         new_memory_size_instance = 200
         new_memory_size_app = 300