From 6353e9428df1d241b97d02c843f1a737e7c36c85 Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 12 Feb 2020 17:18:48 -0700 Subject: [PATCH] Error when archive-get/archive-push/restore are not run on a PostgreSQL host. This error was lost during the migration to C. The error that occurred instead (generally an SSH auth error) was hard to debug. Restore the original behavior by throwing an error immediately if pg1-host is configured for any of these commands. reset-pg1-host can be used to suppress the error when required. --- doc/xml/release.xml | 9 +++++++ src/command/archive/get/get.c | 6 +++++ src/command/archive/push/push.c | 6 +++++ test/src/module/command/archiveGetTest.c | 31 ++++++++++++++++++++++- test/src/module/command/archivePushTest.c | 25 +++++++++++++++++- 5 files changed, 75 insertions(+), 2 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index cefa6a0a4..46ae16d1a 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -25,6 +25,15 @@

Prevent defunct processes in asynchronous archive commands.

+ + + + + + +

Error when archive-get/archive-push/restore are not run on a host.

+ + diff --git a/src/command/archive/get/get.c b/src/command/archive/get/get.c index 8f425c4d0..2bed5682e 100644 --- a/src/command/archive/get/get.c +++ b/src/command/archive/get/get.c @@ -107,6 +107,9 @@ cmdArchiveGet(void) { FUNCTION_LOG_VOID(logLevelDebug); + // PostgreSQL must be local + pgIsLocalVerify(); + // Set the result assuming the archive file will not be found int result = 1; @@ -315,6 +318,9 @@ cmdArchiveGetAsync(void) { FUNCTION_LOG_VOID(logLevelDebug); + // PostgreSQL must be local + pgIsLocalVerify(); + MEM_CONTEXT_TEMP_BEGIN() { TRY_BEGIN() diff --git a/src/command/archive/push/push.c b/src/command/archive/push/push.c index d08d630f0..8e36c5b98 100644 --- a/src/command/archive/push/push.c +++ b/src/command/archive/push/push.c @@ -257,6 +257,9 @@ cmdArchivePush(void) ASSERT(cfgCommand() == cfgCmdArchivePush); + // PostgreSQL must be local + pgIsLocalVerify(); + MEM_CONTEXT_TEMP_BEGIN() { // Make sure there is a parameter to retrieve the WAL segment from @@ -428,6 +431,9 @@ cmdArchivePushAsync(void) ASSERT(cfgCommand() == cfgCmdArchivePush && cfgCommandRole() == cfgCmdRoleAsync); + // PostgreSQL must be local + pgIsLocalVerify(); + MEM_CONTEXT_TEMP_BEGIN() { // Make sure there is a parameter with the wal path diff --git a/test/src/module/command/archiveGetTest.c b/test/src/module/command/archiveGetTest.c index 17aa03018..423220980 100644 --- a/test/src/module/command/archiveGetTest.c +++ b/test/src/module/command/archiveGetTest.c @@ -330,6 +330,21 @@ testRun(void) TEST_ERROR(cmdArchiveGetAsync(), ParamInvalidError, "at least one wal segment is required"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("command must be run on the pg host"); + + StringList *argList = strLstNew(); + strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host"); + strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg"); + strLstAddZ(argList, "--" CFGOPT_REPO1_PATH "=/repo"); + strLstAddZ(argList, "--" CFGOPT_SPOOL_PATH "=/spool"); + strLstAddZ(argList, "--" CFGOPT_ARCHIVE_ASYNC); + strLstAddZ(argList, "--" CFGOPT_STANZA "=test2"); + strLstAddZ(argList, "000000010000000100000001"); + harnessCfgLoadRole(cfgCmdArchiveGet, cfgCmdRoleAsync, argList); + + TEST_ERROR(cmdArchiveGetAsync(), HostInvalidError, "archive-get command must be run on the PostgreSQL host"); + // Create pg_control file and archive.info // ------------------------------------------------------------------------------------------------------------------------- storagePutP( @@ -347,7 +362,7 @@ testRun(void) // Get a single segment // ------------------------------------------------------------------------------------------------------------------------- - StringList *argList = strLstDup(argCleanList); + argList = strLstDup(argCleanList); strLstAddZ(argList, "000000010000000100000001"); harnessCfgLoadRole(cfgCmdArchiveGet, cfgCmdRoleAsync, argList); @@ -474,7 +489,21 @@ testRun(void) // ***************************************************************************************************************************** if (testBegin("cmdArchiveGet()")) { + TEST_TITLE("command must be run on the pg host"); + StringList *argList = strLstNew(); + strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host"); + strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg"); + strLstAddZ(argList, "--" CFGOPT_REPO1_PATH "=/repo"); + strLstAddZ(argList, "--" CFGOPT_STANZA "=test2"); + strLstAddZ(argList, "000000010000000100000001"); + strLstAddZ(argList, "pg_wal/000000010000000100000001"); + harnessCfgLoadRole(cfgCmdArchiveGet, cfgCmdRoleDefault, argList); + + TEST_ERROR(cmdArchiveGet(), HostInvalidError, "archive-get command must be run on the PostgreSQL host"); + + // ------------------------------------------------------------------------------------------------------------------------- + argList = strLstNew(); strLstAddZ(argList, "pgbackrest-bogus"); // Break this until async tests are setup correctly strLstAddZ(argList, "--archive-timeout=1"); strLstAdd(argList, strNewFmt("--lock-path=%s/lock", testPath())); diff --git a/test/src/module/command/archivePushTest.c b/test/src/module/command/archivePushTest.c index db6bbf158..0589dfcd2 100644 --- a/test/src/module/command/archivePushTest.c +++ b/test/src/module/command/archivePushTest.c @@ -176,7 +176,18 @@ testRun(void) // ***************************************************************************************************************************** if (testBegin("Synchronous cmdArchivePush(), archivePushFile() and archivePushProtocol()")) { + TEST_TITLE("command must be run on the pg host"); + StringList *argList = strLstNew(); + strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host"); + strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg"); + strLstAddZ(argList, "--" CFGOPT_STANZA "=test2"); + harnessCfgLoadRole(cfgCmdArchivePush, cfgCmdRoleDefault, argList); + + TEST_ERROR(cmdArchivePush(), HostInvalidError, "archive-push command must be run on the PostgreSQL host"); + + // ------------------------------------------------------------------------------------------------------------------------- + argList = strLstNew(); strLstAddZ(argList, "--stanza=test"); harnessCfgLoad(cfgCmdArchivePush, argList); @@ -412,9 +423,21 @@ testRun(void) { harnessLogLevelSet(logLevelDetail); + TEST_TITLE("command must be run on the pg host"); + + StringList *argList = strLstNew(); + strLstAddZ(argList, "--" CFGOPT_PG1_HOST "=host"); + strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/pg"); + strLstAddZ(argList, "--" CFGOPT_SPOOL_PATH "=/spool"); + strLstAddZ(argList, "--" CFGOPT_STANZA "=test2"); + strLstAddZ(argList, "--" CFGOPT_ARCHIVE_ASYNC); + harnessCfgLoadRole(cfgCmdArchivePush, cfgCmdRoleAsync, argList); + + TEST_ERROR(cmdArchivePush(), HostInvalidError, "archive-push command must be run on the PostgreSQL host"); + // Call with a bogus exe name so the async process will error out and we can make sure timeouts work // ------------------------------------------------------------------------------------------------------------------------- - StringList *argList = strLstNew(); + argList = strLstNew(); strLstAddZ(argList, "pgbackrest-bogus"); strLstAddZ(argList, "--stanza=test"); strLstAddZ(argList, "--archive-async");