diff options
author | Phil Dibowitz <phil@ipom.com> | 2019-11-01 17:59:04 -0700 |
---|---|---|
committer | Phil Dibowitz <phil@ipom.com> | 2019-11-05 13:14:27 -0800 |
commit | 9cb02c550834c9b60474c33961e73af14918ef37 (patch) | |
tree | f3095b3a24afe8a5adada60c8c638ef356e43072 | |
parent | f4af31e4284f18d78abf318cfbd77ed0cb321cdc (diff) | |
download | chef-9cb02c550834c9b60474c33961e73af14918ef37.tar.gz |
Fix enable on indirect units
This follows this `is_masked` path to ensure we don't try to enable OR
disable systemd units if they are indirect
Fixes #9041
Signed-off-by: Phil Dibowitz <phil@ipom.com>
-rw-r--r-- | lib/chef/provider/service/systemd.rb | 16 | ||||
-rw-r--r-- | lib/chef/provider/systemd_unit.rb | 16 | ||||
-rw-r--r-- | lib/chef/resource/service.rb | 9 | ||||
-rw-r--r-- | lib/chef/resource/systemd_unit.rb | 1 | ||||
-rw-r--r-- | spec/unit/provider/service/systemd_service_spec.rb | 28 | ||||
-rw-r--r-- | spec/unit/provider/systemd_unit_spec.rb | 61 |
6 files changed, 121 insertions, 10 deletions
diff --git a/lib/chef/provider/service/systemd.rb b/lib/chef/provider/service/systemd.rb index 6d5bc338c7..c7c42f82f5 100644 --- a/lib/chef/provider/service/systemd.rb +++ b/lib/chef/provider/service/systemd.rb @@ -51,6 +51,7 @@ class Chef::Provider::Service::Systemd < Chef::Provider::Service::Simple current_resource.running(false) current_resource.enabled(false) current_resource.masked(false) + current_resource.indirect(false) end else current_resource.running(is_active?) @@ -58,6 +59,7 @@ class Chef::Provider::Service::Systemd < Chef::Provider::Service::Simple current_resource.enabled(is_enabled?) current_resource.masked(is_masked?) + current_resource.indirect(is_indirect?) current_resource end @@ -142,11 +144,19 @@ class Chef::Provider::Service::Systemd < Chef::Provider::Service::Simple end def enable_service + if current_resource.masked || current_resource.indirect + logger.trace("#{new_resource} cannot be enabled: it is masked or indirect") + return + end options, args = get_systemctl_options_args shell_out!("#{systemctl_path} #{args} enable #{Shellwords.escape(new_resource.service_name)}", **options) end def disable_service + if current_resource.masked || current_resource.indirect + logger.trace("#{new_resource} cannot be disabled: it is masked or indirect") + return + end options, args = get_systemctl_options_args shell_out!("#{systemctl_path} #{args} disable #{Shellwords.escape(new_resource.service_name)}", **options) end @@ -171,6 +181,12 @@ class Chef::Provider::Service::Systemd < Chef::Provider::Service::Simple shell_out("#{systemctl_path} #{args} is-enabled #{Shellwords.escape(new_resource.service_name)} --quiet", **options).exitstatus == 0 end + def is_indirect? + options, args = get_systemctl_options_args + s = shell_out("#{systemctl_path} #{args} is-enabled #{Shellwords.escape(new_resource.service_name)}", **options) + s.stdout.include?("indirect") + end + def is_masked? options, args = get_systemctl_options_args s = shell_out("#{systemctl_path} #{args} is-enabled #{Shellwords.escape(new_resource.service_name)}", **options) diff --git a/lib/chef/provider/systemd_unit.rb b/lib/chef/provider/systemd_unit.rb index f7fb897f16..00e17af0b1 100644 --- a/lib/chef/provider/systemd_unit.rb +++ b/lib/chef/provider/systemd_unit.rb @@ -42,6 +42,7 @@ class Chef current_resource.active(active?) current_resource.masked(masked?) current_resource.static(static?) + current_resource.indirect(indirect?) current_resource.triggers_reload(new_resource.triggers_reload) current_resource @@ -90,8 +91,11 @@ class Chef if current_resource.static logger.trace("#{new_resource.unit_name} is a static unit, enabling is a NOP.") end + if current_resource.indirect + logger.trace("#{new_resource.unit_name} is an indirect unit, enabling is a NOP.") + end - unless current_resource.enabled || current_resource.static + unless current_resource.enabled || current_resource.static || current_resource.indirect converge_by("enabling unit: #{new_resource.unit_name}") do systemctl_execute!(:enable, new_resource.unit_name) logger.info("#{new_resource} enabled") @@ -104,7 +108,11 @@ class Chef logger.trace("#{new_resource.unit_name} is a static unit, disabling is a NOP.") end - if current_resource.enabled && !current_resource.static + if current_resource.indirect + logger.trace("#{new_resource.unit_name} is an indirect unit, enabling is a NOP.") + end + + if current_resource.enabled && !current_resource.static && !current_resource.indirect converge_by("disabling unit: #{new_resource.unit_name}") do systemctl_execute!(:disable, new_resource.unit_name) logger.info("#{new_resource} disabled") @@ -210,6 +218,10 @@ class Chef systemctl_execute("is-enabled", new_resource.unit_name).stdout.include?("static") end + def indirect? + systemctl_execute("is-enabled", new_resource.unit_name).stdout.include?("indirect") + end + private def unit_path diff --git a/lib/chef/resource/service.rb b/lib/chef/resource/service.rb index c5197d5f06..1674754208 100644 --- a/lib/chef/resource/service.rb +++ b/lib/chef/resource/service.rb @@ -159,6 +159,15 @@ class Chef ) end + # if the service is indirect or not + def indirect(arg = nil) + set_or_return( + :indirect, + arg, + kind_of: [ TrueClass, FalseClass ] + ) + end + def options(arg = nil) set_or_return( :options, diff --git a/lib/chef/resource/systemd_unit.rb b/lib/chef/resource/systemd_unit.rb index 89bc30b9d1..f121e4a7c2 100644 --- a/lib/chef/resource/systemd_unit.rb +++ b/lib/chef/resource/systemd_unit.rb @@ -42,6 +42,7 @@ class Chef property :active, [TrueClass, FalseClass], skip_docs: true property :masked, [TrueClass, FalseClass], skip_docs: true property :static, [TrueClass, FalseClass], skip_docs: true + property :indirect, [TrueClass, FalseClass], skip_docs: true # User-provided properties property :user, String, desired_state: false, diff --git a/spec/unit/provider/service/systemd_service_spec.rb b/spec/unit/provider/service/systemd_service_spec.rb index 15b79922b0..983d524d25 100644 --- a/spec/unit/provider/service/systemd_service_spec.rb +++ b/spec/unit/provider/service/systemd_service_spec.rb @@ -36,11 +36,11 @@ describe Chef::Provider::Service::Systemd do let(:provider) { Chef::Provider::Service::Systemd.new(new_resource, run_context) } let(:shell_out_success) do - double("shell_out", exitstatus: 0, error?: false) + double("shell_out", exitstatus: 0, error?: false, stdout: "") end let(:shell_out_failure) do - double("shell_out", exitstatus: 1, error?: true) + double("shell_out", exitstatus: 1, error?: true, stdout: "") end let(:current_resource) { Chef::Resource::Service.new(service_name) } @@ -56,6 +56,7 @@ describe Chef::Provider::Service::Systemd do allow(provider).to receive(:is_active?).and_return(false) allow(provider).to receive(:is_enabled?).and_return(false) allow(provider).to receive(:is_masked?).and_return(false) + allow(provider).to receive(:is_indirect?).and_return(false) end it "should create a current resource with the name of the new resource" do @@ -359,6 +360,29 @@ describe Chef::Provider::Service::Systemd do expect(provider.is_masked?).to be false end end + + describe "is_indirect?" do + before(:each) do + provider.current_resource = current_resource + current_resource.service_name(service_name) + allow(provider).to receive(:which).with("systemctl").and_return(systemctl_path.to_s) + end + + it "should return true if '#{systemctl_path} --system is-enabled service_name' returns 'indirect'" do + expect(provider).to receive(:shell_out).with("#{systemctl_path} --system is-enabled #{service_name_escaped}", {}).and_return(double(stdout: "indirect", exitstatus: shell_out_success)) + expect(provider.is_indirect?).to be true + end + + it "should return false if '#{systemctl_path} --system is-enabled service_name' returns 0 and outputs something other than 'indirect'" do + expect(provider).to receive(:shell_out).with("#{systemctl_path} --system is-enabled #{service_name_escaped}", {}).and_return(double(stdout: "enabled", exitstatus: shell_out_success)) + expect(provider.is_indirect?).to be false + end + + it "should return false if '#{systemctl_path} --system is-enabled service_name' returns anything except 0 and outputs somethign other than 'indirect''" do + expect(provider).to receive(:shell_out).with("#{systemctl_path} --system is-enabled #{service_name_escaped}", {}).and_return(double(stdout: "enabled", exitstatus: shell_out_failure)) + expect(provider.is_indirect?).to be false + end + end end end end diff --git a/spec/unit/provider/systemd_unit_spec.rb b/spec/unit/provider/systemd_unit_spec.rb index 00f39d0fd9..61caaef8bf 100644 --- a/spec/unit/provider/systemd_unit_spec.rb +++ b/spec/unit/provider/systemd_unit_spec.rb @@ -65,11 +65,19 @@ describe Chef::Provider::SystemdUnit do end let(:shell_out_masked) do - double("shell_out", exit_status: 0, error?: false, stdout: "masked") + double("shell_out", exitstatus: 0, error?: false, stdout: "masked") end let(:shell_out_static) do - double("shell_out", exit_status: 0, error?: false, stdout: "static") + double("shell_out", exitstatus: 0, error?: false, stdout: "static") + end + + let(:shell_out_disabled) do + double("shell_out", exitstatus: 1, error?: true, stdout: "disabled") + end + + let(:shell_out_indirect) do + double("shell_out", exitstatus: 0, error?: true, stdout: "indirect") end before(:each) do @@ -858,8 +866,8 @@ describe Chef::Provider::SystemdUnit do it "returns false when unit is not enabled" do current_resource.user(user_name) expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--user", "is-enabled", unit_name, user_cmd_opts) - .and_return(shell_out_failure) + .with(systemctl_path, "--user", "is-enabled", unit_name, user_cmd_opts) + .and_return(shell_out_disabled) expect(provider.enabled?).to be false end end @@ -874,8 +882,8 @@ describe Chef::Provider::SystemdUnit do it "returns false when unit is not enabled" do expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "is-enabled", unit_name) - .and_return(shell_out_failure) + .with(systemctl_path, "--system", "is-enabled", unit_name) + .and_return(shell_out_disabled) expect(provider.enabled?).to be false end end @@ -962,6 +970,47 @@ describe Chef::Provider::SystemdUnit do end end end + + describe "#indirect?" do + before(:each) do + provider.current_resource = current_resource + allow(provider).to receive(:which).with("systemctl").and_return(systemctl_path.to_s) + end + + context "when a user is specified" do + it "returns true when the unit is indirect" do + current_resource.user(user_name) + expect(provider).to receive(:shell_out_compacted) + .with(systemctl_path, "--user", "is-enabled", unit_name, user_cmd_opts) + .and_return(shell_out_indirect) + expect(provider.indirect?).to be true + end + + it "returns false when the unit is not indirect" do + current_resource.user(user_name) + expect(provider).to receive(:shell_out_compacted) + .with(systemctl_path, "--user", "is-enabled", unit_name, user_cmd_opts) + .and_return(shell_out_static) + expect(provider.indirect?).to be false + end + end + + context "when no user is specified" do + it "returns true when the unit is indirect" do + expect(provider).to receive(:shell_out_compacted) + .with(systemctl_path, "--system", "is-enabled", unit_name) + .and_return(shell_out_indirect) + expect(provider.indirect?).to be true + end + + it "returns false when the unit is not indirect" do + expect(provider).to receive(:shell_out_compacted) + .with(systemctl_path, "--system", "is-enabled", unit_name) + .and_return(shell_out_static) + expect(provider.indirect?).to be false + end + end + end end end end |