update syntax of `update-rc.d` commands in enable & disable actions

Previous implementation was built for an outdated version of
`update-rc.d` and it used commands that no longer work.

The `stop` and `start` commands were removed in 2.88.dsf-42 [1]:

```
  * sysv-rc:
    - update-rc.d no longer supports non-dependency-based boot.
      + Remove non-insserv codepaths.
      + Warn if the start or stop actions are used.
      + Skip runlevel mismatch warnings if default action is used
        (no arguments to validate).
      + Update manual page to remove start and stop actions, plus
        manual setting of boot sequence ordering; note that start
        and stop are no longer supported.  Closes: #606505.
```

and has been since ported to all currently supported Debian
distributions (including Jessie, released in 2015).

Both `start` and `stop` commands now effectively call `defaults`.

As a result of this `action_disable` did not disable a service, it would
enable it instead. Chef would execute the following commands on disable:

```
/usr/sbin/update-rc.d -f <service_name> remove
/usr/sbin/update-rc.d -f <service_name> stop 80 2 3 4 5 .
```

But `update-rc.d` would effectively run:

```
/usr/sbin/update-rc.d -f <service_name> remove
/usr/sbin/update-rc.d <service_name> defaults
```

And the service gets enabled instead.

With changes in this commit Chef will effectively run the following
commands:

```
/usr/sbin/update-rc.d -f <service_name> remove
/usr/sbin/update-rc.d <service_name> defaults
/usr/sbin/update-rc.d <service_name> disable
```

The service is now disabled, as expected.

Additionally `priority` support has been dropped entirely from code as
it's not supported by `update-rc.d` either.

It's potentially a breaking change on a very outdated distributions and
it could be worked around with additional `if/else` branches, but given
all non-EOLed distros have up-to-date version of `update-rc.d` available
it didn't feel worth the complexity.

[1] https://launchpad.net/debian/+source/sysvinit/2.88dsf-42

Signed-off-by: Rob Ulejczyk <rulejczyk@gmail.com>

2 files changed, 47 insertions, 29 deletions

diff --git a/lib/chef/provider/service/debian.rb b/lib/chef/provider/service/debian.rb
index 9f48504d5e..6774b214ae 100644
--- a/lib/chef/provider/service/debian.rb
+++ b/lib/chef/provider/service/debian.rb
@@ -149,44 +149,42 @@ class Chef
         end
 
         def enable_service
-          if new_resource.priority.is_a? Integer
-            shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} remove")
-            shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} defaults #{new_resource.priority} #{100 - new_resource.priority}")
-          elsif new_resource.priority.is_a? Hash
+          if new_resource.priority.is_a? Hash
             # we call the same command regardless of we're enabling or disabling
             # users passing a Hash are responsible for setting their own start priorities
             set_priority
-          else # No priority, go with update-rc.d defaults
+          else # No priority or Integer. Either way go with update-rc.d defaults
             shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} remove")
             shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} defaults")
           end
         end
 
         def disable_service
-          if new_resource.priority.is_a? Integer
-            # Stop processes in reverse order of start using '100 - start_priority'
-            shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} remove")
-            shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} stop #{100 - new_resource.priority} 2 3 4 5 .")
-          elsif new_resource.priority.is_a? Hash
+          if new_resource.priority.is_a? Hash
             # we call the same command regardless of we're enabling or disabling
             # users passing a Hash are responsible for setting their own stop priorities
             set_priority
-          else
-            # no priority, using '100 - 20 (update-rc.d default)' to stop in reverse order of start
+          else # No priority or Integer. Either way disable in all levels.
             shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} remove")
-            shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} stop 80 2 3 4 5 .")
+            shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} defaults")
+            shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} disable")
           end
         end
 
         def set_priority
-          args = ""
-          new_resource.priority.each do |level, o|
-            action = o[0]
-            priority = o[1]
-            args += "#{action} #{priority} #{level} . "
-          end
+          # Reset priorities to default values before applying customizations. This way
+          # the final state will always be consistent, regardless if all runlevels were
+          # provided.
           shell_out!("/usr/sbin/update-rc.d -f #{new_resource.service_name} remove")
-          shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} #{args}")
+          shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} defaults")
+
+          # Note the `_priority` is not used. update-rc.d does not support priorities
+          # anymore, this feature has been dropped in sysvinit 2.88dsf-42.
+          new_resource.priority.each do |level, (action, _priority)|
+            disable_or_enable = (action == :start ? "enable" : "disable")
+
+            shell_out!("/usr/sbin/update-rc.d #{new_resource.service_name} #{disable_or_enable} #{level}")
+          end
         end
       end
     end
diff --git a/spec/unit/provider/service/debian_service_spec.rb b/spec/unit/provider/service/debian_service_spec.rb
index 5f89605b2e..0aef4bc760 100644
--- a/spec/unit/provider/service/debian_service_spec.rb
+++ b/spec/unit/provider/service/debian_service_spec.rb
@@ -183,7 +183,7 @@ describe Chef::Provider::Service::Debian do
   describe "enable_service" do
     let(:service_name) { @new_resource.service_name }
     context "when the service doesn't set a priority" do
-      it "calls update-rc.d 'service_name' defaults" do
+      it "calls update-rc.d remove => defaults" do
         expect_commands(@provider, [
           "/usr/sbin/update-rc.d -f #{service_name} remove",
           "/usr/sbin/update-rc.d #{service_name} defaults",
@@ -197,10 +197,10 @@ describe Chef::Provider::Service::Debian do
         @new_resource.priority(75)
       end
 
-      it "calls update-rc.d 'service_name' defaults" do
+      it "calls update-rc.d remove => defaults" do
         expect_commands(@provider, [
           "/usr/sbin/update-rc.d -f #{service_name} remove",
-          "/usr/sbin/update-rc.d #{service_name} defaults 75 25",
+          "/usr/sbin/update-rc.d #{service_name} defaults",
         ])
         @provider.enable_service
       end
@@ -211,10 +211,12 @@ describe Chef::Provider::Service::Debian do
         @new_resource.priority(2 => [:start, 20], 3 => [:stop, 55])
       end
 
-      it "calls update-rc.d 'service_name' with those priorities" do
+      it "calls update-rc.d remove => defaults => enable|disable <runlevel>" do
         expect_commands(@provider, [
           "/usr/sbin/update-rc.d -f #{service_name} remove",
-          "/usr/sbin/update-rc.d #{service_name} start 20 2 . stop 55 3 . ",
+          "/usr/sbin/update-rc.d #{service_name} defaults",
+          "/usr/sbin/update-rc.d #{service_name} enable 2",
+          "/usr/sbin/update-rc.d #{service_name} disable 3",
         ])
         @provider.enable_service
       end
@@ -224,10 +226,11 @@ describe Chef::Provider::Service::Debian do
   describe "disable_service" do
     let(:service_name) { @new_resource.service_name }
     context "when the service doesn't set a priority" do
-      it "calls update-rc.d -f 'service_name' remove + stop with default priority" do
+      it "calls update-rc.d remove => defaults => disable" do
         expect_commands(@provider, [
           "/usr/sbin/update-rc.d -f #{service_name} remove",
-          "/usr/sbin/update-rc.d -f #{service_name} stop 80 2 3 4 5 .",
+          "/usr/sbin/update-rc.d #{service_name} defaults",
+          "/usr/sbin/update-rc.d #{service_name} disable",
         ])
         @provider.disable_service
       end
@@ -238,10 +241,27 @@ describe Chef::Provider::Service::Debian do
         @new_resource.priority(75)
       end
 
-      it "calls update-rc.d -f 'service_name' remove + stop with the specified priority" do
+      it "calls update-rc.d remove => defaults => disable" do
         expect_commands(@provider, [
           "/usr/sbin/update-rc.d -f #{service_name} remove",
-          "/usr/sbin/update-rc.d -f #{service_name} stop #{100 - @new_resource.priority} 2 3 4 5 .",
+          "/usr/sbin/update-rc.d #{service_name} defaults",
+          "/usr/sbin/update-rc.d #{service_name} disable",
+        ])
+        @provider.disable_service
+      end
+    end
+
+    context "when the service sets complex priorities" do
+      before do
+        @new_resource.priority(2 => [:start, 20], 3 => [:stop, 55])
+      end
+
+      it "calls update-rc.d remove => defaults => enable|disable <runlevel>" do
+        expect_commands(@provider, [
+          "/usr/sbin/update-rc.d -f #{service_name} remove",
+          "/usr/sbin/update-rc.d #{service_name} defaults",
+          "/usr/sbin/update-rc.d #{service_name} enable 2",
+          "/usr/sbin/update-rc.d #{service_name} disable 3",
         ])
         @provider.disable_service
       end