From 3c95205e0253895e477b9135c10f5175d4166cfc Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Wed, 23 Oct 2019 22:36:50 +0200 Subject: [PATCH] Remove keystone::service validation This patch removes the validation in the keyston::service class. This functionality should be replaced by using something like the healthcheck module [1]. In the future somebody might want to implement a keystone_validator provider that does a proper keystone check but the http_conn_validator should be sufficient enough. [1] https://github.com/voxpupuli/puppet-healthcheck Change-Id: Ia20cf42ec23cdbfa1a499b3c5fcece1e5bbb8c22 --- manifests/init.pp | 83 ++++------- manifests/service.pp | 68 +++------ ...e-service-validation-35fd28183776f94f.yaml | 9 ++ spec/classes/keystone_init_spec.rb | 31 ---- spec/classes/keystone_service_spec.rb | 135 +++++++++++++----- 5 files changed, 165 insertions(+), 161 deletions(-) create mode 100644 releasenotes/notes/deprecate-service-validation-35fd28183776f94f.yaml diff --git a/manifests/init.pp b/manifests/init.pp index 9e8bf6575..5e0eb927b 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -310,26 +310,6 @@ # (string value) # Defaults to '/C=US/ST=Unset/L=Unset/O=Unset/CN=localhost' # -# [*validate_service*] -# (Optional) Whether to validate keystone connections after -# the service is started. -# Defaults to false -# -# [*validate_insecure*] -# (Optional) Whether to validate keystone connections -# using the --insecure option with keystone client. -# Defaults to false -# -# [*validate_cacert*] -# (Optional) Whether to validate keystone connections -# using the specified argument with the --os-cacert option -# with keystone client. -# Defaults to undef -# -# [*validate_auth_url*] -# (Optional) The url to validate keystone against -# Defaults to undef -# # [*service_name*] # (Optional) Name of the service that will be providing the # server functionality of keystone. For example, the default @@ -349,7 +329,6 @@ # one called keystone-admin (as per the new Debian package # which uses UWSGI instead of Apache). # Defaults to '$::keystone::params::service_name' -# NOTE: validate_service only applies if the default value is used. # # [*max_token_size*] # (Optional) maximum allowable Keystone token size @@ -566,6 +545,26 @@ # (Optional) Driver to use for managing tokens. # Defaults to undef # +# [*validate_service*] +# (Optional) Whether to validate keystone connections after +# the service is started. +# Defaults to undef +# +# [*validate_insecure*] +# (Optional) Whether to validate keystone connections +# using the --insecure option with keystone client. +# Defaults to undef +# +# [*validate_cacert*] +# (Optional) Whether to validate keystone connections +# using the specified argument with the --os-cacert option +# with keystone client. +# Defaults to undef +# +# [*validate_auth_url*] +# (Optional) The url to validate keystone against +# Defaults to undef +# # == Dependencies # None # @@ -655,10 +654,6 @@ class keystone( $notification_format = $::os_service_default, $control_exchange = $::os_service_default, $rpc_response_timeout = $::os_service_default, - $validate_service = false, - $validate_insecure = false, - $validate_auth_url = false, - $validate_cacert = undef, $service_name = $::keystone::params::service_name, $max_token_size = $::os_service_default, $sync_db = true, @@ -699,6 +694,10 @@ class keystone( $public_workers = undef, $cache_dir = undef, $token_driver = undef, + $validate_service = undef, + $validate_insecure = undef, + $validate_auth_url = undef, + $validate_cacert = undef, ) inherits keystone::params { include ::keystone::deps @@ -961,35 +960,15 @@ admin_token will be removed in a later release") case $service_name { $::keystone::params::service_name, 'keystone-public-keystone-admin' : { $service_name_real = $::keystone::params::service_name - if $validate_service { - if $validate_auth_url { - $v_auth_url = $validate_auth_url - } else { - $v_auth_url = $admin_endpoint - } - class { '::keystone::service': - ensure => $service_ensure, - service_name => $service_name, - enable => $enabled, - hasstatus => true, - hasrestart => true, - validate => true, - admin_endpoint => $v_auth_url, - admin_token => $admin_token, - insecure => $validate_insecure, - cacert => $validate_cacert, - } - } else { - class { '::keystone::service': - ensure => $service_ensure, - service_name => $service_name, - enable => $enabled, - hasstatus => true, - hasrestart => true, - validate => false, - } + class { '::keystone::service': + ensure => $service_ensure, + service_name => $service_name, + enable => $enabled, + hasstatus => true, + hasrestart => true, } + if $service_name == $::keystone::params::service_name { warning("Keystone under Eventlet has been deprecated during the Kilo cycle. \ Support for deploying under eventlet will be dropped as of the M-release of OpenStack.") diff --git a/manifests/service.pp b/manifests/service.pp index 3e7dac7a7..bdc554b6f 100644 --- a/manifests/service.pp +++ b/manifests/service.pp @@ -9,28 +9,30 @@ # === Parameters # # [*ensure*] -# (optional) The desired state of the keystone service +# (Optional) The desired state of the keystone service # Defaults to undef # # [*service_name*] -# (optional) The name of the keystone service +# (Optional) The name of the keystone service # Defaults to $::keystone::params::service_name # # [*enable*] -# (optional) Whether to enable the keystone service +# (Optional) Whether to enable the keystone service # Defaults to true # # [*hasstatus*] -# (optional) Whether the keystone service has status +# (Optional) Whether the keystone service has status # Defaults to true # # [*hasrestart*] -# (optional) Whether the keystone service has restart +# (Optional) Whether the keystone service has restart # Defaults to true # +## DEPRECATED PARAMS +# # [*validate*] # (optional) Whether to validate the service is working after any service refreshes -# Defaults to false +# Defaults to undef # # [*admin_token*] # (optional) The admin token to use for validation @@ -38,20 +40,20 @@ # # [*admin_endpoint*] # (optional) The admin endpont to use for validation -# Defaults to 'http://localhost:5000/v2.0' +# Defaults to undef # # [*retries*] # (optional) Number of times to retry validation -# Defaults to 10 +# Defaults to undef # # [*delay*] # (optional) Number of seconds between validation attempts -# Defaults to 2 +# Defaults to undef # # [*insecure*] # (optional) Whether to validate keystone connections # using the --insecure option with keystone client. -# Defaults to false +# Defaults to undef # # [*cacert*] # (optional) Whether to validate keystone connections @@ -59,25 +61,25 @@ # with keystone client. # Defaults to undef # -class keystone::service( +class keystone::service ( $ensure = undef, $service_name = $::keystone::params::service_name, $enable = true, $hasstatus = true, $hasrestart = true, - $validate = false, + ## DEPRECATED PARAMS + $validate = undef, $admin_token = undef, - $admin_endpoint = 'http://localhost:5000/v2.0', - $retries = 10, - $delay = 2, - $insecure = false, + $admin_endpoint = undef, + $retries = undef, + $delay = undef, + $insecure = undef, $cacert = undef, -) { +) inherits keystone::params { include ::keystone::deps - include ::keystone::params - if ($service_name == 'keystone-public-keystone-admin'){ + if $service_name == 'keystone-public-keystone-admin' { service { 'keystone-public': ensure => $ensure, name => 'keystone-public', @@ -86,6 +88,7 @@ class keystone::service( hasrestart => $hasrestart, tag => 'keystone-service', } + service { 'keystone-admin': ensure => $ensure, name => 'keystone-admin', @@ -104,31 +107,4 @@ class keystone::service( tag => 'keystone-service', } } - - if $insecure { - $insecure_s = '--insecure' - } else { - $insecure_s = '' - } - - if $cacert { - $cacert_s = "--os-cacert ${cacert}" - } else { - $cacert_s = '' - } - - if $validate and $admin_token and $admin_endpoint { - $cmd = "openstack --os-auth-url ${admin_endpoint} --os-token ${admin_token} ${insecure_s} ${cacert_s} user list" - $catch = 'name' - exec { 'validate_keystone_connection': - path => '/usr/bin:/bin:/usr/sbin:/sbin', - provider => shell, - command => $cmd, - subscribe => Service['keystone'], - refreshonly => true, - tries => $retries, - try_sleep => $delay, - notify => Anchor['keystone::service::end'], - } - } } diff --git a/releasenotes/notes/deprecate-service-validation-35fd28183776f94f.yaml b/releasenotes/notes/deprecate-service-validation-35fd28183776f94f.yaml new file mode 100644 index 000000000..b4c61ef0b --- /dev/null +++ b/releasenotes/notes/deprecate-service-validation-35fd28183776f94f.yaml @@ -0,0 +1,9 @@ +--- +deprecations: + - | + The service validation in keystone::service is deprecated so the following + parameters in keystone::service has no effect anymore, validate, admin_token, + admin_endpoint, retries, delay, insecure, cacert. + - | + The service validation in ::keystone is deprecated so the following parameters + has no effect validate_service, validate_insecure, validate_auth_url and validate_cacert. diff --git a/spec/classes/keystone_init_spec.rb b/spec/classes/keystone_init_spec.rb index 13f0d0a0c..9b8f242ad 100644 --- a/spec/classes/keystone_init_spec.rb +++ b/spec/classes/keystone_init_spec.rb @@ -715,37 +715,6 @@ describe 'keystone' do it { is_expected.to contain_keystone_config('catalog/template_file').with_value('/etc/keystone/default_catalog.templates') } end - describe 'with overridden validation_auth_url' do - let :params do - { - :admin_token => 'service_token', - :validate_service => true, - :validate_auth_url => 'http://some.host:5000', - :admin_endpoint => 'http://some.host:5000' - } - end - - it { is_expected.to contain_class('keystone::service').with( - 'validate' => true, - 'admin_endpoint' => 'http://some.host:5000' - )} - end - - describe 'with service validation' do - let :params do - { - :admin_token => 'service_token', - :validate_service => true, - :admin_endpoint => 'http://some.host:5000' - } - end - - it { is_expected.to contain_class('keystone::service').with( - 'validate' => true, - 'admin_endpoint' => 'http://some.host:5000' - )} - end - describe 'setting another template catalog' do let :params do { diff --git a/spec/classes/keystone_service_spec.rb b/spec/classes/keystone_service_spec.rb index 9a7967687..975bba57e 100644 --- a/spec/classes/keystone_service_spec.rb +++ b/spec/classes/keystone_service_spec.rb @@ -1,40 +1,111 @@ require 'spec_helper' describe 'keystone::service' do - - let :facts do - @default_facts.merge({ - :osfamily => 'Debian', - :os => { :name => 'Debian', :family => 'Debian', :release => { :major => '8', :minor => '0' } }, - }) - end - - describe "with default parameters" do - it { is_expected.to contain_service('keystone').with( - :ensure => nil, - :enable => true, - :hasstatus => true, - :hasrestart => true, - :tag => 'keystone-service', - ) } - it { is_expected.to_not contain_exec('validate_keystone_connection') } - end - - describe "with validation on" do + shared_examples 'keystone::service' do let :params do - { - :validate => 'true', - :admin_token => 'admintoken' - } + {} end - it { is_expected.to contain_service('keystone').with( - :ensure => nil, - :enable => true, - :hasstatus => true, - :hasrestart => true, - :tag => 'keystone-service', - ) } - it { is_expected.to contain_exec('validate_keystone_connection') } + context 'with default parameters' do + it { is_expected.to contain_service('keystone').with( + :ensure => nil, + :name => platform_params[:service_name], + :enable => true, + :hasstatus => true, + :hasrestart => true, + :tag => 'keystone-service', + )} + end + + context 'with overriden parameters' do + before do + params.merge!( + :ensure => 'present', + :enable => false, + :hasstatus => false, + :hasrestart => false + ) + end + + it { is_expected.to contain_service('keystone').with( + :ensure => 'present', + :name => platform_params[:service_name], + :enable => false, + :hasstatus => false, + :hasrestart => false, + )} + end + + context 'with service_name set to keystone-public-keystone-admin' do + before do + params.merge!( :service_name => 'keystone-public-keystone-admin' ) + end + + it { is_expected.to contain_service('keystone-public').with( + :ensure => nil, + :name => 'keystone-public', + :enable => true, + :hasstatus => true, + :hasrestart => true, + :tag => 'keystone-service', + )} + + it { is_expected.to contain_service('keystone-admin').with( + :ensure => nil, + :name => 'keystone-admin', + :enable => true, + :hasstatus => true, + :hasrestart => true, + :tag => 'keystone-service', + )} + end + + context 'with overriden parameters and service_name set to keystone-public-keystone-admin' do + before do + params.merge!( + :ensure => 'present', + :service_name => 'keystone-public-keystone-admin', + :enable => false, + :hasstatus => false, + :hasrestart => false + ) + end + + it { is_expected.to contain_service('keystone-public').with( + :ensure => 'present', + :name => 'keystone-public', + :enable => false, + :hasstatus => false, + :hasrestart => false, + )} + + it { is_expected.to contain_service('keystone-admin').with( + :ensure => 'present', + :name => 'keystone-admin', + :enable => false, + :hasstatus => false, + :hasrestart => false, + )} + 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()) + end + + let(:platform_params) do + if facts[:osfamily ] == 'RedHat' + { :service_name => 'openstack-keystone' } + else + { :service_name => 'keystone' } + end + end + + it_behaves_like 'keystone::service' + end end end