search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

file.recurse with multiple sources unexpected behavior when dynamic sources are provided. #54102

Open ghost opened 5 years ago

ghost commented 5 years ago

Description of Issue

When multiple sources are provided in file.recurse, and if one of the sources may be excluded by some grains data, file.recurse picks up the last source instead of the first source.

Setup

minion grains

$ echo xfiles: foobar > /etc/salt/grains

/srv/salt/base/init.sls

"update /xfiles":
  file.recurse:
  - replace: True
  - source:
    - salt://xfiles/{{ grains.id }}/
    {%- if salt["grains.get"]("xfiles") | length > 0  -%}
    - salt://xfiles/{{ salt["grains.get"]("xfiles") }}/
    {%- endif %}
    - salt://xfiles/_default/
  - name: /salt-test/

/srv/salt/xfiles/

# controller-111.internal is my salt-minion's ID
$ mkdir -pv /srv/salt/xfiles/{_default,foobar,controller-111.internal}
$ echo from _default > xfiles/_default/.empty
$ echo from foobar > xfiles/foobar/.empty
$ echo from top level > xfiles/controller-111.internal/.empty

Steps to Reproduce Issue

Step 1: With dynamic list of sources

Now execute highstate on the minion, and see the results

root@controller-111:/srv/salt# salt '*' state.highstate 
controller-111.internal:
----------
          ID: update /xfiles
    Function: file.recurse
        Name: /salt-test/
      Result: True
     Comment: The directory /salt-test/ is in the correct state
     Started: 15:59:48.480779
    Duration: 49.9999999993 ms
     Changes:   

Summary for controller-111.internal
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time:  50.000 ms

The result is not as expected

$ cat /salt-test/.empty 
from _default

Step 2: Without dynamic list of sources:

Now uncomment the dynamic block

"update /xfiles":
  file.recurse:
  - replace: True
  - source:
    - salt://xfiles/{{ grains.id }}/
    #{%- if salt["grains.get"]("xfiles") | length > 0  -%}
    #- salt://xfiles/{{ salt["grains.get"]("xfiles") }}/
    #{%- endif %}
    - salt://xfiles/_default/
  - name: /salt-test/

Apply the new state and see the expected resulted file

root@controller-111:/srv/salt# salt '*' state.highstate
controller-111.internal:
----------
          ID: update /xfiles
    Function: file.recurse
        Name: /salt-test/
      Result: True
     Comment: Recursively updated /salt-test/
     Started: 16:02:21.186379
    Duration: 90.0000000001 ms
     Changes:   
              ----------
              /salt-test/.empty:
                  ----------
                  diff:
                      --- 
                      +++ 
                      @@ -1 +1 @@
                      -from _default
                      +from top level

Summary for controller-111.internal
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  90.000 ms

The resulted file is execpted:

$; cat /salt-test/.empty 
from top level

Versions Report


root@controller-111:~/src/salt-bootstrap# salt --versions-report
Salt Version:
           Salt: 2019.8.0-50-g57fd04b

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: 0.27.0
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.15+ (default, Nov 27 2018, 23:36:35)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-55-generic
         system: Linux
        version: Ubuntu 18.04 bionic

``
ghost commented 5 years ago

I think this may not be a bug. According to the documentation https://docs.saltstack.com/en/latest/ref/states/all/salt.states.file.html#salt.states.file.recurse is supposed to receive a single source (not a list of source) . I see some ticket with multiple sources though (e.g, https://github.com/saltstack/salt/issues/9304)

garethgreenaway commented 5 years ago

@kyanh-vX7HLAHZLPsUxeHD Multiple sources are supported and the code should pick the first one from the list that exists. If you remove the if statement with the grains does it work as expected and the first one in the list that exists is picked?

icy commented 5 years ago

@garethgreenaway Yes it works well when I remove the if statement. The test setup is on my office workstation I will give more details on Monday. Thank you.

ghost commented 5 years ago

If you remove the if statement with the grains does it work as expected and the first one in the list that exists is picked?

Yes, it does, as seen below

# remove the `if` statement

"update /xfiles":
  file.recurse:
  - replace: True
  - source:
    - salt://xfiles/{{ grains.id }}/
    - salt://xfiles/{{ grains.xfiles }}/
    - salt://xfiles/_default/
  - name: /salt-test

# Now run highstate command. The result is as expected

root@controller-111:/srv/salt# salt '*' state.highstate 
controller-111.internal:
----------
          ID: update /xfiles
    Function: file.recurse
        Name: /salt-test/
      Result: True
     Comment: Recursively updated /salt-test/
     Started: 07:06:59.594404
    Duration: 89.9999999965 ms
     Changes:
              ----------
              /salt-test/.empty:
                  ----------
                  diff:
                      ---
                      +++
                      @@ -1 +1 @@
                      -from _default
                      +from top level

Summary for controller-111.internal
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  90.000 ms

; cat /salt-test/.empty
from top level
ghost commented 5 years ago

I've found that the use of -%} does matter (whiltespace controlling in Jinjra template). The following code will generate errors

# Using -%}

    {%- if salt["grains.get"]("xfiles") | length > 0 -%}
    - salt://xfiles/{{ salt["grains.get"]("xfiles") }}/
    {%- endif -%}

root@controller-111:/srv/salt# salt '*' state.highstate ; cat /salt-test/.empty 
controller-111.internal:
----------
          ID: update /xfiles
    Function: file.recurse
        Name: /salt-test/
      Result: False
     Comment: Recurse failed: none of the specified sources were found
     Started: 09:49:26.318829
    Duration: 29.9999999988 ms
     Changes:

Now remove them both:

    {%- if salt["grains.get"]("xfiles") | length > 0 %}
    - salt://xfiles/{{ salt["grains.get"]("xfiles") }}/
    {%- endif %}
    - salt://xfiles/_default/

This code works perfectly. I think I totally misunderstood how whitespace control works.