diff options
author | Lamont Granquist <lamont@opscode.com> | 2021-01-25 10:13:57 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-25 10:13:57 -0800 |
commit | 7e27fdbf2bf2722bf95ee57ed219598987b07883 (patch) | |
tree | 5a69cda7cbaf1b036836c1c71067f8b4c69eab10 | |
parent | 1899968131a27f703deac7de8f186abcf12da86b (diff) | |
parent | 697ddc41c7665da35c582b5b86aa28a80a6fc124 (diff) | |
download | chef-7e27fdbf2bf2722bf95ee57ed219598987b07883.tar.gz |
Merge pull request #10925 from joshuamiller01/get_systemd_service_faster_for_systemd_unit
load_current_resource for systemd_unit more efficiently
-rw-r--r-- | lib/chef/provider/systemd_unit.rb | 36 | ||||
-rw-r--r-- | spec/unit/provider/systemd_unit_spec.rb | 139 |
2 files changed, 110 insertions, 65 deletions
diff --git a/lib/chef/provider/systemd_unit.rb b/lib/chef/provider/systemd_unit.rb index 97043e48c7..fc7c934bc1 100644 --- a/lib/chef/provider/systemd_unit.rb +++ b/lib/chef/provider/systemd_unit.rb @@ -55,6 +55,26 @@ class Chef end end + def systemd_unit_status + @systemd_unit_status ||= begin + # Collect all the status information for a unit and return it at once + # This may fail if we are managing a template unit (e.g. with '@'), in which case + # we just ignore the error because unit status is irrelevant in that case + s = shell_out(*systemctl_cmd, "show", "-p", "UnitFileState", "-p", "ActiveState", new_resource.unit_name, systemctl_opts) + # e.g. /bin/systemctl --system show -p UnitFileState -p ActiveState syslog.socket + # Returns something like: + # ActiveState=inactive + # UnitFileState=static + status = {} + s.stdout.each_line do |line| + k, v = line.strip.split("=") + status[k] = v + end + + status + end + end + action :create do if current_resource.content != new_resource.to_ini converge_by("creating unit: #{new_resource.unit_name}") do @@ -201,23 +221,29 @@ class Chef end def active? - systemctl_execute("is-active", new_resource.unit_name).exitstatus == 0 + # Note: "activating" is not active (as with type=notify or a oneshot) + systemd_unit_status["ActiveState"] == "active" end def enabled? - systemctl_execute("is-enabled", new_resource.unit_name).exitstatus == 0 + # See https://github.com/systemd/systemd/blob/master/src/systemctl/systemctl-is-enabled.c + # Note: enabled-runtime is excluded because this is volatile, and the state of enabled-runtime + # specifically means that the service is not enabled + %w{enabled static generated alias indirect}.include?(systemd_unit_status["UnitFileState"]) end def masked? - systemctl_execute("status", new_resource.unit_name).stdout.include?("masked") + # Note: masked-runtime is excluded, because runtime is volatile, and + # because masked-runtime is not masked. + systemd_unit_status["UnitFileState"] == "masked" end def static? - systemctl_execute("is-enabled", new_resource.unit_name).stdout.include?("static") + systemd_unit_status["UnitFileState"] == "static" end def indirect? - systemctl_execute("is-enabled", new_resource.unit_name).stdout.include?("indirect") + systemd_unit_status["UnitFileState"] == "indirect" end private diff --git a/spec/unit/provider/systemd_unit_spec.rb b/spec/unit/provider/systemd_unit_spec.rb index d31d5f0919..bc64fe4175 100644 --- a/spec/unit/provider/systemd_unit_spec.rb +++ b/spec/unit/provider/systemd_unit_spec.rb @@ -117,6 +117,7 @@ describe Chef::Provider::SystemdUnit, :linux_only do allow(provider).to receive(:enabled?).and_return(false) allow(provider).to receive(:masked?).and_return(false) allow(provider).to receive(:static?).and_return(false) + allow(provider).to receive(:indirect?).and_return(false) end it "should create a current resource with the name of the new resource" do @@ -811,6 +812,49 @@ describe Chef::Provider::SystemdUnit, :linux_only do end end + def with_systemctl_show(systemctl_path, instance, opts, stdout) + systemctl_show = [systemctl_path, instance, "show", "-p", "UnitFileState", "-p", "ActiveState", unit_name] + expect(provider).to receive(:shell_out).with(*systemctl_show, opts).and_return(double(stdout: stdout, exitstatus: 0, error?: false)) + end + + describe "systemd_unit_status" do + before(:each) do + provider.current_resource = current_resource + current_resource.unit_name(unit_name) + end + + it "should return status if '#{systemctl_path} --system show -p UnitFileState -p ActiveState unit_name' returns 0 and has nil" do + # No unit known for this service, and inactive + nil_and_inactive = <<-STDOUT + ActiveState=inactive + UnitFileState= + STDOUT + nil_and_inactive_h = { + "ActiveState" => "inactive", + "UnitFileState" => nil, + } + with_systemctl_show(systemctl_path, "--system", {}, nil_and_inactive) + expect(provider.systemd_unit_status).to eql(nil_and_inactive_h) + end + + it "should not error if '#{systemctl_path} --system show' is run against a template unit" do + current_resource.unit_name("foo@.service") + template_error = "Failed to get properties: Unit name foo@.service is neither a valid invocation ID nor unit name." + systemctl_show = [systemctl_path, "--system", "show", "-p", "UnitFileState", "-p", "ActiveState", "foo@.service"] + expect(provider).to receive(:shell_out).with(*systemctl_show, {}).and_return(double(stdout: "", stderr: template_error, exitstatus: 1, error?: true)) + expect(provider.systemd_unit_status).to eql({}) + end + end + + enabled_and_active = <<-STDOUT + ActiveState=active + UnitFileState=enabled + STDOUT + disabled_and_inactive = <<-STDOUT + ActiveState=disabled + UnitFileState=inactive + STDOUT + describe "#active?" do before(:each) do provider.current_resource = current_resource @@ -820,33 +864,25 @@ describe Chef::Provider::SystemdUnit, :linux_only do context "when a user is specified" do it "returns true when unit is active" do current_resource.user(user_name) - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--user", "is-active", unit_name, user_cmd_opts) - .and_return(shell_out_success) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, enabled_and_active) expect(provider.active?).to be true end it "returns false when unit is inactive" do current_resource.user(user_name) - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--user", "is-active", unit_name, user_cmd_opts) - .and_return(shell_out_failure) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, disabled_and_inactive) expect(provider.active?).to be false end end context "when no user is specified" do it "returns true when unit is active" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "is-active", unit_name) - .and_return(shell_out_success) + with_systemctl_show(systemctl_path, "--system", {}, enabled_and_active) expect(provider.active?).to be true end it "returns false when unit is not active" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "is-active", unit_name) - .and_return(shell_out_failure) + with_systemctl_show(systemctl_path, "--system", {}, disabled_and_inactive) expect(provider.active?).to be false end end @@ -861,33 +897,25 @@ describe Chef::Provider::SystemdUnit, :linux_only do context "when a user is specified" do it "returns true when unit is 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_success) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, enabled_and_active) expect(provider.enabled?).to be true end 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_disabled) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, disabled_and_inactive) expect(provider.enabled?).to be false end end context "when no user is specified" do it "returns true when unit is enabled" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "is-enabled", unit_name) - .and_return(shell_out_success) + with_systemctl_show(systemctl_path, "--system", {}, enabled_and_active) expect(provider.enabled?).to be true end 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_disabled) + with_systemctl_show(systemctl_path, "--system", {}, disabled_and_inactive) expect(provider.enabled?).to be false end end @@ -899,36 +927,33 @@ describe Chef::Provider::SystemdUnit, :linux_only do allow(provider).to receive(:which).with("systemctl").and_return(systemctl_path.to_s) end + masked_and_inactive = <<-STDOUT + ActiveState=inactive + UnitFileState=masked + STDOUT + context "when a user is specified" do it "returns true when the unit is masked" do current_resource.user(user_name) - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--user", "status", unit_name, user_cmd_opts) - .and_return(shell_out_masked) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, masked_and_inactive) expect(provider.masked?).to be true end it "returns false when the unit is not masked" do current_resource.user(user_name) - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--user", "status", unit_name, user_cmd_opts) - .and_return(shell_out_static) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, enabled_and_active) expect(provider.masked?).to be false end end context "when no user is specified" do it "returns true when the unit is masked" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "status", unit_name) - .and_return(shell_out_masked) + with_systemctl_show(systemctl_path, "--system", {}, masked_and_inactive) expect(provider.masked?).to be true end it "returns false when the unit is not masked" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "status", unit_name) - .and_return(shell_out_static) + with_systemctl_show(systemctl_path, "--system", {}, enabled_and_active) expect(provider.masked?).to be false end end @@ -940,36 +965,33 @@ describe Chef::Provider::SystemdUnit, :linux_only do allow(provider).to receive(:which).with("systemctl").and_return(systemctl_path.to_s) end + static_and_active = <<-STDOUT + ActiveState=active + UnitFileState=static + STDOUT + context "when a user is specified" do it "returns true when the unit is static" 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) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, static_and_active) expect(provider.static?).to be true end it "returns false when the unit is not static" 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_masked) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, enabled_and_active) expect(provider.static?).to be false end end context "when no user is specified" do it "returns true when the unit is static" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "is-enabled", unit_name) - .and_return(shell_out_static) + with_systemctl_show(systemctl_path, "--system", {}, static_and_active) expect(provider.static?).to be true end it "returns false when the unit is not static" do - expect(provider).to receive(:shell_out_compacted) - .with(systemctl_path, "--system", "is-enabled", unit_name) - .and_return(shell_out_masked) + with_systemctl_show(systemctl_path, "--system", {}, enabled_and_active) expect(provider.static?).to be false end end @@ -981,36 +1003,33 @@ describe Chef::Provider::SystemdUnit, :linux_only do allow(provider).to receive(:which).with("systemctl").and_return(systemctl_path.to_s) end + indirect_and_inactive = <<-STDOUT + ActiveState=inactive + UnitFileState=indirect + STDOUT + 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) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, indirect_and_inactive) 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) + with_systemctl_show(systemctl_path, "--user", user_cmd_opts, enabled_and_active) 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) + with_systemctl_show(systemctl_path, "--system", {}, indirect_and_inactive) 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) + with_systemctl_show(systemctl_path, "--system", {}, enabled_and_active) expect(provider.indirect?).to be false end end |