From bd929dd3c6255e9fcde28b8eb4e5fcfbb210e59f Mon Sep 17 00:00:00 2001 From: Christopher Thorn Date: Tue, 20 Feb 2024 10:42:24 -0800 Subject: [PATCH 1/2] Revert "Revert "SystemD and Windows resources when not resource found will now return error"" This reverts commit 1818d78c1024a2452eea82ea1aca83efe5be4ece. --- lib/puppet/application/resource.rb | 7 ++-- lib/puppet/provider/service/systemd.rb | 14 ++++++++ lib/puppet/provider/service/windows.rb | 2 +- lib/puppet/type/service.rb | 6 ++++ spec/unit/application/resource_spec.rb | 11 ++++++- spec/unit/provider/service/systemd_spec.rb | 38 ++++++++++++++++++---- spec/unit/provider/service/windows_spec.rb | 4 +-- spec/unit/type/service_spec.rb | 6 ++++ 8 files changed, 76 insertions(+), 12 deletions(-) diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb index 80a54d09946..5e4e812e0ca 100644 --- a/lib/puppet/application/resource.rb +++ b/lib/puppet/application/resource.rb @@ -236,8 +236,11 @@ def find_or_save_resources(type, name, params) resource = Puppet::Resource.new(type, name, :parameters => params) # save returns [resource that was saved, transaction log from applying the resource] - save_result = Puppet::Resource.indirection.save(resource, key) - [save_result.first] + save_result, report = Puppet::Resource.indirection.save(resource, key) + status = report.resource_statuses[resource.ref] + raise "Failed to manage resource #{resource.ref}" if status&.failed? + + [save_result] end else if type == "file" diff --git a/lib/puppet/provider/service/systemd.rb b/lib/puppet/provider/service/systemd.rb index b1961a3cefa..da54520faec 100644 --- a/lib/puppet/provider/service/systemd.rb +++ b/lib/puppet/provider/service/systemd.rb @@ -165,6 +165,20 @@ def daemon_reload? end end + # override base#status + def status + if exist? + status = service_command(:status, false) + if status.exitstatus == 0 + return :running + else + return :stopped + end + else + return :absent + end + end + def enable self.unmask systemctl_change_enable(:enable) diff --git a/lib/puppet/provider/service/windows.rb b/lib/puppet/provider/service/windows.rb index 102c7f0986a..2dfc63e369b 100644 --- a/lib/puppet/provider/service/windows.rb +++ b/lib/puppet/provider/service/windows.rb @@ -90,7 +90,7 @@ def stop end def status - return :stopped unless Puppet::Util::Windows::Service.exists?(@resource[:name]) + return :absent unless Puppet::Util::Windows::Service.exists?(@resource[:name]) current_state = Puppet::Util::Windows::Service.service_state(@resource[:name]) state = case current_state diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb index f012baf9f81..6bd08482086 100644 --- a/lib/puppet/type/service.rb +++ b/lib/puppet/type/service.rb @@ -110,6 +110,12 @@ def insync?(current) provider.start end + newvalue(:absent) + + validate do |val| + fail "Managing absent on a service is not supported" if val.to_s == 'absent' + end + aliasvalue(:false, :stopped) aliasvalue(:true, :running) diff --git a/spec/unit/application/resource_spec.rb b/spec/unit/application/resource_spec.rb index b257696eac8..46078f25e23 100644 --- a/spec/unit/application/resource_spec.rb +++ b/spec/unit/application/resource_spec.rb @@ -118,12 +118,19 @@ @resource_app.main end + before :each do + allow(@res).to receive(:ref).and_return("type/name") + end + it "should add given parameters to the object" do allow(@resource_app.command_line).to receive(:args).and_return(['type','name','param=temp']) expect(Puppet::Resource.indirection).to receive(:save).with(@res, 'type/name').and_return([@res, @report]) expect(Puppet::Resource).to receive(:new).with('type', 'name', {:parameters => {'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 6db6f8aa95e..94fde19f33f 100644 --- a/spec/unit/provider/service/systemd_spec.rb +++ b/spec/unit/provider/service/systemd_spec.rb @@ -389,19 +389,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 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 @@ -409,6 +432,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..c706951aa6a 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", :ensure => :absent) }.to raise_error(Puppet::Error, /Managing absent on a service is not supported/) + end + end + describe "the enable property" do before :each do allow(@provider.class).to receive(:supports_parameter?).and_return(true) From 248dbe58044f17172c2f38638fa984115a51b1e9 Mon Sep 17 00:00:00 2001 From: Christopher Thorn Date: Tue, 20 Feb 2024 10:47:55 -0800 Subject: [PATCH 2/2] Add options for failing resource check on command line --- lib/puppet/application/resource.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb index 5e4e812e0ca..b619d13159c 100644 --- a/lib/puppet/application/resource.rb +++ b/lib/puppet/application/resource.rb @@ -15,6 +15,7 @@ def preinit option("--verbose", "-v") option("--edit", "-e") option("--to_yaml", "-y") + option('--fail', '-f') option("--types", "-t") do |_arg| env = Puppet.lookup(:environments).get(Puppet[:environment]) || create_default_environment @@ -109,6 +110,9 @@ def help Output found resources in yaml format, suitable to use with Hiera and create_resources. + * --fail: + Fails and returns an exit code of 1 if the resource is not found. + EXAMPLE ------- This example uses `puppet resource` to return a Puppet configuration for @@ -238,7 +242,7 @@ def find_or_save_resources(type, name, params) # save returns [resource that was saved, transaction log from applying the resource] save_result, report = Puppet::Resource.indirection.save(resource, key) status = report.resource_statuses[resource.ref] - raise "Failed to manage resource #{resource.ref}" if status&.failed? + raise "Failed to manage resource #{resource.ref}" if status&.failed? && !options[:fail].nil? && options[:fail] [save_result] end