Fix rollback option in cli_config module (#44834)

* Fix rollback option in cli_config module

* Update rollback flag in cliconf plugins
* Add rollback api for junos cliconf plugin

* Update doc

* Update doc

lib/ansible/modules/network/cli/cli_config.py

@@ -25,9 +25,9 @@ description:
 options:
   config:
     description:
-      - The config to be pushed to the network device. This is a
+      - The config to be pushed to the network device. This argument
-        required argument.
+        is mutually exclusive with C(rollback) and either one of the
-    required: true
+        option should be given as input.
     type: 'str'
   commit:
     description:
@@ -51,6 +51,7 @@ options:
         argument.  If the specified rollback identifier does not
         exist on the remote device, the module will fail. To rollback
         to the most recent commit, set the C(rollback) argument to 0.
+        This option is mutually exclusive with C(config).
   commit_comment:
     description:
       - The C(commit_comment) argument specifies a text string to be used
@@ -157,7 +158,7 @@ def validate_args(module, capabilities):
             not capabilities['device_operations']['supports_replace']):
         module.fail_json(msg='replace is not supported on this platform')
 
-    if (module.params['rollback'] and
+    if (module.params['rollback'] is not None and
             not capabilities['device_operations']['supports_rollback']):
         module.fail_json(msg='rollback is not supported on this platform')
 
@@ -193,7 +194,7 @@ def run(module, capabilities, connection, candidate, running):
     banner_diff = {}
 
     replace = module.params['replace']
-    rollback = module.params['rollback']
+    rollback_id = module.params['rollback']
     commit_comment = module.params['commit_comment']
     multiline_delimiter = module.params['multiline_delimiter']
     diff_replace = module.params['diff_replace']
@@ -207,7 +208,12 @@ def run(module, capabilities, connection, candidate, running):
     elif replace in ('no', 'false', 'False'):
         replace = False
 
-    if capabilities['device_operations']['supports_onbox_diff']:
+    if rollback_id is not None:
+        resp = connection.rollback(rollback_id, commit)
+        if 'diff' in resp:
+            result['changed'] = True
+
+    elif capabilities['device_operations']['supports_onbox_diff']:
         if diff_replace:
             module.warn('diff_replace is ignored as the device supports onbox diff')
         if diff_match:
@@ -280,7 +286,7 @@ def main():
     """main entry point for execution
     """
     argument_spec = dict(
-        config=dict(required=True, type='str'),
+        config=dict(type='str'),
         commit=dict(type='bool'),
         replace=dict(type='str'),
         rollback=dict(type='int'),
@@ -292,7 +298,12 @@ def main():
         diff_ignore_lines=dict(type='list')
     )
 
+    mutually_exclusive = [('config', 'rollback')]
+    required_one_of = [['config', 'rollback']]
+
     module = AnsibleModule(argument_spec=argument_spec,
+                           mutually_exclusive=mutually_exclusive,
+                           required_one_of=required_one_of,
                            supports_check_mode=True)
 
     result = {'changed': False}

lib/ansible/plugins/cliconf/__init__.py

@@ -307,6 +307,15 @@ class CliconfBase(AnsiblePlugin):
         """
         return self._connection.method_not_found("discard_changes is not supported by network_os %s" % self._play_context.network_os)
 
+    def rollback(self, rollback_id, commit=True):
+        """
+
+        :param rollback_id: The commit id to which configuration should be rollbacked
+        :param commit: Flag to indicate if changes should be committed or not
+        :return: Returns diff between before and after change.
+        """
+        pass
+
     def copy_file(self, source=None, destination=None, proto='scp', timeout=30):
         """Copies file over scp/sftp to remote device
 

lib/ansible/plugins/cliconf/eos.py

@@ -268,7 +268,7 @@ class Cliconf(CliconfBase):
         return {
             'supports_diff_replace': True,
             'supports_commit': True if self.supports_sessions else False,
-            'supports_rollback': True if self.supports_sessions else False,
+            'supports_rollback': False,
             'supports_defaults': False,
             'supports_onbox_diff': True if self.supports_sessions else False,
             'supports_commit_comment': False,

lib/ansible/plugins/cliconf/iosxr.py

@@ -188,7 +188,7 @@ class Cliconf(CliconfBase):
         return {
             'supports_diff_replace': False,
             'supports_commit': True,
-            'supports_rollback': True,
+            'supports_rollback': False,
             'supports_defaults': False,
             'supports_onbox_diff': True,
             'supports_commit_comment': True,

lib/ansible/plugins/cliconf/junos.py

@@ -174,6 +174,17 @@ class Cliconf(CliconfBase):
 
         return resp
 
+    @configure
+    def rollback(self, rollback_id, commit=True):
+        resp = {}
+        self.send_command('rollback %s' % int(rollback_id))
+        resp['diff'] = self.compare_configuration()
+        if commit:
+            self.commit()
+        else:
+            self.discard_changes()
+        return resp
+
     def get_diff(self, rollback_id=None):
         diff = {'config_diff': None}
         response = self.compare_configuration(rollback_id=rollback_id)

test/integration/targets/junos_config/tests/cli_config/cli_basic.yaml

@@ -2,12 +2,17 @@
 - debug: msg="START cli_config/cli_basic.yaml on connection={{ ansible_connection }}"
 
 - name: setup
-  cli_config: &rm
+  cli_config: &rm1
     config: delete interfaces ge-0/0/1
   become: yes
 
+- name: setup
+  cli_config: &rm2
+    config: delete interfaces ge-0/0/2
+  become: yes
+
 - name: configure device with config
-  cli_config: &conf
+  cli_config: &conf1
     config: set interfaces ge-0/0/1 description 'test-interface'
   register: result
 
@@ -16,14 +21,32 @@
       - "result.changed == true"
 
 - name: Idempotence
-  cli_config: *conf
+  cli_config: *conf1
   register: result
 
 - assert:
     that:
       - "result.changed == false"
 
+- name: configure device with config
+  cli_config: &conf2
+    config: set interfaces ge-0/0/2 description 'test-interface'
+  register: result
+
+- name: test rollabck
+  cli_config:
+    rollback: 1
+  register: result
+
+- assert:
+    that:
+      - "result.changed == true"
+      - "'ge-0/0/2' in result.diff.prepared"
+
 - name: teardown
-  cli_config: *rm
+  cli_config: *rm1
+
+- name: teardown
+  cli_config: *rm2
 
 - debug: msg="END cli_config/cli_basic.yaml on connection={{ ansible_connection }}"