[1843] revert script config changes if script fails

diff --git a/src/bin/bindctl/bindcmd.py b/src/bin/bindctl/bindcmd.py
index 41c1f779fcf787fdd358dce81da24a2b790b6159..64ff878130442f6618017269f24dd70851a6bd00 100644 (file)
--- a/src/bin/bindctl/bindcmd.py
+++ b/src/bin/bindctl/bindcmd.py
@@ -38,6 +38,7 @@ from hashlib import sha1
 import csv
 import pwd
 import getpass
+import copy
 
 try:
     from collections import OrderedDict
@@ -782,6 +783,9 @@ class BindCmdInterpreter(Cmd):
         '''
         verbose = False
         # TODO: revert local changes on failure
+        # make sure it's a copy
+        local_changes_backup =\
+            copy.deepcopy(self.config_data.get_local_changes())
         try:
             for line in commands:
                 line = line.strip()
@@ -802,14 +806,15 @@ class BindCmdInterpreter(Cmd):
                     cmd = BindCmdParse(line)
                     self._validate_cmd(cmd)
                     self._handle_cmd(cmd)
-        except isc.config.ModuleCCSessionError as mcse:
-            print(str(mcse))
-        except (IOError, http.client.HTTPException,
+        except (isc.config.ModuleCCSessionError,
+                IOError, http.client.HTTPException,
                 BindCtlException, isc.cc.data.DataTypeError,
                 isc.cc.data.DataNotFoundError,
                 isc.cc.data.DataAlreadyPresentError,
                 KeyError) as err:
             print('Error: ', err)
+            # revert changes
+            self.config_data.set_local_changes(local_changes_backup)
 
     def apply_cmd(self, cmd):
         '''Handles a general module command'''

diff --git a/src/lib/python/isc/config/config_data.py b/src/lib/python/isc/config/config_data.py
index 19d9673bc002911c844967b8a3d79a08fd525966..b7adef861315653c477d10f30aa45db33aa2df5c 100644 (file)
--- a/src/lib/python/isc/config/config_data.py
+++ b/src/lib/python/isc/config/config_data.py
@@ -420,6 +420,14 @@ class MultiConfigData:
            manager or the modules."""
         return self._local_changes
 
+    def set_local_changes(self, new_local_changes):
+        """Sets the entire set of local changes, used when reverting
+           changes done automatically in case there was a problem (e.g.
+           when executing commands from a script that fails halfway
+           through).
+        """
+        self._local_changes = new_local_changes
+
     def clear_local_changes(self):
         """Reverts all local changes"""
         self._local_changes = {}

diff --git a/tests/lettuce/data/commands/bad_command b/tests/lettuce/data/commands/bad_command
index d78470aa62ebb8c4baf5d29c282efa0ddb3ccbd9..95d1694a1c42fb72b47411fd31da98c4283cd79b 100644 (file)
--- a/tests/lettuce/data/commands/bad_command
+++ b/tests/lettuce/data/commands/bad_command
@@ -1,3 +1,9 @@
 !echo shouldshow
-some bad command
+# just add something so the test can verify it's reverted
+config add /Boss/components b10-auth
+config set /Boss/components/b10-auth/kind needed
+config set /Boss/components/b10-auth/special auth
+bad command
+# this should not be reached
 !echo shouldnotshow
+config commit

diff --git a/tests/lettuce/features/bindctl_commands.feature b/tests/lettuce/features/bindctl_commands.feature
index 7363e634284ee568ddcf9518df3a3248939fc843..0c4db5bd15a414843f0fcb0004c7deaa57a12596 100644 (file)
--- a/tests/lettuce/features/bindctl_commands.feature
+++ b/tests/lettuce/features/bindctl_commands.feature
@@ -68,7 +68,7 @@ Feature: control with bindctl
     Scenario: Executing scripts from files
         # This test tests the 'execute' command, which reads and executes
         # bindctl commands from a file
-        Given I have bind10 running with configuration bindctl_commands.config
+        Given I have bind10 running with configuration bindctl/bindctl.config
         And wait for bind10 stderr message BIND10_STARTED_CC
         And wait for bind10 stderr message CMDCTL_STARTED
 
@@ -97,6 +97,29 @@ Feature: control with bindctl
         last bindctl output should contain shouldshow
         last bindctl output should contain Error
         last bindctl output should not contain shouldnotshow
+        # This would fail if the entire list was passed, or the configuratio
+        send bind10 the command config show Boss/components
+        last bindctl output should not contain b10-auth
+
+        # the bad command should also keep existing changes intact,
+        # i.e. if we add something, then run a failing script, our
+        # addition should still be there, but those from the script
+        # should not
+        When I send bind10 the following commands:
+        """
+        config add Boss/components b10-resolver
+        config set Boss/components/b10-resolver/kind dispensable
+        config set Boss/components/b10-resolver/special resolver
+        execute file data/commands/bad_command
+        config commit
+        """
+        last bindctl output should contain shouldshow
+        last bindctl output should contain Error
+        last bindctl output should not contain shouldnotshow
+        # This would fail if the entire list was passed, or the configuratio
+        send bind10 the command config show Boss/components
+        last bindctl output should not contain b10-auth
+        last bindctl output should contain b10-resolver
 
         # nested_command contains another execute script
         When I send bind10 the command execute file data/commands/nested