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

Do not reboot system when runnnig puppet resource service #1473

Conversation

stschulte
Copy link
Contributor

Some initscripts are not save to execute on different systems. Example:
If you run /etc/init.d/reboot.sh status on a gentoo system you will
immediatly reboot the system.

This can be a problem if you run puppet resource service as puppet
will try to get a list of services and then runs the status command on
all of them.

The issue was first addressed in redmine ticket #14615 and #14761 and
introduced different exclude lists for the gentoo provider and the
redhat provider. Unfortunately this did not resolve the problem because
when running puppet resource service puppet will not just query the
most suitable provider for a list of services but all suitable
providers. So if the gentoo provider now hides the reboot.sh script,
but the init provider (which is also suitable) does not, puppet will
ask the init provider to get the status of the service. As a result
the init provider will now happily reboot the system.

Move the exclude lists into the init provider so the init provider does
the filtering. Each provider that uses the init provider as its parent
does not have to do any filtering anymore.

This fix assumes that an initscript that has to be excluded on one
system is never a valid initscript on another system. Given the recent
list of excludes (reboot.sh, shutdown.sh, etc) this seems valid

It seems to be easier to work with real objects.
The test `!File.directory?` is redundant because if the item actually is
a directory, we cannot reach that statement.
Instead of stubbing every call to File.directory? that may have side
effects in other areas of puppet be more explicit in what we actually
expect.

This may also prevent tests to pass incidentally.
Let the instances method create real provider instances and just check
the returnvalue
Some initscripts are not save to execute on different systems. Example:
If you run `/etc/init.d/reboot.sh status` on a gentoo system you will
immediatly reboot the system.

This can be a problem if you run `puppet resource service` as puppet
will try to get a list of services and then runs the status command on
all of them.

The issue was first addressed in redmine ticket #14615 and #14761 and
introduced different exclude lists for the gentoo provider and the
redhat provider. Unfortunately this did not resolve the problem because
when running `puppet resource service` puppet will not just query the
most suitable provider for a list of services but all suitable
providers. So if the gentoo provider now hides the `reboot.sh` script,
but the init provider (which is also suitable) does not, puppet will
ask the init provider to get the status of the service. As a result
the init provider will now happily reboot the system.

Move the exclude lists into the init provider so the init provider does
the filtering. Each provider that uses the `init` provider as its parent
does not have to do any filtering anymore.

This fix assumes that an initscript that has to be excluded on one
system is never a valid initscript on another system. Given the recent
list of excludes (reboot.sh, shutdown.sh, etc) this seems valid
@adrienthebo
Copy link
Contributor

Wow, gotta love init scripts that immediately reboot the system. :)

One thing that comes to mind when I look at this pull request is how the init provider is probably going to accrue more and more excluded init script names across all platforms. Is it possible to do the init script exclusions in the init provider but somehow break out the actual list of init scripts to exclude on a per-platform basis?

@stschulte
Copy link
Contributor Author

The initscript is a suitable provider on most systems so it should definitely handle the filtering of unsafe initscripts. I could make the exclude list platform dependend but I am not sure how far the exlclude lists will overlap (e.g. redhat and sles (two differen osfamiilies) seem to have the same exclude list)

So I currently don't see a huge benefit in making a distinction. When I look at the current list of initscripts the init provider is filtering out, I guess the chances of having an operatingsystem where reboot is a valid service is less likely than puppet suddently rebooting a system noone has thought of yet (e.g. does ArchLinux have a reboot script?)

So in my opinion the current implementation should be ok (at least for now).

@adrienthebo
Copy link
Contributor

That sounds reasonable. I asked for the sake of thoroughness but I agree with you, this seems like it'll solve the problem for now and I don't foresee a case that'll not work will with this.

adrienthebo added a commit that referenced this pull request Feb 14, 2013
…utdowns_system

Do not reboot system when runnnig puppet resource service
@adrienthebo adrienthebo merged commit 7d6515f into puppetlabs:master Feb 14, 2013
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.

2 participants