From fb467821498e6a9f8af8b3ca56d26853b5d27e9b Mon Sep 17 00:00:00 2001
From: Taylan Develioglu <taylan.develioglu@booking.com>
Date: Mon, 19 Sep 2016 11:08:48 +0200
Subject: [PATCH] Decouple managing service and init system from eachother.

Previously the logic didn't make sense, where one could pass a bool
to the parameter `init_style` that expects a string to toggle the
management of service files in the init system and it was impossible to
let the module manage the service but not the init system.

This breaks the common case of where you want to let a package prep the
init system but still let the module manage the service.

This commit decouples the service resource from management of the init
system so it offers the choice of using the module-supplied init files.
It adds a new parameter to the public class `manage_init` that allows
fully shutting down management of the init system so the module can
cover all use cases.
---
 manifests/config.pp       | 2 +-
 manifests/init.pp         | 6 ++++++
 manifests/params.pp       | 6 ++----
 manifests/run_service.pp  | 2 +-
 spec/classes/init_spec.rb | 6 +++---
 5 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/manifests/config.pp b/manifests/config.pp
index fce61b6a..c489c37d 100644
--- a/manifests/config.pp
+++ b/manifests/config.pp
@@ -15,7 +15,7 @@
   $purge = true,
 ) {
 
-  if $::consul::init_style {
+  if $::consul::manage_init {
 
     case $::consul::init_style {
       'upstart': {
diff --git a/manifests/init.pp b/manifests/init.pp
index 0aeefd58..2a66f6a7 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -63,6 +63,10 @@
 # [*manage_group*]
 #   Whether to create/manage the group that should own the consul configuration files.
 #  
+# [*manage_init*]
+#   Whether to create and manage an init system file for the service. (e.g.
+#   systemd unit file, or sysv-rc initscript)
+#
 # [*manage_service*]
 #   Whether to manage the consul service.
 #  
@@ -157,6 +161,7 @@
   $install_method        = $::consul::params::install_method,
   $join_wan              = $::consul::params::join_wan,
   $manage_group          = $::consul::params::manage_group,
+  $manage_init           = $::consul::params::manage_init,
   $manage_service        = $::consul::params::manage_service,
   $manage_user           = $::consul::params::manage_user,
   $os                    = $::consul::params::os,
@@ -197,6 +202,7 @@
   validate_hash($watches)
   validate_hash($checks)
   validate_hash($acls)
+  validate_bool($manage_init)
 
   $config_hash_real = deep_merge($config_defaults, $config_hash)
 
diff --git a/manifests/params.pp b/manifests/params.pp
index b0cf905d..ba87ebd8 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -20,6 +20,7 @@
   $group                 = 'consul'
   $install_method        = 'url'
   $join_wan              = false
+  $manage_init           = true
   $manage_group          = true
   $manage_service        = true
   $manage_user           = true
@@ -93,9 +94,6 @@
   } elsif $::operatingsystem == 'Amazon' {
     $init_style = 'init'
   } else {
-    $init_style = undef
-  }
-  if $init_style == undef {
-    fail('Unsupported OS')
+    fail('Cannot determine init_style, unsupported OS')
   }
 }
diff --git a/manifests/run_service.pp b/manifests/run_service.pp
index e1042763..c7e2b94c 100644
--- a/manifests/run_service.pp
+++ b/manifests/run_service.pp
@@ -10,7 +10,7 @@
     default   => 'consul',
   }
 
-  if $::consul::manage_service == true and $::consul::init_style {
+  if $::consul::manage_service == true {
     service { 'consul':
       ensure   => $::consul::service_ensure,
       name     => $init_selector,
diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
index 8466b70d..4110db71 100644
--- a/spec/classes/init_spec.rb
+++ b/spec/classes/init_spec.rb
@@ -694,9 +694,9 @@
     it { should contain_file('/lib/systemd/system/consul.service').with_content(/consul agent/) }
   end
 
-  context "When asked not to manage the init_style" do
-    let(:params) {{ :init_style => false }}
-    it { should contain_class('consul').with_init_style(false) }
+  context "When asked not to manage the init system" do
+    let(:params) {{ :manage_init => false }}
+    it { should contain_class('consul').with_manage_init(false) }
     it { should_not contain_file("/etc/init.d/consul") }
     it { should_not contain_file("/lib/systemd/system/consul.service") }
   end