polishing provider_resolver

* makes provides? more correct for linux services
* makes supports? more targetted for linux services
* remove provider_resolver from the run_context
* fix timestamped deploy spec
* add more specs

16 files changed, 216 insertions, 72 deletions

diff --git a/lib/chef/provider/service/aixinit.rb b/lib/chef/provider/service/aixinit.rb
index ab4b8e5406..19beac79f0 100644
--- a/lib/chef/provider/service/aixinit.rb
+++ b/lib/chef/provider/service/aixinit.rb
@@ -114,4 +114,4 @@ class Chef
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/lib/chef/provider/service/arch.rb b/lib/chef/provider/service/arch.rb
index 888fb3fdf5..e7fbcc820c 100644
--- a/lib/chef/provider/service/arch.rb
+++ b/lib/chef/provider/service/arch.rb
@@ -23,7 +23,7 @@ class Chef::Provider::Service::Arch < Chef::Provider::Service::Init
   provides :service, platform_family: "arch"
 
   def self.supports?(resource, action)
-    ::File.exist?("/etc/rc.d/#{resource.service_name}")
+    Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:etc_rcd)
   end
 
   def initialize(new_resource, run_context)
diff --git a/lib/chef/provider/service/debian.rb b/lib/chef/provider/service/debian.rb
index 25b1960b26..01505924cb 100644
--- a/lib/chef/provider/service/debian.rb
+++ b/lib/chef/provider/service/debian.rb
@@ -27,8 +27,12 @@ class Chef
 
         provides :service, platform_family: "debian"
 
+        def self.provides?(node, resource)
+          super && Chef::Platform::ServiceHelpers.service_resource_providers.include?(:debian)
+        end
+
         def self.supports?(resource, action)
-          Chef::Platform::ServiceHelpers.service_resource_providers.include?(:debian)
+          Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:initd)
         end
 
         def load_current_resource
diff --git a/lib/chef/provider/service/init.rb b/lib/chef/provider/service/init.rb
index ab40a720f6..0a219a69e1 100644
--- a/lib/chef/provider/service/init.rb
+++ b/lib/chef/provider/service/init.rb
@@ -28,6 +28,10 @@ class Chef
 
         provides :service, os: "!windows"
 
+        def self.supports?(resource, action)
+          Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:initd)
+        end
+
         def initialize(new_resource, run_context)
           super
           @init_command = "/etc/init.d/#{@new_resource.service_name}"
diff --git a/lib/chef/provider/service/insserv.rb b/lib/chef/provider/service/insserv.rb
index df5a162a45..37901b7802 100644
--- a/lib/chef/provider/service/insserv.rb
+++ b/lib/chef/provider/service/insserv.rb
@@ -26,8 +26,12 @@ class Chef
 
         provides :service, os: "linux"
 
+        def self.provides?(node, resource)
+          super && Chef::Platform::ServiceHelpers.service_resource_providers.include?(:insserv)
+        end
+
         def self.supports?(resource, action)
-          Chef::Platform::ServiceHelpers.service_resource_providers.include?(:insserv)
+          Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:upstart)
         end
 
         def load_current_resource
diff --git a/lib/chef/provider/service/invokercd.rb b/lib/chef/provider/service/invokercd.rb
index c7472211bc..5ff24e0dbb 100644
--- a/lib/chef/provider/service/invokercd.rb
+++ b/lib/chef/provider/service/invokercd.rb
@@ -25,8 +25,12 @@ class Chef
 
         provides :service, platform_family: "debian"
 
+        def self.provides?(node, resource)
+          super && Chef::Platform::ServiceHelpers.service_resource_providers.include?(:invokercd)
+        end
+
         def self.supports?(resource, action)
-          Chef::Platform::ServiceHelpers.service_resource_providers.include?(:invokerc)
+          Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:initd)
         end
 
         def initialize(new_resource, run_context)
diff --git a/lib/chef/provider/service/redhat.rb b/lib/chef/provider/service/redhat.rb
index 90744ae268..850953125e 100644
--- a/lib/chef/provider/service/redhat.rb
+++ b/lib/chef/provider/service/redhat.rb
@@ -28,8 +28,12 @@ class Chef
 
         provides :service, platform_family: [ "rhel", "fedora", "suse" ]
 
+        def self.provides?(node, resource)
+          super && Chef::Platform::ServiceHelpers.service_resource_providers.include?(:redhat)
+        end
+
         def self.supports?(resource, action)
-          Chef::Platform::ServiceHelpers.service_resource_providers.include?(:redhat)
+          Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:initd)
         end
 
         def initialize(new_resource, run_context)
diff --git a/lib/chef/provider/service/systemd.rb b/lib/chef/provider/service/systemd.rb
index 717ff7dc5c..9085ffde2e 100644
--- a/lib/chef/provider/service/systemd.rb
+++ b/lib/chef/provider/service/systemd.rb
@@ -28,8 +28,12 @@ class Chef::Provider::Service::Systemd < Chef::Provider::Service::Simple
 
   attr_accessor :status_check_success
 
+  def self.provides?(node, resource)
+    super && Chef::Platform::ServiceHelpers.service_resource_providers.include?(:systemd)
+  end
+
   def self.supports?(resource, action)
-    Chef::Platform::ServiceHelpers.service_resource_providers.include?(:systemd)
+    Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:systemd)
   end
 
   def load_current_resource
diff --git a/lib/chef/provider/service/upstart.rb b/lib/chef/provider/service/upstart.rb
index 41bd850d6a..3a3ddb2385 100644
--- a/lib/chef/provider/service/upstart.rb
+++ b/lib/chef/provider/service/upstart.rb
@@ -29,9 +29,12 @@ class Chef
 
         provides :service, os: "linux"
 
+        def self.provides?(node, resource)
+          super && Chef::Platform::ServiceHelpers.service_resource_providers.include?(:upstart)
+        end
+
         def self.supports?(resource, action)
-          Chef::Platform::ServiceHelpers.service_resource_providers.include?(:upstart) &&
-            Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:upstart)
+          Chef::Platform::ServiceHelpers.config_for_service(resource.service_name).include?(:upstart)
         end
 
         # Upstart does more than start or stop a service, creating multiple 'states' [1] that a service can be in.
diff --git a/lib/chef/provider_resolver.rb b/lib/chef/provider_resolver.rb
index 0c3a798d3c..247102f191 100644
--- a/lib/chef/provider_resolver.rb
+++ b/lib/chef/provider_resolver.rb
@@ -23,22 +23,42 @@ class Chef
   class ProviderResolver
 
     attr_reader :node
+    attr_reader :resource
+    attr_reader :action
 
-    def initialize(node)
+    def initialize(node, resource, action)
       @node = node
+      @resource = resource
+      @action = action
     end
 
     # return a deterministically sorted list of Chef::Provider subclasses
     def providers
-      Chef::Provider.descendants.sort {|a,b| a.to_s <=> b.to_s }
+      @providers ||= Chef::Provider.descendants.sort {|a,b| a.to_s <=> b.to_s }
     end
 
-    def resolve(resource, action)
+    def resolve
       maybe_explicit_provider(resource) ||
         maybe_dynamic_provider_resolution(resource, action) ||
         maybe_chef_platform_lookup(resource)
     end
 
+    # this cut looks at if the provider can handle the resource type on the node
+    def enabled_handlers
+      @enabled_handlers ||=
+        providers.select do |klass|
+          klass.provides?(node, resource)
+        end
+    end
+
+    # this cut looks at if the provider can handle the specific resource and action
+    def supported_handlers
+      @supported_handlers ||=
+        enabled_handlers.select do |klass|
+          klass.supports?(resource, action)
+        end
+    end
+
     private
 
     # if resource.provider is set, just return one of those objects
@@ -49,20 +69,9 @@ class Chef
 
     # try dynamically finding a provider based on querying the providers to see what they support
     def maybe_dynamic_provider_resolution(resource, action)
-      # this cut only depends on the node value and is going to be static for all nodes
-      # will contain all providers that could possibly support a resource on a node
-      enabled_handlers = providers.select do |klass|
-        klass.provides?(node, resource)
-      end
-
       # log this so we know what providers will work for the generic resource on the node (early cut)
       Chef::Log.debug "providers for generic #{resource.resource_name} resource enabled on node include: #{enabled_handlers}"
 
-      # ask all the enabled providers if they can actually support the resource
-      supported_handlers = enabled_handlers.select do |klass|
-        klass.supports?(resource, action)
-      end
-
       # what providers were excluded by machine state (late cut)
       Chef::Log.debug "providers that refused resource #{resource} were: #{enabled_handlers - supported_handlers}"
       Chef::Log.debug "providers that support resource #{resource} include: #{supported_handlers}"
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index c38f36aa72..8d964da66d 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -29,6 +29,7 @@ require 'chef/resource/conditional_action_not_nothing'
 require 'chef/resource_collection'
 require 'chef/node_map'
 require 'chef/node'
+require 'chef/provider_resolver'
 require 'chef/platform'
 
 require 'chef/mixin/deprecation'
@@ -679,7 +680,7 @@ F
     end
 
     def provider_for_action(action)
-      provider = run_context.provider_resolver.resolve(self, action).new(self, run_context)
+      provider = Chef::ProviderResolver.new(node, self, action).resolve.new(self, run_context)
       provider.action = action
       provider
     end
diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb
index 18d353ac61..1a2d7ba3a3 100644
--- a/lib/chef/run_context.rb
+++ b/lib/chef/run_context.rb
@@ -18,7 +18,6 @@
 # limitations under the License.
 
 require 'chef/resource_collection'
-require 'chef/provider_resolver'
 require 'chef/cookbook_version'
 require 'chef/node'
 require 'chef/role'
@@ -51,9 +50,6 @@ class Chef
     # recipes, which is triggered by #load. (See also: CookbookCompiler)
     attr_accessor :resource_collection
 
-    # Chef::ProviderResolver for this run
-    attr_accessor :provider_resolver
-
     # A Hash containing the immediate notifications triggered by resources
     # during the converge phase of the chef run.
     attr_accessor :immediate_notification_collection
@@ -87,7 +83,6 @@ class Chef
 
       @node.run_context = self
       @cookbook_compiler = nil
-      @provider_resolver = Chef::ProviderResolver.new(@node)
     end
 
     # Triggers the compile phase of the chef run. Implemented by
diff --git a/spec/support/shared/unit/resource/static_provider_resolution.rb b/spec/support/shared/unit/resource/static_provider_resolution.rb
index 147852598a..2bc4c70d95 100644
--- a/spec/support/shared/unit/resource/static_provider_resolution.rb
+++ b/spec/support/shared/unit/resource/static_provider_resolution.rb
@@ -43,12 +43,7 @@ def static_provider_resolution(opts={})
       node
     }
     let(:events) { Chef::EventDispatch::Dispatcher.new }
-    let(:provider_resolver) { Chef::ProviderResolver.new(node) }
-    let(:run_context) {
-      run_context = Chef::RunContext.new(node, {}, events)
-      run_context.provider_resolver = provider_resolver
-      run_context
-    }
+    let(:run_context) { Chef::RunContext.new(node, {}, events) }
     let(:resource) { resource_class.new("foo", run_context) }
 
     it "should return a #{resource_class}" do
diff --git a/spec/unit/provider_resolver_spec.rb b/spec/unit/provider_resolver_spec.rb
index 927cca4f58..5be6ebe661 100644
--- a/spec/unit/provider_resolver_spec.rb
+++ b/spec/unit/provider_resolver_spec.rb
@@ -33,11 +33,11 @@ describe Chef::ProviderResolver do
     node
   end
 
-  let(:provider_resolver) { Chef::ProviderResolver.new(node) }
+  let(:provider_resolver) { Chef::ProviderResolver.new(node, resource, action) }
 
   let(:action) { :start }
 
-  let(:resolved_provider) { provider_resolver.resolve(resource, action) }
+  let(:resolved_provider) { provider_resolver.resolve }
 
   let(:provider) { nil }
 
@@ -63,9 +63,9 @@ describe Chef::ProviderResolver do
       allow(resource).to receive(:service_name).and_return("ntp")
     end
 
-    shared_examples_for "a debian platform with upstart and update-rc.d" do
+    shared_examples_for "an ubuntu platform with upstart, update-rc.d and systemd" do
       before do
-        stub_service_providers(:debian, :invokercd, :upstart)
+        stub_service_providers(:debian, :invokercd, :upstart, :systemd)
       end
 
       it "when only the SysV init script exists, it returns a Service::Debian provider" do
@@ -93,6 +93,136 @@ describe Chef::ProviderResolver do
       end
     end
 
+    shared_examples_for "an ubuntu platform with upstart and update-rc.d" do
+      before do
+        stub_service_providers(:debian, :invokercd, :upstart)
+      end
+
+      # needs to be handled by the highest priority init.d handler
+      context "when only the SysV init script exists" do
+        before do
+          allow(Chef::Platform::ServiceHelpers).to receive(:config_for_service).with("ntp")
+            .and_return( [ :initd ] )
+        end
+
+        it "enables init, invokercd, debian and upstart providers" do
+          expect(provider_resolver.enabled_handlers).to include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+            Chef::Provider::Service::Upstart,
+          )
+        end
+
+        it "supports all the enabled handlers except for upstart" do
+          expect(provider_resolver.supported_handlers).to include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+          )
+          expect(provider_resolver.supported_handlers).to_not include(
+            Chef::Provider::Service::Upstart,
+          )
+        end
+
+        it "returns a Service::Debian provider" do
+          expect(resolved_provider).to eql(Chef::Provider::Service::Debian)
+        end
+      end
+
+      # on ubuntu this must be handled by upstart, the init script will exit 1 and fail
+      context "when both SysV and Upstart scripts exist" do
+        before do
+          allow(Chef::Platform::ServiceHelpers).to receive(:config_for_service).with("ntp")
+            .and_return( [ :initd, :upstart ] )
+        end
+
+        it "enables init, invokercd, debian and upstart providers" do
+          expect(provider_resolver.enabled_handlers).to include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+            Chef::Provider::Service::Upstart,
+          )
+        end
+
+        it "supports all the enabled handlers" do
+          expect(provider_resolver.supported_handlers).to include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+            Chef::Provider::Service::Upstart,
+          )
+        end
+
+        it "returns a Service::Upstart provider" do
+          expect(resolved_provider).to eql(Chef::Provider::Service::Upstart)
+        end
+      end
+
+      # this case is a pure-upstart script which is easy
+      context "when only the Upstart script exists" do
+        before do
+          allow(Chef::Platform::ServiceHelpers).to receive(:config_for_service).with("ntp")
+            .and_return( [ :upstart ] )
+        end
+
+        it "enables init, invokercd, debian and upstart providers" do
+          expect(provider_resolver.enabled_handlers).to include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+            Chef::Provider::Service::Upstart,
+          )
+        end
+
+        it "supports only the upstart handler" do
+          expect(provider_resolver.supported_handlers).to include(
+            Chef::Provider::Service::Upstart,
+          )
+          expect(provider_resolver.supported_handlers).to_not include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+          )
+        end
+
+        it "returns a Service::Upstart provider" do
+          expect(resolved_provider).to eql(Chef::Provider::Service::Upstart)
+        end
+      end
+
+      # this case is important to get correct for why-run when no config is setup
+      context "when both do not exist" do
+        before do
+          allow(Chef::Platform::ServiceHelpers).to receive(:config_for_service).with("ntp")
+            .and_return( [ ] )
+        end
+
+        it "enables init, invokercd, debian and upstart providers" do
+          expect(provider_resolver.enabled_handlers).to include(
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+            Chef::Provider::Service::Upstart,
+          )
+        end
+
+        it "no providers claim to support the resource" do
+          expect(provider_resolver.supported_handlers).to_not include(
+            Chef::Provider::Service::Upstart,
+            Chef::Provider::Service::Debian,
+            Chef::Provider::Service::Init,
+            Chef::Provider::Service::Invokercd,
+          )
+        end
+
+        it "returns a Debian Provider" do
+          expect(resolved_provider).to eql(Chef::Provider::Service::Upstart)
+        end
+      end
+    end
+
     shared_examples_for "a debian platform using the insserv provider" do
       context "with a default install" do
         before do
@@ -137,7 +267,13 @@ describe Chef::ProviderResolver do
       end
     end
 
-    describe "on Linux" do
+    describe "on Ubuntu 14.10" do
+      let(:os) { "linux" }
+      let(:platform) { "ubuntu" }
+      let(:platform_family) { "debian" }
+      let(:platform_version) { "14.04" }
+
+      it_behaves_like "an ubuntu platform with upstart, update-rc.d and systemd"
     end
 
     describe "on Ubuntu 14.04" do
@@ -146,7 +282,7 @@ describe Chef::ProviderResolver do
       let(:platform_family) { "debian" }
       let(:platform_version) { "14.04" }
 
-      it_behaves_like "a debian platform with upstart and update-rc.d"
+      it_behaves_like "an ubuntu platform with upstart and update-rc.d"
     end
 
     describe "on Ubuntu 10.04" do
@@ -155,7 +291,7 @@ describe Chef::ProviderResolver do
       let(:platform_family) { "debian" }
       let(:platform_version) { "10.04" }
 
-      it_behaves_like "a debian platform with upstart and update-rc.d"
+      it_behaves_like "an ubuntu platform with upstart and update-rc.d"
     end
 
     # old debian uses the Debian provider (does not have insserv or upstart, or update-rc.d???)
diff --git a/spec/unit/resource/timestamped_deploy_spec.rb b/spec/unit/resource/timestamped_deploy_spec.rb
index 2af9d8bb0f..eca6c570d4 100644
--- a/spec/unit/resource/timestamped_deploy_spec.rb
+++ b/spec/unit/resource/timestamped_deploy_spec.rb
@@ -20,35 +20,14 @@ require 'spec_helper'
 
 describe Chef::Resource::TimestampedDeploy, "initialize" do
 
-  let(:node) {
-    node = Chef::Node.new
-    node.automatic_attrs[:os] = 'linux'
-    node.automatic_attrs[:platform_family] = 'rhel'
-    node
-  }
-  let(:events) { Chef::EventDispatch::Dispatcher.new }
-  let(:provider_resolver) { Chef::ProviderResolver.new(node) }
-  let(:run_context) {
-    run_context = Chef::RunContext.new(node, {}, events)
-    run_context.provider_resolver = provider_resolver
-    run_context
-  }
-  let(:resource) { Chef::Resource::TimestampedDeploy.new("stuff", run_context) }
+  static_provider_resolution(
+    resource: Chef::Resource::TimestampedDeploy,
+    provider: Chef::Provider::Deploy::Timestamped,
+    name: :deploy,
+    action: :deploy,
+    os: 'linux',
+    platform_family: 'rhel',
+  )
 
-  it "should return a Chef::Resource::TimestampedDeploy" do
-    expect(resource).to be_a_kind_of(Chef::Resource::TimestampedDeploy)
-  end
-
-  it "should set the resource_name to :timestamped_deploy" do
-    expect(resource.resource_name).to eql(:deploy)
-  end
-
-  it "should leave the provider nil" do
-    expect(resource.provider).to eql(nil)
-  end
-
-  it "should resolve to a Chef::Provider::Deploy::Timestamped" do
-    expect(resource.provider_for_action(:install)).to be_a(Chef::Provider::Deploy::Timestamped)
-  end
 end
 
diff --git a/spec/unit/runner_spec.rb b/spec/unit/runner_spec.rb
index 9bd4199418..b30f818da1 100644
--- a/spec/unit/runner_spec.rb
+++ b/spec/unit/runner_spec.rb
@@ -99,13 +99,15 @@ describe Chef::Runner do
   end
 
   context "when we fall through to old Chef::Platform resolution" do
+    let(:provider_resolver) { Chef::ProviderResolver.new(node, first_resource, nil) }
     before do
       # set up old Chef::Platform resolution instead of provider_resolver
       Chef::Platform.set(
         :resource => :cat,
         :provider => Chef::Provider::SnakeOil
       )
-      allow(run_context.provider_resolver).to receive(:maybe_dynamic_provider_resolution).with(first_resource, anything()).and_return(nil)
+      allow(Chef::ProviderResolver).to receive(:new).and_return(provider_resolver)
+      allow(provider_resolver).to receive(:maybe_dynamic_provider_resolution).with(first_resource, anything()).and_return(nil)
     end
 
     it "should use the platform provider if it has one" do