CHANGELOG.md

@@ -2,6 +2,19 @@
 
 All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org).
 
+## [v2.2.0](https://github.com/puppetlabs/puppetlabs-pe_databases/tree/v2.2.0) (2021-10-06)
+
+[Full Changelog](https://github.com/puppetlabs/puppetlabs-pe_databases/compare/v2.1.1...v2.2.0)
+
+### Added
+
+- \(SUP-2677\) Deprecate backup functionality [\#96](https://github.com/puppetlabs/puppetlabs-pe_databases/pull/96) ([elainemccloskey](https://github.com/elainemccloskey))
+
+### Fixed
+
+- \(SUP-2571\) Purge legacy cron jobs if present [\#97](https://github.com/puppetlabs/puppetlabs-pe_databases/pull/97) ([gavindidrichsen](https://github.com/gavindidrichsen))
+- \(SUP-2557\) Ensure backup class is not included by default [\#95](https://github.com/puppetlabs/puppetlabs-pe_databases/pull/95) ([m0dular](https://github.com/m0dular))
+
 ## [v2.1.1](https://github.com/puppetlabs/puppetlabs-pe_databases/tree/v2.1.1) (2021-09-30)
 
 [Full Changelog](https://github.com/puppetlabs/puppetlabs-pe_databases/compare/v2.1.0...v2.1.1)

README.md

@@ -48,6 +48,11 @@ It is not recommended to classify using a pre-existing node group in the PE Cons
 
 ### Backup Schedule
 
+> WARNING: The backup functionality in this module has been deprecated and will be removed in a future release. 
+Please refer to the [PE Backup and Restore documentation](https://puppet.com/docs/pe/latest/backing_up_and_restoring_pe.html) for details on how to backup.
+You should ensure the parameter `pe_databases::manage_database_backups` and any parameters from the `pe_databases::backup` class are removed from classification or hiera.
+You should also clean up associated crontab entries.
+
 Backups are not activated by default but can be enabled by setting the following parameter:
 
 Hiera classification example 
@@ -90,6 +95,11 @@ Those defaults change based on the `$all_in_one_pe` parameter.
 
 ## Backups
 
+> WARNING: The backup functionality in this module has been deprecated and will be removed in a future release. 
+Please refer to the [PE Backup and Restore documentation](https://puppet.com/docs/pe/latest/backing_up_and_restoring_pe.html) for details on how to backup.
+You should ensure the parameter `pe_databases::manage_database_backups` and any parameters from the `pe_databases::backup` class are removed from classification or hiera.
+You should also clean up associated crontab entries.
+
 This is the documentation for Pupet Enterprise backups:
 
 https://puppet.com/docs/pe/latest/backing_up_and_restoring_pe.html

manifests/init.pp

@@ -43,10 +43,16 @@
           include pe_databases::postgresql_settings::table_settings
         }
       }
-      if defined('$manage_database_backups') {
+      # Because this parameter is a value of undef with a data type of Undef,
+      # We can the NotUndef type to determine if the value has been set
+      if $manage_database_backups =~ NotUndef {
         class { 'pe_databases::backup':
           disable_maintenance => ! $manage_database_backups,
         }
+        notify { 'pe_databases_backup_deprecate_warn':
+          message  => 'The backup functionality in the pe_databases module has been deprecated and will be removed in a future release',
+          loglevel => warning,
+        }
       }
     }
     else {

manifests/pg_repack.pp

@@ -61,12 +61,19 @@
     }
   }
 
-  # Legacy cleanup
+  # Ensure legacy vaccum and pg_repack crons are purged.
+  # If someone upgrades from an ancient v0.x version of the pe_databases module to 2.0 or newer, 
+  # the old cron jobs running vaccuum full will not be cleaned up. This can result in a deadlock 
+  # when both pg_repack and vacuum full attempt to update a table
   $legacy_crons = [
     'pg_repack facts tables', 'pg_repack catalogs tables', 'pg_repack other tables',
-    'pg_repack reports tables', 'pg_repack resource_events tables'
+    'pg_repack reports tables', 'pg_repack resource_events tables',
+    'VACUUM FULL facts tables',
+    'VACUUM FULL catalogs tables',
+    'VACUUM FULL other tables',
+    'Maintain PE databases',
   ]
   cron { $legacy_crons:
       ensure => absent,
-    }
   }
+}

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "puppetlabs-pe_databases",
-  "version": "2.1.1",
+  "version": "2.2.0",
   "author": "puppetlabs",
   "summary": "A Puppet Module for Backing Up / Maintaining / Tuning Your Puppet Enterprise Databases",
   "license": "Apache-2.0",

spec/acceptance/backup_spec.rb

@@ -8,13 +8,8 @@ class { 'pe_databases':
        }#{' '}
        MANIFEST
 
-    # Run it twice and test for idempotency
-    idempotent_apply(pp)
-  end
-  it 'checks if backup cron jobs are up' do
-    run_shell('crontab -l -u pe-postgres') do |r|
-      expect(r.stdout).to match(%r{pe-activity, pe-classifier, pe-inventory, pe-orchestrator, pe-postgres, pe-rbac})
-      expect(r.stdout).to match(%r{pe-puppetdb})
-    end
+    # Expect a change due to backup notify
+    apply_manifest(pp)
+    apply_manifest(pp, expect_changes: true)
   end
 end

spec/classes/backup_spec.rb

@@ -0,0 +1,13 @@
+require 'spec_helper'
+
+describe 'pe_databases::backup' do
+  context 'when backing up tables' do
+    let(:pre_condition) { 'include pe_databases' }
+
+    it {
+      # I have no idea how this works, but these are the resources we should end up with
+      is_expected.to contain_cron('puppet_enterprise_database_backup_[pe-activity, pe-classifier, pe-inventory, pe-orchestrator, pe-postgres, pe-rbac]')
+      is_expected.to contain_cron('puppet_enterprise_database_backup_[pe-puppetdb]')
+    }
+  end
+end

spec/classes/init_spec.rb

@@ -1,14 +1,6 @@
 require 'spec_helper'
 
 describe 'pe_databases' do
-  let(:params) do
-    {
-      manage_database_backups:  false,
-      manage_postgresql_settings: false,
-      manage_table_settings:  false,
-    }
-  end
-
   on_supported_os.each do |os, os_facts|
     context "on #{os}" do
       let(:facts) { os_facts }
@@ -32,4 +24,14 @@
 
     it { is_expected.to contain_notify('pe_databases_systemd_warn') }
   end
+
+  context 'backups are not included by default' do
+    it { is_expected.not_to contain_class('pe_databases::backup') }
+  end
+
+  context 'backups are included if configured' do
+    let(:params) { { manage_database_backups: true } }
+
+    it { is_expected.to contain_class('pe_databases::backup') }
+  end
 end