diff options
author | Jay Mundrawala <jdmundrawala@gmail.com> | 2015-03-25 14:53:56 -0700 |
---|---|---|
committer | Jay Mundrawala <jdmundrawala@gmail.com> | 2015-03-25 14:53:56 -0700 |
commit | be67482cf2b8819a821bae96c8abd37a649e4c1c (patch) | |
tree | 80a5a1186beaff747ac196e1a648965b9879f1bf | |
parent | b6eb10f36ae82f0bcf56a04963034abc4f70fc7c (diff) | |
download | chef-be67482cf2b8819a821bae96c8abd37a649e4c1c.tar.gz |
Revert "Merge pull request #2956 from chef/lcg/deploy-fixes"
This reverts commit 2603e2153d6ab50179d2278025a51579edb9033f, reversing
changes made to a7f5c92960aedf8d5bfc71abbce430ab075e016a.
Conflicts:
CHANGELOG.md
-rw-r--r-- | CHANGELOG.md | 3 | ||||
-rw-r--r-- | lib/chef/mixin/params_validate.rb | 61 | ||||
-rw-r--r-- | lib/chef/resource.rb | 9 | ||||
-rw-r--r-- | lib/chef/resource/deploy.rb | 266 | ||||
-rw-r--r-- | lib/chef/resource/lwrp_base.rb | 8 | ||||
-rw-r--r-- | spec/unit/mixin/params_validate_spec.rb | 136 | ||||
-rw-r--r-- | spec/unit/resource/deploy_spec.rb | 27 |
7 files changed, 304 insertions, 206 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d6a409072..e547ed7d08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,6 @@ ## Unreleased * Update policyfile API usage to match forthcoming Chef Server release -* make deploy resource attributes nillable (`symlink_before_migrate nil`) works now -* mixin the LWRP attribute DSL method into Chef::Resource directly -* make all LWRP attributes nillable * `knife ssh` now has an --exit-on-error option that allows users to fail-fast rather than moving on to the next machine. * migrate macosx, windows, openbsd, and netbsd resources to dynamic resolution diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index baf210bfc5..78d72dc801 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -81,58 +81,34 @@ class Chef DelayedEvaluator.new(&block) end - NULL_ARG = Object.new - - def nillable_set_or_return(symbol, arg, validation) + def set_or_return(symbol, arg, validation) iv_symbol = "@#{symbol.to_s}".to_sym - if NULL_ARG.equal?(arg) - if self.instance_variable_defined?(iv_symbol) == true - get_ivar(iv_symbol, symbol, validation) + if arg == nil && self.instance_variable_defined?(iv_symbol) == true + ivar = self.instance_variable_get(iv_symbol) + if(ivar.is_a?(DelayedEvaluator)) + validate({ symbol => ivar.call }, { symbol => validation })[symbol] else - # on access we create the iv and set it to nil for back-compat - set_ivar(iv_symbol, symbol, nil, validation) + ivar end else - set_ivar(iv_symbol, symbol, arg, validation) - end - end + if(arg.is_a?(DelayedEvaluator)) + val = arg + else + val = validate({ symbol => arg }, { symbol => validation })[symbol] - def set_or_return(symbol, arg, validation) - iv_symbol = "@#{symbol.to_s}".to_sym - if arg == nil && self.instance_variable_defined?(iv_symbol) == true - get_ivar(iv_symbol, symbol, validation) - else - set_ivar(iv_symbol, symbol, arg, validation) + # Handle the case where the "default" was a DelayedEvaluator. In + # this case, the block yields an optional parameter of +self+, + # which is the equivalent of "new_resource" + if val.is_a?(DelayedEvaluator) + val = val.call(self) + end + end + self.instance_variable_set(iv_symbol, val) end end private - def get_ivar(iv_symbol, symbol, validation) - ivar = self.instance_variable_get(iv_symbol) - if(ivar.is_a?(DelayedEvaluator)) - validate({ symbol => ivar.call }, { symbol => validation })[symbol] - else - ivar - end - end - - def set_ivar(iv_symbol, symbol, arg, validation) - if(arg.is_a?(DelayedEvaluator)) - val = arg - else - val = validate({ symbol => arg }, { symbol => validation })[symbol] - - # Handle the case where the "default" was a DelayedEvaluator. In - # this case, the block yields an optional parameter of +self+, - # which is the equivalent of "new_resource" - if val.is_a?(DelayedEvaluator) - val = val.call(self) - end - end - self.instance_variable_set(iv_symbol, val) - end - # Return the value of a parameter, or nil if it doesn't exist. def _pv_opts_lookup(opts, key) if opts.has_key?(key.to_s) @@ -263,3 +239,4 @@ class Chef end end end + diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 2df73a52e8..ea220b6c70 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -997,15 +997,6 @@ class Chef end # - # DSL method used to define attribute on a resource wrapping params_validate - # - def self.attribute(attr_name, validation_opts={}) - define_method(attr_name) do |arg=NULL_ARG| - nillable_set_or_return(attr_name.to_sym, arg, validation_opts) - end - end - - # # The cookbook in which this Resource was defined (if any). # # @return Chef::CookbookVersion The cookbook in which this Resource was defined. diff --git a/lib/chef/resource/deploy.rb b/lib/chef/resource/deploy.rb index fade82a972..9beeb02534 100644 --- a/lib/chef/resource/deploy.rb +++ b/lib/chef/resource/deploy.rb @@ -83,7 +83,6 @@ class Chef @keep_releases = 5 @enable_checkout = true @checkout_branch = "deploy" - @timeout = nil end # where the checked out/cloned code goes @@ -106,18 +105,42 @@ class Chef end # note: deploy_to is your application "meta-root." - attribute :deploy_to, :kind_of => [ String ] + def deploy_to(arg=nil) + set_or_return( + :deploy_to, + arg, + :kind_of => [ String ] + ) + end - attribute :repo, :kind_of => [ String ] + def repo(arg=nil) + set_or_return( + :repo, + arg, + :kind_of => [ String ] + ) + end alias :repository :repo - attribute :remote, :kind_of => [ String ] + def remote(arg=nil) + set_or_return( + :remote, + arg, + :kind_of => [ String ] + ) + end - attribute :role, :kind_of => [ String ] + def role(arg=nil) + set_or_return( + :role, + arg, + :kind_of => [ String ] + ) + end - def restart_command(arg=NULL_ARG, &block) - arg = block if block_given? - nillable_set_or_return( + def restart_command(arg=nil, &block) + arg ||= block + set_or_return( :restart_command, arg, :kind_of => [ String, Proc ] @@ -125,60 +148,155 @@ class Chef end alias :restart :restart_command - attribute :migrate, :kind_of => [ TrueClass, FalseClass ] + def migrate(arg=nil) + set_or_return( + :migrate, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end - attribute :migration_command, kind_of: String + def migration_command(arg=nil) + set_or_return( + :migration_command, + arg, + :kind_of => [ String ] + ) + end - attribute :rollback_on_error, :kind_of => [ TrueClass, FalseClass ] + def rollback_on_error(arg=nil) + set_or_return( + :rollback_on_error, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end - attribute :user, kind_of: String + def user(arg=nil) + set_or_return( + :user, + arg, + :kind_of => [ String ] + ) + end - attribute :group, kind_of: [ String ] + def group(arg=nil) + set_or_return( + :group, + arg, + :kind_of => [ String ] + ) + end - attribute :enable_submodules, kind_of: [ TrueClass, FalseClass ] + def enable_submodules(arg=nil) + set_or_return( + :enable_submodules, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end - attribute :shallow_clone, kind_of: [ TrueClass, FalseClass ] + def shallow_clone(arg=nil) + set_or_return( + :shallow_clone, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end - attribute :repository_cache, kind_of: String + def repository_cache(arg=nil) + set_or_return( + :repository_cache, + arg, + :kind_of => [ String ] + ) + end - attribute :copy_exclude, kind_of: String + def copy_exclude(arg=nil) + set_or_return( + :copy_exclude, + arg, + :kind_of => [ String ] + ) + end - attribute :revision, kind_of: String + def revision(arg=nil) + set_or_return( + :revision, + arg, + :kind_of => [ String ] + ) + end alias :branch :revision - attribute :git_ssh_wrapper, kind_of: String + def git_ssh_wrapper(arg=nil) + set_or_return( + :git_ssh_wrapper, + arg, + :kind_of => [ String ] + ) + end alias :ssh_wrapper :git_ssh_wrapper - attribute :svn_username, kind_of: String + def svn_username(arg=nil) + set_or_return( + :svn_username, + arg, + :kind_of => [ String ] + ) + end - attribute :svn_password, kind_of: String + def svn_password(arg=nil) + set_or_return( + :svn_password, + arg, + :kind_of => [ String ] + ) + end - attribute :svn_arguments, kind_of: String + def svn_arguments(arg=nil) + set_or_return( + :svn_arguments, + arg, + :kind_of => [ String ] + ) + end - attribute :svn_info_args, kind_of: String + def svn_info_args(arg=nil) + set_or_return( + :svn_arguments, + arg, + :kind_of => [ String ]) + end - def scm_provider(arg=NULL_ARG) + def scm_provider(arg=nil) klass = if arg.kind_of?(String) || arg.kind_of?(Symbol) lookup_provider_constant(arg) else arg end - nillable_set_or_return( + set_or_return( :scm_provider, klass, :kind_of => [ Class ] ) end - attribute :svn_force_export, kind_of: [ TrueClass, FalseClass ] + def svn_force_export(arg=nil) + set_or_return( + :svn_force_export, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end - def environment(arg=NULL_ARG) + def environment(arg=nil) if arg.is_a?(String) Chef::Log.debug "Setting RAILS_ENV, RACK_ENV, and MERB_ENV to `#{arg}'" Chef::Log.warn "[DEPRECATED] please modify your deploy recipe or attributes to set the environment using a hash" arg = {"RAILS_ENV"=>arg,"MERB_ENV"=>arg,"RACK_ENV"=>arg} end - nillable_set_or_return( + set_or_return( :environment, arg, :kind_of => [ Hash ] @@ -186,8 +304,8 @@ class Chef end # The number of old release directories to keep around after cleanup - def keep_releases(arg=NULL_ARG) - [nillable_set_or_return( + def keep_releases(arg=nil) + [set_or_return( :keep_releases, arg, :kind_of => [ Integer ]), 1].max @@ -197,7 +315,13 @@ class Chef # SCM clone/checkout before symlinking. Use this to get rid of files and # directories you want to be shared between releases. # Default: ["log", "tmp/pids", "public/system"] - attribute :purge_before_symlink, kind_of: Array + def purge_before_symlink(arg=nil) + set_or_return( + :purge_before_symlink, + arg, + :kind_of => Array + ) + end # An array of paths, relative to your app's root, where you expect dirs to # exist before symlinking. This runs after #purge_before_symlink, so you @@ -207,7 +331,13 @@ class Chef # then specify tmp here so that the tmp directory will exist when you # symlink the pids directory in to the current release. # Default: ["tmp", "public", "config"] - attribute :create_dirs_before_symlink, kind_of: Array + def create_dirs_before_symlink(arg=nil) + set_or_return( + :create_dirs_before_symlink, + arg, + :kind_of => Array + ) + end # A Hash of shared/dir/path => release/dir/path. This attribute determines # which files and dirs in the shared directory get symlinked to the current @@ -215,7 +345,13 @@ class Chef # $shared/pids that you would like to symlink as $current_release/tmp/pids # you specify it as "pids" => "tmp/pids" # Default {"system" => "public/system", "pids" => "tmp/pids", "log" => "log"} - attribute :symlinks, kind_of: Hash + def symlinks(arg=nil) + set_or_return( + :symlinks, + arg, + :kind_of => Hash + ) + end # A Hash of shared/dir/path => release/dir/path. This attribute determines # which files in the shared directory get symlinked to the current release @@ -224,44 +360,74 @@ class Chef # For a rails/merb app, this is used to link in a known good database.yml # (with the production db password) before running migrate. # Default {"config/database.yml" => "config/database.yml"} - attribute :symlink_before_migrate, kind_of: Hash + def symlink_before_migrate(arg=nil) + set_or_return( + :symlink_before_migrate, + arg, + :kind_of => Hash + ) + end # Callback fires before migration is run. - def before_migrate(arg=NULL_ARG, &block) - arg = block if block_given? - nillable_set_or_return(:before_migrate, arg, kind_of: [Proc, String]) + def before_migrate(arg=nil, &block) + arg ||= block + set_or_return(:before_migrate, arg, :kind_of => [Proc, String]) end # Callback fires before symlinking - def before_symlink(arg=NULL_ARG, &block) - arg = block if block_given? - nillable_set_or_return(:before_symlink, arg, kind_of: [Proc, String]) + def before_symlink(arg=nil, &block) + arg ||= block + set_or_return(:before_symlink, arg, :kind_of => [Proc, String]) end # Callback fires before restart - def before_restart(arg=NULL_ARG, &block) - arg = block if block_given? - nillable_set_or_return(:before_restart, arg, kind_of: [Proc, String]) + def before_restart(arg=nil, &block) + arg ||= block + set_or_return(:before_restart, arg, :kind_of => [Proc, String]) end # Callback fires after restart - def after_restart(arg=NULL_ARG, &block) - arg = block if block_given? - nillable_set_or_return(:after_restart, arg, kind_of: [Proc, String]) + def after_restart(arg=nil, &block) + arg ||= block + set_or_return(:after_restart, arg, :kind_of => [Proc, String]) end - attribute :additional_remotes, kind_of: Hash + def additional_remotes(arg=nil) + set_or_return( + :additional_remotes, + arg, + :kind_of => Hash + ) + end - attribute :enable_checkout, kind_of: [ TrueClass, FalseClass ] + def enable_checkout(arg=nil) + set_or_return( + :enable_checkout, + arg, + :kind_of => [TrueClass, FalseClass] + ) + end - attribute :checkout_branch, kind_of: String + def checkout_branch(arg=nil) + set_or_return( + :checkout_branch, + arg, + :kind_of => String + ) + end # FIXME The Deploy resource may be passed to an SCM provider as its # resource. The SCM provider knows that SCM resources can specify a # timeout for SCM operations. The deploy resource must therefore support # a timeout method, but the timeout it describes is for SCM operations, # not the overall deployment. This is potentially confusing. - attribute :timeout, kind_of: Integer + def timeout(arg=nil) + set_or_return( + :timeout, + arg, + :kind_of => Integer + ) + end end end diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb index a777c511f0..ce72e98028 100644 --- a/lib/chef/resource/lwrp_base.rb +++ b/lib/chef/resource/lwrp_base.rb @@ -70,6 +70,14 @@ class Chef alias_method :resource_name=, :resource_name end + # Define an attribute on this resource, including optional validation + # parameters. + def self.attribute(attr_name, validation_opts={}) + define_method(attr_name) do |arg=nil| + set_or_return(attr_name.to_sym, arg, validation_opts) + end + end + # Sets the default action def self.default_action(action_name=NULL_ARG) unless action_name.equal?(NULL_ARG) diff --git a/spec/unit/mixin/params_validate_spec.rb b/spec/unit/mixin/params_validate_spec.rb index 1b61f9b238..85e1c1abab 100644 --- a/spec/unit/mixin/params_validate_spec.rb +++ b/spec/unit/mixin/params_validate_spec.rb @@ -339,83 +339,69 @@ describe Chef::Mixin::ParamsValidate do end.to raise_error(Chef::Exceptions::ValidationFailed) end - def self.test_set_or_return_method(method) - # caller is responsible for passing in the right arg to get 'return' behavior - return_arg = method == :nillable_set_or_return ? TinyClass::NULL_ARG : nil - - it "#{method} should set and return a value, then return the same value" do - value = "meow" - expect(@vo.send(method,:test, value, {}).object_id).to eq(value.object_id) - expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id) - end - - it "#{method} should set and return a default value when the argument is nil, then return the same value" do - value = "meow" - expect(@vo.send(method,:test, return_arg, { :default => value }).object_id).to eq(value.object_id) - expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id) - end - - it "#{method} should raise an ArgumentError when argument is nil and required is true" do - expect { - @vo.send(method,:test, return_arg, { :required => true }) - }.to raise_error(ArgumentError) - end - - it "#{method} should not raise an error when argument is nil and required is false" do - expect { - @vo.send(method,:test, return_arg, { :required => false }) - }.not_to raise_error - end - - it "#{method} should set and return @name, then return @name for foo when argument is nil" do - value = "meow" - expect(@vo.send(method,:name, value, { }).object_id).to eq(value.object_id) - expect(@vo.send(method,:foo, return_arg, { :name_attribute => true }).object_id).to eq(value.object_id) - end - - it "#{method} should allow DelayedEvaluator instance to be set for value regardless of restriction" do - value = Chef::DelayedEvaluator.new{ 'test' } - @vo.send(method,:test, value, {:kind_of => Numeric}) - end - - it "#{method} should raise an error when delayed evaluated attribute is not valid" do - value = Chef::DelayedEvaluator.new{ 'test' } - @vo.send(method,:test, value, {:kind_of => Numeric}) - expect do - @vo.send(method,:test, return_arg, {:kind_of => Numeric}) - end.to raise_error(Chef::Exceptions::ValidationFailed) - end - - it "#{method} should create DelayedEvaluator instance when #lazy is used" do - @vo.send(method,:delayed, @vo.lazy{ 'test' }, {}) - expect(@vo.instance_variable_get(:@delayed)).to be_a(Chef::DelayedEvaluator) - end - - it "#{method} should execute block on each call when DelayedEvaluator" do - value = 'fubar' - @vo.send(method,:test, @vo.lazy{ value }, {}) - expect(@vo.send(method,:test, return_arg, {})).to eq('fubar') - value = 'foobar' - expect(@vo.send(method,:test, return_arg, {})).to eq('foobar') - value = 'fauxbar' - expect(@vo.send(method,:test, return_arg, {})).to eq('fauxbar') - end - - it "#{method} should not evaluate non DelayedEvaluator instances" do - value = lambda{ 'test' } - @vo.send(method,:test, value, {}) - expect(@vo.send(method,:test, return_arg, {}).object_id).to eq(value.object_id) - expect(@vo.send(method,:test, return_arg, {})).to be_a(Proc) - end + it "should set and return a value, then return the same value" do + value = "meow" + expect(@vo.set_or_return(:test, value, {}).object_id).to eq(value.object_id) + expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id) end - test_set_or_return_method(:set_or_return) - test_set_or_return_method(:nillable_set_or_return) + it "should set and return a default value when the argument is nil, then return the same value" do + value = "meow" + expect(@vo.set_or_return(:test, nil, { :default => value }).object_id).to eq(value.object_id) + expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id) + end + + it "should raise an ArgumentError when argument is nil and required is true" do + expect { + @vo.set_or_return(:test, nil, { :required => true }) + }.to raise_error(ArgumentError) + end + + it "should not raise an error when argument is nil and required is false" do + expect { + @vo.set_or_return(:test, nil, { :required => false }) + }.not_to raise_error + end + + it "should set and return @name, then return @name for foo when argument is nil" do + value = "meow" + expect(@vo.set_or_return(:name, value, { }).object_id).to eq(value.object_id) + expect(@vo.set_or_return(:foo, nil, { :name_attribute => true }).object_id).to eq(value.object_id) + end - it "nillable_set_or_return supports nilling values" do - expect(@vo.nillable_set_or_return(:test, "meow", {})).to eq("meow") - expect(@vo.nillable_set_or_return(:test, TinyClass::NULL_ARG, {})).to eq("meow") - expect(@vo.nillable_set_or_return(:test, nil, {})).to be_nil - expect(@vo.nillable_set_or_return(:test, TinyClass::NULL_ARG, {})).to be_nil + it "should allow DelayedEvaluator instance to be set for value regardless of restriction" do + value = Chef::DelayedEvaluator.new{ 'test' } + @vo.set_or_return(:test, value, {:kind_of => Numeric}) end + + it "should raise an error when delayed evaluated attribute is not valid" do + value = Chef::DelayedEvaluator.new{ 'test' } + @vo.set_or_return(:test, value, {:kind_of => Numeric}) + expect do + @vo.set_or_return(:test, nil, {:kind_of => Numeric}) + end.to raise_error(Chef::Exceptions::ValidationFailed) + end + + it "should create DelayedEvaluator instance when #lazy is used" do + @vo.set_or_return(:delayed, @vo.lazy{ 'test' }, {}) + expect(@vo.instance_variable_get(:@delayed)).to be_a(Chef::DelayedEvaluator) + end + + it "should execute block on each call when DelayedEvaluator" do + value = 'fubar' + @vo.set_or_return(:test, @vo.lazy{ value }, {}) + expect(@vo.set_or_return(:test, nil, {})).to eq('fubar') + value = 'foobar' + expect(@vo.set_or_return(:test, nil, {})).to eq('foobar') + value = 'fauxbar' + expect(@vo.set_or_return(:test, nil, {})).to eq('fauxbar') + end + + it "should not evaluate non DelayedEvaluator instances" do + value = lambda{ 'test' } + @vo.set_or_return(:test, value, {}) + expect(@vo.set_or_return(:test, nil, {}).object_id).to eq(value.object_id) + expect(@vo.set_or_return(:test, nil, {})).to be_a(Proc) + end + end diff --git a/spec/unit/resource/deploy_spec.rb b/spec/unit/resource/deploy_spec.rb index 07f5f973c0..0403a7ba6b 100644 --- a/spec/unit/resource/deploy_spec.rb +++ b/spec/unit/resource/deploy_spec.rb @@ -30,35 +30,12 @@ describe Chef::Resource::Deploy do class << self - - def resource_has_a_hash_attribute(attr_name) - it "has a Hash attribute for #{attr_name.to_s}" do - @resource.send(attr_name, {foo: "bar"}) - expect(@resource.send(attr_name)).to eql({foo: "bar"}) - expect {@resource.send(attr_name, 8675309)}.to raise_error(ArgumentError) - end - - it "the Hash attribute for #{attr_name.to_s} is nillable" do - @resource.send(attr_name, {foo: "bar"}) - expect(@resource.send(attr_name)).to eql({foo: "bar"}) - @resource.send(attr_name, nil) - expect(@resource.send(attr_name)).to eql(nil) - end - end - def resource_has_a_string_attribute(attr_name) it "has a String attribute for #{attr_name.to_s}" do @resource.send(attr_name, "this is a string") expect(@resource.send(attr_name)).to eql("this is a string") expect {@resource.send(attr_name, 8675309)}.to raise_error(ArgumentError) end - - it "the String attribute for #{attr_name.to_s} is nillable" do - @resource.send(attr_name, "this is a string") - expect(@resource.send(attr_name)).to eql("this is a string") - @resource.send(attr_name, nil) - expect(@resource.send(attr_name)).to eql(nil) - end end def resource_has_a_boolean_attribute(attr_name, opts={:defaults_to=>false}) @@ -212,10 +189,6 @@ describe Chef::Resource::Deploy do expect(@resource.symlink_before_migrate).to eq({"wtf?" => "wtf is going on"}) end - resource_has_a_hash_attribute :symlink_before_migrate - resource_has_a_hash_attribute :symlinks - resource_has_a_hash_attribute :additional_remotes - resource_has_a_callback_attribute :before_migrate resource_has_a_callback_attribute :before_symlink resource_has_a_callback_attribute :before_restart |