Refactor api service management

Drop validation of service name which is not implemented for
the other daemons, to simplify handling of the provided service name.

This allows us to more easily offload the service name definition to
hiera data in the near future.

Change-Id: I2467346e77ecb853f7b63cc072a4bfca1e0a4362
Signed-off-by: Takashi Kajinami <kajinamit@oss.nttdata.com>

manifests/api.pp

@@ -117,7 +117,7 @@ class barbican::api (
   Boolean $enabled                        = true,
   Boolean $sync_db                        = true,
   $db_auto_create                         = $facts['os_service_default'],
-  $service_name                           = $barbican::params::api_service_name,
+  String[1] $service_name                 = $barbican::params::api_service_name,
   $enable_proxy_headers_parsing           = $facts['os_service_default'],
   $max_request_body_size                  = $facts['os_service_default'],
   $max_limit_paging                       = $facts['os_service_default'],
@@ -190,64 +190,59 @@ class barbican::api (
   }
 
   if $manage_service {
-    if $enabled {
+    case $service_name {
-      $service_ensure = 'running'
+      'httpd': {
-    } else {
+        Service <| title == 'httpd' |> { tag +> 'barbican-service' }
-      $service_ensure = 'stopped'
-    }
 
-    if $service_name == $barbican::params::api_service_name {
+        if $barbican::params::api_service_name {
-      if $facts['os']['name'] == 'Ubuntu' {
+          service { 'barbican-api':
-        fail('With Ubuntu packages the service_name must be set to httpd as there is no eventlet init script.')
+            ensure => 'stopped',
-      }
+            name   => $barbican::params::api_service_name,
+            enable => false,
+            tag    => 'barbican-service',
+          }
 
-      service { 'barbican-api':
+          # we need to make sure barbican-api is stopped before trying to start apache
-        ensure     => $service_ensure,
+          Service['barbican-api'] -> Service['httpd']
-        name       => $barbican::params::api_service_name,
+        }
-        enable     => $enabled,
+
-        hasstatus  => true,
+        # On any paste-api.ini config change, we must restart Barbican API.
-        hasrestart => true,
+        Barbican_api_paste_ini<||> ~> Service['httpd']
-        tag        => 'barbican-service',
+      }
-      }
+      default: {
-
+        $service_ensure = $enabled ? {
-      # Debian is using UWSGI, not gunicorn
+          true    => 'running',
-      if $facts['os']['name'] != 'Debian' {
+          default => 'stopped',
-        $bind_host_real = normalize_ip_for_uri($bind_host)
-
-        file_line { 'Modify bind_port in gunicorn-config.py':
-          path    => '/etc/barbican/gunicorn-config.py',
-          line    => "bind = '${bind_host_real}:${bind_port}'",
-          match   => '^bind = .*',
-          tag     => 'modify-bind-port',
-          require => Anchor['barbican::config::begin'],
-          before  => Anchor['barbican::config::end'],
-          notify  => Service['barbican-api'],
         }
-      }
 
-      # On any paste-api.ini config change, we must restart Barbican API.
-      Barbican_api_paste_ini<||> ~> Service['barbican-api']
-      # On any uwsgi config change, we must restart Barbican API.
-      Barbican_api_uwsgi_config<||> ~> Service['barbican-api']
-    } elsif $service_name == 'httpd' {
-      # Ubuntu packages does not have a barbican-api service
-      if $facts['os']['name'] != 'Ubuntu' {
         service { 'barbican-api':
-          ensure => 'stopped',
+          ensure     => $service_ensure,
-          name   => $barbican::params::api_service_name,
+          name       => $service_name,
-          enable => false,
+          enable     => $enabled,
-          tag    => 'barbican-service',
+          hasstatus  => true,
+          hasrestart => true,
+          tag        => 'barbican-service',
         }
 
-        # we need to make sure barbican-api is stopped before trying to start apache
+        if $facts['os']['family'] == 'RedHat' {
-        Service['barbican-api'] -> Service[$service_name]
+          $bind_host_real = normalize_ip_for_uri($bind_host)
-      }
 
-      Service <| title == 'httpd' |> { tag +> 'barbican-service' }
+          file_line { 'Modify bind_port in gunicorn-config.py':
-      # On any paste-api.ini config change, we must restart Barbican API.
+            path    => '/etc/barbican/gunicorn-config.py',
-      Barbican_api_paste_ini<||> ~> Service[$service_name]
+            line    => "bind = '${bind_host_real}:${bind_port}'",
-    } else {
+            match   => '^bind = .*',
-      fail('Invalid service_name.')
+            tag     => 'modify-bind-port',
+            require => Anchor['barbican::config::begin'],
+            before  => Anchor['barbican::config::end'],
+            notify  => Service['barbican-api'],
+          }
+        }
+
+        # On any paste-api.ini config change, we must restart Barbican API.
+        Barbican_api_paste_ini<||> ~> Service['barbican-api']
+        # On any uwsgi config change, we must restart Barbican API.
+        Barbican_api_uwsgi_config<||> ~> Service['barbican-api']
+      }
     }
   }
 

manifests/params.pp

@@ -28,7 +28,14 @@ class barbican::params {
     }
     'Debian': {
       $common_package_name            = 'barbican-common'
-      $api_service_name               = 'barbican-api'
+      case $facts['os']['name'] {
+        'Ubuntu': {
+          $api_service_name = undef
+        }
+        default: {
+          $api_service_name = 'barbican-api'
+        }
+      }
       $api_package_name               = 'barbican-api'
       $worker_package_name            = 'barbican-worker'
       $worker_service_name            = 'barbican-worker'

spec/classes/barbican_api_spec.rb

@@ -21,172 +21,131 @@ require 'spec_helper'
 
 describe 'barbican::api' do
 
-  shared_examples_for 'barbican api' do
+  shared_examples_for 'barbican::api' do
-
+    let :pre_condition do
-    let :default_params do
+      <<-EOS
-      {
+      class { "barbican::keystone::authtoken":
-        :max_allowed_secret_in_bytes       => '<SERVICE DEFAULT>',
+        password => "secret",
-        :max_allowed_request_size_in_bytes => '<SERVICE DEFAULT>',
-        :manage_service                    => true,
-        :enabled                           => true,
-        :enabled_secretstore_plugins       => ['<SERVICE DEFAULT>'],
-        :enabled_crypto_plugins            => ['<SERVICE DEFAULT>'],
-        :auth_strategy                     => 'keystone',
-        :service_name                      => platform_params[:service_name],
-        :enable_proxy_headers_parsing      => '<SERVICE DEFAULT>',
-        :max_request_body_size             => '<SERVICE DEFAULT>',
-        :max_limit_paging                  => '<SERVICE DEFAULT>',
-        :default_limit_paging              => '<SERVICE DEFAULT>',
-        :multiple_secret_stores_enabled    => false,
-        :enabled_secret_stores             => 'simple_crypto',
       }
+      include apache
+EOS
     end
 
-    [
+    context 'with defaults' do
-      {},
-      {
-        :manage_service                    => true,
-        :enabled                           => false,
-        :enabled_secretstore_plugins       => ['dogtag_crypto', 'store_crypto', 'kmip'],
-        :enabled_crypto_plugins            => ['simple_crypto'],
-        :max_allowed_secret_in_bytes       => 20000,
-        :max_allowed_request_size_in_bytes => 2000000,
-        :enable_proxy_headers_parsing      => false,
-        :max_request_body_size             => '102400',
-        :max_limit_paging                  => 100,
-        :default_limit_paging              => 10,
-        :multiple_secret_stores_enabled    => true,
-        :enabled_secret_stores             => 'simple_crypto,dogtag,kmip',
-      }
-    ].each do |param_set|
-      describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do
-        let :pre_condition do
-          'class { "barbican::keystone::authtoken": password => "secret", }
-          include apache'
-        end
-
-        let :param_hash do
-          default_params.merge(param_set)
-        end
-
-        let :params do
-          default_params.merge(param_set)
-        end
-
-        it { is_expected.to contain_class('barbican::deps') }
-        it { is_expected.to contain_class('barbican::db') }
-        it { is_expected.to contain_class('barbican::policy') }
-
-        it { is_expected.to contain_package('barbican-api').with(
-            :tag => ['openstack', 'barbican-package'],
-         )}
-
-        it 'is_expected.to set default parameters' do
-          [
-            'max_allowed_secret_in_bytes',
-            'max_allowed_request_size_in_bytes',
-          ].each do |config|
-            is_expected.to contain_barbican_config("DEFAULT/#{config}").with_value(param_hash[config.intern])
-          end
-        end
-
-        it 'configures enabled plugins' do
-          is_expected.to contain_barbican_config('secretstore/enabled_secretstore_plugins') \
-            .with_value(param_hash[:enabled_secretstore_plugins])
-          is_expected.to contain_barbican_config('crypto/enabled_crypto_plugins') \
-            .with_value(param_hash[:enabled_crypto_plugins])
-        end
-
-        it 'configures plugins in multiple plugin config' do
-          is_expected.to contain_barbican_config('secretstore/stores_lookup_suffix') \
-            .with_value(param_hash[:enabled_secret_stores])
-          is_expected.to contain_barbican_config('secretstore/enable_multiple_secret_stores') \
-            .with_value(param_hash[:multiple_secret_stores_enabled])
-        end
-      end
-    end
-
-    describe 'with enable_proxy_headers_parsing' do
-      let :pre_condition do
-        'class { "barbican::keystone::authtoken": password => "secret", }
-        include apache'
-      end
-
       let :params do
-        default_params.merge!({:enable_proxy_headers_parsing => true })
+        {
+          :service_name => 'httpd'
+        }
       end
 
-      it { is_expected.to contain_oslo__middleware('barbican_config').with(
+      it { is_expected.to contain_class('barbican::deps') }
-        :enable_proxy_headers_parsing => true,
+      it { is_expected.to contain_class('barbican::db') }
-      )}
+      it { is_expected.to contain_class('barbican::policy') }
+
+      it { is_expected.to contain_package('barbican-api').with(
+        :ensure => 'present',
+        :name   => platform_params[:package_name],
+        :tag    => ['openstack', 'barbican-package'],
+       )}
+
+      it 'sets default parameters' do
+        is_expected.to contain_barbican_config('secretstore/enabled_secretstore_plugins').with_value('<SERVICE DEFAULT>')
+        is_expected.to contain_barbican_config('crypto/enabled_crypto_plugins').with_value('<SERVICE DEFAULT>')
+        is_expected.to contain_barbican_config('secretstore/enable_multiple_secret_stores').with_value(false)
+        is_expected.to contain_barbican_config('secretstore/stores_lookup_suffix').with_value('simple_crypto')
+
+        is_expected.to contain_barbican_config('DEFAULT/db_auto_create').with_value('<SERVICE DEFAULT>')
+
+        is_expected.to contain_oslo__middleware('barbican_config').with(
+          :enable_proxy_headers_parsing => '<SERVICE DEFAULT>',
+          :max_request_body_size        => '<SERVICE DEFAULT>',
+        )
+        is_expected.to contain_barbican_config('DEFAULT/max_limit_paging').with_value('<SERVICE DEFAULT>')
+        is_expected.to contain_barbican_config('DEFAULT/default_limit_paging').with_value('<SERVICE DEFAULT>')
+      end
     end
 
-    describe 'with max_request_body_size' do
+    context 'with parameters' do
-      let :pre_condition do
-        'class { "barbican::keystone::authtoken": password => "secret", }
-        include apache'
-      end
-
       let :params do
-        default_params.merge!({:max_request_body_size => '102400' })
+        {
+          :service_name                      => 'httpd',
+          :enabled_secretstore_plugins       => ['dogtag_crypto', 'store_crypto', 'kmip'],
+          :enabled_crypto_plugins            => ['simple_crypto'],
+          :multiple_secret_stores_enabled    => true,
+          :enabled_secret_stores             => ['simple_crypto', 'dogtag', 'kmip'],
+          :db_auto_create                    => false,
+          :max_allowed_secret_in_bytes       => 20000,
+          :max_allowed_request_size_in_bytes => 2000000,
+          :enable_proxy_headers_parsing      => false,
+          :max_request_body_size             => 102400,
+          :max_limit_paging                  => 100,
+          :default_limit_paging              => 10,
+        }
       end
 
-      it { is_expected.to contain_oslo__middleware('barbican_config').with(
+      it 'sets the provided parameters' do
-        :max_request_body_size => '102400',
+        is_expected.to contain_barbican_config('secretstore/enabled_secretstore_plugins').with_value(['dogtag_crypto', 'store_crypto', 'kmip'])
-      )}
+        is_expected.to contain_barbican_config('crypto/enabled_crypto_plugins').with_value(['simple_crypto'])
-    end
+        is_expected.to contain_barbican_config('secretstore/enable_multiple_secret_stores').with_value(true)
+        is_expected.to contain_barbican_config('secretstore/stores_lookup_suffix').with_value('simple_crypto,dogtag,kmip')
 
-    describe 'with keystone auth' do
+        is_expected.to contain_barbican_config('DEFAULT/db_auto_create').with_value(false)
-      let :pre_condition do
-        'class { "barbican::keystone::authtoken": password => "secret", }
-         include apache'
-      end
 
-      let :params do
+        is_expected.to contain_oslo__middleware('barbican_config').with(
-        default_params.merge({
+          :enable_proxy_headers_parsing => false,
-          :auth_strategy => 'keystone',
+          :max_request_body_size        => 102400,
-        })
+        )
-      end
+        is_expected.to contain_barbican_config('DEFAULT/max_limit_paging').with_value(100)
-
+        is_expected.to contain_barbican_config('DEFAULT/default_limit_paging').with_value(10)
-      it 'is_expected.to set keystone params correctly' do
-        is_expected.to contain_class('barbican::keystone::authtoken')
       end
     end
   end
 
-  shared_examples_for 'barbican api redhat' do
+  shared_examples_for 'barbican::api in RedHat' do
-    let :param_hash do
-      {
-        :manage_service => true,
-        :enabled        => true,
-        :auth_strategy  => 'keystone',
-      }
-    end
     let :pre_condition do
-      'class { "barbican::keystone::authtoken": password => "secret", }'
+      <<-EOS
+      class { "barbican::keystone::authtoken":
+        password => "secret",
+      }
+EOS
     end
-    context 'redhat systems eventlet service enabled' do
-      describe 'should contain eventlet service' do
-        it { is_expected.to contain_service('barbican-api').with(
-          'ensure'     => (param_hash[:manage_service] && param_hash[:enabled]) ? 'running': 'stopped',
-          'enable'     => param_hash[:enabled],
-          'hasstatus'  => true,
-          'hasrestart' => true,
-          'tag'        => 'barbican-service',
-        ) }
-      end
-    end
-    context 'on redhat systems eventlet service disabled' do
-      describe 'with disabled service managing' do
-        let :params do
-          {
-            :manage_service => false,
-            :enabled        => false,
-          }
-        end
 
-        it { is_expected.to_not contain_service('barbican-api') }
+    context 'with defaults' do
-      end
+      it { is_expected.to contain_service('barbican-api').with(
+        :ensure     => 'running',
+        :name       => platform_params[:service_name],
+        :enable     => true,
+        :hasstatus  => true,
+        :hasrestart => true,
+        :tag        => 'barbican-service',
+      )}
+      it { is_expected.to contain_file_line('Modify bind_port in gunicorn-config.py').with(
+        :path  => '/etc/barbican/gunicorn-config.py',
+        :line  => 'bind = \'0.0.0.0:9311\'',
+        :match => '^bind = .*',
+        :tag   => 'modify-bind-port',
+      ) }
+    end
+  end
+
+  shared_examples_for 'barbican::api in Debian' do
+    let :pre_condition do
+      <<-EOS
+      class { "barbican::keystone::authtoken":
+        password => "secret",
+      }
+EOS
+    end
+
+    context 'with defaults' do
+      it { is_expected.to contain_service('barbican-api').with(
+        :ensure     => 'running',
+        :name       => platform_params[:service_name],
+        :enabled    => true,
+        :hasstatus  => true,
+        :hasrestart => true,
+        :tag        => 'barbican-service',
+      )}
     end
   end
 
@@ -198,19 +157,37 @@ describe 'barbican::api' do
         facts.merge(OSDefaults.get_facts())
       end
 
-      case facts[:os]['family']
+      let (:platform_params) do
-      when 'RedHat'
+        case facts[:os]['family']
-        let (:platform_params) do
+        when 'RedHat'
-          { :service_name => 'openstack-barbican-api' }
+          {
-        end
+            :package_name => 'openstack-barbican-api',
-        it_behaves_like 'barbican api redhat'
+            :service_name => 'openstack-barbican-api'
-      when 'Debian'
+          }
-        let (:platform_params) do
+        when 'Debian'
-          { :service_name => 'httpd' }
+          case facts[:os]['name']
+          when 'Debian'
+            {
+              :package_name => 'barbican-api',
+              :service_name => 'barbican-api'
+            }
+          when 'Ubuntu'
+            {
+              :package_name => 'barbican-api',
+            }
+          end
         end
       end
 
-      it_behaves_like 'barbican api'
+      it_behaves_like 'barbican::api'
+      case facts[:os]['family']
+      when 'RedHat'
+        it_behaves_like 'barbican::api in RedHat'
+      when 'Deiban'
+        if facts[:os]['name'] == 'Debian'
+          it_behaves_like 'barbican::api in Debian'
+        end
+      end
     end
   end
 end