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

Manage smartd service and don't attempt to stop if package is absent #43

Merged

Conversation

queeno
Copy link
Contributor

@queeno queeno commented Jun 12, 2015

The smartd service resource has been wrapped in an if statement and controlled by the $manage_service parameter, so that a user of the module can choose manage the service externally.

If the smartd package is not installed, puppet will attempt to stop a service that doesn't exist. For this reason, I believe the smartd service should not be managed if $ensure is set to absent or purged.

In fact, if smartd is getting uninstalled, the deb script will take care to stop the service anyway.

The smartd service resource has been wrapped in an if statement and
controlled by the $manage_service parameter, so that a user of the
module can choose manage the service externally.
@jhoblitt
Copy link
Owner

@queeno This looks reasonable -- are you willing to take a stab at updating the unit tests?

If the smartd package is not installed, puppet will attempt to stop
a service that doesn't exist. For this reason, the smartd service should
not be managed if ensure is set to absent or purged.
If smartd is getting uninstalled, the deb script will take care to stop
the service anyway.
It should be $manage_service not $service_manage!
@queeno queeno force-pushed the manage_service_and_dont_attempt_to_stop branch from 609d30e to 8b633d9 Compare June 12, 2015 18:09
I've added and amended some unit tests in order to check the
$manage_service functionality introduced in previous commits.
@queeno queeno force-pushed the manage_service_and_dont_attempt_to_stop branch from c896f07 to 19be823 Compare June 12, 2015 18:13
@queeno
Copy link
Contributor Author

queeno commented Jun 12, 2015

@jhoblitt Unit tests fixed ;-) Thanks for your quick response!

}
'absent', 'purged': {
$pkg_ensure = $ensure
$svc_ensure = 'stopped'
$svc_enable = false
$file_ensure = 'absent'
$srv_manage = false
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this imply that the service will never be stopped unless the packaging has an uninstall script that handles it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Debian systems the prerm package script would normally handle that. smartmontools is not an exception:

#!/bin/sh
set -e
# Automatically added by dh_installinit
if [ -x "/etc/init.d/smartmontools" ] || [ -e "/etc/init/smartmontools.conf" ]; then
        invoke-rc.d smartmontools stop || exit $?
fi
# End automatically added section

If you think about that, when you remove a package, whether it is in windows or linux, you also expect all the processes/services to have gone by the time the package is removed. If this doesn't happen, I would blame the package configuration scripts, not the configuration management tool.

I believe $srv_manage should be always equal to false when the package is absent. Otherwise how would you manage the service of an app that doesn't exist on the system?

jhoblitt pushed a commit that referenced this pull request Oct 14, 2015
…_stop

Manage smartd service and don't attempt to stop if package is absent
@jhoblitt jhoblitt merged commit 0d434ce into jhoblitt:master Oct 14, 2015
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