Merge pull request #7274 from mal/fix-systemd-user-calls

Fix systemd_unit user context

4 files changed, 89 insertions, 44 deletions

diff --git a/lib/chef/provider/service/systemd.rb b/lib/chef/provider/service/systemd.rb
index 1bcc2f3a00..704f94a7ff 100644
--- a/lib/chef/provider/service/systemd.rb
+++ b/lib/chef/provider/service/systemd.rb
@@ -77,7 +77,7 @@ class Chef::Provider::Service::Systemd < Chef::Provider::Service::Simple
 
   def get_systemctl_options_args
     if new_resource.user
-      uid = Etc.getpwuid(new_resource.user).uid
+      uid = Etc.getpwnam(new_resource.user).uid
       options = {
         :environment => {
           "DBUS_SESSION_BUS_ADDRESS" => "unix:path=/run/user/#{uid}/bus",
diff --git a/lib/chef/provider/systemd_unit.rb b/lib/chef/provider/systemd_unit.rb
index d8c83d2b4b..0c02b4491c 100644
--- a/lib/chef/provider/systemd_unit.rb
+++ b/lib/chef/provider/systemd_unit.rb
@@ -219,7 +219,7 @@ class Chef
       end
 
       def daemon_reload
-        shell_out_with_systems_locale!("#{systemctl_path} daemon-reload")
+        shell_out_with_systems_locale!("#{systemctl_cmd} daemon-reload", systemctl_opts)
       end
 
       def systemctl_execute!(action, unit)
@@ -245,7 +245,7 @@ class Chef
       def systemctl_opts
         @systemctl_opts ||=
           if new_resource.user
-            uid = Etc.getpwuid(new_resource.user).uid
+            uid = Etc.getpwnam(new_resource.user).uid
             {
               :user => new_resource.user,
               :environment => {
diff --git a/spec/unit/provider/service/systemd_service_spec.rb b/spec/unit/provider/service/systemd_service_spec.rb
index a9c11b7c18..8286176fed 100644
--- a/spec/unit/provider/service/systemd_service_spec.rb
+++ b/spec/unit/provider/service/systemd_service_spec.rb
@@ -47,7 +47,7 @@ describe Chef::Provider::Service::Systemd do
 
   before(:each) do
     allow(Chef::Resource::Service).to receive(:new).with(service_name).and_return(current_resource)
-    allow(Etc).to receive(:getpwuid).and_return(OpenStruct.new(uid: 10000))
+    allow(Etc).to receive(:getpwnam).and_return(OpenStruct.new(uid: 10000))
   end
 
   describe "load_current_resource" do
diff --git a/spec/unit/provider/systemd_unit_spec.rb b/spec/unit/provider/systemd_unit_spec.rb
index 15d5944992..c1fe08ff94 100644
--- a/spec/unit/provider/systemd_unit_spec.rb
+++ b/spec/unit/provider/systemd_unit_spec.rb
@@ -74,7 +74,7 @@ describe Chef::Provider::SystemdUnit do
   end
 
   before(:each) do
-    allow(Etc).to receive(:getpwuid).and_return(OpenStruct.new(uid: 1000))
+    allow(Etc).to receive(:getpwnam).and_return(OpenStruct.new(uid: 1000))
     allow(Chef::Resource::SystemdUnit).to receive(:new)
                                             .with(unit_name)
                                             .and_return(current_resource)
@@ -280,49 +280,53 @@ describe Chef::Provider::SystemdUnit do
           provider.action_create
         end
 
-        it "triggers a daemon-reload when creating a unit with triggers_reload" do
-          allow(provider).to receive(:manage_unit_file).with(:create)
-          expect(new_resource.triggers_reload).to eq true
-          allow(provider).to receive(:shell_out_with_systems_locale!)
-          expect(provider).to receive(:shell_out_with_systems_locale!)
-                                .with("#{systemctl_path} daemon-reload")
-          provider.action_create
-        end
+        context "when a user is specified" do
+          it "triggers a daemon-reload when creating a unit with triggers_reload" do
+            new_resource.user("joe")
+            allow(provider).to receive(:manage_unit_file).with(:create)
+            expect(new_resource.triggers_reload).to eq true
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to receive(:shell_out_with_systems_locale!)
+                                  .with("#{systemctl_path} --user daemon-reload", user_cmd_opts)
+            provider.action_create
+          end
 
-        it "triggers a daemon-reload when deleting a unit with triggers_reload" do
-          allow(File).to receive(:exist?)
-                           .with(unit_path_system)
-                           .and_return(true)
-          allow(provider).to receive(:manage_unit_file).with(:delete)
-          expect(new_resource.triggers_reload).to eq true
-          allow(provider).to receive(:shell_out_with_systems_locale!)
-          expect(provider).to receive(:shell_out_with_systems_locale!)
-                                .with("#{systemctl_path} daemon-reload")
-          provider.action_delete
-        end
+          it "triggers a daemon-reload when deleting a unit with triggers_reload" do
+            new_resource.user("joe")
+            allow(File).to receive(:exist?)
+                             .with(unit_path_user)
+                             .and_return(true)
+            allow(provider).to receive(:manage_unit_file).with(:delete)
+            expect(new_resource.triggers_reload).to eq true
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to receive(:shell_out_with_systems_locale!)
+                                  .with("#{systemctl_path} --user daemon-reload", user_cmd_opts)
+            provider.action_delete
+          end
 
-        it "does not trigger a daemon-reload when creating a unit without triggers_reload" do
-          new_resource.triggers_reload(false)
-          allow(provider).to receive(:manage_unit_file).with(:create)
-          allow(provider).to receive(:shell_out_with_systems_locale!)
-          expect(provider).to_not receive(:shell_out_with_systems_locale!)
-                                    .with("#{systemctl_path} daemon-reload")
-          provider.action_create
-        end
+          it "does not trigger a daemon-reload when creating a unit without triggers_reload" do
+            new_resource.user("joe")
+            new_resource.triggers_reload(false)
+            allow(provider).to receive(:manage_unit_file).with(:create)
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to_not receive(:shell_out_with_systems_locale!)
+                                      .with("#{systemctl_path} --user daemon-reload", user_cmd_opts)
+            provider.action_create
+          end
 
-        it "does not trigger a daemon-reload when deleting a unit without triggers_reload" do
-          new_resource.triggers_reload(false)
-          allow(File).to receive(:exist?)
-                           .with(unit_path_system)
-                           .and_return(true)
-          allow(provider).to receive(:manage_unit_file).with(:delete)
-          allow(provider).to receive(:shell_out_with_systems_locale!)
-          expect(provider).to_not receive(:shell_out_with_systems_locale!)
-                                    .with("#{systemctl_path} daemon-reload")
-          provider.action_delete
-        end
+          it "does not trigger a daemon-reload when deleting a unit without triggers_reload" do
+            new_resource.user("joe")
+            new_resource.triggers_reload(false)
+            allow(File).to receive(:exist?)
+                             .with(unit_path_user)
+                             .and_return(true)
+            allow(provider).to receive(:manage_unit_file).with(:delete)
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to_not receive(:shell_out_with_systems_locale!)
+                                      .with("#{systemctl_path} --user daemon-reload", user_cmd_opts)
+            provider.action_delete
+          end
 
-        context "when a user is specified" do
           it "deletes the file when it exists" do
             new_resource.user("joe")
             allow(File).to receive(:exist?)
@@ -364,6 +368,47 @@ describe Chef::Provider::SystemdUnit do
         end
 
         context "when no user is specified" do
+          it "triggers a daemon-reload when creating a unit with triggers_reload" do
+            allow(provider).to receive(:manage_unit_file).with(:create)
+            expect(new_resource.triggers_reload).to eq true
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to receive(:shell_out_with_systems_locale!)
+                                  .with("#{systemctl_path} --system daemon-reload", {})
+            provider.action_create
+          end
+
+          it "triggers a daemon-reload when deleting a unit with triggers_reload" do
+            allow(File).to receive(:exist?)
+                             .with(unit_path_system)
+                             .and_return(true)
+            allow(provider).to receive(:manage_unit_file).with(:delete)
+            expect(new_resource.triggers_reload).to eq true
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to receive(:shell_out_with_systems_locale!)
+                                  .with("#{systemctl_path} --system daemon-reload", {})
+            provider.action_delete
+          end
+
+          it "does not trigger a daemon-reload when creating a unit without triggers_reload" do
+            new_resource.triggers_reload(false)
+            allow(provider).to receive(:manage_unit_file).with(:create)
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to_not receive(:shell_out_with_systems_locale!)
+                                      .with("#{systemctl_path} --system daemon-reload", {})
+            provider.action_create
+          end
+
+          it "does not trigger a daemon-reload when deleting a unit without triggers_reload" do
+            new_resource.triggers_reload(false)
+            allow(File).to receive(:exist?)
+                             .with(unit_path_system)
+                             .and_return(true)
+            allow(provider).to receive(:manage_unit_file).with(:delete)
+            allow(provider).to receive(:shell_out_with_systems_locale!)
+            expect(provider).to_not receive(:shell_out_with_systems_locale!)
+                                      .with("#{systemctl_path} --system daemon-reload", {})
+            provider.action_delete
+          end
           it "deletes the file when it exists" do
             allow(File).to receive(:exist?)
                              .with(unit_path_system)