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

Small fix-ups for unprivileged Puppet runs #1022

Closed
wants to merge 2 commits into from

Conversation

lkundrak
Copy link
Contributor

I often run puppet --noop unprivileged to conduct a basic sanity check of the recipes (such as checking for duplicate definitions or syntax errors). These changesets fix unnecessary abort that cause puppet to stop way too early.

...and deal with them gracefully without aborting the run.
A simple reproducer of the problem (abort), to be run unprivileged:

puppet apply --execute "file { '/root/chuj': ensure => absent; }"
The user resource itself should fail itself and this prevents falling back to
that. Simple test case (unprivileged):

before$ puppet apply --execute 'user { "kokot": managehome => true }'
Parameter managehome failed: Validate method failed for class managehome: undefined method `manages_homedir?' for NilClass:Class
after$ puppet apply --execute 'user { "kokot": managehome => true }'
err: Could not find a suitable provider for user
notice: Finished catalog run in 0.10 seconds
@jamtur01
Copy link
Contributor

Hi! Is there a ticket for these? And do the tests pass with these changes? Thanks!

@lkundrak
Copy link
Contributor Author

Tests do pass. There is no ticket.

@zaphod42
Copy link
Contributor

@lkundrak, thanks for the submission. Can you open a ticket (or find one that is already open) that describes the problem that this fixes? Ideally it would show the error that you saw and describe what you were trying to do. Once you have that ticket number you can rebase the commits to include the ticket number. This help us and others a lot in understanding what the change does.

Once you have that we'll also need to get some tests in place to make sure we don't regress on this functionality. I can help you with that if you need any help.

@grimradical
Copy link
Member

This has been in stasis for a while. Without any tests to help ensure that we aren't regressing we can't merge this as-is. Once tests have been added, please reopen (or file a new pull req) and we'll take a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants