Updates for the unarchive module and action_plugin.

There is a bit going on with the changes here. Most of the changes are cleanup of files so that they line up with the standard files.

PR #5136 was merged into the current devel and brought up to working order. A few bug fixes had to be done to get the code to test correctly. Thanks out to @pib!

Issue #5431 was not able to be confirmed as it behaved as expected with a sudo user.

Tests were added via a playbook with archive files to verify functionality.

All tests fire clean including custom playbooks across multiple linux and solaris systems.

lib/ansible/runner/action_plugins/unarchive.py

@@ -28,11 +28,9 @@ from ansible.runner.return_data import ReturnData
 import sys
 reload(sys)
 sys.setdefaultencoding("utf8")
-#import base64
-#import stat
-#import tempfile
 import pipes
 
+
 class ActionModule(object):
 
     TRANSFERS_FILES = True
@@ -53,7 +51,7 @@ class ActionModule(object):
         copy    = utils.boolean(options.get('copy', 'yes'))
 
         if source is None or dest is None:
-            result=dict(failed=True, msg="src (or content) and dest are required")
+            result = dict(failed=True, msg="src (or content) and dest are required")
             return ReturnData(conn=conn, result=result)
 
         source = template.template(self.runner.basedir, source, inject)
@@ -65,7 +63,7 @@ class ActionModule(object):
 
         remote_md5 = self.runner._remote_md5(conn, tmp, dest)
         if remote_md5 != '3':
-            result = dict(failed=True, msg="dest must be an existing dir")
+            result = dict(failed=True, msg="dest must be an existing dir", rc=remote_md5)
             return ReturnData(conn=conn, result=result)
 
         if copy:

library/files/unarchive

@@ -65,35 +65,37 @@ EXAMPLES = '''
 - unarchive: src=foo.tgz dest=/var/lib/foo
 '''
 
-
 import os
+
+
 # class to handle .zip files
 class _zipfile(object):
     
-    def __init__(self,src,dest,module):
+    def __init__(self, src, dest, module):
         self.src = src
         self.dest = dest
         self.module = module
 
     def is_unarchived(self):
-        return dict(bool = False)
+        return dict(unarchived=False)
 
     def unarchive(self):
-        cmd = 'unzip -o "%s" -d "%s"'  % (self.src,self.dest)
+        cmd = 'unzip -o "%s" -d "%s"' % (self.src, self.dest)
         rc, out, err = self.module.run_command(cmd)
-        return dict(cmd = cmd, rc=rc, out=out, err=err)
+        return dict(cmd=cmd, rc=rc, out=out, err=err)
 
     def can_handle_archive(self):
-        cmd = 'unzip -l "%s"' % (self.src)
+        cmd = 'unzip -l "%s"' % self.src
         rc, out, err = self.module.run_command(cmd)
         if rc == 0:
             return True
         return False
 
+
 # class to handle gzipped tar files
 class _tgzfile(object):
     
-    def __init__(self,src,dest,module):
+    def __init__(self, src, dest, module):
         self.src = src
         self.dest = dest
         self.module = module
@@ -102,56 +104,62 @@ class _tgzfile(object):
     def is_unarchived(self):
         dirof = os.path.dirname(self.dest)
         destbase = os.path.basename(self.dest)
-        cmd = 'tar -v -C "%s" --diff -%sf "%s"'  % (self.dest, self.zipflag,self.src)
+        cmd = 'tar -v -C "%s" --diff -%sf "%s"' % (self.dest, self.zipflag, self.src)
         rc, out, err = self.module.run_command(cmd)
-        bool = (rc == 0)
+        unarchived = (rc == 0)
-        return dict( bool = bool, rc = rc , out = out, err = err, cmd = cmd)
+        return dict(unarchived=unarchived, rc=rc, out=out, err=err, cmd=cmd)
 
     def unarchive(self):
-        cmd = 'tar -C "%s" -x%sf "%s"' % (self.dest,self.zipflag,self.src)
+        cmd = 'tar -C "%s" -x%sf "%s"' % (self.dest, self.zipflag, self.src)
         rc, out, err = self.module.run_command(cmd)
-        return dict(cmd = cmd, rc=rc, out=out, err=err)
+        return dict(cmd=cmd, rc=rc, out=out, err=err)
 
     def can_handle_archive(self):
-        cmd = 'tar -t%sf "%s"' % (self.zipflag,self.src)
+        cmd = 'tar -t%sf "%s"' % (self.zipflag, self.src)
         rc, out, err = self.module.run_command(cmd)
         if rc == 0:
+            if len(out.splitlines(True)) > 0:
                 return True
         return False
 
+
 # class to handle tar files that aren't compressed
 class _tarfile(_tgzfile):
-    def __init__(self,src,dest,module):
+    def __init__(self, src, dest, module):
         self.src = src
         self.dest = dest
         self.module = module
         self.zipflag = ''
 
+
 # class to handle bzip2 compressed tar files
 class _tarbzip(_tgzfile):
-    def __init__(self,src,dest,module):
+    def __init__(self, src, dest, module):
         self.src = src
         self.dest = dest
         self.module = module
         self.zipflag = 'j'
 
+
 # class to handle xz compressed tar files
 class _tarxz(_tgzfile):
-    def __init__(self,src,dest,module):
+    def __init__(self, src, dest, module):
         self.src = src
         self.dest = dest
         self.module = module
         self.zipflag = 'J'
 
+
 # try handlers in order and return the one that works or bail if none work
-def pick_handler(src,dest,module):
+def pick_handler(src, dest, module):
     handlers = [_tgzfile, _zipfile, _tarfile, _tarbzip, _tarxz]
     for handler in handlers:
-        obj = handler(src,dest,module)
+        obj = handler(src, dest, module)
         if obj.can_handle_archive():
             return obj
     raise RuntimeError('Failed to find handler to unarchive "%s"' % src)
 
+
 def main():
     module = AnsibleModule(
         # not checking because of daisy chain to file module
@@ -166,37 +174,36 @@ def main():
 
     src    = os.path.expanduser(module.params['src'])
     dest   = os.path.expanduser(module.params['dest'])
+    copy   = module.params['copy']
 
     # did tar file arrive?
     if not os.path.exists(src):
         if copy:
-            module.fail_json(msg="Source '%s' failed to transfer" % (src))
+            module.fail_json(msg="Source '%s' failed to transfer" % src)
         else:
-            module.fail_json(msg="Source '%s' does not exist" % (src))
+            module.fail_json(msg="Source '%s' does not exist" % src)
     if not os.access(src, os.R_OK):
-        module.fail_json(msg="Source '%s' not readable" % (src))
+        module.fail_json(msg="Source '%s' not readable" % src)
 
-    # is dest OK to recieve tar file?
+    # is dest OK to receive tar file?
     if not os.path.exists(os.path.dirname(dest)):
         module.fail_json(msg="Destination directory '%s' does not exist" % (os.path.dirname(dest)))
     if not os.access(os.path.dirname(dest), os.W_OK):
         module.fail_json(msg="Destination '%s' not writable" % (os.path.dirname(dest)))
 
-    handler = pick_handler(src,dest,module)
+    handler = pick_handler(src, dest, module)
 
-    res_args = dict( handler=handler.__class__.__name__, dest = dest, src = src )
+    res_args = dict(handler=handler.__class__.__name__, dest=dest, src=src)
 
     # do we need to do unpack?
-    namelist = ['bool','rc','out','err','cmd']
     res_args['check_results'] = handler.is_unarchived()
-    if res_args['check_results']['bool']:
+    if res_args['check_results']['unarchived']:
         res_args['changed'] = False
         module.exit_json(**res_args)
 
     # do the unpack
     try:
         results = handler.unarchive()
-        #results = (src,dest,module)
     except IOError:
         module.fail_json(msg="failed to unpack %s to %s" % (src, dest))
 

test/TestPlayBook.py

@@ -82,6 +82,7 @@ class TestCallbacks(object):
     def on_no_hosts(self):
         pass
 
+
 class TestPlaybook(unittest.TestCase):
 
     def setUp(self):
@@ -119,8 +120,7 @@ class TestPlaybook(unittest.TestCase):
         filename = os.path.join(self.stage_dir, filename)
         return filename
 
-   def _run(self, test_playbook, host_list='test/ansible_hosts', 
+    def _run(self, test_playbook, host_list='test/ansible_hosts', extra_vars=None):
-            extra_vars=None):
         ''' run a module and get the localhost results '''
         # This ensures tests are independent of eachother
         global EVENTS
@@ -410,6 +410,23 @@ class TestPlaybook(unittest.TestCase):
         assert utils.jsonify(expected, format=True) == utils.jsonify(actual,format=True)
 
 
+    def test_unarchive(self):
+        pb = 'test/playbook-unarchive.yml'
+        actual = self._run(pb)
+
+        expected =  {
+            "localhost": {
+                "changed": 41,
+                "failures": 0,
+                "ok": 45,
+                "skipped": 0,
+                "unreachable": 0
+            }
+        }
+
+        assert utils.jsonify(expected, format=True) == utils.jsonify(actual,format=True)
+
+
     def _compare_file_output(self, filename, expected_lines):
         actual_lines = []
         with open(filename) as f:

test/playbook-unarchive.yml

@@ -0,0 +1,78 @@
+---
+# To run me manually, use: -i "localhost,"
+- hosts: localhost
+  connection: local
+  gather_facts: no
+  vars:
+    - testdir: /tmp/ansible-unarchive
+    - filesdir: test_unarchive/files
+  tasks:
+
+   - name: "Simple tar unarchive."
+     command: rm -rf {{testdir}}
+   - file: state=directory dest={{testdir}}
+   - unarchive: src={{filesdir}}/test.tar dest={{testdir}}
+     register: res
+   - command: test -f {{testdir}}/foo
+   - command: test "{{res.changed}}" = "True"
+   - unarchive: src={{filesdir}}/test.tar dest={{testdir}}
+     register: res
+   - command: test "{{res.changed}}" = "False"
+
+   - name: "Simple tar.gz unarchive."
+     command: rm -rf {{testdir}}
+   - file: state=directory dest={{testdir}}
+   - unarchive: src={{filesdir}}/test.tar.gz dest={{testdir}}
+     register: res
+   - command: test -f {{testdir}}/foo
+   - command: test "{{res.changed}}" = "True"
+   - unarchive: src={{filesdir}}/test.tar.gz dest={{testdir}}
+     register: res
+   - command: test "{{res.changed}}" = "False"
+
+   - name: "Simple zip unarchive."
+     command: rm -rf {{testdir}}
+   - file: state=directory dest={{testdir}}
+   - unarchive: src={{filesdir}}/test.zip dest={{testdir}}
+     register: res
+   - command: test -f {{testdir}}/foo
+   - command: test "{{res.changed}}" = "True"
+   - unarchive: src={{filesdir}}/test.zip dest={{testdir}}
+     register: res
+   - command: test "{{res.changed}}" = "True"
+
+   - name: "Unarchive a local tar file."
+     command : rm -rf {{testdir}}
+   - file: state=directory dest={{testdir}}
+   - copy: src={{filesdir}}/test.tar dest={{testdir}}
+   - unarchive: src={{testdir}}/test.tar dest={{testdir}}
+     register: res
+   - command: test -f {{testdir}}/foo
+   - command: test "{{res.changed}}" = "True"
+   - unarchive: src={{testdir}}/test.tar dest={{testdir}}
+     register: res
+   - command: test "{{res.changed}}" = "False"
+
+   - name: "Unarchive a local tar.gz file."
+     command : rm -rf {{testdir}}
+   - file: state=directory dest={{testdir}}
+   - copy: src={{filesdir}}/test.tar.gz dest={{testdir}}
+   - unarchive: src={{testdir}}/test.tar.gz dest={{testdir}}
+     register: res
+   - command: test -f {{testdir}}/foo
+   - command: test "{{res.changed}}" = "True"
+   - unarchive: src={{testdir}}/test.tar.gz dest={{testdir}}
+     register: res
+   - command: test "{{res.changed}}" = "False"
+
+   - name: "Unarchive a local zip file."
+     command : rm -rf {{testdir}}
+   - file: state=directory dest={{testdir}}
+   - copy: src={{filesdir}}/test.zip dest={{testdir}}
+   - unarchive: src={{testdir}}/test.zip dest={{testdir}}
+     register: res
+   - command: test -f {{testdir}}/foo
+   - command: test "{{res.changed}}" = "True"
+   - unarchive: src={{testdir}}/test.zip dest={{testdir}}
+     register: res
+   - command: test "{{res.changed}}" = "True"