Fixing #571 -- provider suitability is now checked at resource

evaluation time, rather than resource instantiation time.  This
means that you don't catch your "errors" as early, but it also
means you should be able to realistically configure a whole host
in one run.

4 files changed, 34 insertions, 14 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a2d87cff0..387ae04e7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,9 @@
+    Fixing #571 -- provider suitability is now checked at resource
+    evaluation time, rather than resource instantiation time.  This
+    means that you don't catch your "errors" as early, but it also
+    means you should be able to realistically configure a whole host
+    in one run.
+
     Moved the configuration of the Node cache to the puppetmasterd
     executable, since it otherwise causes caches to be used in all
     cases, which we don't want (e.g., bin/puppet was using them).
diff --git a/lib/puppet/metatype/evaluation.rb b/lib/puppet/metatype/evaluation.rb
index 8d435333f..ff1eddb55 100644
--- a/lib/puppet/metatype/evaluation.rb
+++ b/lib/puppet/metatype/evaluation.rb
@@ -4,6 +4,11 @@ class Puppet::Type
     # This returns any changes resulting from testing, thus 'collect' rather
     # than 'each'.
     def evaluate
+        if self.provider.is_a?(Puppet::Provider)
+            unless provider.class.suitable?
+                raise Puppet::Error, "Provider %s is not functional on this platform" % provider.class.name
+            end
+        end
         #Puppet.err "Evaluating %s" % self.path.join(":")
         unless defined? @evalcount
             self.err "No evalcount defined on '%s' of type '%s'" %
diff --git a/lib/puppet/metatype/providers.rb b/lib/puppet/metatype/providers.rb
index c302d9928..6308f7e54 100644
--- a/lib/puppet/metatype/providers.rb
+++ b/lib/puppet/metatype/providers.rb
@@ -188,15 +188,8 @@ class Puppet::Type
                     provider_class = provider_class.class.name
                 end
 
-                if provider = @resource.class.provider(provider_class)
-                    unless provider.suitable?
-                        raise ArgumentError,
-                            "Provider '%s' is not functional on this platform" %
-                            [provider_class]
-                    end
-                else
-                    raise ArgumentError, "Invalid %s provider '%s'" %
-                        [@resource.class.name, provider_class]
+                unless provider = @resource.class.provider(provider_class)
+                    raise ArgumentError, "Invalid %s provider '%s'" % [@resource.class.name, provider_class]
                 end
             end
 
diff --git a/test/ral/manager/provider.rb b/test/ral/manager/provider.rb
index bb7a21485..89aa49b9e 100755
--- a/test/ral/manager/provider.rb
+++ b/test/ral/manager/provider.rb
@@ -73,11 +73,6 @@ class TestTypeProviders < Test::Unit::TestCase
             confine :exists => "/no/such/file"
         end
 
-        inst = provider.new(:name => "bar")
-        assert_raise(Puppet::Error, "Did not fail on unsuitable provider instance") do
-            resource = @type.create :name => "bar", :provider => inst
-        end
-
         # And make sure the provider must be a valid provider type for this resource
         pkgprov = Puppet::Type.type(:package).create(:name => "yayness").provider
         assert(provider, "did not get package provider")
@@ -87,5 +82,26 @@ class TestTypeProviders < Test::Unit::TestCase
         end
 
     end
+
+    # #571 -- so we can cause a provider to become suitable within
+    # a run.
+    def test_unsuitable_providers_should_not_fail_at_initialization
+        Puppet::Type.type(:user).provider(:useradd).stubs(:suitable?).returns false
+
+        assert_nothing_raised("Unsuitable providers failed at initialization") do
+            Puppet::Type.type(:user).create :name => "luke", :ensure => :present, :provider => :useradd
+        end
+    end
+
+    # #571 -- so we can cause a provider to become suitable within
+    # a run.
+    def test_unsuitable_providers_should_fail_at_evaluation
+        Puppet::Type.type(:user).provider(:useradd).stubs(:suitable?).returns false
+
+        user = Puppet::Type.type(:user).create :name => "luke", :ensure => :present, :provider => :useradd
+        assert_raise(Puppet::Error, "Unsuitable provider did not fail at evaluation") do
+            user.evaluate
+        end
+    end
 end