Skip to content

Maint/dnm/systemd service not found parameter #9256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added lib/puppet/.DS_Store
Binary file not shown.
7 changes: 5 additions & 2 deletions lib/puppet/application/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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? && !Puppet.settings[:fail_if_resource_service_not_found].nil? && Puppet.settings[:fail_if_resource_service_not_found]

[save_result]
end
else
if type == "file"
Expand Down
6 changes: 6 additions & 0 deletions lib/puppet/defaults.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ def self.initialize_default_settings!(settings)
directory, but if it's running as any other user, it defaults to being
in the user's home directory.",
},
:fail_if_resource_service_not_found => {
:default => false,
:type => :boolean,
:desc => "Fails the `puppet resource service` check on a given service if the service
is not found. Defaults to false",
},
Copy link
Contributor

@joshcooper joshcooper Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the behavior only affects the puppet resource application, I think making this an application option makes sense. Also could then be shorted to fail-when-not-found or similar? Also reminds me of the curl --fail option (to return non-zero exit code when the server returns HTTP 5xx).

:vardir => {
:default => nil,
:type => :directory,
Expand Down
14 changes: 14 additions & 0 deletions lib/puppet/provider/service/systemd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/provider/service/windows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/puppet/type/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
11 changes: 10 additions & 1 deletion spec/unit/application/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
62 changes: 56 additions & 6 deletions spec/unit/provider/service/systemd_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,26 +389,76 @@
# 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

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'))
Puppet.settings[:fail_if_resource_service_not_found]
expect(provider).to receive(:execute).
with(['/bin/systemctl','cat', '--', 'doesnotexist.service'], {:failonfail=>false}).
and_return(Puppet::Util::Execution::ProcessOutput.new("No files found for doesnotexist.service.\n", 1))
expect(provider.status).to eq(:absent)
end

# it 'when called on a service that does not exist returns stopped when no-fail_if_resource_service_not_found option used' do
# provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'doesnotexist.service'))
#
# Open question, how do i set the negative of fail_if_resource_service_not_found
#
# Puppet.settings[:no-fail_if_resource_service_not_found]
# expect(provider).to receive(:execute).
# with(['/bin/systemctl','cat', '--', 'doesnotexist.service'], {:failonfail=>false}).
# and_return(Puppet::Util::Execution::ProcessOutput.new("No files found for doesnotexist.service.\n", 1))
# 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
}

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)
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/provider/service/windows_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions spec/unit/type/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down