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

…sent

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

lib/puppet/provider/service/windows.rb

@@ -92,25 +92,27 @@ def stop
   end
 
   def status
-    return :stopped unless Puppet::Util::Windows::Service.exists?(@resource[:name])
-
-    current_state = Puppet::Util::Windows::Service.service_state(@resource[:name])
-    state = case current_state
-      when :SERVICE_STOPPED,
-           :SERVICE_STOP_PENDING
-        :stopped
-      when :SERVICE_PAUSED,
-           :SERVICE_PAUSE_PENDING
-        :paused
-      when :SERVICE_RUNNING,
-           :SERVICE_CONTINUE_PENDING,
-           :SERVICE_START_PENDING
-        :running
-      else
-        raise Puppet::Error.new(_("Unknown service state '%{current_state}' for service '%{resource_name}'") % { current_state: current_state, resource_name: @resource[:name] })
+    if Puppet::Util::Windows::Service.exists?(@resource[:name])
+      current_state = Puppet::Util::Windows::Service.service_state(@resource[:name])
+      state = case current_state
+        when :SERVICE_STOPPED,
+             :SERVICE_STOP_PENDING
+          :stopped
+        when :SERVICE_PAUSED,
+             :SERVICE_PAUSE_PENDING
+          :paused
+        when :SERVICE_RUNNING,
+             :SERVICE_CONTINUE_PENDING,
+             :SERVICE_START_PENDING
+          :running
+        else
+          raise Puppet::Error.new(_("Unknown service state '%{current_state}' for service '%{resource_name}'") % { current_state: current_state, resource_name: @resource[:name] })
+      end
+      debug("Service #{@resource[:name]} is #{current_state}")
+      state
+    else
+      return :absent
     end
-    debug("Service #{@resource[:name]} is #{current_state}")
-    state
   rescue => detail
     Puppet.warning("Status for service #{@resource[:name]} could not be retrieved: #{detail}")
     :stopped

lib/puppet/type/service.rb

@@ -112,6 +112,10 @@ def insync?(current)
 
       newvalue(:absent)
 
+      validate do |val|
+        fail "Managing absent on a service is not supported" if val.equal?(: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,14 @@ 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,30 +388,24 @@
   # 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(:exist?).and_return(true)
+      expect(provider).to receive(:service_command).with(:status, false).and_return(Puppet::Util::Execution::ProcessOutput.new("running\n", 0))
       expect(provider.status).to eq(:running)
     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','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/foo'], {:failonfail => false, :override_locale => false, :squelch => false, :combine => true})
-        .and_return(process_output)
-      provider.status
+    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(:exist?).and_return(true)
+      expect(provider).to receive(:service_command).with(:status, false).and_return(Puppet::Util::Execution::ProcessOutput.new("stopped\n", 3))
+      expect(provider.status).to eq(:stopped)
     end
   end
 

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", :absent => 'sure') }.to raise_error(Puppet::Error)
+    end
+  end
+
   describe "the enable property" do
     before :each do
       allow(@provider.class).to receive(:supports_parameter?).and_return(true)