openstack/puppet-keystone
Code Issues Proposed changes

Remove keystone::service validation

Browse Source 
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
This commit is contained in:
Tobias Urdin 2019-10-23 22:36:50 +02:00
parent a8f3616a48
commit 3c95205e02
5 changed files with 165 additions and 161 deletions

83
manifests/init.pp

@ -310,26 +310,6 @@
# (string value) # (string value)
# Defaults to '/C=US/ST=Unset/L=Unset/O=Unset/CN=localhost' # 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*] # [*service_name*]
# (Optional) Name of the service that will be providing the # (Optional) Name of the service that will be providing the
# server functionality of keystone. For example, the default # server functionality of keystone. For example, the default
@ -349,7 +329,6 @@
# one called keystone-admin (as per the new Debian package # one called keystone-admin (as per the new Debian package
# which uses UWSGI instead of Apache). # which uses UWSGI instead of Apache).
# Defaults to '$::keystone::params::service_name' # Defaults to '$::keystone::params::service_name'
# NOTE: validate_service only applies if the default value is used.
# #
# [*max_token_size*] # [*max_token_size*]
# (Optional) maximum allowable Keystone token size # (Optional) maximum allowable Keystone token size
@ -566,6 +545,26 @@
# (Optional) Driver to use for managing tokens. # (Optional) Driver to use for managing tokens.
# Defaults to undef # 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 # == Dependencies
# None # None
# #
@ -655,10 +654,6 @@ class keystone(
$notification_format = $::os_service_default, $notification_format = $::os_service_default,
$control_exchange = $::os_service_default, $control_exchange = $::os_service_default,
$rpc_response_timeout = $::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, $service_name = $::keystone::params::service_name,
$max_token_size = $::os_service_default, $max_token_size = $::os_service_default,
$sync_db = true, $sync_db = true,
@ -699,6 +694,10 @@ class keystone(
$public_workers = undef, $public_workers = undef,
$cache_dir = undef, $cache_dir = undef,
$token_driver = undef, $token_driver = undef,
$validate_service = undef,
$validate_insecure = undef,
$validate_auth_url = undef,
$validate_cacert = undef,
) inherits keystone::params { ) inherits keystone::params {
include ::keystone::deps include ::keystone::deps
@ -961,35 +960,15 @@ admin_token will be removed in a later release")
case $service_name { case $service_name {
$::keystone::params::service_name, 'keystone-public-keystone-admin' : { $::keystone::params::service_name, 'keystone-public-keystone-admin' : {
$service_name_real = $::keystone::params::service_name $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': class { '::keystone::service':
ensure => $service_ensure, ensure => $service_ensure,
service_name => $service_name, service_name => $service_name,
enable => $enabled, enable => $enabled,
hasstatus => true, hasstatus => true,
hasrestart => 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,
}
} }
if $service_name == $::keystone::params::service_name { if $service_name == $::keystone::params::service_name {
warning("Keystone under Eventlet has been deprecated during the Kilo cycle. \ 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.") Support for deploying under eventlet will be dropped as of the M-release of OpenStack.")

68
manifests/service.pp

@ -9,28 +9,30 @@
# === Parameters # === Parameters
# #
# [*ensure*] # [*ensure*]
# (optional) The desired state of the keystone service # (Optional) The desired state of the keystone service
# Defaults to undef # Defaults to undef
# #
# [*service_name*] # [*service_name*]
# (optional) The name of the keystone service # (Optional) The name of the keystone service
# Defaults to $::keystone::params::service_name # Defaults to $::keystone::params::service_name
# #
# [*enable*] # [*enable*]
# (optional) Whether to enable the keystone service # (Optional) Whether to enable the keystone service
# Defaults to true # Defaults to true
# #
# [*hasstatus*] # [*hasstatus*]
# (optional) Whether the keystone service has status # (Optional) Whether the keystone service has status
# Defaults to true # Defaults to true
# #
# [*hasrestart*] # [*hasrestart*]
# (optional) Whether the keystone service has restart # (Optional) Whether the keystone service has restart
# Defaults to true # Defaults to true
# #
## DEPRECATED PARAMS
#
# [*validate*] # [*validate*]
# (optional) Whether to validate the service is working after any service refreshes # (optional) Whether to validate the service is working after any service refreshes
# Defaults to false # Defaults to undef
# #
# [*admin_token*] # [*admin_token*]
# (optional) The admin token to use for validation # (optional) The admin token to use for validation
@ -38,20 +40,20 @@
# #
# [*admin_endpoint*] # [*admin_endpoint*]
# (optional) The admin endpont to use for validation # (optional) The admin endpont to use for validation
# Defaults to 'http://localhost:5000/v2.0' # Defaults to undef
# #
# [*retries*] # [*retries*]
# (optional) Number of times to retry validation # (optional) Number of times to retry validation
# Defaults to 10 # Defaults to undef
# #
# [*delay*] # [*delay*]
# (optional) Number of seconds between validation attempts # (optional) Number of seconds between validation attempts
# Defaults to 2 # Defaults to undef
# #
# [*insecure*] # [*insecure*]
# (optional) Whether to validate keystone connections # (optional) Whether to validate keystone connections
# using the --insecure option with keystone client. # using the --insecure option with keystone client.
# Defaults to false # Defaults to undef
# #
# [*cacert*] # [*cacert*]
# (optional) Whether to validate keystone connections # (optional) Whether to validate keystone connections
@ -59,25 +61,25 @@
# with keystone client. # with keystone client.
# Defaults to undef # Defaults to undef
# #
class keystone::service( class keystone::service (
$ensure = undef, $ensure = undef,
$service_name = $::keystone::params::service_name, $service_name = $::keystone::params::service_name,
$enable = true, $enable = true,
$hasstatus = true, $hasstatus = true,
$hasrestart = true, $hasrestart = true,
$validate = false, ## DEPRECATED PARAMS
$validate = undef,
$admin_token = undef, $admin_token = undef,
$admin_endpoint = 'http://localhost:5000/v2.0', $admin_endpoint = undef,
$retries = 10, $retries = undef,
$delay = 2, $delay = undef,
$insecure = false, $insecure = undef,
$cacert = undef, $cacert = undef,
) { ) inherits keystone::params {
include ::keystone::deps include ::keystone::deps
include ::keystone::params
if ($service_name == 'keystone-public-keystone-admin'){ if $service_name == 'keystone-public-keystone-admin' {
service { 'keystone-public': service { 'keystone-public':
ensure => $ensure, ensure => $ensure,
name => 'keystone-public', name => 'keystone-public',
@ -86,6 +88,7 @@ class keystone::service(
hasrestart => $hasrestart, hasrestart => $hasrestart,
tag => 'keystone-service', tag => 'keystone-service',
} }
service { 'keystone-admin': service { 'keystone-admin':
ensure => $ensure, ensure => $ensure,
name => 'keystone-admin', name => 'keystone-admin',
@ -104,31 +107,4 @@ class keystone::service(
tag => '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'],
}
}
} }

9
releasenotes/notes/deprecate-service-validation-35fd28183776f94f.yaml Normal file

@ -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.

31
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') } it { is_expected.to contain_keystone_config('catalog/template_file').with_value('/etc/keystone/default_catalog.templates') }
end 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 describe 'setting another template catalog' do
let :params do let :params do
{ {

135
spec/classes/keystone_service_spec.rb

@ -1,40 +1,111 @@
require 'spec_helper' require 'spec_helper'
describe 'keystone::service' do describe 'keystone::service' do
shared_examples '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
let :params do let :params do
{ {}
:validate => 'true',
:admin_token => 'admintoken'
}
end end
it { is_expected.to contain_service('keystone').with( context 'with default parameters' do
:ensure => nil, it { is_expected.to contain_service('keystone').with(
:enable => true, :ensure => nil,
:hasstatus => true, :name => platform_params[:service_name],
:hasrestart => true, :enable => true,
:tag => 'keystone-service', :hasstatus => true,
) } :hasrestart => true,
it { is_expected.to contain_exec('validate_keystone_connection') } :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
end end