Infra upgrade improvement

.sync.yml

@@ -14,6 +14,8 @@ Rakefile:
   changelog_since_tag: '2.1.0'
   extras:
     - 'PuppetSyntax.exclude_paths = ["plans/**/*.pp", "vendor/**/*"]'
+spec/spec_helper.rb:
+    mock_with: ':rspec'
 .gitignore:
   paths:
     - '.rerun.json'

CHANGELOG.md

@@ -5,7 +5,7 @@
 
 ### Improvements
 
-- Handle exit code 11 from replica upgrade (still waiting for PuppetDB sync to complete) gracefully.
+- Handle exit code 11 from replica upgrade task gracefully. Code 11 means "PuppetDB sync in progress but not yet complete".
 
 ## 2.4.5
 ### Summary

metadata.json

@@ -62,7 +62,7 @@
       "version_requirement": ">= 6.0.2 < 7.0.0"
     }
   ],
-  "pdk-version": "1.18.0",
+  "pdk-version": "1.18.1",
   "template-url": "https://github.com/puppetlabs/pdk-templates.git#1.18.0",
   "template-ref": "tags/1.18.0-0-g095317c"
 }

spec/spec_helper.rb

@@ -1,5 +1,9 @@
 # frozen_string_literal: true
 
+RSpec.configure do |c|
+  c.mock_with :rspec
+end
+
 require 'puppetlabs_spec_helper/module_spec_helper'
 require 'rspec-puppet-facts'
 

spec/spec_helper_local.rb

@@ -2,6 +2,10 @@
 
 require 'puppet'
 
+# This environment variable can be read by Ruby Bolt tasks to prevent unwanted
+# auto-execution, enabling easy unit testing.
+ENV["RSPEC_UNIT_TEST_MODE"] ||= "TRUE"
+
 if Gem::Version.new(Puppet.version) >= Gem::Version.new('6.0.0')
   begin
     require 'bolt_spec/plans'

spec/unit/task/puppet_infra_upgrade_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+require_relative '../../../tasks/puppet_infra_upgrade'
+
+describe PEAdm::Task::PuppetInfraUpgrade do
+  context 'replica' do
+    let(:upgrade) do
+      described_class.new('type' => 'replica',
+                          'targets' => ['replica.example.com'],
+                          'wait_until_connected_timeout' => 120)
+    end
+
+    it 'Returns when the command exits with an expected code' do
+      status_dbl_0 = double('Status', :exitstatus => 0)
+      status_dbl_11 = double('Status', :exitstatus => 11)
+      allow(STDOUT).to receive(:puts)
+      allow(upgrade).to receive(:wait_until_connected)
+
+      allow(Open3).to receive(:capture2e).and_return(['hello world', status_dbl_0])
+      expect(upgrade.execute!).to eq(nil)
+
+      allow(Open3).to receive(:capture2e).and_return(['hello world', status_dbl_11])
+      expect(upgrade.execute!).to eq(nil)
+    end
+
+    it 'Exits non-zero when the command exits with an unexpected code' do
+      status_dbl_1 = double('Status', :exitstatus => 1)
+      allow(STDOUT).to receive(:puts)
+      allow(upgrade).to receive(:wait_until_connected)
+      allow(Open3).to receive(:capture2e).and_return(['hello world', status_dbl_1])
+      expect { upgrade.execute! }.to raise_error(SystemExit) do |error|
+        expect(error.status).to eq(1)
+      end
+    end
+  end
+
+  context 'compiler' do
+    let(:upgrade) do
+      described_class.new('type' => 'compiler',
+                          'targets' => ['replica.example.com'],
+                          'wait_until_connected_timeout' => 120)
+    end
+  end
+end

tasks/puppet_infra_upgrade.rb

@@ -8,76 +8,93 @@
 require 'timeout'
 require 'etc'
 
-def main
-  params     = JSON.parse(STDIN.read)
-  type       = params['type']
-  targets    = params['targets']
-  timeout    = params['wait_until_connected_timeout']
-  token_file = params['token_file'] || File.join(Etc.getpwuid.dir, '.puppetlabs', 'token')
+class PEAdm
+  class Task
+    class PuppetInfraUpgrade
+      def initialize(params)
+        @type       = params['type']
+        @targets    = params['targets']
+        @timeout    = params['wait_until_connected_timeout']
+        @token_file = params['token_file']
+      end
 
-  exit 0 if targets.empty?
+      def execute!
+        exit 0 if @targets.empty?
+        token_file = @token_file || File.join(Etc.getpwuid.dir, '.puppetlabs', 'token')
 
-  cmd = ['/opt/puppetlabs/bin/puppet-infrastructure', '--render-as', 'json', 'upgrade']
-  cmd << '--token-file' << token_file unless params['token_file'].nil?
-  cmd << type << targets.join(',')
+        cmd = ['/opt/puppetlabs/bin/puppet-infrastructure', '--render-as', 'json', 'upgrade']
+        cmd << '--token-file' << token_file unless @token_file.nil?
+        cmd << @type << @targets.join(',')
 
-  wait_until_connected(nodes: targets, token_file: token_file, timeout: timeout)
+        wait_until_connected(nodes: @targets, token_file: token_file, timeout: @timeout)
 
-  stdouterr, status = Open3.capture2e(*cmd)
-  puts stdouterr
-  if status.success?
-    exit 0
-  elsif status.exitstatus == 11 # Waiting for PuppetDB sync to complete, but otherwise successful
-    exit 0
-  else
-    exit status.exitstatus
-  end
-end
+        stdouterr, status = Open3.capture2e(*cmd)
+        STDOUT.puts stdouterr
 
-def inventory_uri
-  @inventory_uri ||= URI.parse('https://localhost:8143/orchestrator/v1/inventory')
-end
+        # Exit code 11 indicates PuppetDB sync in progress, just not yet
+        # finished. We consider that success.
+        if [0, 11].include?(status.exitstatus)
+          return
+        else
+          exit status.exitstatus
+        end
+      end
 
-def request_object(nodes:, token_file:)
-  token = File.read(token_file)
-  body = {
-    'nodes' => nodes,
-  }.to_json
+      def inventory_uri
+        @inventory_uri ||= URI.parse('https://localhost:8143/orchestrator/v1/inventory')
+      end
 
-  request = Net::HTTP::Post.new(inventory_uri.request_uri)
-  request['Content-Type'] = 'application/json'
-  request['X-Authentication'] = token
-  request.body = body
+      def request_object(nodes:, token_file:)
+        token = File.read(token_file)
+        body = {
+          'nodes' => nodes,
+        }.to_json
 
-  request
-end
+        request = Net::HTTP::Post.new(inventory_uri.request_uri)
+        request['Content-Type'] = 'application/json'
+        request['X-Authentication'] = token
+        request.body = body
 
-def http_object
-  http = Net::HTTP.new(inventory_uri.host, inventory_uri.port)
-  http.use_ssl = true
-  http.verify_mode = OpenSSL::SSL::VERIFY_NONE
+        request
+      end
 
-  http
-end
+      def http_object
+        http = Net::HTTP.new(inventory_uri.host, inventory_uri.port)
+        http.use_ssl = true
+        http.verify_mode = OpenSSL::SSL::VERIFY_NONE
+
+        http
+      end
 
-def wait_until_connected(nodes:, token_file:, timeout: 120)
-  http = http_object
-  request = request_object(nodes: nodes, token_file: token_file)
-  inventory = {}
-  Timeout.timeout(timeout) do
-    loop do
-      response = http.request(request)
-      raise unless response.is_a? Net::HTTPSuccess
-      inventory = JSON.parse(response.body)
-      break if inventory['items'].all? { |item| item['connected'] }
-      sleep(1)
+      def wait_until_connected(nodes:, token_file:, timeout: 120)
+        http = http_object
+        request = request_object(nodes: nodes, token_file: token_file)
+        inventory = {}
+        Timeout.timeout(timeout) do
+          loop do
+            response = http.request(request)
+            unless response.is_a? Net::HTTPSuccess
+              raise "Unexpected result from orchestrator: #{response.class}\n#{response}"
+            end
+            inventory = JSON.parse(response.body)
+            break if inventory['items'].all? { |item| item['connected'] }
+            sleep(1)
+          end
+        end
+      rescue Timeout::Error
+        raise 'Timed out waiting for nodes to be connected to orchestrator: ' +
+              inventory['items'].reject { |item| item['connected'] }
+                                .map { |item| item['name'] }
+                                .to_s
+      end
     end
   end
-rescue Timeout::Error
-  raise 'Timed out waiting for nodes to be connected to orchestrator: ' +
-        inventory['items'].reject { |item| item['connected'] }
-                          .map { |item| item['name'] }
-                          .to_s
 end
 
-main
+# Run the task unless an environment flag has been set, signaling not to. The
+# environment flag is used to disable auto-execution and enable Ruby unit
+# testing of this task.
+unless ENV['RSPEC_UNIT_TEST_MODE']
+  upgrade = PEAdm::Task::PuppetInfraUpgrade.new(JSON.parse(STDIN.read))
+  upgrade.execute!
+end