Skip to content

Commit

Permalink
test: fix bucket cleanup logic (#1813)
Browse files Browse the repository at this point in the history 
Some tests have not been properly cleaning up buckets and leaking resources outside the suite.

Cleanup of existing leaked buckets will be handled separately.
  • Loading branch information
@BenWhitehead
BenWhitehead committed Dec 15, 2022
1 parent 92c9644 commit 0af6693
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 74 deletions.
6 changes: 4 additions & 2 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,8 @@ public void testBucketWithBucketPolicyOnlyEnabled() throws Exception {
// 4. Expect failure when attempting to list default ACLs for BucketPolicyOnly bucket

// TODO: temp bucket
String randBucketName = generator.randomBucketName();
try {
String randBucketName = generator.randomBucketName();
storage.create(
Bucket.newBuilder(randBucketName)
.setIamConfiguration(
Expand Down Expand Up @@ -640,6 +640,7 @@ public void testBucketWithBucketPolicyOnlyEnabled() throws Exception {
// Expected: Listing legacy ACLs should fail on a BPO enabled bucket
}
} finally {
BucketCleaner.doCleanup(randBucketName, storage);
}
}

Expand All @@ -653,8 +654,8 @@ public void testBucketWithUniformBucketLevelAccessEnabled() throws Exception {
// 4. Expect failure when attempting to list default ACLs for UniformBucketLevelAccess bucket

// TODO: temp bucket
String randBucketName = generator.randomBucketName();
try {
String randBucketName = generator.randomBucketName();
storage.create(
Bucket.newBuilder(randBucketName)
.setIamConfiguration(
Expand All @@ -681,6 +682,7 @@ public void testBucketWithUniformBucketLevelAccessEnabled() throws Exception {
// Expected: Listing legacy ACLs should fail on a BPO enabled bucket
}
} finally {
BucketCleaner.doCleanup(randBucketName, storage);
}
}

Expand Down
71 changes: 29 additions & 42 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketLifecycleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,23 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import com.google.cloud.storage.Bucket;
import com.google.cloud.storage.BucketInfo;
import com.google.cloud.storage.BucketInfo.LifecycleRule;
import com.google.cloud.storage.BucketInfo.LifecycleRule.AbortIncompleteMPUAction;
import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleAction;
import com.google.cloud.storage.BucketInfo.LifecycleRule.LifecycleCondition;
import com.google.cloud.storage.BucketInfo.LifecycleRule.SetStorageClassLifecycleAction;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.Storage.BucketField;
import com.google.cloud.storage.StorageClass;
import com.google.cloud.storage.TransportCompatibility.Transport;
import com.google.cloud.storage.it.runner.StorageITRunner;
import com.google.cloud.storage.it.runner.annotations.Backend;
import com.google.cloud.storage.it.runner.annotations.CrossRun;
import com.google.cloud.storage.it.runner.annotations.Inject;
import com.google.cloud.storage.it.runner.registry.Generator;
import com.google.cloud.storage.testing.RemoteStorageHelper;
import com.google.common.collect.ImmutableList;
import java.time.OffsetDateTime;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.junit.runner.RunWith;

Expand Down Expand Up @@ -70,9 +65,9 @@ public class ITBucketLifecycleTest {
@Inject public Generator generator;

@Test
public void testGetBucketLifecycleRules() {
public void testGetBucketLifecycleRules() throws Exception {
String lifecycleTestBucketName = generator.randomBucketName();
storage.create(
BucketInfo bucketInfo =
BucketInfo.newBuilder(lifecycleTestBucketName)
.setLocation("us")
.setLifecycleRules(
Expand All @@ -90,16 +85,13 @@ public void testGetBucketLifecycleRules() {
.setCustomTimeBeforeOffsetDateTime(OffsetDateTime.now())
.setDaysSinceCustomTime(30)
.build())))
.build());
Bucket remoteBucket =
storage.get(lifecycleTestBucketName, Storage.BucketGetOption.fields(BucketField.LIFECYCLE));
LifecycleRule lifecycleRule = remoteBucket.getLifecycleRules().get(0);
try {
assertTrue(
lifecycleRule
.getAction()
.getActionType()
.equals(LifecycleRule.SetStorageClassLifecycleAction.TYPE));
.build();
try (TemporaryBucket tempB =
TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
BucketInfo remoteBucket = tempB.getBucket();
LifecycleRule lifecycleRule = remoteBucket.getLifecycleRules().get(0);
assertThat(lifecycleRule.getAction().getActionType())
.isEqualTo(SetStorageClassLifecycleAction.TYPE);
assertEquals(3, lifecycleRule.getCondition().getNumberOfNewerVersions().intValue());
assertNotNull(lifecycleRule.getCondition().getCreatedBeforeOffsetDateTime());
assertFalse(lifecycleRule.getCondition().getIsLive());
Expand All @@ -109,51 +101,46 @@ public void testGetBucketLifecycleRules() {
assertNotNull(lifecycleRule.getCondition().getNoncurrentTimeBeforeOffsetDateTime());
assertEquals(30, lifecycleRule.getCondition().getDaysSinceCustomTime().intValue());
assertNotNull(lifecycleRule.getCondition().getCustomTimeBeforeOffsetDateTime());
} finally {
storage.delete(lifecycleTestBucketName);
}
}

@Test
public void testGetBucketAbortMPULifecycle() {
public void testGetBucketAbortMPULifecycle() throws Exception {
String lifecycleTestBucketName = generator.randomBucketName();
storage.create(
BucketInfo bucketInfo =
BucketInfo.newBuilder(lifecycleTestBucketName)
.setLocation("us")
.setLifecycleRules(
ImmutableList.of(
new LifecycleRule(
LifecycleAction.newAbortIncompleteMPUploadAction(),
LifecycleCondition.newBuilder().setAge(1).build())))
.build());
Bucket remoteBucket =
storage.get(lifecycleTestBucketName, Storage.BucketGetOption.fields(BucketField.LIFECYCLE));
LifecycleRule lifecycleRule = remoteBucket.getLifecycleRules().get(0);
try {
.build();
try (TemporaryBucket tempB =
TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
BucketInfo remoteBucket = tempB.getBucket();
LifecycleRule lifecycleRule = remoteBucket.getLifecycleRules().get(0);
assertEquals(AbortIncompleteMPUAction.TYPE, lifecycleRule.getAction().getActionType());
assertEquals(1, lifecycleRule.getCondition().getAge().intValue());
} finally {
storage.delete(lifecycleTestBucketName);
}
}

@Test
public void testDeleteLifecycleRules() throws ExecutionException, InterruptedException {
public void testDeleteLifecycleRules() throws Exception {
String bucketName = generator.randomBucketName();
Bucket bucket =
storage.create(
BucketInfo.newBuilder(bucketName)
.setLocation("us")
.setLifecycleRules(LIFECYCLE_RULES)
.build());
assertThat(bucket.getLifecycleRules()).isNotNull();
assertThat(bucket.getLifecycleRules()).hasSize(2);
try {
Bucket updatedBucket = bucket.toBuilder().deleteLifecycleRules().build();
BucketInfo bucketInfo =
BucketInfo.newBuilder(bucketName)
.setLocation("us")
.setLifecycleRules(LIFECYCLE_RULES)
.build();
try (TemporaryBucket tempB =
TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
BucketInfo bucket = tempB.getBucket();
assertThat(bucket.getLifecycleRules()).isNotNull();
assertThat(bucket.getLifecycleRules()).hasSize(2);
BucketInfo updatedBucket = bucket.toBuilder().deleteLifecycleRules().build();
storage.update(updatedBucket);
assertThat(updatedBucket.getLifecycleRules()).hasSize(0);
} finally {
RemoteStorageHelper.forceDelete(storage, bucketName, 5, TimeUnit.SECONDS);
}
}
}
65 changes: 36 additions & 29 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.Storage.BlobField;
import com.google.cloud.storage.Storage.BucketField;
import com.google.cloud.storage.Storage.BucketTargetOption;
import com.google.cloud.storage.TransportCompatibility.Transport;
import com.google.cloud.storage.it.runner.StorageITRunner;
import com.google.cloud.storage.it.runner.annotations.Backend;
Expand Down Expand Up @@ -130,38 +131,45 @@ public void testGetBucketAllSelectedFields() {
@Test
// Cannot turn on for GRPC until b/246634709 is resolved, verified locally.
@CrossRun.Exclude(transports = Transport.GRPC)
public void testBucketLocationType() {
public void testBucketLocationType() throws Exception {
String bucketName = generator.randomBucketName();
Bucket bucket = storage.create(BucketInfo.newBuilder(bucketName).setLocation("us").build());
BucketInfo bucketInfo = BucketInfo.newBuilder(bucketName).setLocation("us").build();
try (TemporaryBucket tempB =
TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
BucketInfo bucket = tempB.getBucket();

assertEquals("multi-region", bucket.getLocationType());
assertEquals("multi-region", bucket.getLocationType());
}
}

@Test
// Cannot turn on for GRPC until creation bug b/246634709 is resolved, verified locally.
@CrossRun.Exclude(transports = Transport.GRPC)
public void testBucketCustomPlacmentConfigDualRegion() {
public void testBucketCustomPlacmentConfigDualRegion() throws Exception {
String bucketName = generator.randomBucketName();
List<String> locations = new ArrayList<>();
locations.add("US-EAST1");
locations.add("US-WEST1");
CustomPlacementConfig customPlacementConfig =
CustomPlacementConfig.newBuilder().setDataLocations(locations).build();
Bucket bucket =
storage.create(
BucketInfo.newBuilder(bucketName)
.setCustomPlacementConfig(customPlacementConfig)
.setLocation("us")
.build());
assertTrue(bucket.getCustomPlacementConfig().getDataLocations().contains("US-EAST1"));
assertTrue(bucket.getCustomPlacementConfig().getDataLocations().contains("US-WEST1"));
assertTrue(bucket.getLocation().equalsIgnoreCase("us"));
BucketInfo bucketInfo =
BucketInfo.newBuilder(bucketName)
.setCustomPlacementConfig(customPlacementConfig)
.setLocation("us")
.build();
try (TemporaryBucket tempB =
TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
BucketInfo bucket = tempB.getBucket();
assertTrue(bucket.getCustomPlacementConfig().getDataLocations().contains("US-EAST1"));
assertTrue(bucket.getCustomPlacementConfig().getDataLocations().contains("US-WEST1"));
assertTrue(bucket.getLocation().equalsIgnoreCase("us"));
}
}

@Test
// Cannot turn on until GRPC Update logic bug is fixed b/247133805
@CrossRun.Exclude(transports = Transport.GRPC)
public void testBucketLogging() {
public void testBucketLogging() throws Exception {
String logsBucketName = generator.randomBucketName();
String loggingBucketName = generator.randomBucketName();

Expand All @@ -176,29 +184,28 @@ public void testBucketLogging() {
.build())
.build();

Bucket logsBucket = null;
Bucket loggingBucket = null;
try {
logsBucket = storage.create(logsBucketInfo);
try (TemporaryBucket tempLogsB =
TemporaryBucket.newBuilder().setBucketInfo(logsBucketInfo).setStorage(storage).build();
TemporaryBucket tempLoggingB =
TemporaryBucket.newBuilder()
.setBucketInfo(loggingBucketInfo)
.setStorage(storage)
.build(); ) {
BucketInfo logsBucket = tempLogsB.getBucket();
BucketInfo loggingBucket = tempLoggingB.getBucket();
assertNotNull(logsBucket);

Policy policy = storage.getIamPolicy(logsBucketName);
assertNotNull(policy);
loggingBucket = storage.create(loggingBucketInfo);
assertEquals(logsBucketName, loggingBucket.getLogging().getLogBucket());
assertEquals("test-logs", loggingBucket.getLogging().getLogObjectPrefix());

// Disable bucket logging.
Bucket updatedBucket = loggingBucket.toBuilder().setLogging(null).build().update();
Bucket updatedBucket =
storage.update(
loggingBucket.toBuilder().setLogging(null).build(),
BucketTargetOption.metagenerationMatch());
assertNull(updatedBucket.getLogging());

} finally {
if (logsBucket != null) {
BucketCleaner.doCleanup(logsBucketName, storage);
}
if (loggingBucket != null) {
BucketCleaner.doCleanup(loggingBucketName, storage);
}
}
}

Expand Down Expand Up @@ -267,7 +274,7 @@ public void testRpoConfig() {

assertEquals("DEFAULT", storage.get(rpoBucket).getRpo().toString());
} finally {
storage.delete(rpoBucket);
BucketCleaner.doCleanup(rpoBucket, storage);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...ud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/StorageInstance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ final class StorageInstance implements ManagedLifecycle {
}

Storage getStorage() {
return proxy;
return storage;
}

@Override
Expand Down

0 comments on commit 0af6693

Please sign in to comment.