spec and code fixes for run_levels

2 files changed, 83 insertions, 44 deletions

diff --git a/lib/chef/provider/service/redhat.rb b/lib/chef/provider/service/redhat.rb
index 094981c1ac..da86b2e2fc 100644
--- a/lib/chef/provider/service/redhat.rb
+++ b/lib/chef/provider/service/redhat.rb
@@ -23,6 +23,11 @@ class Chef
     class Service
       class Redhat < Chef::Provider::Service::Init
 
+        # @api private
+        attr_accessor :service_missing
+        # @api private
+        attr_accessor :current_run_levels
+
         provides :service, platform_family: %w(rhel fedora suse) do |node|
           Chef::Platform::ServiceHelpers.service_resource_providers.include?(:redhat)
         end
@@ -36,11 +41,15 @@ class Chef
 
         def initialize(new_resource, run_context)
           super
-          @init_command = "/sbin/service #{@new_resource.service_name}"
-          @new_resource.supports[:status] = true
+          @init_command = "/sbin/service #{new_resource.service_name}"
+          new_resource.supports[:status] = true
           @service_missing = false
           @current_run_levels = []
-          @run_levels = @new_resource.run_levels
+        end
+
+        # @api private
+        def run_levels
+          new_resource.run_levels
         end
 
         def define_resource_requirements
@@ -49,12 +58,12 @@ class Chef
           requirements.assert(:all_actions) do |a|
             chkconfig_file = "/sbin/chkconfig"
             a.assertion { ::File.exists? chkconfig_file  }
-            a.failure_message Chef::Exceptions::Service, "#{chkconfig_file} does not exist!"
+            a.failure_message Chef::Exceptions::Service, "#{chkconfig_file} dbleoes not exist!"
           end
 
           requirements.assert(:start, :enable, :reload, :restart) do |a|
             a.assertion { !@service_missing }
-            a.failure_message Chef::Exceptions::Service, "#{@new_resource}: unable to locate the init.d script!"
+            a.failure_message Chef::Exceptions::Service, "#{new_resource}: unable to locate the init.d script!"
             a.whyrun "Assuming service would be disabled. The init script is not presently installed."
           end
         end
@@ -63,44 +72,44 @@ class Chef
           super
 
           if ::File.exists?("/sbin/chkconfig")
-            chkconfig = shell_out!("/sbin/chkconfig --list #{@current_resource.service_name}", :returns => [0,1])
-            unless @run_levels.nil? or @run_levels.empty?
-              all_levels_enabled = true
-              chkconfig.split(/\s+/)[1..-1].each do |level|
+            chkconfig = shell_out!("/sbin/chkconfig --list #{current_resource.service_name}", :returns => [0,1])
+            unless run_levels.nil? or run_levels.empty?
+              all_levels_match = true
+              chkconfig.stdout.split(/\s+/)[1..-1].each do |level|
                 index = level.split(':').first
                 status = level.split(':').last
-                if @run_levels.include?(index)
-                  if status =~ CHKCONFIG_ON
-                    @current_run_levels << index
-                  else
-                    all_levels_enabled = false
-                  end
+                if level =~ CHKCONFIG_ON
+                  @current_run_levels << index.to_i
+                  all_levels_match = false unless run_levels.include?(index.to_i)
+                else
+                  all_levels_match = false if run_levels.include?(index.to_i)
                 end
               end
-              @current_resource.enabled(all_levels_enabled)
+              current_resource.enabled(all_levels_match)
             else
-              @current_resource.enabled(!!(chkconfig.stdout =~ CHKCONFIG_ON))
+              current_resource.enabled(!!(chkconfig.stdout =~ CHKCONFIG_ON))
             end
             @service_missing = !!(chkconfig.stderr =~ CHKCONFIG_MISSING)
           end
 
-          @current_resource
+          current_resource
         end
 
+        # @api private
         def levels
-          (@run_levels.nil? or @run_levels.empty?) ? "" : "--level #{@run_levels.join('')} "
+          (run_levels.nil? or run_levels.empty?) ? "" : "--level #{run_levels.join('')} "
         end
 
         def enable_service()
-          unless @run_levels.nil? or @run_levels.empty?
-            disable_levels = @current_run_levels - @run_levels
-            shell_out! "/sbin/chkconfig --level #{disable_levels.join('')} #{@new_resource.service_name} off" unless disable_levels.empty?
+          unless run_levels.nil? or run_levels.empty?
+            disable_levels = current_run_levels - run_levels
+            shell_out! "/sbin/chkconfig --level #{disable_levels.join('')} #{new_resource.service_name} off" unless disable_levels.empty?
           end
-          shell_out! "/sbin/chkconfig #{levels}#{@new_resource.service_name} on"
+          shell_out! "/sbin/chkconfig #{levels}#{new_resource.service_name} on"
         end
 
         def disable_service()
-          shell_out! "/sbin/chkconfig #{levels}#{@new_resource.service_name} off"
+          shell_out! "/sbin/chkconfig #{levels}#{new_resource.service_name} off"
         end
       end
     end
diff --git a/spec/unit/provider/service/redhat_spec.rb b/spec/unit/provider/service/redhat_spec.rb
index 78be98f533..1cb985714f 100644
--- a/spec/unit/provider/service/redhat_spec.rb
+++ b/spec/unit/provider/service/redhat_spec.rb
@@ -69,9 +69,9 @@ describe "Chef::Provider::Service::Redhat" do
         expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:off   2:off   3:off   4:off   5:on  6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
-        expect(@provider.instance_variable_get("@service_missing")).to be_falsey
+        expect(@provider.service_missing).to be false
         @provider.load_current_resource
-        expect(@current_resource.enabled).to be_truthy
+        expect(@current_resource.enabled).to be true
       end
 
       it "sets the current enabled status to false if the regex does not match" do
@@ -79,31 +79,45 @@ describe "Chef::Provider::Service::Redhat" do
         expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:off   2:off   3:off   4:off   5:off   6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
-        expect(@provider.instance_variable_get("@service_missing")).to be_falsey
+        expect(@provider.service_missing).to be false
         expect(@provider.load_current_resource).to eql(@current_resource)
-        expect(@current_resource.enabled).to be_falsey
+        expect(@current_resource.enabled).to be false
       end
 
       it "sets the current enabled status to true if the service is enabled at specified run levels" do
         status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
-        @current_resource.instance_variable_set(:@run_levels, [ 1, 2 ])
-        @provider.should_receive(:shell_out).with("/sbin/service chef status").and_return(status)
+        @new_resource.run_levels([1, 2])
+        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:on   2:on   3:off   4:off   5:off   6:off", :stderr => "")
-        @provider.should_receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
-        @provider.instance_variable_get("@service_missing").should be_false
-        @provider.load_current_resource.should 
-        @current_resource.enabled.should be_true
+        expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
+        expect(@provider.service_missing).to be false
+        @provider.load_current_resource
+        expect(@current_resource.enabled).to be true
+        expect(@provider.current_run_levels).to eql([1, 2])
+      end
+
+      it "sets the current enabled status to false if the service is enabled at a run level it should not" do
+        status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
+        @new_resource.run_levels([1, 2])
+        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
+        chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:on   2:on   3:on   4:off   5:off   6:off", :stderr => "")
+        expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
+        expect(@provider.service_missing).to be false
+        @provider.load_current_resource
+        expect(@current_resource.enabled).to be false
+        expect(@provider.current_run_levels).to eql([1, 2, 3])
       end
 
       it "sets the current enabled status to false if the service is not enabled at specified run levels" do
         status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
-        @current_resource.instance_variable_set(:@run_levels, [ 2 ])
-        @provider.should_receive(:shell_out).with("/sbin/service chef status").and_return(status)
+        @new_resource.run_levels([ 2 ])
+        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:on   2:off   3:off   4:off   5:off   6:off", :stderr => "")
-        @provider.should_receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
-        @provider.instance_variable_get("@service_missing").should be_false
-        @provider.load_current_resource.should 
-        @current_resource.enabled.should be_true
+        expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
+        expect(@provider.service_missing).to be false
+        @provider.load_current_resource
+        expect(@current_resource.enabled).to be false
+        expect(@provider.current_run_levels).to eql([1])
       end
     end
 
@@ -168,8 +182,24 @@ describe "Chef::Provider::Service::Redhat" do
     end
 
     it "should call chkconfig to add 'service_name' at specified run_levels" do
-      @provider.instance_variable_set(:@run_levels, [ 1,2 ])
-      @provider.should_receive(:shell_out!).with("/sbin/chkconfig --level 12 #{@new_resource.service_name} on")
+      allow(@provider).to receive(:run_levels).and_return([1, 2])
+      expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --level 12 #{@new_resource.service_name} on")
+      @provider.enable_service
+    end
+
+    it "should call chkconfig to add 'service_name' at specified run_levels when run_levels do not match" do
+      allow(@provider).to receive(:run_levels).and_return([1, 2])
+      allow(@provider).to receive(:current_run_levels).and_return([1, 3])
+      expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --level 12 #{@new_resource.service_name} on")
+      expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --level 3 #{@new_resource.service_name} off")
+      @provider.enable_service
+    end
+
+    it "should call chkconfig to add 'service_name' at specified run_levels if there is an extra run_level" do
+      allow(@provider).to receive(:run_levels).and_return([1, 2])
+      allow(@provider).to receive(:current_run_levels).and_return([1, 2, 3])
+      expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --level 12 #{@new_resource.service_name} on")
+      expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --level 3 #{@new_resource.service_name} off")
       @provider.enable_service
     end
   end
@@ -181,8 +211,8 @@ describe "Chef::Provider::Service::Redhat" do
     end
 
     it "should call chkconfig to del 'service_name' at specified run_levels" do
-      @provider.instance_variable_set(:@run_levels, [ 1,2 ])
-      @provider.should_receive(:shell_out!).with("/sbin/chkconfig --level 12 #{@new_resource.service_name} off")
+      allow(@provider).to receive(:run_levels).and_return([1, 2])
+      expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --level 12 #{@new_resource.service_name} off")
       @provider.disable_service
     end
   end