Skip to content

Commit

Permalink
Storage NIO: Stop leaving leftover buckets in IT (#3898)
Browse files Browse the repository at this point in the history 
* Stop leaving leftover buckets in ITGcsNIO

This integration test now deletes all the buckets it creates.

The issue was that requester-pays buckets couldn't be deleted by the
existing code. This PR updates RemoteStorageHelper so it can be given a
userProject for requester-pays buckets.

* Don't set userProject if empty, add test

RemoteStorageHelper now doesn't set the userProject option if it's
empty. Also, added testForceDeleteRetriesWithUserProject unit test.
  • Loading branch information
@jean-philippe-martin @chingor13
jean-philippe-martin authored and chingor13 committed Nov 7, 2018
1 parent 95d4641 commit b43b713
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 10 deletions.
10 changes: 7 additions & 3 deletions ...trib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,13 @@ public static void beforeClass() throws IOException {

@AfterClass
public static void afterClass() throws ExecutionException, InterruptedException {
if (storage != null && !RemoteStorageHelper.forceDelete(storage, BUCKET, 5, TimeUnit.SECONDS) &&
log.isLoggable(Level.WARNING)) {
log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", BUCKET);
if (storage != null) {
for (String bucket : new String[]{BUCKET, REQUESTER_PAYS_BUCKET}) {
if (!RemoteStorageHelper.forceDelete(storage, bucket, 5, TimeUnit.SECONDS, project) &&
log.isLoggable(Level.WARNING)) {
log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", bucket);
}
}
}
}

Expand Down
62 changes: 56 additions & 6 deletions ...gle-cloud-storage/src/main/java/com/google/cloud/storage/testing/RemoteStorageHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
Expand All @@ -41,6 +42,8 @@
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;
import java.util.logging.Logger;

import com.google.common.base.Strings;
import org.threeten.bp.Duration;

/**
Expand Down Expand Up @@ -121,8 +124,29 @@ public void run() {
*/
public static Boolean forceDelete(Storage storage, String bucket, long timeout, TimeUnit unit)
throws InterruptedException, ExecutionException {
return forceDelete(storage, bucket, timeout, unit, "");
}

/**
* Deletes a bucket, even if non-empty. Objects in the bucket are listed and deleted until bucket
* deletion succeeds or {@code timeout} expires. To allow for the timeout, this method uses a
* separate thread to send the delete requests. Use
* {@link #forceDelete(Storage storage, String bucket)} if spawning an additional thread is
* undesirable, such as in the App Engine production runtime.
*
* @param storage the storage service to be used to issue requests
* @param bucket the bucket to be deleted
* @param timeout the maximum time to wait
* @param unit the time unit of the timeout argument
* @param userProject the project to bill for requester-pays buckets (or "")
* @return true if deletion succeeded, false if timeout expired
* @throws InterruptedException if the thread deleting the bucket is interrupted while waiting
* @throws ExecutionException if an exception was thrown while deleting bucket or bucket objects
*/
public static Boolean forceDelete(Storage storage, String bucket, long timeout, TimeUnit unit, String userProject)
throws InterruptedException, ExecutionException {
ExecutorService executor = Executors.newSingleThreadExecutor();
Future<Boolean> future = executor.submit(new DeleteBucketTask(storage, bucket));
Future<Boolean> future = executor.submit(new DeleteBucketTask(storage, bucket, userProject));
try {
return future.get(timeout, unit);
} catch (TimeoutException ex) {
Expand Down Expand Up @@ -210,26 +234,52 @@ private static RetrySettings retrySettings() {

private static class DeleteBucketTask implements Callable<Boolean> {

private Storage storage;
private String bucket;
private final Storage storage;
private final String bucket;
private final String userProject;

public DeleteBucketTask(Storage storage, String bucket) {
this.storage = storage;
this.bucket = bucket;
this.userProject = "";
}

public DeleteBucketTask(Storage storage, String bucket, String userProject) {
this.storage = storage;
this.bucket = bucket;
this.userProject = userProject;
}

@Override
public Boolean call() {
while (true) {
ArrayList<BlobId> ids = new ArrayList<>();
for (BlobInfo info : storage.list(bucket, BlobListOption.versions(true)).getValues()) {
Page<Blob> listedBlobs;
if (Strings.isNullOrEmpty(userProject)) {
listedBlobs = storage.list(bucket, BlobListOption.versions(true));
} else {
listedBlobs = storage.list(bucket, BlobListOption.versions(true), BlobListOption.userProject(userProject));
}
for (BlobInfo info : listedBlobs.getValues()) {
ids.add(info.getBlobId());
}
if (!ids.isEmpty()) {
storage.delete(ids);
List<Boolean> results = storage.delete(ids);
if (!Strings.isNullOrEmpty(userProject)) {
for (int i=0; i<results.size(); i++) {
if (!results.get(i)) {
// deleting that blob failed. Let's try in a different way.
storage.delete(bucket, ids.get(i).getName(), Storage.BlobSourceOption.userProject(userProject));
}
}
}
}
try {
storage.delete(bucket);
if (Strings.isNullOrEmpty(userProject)) {
storage.delete(bucket);
} else {
storage.delete(bucket, Storage.BucketSourceOption.userProject(userProject));
}
return true;
} catch (StorageException e) {
if (e.getCode() == 409) {
Expand Down
26 changes: 25 additions & 1 deletion ...cloud-storage/src/test/java/com/google/cloud/storage/testing/RemoteStorageHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ public class RemoteStorageHelperTest {
private static final InputStream JSON_KEY_STREAM = new ByteArrayInputStream(JSON_KEY.getBytes());
private static final StorageException RETRYABLE_EXCEPTION = new StorageException(409, "");
private static final StorageException FATAL_EXCEPTION = new StorageException(500, "");
private static final String BLOB_NAME2 ="n2";
private static final BlobId BLOB_ID1 = BlobId.of(BUCKET_NAME, "n1");
private static final BlobId BLOB_ID2 = BlobId.of(BUCKET_NAME, "n2");
private static final BlobId BLOB_ID2 = BlobId.of(BUCKET_NAME, BLOB_NAME2);

private Blob blob1;
private Blob blob2;
Expand Down Expand Up @@ -221,6 +222,29 @@ public void testForceDeleteNoTimeoutFail() {
}
}

@Test
public void testForceDeleteRetriesWithUserProject() throws Exception {
final String USER_PROJECT="user-project";
Storage storageMock = EasyMock.createMock(Storage.class);
EasyMock.expect(blob1.getBlobId()).andReturn(BLOB_ID1);
EasyMock.expect(blob2.getBlobId()).andReturn(BLOB_ID2);
EasyMock.expect(blob2.getName()).andReturn(BLOB_NAME2);
ArrayList<BlobId> ids = new ArrayList<>();
ids.add(BLOB_ID1);
ids.add(BLOB_ID2);
EasyMock.expect(storageMock.delete(ids)).andReturn(ImmutableList.of(Boolean.TRUE, Boolean.FALSE)).anyTimes();
EasyMock.expect(storageMock.delete(BUCKET_NAME, BLOB_NAME2, Storage.BlobSourceOption.userProject(USER_PROJECT))).andReturn(true).anyTimes();
EasyMock.expect(storageMock.list(BUCKET_NAME, BlobListOption.versions(true), BlobListOption.userProject(USER_PROJECT)))
.andReturn(blobPage);
EasyMock.expect(storageMock.delete(BUCKET_NAME, Storage.BucketSourceOption.userProject(USER_PROJECT))).andReturn(true);
EasyMock.replay(storageMock, blob1, blob2);
try {
RemoteStorageHelper.forceDelete(storageMock, BUCKET_NAME, 5, TimeUnit.SECONDS, USER_PROJECT);
} finally {
EasyMock.verify(storageMock);
}
}

@Test
public void testCreateFromStream() {
RemoteStorageHelper helper = RemoteStorageHelper.create(PROJECT_ID, JSON_KEY_STREAM);
Expand Down

0 comments on commit b43b713

Please sign in to comment.