From 6ba31799162f1dc3bc7ded0d1a13955007159e33 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Fri, 31 Oct 2014 15:27:34 -0700 Subject: [PATCH] Fix data directory handling The change introduced in b781849882ee5b36135f8d3abafa449721e19565 added a complex operation that was not handled correctly for all operating systems. This fix includes the following corrections: - Change the systemd config and reload systemd for datadir changes in RHEL 7, and move configuration for this into postgresql::server::config since it is managing both the PGDATA and PGPORT variables - Make sure Debian systems stop the service before changing the datadir - Recreate cert links after running initdb in Debian and early ubuntu - Change the port in the port spec to avoid selinux issues - Turn off selinux in pgdata spec to avoid selinux issues - Correct syntax for describing presence of a directory in pgdata spec - Move the pgdata spec to the end of the tests so that puppet doesn't have to manager purging and recreating the original datadir - Update README to describe all caveats of using this parameter --- README.md | 6 +- manifests/server/config.pp | 19 +++++ manifests/server/config_entry.pp | 81 ++++++++----------- manifests/server/initdb.pp | 18 +++++ spec/acceptance/alternative_port_spec.rb | 6 +- ...a_spec.rb => z_alternative_pgdata_spec.rb} | 17 +++- spec/unit/defines/server/config_entry_spec.rb | 6 +- templates/systemd-override.erb | 4 + templates/systemd-port-override.erb | 3 - 9 files changed, 99 insertions(+), 61 deletions(-) rename spec/acceptance/{alternative_pgdata_spec.rb => z_alternative_pgdata_spec.rb} (54%) create mode 100644 templates/systemd-override.erb delete mode 100644 templates/systemd-port-override.erb diff --git a/README.md b/README.md index 5f33146..a0bf04c 100644 --- a/README.md +++ b/README.md @@ -335,7 +335,9 @@ Path to your `postgresql.conf` file. If false, disables the defaults supplied with the module for `pg\_hba.conf`. This is useful if you disagree with the defaults and wish to override them yourself. Be sure that your changes of course align with the rest of the module, as some access is required to perform basic `psql` operations for example. ####`datadir` -This setting can be used to override the default postgresql data directory for the target platform. If not specified, the module will use whatever directory is the default for your OS distro. +This setting can be used to override the default postgresql data directory for the target platform. If not specified, the module will use whatever directory is the default for your OS distro. Please note that changing the datadir after installation will cause the server to come to a full stop before being able to make the change. For RedHat systems, the data directory must be labeled appropriately for SELinux. On Ubuntu, you need to explicitly set needs\_initdb to true in order to allow Puppet to initialize the database in the new datadir (needs\_initdb defaults to true on other systems). + +Warning: If datadir is changed from the default, puppet will not manage purging of the original data directory, which will cause it to fail if the data directory is changed back to the original. ####`confdir` This setting can be used to override the default postgresql configuration directory for the target platform. If not specified, the module will use whatever directory is the default for your OS distro. @@ -407,7 +409,7 @@ This setting is used to specify the name of the default database to connect with This value defaults to `localhost`, meaning the postgres server will only accept connections from localhost. If you'd like to be able to connect to postgres from remote machines, you can override this setting. A value of `*` will tell postgres to accept connections from any remote machine. Alternately, you can specify a comma-separated list of hostnames or IP addresses. (For more info, have a look at the `postgresql.conf` file from your system's postgres package). ####`port` -This value defaults to `5432`, meaning the postgres server will listen on TCP port 5432. Note that the same port number is used for all IP addresses the server listens on. +This value defaults to `5432`, meaning the postgres server will listen on TCP port 5432. Note that the same port number is used for all IP addresses the server listens on. Also note that for RedHat systems and early Debian systems, changing the port will cause the server to come to a full stop before being able to make the change. ####`ip_mask_deny_postgres_user` This value defaults to `0.0.0.0/0`. Sometimes it can be useful to block the superuser account from remote connections if you are allowing other database users to connect remotely. Set this to an IP and mask for which you want to deny connections by the postgres superuser account. So, e.g., the default value of `0.0.0.0/0` will match any remote IP and deny access, so the postgres user won't be able to connect remotely at all. Conversely, a value of `0.0.0.0/32` would not match any remote IP, and thus the deny rule will not be applied and the postgres user will be allowed to connect. diff --git a/manifests/server/config.pp b/manifests/server/config.pp index 5d1c693..26645f5 100644 --- a/manifests/server/config.pp +++ b/manifests/server/config.pp @@ -124,4 +124,23 @@ class postgresql::server::config { notify => Class['postgresql::server::reload'], } } + + if $::osfamily == 'RedHat' { + if $::operatingsystemrelease =~ /^7/ or $::operatingsystem == 'Fedora' { + file { 'systemd-override': + ensure => present, + path => '/etc/systemd/system/postgresql.service', + owner => root, + group => root, + content => template('postgresql/systemd-override.erb'), + notify => [ Exec['restart-systemd'], Class['postgresql::server::service'] ], + before => Class['postgresql::server::reload'], + } + exec { "restart-systemd": + command => 'systemctl daemon-reload', + refreshonly => true, + path => '/bin:/usr/bin:/usr/local/bin' + } + } + } } diff --git a/manifests/server/config_entry.pp b/manifests/server/config_entry.pp index cd73520..9ba46bf 100644 --- a/manifests/server/config_entry.pp +++ b/manifests/server/config_entry.pp @@ -30,47 +30,38 @@ define postgresql::server::config_entry ( } } - # We have to handle ports in a weird and special way. On early Debian and - # Ubuntu we have to ensure we stop the service completely. On Redhat we - # either have to create a systemd override for the port or update the - # sysconfig file. + # We have to handle ports and the data directory in a weird and + # special way. On early Debian and Ubuntu and RHEL we have to ensure + # we stop the service completely. On RHEL 7 we either have to create + # a systemd override for the port or update the sysconfig file, but this + # is managed for us in postgresql::server::config. if $::operatingsystem == 'Debian' or $::operatingsystem == 'Ubuntu' { - if $::operatingsystemrelease =~ /^6/ or $::operatingsystemrelease =~ /^10\.04/ { - if $name == 'port' { - exec { 'postgresql_stop': + if $name == 'port' and ( $::operatingsystemrelease =~ /^6/ or $::operatingsystemrelease =~ /^10\.04/ ) { + exec { "postgresql_stop_${name}": command => "service ${::postgresql::server::service_name} stop", onlyif => "service ${::postgresql::server::service_name} status", unless => "grep 'port = ${value}' ${::postgresql::server::postgresql_conf_path}", path => '/usr/sbin:/sbin:/bin:/usr/bin:/usr/local/bin', before => Postgresql_conf[$name], } + } + elsif $name == 'data_directory' { + exec { "postgresql_stop_${name}": + command => "service ${::postgresql::server::service_name} stop", + onlyif => "service ${::postgresql::server::service_name} status", + unless => "grep \"data_directory = '${value}'\" ${::postgresql::server::postgresql_conf_path}", + path => '/usr/sbin:/sbin:/bin:/usr/bin:/usr/local/bin', + before => Postgresql_conf[$name], } } } if $::osfamily == 'RedHat' { - if $::operatingsystemrelease =~ /^7/ or $::operatingsystem == 'Fedora' { - if $name == 'port' { - file { 'systemd-port-override': - ensure => present, - path => '/etc/systemd/system/postgresql.service', - owner => root, - group => root, - content => template('postgresql/systemd-port-override.erb'), - notify => [ Exec['restart-systemd'], Class['postgresql::server::service'] ], - before => Class['postgresql::server::reload'], - } - exec { 'restart-systemd': - command => 'systemctl daemon-reload', - refreshonly => true, - path => '/bin:/usr/bin:/usr/local/bin' - } - } - } else { + if ! ($::operatingsystemrelease =~ /^7/ or $::operatingsystem == 'Fedora') { if $name == 'port' { # We need to force postgresql to stop before updating the port # because puppet becomes confused and is unable to manage the # service appropriately. - exec { 'postgresql_stop': + exec { "postgresql_stop_${name}": command => "service ${::postgresql::server::service_name} stop", onlyif => "service ${::postgresql::server::service_name} status", unless => "grep 'PGPORT=${value}' /etc/sysconfig/pgsql/postgresql", @@ -86,26 +77,24 @@ define postgresql::server::config_entry ( notify => Class['postgresql::server::service'], before => Class['postgresql::server::reload'], } - } else { - if $name == 'data_directory' { - # We need to force postgresql to stop before updating the data directory - # otherwise init script breaks - exec { "postgresql_${name}": - command => "service ${::postgresql::server::service_name} stop", - onlyif => "service ${::postgresql::server::service_name} status", - unless => "grep 'PGDATA=${value}' /etc/sysconfig/pgsql/postgresql", - path => '/sbin:/bin:/usr/bin:/usr/local/bin', - require => File['/etc/sysconfig/pgsql/postgresql'], - } -> - augeas { 'override PGDATA in /etc/sysconfig/pgsql/postgresql': - lens => 'Shellvars.lns', - incl => '/etc/sysconfig/pgsql/*', - context => '/files/etc/sysconfig/pgsql/postgresql', - changes => "set PGDATA ${value}", - require => File['/etc/sysconfig/pgsql/postgresql'], - notify => Class['postgresql::server::service'], - before => Class['postgresql::server::reload'], - } + } elsif $name == 'data_directory' { + # We need to force postgresql to stop before updating the data directory + # otherwise init script breaks + exec { "postgresql_${name}": + command => "service ${::postgresql::server::service_name} stop", + onlyif => "service ${::postgresql::server::service_name} status", + unless => "grep 'PGDATA=${value}' /etc/sysconfig/pgsql/postgresql", + path => '/sbin:/bin:/usr/bin:/usr/local/bin', + require => File['/etc/sysconfig/pgsql/postgresql'], + } -> + augeas { 'override PGDATA in /etc/sysconfig/pgsql/postgresql': + lens => 'Shellvars.lns', + incl => '/etc/sysconfig/pgsql/*', + context => '/files/etc/sysconfig/pgsql/postgresql', + changes => "set PGDATA ${value}", + require => File['/etc/sysconfig/pgsql/postgresql'], + notify => Class['postgresql::server::service'], + before => Class['postgresql::server::reload'], } } } diff --git a/manifests/server/initdb.pp b/manifests/server/initdb.pp index 0d9c65b..d1c61c3 100644 --- a/manifests/server/initdb.pp +++ b/manifests/server/initdb.pp @@ -62,5 +62,23 @@ class postgresql::server::initdb { logoutput => on_failure, require => File[$require_before_initdb], } + # The package will take care of this for us the first time, but if we + # ever need to init a new db we need to make these links explicitly + if $::operatingsystem == 'Debian' or $::operatingsystem == 'Ubuntu' { + if $::operatingsystemrelease =~ /^6/ or $::operatingsystemrelease =~ /^7/ or $::operatingsystemrelease =~ /^10\.04/ or $::operatingsystemrelease =~ /^12\.04/ { + file { 'server.crt': + ensure => link, + path => "${datadir}/server.crt", + target => '/etc/ssl/certs/ssl-cert-snakeoil.pem', + require => Exec['postgresql_initdb'], + } + file { 'server.key': + ensure => link, + path => "${datadir}/server.key", + target => '/etc/ssl/private/ssl-cert-snakeoil.key', + require => Exec['postgresql_initdb'], + } + } + } } } diff --git a/spec/acceptance/alternative_port_spec.rb b/spec/acceptance/alternative_port_spec.rb index 1dab536..fbfad16 100644 --- a/spec/acceptance/alternative_port_spec.rb +++ b/spec/acceptance/alternative_port_spec.rb @@ -5,19 +5,19 @@ require 'spec_helper_acceptance' describe 'postgres::server', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do it 'on an alternative port' do pp = <<-EOS - class { 'postgresql::server': port => '5433' } + class { 'postgresql::server': port => '55433' } EOS apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end - describe port(5433) do + describe port(55433) do it { is_expected.to be_listening } end it 'can connect with psql' do - psql('-p 5433 --command="\l" postgres', 'postgres') do |r| + psql('-p 55433 --command="\l" postgres', 'postgres') do |r| expect(r.stdout).to match(/List of databases/) end end diff --git a/spec/acceptance/alternative_pgdata_spec.rb b/spec/acceptance/z_alternative_pgdata_spec.rb similarity index 54% rename from spec/acceptance/alternative_pgdata_spec.rb rename to spec/acceptance/z_alternative_pgdata_spec.rb index 0dd6400..bf1825a 100644 --- a/spec/acceptance/alternative_pgdata_spec.rb +++ b/spec/acceptance/z_alternative_pgdata_spec.rb @@ -2,18 +2,27 @@ require 'spec_helper_acceptance' # These tests ensure that postgres can change itself to an alternative pgdata # location properly. + +# Allow postgresql to use /tmp/* as a datadir +if fact('osfamily') == 'RedHat' + shell("setenforce 0") +end + describe 'postgres::server', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do it 'on an alternative pgdata location' do pp = <<-EOS - class { 'postgresql::server': datadir => '/var/pgsql' } + #file { '/var/lib/pgsql': ensure => directory, } -> + # needs_initdb will be true by default for all OS's except Debian + # in order to change the datadir we need to tell it explicitly to call initdb + class { 'postgresql::server': datadir => '/tmp/data', needs_initdb => true } EOS apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end - - describe "Alternate Directory" do - File.directory?("/var/pgsql").should be true + + describe file('/tmp/data') do + it { should be_directory } end it 'can connect with psql' do diff --git a/spec/unit/defines/server/config_entry_spec.rb b/spec/unit/defines/server/config_entry_spec.rb index 4c3d9a9..34963b8 100644 --- a/spec/unit/defines/server/config_entry_spec.rb +++ b/spec/unit/defines/server/config_entry_spec.rb @@ -44,7 +44,7 @@ describe 'postgresql::server::config_entry', :type => :define do let(:params) {{ :ensure => 'present', :name => 'port_spec', :value => '5432' }} it 'stops postgresql and changes the port' do - is_expected.to contain_exec('postgresql_stop') + is_expected.to contain_exec('postgresql_stop_port') is_expected.to contain_augeas('override PGPORT in /etc/sysconfig/pgsql/postgresql') end end @@ -63,7 +63,7 @@ describe 'postgresql::server::config_entry', :type => :define do let(:params) {{ :ensure => 'present', :name => 'port_spec', :value => '5432' }} it 'stops postgresql and changes the port' do - is_expected.to contain_file('systemd-port-override') + is_expected.to contain_file('systemd-override') is_expected.to contain_exec('restart-systemd') end end @@ -82,7 +82,7 @@ describe 'postgresql::server::config_entry', :type => :define do let(:params) {{ :ensure => 'present', :name => 'port_spec', :value => '5432' }} it 'stops postgresql and changes the port' do - is_expected.to contain_file('systemd-port-override') + is_expected.to contain_file('systemd-override') is_expected.to contain_exec('restart-systemd') end end diff --git a/templates/systemd-override.erb b/templates/systemd-override.erb new file mode 100644 index 0000000..1afb2a8 --- /dev/null +++ b/templates/systemd-override.erb @@ -0,0 +1,4 @@ +.include /lib/systemd/system/postgresql.service +[Service] +Environment=PGPORT=<%= @port %> +Environment=PGDATA=<%= @datadir %> diff --git a/templates/systemd-port-override.erb b/templates/systemd-port-override.erb deleted file mode 100644 index c3f24a6..0000000 --- a/templates/systemd-port-override.erb +++ /dev/null @@ -1,3 +0,0 @@ -.include /lib/systemd/system/postgresql.service -[Service] -Environment=PGPORT=<%= @value %>