Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix salt-ssh pillar bug for state.sls_id function #53348

Merged
merged 10 commits into from
Apr 29, 2020

Conversation

xuhcc
Copy link
Contributor

@xuhcc xuhcc commented Jun 3, 2019

What does this PR do?

Fixes #51353

Previous Behavior

salt-ssh ignored pillar CLI argument during execution of state.sls_id function

New Behavior

now it should work

Tests written?

No

Commits signed with GPG?

No

Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for pushing the fix for this :) really appreciate it. any chance we can get a test added here to insure this does not regress in the future? Should be pretty easy to use the current state.sls_id tests in tests/integration/ssh/test_state.py for an idea on how to approach the test.

@Ch3LL Ch3LL added the bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch label Jun 5, 2019
@xuhcc
Copy link
Contributor Author

xuhcc commented Jun 6, 2019

@Ch3LL Unfortunately I can't get integration tests working.

$ python tests/runtests.py -n integration.ssh.test_state.SSHStateTest.test_state_sls_id
 --------  Skipped Tests  ------------------------
   -> integration.ssh.test_state.SSHStateTest.test_state_sls_id  ->  SSH tests are disabled

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 6, 2019

what if you add the arg --ssh

@Ch3LL Ch3LL added the ZRELEASED - Neon retired label label Jun 6, 2019
@Ch3LL Ch3LL changed the base branch from develop to neon June 6, 2019 21:23
@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 6, 2019

FYI I migrated this PR from develop to neon to ensure it is included in the upcoming neon release. Let me know if this caused any issues. Thanks

@xuhcc
Copy link
Contributor Author

xuhcc commented Jun 6, 2019

@Ch3LL --ssh worked, thank you! I added the test.

tests/integration/ssh/test_state.py Outdated Show resolved Hide resolved
@xuhcc xuhcc requested a review from a team as a code owner February 12, 2020 13:38
@ghost ghost requested a review from waynew February 12, 2020 13:38
@garethgreenaway garethgreenaway added ZRelease-Sodium retired label and removed ZRELEASED - Neon retired label labels Feb 14, 2020
waynew
waynew previously approved these changes Mar 10, 2020
@waynew
Copy link
Contributor

waynew commented Mar 10, 2020

re-run all

@sagetherage
Copy link
Contributor

@xuhcc will you rebase against master, please?

@xuhcc xuhcc dismissed waynew’s stale review April 1, 2020 21:20

The base branch was changed.

@xuhcc xuhcc changed the base branch from neon to master April 1, 2020 21:20
@xuhcc
Copy link
Contributor Author

xuhcc commented Apr 2, 2020

@sagetherage Done!

@Ch3LL
Copy link
Contributor

Ch3LL commented Apr 23, 2020

@xuhcc looks like there is a merge conflict here.

@Ch3LL
Copy link
Contributor

Ch3LL commented Apr 27, 2020

looks like just a lint and pre-commit failure now.

@dwoz dwoz merged commit 671eb1e into saltstack:master Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch Reviewers-Assigned ZRelease-Sodium retired label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

state.sls_id fails to read pillar data from CLI arg
7 participants