Salt: archive.extracted issue when source_hash_update=True and extracted files does not exist

[amontalban]

Description of Issue/Question

Not sure if I'm not using archive.extracted properly or if this is a bug, I think that it should check for the tarball content besides the hashes are not changed and extract if files are missing.

Setup

perlbrew:
  archive.extracted:
    - name: /
    - source: http://perl.company.com/perlbrew.tar.gz
    - source_hash: http://perl.company.com/sha256sum.txt
    - source_hash_update: True
    - overwrite: True

Output

[DEBUG   ] LazyLoaded archive.extracted
[INFO    ] Running state [/] at time 04:15:28.018056
[INFO    ] Executing state archive.extracted for /
[DEBUG   ] Requesting URL http://perl.company.com/sha256sum.txt using GET method
[DEBUG   ] file.extract_hash: Extracting any supported hash for file matching: http://perl.company.com/perlbrew.tar.gz
[DEBUG   ] file.extract_hash: Line '009f621fbadbea9999c4a33fcb3c658703315344a4e9c0be4e3685a94d262d73  perlbrew.tar.gz' matches source 'http://perl.company.com/perlbrew.tar.gz'
[DEBUG   ] file.extract_hash: Line '44e5284d273dc486ec40ab3360a8bfcee6b83a06529ddfb1202dbdf905c39665  company_mfa.tar.gz' contains sha256 hash '44e5284d273dc486ec40ab3360a8bfcee6b83a06529ddfb1202dbdf905c39665', but line did not meet the search criteria
[DEBUG   ] file.extract_hash: Line '951c397e88fec12933657751b7bda8b21a4b683a3591cd12c8fb84f54ecacf47  company_transtool_api.tar.gz' contains sha256 hash '951c397e88fec12933657751b7bda8b21a4b683a3591cd12c8fb84f54ecacf47', but line did not meet the search criteria
[DEBUG   ] file.extract_hash: Returning sha256 hash '009f621fbadbea9999c4a33fcb3c658703315344a4e9c0be4e3685a94d262d73' as a match of http://perl.company.com/perlbrew.tar.gz
[INFO    ] Hash 009f621fbadbea9999c4a33fcb3c658703315344a4e9c0be4e3685a94d262d73 has not changed
[INFO    ] Completed state [/] at time 04:15:28.138946 duration_in_ms=120.89

Steps to Reproduce Issue

Run the above state, then remove the extracted tarball path and run the state again. Salt will compare hashes and not extract as stated in documentation:

Salt now uses a function called archive.list to get a list of files/directories in the archive. Using this information, the state can now check the minion to see if any paths are missing, and know whether or not the archive needs to be extracted. This makes the if_missing argument unnecessary in most use cases.

Versions Report

Salt Version:
           Salt: 2016.11.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.5.3
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.7
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.13 (default, Jan 12 2017, 01:15:13)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 16.0.1
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.2
            ZMQ: 4.1.5

System Versions:
           dist:   
        machine: amd64
        release: 10.3-RELEASE-p11
         system: FreeBSD
        version: Not Installed

[gtmanfred]

This appears to be a bug with source_hash_update, Where it will only update if the source_hash changes.

[root@salt ~]# cat /srv/salt/test.sls
domjudge:
  archive.extracted:
    - name: /opt/
    - source: https://www.domjudge.org/releases/domjudge-5.1.3.tar.gz
    - source_hash: https://www.domjudge.org/releases/domjudge-5.1.3.tar.gz.sha256sum
    - source_hash_update: True

[root@salt ~]# salt-call state.apply test
[WARNING ] /usr/lib/python2.7/site-packages/salt/states/file.py:1557: DeprecationWarning: Parameter 'env' has been detected in the argument list.  This parameter is no longer used and has been replaced by 'saltenv' as of Salt 2016.11.0.  This warning will be removed in Salt Oxygen.

local:
----------
          ID: domjudge
    Function: archive.extracted
        Name: /opt/
      Result: True
     Comment: https://www.domjudge.org/releases/domjudge-5.1.3.tar.gz extracted to /opt/
     Started: 17:04:10.806233
    Duration: 2886.499 ms
     Changes:
              ----------
              extracted_files:
                  - domjudge-5.1.3
                  - domjudge-5.1.3/configure
                  - domjudge-5.1.3/aclocal.m4
<--snip-->
                  - domjudge-5.1.3/COPYING
                  - domjudge-5.1.3/CODINGSTYLE
                  - domjudge-5.1.3/.travis.yml

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   2.886 s
[root@salt ~]# rm /opt/domjudge-5.1.3/COPYING
rm: remove regular file ‘/opt/domjudge-5.1.3/COPYING’? y
[root@salt ~]# salt-call state.apply test
local:
----------
          ID: domjudge
    Function: archive.extracted
        Name: /opt/
      Result: True
     Comment: Hash 309db88b673f3688f48cddac7cb419900ee2a86387dce7413921b669432e91f8 has not changed
     Started: 17:04:28.925541
    Duration: 473.766 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 473.766 ms
[root@salt ~]# rm -rf /opt/domjudge-5.1.3/
[root@salt ~]# salt-call state.apply test
local:
----------
          ID: domjudge
    Function: archive.extracted
        Name: /opt/
      Result: True
     Comment: Hash 309db88b673f3688f48cddac7cb419900ee2a86387dce7413921b669432e91f8 has not changed
     Started: 17:04:35.777019
    Duration: 493.775 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 493.775 ms

Without the source_hash_update: True this works.

@terminalmage you might be interested in this.

Thanks,
Daniel

[terminalmage]

It was designed to work the way that it does, I carried over that feature from 2016.3 when I rewrote the state for 2016.11. However, the behavior does not quite match the documentation, and there is definitely a use case for proceeding with extraction in the event that files are missing. I'll get this taken care of for 2016.11.4.

[jerrykan]

I have experience this issue which had me scratching my head for quite a while... but I've also hit a related use-case that would be worth taking into consideration

Say I have the following state:

file-archive:
  archive.extracted:
    - name: /opt/tarball
    - source: http://fileserver/tarball-v1.tar.gz
    - source_hash: md5sum=<md5_hash_of_tarball_v1>
    - source_hash_update: True
    - options: '--strip-components=1'
    - enforce_toplevel: False

This creates the hash file like /var/cache/salt/minion/files/base/_tarball-v1.tar.gz.hash

I then want to push out an updated archive, so I update the state source and source_hash as follows:

file-archive:
  archive.extracted:
    - name: /opt/tarball
    - source: http://fileserver/tarball-v2.tar.gz
    - source_hash: md5sum=<md5_hash_of_tarball-v2>
    - source_hash_update: True
    - options: '--strip-components=1'
    - enforce_toplevel: False

Which then creates a hash file like /var/cache/salt/minion/files/base/_tarball-v2.tar.gz.hash

If I then realise there was a mistake with the updated archive and I want to push out the original archive again, I revert the changes to the state, but because the /var/cache/salt/minion/files/base/_tarball-v2.tar.gz.hash file already exists nothing will be extracted.

Maybe the hash file that is created should be associated with the state id and not the source file? This would catch if the source_hash has been updated either because the source file contents has changed, or a different source is being used.

[jerrykan]

I have also created issues #40482 and #40484 which may be related, but I wasn't totally sure. I figured I'd put them in their own issues in case they need to be tracked separately.

[terminalmage]

Fixed in https://github.com/saltstack/salt/pull/40551.

[jerrykan]

I can confirm that this seems to be working as expected in the develop branch

[jerrykan]

Sorry, ignore that last comment, I wasn't testing it correctly. Plus the changes haven't made their way to the develop branch yet :D

[jerrykan]

I've tested this against saltstack/salt#40551 and the archive is extracted if the exploded path has been removed. This works both if source_hash_update is set to True or not.