Skip to content

Conversation

@meaksh
Copy link
Contributor

@meaksh meaksh commented Mar 27, 2020

What does this PR do?

This PR fixes a typo when sanitizing the kwargs that are passed to msgpack. Particularly, the raw parameter is introduced on version 0.5.2 and not in 0.5.5. This typo is clashing with https://github.com/saltstack/salt/blob/master/salt/payload.py#L108 so, in case you're running with i.a. msgpack version 0.5.4, this conflict will lead you to some problems when managing the file_list fileserver cache.

One collateral effect of this, is file.recurse state not working.

Example using msgpack 0.5.4:

srv:/srv/salt # find .
.
./foobar1_dir
./foobar1_dir/testfile
./test_state.sls
./foobar2_dir
./foobar2_dir/testfile

srv:/srv/salt # cat test_state.sls 
dummy_state:
  file.recurse:
    - name: /tmp/foobar1
    - source: salt://foobar1_dir

srv:/srv/salt # salt-call --local state.apply test_state
[ERROR   ] Recurse failed: none of the specified sources were found
local:
----------
          ID: dummy_state
    Function: file.recurse
        Name: /tmp/foobar1
      Result: False
     Comment: Recurse failed: none of the specified sources were found
     Started: 16:49:38.303405
    Duration: 30.068 ms
     Changes:   

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

With version 0.5.6 (or with the changes from this PR):

srv:/srv/salt # salt-call --local state.apply test_state

local:
----------
          ID: dummy_state
    Function: file.recurse
        Name: /tmp/foobar1
      Result: True
     Comment: Recursively updated /tmp/foobar1
     Started: 16:47:03.436409
    Duration: 87.635 ms
     Changes:   
              ----------
              /tmp/foobar1/testfile:
                  ----------
                  diff:
                      New file
                  mode:
                      0644

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  87.635 ms

Tests written?

[NOTICE] Bug fixes or features added to Salt require tests.
Please review the test documentation for details on how to implement tests into Salt's test suite.

Yes/No

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@meaksh meaksh requested a review from a team as a code owner March 27, 2020 16:14
@ghost ghost requested a review from dwoz March 27, 2020 16:14
@DmitryKuzmenko
Copy link
Contributor

DmitryKuzmenko
DmitryKuzmenko previously approved these changes Apr 3, 2020
@dwoz
Copy link
Contributor

dwoz commented Apr 11, 2020

@dwoz dwoz merged commit 76b933f into saltstack:master Apr 11, 2020
@sagetherage sagetherage added the ZRelease-Sodium retired label label May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ZRelease-Sodium retired label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants