Merge pull request #9198 from puppetlabs/revert-9175-bug_fix/main/error_when_resource_not_found

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

Author: cthorn42

Diff for: lib/puppet/application/resource.rb

@@ -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"

Diff for: lib/puppet/provider/service/systemd.rb

@@ -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)

Diff for: lib/puppet/provider/service/windows.rb

@@ -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

Diff for: lib/puppet/type/service.rb

@@ -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)
 

Diff for: spec/unit/application/resource_spec.rb

@@ -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
@@ -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
 

Diff for: spec/unit/provider/service/systemd_spec.rb

@@ -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)

Diff for: spec/unit/provider/service/windows_spec.rb

@@ -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

Diff for: spec/unit/type/service_spec.rb

@@ -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)