[1843] revert script config changes if script fails

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'''

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 = {}

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

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