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.

Sign up for GitHub

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

Jump to bottom

Fix test output for running services #60150

Merged
merged 1 commit into from May 7, 2021
Merged

Fix test output for running services #60150

merged 1 commit into from May 7, 2021

Conversation

dmurphy18
Copy link
Contributor

What does this PR do?

When using test=True with service running, output correctly what will happen to a service that is already running, namely restart

What issues does this PR fix or reference?

Fixes: #60120

Previous Behavior

With test=True, the service.running output showed what status the service was in and not what would happen to it

Comment: The service nginx is already running

New Behavior

With test=True, the service output now reflects that the service will be restarted when appropriate.

Comment: The service nginx is set to restart

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

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.

@dmurphy18 dmurphy18 requested a review from a team as a code owner May 6, 2021 19:03
@dmurphy18 dmurphy18 requested review from garethgreenaway and removed request for a team May 6, 2021 19:03
@garethgreenaway garethgreenaway merged commit 1f74210 into saltstack:master May 7, 2021
@dmurphy18 dmurphy18 mentioned this pull request Jun 1, 2021
Closed
@OrangeDog OrangeDog mentioned this pull request Oct 6, 2021
Closed
garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Oct 8, 2021
@garethgreenaway
Reverting changes in PR saltstack#60150. Updating installed and remov…
c9e63c4 
…ed functions to return changes when test=True.
@garethgreenaway garethgreenaway mentioned this pull request Oct 8, 2021
Closed
3 tasks
garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Oct 8, 2021
@garethgreenaway
Reverting changes in PR saltstack#60150. Updating installed and remov…
d19821f 
…ed functions to return changes when test=True.
@garethgreenaway garethgreenaway mentioned this pull request Oct 8, 2021
Merged
3 tasks
Ch3LL pushed a commit that referenced this pull request Oct 11, 2021
@garethgreenaway
Reverting changes in PR #60150. Updating installed and removed functi…
d1587a3 
…ons to return changes when test=True.
Ch3LL pushed a commit to Ch3LL/salt that referenced this pull request Oct 12, 2021
@garethgreenaway
Reverting changes in PR saltstack#60150. Updating installed and remov…
59d0710 
…ed functions to return changes when test=True.
Ch3LL pushed a commit that referenced this pull request Oct 22, 2021
@garethgreenaway
Reverting changes in PR #60150. Updating installed and removed functi…
29e7e19 
…ons to return changes when test=True.
Ch3LL pushed a commit that referenced this pull request Dec 13, 2021
@garethgreenaway @s0undt3ch
@ScriptAutomate @waynew @nicholasmhughes @dwoz @meaksh @twangboy
Merge freeze into master (#61310)
0ae12d2 
* Redirect imports of ``salt.ext.six`` to ``six``

Fixes #60966

* Latest changelog update for 3004

* Handle signals and properly exit, instead of raising exceptions.

This was introduced in 26fcda5

Fixes #60391
Fixes #60963

* Add test for #61003

* Fix #61003

Restored the previously shifted check for version_to_remove in
old[target]. This had been extracted along with the correctly extracted
double pkg_params[target] lookup, but that lost the `target in old`
guard.

Putting the check back here prevents KeyError when looking for a
non-existent target in `old`.

* Handle various architecture formats in aptpkg module

* Write file even if does not exist

* only run test on debian based platforms

* remove extra space for arch

* convert pathlib to string for pkgrepo test

* Use temporary files first then copy to sources files

* fixes #59182 fix handling of duplicate keys in rest_cherrypy data

* added changelog

* remove log messages to prevent leaks of sensitive info

* Reverting changes in PR #60150. Updating installed and removed functions to return changes when test=True.

* Adding changelog.

* Add a test and fix for extra-filerefs

* Do not break master_tops for minion with version lower to 3003

* Add changelog file

* Add extra comment to clarify discussion

* Update changelog file

* Add deprecated changelog

* Assert that the command didn't finish

Refs #60972

* Always restore signals, even when exceptions occur

* Reset signal handlers before starting the process

* Make sure that the `ProcessManager` doesn't always ignore signals

* Provide valid default value for bootstrap_delay

* Update changelog for 3004

* Update changelog and release notes for 3004

* Add PR 61020 to changelog

* Update release notes index file and create 3005 file

* Update docs

* bad merge from master, redoing the PR for adding the Deltaproxy documentation.

* Adding doc/ref/configuration/delta_proxy.rst.

* cleaning up this paragraph, adding in network administrators in addition to system administrators.

Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: ScriptAutomate <derek@icanteven.io>
Co-authored-by: Wayne Werner <wwerner@vmware.com>
Co-authored-by: Megan Wilhite <mwilhite@vmware.com>
Co-authored-by: nicholasmhughes <nicholasmhughes@gmail.com>
Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com>
Co-authored-by: Pablo Suárez Hernández <psuarezhernandez@suse.com>
Co-authored-by: Alyssa Rock <arock@saltstack.com>
Co-authored-by: Twangboy <shane.d.lee@gmail.com>
garethgreenaway added a commit to bryceml/salt that referenced this pull request Jan 21, 2022
@garethgreenaway
Reverting changes in PR saltstack#60150. Updating installed and remov…
b521d21 
…ed functions to return changes when test=True.
garethgreenaway added a commit to bryceml/salt that referenced this pull request Jan 21, 2022
@garethgreenaway @s0undt3ch
@ScriptAutomate @waynew @nicholasmhughes @dwoz @meaksh @twangboy
Merge freeze into master (saltstack#61310)
dbccc75 
* Redirect imports of ``salt.ext.six`` to ``six``

Fixes saltstack#60966

* Latest changelog update for 3004

* Handle signals and properly exit, instead of raising exceptions.

This was introduced in 26fcda5

Fixes saltstack#60391
Fixes saltstack#60963

* Add test for saltstack#61003

* Fix saltstack#61003

Restored the previously shifted check for version_to_remove in
old[target]. This had been extracted along with the correctly extracted
double pkg_params[target] lookup, but that lost the `target in old`
guard.

Putting the check back here prevents KeyError when looking for a
non-existent target in `old`.

* Handle various architecture formats in aptpkg module

* Write file even if does not exist

* only run test on debian based platforms

* remove extra space for arch

* convert pathlib to string for pkgrepo test

* Use temporary files first then copy to sources files

* fixes saltstack#59182 fix handling of duplicate keys in rest_cherrypy data

* added changelog

* remove log messages to prevent leaks of sensitive info

* Reverting changes in PR saltstack#60150. Updating installed and removed functions to return changes when test=True.

* Adding changelog.

* Add a test and fix for extra-filerefs

* Do not break master_tops for minion with version lower to 3003

* Add changelog file

* Add extra comment to clarify discussion

* Update changelog file

* Add deprecated changelog

* Assert that the command didn't finish

Refs saltstack#60972

* Always restore signals, even when exceptions occur

* Reset signal handlers before starting the process

* Make sure that the `ProcessManager` doesn't always ignore signals

* Provide valid default value for bootstrap_delay

* Update changelog for 3004

* Update changelog and release notes for 3004

* Add PR 61020 to changelog

* Update release notes index file and create 3005 file

* Update docs

* bad merge from master, redoing the PR for adding the Deltaproxy documentation.

* Adding doc/ref/configuration/delta_proxy.rst.

* cleaning up this paragraph, adding in network administrators in addition to system administrators.

Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: ScriptAutomate <derek@icanteven.io>
Co-authored-by: Wayne Werner <wwerner@vmware.com>
Co-authored-by: Megan Wilhite <mwilhite@vmware.com>
Co-authored-by: nicholasmhughes <nicholasmhughes@gmail.com>
Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com>
Co-authored-by: Pablo Suárez Hernández <psuarezhernandez@suse.com>
Co-authored-by: Alyssa Rock <arock@saltstack.com>
Co-authored-by: Twangboy <shane.d.lee@gmail.com>
garethgreenaway added a commit that referenced this pull request Apr 18, 2022
@dwoz @s0undt3ch
@ScriptAutomate @waynew @nicholasmhughes @garethgreenaway @meaksh @krionbsd @Ch3LL @frogunder @MKLeb
Merge forward from 3004.1 (#61888)
4223023 
* Redirect imports of ``salt.ext.six`` to ``six``

Fixes #60966

* Latest changelog update for 3004

* Handle signals and properly exit, instead of raising exceptions.

This was introduced in 26fcda5

Fixes #60391
Fixes #60963

* Add test for #61003

* Fix #61003

Restored the previously shifted check for version_to_remove in
old[target]. This had been extracted along with the correctly extracted
double pkg_params[target] lookup, but that lost the `target in old`
guard.

Putting the check back here prevents KeyError when looking for a
non-existent target in `old`.

* Handle various architecture formats in aptpkg module

* Write file even if does not exist

* only run test on debian based platforms

* remove extra space for arch

* convert pathlib to string for pkgrepo test

* Use temporary files first then copy to sources files

* fixes #59182 fix handling of duplicate keys in rest_cherrypy data

* added changelog

* remove log messages to prevent leaks of sensitive info

* Reverting changes in PR #60150. Updating installed and removed functions to return changes when test=True.

* Adding changelog.

* Add a test and fix for extra-filerefs

* Do not break master_tops for minion with version lower to 3003

* Add changelog file

* Add extra comment to clarify discussion

* Update changelog file

* Add deprecated changelog

* Assert that the command didn't finish

Refs #60972

* Always restore signals, even when exceptions occur

* Reset signal handlers before starting the process

* Make sure that the `ProcessManager` doesn't always ignore signals

* Provide valid default value for bootstrap_delay

* Update changelog for 3004

* Update changelog and release notes for 3004

* Add PR 61020 to changelog

* Change MD5 to SHA256 fingerprint for new github.com fingerprint

* Check only ssh-rsa encyption for set_known_host

* Use main branch for kitchen-docker project

* Add tests for validate_tgt

This function evolved over the years, but never had any tests. We're
adding tests now to cover the various cases:

- there are no valid minions (currently fails, should return False)
- there are target minions that aren't in valid minions (correctly
  fails)
- target minions are a subset of valid minions (i.e. all of the target
  minions are found in the valid minions -- there are no extras)
  (correctly passes)

* Refactor

minions should be a subset of v_minions - the extra code was just
getting in the way. Also, this function evolved over time but the
docstring never kept up. Updated the docstring to more accurately
describe the function's behavior.

* Fix #60413

When using a syndic and user auth, it was possible for v_minions and
minions to be two empty sets, which returned True. This allowed the user
to still publish the function. The Syndic would get the published event
and apply it, even though it should have been rejected.

However, if there are no valid minions, then it doesn't matter what the
targets are -- there are not valid targets, so there's no reason to do
any further checks.

* Rename changelog to security

* add cve# to changelog

* Sign pillar data

* Add regression tests for CVE-2022-22934

* Add changelog for cve-2022-22934

* Provide users with a nice warning when something goes wrong

* Rename changelog file

* Fix wart in tests

* Return bool when using m2crypo

* Limit the amount of empty space while searching ifconfig output

* Update changelog/cve-2020-22937.security

Co-authored-by: Megan Wilhite <megan.wilhite@gmail.com>

* Prevent auth replays and sign replies

* Add tests for cve-2022-22935

* Add changelog for cve-2020-22935

* Fix typo

* Prevent replays of file server requests

* Add regresion tests for fileserver nonce

* Add changelog for cve-2022-22936

* Job replay mitigation

* Fix merge warts

* more test fixes

* Fix auth tests on windows

* Remove unwanted requirements change

* Clean up cruft

* update docs for 3004.1 release

* Fix warts in new minion auth

* Test fix

* Update release notes

* Remove cve from non cve worty issue

* Add serial to payload in publisher process

* Fix channel tests

Fix broken channel tests by populating an AES key and serial.

* Windows test fix

* windows tests plz work

Co-authored-by: Pedro Algarvio <pedro@algarvio.me>
Co-authored-by: ScriptAutomate <derek@icanteven.io>
Co-authored-by: Wayne Werner <wwerner@vmware.com>
Co-authored-by: Megan Wilhite <mwilhite@vmware.com>
Co-authored-by: nicholasmhughes <nicholasmhughes@gmail.com>
Co-authored-by: Gareth J. Greenaway <gareth@saltstack.com>
Co-authored-by: Pablo Suárez Hernández <psuarezhernandez@suse.com>
Co-authored-by: Alyssa Rock <arock@saltstack.com>
Co-authored-by: krionbsd <krion@FreeBSD.org>
Co-authored-by: Megan Wilhite <megan.wilhite@gmail.com>
Co-authored-by: Frode Gundersen <frogunder@gmail.com>
Co-authored-by: MKLeb <calebb@vmware.com>
@dmurphy18 dmurphy18 deleted the bug60120 branch May 3, 2023 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garethgreenaway garethgreenaway garethgreenaway approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] salt-ssh test=True doesn't show a service is going to get restarted for a pkg, but will for a file
2 participants
@dmurphy18 @garethgreenaway