Skip to content
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

Revert "SystemD and Windows resources when not resource found will now return error" #9198

Merged
Merged
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
7 changes: 2 additions & 5 deletions lib/puppet/application/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,8 @@ 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, 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 ]
save_result = Puppet::Resource.indirection.save(resource, key)
[ save_result.first ]
end
else
if type == "file"
Expand Down
14 changes: 0 additions & 14 deletions lib/puppet/provider/service/systemd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,20 +163,6 @@ 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 :absent unless Puppet::Util::Windows::Service.exists?(@resource[:name])
return :stopped 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: 0 additions & 6 deletions lib/puppet/type/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@ 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: 1 addition & 10 deletions spec/unit/application/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,12 @@
@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 @@ -147,13 +140,11 @@ 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
38 changes: 6 additions & 32 deletions spec/unit/provider/service/systemd_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -388,52 +388,26 @@
# 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 '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
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))
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
}

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 absent" do
it "should report a nonexistent service as stopped" do
allow(service_util).to receive(:exists?).with(resource[:name]).and_return(false)

expect(provider.status).to eql(:absent)
expect(provider.status).to eql(:stopped)
end

it "should report service as stopped when status cannot be retrieved" do
Expand Down
6 changes: 0 additions & 6 deletions spec/unit/type/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ 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