(PE-36365) Puppet resource service should error if service is not present

Previously if a service was not present on a system and a user tried to manage that
service with `puppet resource service`, and that service wasn't there an error message
would appear on the screen but the puppet command would return a 0 exit code.
This commit is meant to bubble up those error messages so they will be reflected on the command
line properly.
First pass will be to update the Windows and Systemd providers.

blank

Author: cthorn42

lib/puppet/application/resource.rb

@@ -237,6 +237,7 @@ def find_or_save_resources(type, name, params)
           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

lib/puppet/provider/service/windows.rb

@@ -92,7 +92,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

@@ -111,6 +111,10 @@ def insync?(current)
       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)