Skip to content

Commit

Permalink
fixed bug that downloaded all cache manifests to same location
Browse files Browse the repository at this point in the history
  • Loading branch information
Chamith Edirisinghe authored and smiklosovic committed Oct 25, 2021
1 parent 5f89acb commit 87da57c
Show file tree
Hide file tree
Showing 18 changed files with 115 additions and 47 deletions.
8 changes: 2 additions & 6 deletions src/main/java/com/instaclustr/esop/azure/AzureRestorer.java
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,8 @@ public void downloadManifestsToDirectory(Path downloadDir) throws Exception {
FileUtils.cleanDirectory(downloadDir.toFile());
final List<String> manifestKeys = getBlobPaths(list(""), s -> s.contains("manifests"));
for (String o: manifestKeys) {
final Path manifestPath = Paths.get(o);
final Path manifestName = manifestPath.getFileName();

final Path destination = downloadDir.resolve(getStorageLocation().nodePath())
.resolve("manifests")
.resolve(manifestName);
Path manifestPath = Paths.get(o).subpath(1, 6);
Path destination = downloadDir.resolve(manifestPath);
downloadFile(destination, objectKeyToRemoteReference(manifestPath));
}
}
Expand Down
6 changes: 1 addition & 5 deletions src/main/java/com/instaclustr/esop/gcp/GCPRestorer.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,7 @@ public void downloadManifestsToDirectory(Path downloadDir) throws Exception {
s -> s.contains("manifests"));
for (String o: manifestKeys) {
Path manifestPath = Paths.get(o);
Path manifestName = manifestPath.getFileName();

Path destination = downloadDir.resolve(getStorageLocation().nodePath())
.resolve("manifests")
.resolve(manifestName);
Path destination = downloadDir.resolve(manifestPath);

downloadFile(destination, objectKeyToRemoteReference(manifestPath));
}
Expand Down
6 changes: 1 addition & 5 deletions src/main/java/com/instaclustr/esop/s3/BaseS3Restorer.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,7 @@ public void downloadManifestsToDirectory(Path downloadDir) throws Exception {
final List<S3ObjectSummary> manifestSumms = listBucket("", s -> s.contains("manifests"));
for (S3ObjectSummary o : manifestSumms) {
Path manifestPath = Paths.get(o.getKey());
Path manifestName = manifestPath.getFileName();

Path destination = downloadDir.resolve(getStorageLocation().nodePath())
.resolve("manifests")
.resolve(manifestName);
Path destination = downloadDir.resolve(manifestPath);

downloadFile(destination, objectKeyToRemoteReference(manifestPath));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,17 @@ protected String[][] hardlinkingArguments(final String cassandraVersion) {
"--create-missing-bucket"
};

final String[] backupArgs2WithSnapshotName = new String[]{
"backup",
"--jmx-service", "127.0.0.1:7199",
"--storage-location=" + getStorageLocationForAnotherCluster(),
"--snapshot-tag=" + snapshotName,
"--data-directory=" + cassandraDir.toAbsolutePath() + "/data",
"--entities=" + systemKeyspace(cassandraVersion) + ",test,test2", // keyspaces
"--k8s-secret-name=" + SIDECAR_SECRET_NAME,
"--create-missing-bucket"
};

// RESTORE

final String[] downloadPhase = new String[]{
Expand Down Expand Up @@ -511,6 +522,7 @@ protected String[][] hardlinkingArguments(final String cassandraVersion) {
truncatePhase,
importPhase,
cleanupPhase,
backupArgs2WithSnapshotName
};
}

Expand Down Expand Up @@ -613,7 +625,15 @@ protected String[][] restoreByImportingIntoDifferentSchemaArguments(final String
};
}

protected abstract String getStorageLocation();
protected String getStorageLocation() {
return protocol() + BUCKET_NAME + "/cluster/datacenter1/node1";
}

protected String getStorageLocationForAnotherCluster() {
return protocol() + BUCKET_NAME + "/cluster2/datacenter1/node1";
}

protected abstract String protocol();

public void inPlaceBackupRestoreTest(final String[][] arguments) throws Exception {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public void testListingAndBackup() throws Exception {

insertAndCallBackupCLI(2, session, arguments[0]); // stefansnapshot-1
insertAndCallBackupCLI(2, session, arguments[1]); // stefansnapshot-2
insertAndCallBackupCLI(2, session, arguments[6]); // stefansnapshot-2 in different storage location


try {
logger.info("Executing the first restoration phase - download {}", asList(arguments[2]));
Expand All @@ -106,6 +108,9 @@ public void testListingAndBackup() throws Exception {
final Path jsonSimpleFile = createTempFile("esop-backup-json-simple", null);
final Path tableComplexFile = createTempFile("esop-backup-table-complex", null);
final Path tableSimpleFile = createTempFile("esop-backup-table-simple", null);
// File for backup in different storage location
final Path jsonComplexFile2 = createTempFile("esop-backup-json-complex2", null);


final String[] jsonComplex = new String[]{
"list",
Expand Down Expand Up @@ -147,6 +152,16 @@ public void testListingAndBackup() throws Exception {
"--cache-dir=" + target(".esop")
};

final String[] jsonComplex2 = new String[]{
"list",
"--storage-location=" + getStorageLocationForAnotherCluster(),
"--skip-node-resolution",
"--human-units",
"--simple-format",
"--to-file=" + jsonComplexFile2.toAbsolutePath(),
"--cache-dir=" + target(".esop")
};

logger.info("Executing listing of json complex format: " + asList(jsonComplex));
Esop.mainWithoutExit(jsonComplex);
logger.info("Executing listing of json simple format: " + asList(jsonSimple));
Expand All @@ -155,6 +170,8 @@ public void testListingAndBackup() throws Exception {
Esop.mainWithoutExit(tableComplex);
logger.info("Executing listing of table simple format: " + asList(tableSimple));
Esop.mainWithoutExit(tableSimple);
logger.info("Executing listing of json complex format (2): " + asList(jsonComplexFile2));
Esop.mainWithoutExit(jsonComplex2);

Manifest.AllManifestsReport report = objectMapper.readValue(Files.readAllBytes(jsonComplexFile), Manifest.AllManifestsReport.class);

Expand Down Expand Up @@ -198,9 +215,38 @@ public void testListingAndBackup() throws Exception {
// delete latest
Esop.mainWithoutExit(delete2);

// we basically deleted everything by deleting both backups
// we basically deleted everything in the first storage location by deleting first two backups
assertEquals(Files.list(Paths.get(getStorageLocation().replaceAll(protocol() + "://", ""), "data")).count(), 0);
assertEquals(Files.list(Paths.get(getStorageLocation().replaceAll(protocol() + "://", ""), "manifests")).count(), 0);

Manifest.AllManifestsReport report2 = objectMapper.readValue(Files.readAllBytes(jsonComplexFile2), Manifest.AllManifestsReport.class);

Optional<Manifest.ManifestReporter.ManifestReport> oldest2 = report2.getOldest();

if (oldest2.isPresent()) {
Assert.fail("Not found the oldest report!");
}

final String oldestBackupName2 = oldest2.get().name;

final String[] delete3 = new String[]{
"remove-backup",
"--storage-location=" + getStorageLocationForAnotherCluster(),
"--skip-node-resolution",
"--backup-name=" + oldestBackupName2,
"--cache-dir=" + target(".esop")
};

logger.info("Executing the third delete - {}", asList(delete3));

// delete backup in different storage location
Esop.mainWithoutExit(delete3);

// check if third backup was actually deleted
assertEquals(Files.list(Paths.get(getStorageLocationForAnotherCluster().replaceAll(protocol() + "://", ""), "data")).count(), 0);
assertEquals(Files.list(Paths.get(getStorageLocationForAnotherCluster().replaceAll(protocol() + "://", ""), "manifests")).count(), 0);


} finally {
cassandra.stop();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ protected String getStorageLocation() {
return protocol() + "://" + BUCKET_NAME + "/cluster/datacenter1/node1";
}

@Override
protected String getStorageLocationForAnotherCluster() {
return protocol() + "://" + BUCKET_NAME + "/cluster2/datacenter1/node1";
}

@Override
protected String protocol() {
return "azure";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public void inject() {
}

@Override
protected String getStorageLocation() {
return "azure://" + BUCKET_NAME + "/cluster/datacenter1/node1";
protected String protocol() {
return "azure://";
}

public void inPlaceTest(final String[][] programArguments) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public void inject() {
}

@Override
protected String getStorageLocation() {
return "gcp://" + BUCKET_NAME + "/cluster/datacenter1/node1";
protected String protocol() {
return "gcp://";
}

public void inPlaceTest(final String[][] programArguments) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ protected String getStorageLocation() {
return protocol() + "://" + BUCKET_NAME + "/cluster/datacenter1/node1";
}

protected String getStorageLocationForAnotherCluster() {
return protocol() + "://" + BUCKET_NAME + "/cluster2/datacenter1/node1";
}


@Override
protected String protocol() {
return "gcp";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ public class LocalBackupTest extends AbstractBackupTest {
@Inject
private OperationsService operationsService;

@Override
protected String protocol() {
return "file://";
}

@BeforeMethod
public void setup() throws Exception {

Expand Down Expand Up @@ -196,6 +201,10 @@ public void testDownload() throws Exception {
protected String getStorageLocation() {
return "file://" + target("backup1") + "/cluster/datacenter1/node1";
}
@Override
protected String getStorageLocationForAnotherCluster() {
return "file://" + target("backup1") + "/cluster2/datacenter1/node1";
}

@Test
public void testUploadTracker() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,17 @@ protected List<Module> getModules() {

@Override
protected String getStorageLocation() {
return protocol() + "://" + target(BUCKET_NAME) + "/cluster/datacenter1/node1";
return protocol() + target(BUCKET_NAME) + "/cluster/datacenter1/node1";
}

@Override
protected String getStorageLocationForAnotherCluster() {
return protocol() + target(BUCKET_NAME) + "/cluster2/datacenter1/node1";
}

@Override
protected String protocol() {
return "file";
return "file://";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ public void teardown() throws Exception {
destroy();
}

@Override
protected String getStorageLocation() {
return "s3://" + BUCKET_NAME + "/cluster/datacenter1/node1";
}

@Override
public S3TransferManagerFactory getTransferManagerFactory() {
return transferManagerFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ public abstract class BaseAWSS3BackupRestoreTest extends AbstractBackupTest {

protected abstract BackupOperationRequest getBackupOperationRequest();

@Override
protected String protocol() {
return "s3://";
}

public void inject() {
final List<Module> modules = new ArrayList<Module>() {{
add(new KubernetesApiModule());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ public void teardown() throws ApiException {
destroy();
}

@Override
protected String getStorageLocation() {
return "s3://" + BUCKET_NAME + "/cluster/datacenter1/node1";
}

@Override
protected void init() throws ApiException {
System.setProperty("kubernetes.client", "true");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,9 @@ protected List<Module> getModules() {
}};
}

@Override
protected String getStorageLocation() {
return protocol() + "://" + BUCKET_NAME + "/cluster/datacenter1/node1";
}

@Override
protected String protocol() {
return "s3";
return "s3://";
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ public abstract class BaseCephS3BackupRestoreTest extends AbstractBackupTest {

protected abstract BackupOperationRequest getBackupOperationRequest();

@Override
protected String protocol() {
return "ceph://";
}

public void inject() {
final List<Module> modules = new ArrayList<Module>() {{
add(new KubernetesApiModule());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ public void teardown() throws Exception {
destroy();
}

@Override
protected String getStorageLocation() {
return "ceph://" + BUCKET_NAME + "/cluster/datacenter1/node1";
}

@Override
public CephS3TransferManagerFactory getTransferManagerFactory() {
return transferManagerFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ public void teardown() throws ApiException {
}

@Override
protected String getStorageLocation() {
return "ceph://" + BUCKET_NAME + "/cluster/datacenter1/node1";
protected String protocol() {
return "ceph://";
}

@Override
Expand Down

0 comments on commit 87da57c

Please sign in to comment.