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 issue with `salt.utils.parsers` on Windows #48077

Merged
merged 3 commits into from Jun 14, 2018

Conversation

Projects
None yet
2 participants
@twangboy
Contributor

twangboy commented Jun 12, 2018

What does this PR do?

Fixes an issue with salt.utils.parsers on Windows. There is no os.getuid function in Windows. The _mixin_before_exit function uses os.getuid to check for root which fails on Windows. This function detects root/admin differently if it's on a Windows box. Both methods are also mocked properly in the tests.

What issues does this PR fix or reference?

Found via failing tests on Jenkins

Previous Behavior

The following tests were failing:
unit.utils.test_parsers.DaemonMixInTestCase.test_pid_deleted_oserror_as_non_root
unit.utils.test_parsers.DaemonMixInTestCase.test_pid_deleted_oserror_as_root

New Behavior

The tests now pass.

Tests written?

Yes

Commits signed with GPG?

Yes

twangboy added some commits Jun 12, 2018

@@ -968,9 +968,17 @@ def _mixin_before_exit(self):
# Log error only when running salt-master as a root user.
# Otherwise this can be ignored, since salt-master is able to
# overwrite the PIDfile on the next start.
if not os.getuid():
logger.info('PIDfile could not be deleted: %s', six.text_type(self.config['pidfile']))
def log_error():

This comment has been minimized.

@cachedout

cachedout Jun 13, 2018

Contributor

Instead of the overhead of a function call here, I think it would be better just to define the error string and then call logger.info in each case, IMHO.

@twangboy

This comment has been minimized.

Contributor

twangboy commented Jun 13, 2018

@cachedout Moved the log calls as requested. Got rid of the function

@cachedout

Very nice. This is quite clean.

@cachedout cachedout merged commit 4b6f1c7 into saltstack:2018.3 Jun 14, 2018

7 of 9 checks passed

default Build finished.
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10675 — FAILURE
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25903 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #17965 — SUCCESS
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5704 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23634 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22596 — SUCCESS
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19758 — SUCCESS
Details

@twangboy twangboy deleted the twangboy:fix_parsers branch Jun 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment