Skip to content

deprecate old code managing sysconfig files #1400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 1 addition & 72 deletions manifests/server/config_entry.pp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#
# @param ensure Removes an entry if set to 'absent'.
# @param value Defines the value for the setting.
# @param path Path for postgresql.conf
# @param path Path for postgresql.conf
#
define postgresql::server::config_entry (
Enum['present', 'absent'] $ensure = 'present',
Expand Down Expand Up @@ -70,10 +70,6 @@
'max_pred_locks_per_transaction' => undef,
}

Exec {
logoutput => 'on_failure',
}

if ! ($name in $requires_restart_until and (
! $requires_restart_until[$name] or
versioncmp($postgresql::server::_version, $requires_restart_until[$name]) < 0
Expand All @@ -91,73 +87,6 @@
}
}

# 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 $facts['os']['name'] == 'Debian' or $facts['os']['name'] == 'Ubuntu' {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer relevant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pretty sure this is no longer needed. And the systemd drop-in is not managed on debian based systems. I would rely on the tests here, but I can also test a bit on a debian based system

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really do wonder how often the data directory is changed. As I mentioned elsewhere, I'm not sure it's supported to have the data directory in postgresql.conf at all. So I think it may be ok to drop. Just a bit unsure about it.

Copy link
Collaborator Author

@SimonHoenscheid SimonHoenscheid Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this on many debian and ubuntu systems, changing the data directory in postgresql.conf works

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably because they store it in /etc rather than /var like Red Hat does (which is crazy).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just verified this on debian 10 with postgresql 11 (debians own package, default), changing port and data directory via postgresql.conf works fine.

if $name == 'data_directory' {
$stop_command = ['service', $postgresql::server::service_name, 'stop']
$stop_onlyif = ['service', $postgresql::server::service_name, 'status']
$stop_unless = [['grep', "data_directory = '${value}'", $postgresql::server::postgresql_conf_path]]
exec { "postgresql_stop_${name}":
command => $stop_command,
onlyif => $stop_onlyif,
unless => $stop_unless,
path => '/usr/sbin:/sbin:/bin:/usr/bin:/usr/local/bin',
before => Postgresql_conf[$name],
}
}
} elsif $facts['os']['family'] == 'RedHat' and versioncmp($facts['os']['release']['major'], '7') < 0 {
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.
$stop_command = ['service', $postgresql::server::service_name, 'stop']
$stop_onlyif = ['service', $postgresql::server::service_name, 'status']
$stop_unless = "grep 'PGPORT=${shell_escape($value)}' /etc/sysconfig/pgsql/postgresql"
exec { "postgresql_stop_${name}":
command => $stop_command,
onlyif => $stop_onlyif,
unless => $stop_unless,
path => '/sbin:/bin:/usr/bin:/usr/local/bin',
require => File['/etc/sysconfig/pgsql/postgresql'],
}
-> augeas { 'override PGPORT in /etc/sysconfig/pgsql/postgresql':
lens => 'Shellvars.lns',
incl => '/etc/sysconfig/pgsql/postgresql',
context => '/files/etc/sysconfig/pgsql/postgresql',
changes => "set PGPORT ${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
$stop_command = ['service', $postgresql::server::service_name, 'stop']
$stop_onlyif = ['service', $postgresql::server::service_name, 'status']
$stop_unless = [['grep', "PGDATA=${value}", '/etc/sysconfig/pgsql/postgresql']]
exec { "postgresql_${name}":
command => $stop_command,
onlyif => $stop_onlyif,
unless => $stop_unless,
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/postgresql',
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'],
}
}
}

postgresql_conf { $name:
ensure => $ensure,
target => $target,
Expand Down
19 changes: 0 additions & 19 deletions manifests/server/instance/config.pp
Original file line number Diff line number Diff line change
Expand Up @@ -216,25 +216,6 @@
}
}

# RedHat-based systems hardcode some PG* variables in the init script, and need to be overriden
# in /etc/sysconfig/pgsql/postgresql. Create a blank file so we can manage it with augeas later.
if $facts['os']['family'] == 'RedHat' and versioncmp($facts['os']['release']['major'], '7') < 0 {
file { '/etc/sysconfig/pgsql/postgresql':
ensure => file,
replace => false,
}

# The init script from the packages of the postgresql.org repository
# sources an alternate sysconfig file.
# I. e. /etc/sysconfig/pgsql/postgresql-9.3 for PostgreSQL 9.3
# Link to the sysconfig file set by this puppet module
file { "/etc/sysconfig/pgsql/postgresql-${version}":
ensure => link,
target => '/etc/sysconfig/pgsql/postgresql',
require => File['/etc/sysconfig/pgsql/postgresql'],
}
}

if ($manage_pg_ident_conf == true) {
concat { $pg_ident_conf_path:
owner => $user,
Expand Down