[pmg-devel] [PATCH pmg-api 2/3] add get_pg_server_version in PMG::Utils

[Fabian Grünbichler]

On August 8, 2019 9:14 am, Stoiko Ivanov wrote:
> PMG renders the postgresql.conf through its templating system (currently the
> shipped template does not use any variables). postgresql.conf (in most
> installations and in both debian and upstream packages) contains a few
> occurrences (datadir, config files, pid-file, cluster name) of the postgres
> major version number (see [0], for a description and why 9.6 and 11 are major
> version numbers). The rendered config should use the correct version number
> for the config of the currently used postgres installation (the one listening
> on the default port (5432) and socket).
> 
> This fixes a bug observed while testing the upgrade to buster and postgres 11:
> * a long running service (pmgmirror, pmgdaemon) still has the old config
>   path in memory (/etc/postgresql/9.6/)
> * while upgrading the pmg-api package the shipped template changes to one
>   with the new major number (11)
> * the next restart of the postgresql cluster fails, with an error not directly
>   related to the broken config file
> 
> By reading [1] the version number through a connection to the current
> postgresql server we rewrite the fitting configfile with the correct paths.
> 
> [0] https://www.postgresql.org/support/versioning/
> [1] https://www.postgresql.org/docs/11/runtime-config-preset.html
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>  src/PMG/Utils.pm | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/PMG/Utils.pm b/src/PMG/Utils.pm
> index 103982e..a773f9d 100644
> --- a/src/PMG/Utils.pm
> +++ b/src/PMG/Utils.pm
> @@ -41,6 +41,7 @@ use base 'Exporter';
>  
>  our @EXPORT_OK = qw(
>  postgres_admin_cmd,
> +get_pg_server_version,

two callers, PMG::Utils is short, no need to export ;)

>  );
>  
>  my $valid_pmg_realms = ['pam', 'pmg', 'quarantine'];
> @@ -1401,4 +1402,20 @@ sub postgres_admin_cmd {
>  	die "setresuid back failed - $!\n";
>  }
>  
> +sub get_pg_server_version {
> +    my $major_ver;
> +    my $parser = sub {
> +	my $line = shift;
> +	$line = PVE::Tools::trim($line);
> +	# example output:
> +	# 9.6.13
> +	# 11.4 (Debian 11.4-1)
> +	($major_ver) = ($line =~ m/^([0-9]+(?:\.[0-9]+)?)\.[0-9]+/);

this will break if postgres ever releases a hotfix release 11.x.y. IMHO 
it would be better to get the first element, and then based on that map

 < 10: $major_ver = first and second element (if available)
 >= 10: $major_ver = first element

since it is unlikely that the major version will change without us 
knowing, this seems safer. a 11.4.1 release might happen for some very 
imporant security fix, who knows ;)

> +    };
> +    postgres_admin_cmd('psql', { outfunc => $parser }, '--quiet',
> +	'--tuples-only', '--no-align', '--command', 'show server_version;');

like Dominik said, die here if $major_ver is undef (and maybe wrap the 
postgres_admin_cmd in an eval as well, to get a uniform error message 
prefix for failure to run command, and failure to parse version string?)

> +
> +    return $major_ver;
> +}
> +
>  1;
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
>