From 39b9c77ca3050a80e284d6dfc4ead961bec4cc00 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Sun, 31 May 2020 22:52:53 +0900 Subject: [PATCH] Refactor placement::api This patch refactors placement::api class with generic_service class to make service/package management more simple. Note that it also fixes incorrect stop of httpd service on non-Debian system. Backport note: This backports doesn't include deprecation part because we shouldn't deprecate parameteres in stable branches. Conflicts: manifests/api.pp Change-Id: I152fb025027480a4a5151d5372dd03618c482b95 (cherry picked from commit 425180f5b8789852cba3511e16218fcb9c4607e2) (cherry picked from commit dbafc9438384318cb66438f098858d608e042785) --- manifests/api.pp | 73 +++++++++++++-------------- manifests/params.pp | 4 +- manifests/wsgi/apache.pp | 10 ++-- spec/classes/placement_api_spec.rb | 81 ++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 43 deletions(-) create mode 100644 spec/classes/placement_api_spec.rb diff --git a/manifests/api.pp b/manifests/api.pp index 01f7423..4a5c2c7 100644 --- a/manifests/api.pp +++ b/manifests/api.pp @@ -36,50 +36,49 @@ # Defaults to false # class placement::api ( - $enabled = true, - $manage_service = true, - $api_service_name = $::placement::params::service_name, - $host = '0.0.0.0', - $port = '8778', - $package_ensure = 'present', - $sync_db = false, + $enabled = true, + $manage_service = true, + $api_service_name = $::placement::params::service_name, + $host = '0.0.0.0', + $port = '8778', + $package_ensure = 'present', + $sync_db = false, ) inherits placement::params { include ::placement::deps - package { 'placement-api': - ensure => $package_ensure, - name => $::placement::params::package_name, - tag => ['openstack', 'placement-package'], + 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 + } else { + $api_service_name_real = $api_service_name + } + + Service <| title == 'httpd' |> { tag +> 'placement-service' } + } else { + $api_service_name_real = $api_service_name } - if $manage_service { - if $enabled { - $service_ensure = 'running' - } else { - $service_ensure = 'stopped' - } - if $api_service_name == $::placement::params::service_name { - service { 'placement-api': - ensure => $service_ensure, - name => $::placement::params::service_name, - enable => $enabled, - hasstatus => true, - hasrestart => true, - tag => ['placement-service', 'placement-db-sync-service'], - } - } elsif $api_service_name == 'httpd' { - include ::apache::params - service { 'placement-api': - ensure => 'stopped', - name => $::placement::params::service_name, - enable => false, - tag => ['placement-service', 'placement-db-sync-service'], - } - Service['placement-api'] -> Service[$api_service_name] - Service<| title == 'httpd' |> { tag +> ['placement-service', 'placement-db-sync-service'] } - } + placement::generic_service { 'api': + service_name => $api_service_name_real, + package_name => $::placement::params::package_name, + manage_service => $manage_service, + enabled => $enabled, + ensure_package => $package_ensure, } + if $sync_db { include ::placement::db::sync } diff --git a/manifests/params.pp b/manifests/params.pp index 6d0a7f1..de109f7 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -19,7 +19,7 @@ class placement::params { $package_name = 'openstack-placement-api' $common_package_name = 'openstack-placement-common' $python_package_name = "python${pyvers_real}-placement" - $service_name = 'httpd' + $service_name = false $public_url = 'http://127.0.0.1/placement' $internal_url = 'http://127.0.0.1/placement' $admin_url = 'http://127.0.0.1/placement' @@ -36,7 +36,7 @@ class placement::params { $service_name = 'placement-api' } default: { - $service_name = 'httpd' + $service_name = false } } $public_url = 'http://127.0.0.1' diff --git a/manifests/wsgi/apache.pp b/manifests/wsgi/apache.pp index b3e869e..010fe7f 100644 --- a/manifests/wsgi/apache.pp +++ b/manifests/wsgi/apache.pp @@ -120,10 +120,12 @@ class placement::wsgi::apache ( include ::apache::mod::ssl } - placement::generic_service { 'api': - service_name => false, - package_name => $::placement::params::package_name, - ensure_package => $ensure_package, + if ! defined(Class['placement::api']) { + placement::generic_service { 'api': + service_name => false, + package_name => $::placement::params::package_name, + ensure_package => $ensure_package, + } } file { $::placement::params::httpd_config_file: diff --git a/spec/classes/placement_api_spec.rb b/spec/classes/placement_api_spec.rb new file mode 100644 index 0000000..0d2b347 --- /dev/null +++ b/spec/classes/placement_api_spec.rb @@ -0,0 +1,81 @@ +require 'spec_helper' + +describe 'placement::api' do + shared_examples 'placement::api' do + + context 'with only required params' do + let :params do + {} + end + + it { should contain_class('placement::deps') } + 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 + + context 'with package_ensure parameter provided' do + let :params do + { :package_ensure => false } + end + + it { should contain_placement__generic_service('api').with( + :ensure_package => false, + ) } + end + + context 'with manage_service parameter provided' do + let :params do + { :manage_service => false } + end + + it { should contain_placement__generic_service('api').with( + :manage_service => false, + ) } + end + + context 'with sync_db parameter provided' do + let :params do + { :sync_db => true } + end + + it { should contain_class('placement::db::sync') } + end + end + + on_supported_os({ + :supported_os => OSDefaults.get_supported_os + }).each do |os,facts| + context "on #{os}" do + let (:facts) do + facts.merge(OSDefaults.get_facts({ + :os_workers => 8, + :fqdn => 'some.host.tld', + :concat_basedir => '/var/lib/puppet/concat', + })) + end + + let(:platform_params) do + case facts[:osfamily] + when 'Debian' + if facts[:os_package_type] == 'debian' + { :service_name => 'placement-api', + :package_name => 'placement-api'} + else + { :service_name => false, + :package_name => 'placement-api'} + end + when 'RedHat' + { :service_name => false, + :package_name => 'openstack-placement-api'} + end + end + + it_behaves_like 'placement::api' + end + end +end