Speed up graph && node processing

- in case of big number of node (more then 200) and tasks
(more then 20000), progress calculation can slow down
- remove status magent call from puppet run (decrease number
  of magent calls from 2 to 1 in case of positive scenario)

Change-Id: I70675a6bbd391d0112c594626bdb0ce7bb9e3e1e

lib/astute/mclients/puppet_mclient.rb

@@ -19,22 +19,12 @@ module Astute
       'running', 'stopped', 'disabled'
     ]
 
-    ALLOWING_STATUSES_TO_RUN = [
-      'stopped', 'succeed'
-    ]
-
-    PUPPET_FILES_TO_CLEANUP = [
-      "/var/lib/puppet/state/last_run_summary.yaml",
-      "/var/lib/puppet/state/last_run_report.yaml"
-    ]
-
     attr_reader :summary, :node_id
 
-    def initialize(ctx, node_id, options, shell_mclient)
+    def initialize(ctx, node_id, options)
       @ctx = ctx
       @node_id = node_id
       @options = options
-      @shell_mclient = shell_mclient
       @summary = {}
     end
 
@@ -48,20 +38,11 @@ module Astute
     # Run puppet on node if available
     # @return [true, false]
     def run
-      actual_status = status
-      if ALLOWING_STATUSES_TO_RUN.include?(actual_status)
-        cleanup!
-        runonce
-        return true
-      end
+      is_succeed, err_msg = runonce
+      return true if is_succeed
 
-      Astute.logger.warn "Unable to start puppet on node #{@node_id}"\
-        " because of status: '#{actual_status}'. Expected: "\
-        "#{ALLOWING_STATUSES_TO_RUN}"
-      false
-    rescue MClientError, MClientTimeout => e
-      Astute.logger.warn "Unable to start puppet on node #{@node_id}. "\
-        "Reason: #{e.message}"
+      Astute.logger.warn "Fail to start puppet on node #{@node_id}. "\
+        "Reason: #{err_msg}"
       false
     end
 
@@ -109,22 +90,24 @@ module Astute
     end
 
     # Run runonce action using mcollective puppet agent
-    # @return [void]
-    # @raise [MClientTimeout, MClientError]
+    # @return [[true, false], String] boolean status of run and error message
     def runonce
-      puppetd.runonce(
+      result = puppetd.runonce(
         :puppet_debug => @options['puppet_debug'],
         :manifest => @options['puppet_manifest'],
         :modules  => @options['puppet_modules'],
         :cwd => @options['cwd'],
         :puppet_noop_run => @options['puppet_noop_run'],
-      )
+      ).first
+      return result[:statuscode] == 0, result[:statusmsg]
+    rescue MClientError, MClientTimeout => e
+      return false, e.message
     end
 
     # Validate puppet status
     # @param [String] status The puppet status
     # @return [void]
-    # @raise [StandardError] Unknown status
+    # @raise [MClientError] Unknown status
     def validate_status!(status)
       unless PUPPET_STATUSES.include?(status)
         raise MClientError, "Unknow status '#{status}' from mcollective agent"
@@ -142,12 +125,6 @@ module Astute
       @summary[:resources]['failed_to_restart'].to_i == 0
     end
 
-    # Clean up puppet report files to exclude incorrect status detection
-    # @return [void]
-    def cleanup!
-      @shell_mclient.run_without_check(rm_cmd)
-    end
-
     # Generate shell cmd for file deletion
     # @return [String] shell cmd for deletion
     def rm_cmd

lib/astute/puppet_job.rb

@@ -115,7 +115,7 @@ module Astute
     # Setup task status
     # @param [String] status The task status
     # @return [void]
-    # @raise [StandardError] Unknown status
+    # @raise [StatusValidationError] Unknown job status
     def task_status=(status)
       if JOB_TASK_STATUSES.include?(status)
         @task_status = status
@@ -162,7 +162,7 @@ module Astute
     # Convert puppet status to task status
     # @param [String] puppet_status The puppet status of task
     # @return [String] Task status
-    # @raise [StandardError] Unknown status
+    # @raise [StatusValidationError] Unknown puppet status
     def puppet_to_task_status(mco_puppet_status)
       case
       when SUCCEED_STATUSES.include?(mco_puppet_status)

lib/astute/task_node.rb

@@ -43,7 +43,6 @@ module Astute
           'nodes' => [{
             'uid' => uid,
             'deployment_graph_task_name' => task.name,
-            'progress' => current_progress_bar,
             'task_status' => task.status.to_s,
           }]
         })

lib/astute/tasks/puppet.rb

@@ -63,7 +63,6 @@ module Astute
           @ctx,
           @task['node_id'],
           @task['parameters'],
-          ShellMClient.new(@ctx, @task['node_id'])
         ),
         {
           'retries' => @task['parameters']['retries'],

spec/unit/mclients/puppet_mclient_spec.rb

@@ -31,11 +31,6 @@ describe Astute::PuppetMClient do
 
   let(:node_id) { 'test_id' }
   let(:puppetd) { mock('puppet_mclient') }
-  let(:shell_mclient) do
-    shell_mclient = mock('shell_mclient')
-    shell_mclient.stubs(:run_without_check)
-    shell_mclient
-  end
 
   let(:options) do
     {
@@ -116,7 +111,7 @@ describe Astute::PuppetMClient do
   end
 
   subject do
-    Astute::PuppetMClient.new(ctx, node_id, options, shell_mclient)
+    Astute::PuppetMClient.new(ctx, node_id, options)
   end
 
   describe '#summary' do
@@ -200,69 +195,51 @@ describe Astute::PuppetMClient do
     end
 
     it 'should return true if happened to start' do
-      subject.expects(:status).returns('stopped')
       puppetd.expects(:runonce).with(
         :puppet_debug => options['puppet_debug'],
         :manifest => options['puppet_manifest'],
         :modules  => options['puppet_modules'],
         :cwd => options['cwd'],
         :puppet_noop_run => options['puppet_noop_run'],
-      )
+      ).returns([:statuscode => 0])
 
       expect(subject.run).to be true
     end
 
     context 'should return false if could not start' do
       it 'if another puppet still running' do
-        subject.expects(:status).returns('running')
-        puppetd.expects(:runonce).never
+        puppetd.expects(:runonce).returns([
+          :statuscode => 1,
+          :statusmsg => 'Lock file and PID file exist; puppet is running.'
+        ])
 
         expect(subject.run).to be false
       end
 
       it 'if puppet was disabled' do
-        subject.expects(:status).returns('disabled')
-        puppetd.expects(:runonce).never
+        puppetd.expects(:runonce).returns([
+          :statuscode => 1,
+          :statusmsg => 'Empty Lock file exists; puppet is disabled.'
+        ])
 
         expect(subject.run).to be false
       end
 
-      it 'if puppet status unknow' do
-        subject.expects(:status).returns('undefined')
-        puppetd.expects(:runonce).never
+      it 'if puppet status unknown' do
+        puppetd.expects(:runonce).returns([
+          :statuscode => 1,
+          :statusmsg => 'Unknown puppet status: unknown'
+        ])
 
         expect(subject.run).to be false
       end
     end
 
     it 'should return false if magent raise error' do
-      subject.expects(:status).returns('stopped')
       puppetd.expects(:runonce).raises(Astute::MClientError, "Custom error")
 
       expect(subject.run).to be false
     end
-
-    context 'should cleanup puppet report files before start' do
-      it 'if puppet was stopped' do
-        subject.stubs(:status).returns('stopped')
-        subject.stubs(:runonce)
-        shell_mclient.unstub(:run_without_check)
-        shell_mclient.expects(:run_without_check)
-          .with('rm -f /var/lib/puppet/state/last_run_summary.yaml'\
-                ' && rm -f /var/lib/puppet/state/last_run_report.yaml').once
-        expect(subject.run).to be true
-      end
-
-      it 'if puppet was succeed' do
-        subject.stubs(:status).returns('succeed')
-        subject.stubs(:runonce)
-        shell_mclient.unstub(:run_without_check)
-        shell_mclient.expects(:run_without_check)
-          .with('rm -f /var/lib/puppet/state/last_run_summary.yaml'\
-                ' && rm -f /var/lib/puppet/state/last_run_report.yaml').once
-        expect(subject.run).to be true
-      end
-    end
   end # run
 
 end

spec/unit/task_node_spec.rb

@@ -300,15 +300,15 @@ describe Astute::TaskNode do
         end
       end
 
-      it 'should report progress if task running' do
+      it 'should report task status if task running' do
         Astute::Puppet.any_instance.expects(:status).returns(:running)
         task_node.run(task)
         ctx.expects(:report).with({
           'nodes' => [{
             'uid' => 'node_id',
             'deployment_graph_task_name' => task.name,
-            'task_status' => 'running',
-            'progress' => 0}]
+            'task_status' => 'running'
+          }]
         })
         task_node.poll
       end