Replace AIX unreliable service group parsing mechanism.

Closes #3327
Closes #3248

3 files changed, 40 insertions, 23 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 83ae4c567c..b837de2325 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@
 * [pr#3295](https://github.com/chef/chef/pull/3295): Stop mutating `new_resource.checksum` in file providers.  Fixes some ChecksumMismatch exceptions like [issue#3168](https://github.com/chef/chef/issues/3168)
 * [pr#3320] Sanitize non-UTF8 characters in the node data before doing node.save().  Works around many UTF8 exception issues reported on node.save().
 * Implemented X-Ops-Server-API-Version with a API version of 0, as well as error handling when the Chef server does not support the API version that the client supports.
+* [pr#3327] Fix unreliable AIX service group parsing mechanism.
 
 ## 12.3.0
 
diff --git a/lib/chef/provider/service/aix.rb b/lib/chef/provider/service/aix.rb
index 0aef62c62e..09ed4bbf01 100644
--- a/lib/chef/provider/service/aix.rb
+++ b/lib/chef/provider/service/aix.rb
@@ -91,15 +91,18 @@ class Chef
 
         protected
         def determine_current_status!
-          Chef::Log.debug "#{@new_resource} using lssrc to check the status "
+          Chef::Log.debug "#{@new_resource} using lssrc to check the status"
           begin
-            services = shell_out!("lssrc -a | grep -w #{@new_resource.service_name}").stdout.split("\n")
-            is_resource_group?(services)
-
-            if services.length == 1 && services[0].split(' ').last == "active"
-              @current_resource.running true
-            else
+            if is_resource_group?
+              # Groups as a whole have no notion of whether they're running
               @current_resource.running false
+            else
+              service = shell_out!("lssrc -s #{@new_resource.service_name}").stdout
+              if service.split(' ').last == 'active'
+                @current_resource.running true
+              else
+                @current_resource.running false
+              end
             end
             Chef::Log.debug "#{@new_resource} running: #{@current_resource.running}"
             # ShellOut sometimes throws different types of Exceptions than ShellCommandFailed.
@@ -112,11 +115,9 @@ class Chef
           end
         end
 
-        def is_resource_group? (services)
-          if services.length > 1
-            Chef::Log.debug("#{@new_resource.service_name} is a group")
-            @is_resource_group = true
-          elsif services[0].split(' ')[1] == @new_resource.service_name
+        def is_resource_group?
+          so = shell_out!("lssrc -g #{@new_resource.service_name}")
+          if so.exitstatus == 0
             Chef::Log.debug("#{@new_resource.service_name} is a group")
             @is_resource_group = true
           end
diff --git a/spec/unit/provider/service/aix_service_spec.rb b/spec/unit/provider/service/aix_service_spec.rb
index 796661145b..a0c8bb3407 100644
--- a/spec/unit/provider/service/aix_service_spec.rb
+++ b/spec/unit/provider/service/aix_service_spec.rb
@@ -51,22 +51,35 @@ describe Chef::Provider::Service::Aix do
       end
 
       it "current resource is running" do
-        expect(@provider).to receive(:shell_out!).with("lssrc -a | grep -w chef").and_return(@status)
-        expect(@provider).to receive(:is_resource_group?).with(["chef chef 12345 active"])
+        expect(@provider).to receive(:shell_out!).with("lssrc -s chef").and_return(@status)
+        expect(@provider).to receive(:is_resource_group?).and_return false
 
         @provider.load_current_resource
         expect(@current_resource.running).to be_truthy
       end
     end
 
-    context "when the service is inoprative" do
+    context "when the service is inoperative" do
       before do
         @status = double("Status", :exitstatus => 0, :stdout => "chef chef inoperative\n")
       end
 
       it "current resource is not running" do
-        expect(@provider).to receive(:shell_out!).with("lssrc -a | grep -w chef").and_return(@status)
-        expect(@provider).to receive(:is_resource_group?).with(["chef chef inoperative"])
+        expect(@provider).to receive(:shell_out!).with("lssrc -s chef").and_return(@status)
+        expect(@provider).to receive(:is_resource_group?).and_return false
+
+        @provider.load_current_resource
+        expect(@current_resource.running).to be_falsey
+      end
+    end
+
+    context "when there is no such service" do
+      before do
+        @status = double("Status", :exitstatus => 1, :stdout => "0513-085 The chef Subsystem is not on file.\n")
+      end
+      it "current resource is not running" do
+        expect(@provider).to receive(:shell_out!).with("lssrc -s chef").and_return(@status)
+        expect(@provider).to receive(:is_resource_group?).and_return false
 
         @provider.load_current_resource
         expect(@current_resource.running).to be_falsey
@@ -75,13 +88,13 @@ describe Chef::Provider::Service::Aix do
   end
 
   describe "is resource group" do
-    context "when there are mutiple subsystems associated with group" do
+    context "when there are multiple subsystems associated with group" do
       before do
         @status = double("Status", :exitstatus => 0, :stdout => "chef1 chef 12345 active\nchef2 chef 12334 active\nchef3 chef inoperative")
       end
 
       it "service is a group" do
-        expect(@provider).to receive(:shell_out!).with("lssrc -a | grep -w chef").and_return(@status)
+        expect(@provider).to receive(:shell_out!).with("lssrc -g chef").and_return(@status)
         @provider.load_current_resource
         expect(@provider.instance_eval("@is_resource_group")).to be_truthy
       end
@@ -93,19 +106,21 @@ describe Chef::Provider::Service::Aix do
       end
 
       it "service is a group" do
-        expect(@provider).to receive(:shell_out!).with("lssrc -a | grep -w chef").and_return(@status)
+        expect(@provider).to receive(:shell_out!).with("lssrc -g chef").and_return(@status)
         @provider.load_current_resource
         expect(@provider.instance_eval("@is_resource_group")).to be_truthy
       end
     end
 
-    context "when there service is a subsytem" do
+    context "when the service is a subsystem" do
       before do
-        @status = double("Status", :exitstatus => 0, :stdout => "chef chef123 inoperative\n")
+        @group_status = double("Status", :exitstatus => 1, :stdout => "0513-086 The chef Group is not on file.\n")
+        @service_status = double("Status", :exitstatus => 0, :stdout => "chef chef inoperative\n")
       end
 
       it "service is a subsystem" do
-        expect(@provider).to receive(:shell_out!).with("lssrc -a | grep -w chef").and_return(@status)
+        expect(@provider).to receive(:shell_out!).with("lssrc -g chef").and_return(@group_status)
+        expect(@provider).to receive(:shell_out!).with("lssrc -s chef").and_return(@service_status)
         @provider.load_current_resource
         expect(@provider.instance_eval("@is_resource_group")).to be_falsey
       end