From 057e2e27822ffd053f735762e1441fccea24c5ed Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 9 Feb 2019 18:57:30 +0200 Subject: [PATCH] Add unimplemented S3 driver method required for archive-get. This was not being caught because the integration tests for S3 were running remotely and going through the Perl code rather than the new C code. Implement the exists method for the S3 driver and add tests to prevent a regression. Reported by mibiio. --- doc/xml/release.xml | 13 +++++ src/storage/driver/s3/storage.c | 30 +++++++++++- test/expect/mock-archive-001.log | 18 +++++++ test/expect/mock-archive-002.log | 18 +++++++ test/expect/mock-archive-003.log | 9 ++++ .../Module/Mock/MockArchiveTest.pm | 39 +++++++++++++++ test/src/module/storage/s3Test.c | 49 ++++++++++++++++++- 7 files changed, 174 insertions(+), 2 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 4fedeb6ba..b987b848a 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -15,6 +15,14 @@ + + + + + +

Add unimplemented S3 driver method required for archive-get.

+ + @@ -6389,6 +6397,11 @@ mtkunkel + + mibiio + mibiio + + Michael Renner terrorobe diff --git a/src/storage/driver/s3/storage.c b/src/storage/driver/s3/storage.c index 2ab4bc81e..6260e0f5d 100644 --- a/src/storage/driver/s3/storage.c +++ b/src/storage/driver/s3/storage.c @@ -395,7 +395,35 @@ storageDriverS3Exists(StorageDriverS3 *this, const String *path) bool result = false; - THROW(AssertError, "NOT YET IMPLEMENTED"); + MEM_CONTEXT_TEMP_BEGIN() + { + HttpQuery *query = httpQueryNew(); + + // Generate the file name as a prefix. Muliple files may be returned but this will narrow down the list. + String *prefix = strNewFmt("%s", strPtr(strSub(path, 1))); + httpQueryAdd(query, S3_QUERY_PREFIX_STR, prefix); + + // Build the query using list type 2 + httpQueryAdd(query, S3_QUERY_LIST_TYPE_STR, S3_QUERY_VALUE_LIST_TYPE_2_STR); + + XmlNode *xmlRoot = xmlDocumentRoot( + xmlDocumentNewBuf(storageDriverS3Request(this, HTTP_VERB_GET_STR, FSLASH_STR, query, NULL, true, false))); + + // Check if the prefix exists. If not then the file definitely does not exist, but if it does we'll need to check the + // exact name to be sure we are not looking at a different file with the same prefix + XmlNodeList *fileList = xmlNodeChildList(xmlRoot, S3_XML_TAG_CONTENTS_STR); + + for (unsigned int fileIdx = 0; fileIdx < xmlNodeLstSize(fileList); fileIdx++) + { + // If the name matches exactly then the file exists + if (strEq(prefix, xmlNodeContent(xmlNodeChild(xmlNodeLstGet(fileList, fileIdx), S3_XML_TAG_KEY_STR, true)))) + { + result = true; + break; + } + } + } + MEM_CONTEXT_TEMP_END(); FUNCTION_LOG_RETURN(BOOL, result); } diff --git a/test/expect/mock-archive-001.log b/test/expect/mock-archive-001.log index b4de9b418..cf8776b7c 100644 --- a/test/expect/mock-archive-001.log +++ b/test/expect/mock-archive-001.log @@ -222,6 +222,12 @@ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-maste P00 INFO: pushed WAL segment 000000010000000100000002 asynchronously P00 INFO: archive-push command end: completed successfully +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push --archive-async [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history +------------------------------------------------------------------------------------------------------------------------------------ +P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/00000002.history] --archive-async --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --spool-path=[TEST_PATH]/db-master/spool --stanza=db +P00 INFO: pushed WAL segment 00000002.history asynchronously +P00 INFO: archive-push command end: completed successfully + > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002 ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002] --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --stanza=db @@ -298,6 +304,18 @@ P00 INFO: archive-get command begin [BACKREST-VERSION]: [000000010000000100000 P00 INFO: found 000000010000000100000002 in the archive P00 INFO: archive-get command end: completed successfully +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async 00000001.history [TEST_PATH]/db-master/db/base/pg_xlog/00000001.history +------------------------------------------------------------------------------------------------------------------------------------ +P00 INFO: archive-get command begin [BACKREST-VERSION]: [00000001.history, [TEST_PATH]/db-master/db/base/pg_xlog/00000001.history] --archive-async --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --spool-path=[TEST_PATH]/db-master/spool --stanza=db +P00 INFO: unable to find 00000001.history in the archive +P00 INFO: archive-get command end: completed successfully + +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async 00000002.history [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history +------------------------------------------------------------------------------------------------------------------------------------ +P00 INFO: archive-get command begin [BACKREST-VERSION]: [00000002.history, [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history] --archive-async --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --spool-path=[TEST_PATH]/db-master/spool --stanza=db +P00 INFO: found 00000002.history in the archive +P00 INFO: archive-get command end: completed successfully + > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002.partial ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002.partial] --no-compress --compress-level=3 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-cipher-pass= --repo1-cipher-type=aes-256-cbc --repo1-path=[TEST_PATH]/db-master/repo --stanza=db diff --git a/test/expect/mock-archive-002.log b/test/expect/mock-archive-002.log index b5077935e..a0a1f711f 100644 --- a/test/expect/mock-archive-002.log +++ b/test/expect/mock-archive-002.log @@ -197,6 +197,12 @@ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-maste P00 INFO: pushed WAL segment 000000010000000100000002 asynchronously P00 INFO: archive-push command end: completed successfully +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push --archive-async [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history +------------------------------------------------------------------------------------------------------------------------------------ +P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/00000002.history] --archive-async --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-host=backup --repo1-host-cmd=[BACKREST-BIN] --repo1-host-config=[TEST_PATH]/backup/pgbackrest.conf --repo1-host-user=[USER-1] --spool-path=[TEST_PATH]/db-master/spool --stanza=db +P00 INFO: pushed WAL segment 00000002.history asynchronously +P00 INFO: archive-push command end: completed successfully + > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002 ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002] --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-host=backup --repo1-host-cmd=[BACKREST-BIN] --repo1-host-config=[TEST_PATH]/backup/pgbackrest.conf --repo1-host-user=[USER-1] --stanza=db @@ -273,6 +279,18 @@ P00 INFO: archive-get command begin [BACKREST-VERSION]: [000000010000000100000 P00 INFO: found 000000010000000100000002 in the archive P00 INFO: archive-get command end: completed successfully +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async 00000001.history [TEST_PATH]/db-master/db/base/pg_xlog/00000001.history +------------------------------------------------------------------------------------------------------------------------------------ +P00 INFO: archive-get command begin [BACKREST-VERSION]: [00000001.history, [TEST_PATH]/db-master/db/base/pg_xlog/00000001.history] --archive-async --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-host=backup --repo1-host-cmd=[BACKREST-BIN] --repo1-host-config=[TEST_PATH]/backup/pgbackrest.conf --repo1-host-user=[USER-1] --spool-path=[TEST_PATH]/db-master/spool --stanza=db +P00 INFO: unable to find 00000001.history in the archive +P00 INFO: archive-get command end: completed successfully + +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async 00000002.history [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history +------------------------------------------------------------------------------------------------------------------------------------ +P00 INFO: archive-get command begin [BACKREST-VERSION]: [00000002.history, [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history] --archive-async --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-host=backup --repo1-host-cmd=[BACKREST-BIN] --repo1-host-config=[TEST_PATH]/backup/pgbackrest.conf --repo1-host-user=[USER-1] --spool-path=[TEST_PATH]/db-master/spool --stanza=db +P00 INFO: found 00000002.history in the archive +P00 INFO: archive-get command end: completed successfully + > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002.partial ------------------------------------------------------------------------------------------------------------------------------------ P00 INFO: archive-push command begin [BACKREST-VERSION]: [[TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002.partial] --no-compress --compress-level=3 --compress-level-network=1 --config=[TEST_PATH]/db-master/pgbackrest.conf --db-timeout=45 --lock-path=[TEST_PATH]/db-master/lock --log-level-console=detail --log-level-file=trace --log-level-stderr=off --log-path=[TEST_PATH]/db-master/log --log-subprocess --no-log-timestamp --pg1-path=[TEST_PATH]/db-master/db/base --protocol-timeout=60 --repo1-host=backup --repo1-host-cmd=[BACKREST-BIN] --repo1-host-config=[TEST_PATH]/backup/pgbackrest.conf --repo1-host-user=[USER-1] --stanza=db diff --git a/test/expect/mock-archive-003.log b/test/expect/mock-archive-003.log index 13aab84e2..51296adf8 100644 --- a/test/expect/mock-archive-003.log +++ b/test/expect/mock-archive-003.log @@ -69,6 +69,9 @@ db-version="9.4" > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push --compress --archive-async --process-max=2 [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002 ------------------------------------------------------------------------------------------------------------------------------------ +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push --archive-async [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history +------------------------------------------------------------------------------------------------------------------------------------ + > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002 ------------------------------------------------------------------------------------------------------------------------------------ P00 ERROR: [044]: WAL segment version 9.4 does not match archive version 8.0 @@ -118,6 +121,12 @@ P00 ERROR: [045]: WAL segment 000000010000000100000002 already exists in the ar > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async --archive-timeout=5 000000010000000100000002 [TEST_PATH]/db-master/db/base/pg_xlog/RECOVERYXLOG ------------------------------------------------------------------------------------------------------------------------------------ +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async 00000001.history [TEST_PATH]/db-master/db/base/pg_xlog/00000001.history +------------------------------------------------------------------------------------------------------------------------------------ + +> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-get --archive-async 00000002.history [TEST_PATH]/db-master/db/base/pg_xlog/00000002.history +------------------------------------------------------------------------------------------------------------------------------------ + > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --stanza=db archive-push [TEST_PATH]/db-master/db/base/pg_xlog/000000010000000100000002.partial ------------------------------------------------------------------------------------------------------------------------------------ diff --git a/test/lib/pgBackRestTest/Module/Mock/MockArchiveTest.pm b/test/lib/pgBackRestTest/Module/Mock/MockArchiveTest.pm index 0bb56ae64..079960bc2 100644 --- a/test/lib/pgBackRestTest/Module/Mock/MockArchiveTest.pm +++ b/test/lib/pgBackRestTest/Module/Mock/MockArchiveTest.pm @@ -159,6 +159,9 @@ sub run # Test that the WAL was pushed $self->archiveCheck($strSourceFile, $strArchiveChecksum, true); + # Remove from archive_status + storageTest()->remove("${strWalPath}/archive_status/${strSourceFile}.ready"); + # Remove WAL storageTest()->remove("${strWalPath}/${strSourceFile}", {bIgnoreMissing => false}); @@ -247,9 +250,29 @@ sub run # Test that the WAL was pushed $self->archiveCheck($strSourceFile, $strArchiveChecksum, true, $oHostDbMaster->spoolPath()); + # Remove from archive_status + storageTest()->remove("${strWalPath}/archive_status/${strSourceFile}.ready"); + # Remove from spool storageTest()->remove($oHostDbMaster->spoolPath() . '/archive/' . $self->stanza() . "/out/${strSourceFile}.ok"); + #--------------------------------------------------------------------------------------------------------------------------- + &log(INFO, ' push history file'); + + storageTest()->put("${strWalPath}/00000002.history", 'HISTORYDATA'); + storageTest()->put("${strWalPath}/archive_status/00000002.history.ready", undef); + + $oHostDbMaster->executeSimple( + "${strCommandPush} --archive-async ${strWalPath}/00000002.history", + {oLogTest => $self->expect()}); + + if (!storageRepo()->exists(STORAGE_REPO_ARCHIVE . qw{/} . PG_VERSION_94 . '-1/00000002.history')) + { + confess 'unable to find history file in archive'; + } + + storageTest()->remove("${strWalPath}/archive_status/00000002.history.ready"); + #--------------------------------------------------------------------------------------------------------------------------- &log(INFO, ' db version mismatch in db section only - archive-push errors but archive-get succeeds'); @@ -374,6 +397,22 @@ sub run confess "archive file '${strWalPath}/RECOVERYXLOG' is not in destination"; } + #--------------------------------------------------------------------------------------------------------------------------- + &log(INFO, " get history file"); + + $oHostDbMaster->executeSimple( + $strCommandGet . " --archive-async 00000001.history ${strWalPath}/00000001.history", + {iExpectedExitStatus => 1, oLogTest => $self->expect()}); + + $oHostDbMaster->executeSimple( + $strCommandGet . " --archive-async 00000002.history ${strWalPath}/00000002.history", + {oLogTest => $self->expect()}); + + if (${storageDb()->get("${strWalPath}/00000002.history")} ne 'HISTORYDATA') + { + confess 'history contents do not match original'; + } + #--------------------------------------------------------------------------------------------------------------------------- &log(INFO, ' .partial WAL'); diff --git a/test/src/module/storage/s3Test.c b/test/src/module/storage/s3Test.c index 088c899a4..e4d8f08fb 100644 --- a/test/src/module/storage/s3Test.c +++ b/test/src/module/storage/s3Test.c @@ -77,6 +77,47 @@ testS3Server(void) harnessTlsServerExpect(testS3ServerRequest(HTTP_VERB_GET, "/file.txt")); harnessTlsServerReply(testS3ServerResponse(303, "Some bad status", "CONTENT")); + // storageDriverExists() + // ------------------------------------------------------------------------------------------------------------------------- + // File missing + harnessTlsServerExpect(testS3ServerRequest(HTTP_VERB_GET, "/?list-type=2&prefix=BOGUS")); + harnessTlsServerReply( + testS3ServerResponse( + 200, "OK", + "" + "" + "")); + + // File exists + harnessTlsServerExpect(testS3ServerRequest(HTTP_VERB_GET, "/?list-type=2&prefix=subdir%2Ffile1.txt")); + harnessTlsServerReply( + testS3ServerResponse( + 200, "OK", + "" + "" + " " + " subdir/file1.txts" + " " + " " + " subdir/file1.txt" + " " + "")); + + // File does not exist but files with same prefix do + harnessTlsServerExpect(testS3ServerRequest(HTTP_VERB_GET, "/?list-type=2&prefix=subdir%2Ffile1.txt")); + harnessTlsServerReply( + testS3ServerResponse( + 200, "OK", + "" + "" + " " + " subdir/file1.txta" + " " + " " + " subdir/file1.txtb" + " " + "")); + // storageDriverList() // ------------------------------------------------------------------------------------------------------------------------- // Throw error @@ -380,6 +421,13 @@ testRun(void) "*** Response Content ***:\n" "CONTENT") + // storageDriverExists() + // ------------------------------------------------------------------------------------------------------------------------- + TEST_RESULT_BOOL(storageExistsNP(s3, strNew("BOGUS")), false, "file does not exist"); + TEST_RESULT_BOOL(storageExistsNP(s3, strNew("subdir/file1.txt")), true, "file exists"); + TEST_RESULT_BOOL( + storageExistsNP(s3, strNew("subdir/file1.txt")), false, "file does not exist but files with same prefix do"); + // storageDriverList() // ------------------------------------------------------------------------------------------------------------------------- TEST_ERROR( @@ -408,7 +456,6 @@ testRun(void) // Coverage for unimplemented functions // ------------------------------------------------------------------------------------------------------------------------- - TEST_ERROR(storageExistsNP(s3, strNew("file.txt")), AssertError, "NOT YET IMPLEMENTED"); TEST_ERROR(storageInfoNP(s3, strNew("file.txt")), AssertError, "NOT YET IMPLEMENTED"); TEST_ERROR(storageNewWriteNP(s3, strNew("file.txt")), AssertError, "NOT YET IMPLEMENTED"); TEST_ERROR(storagePathRemoveNP(s3, strNew("path")), AssertError, "NOT YET IMPLEMENTED");