From 5104059cb3a7143a472b62be4121310fba06bb8d Mon Sep 17 00:00:00 2001 From: Christopher Thorn Date: Tue, 14 Nov 2023 13:29:45 -0800 Subject: [PATCH] (PE-36365) Puppet resource service should error if service is not present Previously if a service was not present on a system and a user tried to manage that service with `puppet resource service`, and that service wasn't there an error message would appear on the screen but the puppet command would return a 0 exit code. This commit is meant to bubble up those error messages so they will be reflected on the command line properly. First pass will be to update the Windows and Systemd providers. blank --- lib/puppet/.DS_Store | Bin 0 -> 6148 bytes lib/puppet/provider/service/windows.rb | 3 +- lib/puppet/type/service.rb | 4 +++ spec/unit/application/resource_spec.rb | 11 +++++- spec/unit/provider/service/systemd_spec.rb | 40 +++++++++++++++++---- spec/unit/provider/service/windows_spec.rb | 4 +-- spec/unit/type/service_spec.rb | 6 ++++ 7 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 lib/puppet/.DS_Store diff --git a/lib/puppet/.DS_Store b/lib/puppet/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..096e719b0e184a6b787cc71828ac2c5d08f70969 GIT binary patch literal 6148 zcmeH~F$w}f3`G;&V!>uh%V|7-Hy9Q@ffuk)Y(zy=u$!a%lL>;WwTS#c@+X-I%f4b~ zBO=;gH*=9rL|VA1%q$E{kvDRYyPRZuTb&R4<6(!I)kksG*6>aS`>{w?k8$%b}%eZ5NH {'param' => 'temp'}}).and_return(@res) + resource_status = instance_double('Puppet::Resource::Status') + allow(@report).to receive(:resource_statuses).and_return({'type/name' => resource_status}) + allow(resource_status).to receive(:failed?).and_return(false) @resource_app.main end end @@ -140,11 +147,13 @@ def exists? true end + def string=(value) + end + def string Puppet::Util::Execution::ProcessOutput.new('test', 0) end end - it "should not emit puppet class tags when printing yaml when strict mode is off" do Puppet[:strict] = :warning diff --git a/spec/unit/provider/service/systemd_spec.rb b/spec/unit/provider/service/systemd_spec.rb index 3f5d5c5f377..2521790fc0a 100644 --- a/spec/unit/provider/service/systemd_spec.rb +++ b/spec/unit/provider/service/systemd_spec.rb @@ -388,19 +388,42 @@ # Note: systemd provider does not care about hasstatus or a custom status # command. I just assume that it does not make sense for systemd. describe "#status" do - it "should return running if the command returns 0" do - provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd.service')) - expect(provider).to receive(:execute) - .with(['/bin/systemctl','is-active', '--', 'sshd.service'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) - .and_return(Puppet::Util::Execution::ProcessOutput.new("active\n", 0)) + it 'when called on a service that does not exist returns absent' do + provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesnotexist.service')) + expect(provider).to receive(:exist?).and_return(false) + expect(provider.status).to eq(:absent) + end + + it 'when called on a service that does exist and is running returns running' do + provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesexist.service')) + expect(provider).to receive(:execute). + with(['/bin/systemctl','cat', '--', 'doesexist.service'], {:failonfail=>false}). + and_return(Puppet::Util::Execution::ProcessOutput.new("# /lib/systemd/system/doesexist.service\n...", 0)).once + expect(provider).to receive(:execute). + with(['/bin/systemctl','is-active', '--', 'doesexist.service'], {:combine=>true, :failonfail=>false, :override_locale=>false, :squelch=>false}). + and_return(Puppet::Util::Execution::ProcessOutput.new("# /lib/systemd/system/doesexist.service\n...", 0)).once expect(provider.status).to eq(:running) end + it 'when called on a service that does exist and is not running returns stopped' do + provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesexist.service')) + expect(provider).to receive(:execute). + with(['/bin/systemctl','cat', '--', 'doesexist.service'], {:failonfail=>false}). + and_return(Puppet::Util::Execution::ProcessOutput.new("# /lib/systemd/system/doesexist.service\n...", 0)).once + expect(provider).to receive(:execute). + with(['/bin/systemctl','is-active', '--', 'doesexist.service'], {:combine=>true, :failonfail=>false, :override_locale=>false, :squelch=>false}). + and_return(Puppet::Util::Execution::ProcessOutput.new("inactive\n", 3)).once + expect(provider.status).to eq(:stopped) + end + [-10,-1,3,10].each { |ec| - it "should return stopped if the command returns something non-0" do + it "should return stopped if the command returns something non-0" do provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd.service')) + expect(provider).to receive(:execute). + with(['/bin/systemctl','cat', '--', 'sshd.service'], {:failonfail=>false}). + and_return(Puppet::Util::Execution::ProcessOutput.new("# /lib/systemd/system/sshd.service\n...", 0)).once expect(provider).to receive(:execute) - .with(['/bin/systemctl','is-active', '--', 'sshd.service'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) + .with(['/bin/systemctl','is-active', '--', 'sshd.service'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) .and_return(Puppet::Util::Execution::ProcessOutput.new("inactive\n", ec)) expect(provider.status).to eq(:stopped) end @@ -408,6 +431,9 @@ it "should use the supplied status command if specified" do provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd.service', :status => '/bin/foo')) + expect(provider).to receive(:execute). + with(['/bin/systemctl','cat', '--', 'sshd.service'], {:failonfail=>false}). + and_return(Puppet::Util::Execution::ProcessOutput.new("# /lib/systemd/system/sshd.service\n...", 0)).once expect(provider).to receive(:execute) .with(['/bin/foo'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true}) .and_return(process_output) diff --git a/spec/unit/provider/service/windows_spec.rb b/spec/unit/provider/service/windows_spec.rb index 47a71254696..0a04bd07064 100644 --- a/spec/unit/provider/service/windows_spec.rb +++ b/spec/unit/provider/service/windows_spec.rb @@ -81,10 +81,10 @@ end describe "#status" do - it "should report a nonexistent service as stopped" do + it "should report a nonexistent service as absent" do allow(service_util).to receive(:exists?).with(resource[:name]).and_return(false) - expect(provider.status).to eql(:stopped) + expect(provider.status).to eql(:absent) end it "should report service as stopped when status cannot be retrieved" do diff --git a/spec/unit/type/service_spec.rb b/spec/unit/type/service_spec.rb index 0b55e0997be..e60829ce53a 100644 --- a/spec/unit/type/service_spec.rb +++ b/spec/unit/type/service_spec.rb @@ -67,6 +67,12 @@ def safely_load_service_type expect(svc.should(:ensure)).to eq(:stopped) end + describe 'the absent property' do + it 'should fail validate if it is a service' do + expect { Puppet::Type.type(:service).new(:name => "service_name", :absent => 'sure') }.to raise_error(Puppet::Error) + end + end + describe "the enable property" do before :each do allow(@provider.class).to receive(:supports_parameter?).and_return(true)