From ba789deac588dea83a129544f8aa00813db30bf0 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Tue, 7 Aug 2012 20:48:54 -0700 Subject: [PATCH 1/4] Add function ensure_resource and defined_with_params This commit adds 2 new functions with unit tests. defined_with_params works similarily to puppet's defined function, except it allows you to also specify a hash of params. defined_with_params will return true if a resource also exists that matches the specified type/title (just like with defined) as well as all of the specified params. ensure_resource is a function that basically combines defined_with_params with create_resources to conditionally create resources only if the specified resource (title, type, params) does not already exist. These functions are created to serve as an alternative to using defined as follows: if ! defined(Package['some_package']) { package { 'some_package': ensure => present, } The issue with this usage is that there is no guarentee about what parameters were set in the previous definition of the package that made its way into the catalog. ensure_resource could be used instead, as: ensure_resource('package', 'some_package', { 'ensure' => 'present' }) This will creat the package resources only if another resource does not exist with the specified parameters. --- .../parser/functions/defined_with_params.rb | 33 +++++++++++++++++ .../parser/functions/ensure_resource.rb | 28 +++++++++++++++ spec/functions/defined_with_params_spec.rb | 31 ++++++++++++++++ spec/functions/ensure_resource_spec.rb | 35 +++++++++++++++++++ 4 files changed, 127 insertions(+) create mode 100644 lib/puppet/parser/functions/defined_with_params.rb create mode 100644 lib/puppet/parser/functions/ensure_resource.rb create mode 100644 spec/functions/defined_with_params_spec.rb create mode 100644 spec/functions/ensure_resource_spec.rb diff --git a/lib/puppet/parser/functions/defined_with_params.rb b/lib/puppet/parser/functions/defined_with_params.rb new file mode 100644 index 0000000..e09e41c --- /dev/null +++ b/lib/puppet/parser/functions/defined_with_params.rb @@ -0,0 +1,33 @@ +# Test whether a given class or definition is defined +require 'puppet/parser/functions' + +Puppet::Parser::Functions.newfunction(:defined_with_params, + :type => :rvalue, + :doc => <<-'ENDOFDOC' +Takes a resource reference and an optional hash of attributes. + +Returns true if a resource with the specified attributes has already been added +to the catalog, and false otherwise. + + user { 'dan': + ensure => present, + } + + if ! defined_with_params(User[dan], {'ensure' => 'present' }) { + user { 'dan': ensure => present, } + } +ENDOFDOC +) do |vals| + reference, params = vals + raise(ArgumentError, 'Must specify a reference') unless reference + params ||= {} + ret = false + if resource = findresource(reference.to_s) + matches = params.collect do |key, value| + resource[key] == value + end + ret = params.empty? || !matches.include?(false) + end + Puppet.debug("Resource #{reference} was not determined to be defined") + ret +end diff --git a/lib/puppet/parser/functions/ensure_resource.rb b/lib/puppet/parser/functions/ensure_resource.rb new file mode 100644 index 0000000..8f9eadf --- /dev/null +++ b/lib/puppet/parser/functions/ensure_resource.rb @@ -0,0 +1,28 @@ +# Test whether a given class or definition is defined +require 'puppet/parser/functions' + +Puppet::Parser::Functions.newfunction(:ensure_resource, + :type => :statement, + :doc => <<-'ENDOFDOC' +Takes a resource type, title, and a list of attributes that describe a +resource. + + user { 'dan': + ensure => present, + } + +This example only creates the resource if it does not already exist: + + ensure_resource('user, 'dan', {'ensure' => 'present' }) +ENDOFDOC +) do |vals| + type, title, params = vals + raise(ArgumentError, 'Must specify a type') unless type + raise(ArgumentError, 'Must specify a title') unless title + params ||= {} + if function_defined_with_params(["#{type}[#{title}]", params]) + Puppet.debug("Resource #{type}[#{title}] does not need to be created b/c it already exists") + else + function_create_resources([type.capitalize, { title => params }]) + end +end diff --git a/spec/functions/defined_with_params_spec.rb b/spec/functions/defined_with_params_spec.rb new file mode 100644 index 0000000..21c08a0 --- /dev/null +++ b/spec/functions/defined_with_params_spec.rb @@ -0,0 +1,31 @@ +#! /usr/bin/env ruby -S rspec +require 'spec_helper' + +require 'rspec-puppet' +describe 'defined_with_params' do + describe 'when a resource is not specified' do + it { should run.with_params().and_raise_error(ArgumentError) } + end + describe 'when compared against a resource with no attributes' do + let :pre_condition do + 'user { "dan": }' + end + it do + should run.with_params('User[dan]', {}).and_return(true) + should run.with_params('User[bob]', {}).and_return(false) + should run.with_params('User[dan]', {'foo' => 'bar'}).and_return(false) + end + end + + describe 'when comparted against a resource with attributes' do + let :pre_condition do + 'user { "dan": ensure => present, shell => "/bin/csh", managehome => false}' + end + it do + should run.with_params('User[dan]', {}).and_return(true) + should run.with_params('User[dan]', {'ensure' => 'present'}).and_return(true) + should run.with_params('User[dan]', {'ensure' => 'present', 'managehome' => false}).and_return(true) + should run.with_params('User[dan]', {'ensure' => 'absent', 'managehome' => false}).and_return(false) + end + end +end diff --git a/spec/functions/ensure_resource_spec.rb b/spec/functions/ensure_resource_spec.rb new file mode 100644 index 0000000..e45a6d0 --- /dev/null +++ b/spec/functions/ensure_resource_spec.rb @@ -0,0 +1,35 @@ +#! /usr/bin/env ruby -S rspec +require 'spec_helper' + +require 'rspec-puppet' +describe 'ensure_resource' do + describe 'when a type or title is not specified' do + it do + should run.with_params().and_raise_error(ArgumentError) + should run.with_params(['type']).and_raise_error(ArgumentError) + end + end + describe 'when compared against a resource with no attributes' do + let :pre_condition do + 'user { "dan": }' + end + it do + should run.with_params('user', 'dan', {}) + compiler.catalog.resource('User[dan]').to_s.should == 'User[dan]' + end + end + + describe 'when comparted against a resource with attributes' do + let :pre_condition do + 'user { "dan": ensure => present, shell => "/bin/csh", managehome => false}' + end + it do + # these first three should not fail + should run.with_params('User', 'dan', {}) + should run.with_params('User', 'dan', {'ensure' => 'present'}) + should run.with_params('User', 'dan', {'ensure' => 'present', 'managehome' => false}) + # test that this fails + should run.with_params('User', 'dan', {'ensure' => 'absent', 'managehome' => false}).and_raise_error(Puppet::Error) + end + end +end From fe85f467c8f8e65c9fd09acff17ac4adb80f12b8 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Mon, 13 Aug 2012 18:52:00 -0700 Subject: [PATCH 2/4] Handle undef for parameter argument This commit adds better handling of the case where undef is passed as the parameter value. This works by converting '' into [] --- lib/puppet/parser/functions/defined_with_params.rb | 4 +++- spec/functions/defined_with_params_spec.rb | 1 + spec/functions/ensure_resource_spec.rb | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/puppet/parser/functions/defined_with_params.rb b/lib/puppet/parser/functions/defined_with_params.rb index e09e41c..d7df306 100644 --- a/lib/puppet/parser/functions/defined_with_params.rb +++ b/lib/puppet/parser/functions/defined_with_params.rb @@ -20,7 +20,9 @@ ENDOFDOC ) do |vals| reference, params = vals raise(ArgumentError, 'Must specify a reference') unless reference - params ||= {} + if (! params) || params == '' + params = {} + end ret = false if resource = findresource(reference.to_s) matches = params.collect do |key, value| diff --git a/spec/functions/defined_with_params_spec.rb b/spec/functions/defined_with_params_spec.rb index 21c08a0..f995b67 100644 --- a/spec/functions/defined_with_params_spec.rb +++ b/spec/functions/defined_with_params_spec.rb @@ -26,6 +26,7 @@ describe 'defined_with_params' do should run.with_params('User[dan]', {'ensure' => 'present'}).and_return(true) should run.with_params('User[dan]', {'ensure' => 'present', 'managehome' => false}).and_return(true) should run.with_params('User[dan]', {'ensure' => 'absent', 'managehome' => false}).and_return(false) + should run.with_params('User[dan]', '').and_return(true) end end end diff --git a/spec/functions/ensure_resource_spec.rb b/spec/functions/ensure_resource_spec.rb index e45a6d0..e03efda 100644 --- a/spec/functions/ensure_resource_spec.rb +++ b/spec/functions/ensure_resource_spec.rb @@ -26,6 +26,7 @@ describe 'ensure_resource' do it do # these first three should not fail should run.with_params('User', 'dan', {}) + should run.with_params('User', 'dan', '') should run.with_params('User', 'dan', {'ensure' => 'present'}) should run.with_params('User', 'dan', {'ensure' => 'present', 'managehome' => false}) # test that this fails From 222a65dfe23e15561369474b8db939aebf07bca4 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Mon, 13 Aug 2012 18:52:56 -0700 Subject: [PATCH 3/4] Add better docs about duplicate resource failures This commit adds better inline documentation explaining how replicate resource definitions can occur if the resource exists and does not have matching parameters. --- lib/puppet/parser/functions/ensure_resource.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/puppet/parser/functions/ensure_resource.rb b/lib/puppet/parser/functions/ensure_resource.rb index 8f9eadf..3205b9b 100644 --- a/lib/puppet/parser/functions/ensure_resource.rb +++ b/lib/puppet/parser/functions/ensure_resource.rb @@ -14,6 +14,11 @@ resource. This example only creates the resource if it does not already exist: ensure_resource('user, 'dan', {'ensure' => 'present' }) + +If the resource already exists but does not match the specified parameters, +this function will attempt to recreate the resource leading to a duplicate +resource definition error. + ENDOFDOC ) do |vals| type, title, params = vals From 5ffd33e8f1e50b717c9a4dae56230221f8ac97e6 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Mon, 13 Aug 2012 18:53:47 -0700 Subject: [PATCH 4/4] re-formatting This commit refactors to ensure 80 character lines. --- lib/puppet/parser/functions/ensure_resource.rb | 2 +- spec/functions/defined_with_params_spec.rb | 13 +++++++++---- spec/functions/ensure_resource_spec.rb | 10 +++++++--- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/puppet/parser/functions/ensure_resource.rb b/lib/puppet/parser/functions/ensure_resource.rb index 3205b9b..6a9e2ed 100644 --- a/lib/puppet/parser/functions/ensure_resource.rb +++ b/lib/puppet/parser/functions/ensure_resource.rb @@ -26,7 +26,7 @@ ENDOFDOC raise(ArgumentError, 'Must specify a title') unless title params ||= {} if function_defined_with_params(["#{type}[#{title}]", params]) - Puppet.debug("Resource #{type}[#{title}] does not need to be created b/c it already exists") + Puppet.debug("Resource #{type}[#{title}] not created b/c it already exists") else function_create_resources([type.capitalize, { title => params }]) end diff --git a/spec/functions/defined_with_params_spec.rb b/spec/functions/defined_with_params_spec.rb index f995b67..28dbab3 100644 --- a/spec/functions/defined_with_params_spec.rb +++ b/spec/functions/defined_with_params_spec.rb @@ -17,16 +17,21 @@ describe 'defined_with_params' do end end - describe 'when comparted against a resource with attributes' do + describe 'when compared against a resource with attributes' do let :pre_condition do 'user { "dan": ensure => present, shell => "/bin/csh", managehome => false}' end it do should run.with_params('User[dan]', {}).and_return(true) - should run.with_params('User[dan]', {'ensure' => 'present'}).and_return(true) - should run.with_params('User[dan]', {'ensure' => 'present', 'managehome' => false}).and_return(true) - should run.with_params('User[dan]', {'ensure' => 'absent', 'managehome' => false}).and_return(false) should run.with_params('User[dan]', '').and_return(true) + should run.with_params('User[dan]', {'ensure' => 'present'} + ).and_return(true) + should run.with_params('User[dan]', + {'ensure' => 'present', 'managehome' => false} + ).and_return(true) + should run.with_params('User[dan]', + {'ensure' => 'absent', 'managehome' => false} + ).and_return(false) end end end diff --git a/spec/functions/ensure_resource_spec.rb b/spec/functions/ensure_resource_spec.rb index e03efda..611666e 100644 --- a/spec/functions/ensure_resource_spec.rb +++ b/spec/functions/ensure_resource_spec.rb @@ -19,7 +19,7 @@ describe 'ensure_resource' do end end - describe 'when comparted against a resource with attributes' do + describe 'when compared against a resource with attributes' do let :pre_condition do 'user { "dan": ensure => present, shell => "/bin/csh", managehome => false}' end @@ -28,9 +28,13 @@ describe 'ensure_resource' do should run.with_params('User', 'dan', {}) should run.with_params('User', 'dan', '') should run.with_params('User', 'dan', {'ensure' => 'present'}) - should run.with_params('User', 'dan', {'ensure' => 'present', 'managehome' => false}) + should run.with_params('User', 'dan', + {'ensure' => 'present', 'managehome' => false} + ) # test that this fails - should run.with_params('User', 'dan', {'ensure' => 'absent', 'managehome' => false}).and_raise_error(Puppet::Error) + should run.with_params('User', 'dan', + {'ensure' => 'absent', 'managehome' => false} + ).and_raise_error(Puppet::Error) end end end