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
Pup 3997 clean logdir #3692
Pup 3997 clean logdir #3692
Conversation
This commit alters the default user and group parameter settings in the create_test_file method of the Puppet::Acceptance::TempFileUtils module. If the host is a master, the value of `puppet config print {user,group}` are used. Otherwise, the appropriate root user and group for the host's platform are used.
This commit adds a get_file_manifest helper to the Puppet::Acceptance::TempFileUtils module. It also modifies acceptance tests that were impacting the state of the `logdir` on SUTs to use this helper to return the state of this file resource to its original state.
CLA signed by all contributors. |
teardown do | ||
on(agent, puppet('config', 'set', 'logdir', "'#{logdir} { owner = root, group = root, mode = 0700 }'", '--confdir', get_test_file_path(agent, ''))) | ||
on(agent, puppet("apply -e \"#{logdir_orig}\"")) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnduarte I'm not following why these changes are necessary. Previously, the test created a temp confdir, and pointed logdir
to another tempdir:
logdir = get_test_file_path(agent, 'log')
...
on(agent, puppet('config', 'set', 'logdir', "'#{logdir} { owner = root, group = root, mode = 0700 }'", '--confdir', get_test_file_path(agent, '')))
Then it executed puppet apply
with the temp confdir:
on(agent, puppet('apply', get_test_file_path(agent, 'site.pp'), '--confdir', get_test_file_path(agent, '')))
So I would expect puppet
to use the temp logdir
specified in the temp confdir
So I don't think this PR addressed the order dependent failures that caused us to revert. I reverted the revert (
|
options[:owner] = 'root' | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to this part, as it eliminates things like ed7e048
@johnduarte I included commit 18954ba in PR #3698, and dropped the other one as it was unused. |
No description provided.