Skip to content

Commit 58fe218

Browse files
author
Ashley Penney
committed
Remove the ensure => absent uninstall code.
This is likely to be a controversial change so I wanted to put some explanation of our reasoning into the commit message. This gets kind of complex so I'll start with the problem and then the reasoning. Problem: We rely heavily on the ability to uninstall and reinstall postgres throughout our testing code, testing features like "can I move from the distribution packages to the upstream packages through the module" and over time we've learnt that the uninstall code simply doesn't work a lot of the time. It leaves traces of postgres behind or fails to remove certain packages on Ubuntu, and generally causes bits to be left on your system that you didn't expect. When we then reinstall things fail because it's not a true clean slate, and this causes us enormous problems during test. We've spent weeks and months working on these tests and they simply don't hold up well across the full range of PE platforms. Reasoning: Due to all these problems we've decided to take a stance on uninstalling in general. We feel that in 2014 it's completely reasonable and normal to have a good provisioning pipeline combined with your configuration management and the "correct" way to uninstall a fully installed service like postgresql is to simply reprovision the server without it in the first place. As a general rule this is how I personally like to work and I think is a good practice. WAIT A MINUTE: We understand that there are environments and situations in which it's not easy to do that. What if you accidently deployed Postgres on 100,000 nodes? When this work is finished I'm going to take a look at building some example 'profiles' to be found under examples/ within this module that can uninstall postgres on popular platforms. These can be modified and used in your specific case to uninstall postgresql. They will be much more brute force and reliant on deleting entire directories and require you to do more work up front in specifying where things are installed but we think it'll prove to be a much cleaner mechanism for this kind of thing rather than trying to weave it into the main module logic itself.
1 parent 46103e7 commit 58fe218

19 files changed

+226
-412
lines changed

‎README.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,6 @@ If `true` this will setup the official PostgreSQL repositories on your host. Def
380380
###Class: postgresql::server
381381
The following list are options that you can set in the `config_hash` parameter of `postgresql::server`.
382382

383-
####`ensure`
384-
This value default to `present`. When set to `absent` it will remove all packages, configuration and data so use this with extreme caution.
385-
386383
####`postgres_password`
387384
This value defaults to `undef`, meaning the super user account in the postgres database is a user called `postgres` and this account does not have a password. If you provide this setting, the module will set the password for the `postgres` user to your specified value.
388385

‎manifests/client.pp

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Install client cli tool. See README.md for more details.
22
class postgresql::client (
3+
$file_ensure = 'file',
34
$package_name = $postgresql::params::client_package_name,
45
$package_ensure = 'present'
56
) inherits postgresql::params {
@@ -11,19 +12,12 @@
1112
tag => 'postgresql',
1213
}
1314

14-
$file_ensure = $package_ensure ? {
15-
'present' => 'file',
16-
true => 'file',
17-
'absent' => 'absent',
18-
false => 'absent',
19-
default => 'file',
20-
}
21-
file { "/usr/local/bin/validate_postgresql_connection.sh":
15+
file { '/usr/local/bin/validate_postgresql_connection.sh':
2216
ensure => $file_ensure,
23-
source => "puppet:///modules/postgresql/validate_postgresql_connection.sh",
17+
source => 'puppet:///modules/postgresql/validate_postgresql_connection.sh',
2418
owner => 0,
2519
group => 0,
26-
mode => 0755,
20+
mode => '0755',
2721
}
2822

2923
}

‎manifests/globals.pp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
# Class for setting cross-class global overrides. See README.md for more
22
# details.
33
class postgresql::globals (
4-
$ensure = undef,
5-
64
$client_package_name = undef,
75
$server_package_name = undef,
86
$contrib_package_name = undef,
@@ -108,7 +106,6 @@
108106
# Workaround the lack of RHEL7 repositories for now.
109107
if ! ($::operatingsystem == 'RedHat' and $::operatingsystemrelease =~ /^7/) {
110108
class { 'postgresql::repo':
111-
ensure => $ensure,
112109
version => $globals_version
113110
}
114111
}

‎manifests/params.pp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# PRIVATE CLASS: do not use directly
22
class postgresql::params inherits postgresql::globals {
3-
$ensure = present
43
$version = $globals_version
54
$postgis_version = $globals_postgis_version
65
$listen_addresses = 'localhost'
@@ -11,10 +10,12 @@
1110
$ipv6acls = []
1211
$encoding = $encoding
1312
$locale = $locale
14-
$service_ensure = undef
13+
$service_ensure = 'running'
14+
$service_enable = true
1515
$service_provider = $service_provider
1616
$manage_firewall = $manage_firewall
1717
$manage_pg_hba_conf = pick($manage_pg_hba_conf, true)
18+
$package_ensure = 'present'
1819

1920
# Amazon Linux's OS Family is 'Linux', operating system 'Amazon'.
2021
case $::osfamily {

‎manifests/repo.pp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# PRIVATE CLASS: do not use directly
22
class postgresql::repo (
3-
$ensure = $postgresql::params::ensure,
43
$version = undef
54
) inherits postgresql::params {
65
case $::osfamily {

‎manifests/repo/apt_postgresql_org.pp

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,22 @@
11
# PRIVATE CLASS: do not use directly
22
class postgresql::repo::apt_postgresql_org inherits postgresql::repo {
33
include ::apt
4-
if($ensure == 'present' or $ensure == true) {
5-
# Here we have tried to replicate the instructions on the PostgreSQL site:
6-
#
7-
# http://www.postgresql.org/download/linux/debian/
8-
#
9-
apt::pin { 'apt.postgresql.org':
10-
originator => 'apt.postgresql.org',
11-
priority => 500,
12-
}->
13-
apt::source { 'apt.postgresql.org':
14-
location => 'http://apt.postgresql.org/pub/repos/apt/',
15-
release => "${::lsbdistcodename}-pgdg",
16-
repos => "main ${version}",
17-
key => 'ACCC4CF8',
18-
key_source => 'https://www.postgresql.org/media/keys/ACCC4CF8.asc',
19-
include_src => false,
20-
}
21-
22-
Apt::Source['apt.postgresql.org']->Package<|tag == 'postgresql'|>
23-
} else {
24-
apt::source { 'apt.postgresql.org':
25-
ensure => absent,
26-
}
27-
apt::pin { 'apt.postgresql.org':
28-
ensure => absent,
29-
}
4+
# Here we have tried to replicate the instructions on the PostgreSQL site:
5+
#
6+
# http://www.postgresql.org/download/linux/debian/
7+
#
8+
apt::pin { 'apt.postgresql.org':
9+
originator => 'apt.postgresql.org',
10+
priority => 500,
11+
}->
12+
apt::source { 'apt.postgresql.org':
13+
location => 'http://apt.postgresql.org/pub/repos/apt/',
14+
release => "${::lsbdistcodename}-pgdg",
15+
repos => "main ${version}",
16+
key => 'ACCC4CF8',
17+
key_source => 'https://www.postgresql.org/media/keys/ACCC4CF8.asc',
18+
include_src => false,
3019
}
20+
21+
Apt::Source['apt.postgresql.org']->Package<|tag == 'postgresql'|>
3122
}

‎manifests/repo/yum_postgresql_org.pp

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,26 @@
44
$package_version = "${version_parts[0]}${version_parts[1]}"
55
$gpg_key_path = "/etc/pki/rpm-gpg/RPM-GPG-KEY-PGDG-${package_version}"
66

7-
if ($ensure == 'present' or $ensure == true) {
8-
file { $gpg_key_path:
9-
source => 'puppet:///modules/postgresql/RPM-GPG-KEY-PGDG',
10-
before => Yumrepo['yum.postgresql.org']
11-
}
12-
13-
if($::operatingsystem == 'Fedora') {
14-
$label1 = 'fedora'
15-
$label2 = $label1
16-
} else {
17-
$label1 = 'redhat'
18-
$label2 = 'rhel'
19-
}
20-
21-
yumrepo { 'yum.postgresql.org':
22-
descr => "PostgreSQL ${version} \$releasever - \$basearch",
23-
baseurl => "http://yum.postgresql.org/${version}/${label1}/${label2}-\$releasever-\$basearch",
24-
enabled => 1,
25-
gpgcheck => 1,
26-
gpgkey => "file:///etc/pki/rpm-gpg/RPM-GPG-KEY-PGDG-${package_version}",
27-
}
7+
file { $gpg_key_path:
8+
source => 'puppet:///modules/postgresql/RPM-GPG-KEY-PGDG',
9+
before => Yumrepo['yum.postgresql.org']
10+
}
2811

29-
Yumrepo['yum.postgresql.org'] -> Package<|tag == 'postgresql'|>
12+
if($::operatingsystem == 'Fedora') {
13+
$label1 = 'fedora'
14+
$label2 = $label1
3015
} else {
31-
yumrepo { 'yum.postgresql.org':
32-
enabled => absent,
33-
}->
34-
file { $gpg_key_path:
35-
ensure => absent,
36-
}
16+
$label1 = 'redhat'
17+
$label2 = 'rhel'
3718
}
19+
20+
yumrepo { 'yum.postgresql.org':
21+
descr => "PostgreSQL ${version} \$releasever - \$basearch",
22+
baseurl => "http://yum.postgresql.org/${version}/${label1}/${label2}-\$releasever-\$basearch",
23+
enabled => 1,
24+
gpgcheck => 1,
25+
gpgkey => "file:///etc/pki/rpm-gpg/RPM-GPG-KEY-PGDG-${package_version}",
26+
}
27+
28+
Yumrepo['yum.postgresql.org'] -> Package<|tag == 'postgresql'|>
3829
}

‎manifests/server.pp

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
# This installs a PostgreSQL server. See README.md for more details.
22
class postgresql::server (
3-
$ensure = $postgresql::params::ensure,
4-
53
$postgres_password = undef,
64

75
$package_name = $postgresql::params::server_package_name,
86
$client_package_name = $postgresql::params::client_package_name,
9-
$package_ensure = $ensure,
7+
$package_ensure = $postgresql::params::package_ensure,
108

119
$plperl_package_name = $postgresql::params::plperl_package_name,
1210

1311
$service_ensure = $postgresql::params::service_ensure,
12+
$service_enable = $postgresql::params::service_enable,
1413
$service_name = $postgresql::params::service_name,
1514
$service_provider = $postgresql::params::service_provider,
1615
$service_status = $postgresql::params::service_status,
@@ -58,26 +57,15 @@
5857
$_version = $version
5958
}
6059

61-
if ($ensure == 'present' or $ensure == true) {
62-
# Reload has its own ordering, specified by other defines
63-
class { "${pg}::reload": require => Class["${pg}::install"] }
64-
65-
anchor { "${pg}::start": }->
66-
class { "${pg}::install": }->
67-
class { "${pg}::initdb": }->
68-
class { "${pg}::config": }->
69-
class { "${pg}::service": }->
70-
class { "${pg}::passwd": }->
71-
class { "${pg}::firewall": }->
72-
anchor { "${pg}::end": }
73-
} else {
74-
anchor { "${pg}::start": }->
75-
class { "${pg}::firewall": }->
76-
class { "${pg}::passwd": }->
77-
class { "${pg}::service": }->
78-
class { "${pg}::install": }->
79-
class { "${pg}::initdb": }->
80-
class { "${pg}::config": }->
81-
anchor { "${pg}::end": }
82-
}
60+
# Reload has its own ordering, specified by other defines
61+
class { "${pg}::reload": require => Class["${pg}::install"] }
62+
63+
anchor { "${pg}::start": }->
64+
class { "${pg}::install": }->
65+
class { "${pg}::initdb": }->
66+
class { "${pg}::config": }->
67+
class { "${pg}::service": }->
68+
class { "${pg}::passwd": }->
69+
class { "${pg}::firewall": }->
70+
anchor { "${pg}::end": }
8371
}

0 commit comments

Comments
 (0)