Merge pull request #336 from puppetlabs/SOLARCH-674

Fix group letter assignments during upgrade

Author: timidri

.github/workflows/test-failover.yaml

@@ -20,6 +20,10 @@ on:
         description: 'Boolean; whether or not to pause for ssh debugging'
         required: true
         default: 'false'
+      log_level:
+        description: 'Bolt log level'
+        required: false
+        default: 'debug'
 
 env:
   HONEYCOMB_WRITEKEY: 7f3c63a70eecc61d635917de46bea4e6
@@ -37,13 +41,12 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        architecture:
-          - "extra-large-with-dr-and-spare-replica"
         version:
           - "${{ github.event.inputs.version }}"
         image:
           - "${{ github.event.inputs.image }}"
-
+        architecture:
+          - "extra-large-with-dr"
     steps:
       - name: 'Start SSH session'
         if: ${{ github.event.inputs.ssh-debugging == 'true' }}
@@ -89,7 +92,7 @@ jobs:
             echo STEP_START=$(date +%s) >> $GITHUB_ENV
           echo ::endgroup::
 
-      - name: 'Provision test cluster (specified architecture with added DR)'
+      - name: 'Provision test cluster (XL with spare replica)'
         timeout-minutes: 15
         run: |
           echo ::group::prepare
@@ -106,13 +109,7 @@ jobs:
                 --modulepath spec/fixtures/modules \
                 provider=provision_service \
                 image=${{ matrix.image }} \
-                architecture=${{ matrix.architecture }}-with-dr
-            buildevents cmd $TRACE_ID $STEP_ID 'bolt task run provision::provision_service' -- \
-              bundle exec bolt bolt task run provision::provision_service \
-                --modulepath spec/fixtures/modules \
-                action=provision
-                platform=${{ matrix.image }} \
-                vars="role: primary"
+                architecture=${{ matrix.architecture }}-and-spare-replica
           echo ::endgroup::
 
           echo ::group::info:request
@@ -136,7 +133,7 @@ jobs:
         timeout-minutes: 120
         run: |
           buildevents cmd $TRACE_ID $STEP_ID 'bolt plan run peadm_spec::install_test_cluster' -- \
-            bundle exec bolt plan run peadm_spec::install_test_cluster \
+            bundle exec bolt plan run peadm_spec::install_test_cluster --log_level ${{ github.event.inputs.log_level }} \
               --inventoryfile spec/fixtures/litmus_inventory.yaml \
               --modulepath spec/fixtures/modules \
               architecture=${{ matrix.architecture }} \
@@ -154,11 +151,9 @@ jobs:
       - name: 'Perform failover'
         run: |
           buildevents cmd $TRACE_ID $STEP_ID 'bolt plan run peadm_spec::perform_failover' -- \
-            bundle exec bolt plan run peadm_spec::perform_failover \
+            bundle exec bolt plan run peadm_spec::perform_failover --log_level ${{ github.event.inputs.log_level }} \
               --inventoryfile spec/fixtures/litmus_inventory.yaml \
-              --modulepath spec/fixtures/modules \
-              platform=${{ matrix.image }} \
-              vars="role: primary"
+              --modulepath spec/fixtures/modules
 
       - name: "Honeycomb: Record falover time"
         if: ${{ always() }}
@@ -178,20 +173,36 @@ jobs:
           done 
           echo "${HOME}/pause absent, continuing workflow."
 
+      - name: Set up yq
+        uses: frenck/action-setup-yq@v1
+        with:
+          version: v4.30.5
+
+      - name: 'Update inventory'
+        run: |
+          # Remove failed primary
+          yq -i 'del(.groups[].targets[] | select(.vars.role == "primary"))' spec/fixtures/litmus_inventory.yaml
+          # Swap primary and replica nodes
+          sed -i.sedbak 's/primary/__tmp__/;s/spare-replica/__tmp2__/;s/replica/primary/;s/__tmp__/replica/;s/__tmp2__/replica/' \
+              spec/fixtures/litmus_inventory.yaml
+          echo ::group::info:inventory
+            sed -e 's/password: .*/password: "[redacted]"/' < spec/fixtures/litmus_inventory.yaml || true
+          echo ::endgroup::
+
       - name: 'Upgrade PE on test cluster'
-        if: ${{ always() && github.event.inputs.version_to_upgrade != '' }}
+        if: ${{ success() && github.event.inputs.version_to_upgrade != '' }}
         timeout-minutes: 120
         run: |
           buildevents cmd $TRACE_ID $STEP_ID 'bolt plan run peadm_spec::upgrade_test_cluster' -- \
-            bundle exec bolt plan run peadm_spec::upgrade_test_cluster \
+            bundle exec bolt plan run peadm_spec::upgrade_test_cluster --log_level ${{ github.event.inputs.log_level }} \
               --inventoryfile spec/fixtures/litmus_inventory.yaml \
               --modulepath spec/fixtures/modules \
-              architecture='extra-large-with-dr' \
+              architecture=${{ matrix.architecture }} \
               download_mode='direct' \
-              version=${{ matrix.version_to_upgrade }}
+              version=${{ github.event.inputs.version_to_upgrade }}
 
       - name: "Honeycomb: Record upgrade time"
-        if: ${{ always() && github.event.inputs.version_to_upgrade != '' }}
+        if: ${{ success() && github.event.inputs.version_to_upgrade != '' }}
         run: |
           echo ::group::honeycomb
             buildevents step $TRACE_ID $STEP_ID $STEP_START 'Upgrade PE on test cluster'

.gitignore

@@ -36,3 +36,4 @@ spec/docker/**/*.tar.gz
 spec/docker/**/*.asc
 spec/docker/**/files/puppet-enterprise*
 spec/docker/.task_cache.json
+.vscode/settings.json

functions/assert_supported_architecture.pp

@@ -33,7 +33,7 @@ function peadm::assert_supported_architecture (
 # lint:ignore:strict_indent
     default: { # Invalid
       out::message(inline_epp(@(HEREDOC)))
-        Invalid architecture! Recieved:
+                Invalid architecture! Received:
           - primary
         <% if $replica_host { -%>
           - primary-replica

functions/assert_supported_pe_version.pp

@@ -12,7 +12,7 @@ function peadm::assert_supported_pe_version (
   if $permit_unsafe_versions {
 # lint:ignore:strict_indent
     warning(@("WARN"/L))
-      WARNING: Permitting unsafe PE versions. This is not supported or tested.
+        WARNING: Permitting unsafe PE versions. This is not supported or tested.
         Proceeding with this action could result in a broken PE Infrastructure.
       | WARN
 # lint:endignore
@@ -21,7 +21,7 @@ function peadm::assert_supported_pe_version (
   if (!$supported and $permit_unsafe_versions) {
 # lint:ignore:strict_indent
     warning(@("WARN"/L))
-      WARNING: PE version ${version} is NOT SUPPORTED!
+        WARNING: PE version ${version} is NOT SUPPORTED!
       | WARN
 # lint:endignore
   }

plans/add_replica.pp

@@ -5,10 +5,11 @@
 # @summary Replace a replica host for a Standard or Large architecture.
 #   Supported use cases:
 #   1: The existing replica is broken, we have a fresh new VM we want to provision the replica to.
-#      The new replica should have the same certname as the broken one.
 # @param primary_host - The hostname and certname of the primary Puppet server
 # @param replica_host - The hostname and certname of the replica VM
-# @param replica_postgresql_host - The hostname and certname of the host with the replica PE-PosgreSQL database. 
+# @param replica_postgresql_host - The hostname and certname of the host with the replica PE-PosgreSQL database.
+# @param token_file - (optional) the token file in a different location than the default.
+# 
 #   Can be a separate host in an XL architecture, or undef in Standard or Large.
 plan peadm::add_replica(
   # Standard or Large
@@ -119,7 +120,8 @@
     # Race condition, where the provision command checks PuppetDB status and
     # probably gets "starting", but fails out because that's not "running".
     # Can remove flag when that issue is fixed.
-    legacy     => true,
+    legacy     => false,
+    # _catch_errors => true, # testing
   )
 
   # start puppet service

plans/modify_certificate.pp

@@ -19,6 +19,7 @@
   # TODO: convert $add_extensions and $remov_extensions  to OIDs, if friendly
   # names have been given
 
+  out::message("peadm::modify_certificate: primary host: ${primary_target} - ${primary_target.name} - ${primary_target.uri}")
   $primary_certname = run_task('peadm::cert_data', $primary_target).first['certname']
 
   # Do the primary first, if it's in the list

plans/subplans/install.pp

@@ -138,7 +138,7 @@
 # lint:ignore:strict_indent
       warning(@("HEREDOC"))
         WARNING: Target name / hostname mismatch: target ${name} reports ${result['hostname']}
-                 Certificate name will be set to target name. Please ensure target name is correct and resolvable
+        Certificate name will be set to target name. Please ensure target name is correct and resolvable
         |-HEREDOC
 # lint:endignore
     }

plans/subplans/modify_certificate.pp

@@ -106,7 +106,7 @@
     # The docs are broken, and the process is unclean. Sadface.
     run_task('service', $target, { action => 'stop', name => 'pe-puppetserver' })
     run_command(@("HEREDOC"/L), $target)
-      rm -f \
+            rm -f \
         /etc/puppetlabs/puppet/ssl/certs/${certname}.pem \
         /etc/puppetlabs/puppet/ssl/private_keys/${certname}.pem \
         /etc/puppetlabs/puppet/ssl/public_keys/${certname}.pem \

plans/subplans/prepare_agent.pp

@@ -8,6 +8,9 @@
   $agent_target    = peadm::get_targets($targets, 1)
   $primary_target  = peadm::get_targets($primary_host, 1)
 
+  out::message("Preparing agent ${agent_target} to connect to ${primary_target}")
+  out::message("agent target ${agent_target} to connect to ${primary_target}")
+
   $dns_alt_names_flag = $dns_alt_names? {
     undef   => [],
     default => ["main:dns_alt_names=${dns_alt_names.join(',')}"],
@@ -80,8 +83,9 @@
 
   # If agent certificate is good but lacks appropriate extensions, plan will still
   # regenerate certificate
+  out::message("primary target: ${primary_target}, certname: ${primary_target.peadm::certname()}, uri: ${primary_target[0].uri}")
   run_plan('peadm::modify_certificate', $agent_target,
-    primary_host     => $primary_target.peadm::certname(),
+    primary_host     => $primary_target,
     add_extensions   => $certificate_extensions,
     force_regenerate => $force_regenerate
   )

plans/upgrade.pp

@@ -255,20 +255,45 @@
       },
     )
 
+    # Log the peadm configuration before node manager setup
+    run_task('peadm::get_peadm_config', $primary_target)
+
     # Update classification. This needs to be done now because if we don't, and
     # the PE Compiler node groups are wrong, then the compilers won't be able to
     # successfully classify and update
+
+    # First, determine the correct hosts for the A and B availability groups
+    $server_a_host = $cert_extensions.dig($primary_target.peadm::certname(), peadm::oid('peadm_availability_group')) ? {
+      'A'     => $primary_target.peadm::certname(),
+      default => $replica_target.peadm::certname(),
+    }
+
+    $server_b_host = $server_a_host ? {
+      $primary_target.peadm::certname() => $replica_target.peadm::certname(),
+      default                           => $primary_target.peadm::certname(),
+    }
+
+    $postgresql_a_host = $cert_extensions.dig($primary_postgresql_target.peadm::certname(), peadm::oid('peadm_availability_group')) ? {
+      'A'     => $primary_postgresql_target.peadm::certname(),
+      default => $replica_postgresql_target.peadm::certname(),
+    }
+
+    $postgresql_b_host = $postgresql_a_host ? {
+      $primary_postgresql_target.peadm::certname() => $replica_postgresql_target.peadm::certname(),
+      default                                      => $primary_postgresql_target.peadm::certname(),
+    }
+
     apply($primary_target) {
       class { 'peadm::setup::node_manager_yaml':
         primary_host => $primary_target.peadm::certname(),
       }
 
       class { 'peadm::setup::node_manager':
         primary_host                     => $primary_target.peadm::certname(),
-        server_a_host                    => $primary_target.peadm::certname(),
-        server_b_host                    => $replica_target.peadm::certname(),
-        postgresql_a_host                => $primary_postgresql_target.peadm::certname(),
-        postgresql_b_host                => $replica_postgresql_target.peadm::certname(),
+        server_a_host                    => $server_a_host,
+        server_b_host                    => $server_b_host,
+        postgresql_a_host                => $postgresql_a_host,
+        postgresql_b_host                => $postgresql_b_host,
         compiler_pool_address            => $compiler_pool_address,
         internal_compiler_a_pool_address => $internal_compiler_a_pool_address,
         internal_compiler_b_pool_address => $internal_compiler_b_pool_address,