From 344d5fa802e71e8652c218fa4c7c2dbd034757ce Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 28 Jun 2021 08:10:38 -0700 Subject: [PATCH 1/2] Fix scope of disable_maintenance param Prior to this commit, the disable_maintenance param was part of a maintenance class that had been removed. This commit fixes that and moves the lookup to the param definition. --- README.md | 2 +- manifests/init.pp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fe80643..35cc614 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ You can configure the retention policy by setting `pe_databases::backup::retenti The maintenance cron jobs will perform a `VACUUM FULL` on various `pe-puppetdb` tables to keep them lean and fast. A `VACUUM FULL` is a blocking operation and you will see the PuppetDB command queue grow while the cron jobs run. The blocking should be short lived and the PuppetDB command queue should work itself down after, however, if for some reason you experience issues you can disable the maintenance cron jobs. -You can do so by setting `pe_databases::maintenance::disable_maintenance: true` in your hieradata. +You can do so by setting `pe_databases::disable_maintenance: true` in your hieradata. With PE 2018.1.7 and 2019.0.2 and newer, this module uses `pg_repack` which does not block. diff --git a/manifests/init.pp b/manifests/init.pp index 946a544..4a9c984 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -4,7 +4,10 @@ class pe_databases ( Boolean $manage_database_backups = false, + # Manage the inclusion of the pg_repack class Boolean $manage_database_maintenance = true, + # Manage the state of the maintenance tasks, i.e. systemd services and timers + Boolean $disable_maintenance = lookup('pe_databases::disable_maintenance', {'default_value' => false}), Boolean $manage_postgresql_settings = true, Boolean $manage_table_settings = true, String $install_dir = '/opt/puppetlabs/pe_databases', @@ -30,7 +33,7 @@ if $facts.dig('pe_databases', 'have_systemd') { if $manage_database_maintenance and (versioncmp('2019.0.2', $facts['pe_server_version']) <= 0) { class {'pe_databases::pg_repack': - disable_maintenance => lookup('pe_databases::maintenance::disable_maintenance', {'default_value' => false}), + disable_maintenance => $disable_maintenance, } if $manage_table_settings { # This is to provide for situations, like PE XL, From b6ef64d7458cc7ffe8811689e49d47b8d826d616 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 29 Jun 2021 15:38:09 -0700 Subject: [PATCH 2/2] Fix managing backup class and resources Prior to this commit, the ensure for the cron entries in the backup class were hard coded to 'present' and could not be removed once set. This commit adds a new parameter to manage the cron jobs based on if $manage_database_backups is set. Restructure if/else logic Prior to this commit, the checks for managing the databases and having systemd were done in the same check, so the warning was confusing. This commit moves $manage_database_maintenance to its own if clause. It also moves the $manage_database_backups inside the version check if, because it makes more sense to put all of the resources behind the version check. --- .github/workflows/pe_latest_testing.yml | 3 +-- .github/workflows/pe_lts_testing.yml | 2 +- .gitignore | 1 - .pdkignore | 2 -- manifests/backup.pp | 8 ++++++- manifests/init.pp | 29 ++++++++++++++----------- metadata.json | 6 ++--- spec/spec_helper.rb | 12 ---------- 8 files changed, 28 insertions(+), 35 deletions(-) diff --git a/.github/workflows/pe_latest_testing.yml b/.github/workflows/pe_latest_testing.yml index 3fdad69..658c430 100644 --- a/.github/workflows/pe_latest_testing.yml +++ b/.github/workflows/pe_latest_testing.yml @@ -53,8 +53,7 @@ jobs: - name: Setup Acceptance Test Matrix id: get-matrix run: | - echo "::set-output name=matrix::{\"platforms\":[{\"label\":\"CentOS-8\",\"provider\":\"provision::provision_service\",\"image\":\"centos-8\"},{\"label\":\"Ubuntu-1804\",\"provider\":\"provision::provision_service\",\"image\":\"ubuntu-1804-lts\"},{\"label\":\"RedHat-8\",\"provider\":\"provision::provision_service\",\"image\":\"rhel-8\"}],\"collection\":[\"2021.2.0\"]}" - + echo "::set-output name=matrix::{\"platforms\":[{\"label\":\"CentOS-8\",\"provider\":\"provision::provision_service\",\"image\":\"centos-8\"},{\"label\":\"Ubuntu-1804\",\"provider\":\"provision::provision_service\",\"image\":\"ubuntu-1804-lts\"}],\"collection\":[\"2021.1.0\"]}" - name: "Honeycomb: Record Setup Test Matrix time" if: ${{ always() }} run: | diff --git a/.github/workflows/pe_lts_testing.yml b/.github/workflows/pe_lts_testing.yml index 0445478..df1ca02 100644 --- a/.github/workflows/pe_lts_testing.yml +++ b/.github/workflows/pe_lts_testing.yml @@ -53,7 +53,7 @@ jobs: - name: Setup Acceptance Test Matrix id: get-matrix run: | - echo "::set-output name=matrix::{\"platforms\":[{\"label\":\"CentOS-7\",\"provider\":\"provision::provision_service\",\"image\":\"centos-7\"},{\"label\":\"CentOS-8\",\"provider\":\"provision::provision_service\",\"image\":\"centos-8\"},{\"label\":\"RedHat-7\",\"provider\":\"provision::provision_service\",\"image\":\"rhel-7\"},{\"label\":\"Ubuntu-1804\",\"provider\":\"provision::provision_service\",\"image\":\"ubuntu-1804-lts\"},{\"label\":\"RedHat-8\",\"provider\":\"provision::provision_service\",\"image\":\"rhel-8\"}],\"collection\":[\"2019.8.7\"]}" + echo "::set-output name=matrix::{\"platforms\":[{\"label\":\"CentOS-7\",\"provider\":\"provision::provision_service\",\"image\":\"centos-7\"},{\"label\":\"CentOS-8\",\"provider\":\"provision::provision_service\",\"image\":\"centos-8\"},{\"label\":\"RedHat-7\",\"provider\":\"provision::provision_service\",\"image\":\"rhel-7\"},{\"label\":\"Ubuntu-1804\",\"provider\":\"provision::provision_service\",\"image\":\"ubuntu-1804-lts\"}],\"collection\":[\"2019.8.6\"]}" - name: "Honeycomb: Record Setup Test Matrix time" if: ${{ always() }} run: | diff --git a/.gitignore b/.gitignore index 988dcbb..2767022 100644 --- a/.gitignore +++ b/.gitignore @@ -25,4 +25,3 @@ .project .envrc /inventory.yaml -/spec/fixtures/litmus_inventory.yaml diff --git a/.pdkignore b/.pdkignore index c538bea..a74c4c4 100644 --- a/.pdkignore +++ b/.pdkignore @@ -25,9 +25,7 @@ .project .envrc /inventory.yaml -/spec/fixtures/litmus_inventory.yaml /appveyor.yml -/.editorconfig /.fixtures.yml /Gemfile /.gitattributes diff --git a/manifests/backup.pp b/manifests/backup.pp index 7beee66..645734c 100644 --- a/manifests/backup.pp +++ b/manifests/backup.pp @@ -29,6 +29,7 @@ String $daily_databases_path = "${pe_databases::install_dir}/default_daily_databases.txt", String $backup_logging_directory = '/var/log/puppetlabs/pe_databases_backup', Integer $retention_policy = 2, + Boolean $disable_maintenance = true, ) { file { $backup_logging_directory : @@ -62,6 +63,11 @@ refreshonly => true, } + $cron_ensure = $disable_maintenance ? { + false => 'present', + default => 'absent', + } + # Since the cron job titles below include the array ('databases') of database names, # the crontab for pe-postgres needs to be reset if the array of database names changes, # otherwise the change create a new cron job and unmanage the old cron job. @@ -72,7 +78,7 @@ $databases_to_backup = $database_backup_set['databases'] $databases = join($databases_to_backup, ' ') cron { "puppet_enterprise_database_backup_${databases_to_backup}": - ensure => present, + ensure => $cron_ensure, command => "${backup_script_path} -l ${backup_logging_directory} -t ${backup_directory} -r ${retention_policy} ${databases}", user => 'pe-postgres', minute => $database_backup_set['schedule']['minute'], diff --git a/manifests/init.pp b/manifests/init.pp index 4a9c984..042d197 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -3,7 +3,7 @@ # @summary Tuning, maintenance, and backups for PE PostgreSQL. class pe_databases ( - Boolean $manage_database_backups = false, + Variant[Boolean,Undef] $manage_database_backups = undef, # Manage the inclusion of the pg_repack class Boolean $manage_database_maintenance = true, # Manage the state of the maintenance tasks, i.e. systemd services and timers @@ -31,19 +31,22 @@ } if $facts.dig('pe_databases', 'have_systemd') { - if $manage_database_maintenance and (versioncmp('2019.0.2', $facts['pe_server_version']) <= 0) { - class {'pe_databases::pg_repack': - disable_maintenance => $disable_maintenance, + if versioncmp('2019.0.2', $facts['pe_server_version']) <= 0 { + if $manage_database_maintenance { + class {'pe_databases::pg_repack': + disable_maintenance => $disable_maintenance, + } + if $manage_table_settings { + # This is to provide for situations, like PE XL, + # where the pe-puppetdb database does not exist on the PostgreSQL system being tuned. + # In PE XL, the Master and Replica run PostgreSQL for all databases *except* for pe-puppetdb. + include pe_databases::postgresql_settings::table_settings + } } - if $manage_table_settings { - # This is to provide for situations, like PE XL, - # where the pe-puppetdb database does not exist on the PostgreSQL system being tuned. - # In PE XL, the Master and Replica run PostgreSQL for all databases *except* for pe-puppetdb. - include pe_databases::postgresql_settings::table_settings - } - - if $manage_database_backups { - include pe_databases::backup + if defined('$manage_database_backups') { + class { 'pe_databases::backup': + disable_maintenance => ! $manage_database_backups, + } } } else { diff --git a/metadata.json b/metadata.json index 61937d5..3f392ac 100644 --- a/metadata.json +++ b/metadata.json @@ -53,7 +53,7 @@ "version_requirement": ">= 5.5.0 < 8.0.0" } ], - "pdk-version": "2.1.1", - "template-url": "https://github.com/puppetlabs/pdk-templates#2.1.1", - "template-ref": "tags/2.1.1-0-g03daa92" + "pdk-version": "2.1.0", + "template-url": "https://github.com/puppetlabs/pdk-templates#2.1.0", + "template-ref": "tags/2.1.0-0-ga675ea5" } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 07db734..16764b6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -48,18 +48,6 @@ c.after(:suite) do RSpec::Puppet::Coverage.report!(0) end - - # Filter backtrace noise - backtrace_exclusion_patterns = [ - %r{spec_helper}, - %r{gems}, - ] - - if c.respond_to?(:backtrace_exclusion_patterns) - c.backtrace_exclusion_patterns = backtrace_exclusion_patterns - elsif c.respond_to?(:backtrace_clean_patterns) - c.backtrace_clean_patterns = backtrace_exclusion_patterns - end end # Ensures that a module is defined