Refactor api service management

Depends-on: https://review.opendev.org/962354
Change-Id: Ie93f2181e291bbf019379e5171924eaef4aee1c5
Signed-off-by: Takashi Kajinami <kajinamit@oss.nttdata.com>

manifests/api.pp

@@ -41,7 +41,7 @@
 class placement::api (
   Boolean $enabled                        = true,
   Boolean $manage_service                 = true,
-  $api_service_name                       = $placement::params::service_name,
+  String[1] $api_service_name             = $placement::params::service_name,
   Stdlib::Ensure::Package $ensure_package = 'present',
   Boolean $sync_db                        = false,
   $enable_proxy_headers_parsing           = $facts['os_service_default'],
@@ -60,29 +60,31 @@ class placement::api (
   }
 
   if $manage_service {
-    if $api_service_name == 'httpd' {
-      # The following logic is currently required only in Debian, because
-      # the other distributions don't provide an independent service for
-      # placement
-      if $placement::params::service_name {
-        service { 'placement-api':
-          ensure => 'stopped',
-          name   => $placement::params::service_name,
-          enable => false,
-          tag    => ['placement-service'],
-        }
-        Service['placement-api'] -> Service[$api_service_name]
-      }
-      $api_service_name_real = false
-      Service <| title == 'httpd' |> { tag +> 'placement-service' }
-    } else {
-      $api_service_name_real = $api_service_name
+    case $api_service_name {
+      'httpd': {
+        Service <| title == 'httpd' |> { tag +> 'placement-service' }
 
-      # On any uwsgi config change, we must restart Placement API.
-      Placement_api_uwsgi_config<||> ~> Service['placement-api']
+        if $placement::params::service_name {
+          service { 'placement-api':
+            ensure => 'stopped',
+            name   => $placement::params::service_name,
+            enable => false,
+            tag    => ['placement-service'],
+          }
+          Service['placement-api'] -> Service['httpd']
+        }
+
+        $api_service_name_real = undef
+      }
+      default: {
+        $api_service_name_real = $api_service_name
+
+        # On any uwsgi config change, we must restart Placement API.
+        Placement_api_uwsgi_config<||> ~> Service['placement-api']
+      }
     }
   } else {
-    $api_service_name_real = $api_service_name
+    $api_service_name_real = undef
   }
 
   placement::generic_service { 'api':

manifests/generic_service.pp

@@ -13,10 +13,12 @@
 # === Parameters:
 #
 # [*package_name*]
-#   (Required) The package name (for the generic_service)
+#   (Optional) The package name (for the generic_service)
+#   Defaults to undef.
 #
 # [*service_name*]
-#   (Required) The service name (for the generic_service)
+#   (Optional) The service name (for the generic_service)
+#   Defaults to undef.
 #
 # [*enabled*]
 #   (Optional) Define if the service must be enabled or not
@@ -31,8 +33,8 @@
 #   Defaults to 'present'.
 #
 define placement::generic_service (
-  $package_name,
-  $service_name,
+  Optional[String[1]] $package_name       = undef,
+  Optional[String[1]] $service_name       = undef,
   Boolean $enabled                        = true,
   Boolean $manage_service                 = true,
   Stdlib::Ensure::Package $ensure_package = 'present'
@@ -44,13 +46,11 @@ define placement::generic_service (
 
   # I need to mark that ths package should be
   # installed before placement_config
-  if ($package_name) {
-    if !defined(Package[$placement_title]) and !defined(Package[$package_name]) {
-      package { $placement_title:
-        ensure => $ensure_package,
-        name   => $package_name,
-        tag    => ['openstack', 'placement-package'],
-      }
+  if $package_name {
+    package { $placement_title:
+      ensure => $ensure_package,
+      name   => $package_name,
+      tag    => ['openstack', 'placement-package'],
     }
   }
 

manifests/params.pp

@@ -25,11 +25,11 @@ class placement::params {
       $python_package_name = 'python3-placement'
       $osc_package_name    = 'python3-osc-placement'
       case $facts['os']['name'] {
-        'Debian': {
-          $service_name    = 'placement-api'
+        'Ubuntu': {
+          $service_name = undef
         }
         default: {
-          $service_name    = undef
+          $service_name = 'placement-api'
         }
       }
       $wsgi_script_source  = '/usr/bin/placement-api'

spec/classes/placement_api_spec.rb

@@ -2,16 +2,22 @@ require 'spec_helper'
 
 describe 'placement::api' do
   shared_examples 'placement::api' do
+    let :params do
+      { :api_service_name => 'httpd'}
+    end
 
-    context 'with only required params' do
-      let :params do
-        {}
-      end
+    let :pre_condition do
+      <<-EOS
+      include apache
+      include placement::wsgi::apache
+EOS
+    end
 
+    context 'with defaults' do
       it { should contain_class('placement::deps') }
       it { should contain_class('placement::policy') }
       it { should contain_placement__generic_service('api').with(
-        :service_name   => platform_params[:service_name],
+        :service_name   => nil,
         :package_name   => platform_params[:package_name],
         :manage_service => true,
         :enabled        => true,
@@ -24,8 +30,8 @@ describe 'placement::api' do
     end
 
     context 'with ensure_package parameter provided' do
-      let :params do
-        { :ensure_package => 'latest' }
+      before :each do
+        params.merge!({ :ensure_package => 'latest' })
       end
 
       it { should contain_placement__generic_service('api').with(
@@ -34,8 +40,8 @@ describe 'placement::api' do
     end
 
     context 'with manage_service parameter provided' do
-      let :params do
-        { :manage_service => false }
+      before :each do
+        params.merge!({ :manage_service => false })
       end
 
       it { should contain_placement__generic_service('api').with(
@@ -44,16 +50,16 @@ describe 'placement::api' do
     end
 
     context 'with sync_db parameter provided' do
-      let :params do
-        { :sync_db => true }
+      before :each do
+        params.merge!({ :sync_db => true })
       end
 
       it { should contain_class('placement::db::sync') }
     end
 
     context 'with enable_proxy_headers_parsing set' do
-      let :params do
-        { :enable_proxy_headers_parsing => true }
+      before :each do
+        params.merge!({ :enable_proxy_headers_parsing => true })
       end
 
       it { should contain_oslo__middleware('placement_config').with(
@@ -62,6 +68,20 @@ describe 'placement::api' do
     end
   end
 
+  shared_examples 'placement::api in Debian' do
+    context 'with defaults' do
+      it { should contain_class('placement::deps') }
+      it { should contain_class('placement::policy') }
+      it { should contain_placement__generic_service('api').with(
+        :service_name   => platform_params[:service_name],
+        :package_name   => platform_params[:package_name],
+        :manage_service => true,
+        :enabled        => true,
+        :ensure_package => 'present',
+      ) }
+    end
+  end
+
   on_supported_os({
     :supported_os   => OSDefaults.get_supported_os
   }).each do |os,facts|
@@ -91,6 +111,9 @@ describe 'placement::api' do
       end
 
       it_behaves_like 'placement::api'
+      if facts[:os]['name'] == 'Debian'
+        it_behaves_like 'placement::api in Debian'
+      end
     end
   end
 end