From d866ce187d45897acb9b099e7a4d77a2aadced8d Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 10 Jun 2011 10:32:30 -0700 Subject: Cleanup indentation, comment, and unused code The Great Reindentation of '10 left certain structures indented incorrectly; this addresses some of these instances. The comment about loading all providers incorrectly stated that we're trying to figure out the type, when we're actually trying to figure out the provider. There was an unused variable initialization that was introduced in 2b14f627, which was reverting c19835c, 9290cc8, and ffb4c2d. Paired-with: Jacob Helwig --- lib/puppet/metatype/manager.rb | 26 ++++++-------------------- lib/puppet/type.rb | 18 ++++++++---------- 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/lib/puppet/metatype/manager.rb b/lib/puppet/metatype/manager.rb index 12cbf645c..3a2f77b6f 100644 --- a/lib/puppet/metatype/manager.rb +++ b/lib/puppet/metatype/manager.rb @@ -61,10 +61,9 @@ module Manager # Then create the class. - klass = genclass( - name, + klass = genclass( + name, :parent => (parent || Puppet::Type), - :overwrite => true, :hash => @types, :attributes => options, @@ -87,13 +86,9 @@ module Manager # Now set up autoload any providers that might exist for this type. - klass.providerloader = Puppet::Util::Autoload.new( - klass, - - "puppet/provider/#{klass.name.to_s}" - ) + klass.providerloader = Puppet::Util::Autoload.new(klass, "puppet/provider/#{klass.name.to_s}") - # We have to load everything so that we can figure out the default type. + # We have to load everything so that we can figure out the default provider. klass.providerloader.loadall klass @@ -103,11 +98,7 @@ module Manager def rmtype(name) # Then create the class. - klass = rmclass( - name, - - :hash => @types - ) + klass = rmclass(name, :hash => @types) singleton_class.send(:remove_method, "new#{name}") if respond_to?("new#{name}") end @@ -132,12 +123,7 @@ module Manager # Create a loader for Puppet types. def typeloader unless defined?(@typeloader) - - @typeloader = Puppet::Util::Autoload.new( - self, - - "puppet/type", :wrap => false - ) + @typeloader = Puppet::Util::Autoload.new(self, "puppet/type", :wrap => false) end @typeloader diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index d24cc8554..58673462a 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -1442,15 +1442,14 @@ class Type self.providify - provider = genclass( - name, - :parent => parent, - :hash => provider_hash, - :prefix => "Provider", - :block => block, - :include => feature_module, - :extend => feature_module, - + provider = genclass( + name, + :parent => parent, + :hash => provider_hash, + :prefix => "Provider", + :block => block, + :include => feature_module, + :extend => feature_module, :attributes => options ) @@ -1581,7 +1580,6 @@ class Type # Collect the current prereqs list.each { |dep| - obj = nil # Support them passing objects directly, to save some effort. unless dep.is_a? Puppet::Type # Skip autorequires that we aren't managing -- cgit From 413b136671232a8a0a9e27c18c1b6547241276e7 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 10 Jun 2011 10:48:36 -0700 Subject: (#4416) Always remove old provider before recreating it In the case where provider class evaluation failed midway, the provider class would be created but not registered. Thus, when checking whether it should be removed, it wasn't found, and wasn't removed. This caused it to then fail to be recreated, because it collided with the existing class. Now we don't bother checking whether the provider is registered before we remove it, since rmclass has the appropriate checks to do the unregistration, and class removal safely. Removing a provider class that has been created but not registered should not be a problem since the only time this can happen is when the class is unusable because of parsing or other fatal errors in the provider itself. Paired-with: Jacob Helwig --- lib/puppet/type.rb | 19 ++++----------- spec/unit/type_spec.rb | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 14 deletions(-) diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 58673462a..1933097da 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -1416,9 +1416,8 @@ class Type def self.provide(name, options = {}, &block) name = Puppet::Util.symbolize(name) - if obj = provider_hash[name] + if unprovide(name) Puppet.debug "Reloading #{name} #{self.name} provider" - unprovide(name) end parent = if pname = options[:parent] @@ -1441,7 +1440,6 @@ class Type self.providify - provider = genclass( name, :parent => parent, @@ -1509,18 +1507,11 @@ class Type end def self.unprovide(name) - if provider_hash.has_key? name - - rmclass( - name, - :hash => provider_hash, - - :prefix => "Provider" - ) - if @defaultprovider and @defaultprovider.name == name - @defaultprovider = nil - end + if @defaultprovider and @defaultprovider.name == name + @defaultprovider = nil end + + rmclass(name, :hash => provider_hash, :prefix => "Provider") end # Return an array of all of the suitable providers. diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb index 9b1f20500..442c759e7 100755 --- a/spec/unit/type_spec.rb +++ b/spec/unit/type_spec.rb @@ -165,6 +165,72 @@ describe Puppet::Type do end end + describe "when creating a provider" do + before :each do + @type = Puppet::Type.newtype(:provider_test_type) + end + + after :each do + @type.provider_hash.clear + end + + it "should create a subclass of Puppet::Provider for the provider" do + provider = @type.provide(:test_provider) + + provider.ancestors.should include(Puppet::Provider) + end + + it "should use a parent class if specified" do + parent_provider = @type.provide(:parent_provider) + child_provider = @type.provide(:child_provider, :parent => parent_provider) + + child_provider.ancestors.should include(parent_provider) + end + + it "should use a parent class if specified by name" do + parent_provider = @type.provide(:parent_provider) + child_provider = @type.provide(:child_provider, :parent => :parent_provider) + + child_provider.ancestors.should include(parent_provider) + end + + it "should raise an error when the parent class can't be found" do + expect { + @type.provide(:child_provider, :parent => :parent_provider) + }.to raise_error(Puppet::DevError, /Could not find parent provider.+parent_provider/) + end + + it "should ensure its type has a 'provider' parameter" do + @type.provide(:test_provider) + + @type.parameters.should include(:provider) + end + + it "should remove a previously registered provider with the same name" do + old_provider = @type.provide(:test_provider) + new_provider = @type.provide(:test_provider) + + old_provider.should_not equal(new_provider) + end + + it "should register itself as a provider for the type" do + provider = @type.provide(:test_provider) + + provider.should == @type.provider(:test_provider) + end + + it "should create a provider when a provider with the same name previously failed" do + @type.provide(:test_provider) do + raise "failed to create this provider" + end rescue nil + + provider = @type.provide(:test_provider) + + provider.ancestors.should include(Puppet::Provider) + provider.should == @type.provider(:test_provider) + end + end + describe "when choosing a default provider" do it "should choose the provider with the highest specificity" do # Make a fake type -- cgit From caca469976cc8b5ff6c7f68d7324eecd35399176 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 10 Jun 2011 10:58:19 -0700 Subject: (#4416) Ensure types are providified after reloading Previously, the 'provider' parameter for a type was only added when creating a provider for that type. This would cause a type to forget about its 'provider' parameter when only the type was reloaded. This was manifesting itself in pluginsync, when a provider plugin would be loaded before its type, causing the type to be autoloaded. The type plugin would then be loaded again by the plugin handler. Because the type => provider information is stored separately from the type, the providers don't need to be reloaded, and thus don't recreate the type's 'provider' parameter. Now we always "providify" the type (add its 'provider' parameter) upon creation, after trying to load its providers, if any providers are present. Paired-with: Jacob Helwig --- lib/puppet/metatype/manager.rb | 1 + spec/integration/type_spec.rb | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/puppet/metatype/manager.rb b/lib/puppet/metatype/manager.rb index 3a2f77b6f..597a89f31 100644 --- a/lib/puppet/metatype/manager.rb +++ b/lib/puppet/metatype/manager.rb @@ -90,6 +90,7 @@ module Manager # We have to load everything so that we can figure out the default provider. klass.providerloader.loadall + klass.providify unless klass.providers.empty? klass end diff --git a/spec/integration/type_spec.rb b/spec/integration/type_spec.rb index 957dfe344..74e268a3c 100755 --- a/spec/integration/type_spec.rb +++ b/spec/integration/type_spec.rb @@ -19,4 +19,15 @@ describe Puppet::Type do type.provider(:myprovider).should equal(provider) end + + it "should not lose its provider parameter when it is reloaded" do + type = Puppet::Type.newtype(:reload_test_type) + + provider = type.provide(:test_provider) + + # reload it + type = Puppet::Type.newtype(:reload_test_type) + + type.parameters.should include(:provider) + end end -- cgit From 381fa409207a1b0d26e7e55ea8cbe45a7e132fdf Mon Sep 17 00:00:00 2001 From: Dominic Maraglia Date: Fri, 10 Jun 2011 14:27:39 -0700 Subject: (#6418) Make test 64118 more portable Test used a bash command negation to negate the a grep: '! grep ensure.*directory /var/opt/lib/pe-puppet/state/state.yaml' which only works on newer bash shells only. Removed the shell negation and use the harnesses ":acceptable_exit_codes" parameter. --- .../tests/ticket_6418_file_recursion_and_audit.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/acceptance/tests/ticket_6418_file_recursion_and_audit.rb b/acceptance/tests/ticket_6418_file_recursion_and_audit.rb index f0a55d029..b21e57ddb 100644 --- a/acceptance/tests/ticket_6418_file_recursion_and_audit.rb +++ b/acceptance/tests/ticket_6418_file_recursion_and_audit.rb @@ -2,11 +2,9 @@ # # AffectedVersion: 2.6.0-2.6.5 # FixedVersion: -# test_name "#6418: file recursion and audit" -on agents, "rm -f /var/lib/puppet/state/state.yaml " manifest = %q{ file { "/tmp/6418": ensure => directory } file { "/tmp/6418/dir": ensure => directory} @@ -17,6 +15,19 @@ manifest = %q{ File["/tmp/6418"] -> File["/tmp/6418/dir"] -> File["/tmp/6418/dir/dir"] -> File["/tmp/6418/dir/dir/dir"] -> File["/tmp/6418-copy"] } +step "Query agent for statefile" +agent=agents.first +on agent, puppet_agent('--configprint statefile') +statefile=stdout.chomp + +step "Remove the statefile on all Agents" +on agents, "rm -f #{statefile}" + step "Apply the manifest" apply_manifest_on agents, manifest -on agents, "! grep ensure.*directory /var/lib/puppet/state/state.yaml" + + +step "Verify corecct file recursion and audit state" +agents.each do |agent| + on(agent, "grep ensure.*directory #{statefile}", :acceptable_exit_codes => [ 1 ]) +end -- cgit