From e5a4ad15de083e4f1bf06953efdd3f12b8c53259 Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Wed, 30 Dec 2015 11:30:56 -0700 Subject: [PATCH] Fix cache and cache memcache configurations This change adds the ability to manage the cache memcache servers. Previously we were enabling the cache if memcache servers were set but improperly configuring the cache configuration options for keystone. The memcache servers were being set for memcache/servers but the cache configuration looks at cache/memcache_servers for its server list. With this change we are adding a cache_memcache_servers parameter to allow the configuring the cache/memcache_servers. We are continuing to use the memcache_servers as the default if cache_memcache_servers is not provided. Additionally we are allowing the manual enabling and disabling of the cache configuration but will fall back to the previous behaviour of enabling the cache if memcache servers are specified. New parameters are: - cache_memcache_servers - cache_enabled Change-Id: I6f86c7f8f55a6f7a7e8caa922c55e618fec8e392 Closes-Bug: #1523393 --- manifests/init.pp | 82 +++++++++++++++---------- spec/classes/keystone_spec.rb | 111 +++++++++++++++++++++++++++++----- 2 files changed, 147 insertions(+), 46 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 899651f6d..3a94b4878 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -88,9 +88,13 @@ # Defaults to /var/cache/keystone. # # [*memcache_servers*] -# (optional) List of memcache servers in format of server:port. +# (optional) List of memcache servers as a comma separated string of +# 'server:port,server:port' or an array of servers ['server:port', +# 'server:port']. # Used with token_driver 'keystone.token.backends.memcache.Token'. -# Defaults to false. Example: ['localhost:11211'] +# This configures the memcache/servers for keystone and is used as a default +# for $cache_memcache_servers if it is not specified. +# Defaults to $::os_service_default # # [*cache_backend*] # (optional) Dogpile.cache backend module. It is recommended that Memcache with pooling @@ -104,6 +108,22 @@ # This has no effects unless 'memcache_servers' is set. # Default to $::os_service_default # +# [*cache_enabled*] +# (optional) Setting this will enable the caching backend for Keystone. +# For legacy purposes, this will be enabled automatically enabled if it is +# not provided and $memcache_servers (or $cache_memcache_servers) is set and +# cache_backend is provided as well. +# Defaults to $::os_service_default +# +# [*cache_memcache_servers*] +# (optional) List of memcache servers to be used with the caching backend to +# configure cache/memcache_servers. +# Specified as as a comma separated string of 'server:port,server:port' or an +# array of servers ['server:port', 'server:port']. +# By default this will be set to the memcache_servers if that is configured +# and this is left unconfigured. +# Default to $::os_service_default +# # [*debug_cache_backend*] # (optional) Extra debugging from the cache backend (cache keys, get/set/delete calls). # This has no effects unless 'memcache_servers' is set. @@ -507,6 +527,8 @@ $manage_service = true, $cache_backend = $::os_service_default, $cache_backend_argument = $::os_service_default, + $cache_enabled = $::os_service_default, + $cache_memcache_servers = $::os_service_default, $debug_cache_backend = $::os_service_default, $token_caching = $::os_service_default, $enabled = true, @@ -696,43 +718,41 @@ } } - # memcache connection config - if ! is_service_default($memcache_servers) and $memcache_servers { - validate_array($memcache_servers) + if !is_service_default($memcache_servers) or !is_service_default($cache_memcache_servers) { Service<| title == 'memcached' |> -> Service['keystone'] - keystone_config { - 'cache/enabled': value => true; - 'memcache/servers': value => join($memcache_servers, ','); - } - if ! is_service_default($cache_backend_argument) { - validate_array($cache_backend_argument) - keystone_config { - 'cache/backend_argument': value => join($cache_backend_argument, ','); - } - } else { - keystone_config { - 'cache/backend_argument': ensure => absent; - } - } + } + + # TODO(aschultz): remove in N cycle + if is_service_default($cache_memcache_servers) and !is_service_default($memcache_servers) { + warning('The keystone module now provides a $cache_memcache_servers to be used with caching. Please specify it separately to configure cache/memcache_servers for keystone. This backwards compatibility will be removed in the N cycle.') + $cache_memcache_servers_real = $memcache_servers } else { - keystone_config { - 'cache/enabled': ensure => absent; - 'cache/backend_argument': ensure => absent; - 'memcache/servers': ensure => absent; - } + $cache_memcache_servers_real = $cache_memcache_servers + } + + # TODO(aschultz): remove in N cycle + if is_service_default($cache_enabled) and (!is_service_default($memcache_servers) or !is_service_default($cache_memcache_servers_real)) and !is_service_default($cache_backend) { + warning('cache_enabled has been added to control weither or not to enable caching. Please specify it separately to configure caching. We have enabled caching as a backwards compatibility that will be removed in the N cycle') + $cache_enabled_real = true + } else { + $cache_enabled_real = $cache_enabled } keystone_config { - 'memcache/dead_retry': value => $memcache_dead_retry; - 'memcache/socket_timeout': value => $memcache_socket_timeout; - 'memcache/pool_maxsize': value => $memcache_pool_maxsize; - 'memcache/pool_unused_timeout': value => $memcache_pool_unused_timeout; + 'cache/backend': value => $cache_backend; + 'cache/backend_argument': value => join(any2array($cache_backend_argument), ','); + 'cache/debug_cache_backend': value => $debug_cache_backend; + 'cache/enabled': value => $cache_enabled_real; 'cache/memcache_dead_retry': value => $memcache_dead_retry; - 'cache/memcache_socket_timeout': value => $memcache_socket_timeout; 'cache/memcache_pool_maxsize': value => $memcache_pool_maxsize; 'cache/memcache_pool_unused_timeout': value => $memcache_pool_unused_timeout; - 'cache/backend': value => $cache_backend; - 'cache/debug_cache_backend': value => $debug_cache_backend; + 'cache/memcache_servers': value => join(any2array($cache_memcache_servers_real), ','); + 'cache/memcache_socket_timeout': value => $memcache_socket_timeout; + 'memcache/dead_retry': value => $memcache_dead_retry; + 'memcache/pool_maxsize': value => $memcache_pool_maxsize; + 'memcache/pool_unused_timeout': value => $memcache_pool_unused_timeout; + 'memcache/servers': value => join(any2array($memcache_servers), ','); + 'memcache/socket_timeout': value => $memcache_socket_timeout; 'token/caching': value => $token_caching; } diff --git a/spec/classes/keystone_spec.rb b/spec/classes/keystone_spec.rb index c4707fc81..63763a22b 100644 --- a/spec/classes/keystone_spec.rb +++ b/spec/classes/keystone_spec.rb @@ -37,6 +37,11 @@ 'revoke_driver' => 'keystone.contrib.revoke.backends.sql.Revoke', 'revoke_by_id' => true, 'cache_dir' => '/var/cache/keystone', + 'memcache_servers' => '', + 'cache_backend' => '', + 'cache_backend_argument' => '', + 'cache_enabled' => '', + 'cache_memcache_servers' => '', 'enable_ssl' => false, 'ssl_certfile' => '/etc/keystone/ssl/certs/keystone.pem', 'ssl_keyfile' => '/etc/keystone/ssl/private/keystonekey.pem', @@ -526,23 +531,110 @@ it { is_expected.to contain_keystone_config('cache/memcache_socket_timeout').with_value('2') } it { is_expected.to contain_keystone_config('cache/memcache_pool_maxsize').with_value('1000') } it { is_expected.to contain_keystone_config('cache/memcache_pool_unused_timeout').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_servers').with_value('SERVER1:11211,SERVER2:11211') } it { is_expected.to contain_package('python-memcache').with( :name => 'python-memcache', :ensure => 'present' ) } end + describe 'configure cache memcache servers if set' do + let :params do + { + 'admin_token' => 'service_token', + 'memcache_servers' => [ 'SERVER1:11211', 'SERVER2:11211' ], + 'token_driver' => 'keystone.token.backends.memcache.Token', + 'cache_backend' => 'dogpile.cache.memcached', + 'cache_backend_argument' => ['url:SERVER3:12211'], + 'cache_memcache_servers' => [ 'SERVER3:11211', 'SERVER4:11211' ], + 'memcache_dead_retry' => '60', + 'memcache_socket_timeout' => '2', + 'memcache_pool_maxsize' => '1000', + 'memcache_pool_unused_timeout' => '60', + } + end + + it { is_expected.to contain_keystone_config("memcache/servers").with_value('SERVER1:11211,SERVER2:11211') } + it { is_expected.to contain_keystone_config('cache/enabled').with_value(true) } + it { is_expected.to contain_keystone_config('token/caching').with_value('') } + it { is_expected.to contain_keystone_config('cache/backend').with_value('dogpile.cache.memcached') } + it { is_expected.to contain_keystone_config('cache/backend_argument').with_value('url:SERVER3:12211') } + it { is_expected.to contain_keystone_config('memcache/dead_retry').with_value('60') } + it { is_expected.to contain_keystone_config('memcache/socket_timeout').with_value('2') } + it { is_expected.to contain_keystone_config('memcache/pool_maxsize').with_value('1000') } + it { is_expected.to contain_keystone_config('memcache/pool_unused_timeout').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_dead_retry').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_socket_timeout').with_value('2') } + it { is_expected.to contain_keystone_config('cache/memcache_pool_maxsize').with_value('1000') } + it { is_expected.to contain_keystone_config('cache/memcache_pool_unused_timeout').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_servers').with_value('SERVER3:11211,SERVER4:11211') } + it { is_expected.to contain_package('python-memcache').with( + :name => 'python-memcache', + :ensure => 'present' + ) } + end + + describe 'configure cache enabled if set' do + let :params do + { + 'admin_token' => 'service_token', + 'memcache_servers' => [ 'SERVER1:11211', 'SERVER2:11211' ], + 'token_driver' => 'keystone.token.backends.memcache.Token', + 'cache_backend' => 'dogpile.cache.memcached', + 'cache_backend_argument' => ['url:SERVER3:12211'], + 'cache_enabled' => false, + 'cache_memcache_servers' => [ 'SERVER3:11211', 'SERVER4:11211' ], + 'memcache_dead_retry' => '60', + 'memcache_socket_timeout' => '2', + 'memcache_pool_maxsize' => '1000', + 'memcache_pool_unused_timeout' => '60', + } + end + + it { is_expected.to contain_keystone_config("memcache/servers").with_value('SERVER1:11211,SERVER2:11211') } + it { is_expected.to contain_keystone_config('cache/enabled').with_value(false) } + it { is_expected.to contain_keystone_config('token/caching').with_value('') } + it { is_expected.to contain_keystone_config('cache/backend').with_value('dogpile.cache.memcached') } + it { is_expected.to contain_keystone_config('cache/backend_argument').with_value('url:SERVER3:12211') } + it { is_expected.to contain_keystone_config('memcache/dead_retry').with_value('60') } + it { is_expected.to contain_keystone_config('memcache/socket_timeout').with_value('2') } + it { is_expected.to contain_keystone_config('memcache/pool_maxsize').with_value('1000') } + it { is_expected.to contain_keystone_config('memcache/pool_unused_timeout').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_dead_retry').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_socket_timeout').with_value('2') } + it { is_expected.to contain_keystone_config('cache/memcache_pool_maxsize').with_value('1000') } + it { is_expected.to contain_keystone_config('cache/memcache_pool_unused_timeout').with_value('60') } + it { is_expected.to contain_keystone_config('cache/memcache_servers').with_value('SERVER3:11211,SERVER4:11211') } + it { is_expected.to contain_package('python-memcache').with( + :name => 'python-memcache', + :ensure => 'present' + ) } + end + + describe 'configure memcache servers with a string' do + let :params do + default_params.merge({ + 'memcache_servers' => 'SERVER1:11211,SERVER2:11211', + 'cache_memcache_servers' => 'SERVER3:11211,SERVER4:11211' + }) + end + + it { is_expected.to contain_keystone_config("memcache/servers").with_value('SERVER1:11211,SERVER2:11211') } + it { is_expected.to contain_keystone_config('cache/memcache_servers').with_value('SERVER3:11211,SERVER4:11211') } + end + + describe 'do not configure memcache servers when not set' do let :params do default_params end - it { is_expected.to contain_keystone_config("cache/enabled").with_ensure('absent') } + it { is_expected.to contain_keystone_config("cache/enabled").with_value('') } it { is_expected.to contain_keystone_config("token/caching").with_value('') } it { is_expected.to contain_keystone_config("cache/backend").with_value('') } - it { is_expected.to contain_keystone_config("cache/backend_argument").with_ensure('absent') } + it { is_expected.to contain_keystone_config("cache/backend_argument").with_value('') } it { is_expected.to contain_keystone_config("cache/debug_cache_backend").with_value('') } - it { is_expected.to contain_keystone_config("memcache/servers").with_ensure('absent') } + it { is_expected.to contain_keystone_config("memcache/servers").with_value('') } it { is_expected.to contain_keystone_config('memcache/dead_retry').with_value('') } it { is_expected.to contain_keystone_config('memcache/pool_maxsize').with_value('') } it { is_expected.to contain_keystone_config('memcache/pool_unused_timeout').with_value('') } @@ -550,18 +642,7 @@ it { is_expected.to contain_keystone_config('cache/memcache_socket_timeout').with_value('') } it { is_expected.to contain_keystone_config('cache/memcache_pool_maxsize').with_value('') } it { is_expected.to contain_keystone_config('cache/memcache_pool_unused_timeout').with_value('') } - end - - describe 'raise error if memcache_servers is not an array' do - let :params do - { - 'admin_token' => 'service_token', - 'memcache_servers' => 'ANY_SERVER:11211' - } - end - - it { expect { is_expected.to contain_class('keystone::params') }.to \ - raise_error(Puppet::Error, /is not an Array/) } + it { is_expected.to contain_keystone_config('cache/memcache_servers').with_value('') } end describe 'when enabling SSL' do