Add manage_service feature

puppet-heat lacks of disabling service managing. This patch adds
$manage_service parameter to all relevant classes.

Closes-bug: #1359823
Change-Id: I54245b39f3484ccdb9910aa0fa4c8dc2bae3f0ce

manifests/api.pp

@@ -1,6 +1,16 @@
 # Installs & configure the heat API service
-
+#
+# == Parameters
+#  [*enabled*]
+#    (optional) Should the service be enabled.
+#    Defaults to true
+#
+#  [*manage_service*]
+#    (optional) Whether the service should be managed by Puppet.
+#    Defaults to true.
+#
 class heat::api (
+  $manage_service    = true,
   $enabled           = true,
   $bind_host         = '0.0.0.0',
   $bind_port         = '8004',
@@ -32,10 +42,12 @@ class heat::api (
     name   => $::heat::params::api_package_name,
   }
 
-  if $enabled {
+  if $manage_service {
-    $service_ensure = 'running'
+    if $enabled {
-  } else {
+      $service_ensure = 'running'
-    $service_ensure = 'stopped'
+    } else {
+      $service_ensure = 'stopped'
+    }
   }
 
   service { 'heat-api':

manifests/api_cfn.pp

@@ -1,6 +1,16 @@
 # Installs & configure the heat CloudFormation API service
-
+#
+# == Parameters
+#  [*enabled*]
+#    (optional) Should the service be enabled.
+#    Defaults to true
+#
+#  [*manage_service*]
+#    (optional) Whether the service should be managed by Puppet.
+#    Defaults to true.
+#
 class heat::api_cfn (
+  $manage_service    = true,
   $enabled           = true,
   $bind_host         = '0.0.0.0',
   $bind_port         = '8000',
@@ -32,10 +42,12 @@ class heat::api_cfn (
     name   => $::heat::params::api_cfn_package_name,
   }
 
-  if $enabled {
+  if $manage_service {
-    $service_ensure = 'running'
+    if $enabled {
-  } else {
+      $service_ensure = 'running'
-    $service_ensure = 'stopped'
+    } else {
+      $service_ensure = 'stopped'
+    }
   }
 
   Package['heat-common'] -> Service['heat-api-cfn']

manifests/api_cloudwatch.pp

@@ -1,6 +1,16 @@
 # Installs & configure the heat CloudWatch API service
-
+#
+# == Parameters
+#  [*enabled*]
+#    (optional) Should the service be enabled.
+#    Defaults to true
+#
+#  [*manage_service*]
+#    (optional) Whether the service should be managed by Puppet.
+#    Defaults to true.
+#
 class heat::api_cloudwatch (
+  $manage_service    = true,
   $enabled           = true,
   $bind_host         = '0.0.0.0',
   $bind_port         = '8003',
@@ -32,12 +42,15 @@ class heat::api_cloudwatch (
     name   => $::heat::params::api_cloudwatch_package_name,
   }
 
-  if $enabled {
+  if $manage_service {
-    $service_ensure = 'running'
+    if $enabled {
-  } else {
+      $service_ensure = 'running'
-    $service_ensure = 'stopped'
+    } else {
+      $service_ensure = 'stopped'
+    }
   }
 
+
   Package['heat-common'] -> Service['heat-api-cloudwatch']
 
   service { 'heat-api-cloudwatch':

manifests/engine.pp

@@ -4,9 +4,13 @@
 #
 # == parameters
 #  [*enabled*]
-#    (optional) The state of the service
+#    (optional) Should the service be enabled.
 #    Defaults to true
 #
+#  [*manage_service*]
+#    (optional) Whether the service should be managed by Puppet.
+#    Defaults to true.
+#
 #  [*heat_stack_user_role*]
 #    (optional) Keystone role for heat template-defined users
 #    Defaults to 'heat_stack_user'
@@ -34,6 +38,7 @@
 
 class heat::engine (
   $auth_encryption_key,
+  $manage_service                = true,
   $enabled                       = true,
   $heat_stack_user_role          = 'heat_stack_user',
   $heat_metadata_server_url      = 'http://127.0.0.1:8000',
@@ -53,10 +58,12 @@ class heat::engine (
     name   => $::heat::params::engine_package_name,
   }
 
-  if $enabled {
+  if $manage_service {
-    $service_ensure = 'running'
+    if $enabled {
-  } else {
+      $service_ensure = 'running'
-    $service_ensure = 'stopped'
+    } else {
+      $service_ensure = 'stopped'
+    }
   }
 
   service { 'heat-engine':

spec/classes/heat_api_cfn_spec.rb

@@ -3,63 +3,127 @@ require 'spec_helper'
 describe 'heat::api_cfn' do
 
   let :params do
-    {
+    { :enabled       => true,
-      :bind_host => '127.0.0.1',
+      :manage_service => true,
-      :bind_port => '1234',
+      :bind_host      => '127.0.0.1',
-      :workers   => '0'
+      :bind_port      => '1234',
-    }
+      :workers        => '0' }
   end
 
-  let :facts do
+  shared_examples_for 'heat-api-cfn' do
-    { :osfamily => 'Debian' }
-  end
 
-  context 'config params' do
+    context 'config params' do
 
-    it { should contain_class('heat') }
+      it { should contain_class('heat') }
-    it { should contain_class('heat::params') }
+      it { should contain_class('heat::params') }
 
-    it { should contain_heat_config('heat_api_cfn/bind_host').with_value( params[:bind_host] ) }
+      it { should contain_heat_config('heat_api_cfn/bind_host').with_value( params[:bind_host] ) }
-    it { should contain_heat_config('heat_api_cfn/bind_port').with_value( params[:bind_port] ) }
+      it { should contain_heat_config('heat_api_cfn/bind_port').with_value( params[:bind_port] ) }
-    it { should contain_heat_config('heat_api_cfn/workers').with_value( params[:workers] ) }
+      it { should contain_heat_config('heat_api_cfn/workers').with_value( params[:workers] ) }
 
-  end
-
-  context 'with SSL socket options set' do
-    let :params do
-      {
-        :use_ssl   => true,
-        :cert_file => '/path/to/cert',
-        :key_file  => '/path/to/key'
-      }
     end
 
-    it { should contain_heat_config('heat_api_cfn/cert_file').with_value('/path/to/cert') }
+    context 'with SSL socket options set' do
-    it { should contain_heat_config('heat_api_cfn/key_file').with_value('/path/to/key') }
+      let :params do
-  end
+        {
+          :use_ssl   => true,
+          :cert_file => '/path/to/cert',
+          :key_file  => '/path/to/key'
+        }
+      end
 
-  context 'with SSL socket options set with wrong parameters' do
+      it { should contain_heat_config('heat_api_cfn/cert_file').with_value('/path/to/cert') }
-    let :params do
+      it { should contain_heat_config('heat_api_cfn/key_file').with_value('/path/to/key') }
-      {
-        :use_ssl   => true,
-        :key_file  => '/path/to/key'
-      }
     end
 
-    it_raises 'a Puppet::Error', /The cert_file parameter is required when use_ssl is set to true/
+    context 'with SSL socket options set with wrong parameters' do
-  end
+      let :params do
+        {
+          :use_ssl   => true,
+          :key_file  => '/path/to/key'
+        }
+      end
 
-  context 'with SSL socket options set to false' do
+      it_raises 'a Puppet::Error', /The cert_file parameter is required when use_ssl is set to true/
-    let :params do
-      {
-        :use_ssl   => false,
-        :cert_file => false,
-        :key_file  => false
-      }
     end
 
-    it { should contain_heat_config('heat_api_cfn/cert_file').with_ensure('absent') }
+    context 'with SSL socket options set to false' do
-    it { should contain_heat_config('heat_api_cfn/key_file').with_ensure('absent') }
+      let :params do
+        {
+          :use_ssl   => false,
+          :cert_file => false,
+          :key_file  => false
+        }
+      end
+
+      it { should contain_heat_config('heat_api_cfn/cert_file').with_ensure('absent') }
+      it { should contain_heat_config('heat_api_cfn/key_file').with_ensure('absent') }
+    end
+
+    [{:enabled => true}, {:enabled => false}].each do |param_hash|
+      context "when service should be #{param_hash[:enabled] ? 'enabled' : 'disabled'}" do
+        before do
+          params.merge!(param_hash)
+        end
+
+        it 'configures heat-api-cfn service' do
+
+          should contain_service('heat-api-cfn').with(
+            :ensure     => (params[:manage_service] && params[:enabled]) ? 'running' : 'stopped',
+            :name       => platform_params[:api_service_name],
+            :enable     => params[:enabled],
+            :hasstatus  => true,
+            :hasrestart => true,
+            :subscribe  => ['Exec[heat-dbsync]']
+          )
+        end
+      end
+    end
+
+    context 'with disabled service managing' do
+      before do
+        params.merge!({
+          :manage_service => false,
+          :enabled        => false })
+      end
+
+      it 'configures heat-api-cfn service' do
+
+        should contain_service('heat-api-cfn').with(
+          :ensure     => nil,
+          :name       => platform_params[:api_service_name],
+          :enable     => false,
+          :hasstatus  => true,
+          :hasrestart => true,
+          :subscribe  => ['Exec[heat-dbsync]']
+        )
+      end
+    end
+  end
+
+
+  context 'on Debian platforms' do
+    let :facts do
+      { :osfamily => 'Debian' }
+    end
+
+    let :platform_params do
+      { :api_service_name => 'heat-api-cfn' }
+    end
+
+    it_configures 'heat-api-cfn'
+  end
+
+  context 'on RedHat platforms' do
+    let :facts do
+      { :osfamily => 'RedHat' }
+    end
+
+    let :platform_params do
+      { :api_service_name => 'openstack-heat-api-cfn' }
+    end
+
+    it_configures 'heat-api-cfn'
   end
 
 end

spec/classes/heat_api_cloudwatch_spec.rb

@@ -3,63 +3,126 @@ require 'spec_helper'
 describe 'heat::api_cloudwatch' do
 
   let :params do
-    {
+    { :enabled        => true,
-      :bind_host => '127.0.0.1',
+      :manage_service => true,
-      :bind_port => '1234',
+      :bind_host      => '127.0.0.1',
-      :workers   => '0'
+      :bind_port      => '1234',
-    }
+      :workers        => '0' }
   end
 
-  let :facts do
+  shared_examples_for 'heat-api-cloudwatch' do
-    { :osfamily => 'Debian' }
-  end
 
-  context 'config params' do
+    context 'config params' do
 
-    it { should contain_class('heat') }
+      it { should contain_class('heat') }
-    it { should contain_class('heat::params') }
+      it { should contain_class('heat::params') }
 
-    it { should contain_heat_config('heat_api_cloudwatch/bind_host').with_value( params[:bind_host] ) }
+      it { should contain_heat_config('heat_api_cloudwatch/bind_host').with_value( params[:bind_host] ) }
-    it { should contain_heat_config('heat_api_cloudwatch/bind_port').with_value( params[:bind_port] ) }
+      it { should contain_heat_config('heat_api_cloudwatch/bind_port').with_value( params[:bind_port] ) }
-    it { should contain_heat_config('heat_api_cloudwatch/workers').with_value( params[:workers] ) }
+      it { should contain_heat_config('heat_api_cloudwatch/workers').with_value( params[:workers] ) }
 
-  end
-
-  context 'with SSL socket options set' do
-    let :params do
-      {
-        :use_ssl   => true,
-        :cert_file => '/path/to/cert',
-        :key_file  => '/path/to/key'
-      }
     end
 
-    it { should contain_heat_config('heat_api_cloudwatch/cert_file').with_value('/path/to/cert') }
+    context 'with SSL socket options set' do
-    it { should contain_heat_config('heat_api_cloudwatch/key_file').with_value('/path/to/key') }
+      let :params do
-  end
+        {
+          :use_ssl   => true,
+          :cert_file => '/path/to/cert',
+          :key_file  => '/path/to/key'
+        }
+      end
 
-  context 'with SSL socket options set with wrong parameters' do
+      it { should contain_heat_config('heat_api_cloudwatch/cert_file').with_value('/path/to/cert') }
-    let :params do
+      it { should contain_heat_config('heat_api_cloudwatch/key_file').with_value('/path/to/key') }
-      {
-        :use_ssl   => true,
-        :key_file  => '/path/to/key'
-      }
     end
 
-    it_raises 'a Puppet::Error', /The cert_file parameter is required when use_ssl is set to true/
+    context 'with SSL socket options set with wrong parameters' do
-  end
+      let :params do
+        {
+          :use_ssl   => true,
+          :key_file  => '/path/to/key'
+        }
+      end
 
-  context 'with SSL socket options set to false' do
+      it_raises 'a Puppet::Error', /The cert_file parameter is required when use_ssl is set to true/
-    let :params do
-      {
-        :use_ssl   => false,
-        :cert_file => false,
-        :key_file  => false
-      }
     end
 
-    it { should contain_heat_config('heat_api_cloudwatch/cert_file').with_ensure('absent') }
+    context 'with SSL socket options set to false' do
-    it { should contain_heat_config('heat_api_cloudwatch/key_file').with_ensure('absent') }
+      let :params do
+        {
+          :use_ssl   => false,
+          :cert_file => false,
+          :key_file  => false
+        }
+      end
+
+      it { should contain_heat_config('heat_api_cloudwatch/cert_file').with_ensure('absent') }
+      it { should contain_heat_config('heat_api_cloudwatch/key_file').with_ensure('absent') }
+    end
+
+    [{:enabled => true}, {:enabled => false}].each do |param_hash|
+      context "when service should be #{param_hash[:enabled] ? 'enabled' : 'disabled'}" do
+        before do
+          params.merge!(param_hash)
+        end
+
+        it 'configures heat-api-cloudwatch service' do
+
+          should contain_service('heat-api-cloudwatch').with(
+            :ensure     => (params[:manage_service] && params[:enabled]) ? 'running' : 'stopped',
+            :name       => platform_params[:api_service_name],
+            :enable     => params[:enabled],
+            :hasstatus  => true,
+            :hasrestart => true,
+            :subscribe  => ['Exec[heat-dbsync]']
+          )
+        end
+      end
+    end
+
+    context 'with disabled service managing' do
+      before do
+        params.merge!({
+          :manage_service => false,
+          :enabled        => false })
+      end
+
+      it 'configures heat-api-cloudwatch service' do
+
+        should contain_service('heat-api-cloudwatch').with(
+          :ensure     => nil,
+          :name       => platform_params[:api_service_name],
+          :enable     => false,
+          :hasstatus  => true,
+          :hasrestart => true,
+          :subscribe  => ['Exec[heat-dbsync]']
+        )
+      end
+    end
+  end
+
+  context 'on Debian platforms' do
+    let :facts do
+      { :osfamily => 'Debian' }
+    end
+
+    let :platform_params do
+      { :api_service_name => 'heat-api-cloudwatch' }
+    end
+
+    it_configures 'heat-api-cloudwatch'
+  end
+
+  context 'on RedHat platforms' do
+    let :facts do
+      { :osfamily => 'RedHat' }
+    end
+
+    let :platform_params do
+      { :api_service_name => 'openstack-heat-api-cloudwatch' }
+    end
+
+    it_configures 'heat-api-cloudwatch'
   end
 
 end

spec/classes/heat_api_spec.rb

@@ -3,63 +3,129 @@ require 'spec_helper'
 describe 'heat::api' do
 
   let :params do
-    {
+    { :enabled        => true,
-      :bind_host => '127.0.0.1',
+      :manage_service => true,
-      :bind_port => '1234',
+      :bind_host      => '127.0.0.1',
-      :workers   => '0'
+      :bind_port      => '1234',
-    }
+      :workers        => '0' }
   end
 
-  let :facts do
+  shared_examples_for 'heat-api' do
-    { :osfamily => 'Debian' }
-  end
 
-  context 'config params' do
+    context 'config params' do
 
-    it { should contain_class('heat') }
+      it { should contain_class('heat') }
-    it { should contain_class('heat::params') }
+      it { should contain_class('heat::params') }
 
-    it { should contain_heat_config('heat_api/bind_host').with_value( params[:bind_host] ) }
+      it { should contain_heat_config('heat_api/bind_host').with_value( params[:bind_host] ) }
-    it { should contain_heat_config('heat_api/bind_port').with_value( params[:bind_port] ) }
+      it { should contain_heat_config('heat_api/bind_port').with_value( params[:bind_port] ) }
-    it { should contain_heat_config('heat_api/workers').with_value( params[:workers] ) }
+      it { should contain_heat_config('heat_api/workers').with_value( params[:workers] ) }
 
-  end
-
-  context 'with SSL socket options set' do
-    let :params do
-      {
-        :use_ssl   => true,
-        :cert_file => '/path/to/cert',
-        :key_file  => '/path/to/key'
-      }
     end
 
-    it { should contain_heat_config('heat_api/cert_file').with_value('/path/to/cert') }
+    context 'with SSL socket options set' do
-    it { should contain_heat_config('heat_api/key_file').with_value('/path/to/key') }
+      let :params do
-  end
+        {
+          :use_ssl   => true,
+          :cert_file => '/path/to/cert',
+          :key_file  => '/path/to/key'
+        }
+      end
 
-  context 'with SSL socket options set with wrong parameters' do
+      it { should contain_heat_config('heat_api/cert_file').with_value('/path/to/cert') }
-    let :params do
+      it { should contain_heat_config('heat_api/key_file').with_value('/path/to/key') }
-      {
-        :use_ssl   => true,
-        :key_file  => '/path/to/key'
-      }
     end
 
-    it_raises 'a Puppet::Error', /The cert_file parameter is required when use_ssl is set to true/
+    context 'with SSL socket options set with wrong parameters' do
-  end
+      let :params do
+        {
+          :use_ssl   => true,
+          :key_file  => '/path/to/key'
+        }
+      end
 
-  context 'with SSL socket options set to false' do
+      it_raises 'a Puppet::Error', /The cert_file parameter is required when use_ssl is set to true/
-    let :params do
-      {
-        :use_ssl   => false,
-        :cert_file => false,
-        :key_file  => false
-      }
     end
 
-    it { should contain_heat_config('heat_api/cert_file').with_ensure('absent') }
+    context 'with SSL socket options set to false' do
-    it { should contain_heat_config('heat_api/key_file').with_ensure('absent') }
+      let :params do
+        {
+          :use_ssl   => false,
+          :cert_file => false,
+          :key_file  => false
+        }
+      end
+
+      it { should contain_heat_config('heat_api/cert_file').with_ensure('absent') }
+      it { should contain_heat_config('heat_api/key_file').with_ensure('absent') }
+    end
+
+
+    [{:enabled => true}, {:enabled => false}].each do |param_hash|
+      context "when service should be #{param_hash[:enabled] ? 'enabled' : 'disabled'}" do
+        before do
+          params.merge!(param_hash)
+        end
+
+        it 'configures heat-api service' do
+
+          should contain_service('heat-api').with(
+            :ensure     => (params[:manage_service] && params[:enabled]) ? 'running' : 'stopped',
+            :name       => platform_params[:api_service_name],
+            :enable     => params[:enabled],
+            :hasstatus  => true,
+            :hasrestart => true,
+            :require    => ['Package[heat-common]', 'Package[heat-api]'],
+            :subscribe  => ['Exec[heat-dbsync]']
+          )
+        end
+      end
+    end
+
+    context 'with disabled service managing' do
+      before do
+        params.merge!({
+          :manage_service => false,
+          :enabled        => false })
+      end
+
+      it 'configures heat-api service' do
+
+        should contain_service('heat-api').with(
+          :ensure     => nil,
+          :name       => platform_params[:api_service_name],
+          :enable     => false,
+          :hasstatus  => true,
+          :hasrestart => true,
+          :require    => ['Package[heat-common]', 'Package[heat-api]'],
+          :subscribe  => ['Exec[heat-dbsync]']
+        )
+      end
+    end
+  end
+
+  context 'on Debian platforms' do
+    let :facts do
+      { :osfamily => 'Debian' }
+    end
+
+    let :platform_params do
+      { :api_service_name => 'heat-api' }
+    end
+
+    it_configures 'heat-api'
+  end
+
+  context 'on RedHat platforms' do
+    let :facts do
+      { :osfamily => 'RedHat' }
+    end
+
+    let :platform_params do
+      { :api_service_name => 'openstack-heat-api' }
+    end
+
+    it_configures 'heat-api'
   end
 
 end

spec/classes/heat_engine_spec.rb

@@ -4,6 +4,7 @@ describe 'heat::engine' do
 
   let :default_params do
     { :enabled                       => true,
+      :manage_service                => true,
       :heat_stack_user_role          => 'heat_stack_user',
       :heat_metadata_server_url      => 'http://127.0.0.1:8000',
       :heat_waitcondition_server_url => 'http://127.0.0.1:8000/v1/waitcondition',
@@ -38,7 +39,7 @@ describe 'heat::engine' do
       it { should contain_package('heat-engine').with_name(os_params[:package_name]) }
 
       it { should contain_service('heat-engine').with(
-        :ensure     => expected_params[:enabled] ? 'running' : 'stopped',
+        :ensure     => (expected_params[:manage_service] && expected_params[:enabled]) ? 'running' : 'stopped',
         :name       => os_params[:service_name],
         :enable     => expected_params[:enabled],
         :hasstatus  => 'true',
@@ -56,6 +57,26 @@ describe 'heat::engine' do
       it { should contain_heat_config('DEFAULT/heat_watch_server_url').with_value( expected_params[:heat_watch_server_url] ) }
       it { should contain_heat_config('DEFAULT/engine_life_check_timeout').with_value( expected_params[:engine_life_check_timeout] ) }
     end
+
+    context 'with disabled service managing' do
+      before do
+        params.merge!({
+          :manage_service => false,
+          :enabled        => false })
+      end
+
+      it { should contain_service('heat-engine').with(
+        :ensure     => nil,
+        :name       => os_params[:service_name],
+        :enable     => false,
+        :hasstatus  => 'true',
+        :hasrestart => 'true',
+        :require    => [ 'File[/etc/heat/heat.conf]',
+                         'Package[heat-common]',
+                         'Package[heat-engine]'],
+        :subscribe  => 'Exec[heat-dbsync]'
+      ) }
+    end
   end
 
   context 'on Debian platforms' do