Idempotency for net_get and net_put modules (#42307)

* Idempotency for net_get and net_put modules

* pep8 warnings fix

* remove import q

lib/ansible/modules/network/files/net_put.py

@@ -49,7 +49,7 @@ options:
         present in the src file. If mode is set to I(binary) then file will be
         copied as it is to destination device.
     default: binary
-    choices: ['binary', 'template']
+    choices: ['binary', 'text']
     version_added: "2.7"
 
 requirements:

lib/ansible/plugins/action/net_get.py

@@ -21,8 +21,10 @@ import copy
 import os
 import time
 import re
+import uuid
+import hashlib
 
-from ansible.module_utils._text import to_text
+from ansible.module_utils._text import to_text, to_bytes
 from ansible.module_utils.connection import Connection
 from ansible.errors import AnsibleError
 from ansible.plugins.action import ActionBase
@@ -75,6 +77,16 @@ class ActionModule(ActionBase):
 
         conn = Connection(socket_path)
 
+        try:
+            changed = self._handle_existing_file(conn, src, dest, proto, sock_timeout)
+            if changed is False:
+                result['changed'] = False
+                result['destination'] = dest
+                return result
+        except Exception as exc:
+            result['msg'] = ('Warning: exception %s idempotency check failed. Check '
+                             'dest' % exc)
+
         try:
             out = conn.get_file(
                 source=src, destination=dest,
@@ -128,3 +140,41 @@ class ActionModule(ActionBase):
             raise AnsibleError('ansible_network_os must be specified on this host to use platform agnostic modules')
 
         return network_os
+
+    def _handle_existing_file(self, conn, source, dest, proto, timeout):
+        if not os.path.exists(dest):
+            return True
+        cwd = self._loader.get_basedir()
+        filename = str(uuid.uuid4())
+        tmp_dest_file = os.path.join(cwd, filename)
+        try:
+            out = conn.get_file(
+                source=source, destination=tmp_dest_file,
+                proto=proto, timeout=timeout
+            )
+        except Exception as exc:
+            os.remove(tmp_dest_file)
+            raise Exception(exc)
+
+        try:
+            with open(tmp_dest_file, 'r') as f:
+                new_content = f.read()
+            with open(dest, 'r') as f:
+                old_content = f.read()
+        except (IOError, OSError) as ioexc:
+            raise IOError(ioexc)
+
+        sha1 = hashlib.sha1()
+        old_content_b = to_bytes(old_content, errors='surrogate_or_strict')
+        sha1.update(old_content_b)
+        checksum_old = sha1.digest()
+
+        sha1 = hashlib.sha1()
+        new_content_b = to_bytes(new_content, errors='surrogate_or_strict')
+        sha1.update(new_content_b)
+        checksum_new = sha1.digest()
+        os.remove(tmp_dest_file)
+        if checksum_old == checksum_new:
+            return False
+        else:
+            return True

lib/ansible/plugins/action/net_put.py

@@ -21,8 +21,10 @@ import copy
 import os
 import time
 import uuid
+import hashlib
+import sys
 
-from ansible.module_utils._text import to_text
+from ansible.module_utils._text import to_text, to_bytes
 from ansible.module_utils.connection import Connection
 from ansible.errors import AnsibleError
 from ansible.plugins.action import ActionBase
@@ -38,6 +40,7 @@ except ImportError:
 class ActionModule(ActionBase):
 
     def run(self, tmp=None, task_vars=None):
+        changed = True
         socket_path = None
         play_context = copy.deepcopy(self._play_context)
         play_context.network_os = self._get_network_os(task_vars)
@@ -70,7 +73,7 @@ class ActionModule(ActionBase):
         if mode is None:
             mode = 'binary'
 
-        if mode == 'template':
+        if mode == 'text':
             try:
                 self._handle_template()
             except ValueError as exc:
@@ -97,6 +100,17 @@ class ActionModule(ActionBase):
 
         if dest is None:
             dest = src_file_path_name
+
+        try:
+            changed = self._handle_existing_file(conn, output_file, dest, proto, sock_timeout)
+            if changed is False:
+                result['changed'] = False
+                result['destination'] = dest
+                return result
+        except Exception as exc:
+            result['msg'] = ('Warning: Exc %s idempotency check failed. Check'
+                             'dest' % exc)
+
         try:
             out = conn.copy_file(
                 source=output_file, destination=dest,
@@ -112,13 +126,55 @@ class ActionModule(ActionBase):
                 result['failed'] = True
                 result['msg'] = ('Exception received : %s' % exc)
 
-        if mode == 'template':
+        if mode == 'text':
             # Cleanup tmp file expanded wih ansible vars
             os.remove(output_file)
 
-        result['changed'] = True
+        result['changed'] = changed
+        result['destination'] = dest
         return result
 
+    def _handle_existing_file(self, conn, source, dest, proto, timeout):
+        cwd = self._loader.get_basedir()
+        filename = str(uuid.uuid4())
+        source_file = os.path.join(cwd, filename)
+        try:
+            out = conn.get_file(
+                source=dest, destination=source_file,
+                proto=proto, timeout=timeout
+            )
+        except Exception as exc:
+            if (to_text(exc)).find("No such file or directory") > 0:
+                return True
+            else:
+                try:
+                    os.remove(source_file)
+                except OSError as osex:
+                    raise Exception(osex)
+
+        try:
+            with open(source, 'r') as f:
+                new_content = f.read()
+            with open(source_file, 'r') as f:
+                old_content = f.read()
+        except (IOError, OSError) as ioexc:
+            raise IOError(ioexc)
+
+        sha1 = hashlib.sha1()
+        old_content_b = to_bytes(old_content, errors='surrogate_or_strict')
+        sha1.update(old_content_b)
+        checksum_old = sha1.digest()
+
+        sha1 = hashlib.sha1()
+        new_content_b = to_bytes(new_content, errors='surrogate_or_strict')
+        sha1.update(new_content_b)
+        checksum_new = sha1.digest()
+        os.remove(source_file)
+        if checksum_old == checksum_new:
+            return False
+        else:
+            return True
+
     def _get_working_path(self):
         cwd = self._loader.get_basedir()
         if self._task._role is not None:

test/integration/targets/ios_file/tests/cli/net_get.yaml

@@ -17,20 +17,23 @@
     src: ios1.cfg
   register: result
 
-- assert:
+- name: setup (remove file from localhost if present)
-    that:
+  file:
-      - result.changed == true
+      path: ios_{{ ansible_host }}.cfg
+      state: absent
+  delegate_to: localhost
 
-- name: get the file from device with dest unspecified
+- name: get the file from device with relative destination
   net_get:
     src: ios1.cfg
+    dest: 'ios_{{ ansible_host }}.cfg'
   register: result
 
 - assert:
     that:
       - result.changed == true
 
-- name: get the file from device with relative destination
+- name: Idempotency check
   net_get:
     src: ios1.cfg
     dest: 'ios_{{ ansible_host }}.cfg'
@@ -38,6 +41,6 @@
 
 - assert:
     that:
-      - result.changed == true
+      - result.changed == false
 
 - debug: msg="END ios cli/net_get.yaml on connection={{ ansible_connection }}"

test/integration/targets/ios_file/tests/cli/net_put.yaml

@@ -12,6 +12,24 @@
       - username {{ ansible_ssh_user }} privilege 15
     match: none
 
+- name: Delete existing file ios1.cfg if presen on remote host
+  ios_command:
+     commands:
+         - command: 'delete /force ios1.cfg'
+  ignore_errors: true
+
+- name: Delete existing file ios.cfg if presen on remote host
+  ios_command:
+     commands:
+         - command: 'delete /force ios.cfg'
+  ignore_errors: true
+
+- name: Delete existing file nonascii.bin if presen on remote host
+  ios_command:
+     commands:
+         - command: 'delete /force nonascii.bin'
+  ignore_errors: true
+
 - name: copy file from controller to ios + scp (Default)
   net_put:
     src: ios1.cfg
@@ -21,6 +39,15 @@
     that:
       - result.changed == true
 
+- name: Idempotency Check
+  net_put:
+    src: ios1.cfg
+  register: result
+
+- assert:
+    that:
+      - result.changed == false
+
 - name: copy file from controller to ios + dest specified
   net_put:
     src: ios1.cfg
@@ -34,7 +61,7 @@
 - name: copy file with non-ascii characters to ios in template mode(Fail case)
   net_put:
     src: nonascii.bin
-    mode: 'template'
+    mode: 'text'
   register: result
   ignore_errors: true