Merge pull request #1771 from opscode/btm/CHEF-5022

CHEF-5022: Differentiate between Windows service startup_types

8 files changed, 286 insertions, 38 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c01fd3bbef..bc30f8a2d9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -278,3 +278,4 @@
 * guard_interpreter attribute: use powershell\_script, other script resources in guards (CHEF-4553)
 * Fix for CHEF-5169: add require for chef/config_fetcher
 * SIGTERM will once-more kill a non-daemonized chef-client (CHEF-5172)
+* Fix Windows services getting stuck in manual startup_type (CHEF-5022)
diff --git a/DOC_CHANGES.md b/DOC_CHANGES.md
index b557601466..ab1f5844a1 100644
--- a/DOC_CHANGES.md
+++ b/DOC_CHANGES.md
@@ -23,3 +23,12 @@ rt string, an enumerable of ports, or a single port number.
 ### Encrypted Data Bags Version 3
 
 Encrypted Data Bag version 3 uses [GCM](http://en.wikipedia.org/wiki/Galois/Counter_Mode) internally. Ruby 2 and OpenSSL version 1.0.1 or higher are required to use it.
+
+### New windows_service resource
+
+The windows_service resource inherits from the service resource and has all the same options but adds an action and attribute.
+
+action :configure_startup - sets the startup type on the resource to the value of the `startup_type` attribute
+attribute startup_type - the value as a symbol that the startup type should be set to on the service, valid options :automatic, :manual, :disabled
+
+Note that the service resource will also continue to set the startup type to automatic or disabled, respectively, when the enabled or disabled actions are used.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index db14882996..9199c827b3 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -97,3 +97,20 @@ On OSX, the 'group' provider would use 'etc' to determine existing groups,
 but 'dscl' to add groups, causing broken idempotency if something existed
 in /etc/group. The provider now uses 'dscl' for both idempotenty checks and
 modifications.
+
+## Windows Service Startup Type
+
+When a Windows service is running and Chef stops it, the startup type will change from automatic to manual. A bug previously existed
+that prevented you from changing the startup type to disabled from manual. Using the enable and disable actions will now correctly set
+the service startup type to automatic and disabled, respectively. A new `windows_service` resource has been added that allows you to 
+specify the startup type as manual:
+
+```
+windows_service "BITS" do
+  action :configure_startup
+  startup_type :manual
+end
+```
+
+You must use the windows_service resource to utilize the `:configure_startup` action and `startup_type` attribute. The service resource
+does not support them.
diff --git a/lib/chef/provider/service/windows.rb b/lib/chef/provider/service/windows.rb
index 2d478fa9fe..d31aad4c9d 100644
--- a/lib/chef/provider/service/windows.rb
+++ b/lib/chef/provider/service/windows.rb
@@ -27,6 +27,7 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   #Win32::Service.get_start_type
   AUTO_START = 'auto start'
+  MANUAL = 'demand start'
   DISABLED = 'disabled'
 
   #Win32::Service.get_current_state
@@ -45,11 +46,16 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
   end
 
   def load_current_resource
-    @current_resource = Chef::Resource::Service.new(@new_resource.name)
+    @current_resource = Chef::Resource::WindowsService.new(@new_resource.name)
     @current_resource.service_name(@new_resource.service_name)
     @current_resource.running(current_state == RUNNING)
     Chef::Log.debug "#{@new_resource} running: #{@current_resource.running}"
-    @current_resource.enabled(start_type == AUTO_START)
+    case current_start_type
+    when AUTO_START
+      @current_resource.enabled(true)
+    when DISABLED
+      @current_resource.enabled(false)
+    end
     Chef::Log.debug "#{@new_resource} enabled: #{@current_resource.enabled}"
     @current_resource
   end
@@ -125,15 +131,7 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   def enable_service
     if Win32::Service.exists?(@new_resource.service_name)
-      if start_type == AUTO_START
-        Chef::Log.debug "#{@new_resource} already enabled - nothing to do"
-      else
-        Win32::Service.configure(
-          :service_name => @new_resource.service_name,
-          :start_type => Win32::Service::AUTO_START
-        )
-        @new_resource.updated_by_last_action(true)
-      end
+      set_startup_type(:automatic)
     else
       Chef::Log.debug "#{@new_resource} does not exist - nothing to do"
     end
@@ -141,26 +139,76 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   def disable_service
     if Win32::Service.exists?(@new_resource.service_name)
-      if start_type == AUTO_START
-        Win32::Service.configure(
-          :service_name => @new_resource.service_name,
-          :start_type => Win32::Service::DISABLED
-        )
-        @new_resource.updated_by_last_action(true)
-      else
-        Chef::Log.debug "#{@new_resource} already disabled - nothing to do"
-      end
+      set_startup_type(:disabled)
     else
       Chef::Log.debug "#{@new_resource} does not exist - nothing to do"
     end
   end
 
+  def action_enable
+    if current_start_type != AUTO_START
+      converge_by("enable service #{@new_resource}") do
+        enable_service
+        Chef::Log.info("#{@new_resource} enabled")
+      end
+    else
+      Chef::Log.debug("#{@new_resource} already enabled - nothing to do")
+    end
+    load_new_resource_state
+    @new_resource.enabled(true)
+  end
+
+  def action_disable
+    if current_start_type != DISABLED
+      converge_by("disable service #{@new_resource}") do
+        disable_service
+        Chef::Log.info("#{@new_resource} disabled")
+      end
+    else
+      Chef::Log.debug("#{@new_resource} already disabled - nothing to do")
+    end
+    load_new_resource_state
+    @new_resource.enabled(false)
+  end
+
+  def action_configure_startup
+    case @new_resource.startup_type
+    when :automatic
+      if current_start_type != AUTO_START
+        converge_by("set service #{@new_resource} startup type to automatic") do
+          set_startup_type(:automatic)
+        end
+      else
+        Chef::Log.debug("#{@new_resource} startup_type already automatic - nothing to do")
+      end
+    when :manual
+      if current_start_type != MANUAL
+        converge_by("set service #{@new_resource} startup type to manual") do
+          set_startup_type(:manual)
+        end
+      else
+        Chef::Log.debug("#{@new_resource} startup_type already manual - nothing to do")
+      end
+    when :disabled
+      if current_start_type != DISABLED
+        converge_by("set service #{@new_resource} startup type to disabled") do
+          set_startup_type(:disabled)
+        end
+      else
+        Chef::Log.debug("#{@new_resource} startup_type already disabled - nothing to do")
+      end
+    end
+
+    # Avoid changing enabled from true/false for now
+    @new_resource.enabled(nil)
+  end
+
   private
   def current_state
     Win32::Service.status(@new_resource.service_name).current_state
   end
 
-  def start_type
+  def current_start_type
     Win32::Service.config_info(@new_resource.service_name).start_type
   end
 
@@ -188,4 +236,22 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
       worker.join
     end
   end
+
+  # Takes Win32::Service start_types
+  def set_startup_type(type)
+    # Set-Service Startup Type => Win32::Service Constant
+    allowed_types = { :automatic => Win32::Service::AUTO_START,
+                      :manual    => Win32::Service::DEMAND_START,
+                      :disabled  => Win32::Service::DISABLED }
+    unless allowed_types.keys.include?(type)
+      raise Chef::Exceptions::ConfigurationError, "#{@new_resource.name}: Startup type '#{type}' is not supported"
+    end
+
+    Chef::Log.debug "#{@new_resource.name} setting start_type to #{type}"
+    Win32::Service.configure(
+      :service_name => @new_resource.service_name,
+      :start_type => allowed_types[type]
+    )
+    @new_resource.updated_by_last_action(true)
+  end
 end
diff --git a/lib/chef/resource/windows_service.rb b/lib/chef/resource/windows_service.rb
new file mode 100644
index 0000000000..5ed8e76cbd
--- /dev/null
+++ b/lib/chef/resource/windows_service.rb
@@ -0,0 +1,53 @@
+#
+# Author:: Bryan McLellan <btm@loftninjas.org>
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/resource/service'
+
+class Chef
+  class Resource
+    class WindowsService < Chef::Resource::Service
+
+      # Until #1773 is resolved, you need to manually specify the windows_service resource
+      # to use action :configure_startup and attribute startup_type
+
+      # provides :service, :on_platforms => ["windows"]
+
+      identity_attr :service_name
+
+      state_attrs :enabled, :running
+
+      def initialize(name, run_context=nil)
+        super
+        @resource_name = :windows_service
+        @provider = Chef::Provider::Service::Windows
+        @allowed_actions.push(:configure_startup)
+        @startup_type = :automatic
+      end
+
+      def startup_type(arg=nil)
+        # Set-Service arguments are automatic and manual
+        # Win32::Service returns 'auto start' or 'demand start' respectively, which the provider currently uses
+        set_or_return(
+          :startup_type,
+          arg,
+          :equal_to => [ :automatic, :manual, :disabled ]
+        )
+      end
+    end
+  end
+end
diff --git a/lib/chef/resources.rb b/lib/chef/resources.rb
index 01e8d63040..93ff682288 100644
--- a/lib/chef/resources.rb
+++ b/lib/chef/resources.rb
@@ -64,6 +64,7 @@ require 'chef/resource/ruby_block'
 require 'chef/resource/scm'
 require 'chef/resource/script'
 require 'chef/resource/service'
+require 'chef/resource/windows_service'
 require 'chef/resource/subversion'
 require 'chef/resource/smartos_package'
 require 'chef/resource/template'
diff --git a/spec/unit/provider/service/windows_spec.rb b/spec/unit/provider/service/windows_spec.rb
index 33d3cde8fd..b16c59189b 100644
--- a/spec/unit/provider/service/windows_spec.rb
+++ b/spec/unit/provider/service/windows_spec.rb
@@ -24,12 +24,14 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
     @node = Chef::Node.new
     @events = Chef::EventDispatch::Dispatcher.new
     @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Service.new("chef")
+    @new_resource = Chef::Resource::WindowsService.new("chef")
     @provider = Chef::Provider::Service::Windows.new(@new_resource, @run_context)
+    @provider.current_resource = Chef::Resource::WindowsService.new("current-chef")
     Object.send(:remove_const, 'Win32') if defined?(Win32)
     Win32 = Module.new
     Win32::Service = Class.new
     Win32::Service::AUTO_START = 0x00000002
+    Win32::Service::DEMAND_START = 0x00000003
     Win32::Service::DISABLED = 0x00000004
     Win32::Service.stub(:status).with(@new_resource.service_name).and_return(
       double("StatusStruct", :current_state => "running"))
@@ -57,6 +59,13 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
     @provider.current_resource.enabled.should be_true
   end
 
+  it "does not set the current resources start type if it is neither AUTO START or DISABLED" do
+    Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
+      double("ConfigStruct", :start_type => "manual"))
+    @provider.load_current_resource
+    @provider.current_resource.enabled.should be_nil
+  end
+
   describe Chef::Provider::Service::Windows, "start_service" do
     before(:each) do
       Win32::Service.stub(:status).with(@new_resource.service_name).and_return(
@@ -192,8 +201,8 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
     end
 
     it "should pass custom timeout to the stop command if provided" do
-      Win32::Service.stub!(:status).with(@new_resource.service_name).and_return(
-        mock("StatusStruct", :current_state => "running"))
+      Win32::Service.stub(:status).with(@new_resource.service_name).and_return(
+        double("StatusStruct", :current_state => "running"))
       @new_resource.timeout 1
       Win32::Service.should_receive(:stop).with(@new_resource.service_name)
       Timeout.timeout(2) do
@@ -246,7 +255,6 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
   end
 
   describe Chef::Provider::Service::Windows, "enable_service" do
-
     before(:each) do
       Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
         double("ConfigStruct", :start_type => "disabled"))
@@ -264,25 +272,48 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
       @provider.enable_service
       @new_resource.updated_by_last_action?.should be_false
     end
+  end
 
-    it "should do nothing if the service is enabled" do
+  describe Chef::Provider::Service::Windows, "action_enable" do
+    it "does nothing if the service is enabled" do
       Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
         double("ConfigStruct", :start_type => "auto start"))
-      Win32::Service.should_not_receive(:configure)
-      @provider.enable_service
-      @new_resource.updated_by_last_action?.should be_false
+      @provider.should_not_receive(:enable_service)
+      @provider.action_enable
+    end
+
+    it "enables the service if it is not set to automatic start" do
+      Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
+        double("ConfigStruct", :start_type => "disabled"))
+      @provider.should_receive(:enable_service)
+      @provider.action_enable
     end
   end
 
-  describe Chef::Provider::Service::Windows, "disable_service" do
+  describe Chef::Provider::Service::Windows, "action_disable" do
+    it "does nothing if the service is disabled" do
+      Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
+        double("ConfigStruct", :start_type => "disabled"))
+      @provider.should_not_receive(:disable_service)
+      @provider.action_disable
+    end
+
+    it "disables the service if it is not set to disabled" do
+      Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
+        double("ConfigStruct", :start_type => "auto start"))
+      @provider.should_receive(:disable_service)
+      @provider.action_disable
+    end
+  end
 
+  describe Chef::Provider::Service::Windows, "disable_service" do
     before(:each) do
       Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
         double("ConfigStruct", :start_type => "auto start"))
     end
 
     it "should disable service" do
-      Win32::Service.should_receive(:configure).with(:service_name => @new_resource.service_name, :start_type => Win32::Service::DISABLED)
+      Win32::Service.should_receive(:configure)
       @provider.disable_service
       @new_resource.updated_by_last_action?.should be_true
     end
@@ -293,15 +324,39 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
       @provider.disable_service
       @new_resource.updated_by_last_action?.should be_false
     end
+  end
 
-    it "should do nothing if the service is disabled" do
-      Win32::Service.stub(:config_info).with(@new_resource.service_name).and_return(
-        double("ConfigStruct", :start_type => "disabled"))
-      @provider.load_current_resource
-      Win32::Service.should_not_receive(:configure)
-      @provider.disable_service
-      @new_resource.updated_by_last_action?.should be_false
+  describe Chef::Provider::Service::Windows, "action_configure_startup" do
+    { :automatic => "auto start", :manual => "demand start", :disabled => "disabled" }.each do |type,win32|
+      it "sets the startup type to #{type} if it is something else" do
+        @new_resource.startup_type(type)
+        @provider.stub(:current_start_type).and_return("fire")
+        @provider.should_receive(:set_startup_type).with(type)
+        @provider.action_configure_startup
+      end
+
+      it "leaves the startup type as #{type} if it is already set" do
+        @new_resource.startup_type(type)
+        @provider.stub(:current_start_type).and_return(win32)
+        @provider.should_not_receive(:set_startup_type).with(type)
+        @provider.action_configure_startup
+      end
     end
+  end
 
+  describe Chef::Provider::Service::Windows, "set_start_type" do
+    allowed_types = { :automatic => Win32::Service::AUTO_START,
+                      :manual    => Win32::Service::DEMAND_START,
+                      :disabled  => Win32::Service::DISABLED }
+    allowed_types.each do |arg,win32_constant|
+      it "when called with #{arg} it calls Win32::Service#configure with #{win32_constant}" do
+        Win32::Service.should_receive(:configure).with(:service_name => @new_resource.service_name, :start_type => win32_constant)
+        @provider.send(:set_startup_type, arg)
+      end
+    end
+
+    it "raises an exception when given an unknown start type" do
+      expect { @provider.send(:set_startup_type, :fire_truck) }.to raise_error(Chef::Exceptions::ConfigurationError)
+    end
   end
 end
diff --git a/spec/unit/resource/windows_service_spec.rb b/spec/unit/resource/windows_service_spec.rb
new file mode 100644
index 0000000000..c92c3be1b0
--- /dev/null
+++ b/spec/unit/resource/windows_service_spec.rb
@@ -0,0 +1,46 @@
+#
+# Author:: Bryan McLellan <btm@loftninjas.org>
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'spec_helper'
+
+describe Chef::Resource::WindowsService, "initialize", :windows_only do
+
+  let(:resource) { Chef::Resource::WindowsService.new("BITS") }
+
+  it "returns a Chef::Resource::WindowsService" do
+    expect(resource).to be_a_kind_of(Chef::Resource::WindowsService)
+  end
+
+  it "sets the resource_name to :windows_service" do
+    expect(resource.resource_name).to eql(:windows_service)
+  end
+
+  it "sets the provider to Chef::Provider::Service::Windows" do
+    expect(resource.provider).to eql(Chef::Provider::Service::Windows)
+  end
+
+  it "supports setting startup_type" do
+    resource.startup_type(:manual)
+    expect(resource.startup_type).to eql(:manual)
+  end
+
+  it "allows the action to be 'configure_startup'" do
+    resource.action :configure_startup
+    resource.action.should == [:configure_startup]
+  end
+end