On 27.09.2019 17:28, Alvaro Herrera wrote:
+ # Now, when pg_rewind apparently succeeded with minimal
permissions,
+ # add REPLICATION privilege. So we could test that new standby
+ # is able to connect to the new master with generated config.
+ $node_standby->psql(
+ 'postgres', "ALTER ROLE rewind_user WITH REPLICATION;");
I think this better use safe_psql.
Yes, indeed.
On 30.09.2019 10:07, Paul Guo wrote:
2) Are you going to leave -R option completely without tap-tests?
Attached is a small patch, which tests -R option along with the
existing
'remote' case. If needed it may be split into two separate cases.
First,
it tests that pg_rewind is able to succeed with minimal permissions
according to the Michael's patch d9f543e [1]. Next, it checks
presence
of standby.signal and adds REPLICATION permission to rewind_user
to test
that new standby is able to start with generated recovery
configuration.
[1]
https://github.com/postgres/postgres/commit/d9f543e9e9be15f92abdeaf870e57ef289020191
It seems that we could further disabling recovery info setting code
for the 'remote' test case?
- my $port_standby = $node_standby->port;
- $node_master->append_conf(
- 'postgresql.conf', qq(
-primary_conninfo='port=$port_standby'
-));
+ if ($test_mode ne "remote")
+ {
+ my $port_standby = $node_standby->port;
+ $node_master->append_conf(
+ 'postgresql.conf',
+ qq(primary_conninfo='port=$port_standby'));
- $node_master->set_standby_mode();
+ $node_master->set_standby_mode();
+ }
Yeah, it makes sense. It is excessive for remote if we add '-R' there.
I've updated and attached my test adding patch.
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
>From b38bc7d71f7e7d68d66d3bf9af4e6371445aeab2 Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <kondratov.alek...@gmail.com>
Date: Fri, 27 Sep 2019 14:30:57 +0300
Subject: [PATCH v10 3/3] Test new standby start with generated config during
pg_rewind remote
---
src/bin/pg_rewind/t/001_basic.pl | 2 +-
src/bin/pg_rewind/t/002_databases.pl | 2 +-
src/bin/pg_rewind/t/003_extrafiles.pl | 2 +-
src/bin/pg_rewind/t/004_pg_xlog_symlink.pl | 2 +-
src/bin/pg_rewind/t/RewindTest.pm | 27 +++++++++++++++-------
5 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 115192170e..c3293e93df 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 10;
+use Test::More tests => 11;
use FindBin;
use lib $FindBin::RealBin;
diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl
index f1eb4fe1d2..1db534c0dc 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 6;
+use Test::More tests => 7;
use FindBin;
use lib $FindBin::RealBin;
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl
index c4040bd562..f4710440fc 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -3,7 +3,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 5;
use File::Find;
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index ed1ddb6b60..639eeb9c91 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -14,7 +14,7 @@ if ($windows_os)
}
else
{
- plan tests => 4;
+ plan tests => 5;
}
use FindBin;
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 68b6004e94..2b45c2789c 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -149,7 +149,7 @@ sub start_master
# Create custom role which is used to run pg_rewind, and adjust its
# permissions to the minimum necessary.
- $node_master->psql(
+ $node_master->safe_psql(
'postgres', "
CREATE ROLE rewind_user LOGIN;
GRANT EXECUTE ON function pg_catalog.pg_ls_dir(text, boolean, boolean)
@@ -266,9 +266,18 @@ sub run_pg_rewind
[
'pg_rewind', "--debug",
"--source-server", $standby_connstr,
- "--target-pgdata=$master_pgdata", "--no-sync"
+ "--target-pgdata=$master_pgdata", "-R", "--no-sync"
],
'pg_rewind remote');
+
+ # Check that standby.signal has been created.
+ ok(-e "$master_pgdata/standby.signal");
+
+ # Now, when pg_rewind apparently succeeded with minimal permissions,
+ # add REPLICATION privilege. So we could test that new standby
+ # is able to connect to the new master with generated config.
+ $node_standby->safe_psql(
+ 'postgres', "ALTER ROLE rewind_user WITH REPLICATION;");
}
else
{
@@ -289,13 +298,15 @@ sub run_pg_rewind
"unable to set permissions for $master_pgdata/postgresql.conf");
# Plug-in rewound node to the now-promoted standby node
- my $port_standby = $node_standby->port;
- $node_master->append_conf(
- 'postgresql.conf', qq(
-primary_conninfo='port=$port_standby'
-));
+ if ($test_mode ne "remote")
+ {
+ my $port_standby = $node_standby->port;
+ $node_master->append_conf(
+ 'postgresql.conf', qq(
+primary_conninfo='port=$port_standby'));
- $node_master->set_standby_mode();
+ $node_master->set_standby_mode();
+ }
# Restart the master to check that rewind went correctly
$node_master->start;
--
2.17.1
