From f02dd508afd6c6615b0ea6bc40a746fb0b217b63 Mon Sep 17 00:00:00 2001 From: Peter Souter Date: Fri, 19 May 2017 12:24:10 +0100 Subject: [PATCH] Add an optional third parameter to redisget() (#209) * Specifies a default value if the value is not found or theres a failure to connect to redis * Adds spec and acceptance tests for new default value --- README.md | 15 ++++++- lib/puppet/parser/functions/redisget.rb | 39 ++++++++++++++--- spec/acceptance/redisget_spec.rb | 58 ++++++++++++++++++++++++- spec/functions/redisget_spec.rb | 39 ++++++++++++++--- 4 files changed, 135 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index bb2ad5eb..99e7fd49 100644 --- a/README.md +++ b/README.md @@ -102,13 +102,24 @@ class { '::redis::sentinel': ## `redisget()` function -`redisget()` takes two arguments that are strings. The first is the key -to be looked up and the second is the URL to the Redis service. +`redisget()` takes two or three arguments that are strings. The first is the key +to be looked up, the second is the URL to the Redis service and the +optional third argument is a default value to use if the key is not +found or connection to the Redis service cannot be made. + +Example of basic usage. ```puppet $version = redisget('version.myapp', 'redis://redis.example.com:6379') ``` +Example with default value specified. This is useful to allow for cached +data in case Redis is not available. + +```puppet +$version = redisget('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) +``` + You must have the 'redis' gem installed on your puppet master. ## Unit testing diff --git a/lib/puppet/parser/functions/redisget.rb b/lib/puppet/parser/functions/redisget.rb index 579aba19..425e33d7 100644 --- a/lib/puppet/parser/functions/redisget.rb +++ b/lib/puppet/parser/functions/redisget.rb @@ -3,26 +3,51 @@ module Puppet::Parser::Functions newfunction(:redisget, :type => :rvalue, :doc => <<-DOC Returns the value of the key being looked up or nil if the key does not -exist. Takes two arguments, the first being a string value of the key to be -looked up and the second is the URL to the Redis service. +exist. Takes two arguments with an optional third. The first being a string +value of the key to be looked up, the second is the URL to the Redis service +and the third optional argument is a default value to be used if the lookup +fails. @param redis_key [String] The key to look up in redis. @param redis_uri [String] The endpoint of the Redis instance. +@param default_value [String] The value to return if the key is not found or + the connection to Redis fails @return [String] The value of the key from redis @return [String] An empty string eg. `''` @example Calling the function. $version = redisget('version.myapp', 'redis://redis.example.com:6379') +@example Calling the function with a default if failure occurs + $version = redisget('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) DOC ) do |args| - raise(Puppet::ParseError, "redisget(): Wrong number of arguments given (#{args.size} for 2)") if args.size != 2 + raise(Puppet::ParseError, "redisget(): Wrong number of arguments given (#{args.size} for 2 or 3)") if args.size != 2 and args.size != 3 key = args[0] url = args[1] - raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{key.class} for String") if key.is_a?(String) == false - raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{url.class} for String") if url.is_a?(String) == false + if args.size == 3 + default = args[2] + raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{default.class} for String) for arg3 (default)") if default.is_a?(String) == false + end - redis = Redis.new(:url => url) - redis.get(key) + raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{key.class} for String) for arg1 (key)") if key.is_a?(String) == false + raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{url.class} for String) for arg2 (url)") if url.is_a?(String) == false + + begin + redis = Redis.new(:url => url) + returned_value = redis.get(key) + if returned_value == nil and defined?(default) != nil + default + else + returned_value + end + rescue Exception => e + if default + debug "Connection to redis failed with #{e} - Returning default value of #{default}" + default + else + raise(Puppet::Error, "connection to redis server failed - #{e}") + end + end end end diff --git a/spec/acceptance/redisget_spec.rb b/spec/acceptance/redisget_spec.rb index a2a49cea..39b9fea3 100644 --- a/spec/acceptance/redisget_spec.rb +++ b/spec/acceptance/redisget_spec.rb @@ -40,7 +40,20 @@ class { '::redis': notify{"mykey value: ${mykey}":} EOS - # Check output for fact string + # Check output for function return value + apply_manifest(pp, :catch_failures => true) do |r| + expect(r.stdout).to match(/mykey value: Hello/) + end + end + + it 'should return a value from valid MyKey with the redisget() function while specifying a default' do + pp = <<-EOS + $mykey = redisget('mykey', 'redis://127.0.0.1:6379', 'default_value') + + notify{"mykey value: ${mykey}":} + EOS + + # Check output for function return value apply_manifest(pp, :catch_failures => true) do |r| expect(r.stdout).to match(/mykey value: Hello/) end @@ -55,10 +68,51 @@ class { '::redis': } EOS - # Check output for fact string + # Check output for function return value apply_manifest(pp, :catch_failures => true) do |r| expect(r.stdout).to match(/foo_key value was empty string/) end end + it 'should return the specified default value when key not present with redisget() function' do + pp = <<-EOS + $foo_key = redisget('foo', 'redis://127.0.0.1:6379', 'default_value') + + notify { $foo_key: } + EOS + + # Check output for function return value + apply_manifest(pp, :catch_failures => true) do |r| + expect(r.stdout).to match(/default_value/) + end + end + + it 'should return the specified default value when connection to redis server fails' do + pp = <<-EOS + # Bogus port for redis server + $foo_key = redisget('foo', 'redis://127.0.0.1:12345', 'default_value') + + notify { $foo_key: } + EOS + + # Check output for function return value + apply_manifest(pp, :catch_failures => true) do |r| + expect(r.stdout).to match(/default_value/) + end + end + + it 'should return an error when specifying a non connectable redis server' do + pp = <<-EOS + # Bogus port for redis server + $foo_key = redisget('foo', 'redis://127.0.0.1:12345') + + notify { $foo_key: } + EOS + + # Check output for error when can't connect to bogus redis + apply_manifest(pp, :acceptable_exit_codes => [1]) do |r| + expect(r.stderr).to match(/Error connecting to Redis on 127.0.0.1:12345 \(Errno::ECONNREFUSED\)/) + end + end + end diff --git a/spec/functions/redisget_spec.rb b/spec/functions/redisget_spec.rb index 2dffda94..c3827c92 100644 --- a/spec/functions/redisget_spec.rb +++ b/spec/functions/redisget_spec.rb @@ -3,15 +3,31 @@ require 'redis' REDIS_URL='redis://localhost:6379' +BROKEN_URL='redis://redis.example.com:1234' describe 'redisget' do - context 'should return nil if key does not exist' do + context 'should error if connection to redis server cannot be made and no default is specified' do + it { is_expected.to run.with_params('nonexistent_key', BROKEN_URL).and_raise_error(Puppet::Error, /connection to redis server failed - getaddrinfo/) } + end + + context 'should return default value if connection to redis server cannot be made and default is specified' do + it { is_expected.to run.with_params('nonexistent_key', BROKEN_URL, 'default_value').and_return('default_value') } + end + + context 'should return nil if key does not exist and no default is specified' do + before { + mr = MockRedis.new + Redis.stubs(:new).returns(mr) + } + it { is_expected.to run.with_params('nonexistent_key', REDIS_URL).and_return(nil) } + end + + context 'should return the default value if specified and key does not exist' do before { mr = MockRedis.new Redis.stubs(:new).returns(mr) - mr.set('nonexistent_key', nil) } - it { is_expected.to run.with_params('nonexistent_key', REDIS_URL).and_return('') } + it { is_expected.to run.with_params('nonexistent_key', REDIS_URL, 'default_value').and_return('default_value') } end context 'should return the value of the specified key' do @@ -23,6 +39,15 @@ it { is_expected.to run.with_params('key', REDIS_URL).and_return('value') } end + context 'should return the value of the specified key and not the default' do + before { + mr = MockRedis.new + Redis.stubs(:new).returns(mr) + mr.set('key', 'value') + } + it { is_expected.to run.with_params('key', REDIS_URL, 'default_value').and_return('value') } + end + describe 'with incorrect arguments' do context 'with no argument specified' do it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) } @@ -32,8 +57,8 @@ it { is_expected.to run.with_params('some_key').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) } end - context 'with more than two arguments specified' do - it { is_expected.to run.with_params('too', 'many', 'args').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) } + context 'with more than three arguments specified' do + it { is_expected.to run.with_params('way', 'too', 'many', 'args').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) } end end @@ -50,6 +75,10 @@ context "specifing second parameter as <#{p}>" do it { is_expected.to run.with_params('valid', p).and_raise_error(Puppet::ParseError, /wrong argument type/i) } end + + context "specifing third parameter as <#{p}>" do + it { is_expected.to run.with_params('valid', p).and_raise_error(Puppet::ParseError, /wrong argument type/i) } + end end end end