Add descriptive error messages to fail_on_transport

This commit updates the `fail_on_transport` function
with an optional `message` parameter that can be
used to supply an error message.

The `peadm::upgrade` and `peadm::subplans::modify_certificate`
plans are also updated to use this parameter to provide
an explanation of why use of the "pcp" transport is resulting
in a failure along with suggestions for how to configure
the plans for success.

Author: Sharpie

REFERENCE.md

@@ -533,7 +533,7 @@ Fails if any nodes have the chosen transport.
 
 Useful for excluding PCP when it's not appopriate
 
-#### `peadm::fail_on_transport(TargetSpec $nodes, String $transport)`
+#### `peadm::fail_on_transport(TargetSpec $nodes, String $transport, String $message)`
 
 Fails if any nodes have the chosen transport.
 

functions/fail_on_transport.pp

@@ -5,12 +5,13 @@
 function peadm::fail_on_transport (
   TargetSpec $nodes,
   String     $transport,
+  String     $message = 'This is not supported.',
 ) {
   $targets = get_targets($nodes)
   $targets.each |$target| {
     if $target.protocol == $transport {
       fail_plan(
-        "${target.name} uses ${transport} transport. This is not supported",
+        "${target.name} uses ${transport} transport: ${message}",
         'unexpected-transport',
         {
           'target'    => $target,

plans/subplans/modify_certificate.pp

@@ -11,10 +11,20 @@
   $target = get_target($targets)
   $primary_target = get_target($primary_host)
 
-  # This plan doesn't work to reissue the master cert over the orchestrator due
-  # to pe-puppetserver needing to restart
   if ($primary_target == $target) {
-    $primary_target.peadm::fail_on_transport('pcp')
+    $primary_target.peadm::fail_on_transport('pcp', @(HEREDOC/n))
+      \nThe "pcp" transport is not available for use with the Primary
+      as peadm::subplans::modify_certificate will cause a restart of the
+      PE Orchestration service.
+
+      Use the "local" transport if running this plan directly from
+      the Primary node, or the "ssh" transport if running this
+      plan from an external Bolt host.
+
+      For information on configuring transports, see:
+
+          https://www.puppet.com/docs/bolt/latest/bolt_transports_reference.html
+      |-HEREDOC
   }
 
   # Figure out some information from the existing certificate

plans/upgrade.pp

@@ -83,7 +83,19 @@
 
   out::message('# Gathering information')
 
-  $primary_target.peadm::fail_on_transport('pcp')
+  $primary_target.peadm::fail_on_transport('pcp', @(HEREDOC/n))
+    \nThe "pcp" transport is not available for use with the Primary
+    as peadm::upgrade will cause a restart of the
+    PE Orchestration service.
+
+    Use the "local" transport if running this plan directly from
+    the Primary node, or the "ssh" transport if running this
+    plan from an external Bolt host.
+
+    For information on configuring transports, see:
+
+        https://www.puppet.com/docs/bolt/latest/bolt_transports_reference.html
+    |-HEREDOC
 
   $platform = run_task('peadm::precheck', $primary_target).first['platform']
 
@@ -141,9 +153,6 @@
   }
 
   peadm::plan_step('preparation') || {
-    # Support for running over the orchestrator transport relies on Bolt being
-    # executed from the primary using the local transport. For now, fail the plan
-    # if the orchestrator is being used for the primary.
     if $download_mode == 'bolthost' {
       # Download the PE tarball on the nodes that need it
       run_plan('peadm::util::retrieve_and_upload', $pe_installer_targets,

spec/functions/fail_on_transport_spec.rb

@@ -27,7 +27,11 @@
   # would require duplicating rspec-puppet code, and that's a far worse sin.
   # rubocop:disable Rspec/NamedSubject
   it 'raises an error when nodes use the specified transport' do
-    expect { subject.execute(nodes, 'pcp') }.to raise_error(Puppet::PreformattedError, %r{target\.example uses pcp transport})
+    expect { subject.execute(nodes, 'pcp') }.to raise_error(Puppet::PreformattedError, %r{target\.example uses pcp transport: This is not supported\.})
+  end
+
+  it 'raises an error with a custom explanation if one is provided' do
+    expect { subject.execute(nodes, 'pcp', 'It would be bad.') }.to raise_error(Puppet::PreformattedError, %r{target\.example uses pcp transport: It would be bad\.})
   end
 
   it 'raises no error when nodes do not use the specified transport' do

spec/plans/subplans/modify_certificate_spec.rb

@@ -39,5 +39,18 @@
         expect(run_plan('peadm::subplans::modify_certificate', params)).to be_ok
       end
     end
+
+    context 'modifying the primary certificate' do
+      it 'fails if the primary is using the PCP transport' do
+        result = run_plan('peadm::subplans::modify_certificate',
+                          { 'targets'          => 'pcp://primary.example',
+                            'primary_host'     => 'pcp://primary.example',
+                            'primary_certname' => 'primary.example' })
+
+        expect(result).not_to be_ok
+        expect(result.value.kind).to eq('unexpected-transport')
+        expect(result.value.msg).to match(%r{The "pcp" transport is not available for use with the Primary})
+      end
+    end
   end
 end

spec/plans/upgrade_spec.rb

@@ -43,4 +43,16 @@ def allow_standard_non_returning_calls
                     'compiler_hosts' => 'compiler',
                     'version' => '2021.7.1')).to be_ok
   end
+
+  it 'fails if the primary uses the pcp transport' do
+    allow_standard_non_returning_calls
+
+    result = run_plan('peadm::upgrade',
+                      'primary_host' => 'pcp://primary.example',
+                      'version' => '2021.7.1')
+
+    expect(result).not_to be_ok
+    expect(result.value.kind).to eq('unexpected-transport')
+    expect(result.value.msg).to match(%r{The "pcp" transport is not available for use with the Primary})
+  end
 end