diff --git a/lib/astute/mclients/puppet_mclient.rb b/lib/astute/mclients/puppet_mclient.rb index bcfa16c4..ca1975f0 100644 --- a/lib/astute/mclients/puppet_mclient.rb +++ b/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 diff --git a/lib/astute/puppet_job.rb b/lib/astute/puppet_job.rb index f0dbe244..5ab94f24 100644 --- a/lib/astute/puppet_job.rb +++ b/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) diff --git a/lib/astute/task_node.rb b/lib/astute/task_node.rb index 8f6d0771..9203b0bf 100644 --- a/lib/astute/task_node.rb +++ b/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, }] }) diff --git a/lib/astute/tasks/puppet.rb b/lib/astute/tasks/puppet.rb index 9f6da23d..5f9fe9bc 100644 --- a/lib/astute/tasks/puppet.rb +++ b/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'], diff --git a/spec/unit/mclients/puppet_mclient_spec.rb b/spec/unit/mclients/puppet_mclient_spec.rb index a7ff3363..c9a38054 100644 --- a/spec/unit/mclients/puppet_mclient_spec.rb +++ b/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 diff --git a/spec/unit/task_node_spec.rb b/spec/unit/task_node_spec.rb index 33ed297c..6ee7383e 100644 --- a/spec/unit/task_node_spec.rb +++ b/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