Salt: archive.extracted with options and user/group

[vquiering]

Description of Issue/Question

Since in version 2016.11.0 "tar_options" has been replaced by "options" my state fails. The error message says that one dir is missing. It is because of the option. It was working before with tar_options.
I got this in two different states. Both are failing since the update.

Setup

seafile-server:
  archive.extracted:
    - name: /opt/seafile/seafile-server-6.0.6
    - source: https://bintray.com/artifact/download/seafile-org/seafile/seafile-server_6.0.6_x86-64.tar.gz 
    - archive_format: tar
    - user: seafile
    - group: seafile
    - skip_verify: True
    - options: "--strip=1"

Steps to Reproduce Issue

salt-call state.sls seafile

      ID: seafile-server
Function: archive.extracted
    Name: /opt/seafile/seafile-server-6.0.6
  Result: False
 Comment: https://bintray.com/artifact/download/seafile-org/seafile/seafile-server_6.0.6_x86-64.tar.gz extracted to /opt/seafile/seafile-server-6.0.6/

          While trying to enforce user/group ownership, the following paths were missing:

          - /opt/seafile/seafile-server-6.0.6/seafile-server-6.0.6/
 Started: 13:23:38.251533
Duration: 2494.352 ms
 Changes:   
          ----------
          extracted_files:
              no tar output so far

Versions Report

Salt Version:
           Salt: 2016.11.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: 3.5.0
       dateutil: 2.2
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.3
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.9 (default, Jun 29 2016, 13:08:31)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.0
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: debian 8.6 
        machine: x86_64
        release: 3.16.0-4-amd64
         system: Linux
        version: debian 8.6

[Ch3LL]

@vquiering looks like i'm able to replicate this and here is a docker container for whoever wants to quickly replicate this:

1. docker run -it -v /home/ch3ll/git/salt/:/testing/ ch3ll/issues:38228 salt-call --local state.sls test -ldebug

ping @terminalmage I believe this is a bug but since the module changed just want to make sure its not a syntax error in the state that needs to be changed or anything like that. Thanks

[terminalmage]

This is not related to the deprecation, tar_options is converted to options under-the-hood.

This has to do, I think, with the strip option you are passing. 2016.11.0 improved the logic used to discover which files should exist if the archive is extracted, by running the new archive.list function. When you use --strip=1 to extract the archive, then a subset of the files get extracted, and that's where that error comes from-- the state expects files to be there that weren't extracted, because this logic isn't aware of the --strip option.

I'll work to detect and account for this.

[terminalmage]

@vquiering Looking at the archive you're trying to extract, you may no longer _need_ to use --strip, now that the state can detect which files should be extracted. Try changing your SLS to the following:

seafile-server:
  archive.extracted:
    - name: /opt/seafile
    - source: https://bintray.com/artifact/download/seafile-org/seafile/seafile-server_6.0.6_x86-64.tar.gz 
    - archive_format: tar
    - user: seafile
    - group: seafile
    - skip_verify: True

[terminalmage]

This is fixed in https://github.com/saltstack/salt/pull/38262. However I think you can safely work around this by not using --strip, as I mentioned in my previous comment.

[vquiering]

@terminalmage Thank you very much. This is now working.

[terminalmage]

@vquiering Is it working with the modified SLS, or with the pull request I opened last night?

[danlsgiga]

Having the same issue after upgrading to 2016.11.1 and using --strip 1 in my extraction options. The state is extracting the tar.gz file everytime I run the highstate causing a lot of load on the minion since there are a number of other triggers that depends on that specific state.
Glad to know a fix has been pushed and merged and hoping to get 2016.11.2 sooner than later.
Thanks!

[danlsgiga]

@terminalmage Not using --strip will extract the archive including the toplevel folder, which is exactly why we use the --strip 1 option for.

[terminalmage]

@danlsgiga Yeah. Some people used --strip so that they could set name (i.e. the path to which the archive would be extracted) as the top-level dir of the archive. This was one workaround to the fact that the state didn't have a good way of knowing whether or not to extract the archive, so it would either check for the existence of this extraction path, or the value of if_missing if passed. I added some more information to the docs explaining this here. Some people would use --strip, some would set if_missing; either approach worked around this basic deficiency of the state.

Indeed, looking at the archive that the OP was trying to extract, it seems like this was the reason for using --strip. That's not to say that there aren't other legitimate uses of --strip. But as a workaround to get Salt to extract the archive, it is no longer needed.

This was an oversight on my part when I worked to improve the state for the 2016.11.0 release. Sorry for the inconvenience. If it's any consolation, when I wrote the fix for this (https://github.com/saltstack/salt/issues/38262) I did add some integration tests which cover the use of --strip.

[danlsgiga]

Hi @terminalmage, I truly appreciate the job done. Thanks for that.

Just to clarify, here's my use case:
I want to extract the tomcat archive in /opt/tomcat, but since the tomcat archive contains the toplevel folder with its version and so forth, I don't want to have that in the fs. That's why I need to use --strip 1 to wipe out the toplevel folder and extract only the actual content of it.

After the change I also need to set the enforce_ownership_on to /opt/tomcat or the state fails trying to apply the permissions to the same structure that exists inside the archive, which won't be in the fs.

Small hiccups that are absolutely normal on big releases like 2016.11.

Anyway, I'm glad you tackled it in a fantastic time and it is already merged for the next point release, which is awesome.

[terminalmage]

@danlsgiga Thanks for being so understanding! So, your ownership enforcement workaround should also be unnecessary with 2016.11.2, as you can see from the below docker container that I created.

(~/git/salt/main is checked out to the head of the 2016.11 branch)

% docker run --rm -it -v ~/git/salt/main:/testing terminalmage/issues:38228
[root@63505564246d /]# salt-call --local state.single archive.extracted /tmp/virtualenv source=/root/virtualenv-15.1.0.tar.gz enforce_toplevel=False user=nobody group=nobody options='--strip 1'
local:
----------
          ID: /tmp/virtualenv
    Function: archive.extracted
      Result: True
     Comment: /root/virtualenv-15.1.0.tar.gz extracted to /tmp/virtualenv/
     Started: 01:04:25.644843
    Duration: 358.421 ms
     Changes:
              ----------
              directories_created:
                  - /tmp/virtualenv/
              extracted_files:
                  no tar output so far
              updated ownership:
                  True

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time: 358.421 ms
[root@63505564246d /]# salt-call --local state.single archive.extracted /tmp/virtualenv source=/root/virtualenv-15.1.0.tar.gz enforce_toplevel=False user=nobody group=nobody options='--strip 1'
local:
----------
          ID: /tmp/virtualenv
    Function: archive.extracted
      Result: True
     Comment: All files in archive are already present
     Started: 01:04:31.684363
    Duration: 351.423 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 351.423 ms
[root@63505564246d /]# ls -l /tmp/virtualenv/
total 128
-rw-r--r-- 1 nobody nobody  1224 Nov 16 02:39 AUTHORS.txt
-rw-r--r-- 1 nobody nobody  1180 Nov 16 02:39 LICENSE.txt
-rw-r--r-- 1 nobody nobody   345 Nov 16 02:39 MANIFEST.in
-rw-r--r-- 1 nobody nobody  3383 Nov 16 02:39 PKG-INFO
-rw-r--r-- 1 nobody nobody  1135 Nov 16 02:39 README.rst
drwxr-xr-x 2 nobody nobody    31 Nov 16 02:39 bin
drwxr-xr-x 2 nobody nobody   178 Nov 16 02:39 docs
drwxr-xr-x 2 nobody nobody    24 Nov 16 02:39 scripts
-rw-r--r-- 1 nobody nobody    88 Nov 16 02:39 setup.cfg
-rw-r--r-- 1 nobody nobody  4043 Nov 16 02:39 setup.py
drwxr-xr-x 2 nobody nobody   135 Nov 16 02:39 tests
drwxr-xr-x 2 nobody nobody   134 Nov 16 02:39 virtualenv.egg-info
-rwxr-xr-x 1 nobody nobody 99021 Nov 16 02:39 virtualenv.py
drwxr-xr-x 2 nobody nobody   234 Nov 16 02:39 virtualenv_embedded
drwxr-xr-x 2 nobody nobody   193 Nov 16 02:39 virtualenv_support

[danlsgiga]

@terminalmage Awesome!! Any idea when 2016.11.2 is coming out?

[terminalmage]

2016.11.1 is going through final testing and is due next Thursday/Friday, so unfortunately this fix won't see a release until probably late January or early February, but those are just guesses. We've been efforting monthly point releases lately, but the holidays may delay things.

[danlsgiga]

2016.11.1 has been released this week if I'm correct. Just wondering since there are couple critical issues that will be fixed in this next release. Anyhow, it will come out when it has to. Thanks @terminalmage, really appeciated!