From 8fb9c1b93f9efb6cfdcf405e5bc76c3cf1518c56 Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Tue, 22 Jun 2021 22:34:00 -0700 Subject: [PATCH 1/5] Improve get_targets to handle empty array input This lets plans pass [] in addition to undef to peadm target inputs to indicate no target. This is useful when building argument lists automatically and passing forward to peadm plans. Co-authored-by: Dimitri Tischenko --- functions/get_targets.pp | 20 +++++++++++------- spec/functions/get_targets_spec.rb | 34 +++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/functions/get_targets.pp b/functions/get_targets.pp index c0438d79..2e34239e 100644 --- a/functions/get_targets.pp +++ b/functions/get_targets.pp @@ -6,14 +6,18 @@ function peadm::get_targets( Variant[TargetSpec, Undef] $spec, Optional[Integer[1,1]] $count = undef, ) { - # If $spec is undef, return an empty array. Otherwise, if $count is 1, return - # the result of get_target() in an array. If $count is undef, return - # get_targets(). - $targets = $spec ? { - undef => [ ], - default => $count ? { - 1 => [get_target($spec)], - undef => get_targets($spec), + # If $spec is undef or empty array, return an empty array. Otherwise, if + # $count is 1, return the result of get_target() in an array. If $count is + # undef, return get_targets(). + case $spec { + Undef, [ ]: { + [ ] # Return empty array + } + default: { + $count ? { + 1 => [get_target($spec)], + undef => get_targets($spec), + } } } } diff --git a/spec/functions/get_targets_spec.rb b/spec/functions/get_targets_spec.rb index 262d9b6a..3281854c 100644 --- a/spec/functions/get_targets_spec.rb +++ b/spec/functions/get_targets_spec.rb @@ -5,12 +5,36 @@ # and functions we cannot do this right now. # https://github.com/puppetlabs/bolt/issues/1688 describe 'peadm::get_targets' do - let(:spec) do - 'some_value_goes_here' + let(:pre_condition) do + 'type TargetSpec = Variant[String[1], Target, Array[TargetSpec]]' end - let(:count) do - 'some_value_goes_here' + + context 'undefined or empty arguments' do + it { is_expected.to run.with_params([], 1).and_return([]) } + it { is_expected.to run.with_params([]).and_return([]) } + it { is_expected.to run.with_params(:undef, 1).and_return([]) } + it { is_expected.to run.with_params(:undef).and_return([]) } + end + + context 'string arguments' do + it 'converts a string input to a Target array without count' do + skip 'Being able to stub the get_targets() function' + is_expected.to run.with_params('fqdn').and_return(['fqdn']) + end + it 'converts a string input to a Target array with count' do + skip 'Being able to stub the get_targets() function' + is_expected.to run.with_params('fqdn', 1).and_return(['fqdn']) + end end - xit { is_expected.to run.with_params(spec, count).and_return('some_value') } + context 'array arguments' do + it 'converts an array input to a Target array without count' do + skip 'Being able to stub the get_targets() function' + is_expected.to run.with_params(['fqdn']).and_return(['fqdn']) + end + it 'converts an array input to a Target array with count' do + skip 'Being able to stub the get_targets() function' + is_expected.to run.with_params(['fqdn'], 1).and_return(['fqdn']) + end + end end From fb0a9347b9517b4519b66281e7c763ecd495543a Mon Sep 17 00:00:00 2001 From: Dimitri Tischenko Date: Tue, 22 Jun 2021 13:02:33 +0200 Subject: [PATCH 2/5] Create peadm_spec::add_replica test plan For the pending peadm::add_replica plan --- .../modules/peadm_spec/plans/add_replica.pp | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 spec/fixtures/modules/peadm_spec/plans/add_replica.pp diff --git a/spec/fixtures/modules/peadm_spec/plans/add_replica.pp b/spec/fixtures/modules/peadm_spec/plans/add_replica.pp new file mode 100644 index 00000000..89cefc48 --- /dev/null +++ b/spec/fixtures/modules/peadm_spec/plans/add_replica.pp @@ -0,0 +1,26 @@ +plan peadm_spec::add_replica( +){ + + $t = get_targets('*') + wait_until_available($t) + + parallelize($t) |$target| { + $fqdn = run_command('hostname -f', $target) + $target.set_var('certname', $fqdn.first['stdout'].chomp) + } + + $primary_host = $t.filter |$n| { $n.vars['role'] == 'primary' } + $replica_host = $t.filter |$n| { $n.vars['role'] == 'replica' } + $replica_postgresql_host = $t.filter |$n| { $n.vars['role'] == 'replica-pdb-postgresql' } + + if $replica_host == [] { + fail_plan('"replica" role missing from inventory, cannot continue') + } + + run_plan('peadm::add_replica', + primary_host => $primary_host, + replica_host => $replica_host, + replica_postgresql_host => $replica_postgresql_host ? { [] => undef, default => $replica_postgresql_host }, + ) + +} From 8780ee7ace6061717b557ac29d8a77bcf9bb1dc3 Mon Sep 17 00:00:00 2001 From: Dimitri Tischenko Date: Fri, 25 Jun 2021 08:56:05 -0700 Subject: [PATCH 3/5] Create peadm::add_replica plan Co-authored-by: Reid Vandewiele --- plans/add_replica.pp | 106 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 plans/add_replica.pp diff --git a/plans/add_replica.pp b/plans/add_replica.pp new file mode 100644 index 00000000..013500f7 --- /dev/null +++ b/plans/add_replica.pp @@ -0,0 +1,106 @@ +# @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. +# Can be a separate host in an XL architecture, or undef in Standard or Large. +plan peadm::add_replica( + # Standard or Large + Peadm::SingleTargetSpec $primary_host, + Peadm::SingleTargetSpec $replica_host, + + # Extra Large + Optional[Peadm::SingleTargetSpec] $replica_postgresql_host = undef, + + # Common Configuration + Optional[String] $token_file = undef, +) { + + $primary_target = peadm::get_targets($primary_host, 1) + $replica_target = peadm::get_targets($replica_host, 1) + $replica_postgresql_target = peadm::get_targets($replica_postgresql_host, 1) + + $certdata = run_task('peadm::cert_data', $primary_target).first.value + $primary_avail_group_letter = $certdata['extensions'][peadm::oid('peadm_availability_group')] + $replica_avail_group_letter = $primary_avail_group_letter ? { 'A' => 'B', 'B' => 'A' } + + # replica certname + any non-certname alt-names from the primary. Make sure + # to Handle the case where there are no alt-names in the primary's certdata. + $dns_alt_names = [$replica_target.peadm::certname()] + (pick($certdata['dns-alt-names'], []) - $certdata['certname']) + + # This has the effect of revoking the node's certificate, if it exists + run_command("puppet infrastructure forget ${replica_target.peadm::certname()}", $primary_target, _catch_errors => true) + + run_task('peadm::agent_install', $replica_target, + server => $primary_target.peadm::certname(), + install_flags => [ + '--puppet-service-ensure', 'stopped', + "extension_requests:${peadm::oid('peadm_role')}=puppet/server", + "extension_requests:${peadm::oid('peadm_availability_group')}=${replica_avail_group_letter}", + "main:certname=${replica_target.peadm::certname()}", + "main:dns_alt_names=${dns_alt_names.join(',')}", + ], + ) + + # clean the cert to make the plan idempotent + run_task('peadm::ssl_clean', $replica_target, + certname => $replica_target.peadm::certname(), + ) + + # Manually submit a CSR + run_task('peadm::submit_csr', $replica_target) + + # On primary, if necessary, sign the certificate request + run_task('peadm::sign_csr', $primary_target, + certnames => [$replica_target.peadm::certname()], + ) + + # On , run the puppet agent + run_task('peadm::puppet_runonce', $replica_target) + + # On the PE-PostgreSQL server in the group + + # Stop puppet and add the following two lines to + # /opt/puppetlabs/server/data/postgresql/11/data/pg_ident.conf + # pe-puppetdb-pe-puppetdb-map pe-puppetdb + # pe-puppetdb-pe-puppetdb-migrator-map pe-puppetdb-migrator + apply($replica_postgresql_target) { + service { 'puppet': + ensure => stopped, + before => File_line['puppetdb-map', 'migrator-map'], + } + + file_line { 'puppetdb-map': + path => '/opt/puppetlabs/server/data/postgresql/11/data/pg_ident.conf', + line => "pe-puppetdb-pe-puppetdb-map ${replica_target.peadm::certname()} pe-puppetdb", + } + + file_line { 'migrator-map': + path => '/opt/puppetlabs/server/data/postgresql/11/data/pg_ident.conf', + line => "pe-puppetdb-pe-puppetdb-migrator-map ${replica_target.peadm::certname()} pe-puppetdb-migrator", + } + + service { 'pe-postgresql': + ensure => running, + subscribe => File_line['puppetdb-map', 'migrator-map'], + } + } + + # Provision the new system as a replica + run_task('peadm::provision_replica', $primary_target, + replica => $replica_target.peadm::certname(), + token_file => $token_file, + + # 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, + ) + + # start puppet service on postgresql host + run_command('systemctl start puppet.service', $replica_postgresql_target) + + return("Added replica ${replica_target}") +} From d111ff21f486fdb9c9c7bedba6bd15ac2967ebe9 Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Wed, 23 Jun 2021 15:43:59 -0700 Subject: [PATCH 4/5] Create basic spec test for add_replica plan This helps validate that the plan is syntactically valid, uses data correctly, etc. Useful for potential refactors. Co-authored-by: Dimitri Tischenko --- spec/plans/add_replica_spec.rb | 55 ++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 spec/plans/add_replica_spec.rb diff --git a/spec/plans/add_replica_spec.rb b/spec/plans/add_replica_spec.rb new file mode 100644 index 00000000..5ca12a62 --- /dev/null +++ b/spec/plans/add_replica_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe 'peadm::install' do + include BoltSpec::Plans + + def allow_standard_non_returning_calls(params) + allow_apply + allow_task('peadm::agent_install') + allow_task('peadm::ssl_clean') + allow_task('peadm::submit_csr') + allow_task('peadm::sign_csr') + allow_task('peadm::puppet_runonce') + allow_task('peadm::provision_replica') + allow_command('systemctl start puppet.service') + allow_command("puppet infrastructure forget #{params['replica_host']}") + allow_command("puppet node purge #{params['replica_host']}") + end + + describe 'basic functionality' do + let(:params) { { 'primary_host' => 'primary', 'replica_host' => 'replica' } } + let(:certdata) { { 'certname' => 'primary', 'extensions' => { '1.3.6.1.4.1.34380.1.1.9813' => 'A' } } } + + it 'runs successfully when the primary doesn\'t have alt-names' do + allow_standard_non_returning_calls(params) + expect_task('peadm::cert_data').always_return(certdata) + expect_task('peadm::agent_install') + .with_params({ 'server' => 'primary', + 'install_flags' => [ + '--puppet-service-ensure', 'stopped', + 'extension_requests:1.3.6.1.4.1.34380.1.1.9812=puppet/server', + 'extension_requests:1.3.6.1.4.1.34380.1.1.9813=B', + 'main:certname=replica', + 'main:dns_alt_names=replica' + ] }) + + expect(run_plan('peadm::add_replica', params)).to be_ok + end + + it 'runs successfully when the primary has alt-names' do + allow_standard_non_returning_calls(params) + expect_task('peadm::cert_data').always_return(certdata.merge({ 'dns-alt-names' => ['primary', 'alt'] })) + expect_task('peadm::agent_install') + .with_params({ 'server' => 'primary', + 'install_flags' => [ + '--puppet-service-ensure', 'stopped', + 'extension_requests:1.3.6.1.4.1.34380.1.1.9812=puppet/server', + 'extension_requests:1.3.6.1.4.1.34380.1.1.9813=B', + 'main:certname=replica', + 'main:dns_alt_names=replica,alt' + ] }) + + expect(run_plan('peadm::add_replica', params)).to be_ok + end + end +end From 8226dffd0369f5df362c9da8275c4a88ff3152ba Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Fri, 25 Jun 2021 09:14:23 -0700 Subject: [PATCH 5/5] Update add_replica Github Actions workflow Update the Github Actions workflow for peadm::add_replica to support optional ssh-debugging and to use the latest LTS by default Co-authored-by: Dimitri Tischenko --- .github/workflows/test-add-replica.yaml | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test-add-replica.yaml b/.github/workflows/test-add-replica.yaml index c3ec8462..81389592 100644 --- a/.github/workflows/test-add-replica.yaml +++ b/.github/workflows/test-add-replica.yaml @@ -1,5 +1,5 @@ --- -name: "Test add_replica plan" +name: "Add Replica test" on: workflow_dispatch: @@ -15,15 +15,19 @@ on: version: description: 'PE version to install' required: true - default: '2019.8.5' + default: '2019.8.7' + ssh-debugging: + description: 'Boolean; whether or not to pause for ssh debugging' + required: true + default: 'false' env: HONEYCOMB_WRITEKEY: 7f3c63a70eecc61d635917de46bea4e6 HONEYCOMB_DATASET: litmus tests jobs: - smoke-test: - name: "${{ matrix.architecture }}, ${{ matrix.image }}, PE ${{ matrix.version }}" + test-add-replica: + name: "PE ${{ matrix.version }} ${{ matrix.architecture }} on ${{ matrix.image }}" runs-on: ubuntu-20.04 env: BOLT_GEM: true @@ -41,6 +45,7 @@ jobs: steps: - name: 'Start SSH session' + if: ${{ github.event.inputs.ssh-debugging == 'true' }} uses: luchihoratiu/debug-via-ssh@main with: NGROK_AUTH_TOKEN: ${{ secrets.NGROK_AUTH_TOKEN }} @@ -83,7 +88,7 @@ jobs: echo STEP_START=$(date +%s) >> $GITHUB_ENV echo ::endgroup:: - - name: 'Provision test cluster' + - name: 'Provision test cluster (specified architecture with added DR)' timeout-minutes: 15 run: | echo ::group::prepare @@ -140,15 +145,15 @@ jobs: echo ::endgroup:: - name: 'Run add_replica plan' - timeout-minutes: 10 + timeout-minutes: 30 run: | buildevents cmd $TRACE_ID $STEP_ID 'bolt plan run peadm_spec::add_replica' -- \ - bundle exec bolt plan run peadm_spec::add_replica \ + bundle exec bolt plan run peadm_spec::add_replica -v \ --inventoryfile spec/fixtures/litmus_inventory.yaml \ --modulepath spec/fixtures/modules - + - name: 'Wait as long as the file ${HOME}/pause file is present' - if: ${{ always() }} + if: ${{ always() && github.event.inputs.ssh-debugging == 'true' }} run: | while [ -f "${HOME}/pause" ] ; do echo "${HOME}/pause present, sleeping for 60 seconds..."