Merge pull request #9175 from cthorn42/bug_fix/main/error_when_resource_not_found

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

Author: joshcooper

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"

lib/puppet/provider/service/systemd.rb

@@ -163,6 +163,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)

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

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)
 

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
 

spec/unit/provider/service/systemd_spec.rb

@@ -388,26 +388,52 @@
   # 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
     }
 
     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)

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

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)