From f100a2c545c40194ba51a4f565fefbd8a6e0ebf2 Mon Sep 17 00:00:00 2001
From: Takashi Kajinami <tkajinam@redhat.com>
Date: Fri, 24 Sep 2021 16:58:52 +0900
Subject: [PATCH] Do not define service resource when manage_service is false

Change-Id: I24364dd2ec612f1d20bc5ff8076af34d7aaa5e9c
---
 manifests/agent/notification.pp               | 16 +++++-----
 manifests/agent/polling.pp                    | 30 +++++++++----------
 .../ceilometer_agent_notification_spec.rb     | 17 +++--------
 spec/classes/ceilometer_agent_polling_spec.rb | 15 +++++++---
 4 files changed, 38 insertions(+), 40 deletions(-)

diff --git a/manifests/agent/notification.pp b/manifests/agent/notification.pp
index cf9823f4..ed2e478e 100644
--- a/manifests/agent/notification.pp
+++ b/manifests/agent/notification.pp
@@ -129,15 +129,15 @@ class ceilometer::agent::notification (
     } else {
       $service_ensure = 'stopped'
     }
-  }
 
-  service { 'ceilometer-agent-notification':
-    ensure     => $service_ensure,
-    name       => $::ceilometer::params::agent_notification_service_name,
-    enable     => $enabled,
-    hasstatus  => true,
-    hasrestart => true,
-    tag        => 'ceilometer-service'
+    service { 'ceilometer-agent-notification':
+      ensure     => $service_ensure,
+      name       => $::ceilometer::params::agent_notification_service_name,
+      enable     => $enabled,
+      hasstatus  => true,
+      hasrestart => true,
+      tag        => 'ceilometer-service'
+    }
   }
 
   if $manage_event_pipeline {
diff --git a/manifests/agent/polling.pp b/manifests/agent/polling.pp
index f5b2a9e2..5d5d51fc 100644
--- a/manifests/agent/polling.pp
+++ b/manifests/agent/polling.pp
@@ -120,14 +120,6 @@ class ceilometer::agent::polling (
     $ipmi_namespace_name = ''
   }
 
-  if $manage_service {
-    if $enabled {
-      $service_ensure = 'running'
-    } else {
-      $service_ensure = 'stopped'
-    }
-  }
-
   $namespaces = delete([$central_namespace_name, $compute_namespace_name, $ipmi_namespace_name], '')
   $namespaces_real = inline_template('<%= @namespaces.select { |x| x and x !~ /^undef/ }.compact.join "," %>')
 
@@ -143,13 +135,21 @@ class ceilometer::agent::polling (
     }
   }
 
-  service { 'ceilometer-polling':
-    ensure     => $service_ensure,
-    name       => $::ceilometer::params::agent_polling_service_name,
-    enable     => $enabled,
-    hasstatus  => true,
-    hasrestart => true,
-    tag        => 'ceilometer-service',
+  if $manage_service {
+    if $enabled {
+      $service_ensure = 'running'
+    } else {
+      $service_ensure = 'stopped'
+    }
+
+    service { 'ceilometer-polling':
+      ensure     => $service_ensure,
+      name       => $::ceilometer::params::agent_polling_service_name,
+      enable     => $enabled,
+      hasstatus  => true,
+      hasrestart => true,
+      tag        => 'ceilometer-service',
+    }
   }
 
   if $manage_polling {
diff --git a/spec/classes/ceilometer_agent_notification_spec.rb b/spec/classes/ceilometer_agent_notification_spec.rb
index 633060d2..96f167fd 100644
--- a/spec/classes/ceilometer_agent_notification_spec.rb
+++ b/spec/classes/ceilometer_agent_notification_spec.rb
@@ -67,7 +67,7 @@ describe 'ceilometer::agent::notification' do
 
         it 'configures ceilometer agent notification service' do
           is_expected.to contain_service('ceilometer-agent-notification').with(
-            :ensure     => (params[:manage_service] && params[:enabled]) ? 'running' : 'stopped',
+            :ensure     => params[:enabled] ? 'running' : 'stopped',
             :name       => platform_params[:agent_notification_service_name],
             :enable     => params[:enabled],
             :hasstatus  => true,
@@ -78,22 +78,13 @@ describe 'ceilometer::agent::notification' do
       end
     end
 
-    context 'with disabled service managing' do
+    context 'with service unmanaged' do
       before do
-        params.merge!({
-          :manage_service => false,
-          :enabled        => false })
+        params.merge!({ :manage_service => false })
       end
 
       it 'configures ceilometer-agent-notification service' do
-        is_expected.to contain_service('ceilometer-agent-notification').with(
-          :ensure     => nil,
-          :name       => platform_params[:agent_notification_service_name],
-          :enable     => false,
-          :hasstatus  => true,
-          :hasrestart => true,
-          :tag        => 'ceilometer-service'
-        )
+        is_expected.to_not contain_service('ceilometer-agent-notification')
       end
     end
 
diff --git a/spec/classes/ceilometer_agent_polling_spec.rb b/spec/classes/ceilometer_agent_polling_spec.rb
index fcd908fa..0b8ee22a 100644
--- a/spec/classes/ceilometer_agent_polling_spec.rb
+++ b/spec/classes/ceilometer_agent_polling_spec.rb
@@ -78,14 +78,13 @@ describe 'ceilometer::agent::polling' do
       it { should contain_ceilometer_config('DEFAULT/polling_namespaces').with_value('compute') }
     end
 
-    context 'with disabled service managing' do
+    context 'with service disabled' do
       before do
-        params.merge!( :manage_service => false,
-                       :enabled        => false )
+        params.merge!( :enabled => false )
       end
 
       it { should contain_service('ceilometer-polling').with(
-        :ensure     => nil,
+        :ensure     => 'stopped',
         :name       => platform_params[:agent_service_name],
         :enable     => false,
         :hasstatus  => true,
@@ -94,6 +93,14 @@ describe 'ceilometer::agent::polling' do
       )}
     end
 
+    context 'with service unmanaged' do
+      before do
+        params.merge!( :manage_service => false )
+      end
+
+      it { should_not contain_service('ceilometer-polling') }
+    end
+
     context 'with polling management enabled and default meters' do
       before do
         params.merge!( :manage_polling => true )