Add enable/disable to MacOSX service provider

v11 version of #1267

5 files changed, 100 insertions, 25 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 78377d38e1..6eac1362e7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 * CHEF-5001: spec tests for multiple rollbacks
 * Added ohai7 'machinename' attribute as source of `node_name` information
 * CHEF-4773: add ruby-shadow support to Mac and FreeBSD distros
+* Service Provider for MacOSX now supports `enable` and `disable`
 
 ## Last Release: 11.10.0 (02/06/2014)
 
diff --git a/CONTRIBUTIONS.md b/CONTRIBUTIONS.md
index 3bf6a131d7..e09c96848e 100644
--- a/CONTRIBUTIONS.md
+++ b/CONTRIBUTIONS.md
@@ -7,4 +7,5 @@ Example Contribution:
 # Chef Client Contributions:
 
 * **jonlives**: Changed the order of recipe and cookbook name setting. Fixes CHEF-5052.
+* **jaymzh**: Service Provider for MacOSX now supports `enable` and `disable`
 
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 73791bd7ab..6c1c5bcb0f 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -9,5 +9,6 @@ Details about the thing that changed that needs to get included in the Release N
 # Chef Client Release Notes:
 
 * CHEF-4773: add ruby-shadow support to Mac and FreeBSD distros
+* Service Provider for MacOSX now supports `enable` and `disable`
 
 # Chef Client Breaking Changes:
diff --git a/lib/chef/provider/service/macosx.rb b/lib/chef/provider/service/macosx.rb
index 4f2de2ccbf..ca78c2eaee 100644
--- a/lib/chef/provider/service/macosx.rb
+++ b/lib/chef/provider/service/macosx.rb
@@ -17,6 +17,7 @@
 #
 
 require 'chef/provider/service'
+require 'rexml/document'
 
 class Chef
   class Provider
@@ -41,6 +42,7 @@ class Chef
           @current_resource.service_name(@new_resource.service_name)
           @plist_size = 0
           @plist = find_service_plist
+          @service_label = find_service_label
           set_service_status
 
           @current_resource
@@ -48,14 +50,6 @@ class Chef
 
         def define_resource_requirements
           #super
-          requirements.assert(:enable) do |a|
-            a.failure_message Chef::Exceptions::UnsupportedAction, "#{self.to_s} does not support :enable"
-          end
-
-          requirements.assert(:disable) do |a|
-            a.failure_message Chef::Exceptions::UnsupportedAction, "#{self.to_s} does not support :disable"
-          end
-
           requirements.assert(:reload) do |a|
             a.failure_message Chef::Exceptions::UnsupportedAction, "#{self.to_s} does not support :reload"
           end
@@ -66,6 +60,12 @@ class Chef
           end
 
           requirements.assert(:all_actions) do |a|
+            a.assertion { !@service_label.to_s.empty? }
+            a.failure_message Chef::Exceptions::Service,
+              "Could not find service's label in plist file '#{@plist}'!"
+          end
+
+          requirements.assert(:all_actions) do |a|
             a.assertion { @plist_size > 0 }
             # No failrue here in original code - so we also will not
             # fail. Instead warn that the service is potentially missing
@@ -74,7 +74,6 @@ class Chef
               @current_resource.running(false)
             end
           end
-
         end
 
         def start_service
@@ -111,19 +110,56 @@ class Chef
           end
         end
 
+        # On OS/X, enabling a service has the side-effect of starting it,
+        # and disabling a service has the side-effect of stopping it.
+        #
+        # This makes some sense on OS/X since launchctl is an "init"-style
+        # supervisor that will restart daemons that are crashing, etc.
+        def enable_service
+          if @current_resource.enabled
+            Chef::Log.debug("#{@new_resource} already enabled, not enabling")
+          else
+            shell_out!(
+              "launchctl load -w '#{@plist}'",
+              :user => @owner_uid, :group => @owner_gid
+            )
+          end
+        end
+
+        def disable_service
+          unless @current_resource.enabled
+            Chef::Log.debug("#{@new_resource} not enabled, not disabling")
+          else
+            shell_out!(
+              "launchctl unload -w '#{@plist}'",
+              :user => @owner_uid, :group => @owner_gid
+            )
+          end
+        end
 
         def set_service_status
-          return if @plist == nil
+          return if @plist == nil or @service_label.to_s.empty?
 
-          @current_resource.enabled(!@plist.nil?)
+          cmd = shell_out(
+            "launchctl list #{@service_label}",
+            :user => @owner_uid, :group => @owner_gid
+          )
+
+          if cmd.exitstatus == 0
+            @current_resource.enabled(true)
+          else
+            @current_resource.enabled(false)
+          end
 
           if @current_resource.enabled
             @owner_uid = ::File.stat(@plist).uid
             @owner_gid = ::File.stat(@plist).gid
 
-            shell_out!("launchctl list", :user => @owner_uid, :group => @owner_gid).stdout.each_line do |line|
+            shell_out!(
+              "launchctl list", :user => @owner_uid, :group => @owner_gid
+            ).stdout.each_line do |line|
               case line
-              when /(\d+|-)\s+(?:\d+|-)\s+(.*\.?)#{@current_resource.service_name}/
+              when /(\d+|-)\s+(?:\d+|-)\s+(.*\.?)#{@service_label}/
                 pid = $1
                 @current_resource.running(!pid.to_i.zero?)
               end
@@ -135,9 +171,27 @@ class Chef
 
       private
 
+        def find_service_label
+          # Most services have the same internal label as the name of the
+          # plist file. However, there is no rule saying that *has* to be
+          # the case, and some core services (notably, ssh) do not follow
+          # this rule.
+
+          # plist files can come in XML or Binary formats. this command
+          # will make sure we get XML every time.
+          plist_xml = shell_out!("plutil -convert xml1 -o - #{@plist}").stdout
+
+          plist_doc = REXML::Document.new(plist_xml)
+          plist_doc.elements[
+            "/plist/dict/key[text()='Label']/following::string[1]/text()"]
+        end
+
         def find_service_plist
           plists = PLIST_DIRS.inject([]) do |results, dir|
-            entries = Dir.glob("#{::File.expand_path(dir)}/*#{@current_resource.service_name}*.plist")
+            edir = ::File.expand_path(dir)
+            entries = Dir.glob(
+              "#{edir}/*#{@current_resource.service_name}*.plist"
+            )
             entries.any? ? results << entries : results
           end
           plists.flatten!
diff --git a/spec/unit/provider/service/macosx_spec.rb b/spec/unit/provider/service/macosx_spec.rb
index 65639f2084..5d224dbd15 100644
--- a/spec/unit/provider/service/macosx_spec.rb
+++ b/spec/unit/provider/service/macosx_spec.rb
@@ -46,14 +46,32 @@ describe Chef::Provider::Service::Macosx do
     let(:events) {Chef::EventDispatch::Dispatcher.new}
     let(:run_context) { Chef::RunContext.new(node, {}, events) }
     let(:provider) { described_class.new(new_resource, run_context) }
-    let(:stdout) { StringIO.new }
+    let(:launchctl_stdout) { StringIO.new }
+    let(:plutil_stdout) { String.new <<-XML }
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+  <key>Label</key>
+  <string>io.redis.redis-server</string>
+</dict>
+</plist>
+XML
 
     ["redis-server", "io.redis.redis-server"].each do |service_name|
       before do
         Dir.stub(:glob).and_return(["/Users/igor/Library/LaunchAgents/io.redis.redis-server.plist"], [])
         provider.stub(:shell_out!).
                  with("launchctl list", {:group => 1001, :user => 101}).
-                 and_return(double("ouput", :stdout => stdout))
+                 and_return(mock("Status", :stdout => launchctl_stdout))
+        provider.stub(:shell_out).
+                 with(/launchctl list /,
+                      {:group => nil, :user => nil}).
+                 and_return(mock("Status",
+                                 :stdout => launchctl_stdout, :exitstatus => 0))
+        provider.stub(:shell_out!).
+                 with(/plutil -convert xml1 -o/).
+                 and_return(mock("Status", :stdout => plutil_stdout))
 
         File.stub(:stat).and_return(double("stat", :gid => 1001, :uid => 101))
       end
@@ -64,7 +82,7 @@ describe Chef::Provider::Service::Macosx do
 
         describe "#load_current_resource" do
           context "when launchctl returns pid in service list" do
-            let(:stdout) { StringIO.new <<-SVC_LIST }
+            let(:launchctl_stdout) { StringIO.new <<-SVC_LIST }
   12761 - 0x100114220.old.machinit.thing
   7777  - io.redis.redis-server
   - - com.lol.stopped-thing
@@ -84,21 +102,21 @@ describe Chef::Provider::Service::Macosx do
           end
 
           describe "running unsupported actions" do
+            let(:launchctl_stdout) { StringIO.new <<-SVC_LIST }
+12761 - 0x100114220.old.machinit.thing
+7777  - io.redis.redis-server
+- - com.lol.stopped-thing
+SVC_LIST
+
             before do
               Dir.stub(:glob).and_return(["/Users/igor/Library/LaunchAgents/io.redis.redis-server.plist"], [])
             end
-            it "should throw an exception when enable action is attempted" do
-              lambda {provider.run_action(:enable)}.should raise_error(Chef::Exceptions::UnsupportedAction)
-            end
             it "should throw an exception when reload action is attempted" do
               lambda {provider.run_action(:reload)}.should raise_error(Chef::Exceptions::UnsupportedAction)
             end
-            it "should throw an exception when disable action is attempted" do
-              lambda {provider.run_action(:disable)}.should raise_error(Chef::Exceptions::UnsupportedAction)
-            end
           end
           context "when launchctl returns empty service pid" do
-            let(:stdout) { StringIO.new <<-SVC_LIST }
+            let(:launchctl_stdout) { StringIO.new <<-SVC_LIST }
   12761 - 0x100114220.old.machinit.thing
   - - io.redis.redis-server
   - - com.lol.stopped-thing
@@ -118,7 +136,7 @@ describe Chef::Provider::Service::Macosx do
           end
 
           context "when launchctl doesn't return service entry at all" do
-            let(:stdout) { StringIO.new <<-SVC_LIST }
+            let(:launchctl_stdout) { StringIO.new <<-SVC_LIST }
   12761 - 0x100114220.old.machinit.thing
   - - com.lol.stopped-thing
   SVC_LIST