From 14efe76864d69e5c16e110246d6ab2b3444360a9 Mon Sep 17 00:00:00 2001 From: Neil Anderson Date: Mon, 29 Jul 2024 17:50:33 +0100 Subject: [PATCH 1/4] (PE-38814) add_compiler - Making primary_postgresql_host and avail_group_letter optional primary_postgresql_host, if not provided will be determined through get_peadm_config avail_group_letter, is defaulting to A --- REFERENCE.md | 8 ++++++-- plans/add_compiler.pp | 23 ++++++++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 84bc011b..39a1bf82 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -1571,10 +1571,12 @@ The following parameters are available in the `peadm::add_compiler` plan: ##### `avail_group_letter` -Data type: `Enum['A', 'B']` +Data type: `Optional[Enum['A', 'B']]` _ Either A or B; whichever of the two letter designations the compiler is being assigned to +Default value: `'A'` + ##### `compiler_host` Data type: `Peadm::SingleTargetSpec` @@ -1597,10 +1599,12 @@ _ The hostname and certname of the primary Puppet server ##### `primary_postgresql_host` -Data type: `Peadm::SingleTargetSpec` +Data type: `Optional[Peadm::SingleTargetSpec]` _ The hostname and certname of the PE-PostgreSQL server with availability group $avail_group_letter +Default value: `undef` + ### `peadm::add_database` The peadm::add_database class. diff --git a/plans/add_compiler.pp b/plans/add_compiler.pp index 1e3d8caa..f89a0bfc 100644 --- a/plans/add_compiler.pp +++ b/plans/add_compiler.pp @@ -7,20 +7,37 @@ # @param primary_host _ The hostname and certname of the primary Puppet server # @param primary_postgresql_host _ The hostname and certname of the PE-PostgreSQL server with availability group $avail_group_letter plan peadm::add_compiler( - Enum['A', 'B'] $avail_group_letter, + Optional[Enum['A', 'B']] $avail_group_letter = 'A' , Optional[String[1]] $dns_alt_names = undef, Peadm::SingleTargetSpec $compiler_host, Peadm::SingleTargetSpec $primary_host, - Peadm::SingleTargetSpec $primary_postgresql_host, + Optional[Peadm::SingleTargetSpec] $primary_postgresql_host = undef, ) { $compiler_target = peadm::get_targets($compiler_host, 1) $primary_target = peadm::get_targets($primary_host, 1) - $primary_postgresql_target = peadm::get_targets($primary_postgresql_host, 1) # Get current peadm config to determine where to setup additional rules for # compiler's secondary PuppetDB instances $peadm_config = run_task('peadm::get_peadm_config', $primary_target).first.value + if $primary_postgresql_host == undef { + # get the external PostgreSQL host for the specified availability group + $external_postgresql_host = $avail_group_letter ? { + 'A' => $peadm_config['params']['primary_postgresql_host'], + default => $peadm_config['params']['replica_postgresql_host'], + } + + # If the external_postgresql_host is undef, use the server for that availability group + $postgresql_host = $external_postgresql_host ? { + undef => $peadm_config['role-letter']['server'][$avail_group_letter], + default => $external_postgresql_host, + } + + $primary_postgresql_target = peadm::get_targets($postgresql_host, 1) + } else { + $primary_postgresql_target = peadm::get_targets($primary_postgresql_host, 1) + } + # Return the opposite server than the compiler to be added so it can be # configured with the appropriate rules for Puppet Server access from # compiler From 2566bffea6fdbb955e606ec378c52884c99f117e Mon Sep 17 00:00:00 2001 From: Neil Anderson Date: Thu, 8 Aug 2024 09:22:43 +0100 Subject: [PATCH 2/4] Updating spec tests for add compiler Removing optional from avail_group_letter as not required with enum default value --- plans/add_compiler.pp | 6 +- spec/plans/add_compiler_spec.rb | 137 ++++++++++++++++++++++---------- 2 files changed, 102 insertions(+), 41 deletions(-) diff --git a/plans/add_compiler.pp b/plans/add_compiler.pp index f89a0bfc..4595919f 100644 --- a/plans/add_compiler.pp +++ b/plans/add_compiler.pp @@ -7,7 +7,7 @@ # @param primary_host _ The hostname and certname of the primary Puppet server # @param primary_postgresql_host _ The hostname and certname of the PE-PostgreSQL server with availability group $avail_group_letter plan peadm::add_compiler( - Optional[Enum['A', 'B']] $avail_group_letter = 'A' , + Enum['A', 'B'] $avail_group_letter = 'A' , Optional[String[1]] $dns_alt_names = undef, Peadm::SingleTargetSpec $compiler_host, Peadm::SingleTargetSpec $primary_host, @@ -33,6 +33,10 @@ default => $external_postgresql_host, } + if $postgresql_host == undef { + fail_plan("No PostgreSQL host found for availability group ${avail_group_letter}") + } + $primary_postgresql_target = peadm::get_targets($postgresql_host, 1) } else { $primary_postgresql_target = peadm::get_targets($primary_postgresql_host, 1) diff --git a/spec/plans/add_compiler_spec.rb b/spec/plans/add_compiler_spec.rb index 5f75b742..b12c003e 100644 --- a/spec/plans/add_compiler_spec.rb +++ b/spec/plans/add_compiler_spec.rb @@ -5,8 +5,9 @@ def allow_standard_non_returning_calls allow_apply - allow_any_task allow_any_command + execute_no_plan + allow_out_message end describe 'basic functionality' do @@ -14,67 +15,123 @@ def allow_standard_non_returning_calls { 'primary_host' => 'primary', 'compiler_host' => 'compiler', - 'avail_group_letter' => 'A', - 'primary_postgresql_host' => 'primary_postgresql', } end let(:cfg) do { - 'params' => { - 'primary_host' => 'primary' + "params" => { + "primary_host" => "primary", + "replica_host" => nil, + "primary_postgresql_host" => nil, + "replica_postgresql_host" => nil }, - 'role-letter' => { - 'server' => { - 'A' => 'server_a', - 'B' => 'server_b' + "role-letter" => { + "server" => { + "A" => "server_a", + "B" => nil + }, + "postgresql": { + "A" => nil, + "B" => nil } } } end - let(:certdata) { { 'certname' => 'primary', 'extensions' => { '1.3.6.1.4.1.34380.1.1.9813' => 'A' } } } it 'runs successfully when no alt-names are specified' do allow_standard_non_returning_calls expect_task('peadm::get_peadm_config').always_return(cfg) + expect_task('peadm::get_psql_version').with_targets(['server_a']) - # TODO: Due to difficulty mocking get_targets, with_params modifier has been commented out expect_plan('peadm::subplans::component_install') - # .with_params({ - # 'targets' => 'compiler', - # 'primary_host' => 'primary', - # 'avail_group_letter' => 'A', - # 'dns_alt_names' => nil, - # 'role' => 'pe_compiler' - # }) + expect_plan('peadm::util::copy_file').be_called_times(1) + expect_task('peadm::puppet_runonce').with_targets(['compiler']) + expect_task('peadm::puppet_runonce').with_targets(['server_a']) + expect(run_plan('peadm::add_compiler', params)).to be_ok + end + + let(:params_with_avail_group_b) do + params.merge({ 'avail_group_letter' => 'B' }) + end + + it 'handles different avail_group_letter values' do + allow_standard_non_returning_calls + cfg['role-letter']['server']['B'] = 'server_b' + + expect_task('peadm::get_peadm_config').always_return(cfg) + expect_task('peadm::get_psql_version').with_targets(['server_b']) + + expect_plan('peadm::subplans::component_install') + expect_plan('peadm::util::copy_file').be_called_times(1) + expect_task('peadm::puppet_runonce').with_targets(['compiler']) + expect_task('peadm::puppet_runonce').with_targets(['server_a']) + expect_task('peadm::puppet_runonce').with_targets(['server_b']) + expect(run_plan('peadm::add_compiler', params_with_avail_group_b)).to be_ok + end + + let(:params_with_primary_postgresql_host) do + params.merge({ 'primary_postgresql_host' => 'custom_postgresql' }) + end + + it 'handles specified primary_postgresql_host' do + allow_standard_non_returning_calls + expect_task('peadm::get_peadm_config').always_return(cfg) + expect_task('peadm::get_psql_version').with_targets(['custom_postgresql']) + + expect_plan('peadm::subplans::component_install') expect_plan('peadm::util::copy_file').be_called_times(1) + expect_task('peadm::puppet_runonce').with_targets(['compiler']) + expect_task('peadm::puppet_runonce').with_targets(['custom_postgresql']) + expect(run_plan('peadm::add_compiler', params_with_primary_postgresql_host)).to be_ok + end + + it 'handles external postgresql host group A' do + allow_standard_non_returning_calls + cfg['params']['primary_postgresql_host'] = 'external_postgresql' + cfg['params']['replica_postgresql_host'] = 'external_postgresql' + + expect_task('peadm::get_peadm_config').always_return(cfg) + expect_task('peadm::get_psql_version').with_targets(['external_postgresql']) + + expect_plan('peadm::subplans::component_install') + expect_plan('peadm::util::copy_file').be_called_times(1) + expect_task('peadm::puppet_runonce').with_targets(['compiler']) + expect_task('peadm::puppet_runonce').with_targets(['external_postgresql']) expect(run_plan('peadm::add_compiler', params)).to be_ok end - context 'with alt-names' do - let(:params2) do - params.merge({ 'dns_alt_names' => 'foo,bar' }) - end - - it 'runs successfully when alt-names are specified' do - allow_standard_non_returning_calls - expect_task('peadm::get_peadm_config').always_return(cfg) - - # TODO: Due to difficulty mocking get_targets, with_params modifier has been commented out - expect_plan('peadm::subplans::component_install') - # .with_params({ - # 'targets' => 'compiler', - # 'primary_host' => 'primary', - # 'avail_group_letter' => 'A', - # 'dns_alt_names' => 'foo,bar', - # 'role' => 'pe_compiler' - # }) - - expect_plan('peadm::util::copy_file').be_called_times(1) - expect(run_plan('peadm::add_compiler', params2)).to be_ok - end + it 'handles external postgresql host group A with replica' do + allow_standard_non_returning_calls + cfg['params']['primary_postgresql_host'] = 'external_postgresql' + cfg['role-letter']['server']['B'] = 'replica' + + expect_task('peadm::get_peadm_config').always_return(cfg) + expect_task('peadm::get_psql_version').with_targets(['external_postgresql']) + + expect_plan('peadm::subplans::component_install') + expect_plan('peadm::util::copy_file').be_called_times(1) + expect_task('peadm::puppet_runonce').with_targets(['compiler']) + expect_task('peadm::puppet_runonce').with_targets(['external_postgresql']) + expect_task('peadm::puppet_runonce').with_targets(['replica']) + expect(run_plan('peadm::add_compiler', params)).to be_ok + end + + it 'handles external postgresql host group B' do + allow_standard_non_returning_calls + cfg['params']['replica_postgresql_host'] = 'replica_external_postgresql' + + expect_task('peadm::get_peadm_config').always_return(cfg) + expect_task('peadm::get_psql_version').with_targets(['replica_external_postgresql']) + + expect_plan('peadm::subplans::component_install') + expect_plan('peadm::util::copy_file').be_called_times(1) + expect_task('peadm::puppet_runonce').with_targets(['compiler']) + expect_task('peadm::puppet_runonce').with_targets(['replica_external_postgresql']) + expect_task('peadm::puppet_runonce').with_targets(['server_a']) + expect(run_plan('peadm::add_compiler', params_with_avail_group_b)).to be_ok end end end From 0b502bb3c3812a535ce1346a8ff8cdf52f6e01e5 Mon Sep 17 00:00:00 2001 From: Neil Anderson Date: Thu, 8 Aug 2024 09:26:38 +0100 Subject: [PATCH 3/4] Updating reference.md --- REFERENCE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/REFERENCE.md b/REFERENCE.md index 39a1bf82..eef3a4ad 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -1571,7 +1571,7 @@ The following parameters are available in the `peadm::add_compiler` plan: ##### `avail_group_letter` -Data type: `Optional[Enum['A', 'B']]` +Data type: `Enum['A', 'B']` _ Either A or B; whichever of the two letter designations the compiler is being assigned to From 788c9f37d083b8f4e202e84bfc590d8b1cb4090d Mon Sep 17 00:00:00 2001 From: Neil Anderson Date: Thu, 8 Aug 2024 09:55:55 +0100 Subject: [PATCH 4/4] Fixing linting issues --- spec/plans/add_compiler_spec.rb | 40 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/spec/plans/add_compiler_spec.rb b/spec/plans/add_compiler_spec.rb index b12c003e..cd493328 100644 --- a/spec/plans/add_compiler_spec.rb +++ b/spec/plans/add_compiler_spec.rb @@ -18,22 +18,30 @@ def allow_standard_non_returning_calls } end + let(:params_with_avail_group_b) do + params.merge({ 'avail_group_letter' => 'B' }) + end + + let(:params_with_primary_postgresql_host) do + params.merge({ 'primary_postgresql_host' => 'custom_postgresql' }) + end + let(:cfg) do { - "params" => { - "primary_host" => "primary", - "replica_host" => nil, - "primary_postgresql_host" => nil, - "replica_postgresql_host" => nil + 'params' => { + 'primary_host' => 'primary', + 'replica_host' => nil, + 'primary_postgresql_host' => nil, + 'replica_postgresql_host' => nil }, - "role-letter" => { - "server" => { - "A" => "server_a", - "B" => nil + 'role-letter' => { + 'server' => { + 'A' => 'server_a', + 'B' => nil }, - "postgresql": { - "A" => nil, - "B" => nil + 'postgresql': { + 'A' => nil, + 'B' => nil } } } @@ -52,10 +60,6 @@ def allow_standard_non_returning_calls expect(run_plan('peadm::add_compiler', params)).to be_ok end - let(:params_with_avail_group_b) do - params.merge({ 'avail_group_letter' => 'B' }) - end - it 'handles different avail_group_letter values' do allow_standard_non_returning_calls cfg['role-letter']['server']['B'] = 'server_b' @@ -71,10 +75,6 @@ def allow_standard_non_returning_calls expect(run_plan('peadm::add_compiler', params_with_avail_group_b)).to be_ok end - let(:params_with_primary_postgresql_host) do - params.merge({ 'primary_postgresql_host' => 'custom_postgresql' }) - end - it 'handles specified primary_postgresql_host' do allow_standard_non_returning_calls