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

add service_ensure support #264

Merged
merged 2 commits into from
Mar 17, 2014

Conversation

welterde
Copy link
Contributor

@welterde welterde commented Mar 3, 2014

Makes it possible to disable the default service resource in case a different supervision system is used (for docker containers for instance)

@@ -44,6 +44,7 @@
$proxy_cache_inactive = $nginx::params::nx_proxy_cache_inactive,
$configtest_enable = $nginx::params::nx_configtest_enable,
$service_restart = $nginx::params::nx_service_restart,
$service_ensure = $nginx::praams::nx_service_ensure,
Copy link
Contributor

Choose a reason for hiding this comment

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

params?

@welterde
Copy link
Contributor Author

welterde commented Mar 4, 2014

Fixed. Quite likely did not make a difference though, as it is defined in service.pp as well.

@@ -15,16 +15,24 @@
# This class file is not called directly
class nginx::service(
$configtest_enable = $nginx::params::nx_configtest_enable,
$service_restart = $nginx::params::nx_service_restart
$service_restart = $nginx::params::nx_service_restart,
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about making these source $nginx::service_ensure? that way, the overrides will pass through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.. that's another way to solve the problem. I just didn't want to do things differently from the other parameters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. I'd like to change that everywhere, so go ahead and start the precedence. 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently trying to fix the unit tests to support the change. I guess that's one of the drawbacks of this method - that testing (sub-)modules independently of each other becomes more complicated (well.. at least testing for default values).

jfryman pushed a commit that referenced this pull request Mar 17, 2014
@jfryman jfryman merged commit e59468a into voxpupuli:master Mar 17, 2014
cegeka-jenkins pushed a commit to cegeka/puppet-nginx that referenced this pull request Oct 23, 2017
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