Use $service_name for service resource title. #1159
Conversation
manifests/init.pp
Outdated
| @@ -150,7 +150,7 @@ | |||
| $service_ensure = running, | |||
| $service_flags = undef, | |||
| $service_restart = undef, | |||
| $service_name = undef, | |||
| $service_name = "nginx", | |||
There was a problem hiding this comment.
Why is that - for security, to stop string evaluation?
There was a problem hiding this comment.
It's a lint rule we have. Strings without variables should be in single quotes. I think it was copied from ruby.
There was a problem hiding this comment.
If you can fix this then the build will pass and I think it's good to go.
manifests/service.pp
Outdated
| @@ -40,7 +40,7 @@ | |||
| if $service_manage { | |||
| case $::osfamily { | |||
| 'OpenBSD': { | |||
| service { 'nginx': | |||
| service { 'puppet_nginx': | |||
There was a problem hiding this comment.
I wonder if we should just use $service_name and drop the name => $service_name.
There was a problem hiding this comment.
Same thing as name defaults to the resource name, just more explicit. Would be tidier that way, I agree.
There was a problem hiding this comment.
IMHO the biggest benefit is that there's no risk of overlapping resource names for different services.
There was a problem hiding this comment.
Agreed, updated with 7e7dbf6
manifests/service.pp
Outdated
| enable => $service_enable, | ||
| flags => $service_flags, | ||
| hasstatus => true, | ||
| hasrestart => true, | ||
| } | ||
| } | ||
| default: { | ||
| service { 'nginx': | ||
| service { $service_name: | ||
| ensure => $service_ensure_real, | ||
| name => $service_name, |
There was a problem hiding this comment.
Oh, and this line can be removed too
alexjfisher
changed the title
There was a problem hiding this comment.
@fnoop Could you update this test
puppet-nginx/spec/classes/nginx_spec.rb
Line 214 in f22fee3
|
@alexjfisher Now the service name is dynamic, I don't know how to do that. Sorry, I don't know how tests work. |
|
It should be |
|
Hurrah, travis passes now :) |
|
@ekohl If I squash and merge, do you know what ends up in the commit message? |
|
@alexjfisher Github shows a dialog when you squash where it prefills a commit message based on all the commits but you can edit it. |
|
Thanks @fnoop |
This allows to specify a custom service name such as 'custom-nginx', while also separately controlling the system nginx service outside of puppet-nginx module.
Closes #1158