From a8d765745d03880e989c56a1cfa2b74d187fce8d Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 29 Aug 2024 17:09:26 -0400 Subject: [PATCH] chore: update BucketCleaner to clean up folders and managed folders in addition to objects --- .kokoro/build.sh | 2 +- google-cloud-storage-control/pom.xml | 5 - .../storage/control/v2/ITFoldersTest.java | 70 --------- google-cloud-storage/pom.xml | 11 ++ .../cloud/storage/it/BucketCleaner.java | 135 +++++++++++++++++- .../cloud/storage/it/TemporaryBucket.java | 15 +- samples/install-without-bom/pom.xml | 8 ++ samples/pom.xml | 4 +- samples/snapshot/pom.xml | 8 ++ samples/snippets/pom.xml | 7 + .../storage/ITVerboseBucketCleanupTest.java | 77 ++++++++++ 11 files changed, 257 insertions(+), 85 deletions(-) delete mode 100644 google-cloud-storage-control/src/test/java/com/google/storage/control/v2/ITFoldersTest.java create mode 100644 samples/snippets/src/test/java/com/example/storage/ITVerboseBucketCleanupTest.java diff --git a/.kokoro/build.sh b/.kokoro/build.sh index 49069482ff..85bd795d0f 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -28,7 +28,7 @@ mvn -version echo ${JOB_TYPE} # attempt to install 3 times with exponential backoff (starting with 10 seconds) -retry_with_backoff 3 10 \ +#retry_with_backoff 3 10 \ mvn install -B -V -ntp \ -DskipTests=true \ -Dclirr.skip=true \ diff --git a/google-cloud-storage-control/pom.xml b/google-cloud-storage-control/pom.xml index 29f8c8a635..9c8f4bdf3a 100644 --- a/google-cloud-storage-control/pom.xml +++ b/google-cloud-storage-control/pom.xml @@ -81,11 +81,6 @@ google-api-services-storage test - - com.google.cloud - google-cloud-storage - test - com.google.auth google-auth-library-oauth2-http diff --git a/google-cloud-storage-control/src/test/java/com/google/storage/control/v2/ITFoldersTest.java b/google-cloud-storage-control/src/test/java/com/google/storage/control/v2/ITFoldersTest.java deleted file mode 100644 index c8609e1c9a..0000000000 --- a/google-cloud-storage-control/src/test/java/com/google/storage/control/v2/ITFoldersTest.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright 2024 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.storage.control.v2; - -import com.google.api.client.http.HttpRequestInitializer; -import com.google.api.client.http.HttpTransport; -import com.google.api.client.json.jackson2.JacksonFactory; -import com.google.api.services.storage.Storage; -import com.google.api.services.storage.model.Bucket; -import com.google.cloud.http.HttpTransportOptions; -import com.google.cloud.storage.StorageOptions; -import java.util.UUID; -import org.junit.Assert; -import org.junit.Test; - -public class ITFoldersTest { - @Test - public void createFolderTest() throws Exception { - // TODO: Update this with manual client implmentation of HNS enabled when published. - // TODO: Refactor to use Storage test infrastructure if extended. - StorageOptions option = StorageOptions.http().build(); - HttpTransportOptions httpTransportOptions = (HttpTransportOptions) option.getTransportOptions(); - HttpTransport transport = httpTransportOptions.getHttpTransportFactory().create(); - HttpRequestInitializer initializer = httpTransportOptions.getHttpRequestInitializer(option); - Storage storage = - new Storage.Builder(transport, new JacksonFactory(), initializer) - .setApplicationName("test") - .build(); - String bucketName = "hns-b-gcs-grpc-team-test-" + UUID.randomUUID(); - Bucket createdBucket = null; - try { - Bucket bucketConfig = - new Bucket() - .setName(bucketName) - .setHierarchicalNamespace(new Bucket.HierarchicalNamespace().setEnabled(true)) - .setIamConfiguration( - new Bucket.IamConfiguration() - .setUniformBucketLevelAccess( - new Bucket.IamConfiguration.UniformBucketLevelAccess().setEnabled(true))); - createdBucket = storage.buckets().insert(option.getProjectId(), bucketConfig).execute(); - StorageControlClient storageControlClient = StorageControlClient.create(); - String folderId = "foldername/"; - Folder folder = - storageControlClient.createFolder( - CreateFolderRequest.newBuilder() - .setParent(BucketName.format("_", bucketName)) - .setFolderId(folderId) - .build()); - Assert.assertEquals(folder.getName(), FolderName.format("_", bucketName, folderId)); - } finally { - if (createdBucket != null) { - storage.buckets().delete(bucketName); - } - } - } -} diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index b4e70fb36b..3d43ebbaf0 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -240,6 +240,17 @@ test ${pubsub-proto.version} + + com.google.cloud + google-cloud-storage-control + test + + + com.google.api.grpc + proto-google-cloud-storage-control-v2 + test + + com.google.cloud google-cloud-core diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/BucketCleaner.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/BucketCleaner.java index c4dba11f85..3f70e9d443 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/BucketCleaner.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/BucketCleaner.java @@ -17,15 +17,27 @@ package com.google.cloud.storage.it; import com.google.api.gax.paging.Page; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.FailedPreconditionException; import com.google.cloud.storage.Blob; import com.google.cloud.storage.Storage; +import com.google.cloud.storage.Storage.BlobField; import com.google.cloud.storage.Storage.BlobListOption; import com.google.cloud.storage.Storage.BlobSourceOption; import com.google.cloud.storage.Storage.BucketSourceOption; +import com.google.common.collect.ImmutableList; +import com.google.storage.control.v2.BucketName; +import com.google.storage.control.v2.Folder; +import com.google.storage.control.v2.StorageControlClient; +import com.google.storage.control.v2.StorageLayout; +import com.google.storage.control.v2.StorageLayoutName; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.stream.StreamSupport; public final class BucketCleaner { @@ -48,12 +60,9 @@ public static void doCleanup(String bucketName, Storage s) { b.getName(), s.delete(b.getBlobId(), BlobSourceOption.userProject(projectId)))) .collect(Collectors.toList()); - List failedDeletes = - deleteResults.stream().filter(r -> !r.success).collect(Collectors.toList()); - failedDeletes.forEach( - r -> LOGGER.warning(String.format("Failed to delete object %s/%s", bucketName, r.name))); + boolean anyFailedObjectDeletes = getIfAnyFailedAndReport(bucketName, deleteResults, "object"); - if (failedDeletes.isEmpty()) { + if (!anyFailedObjectDeletes) { s.delete(bucketName, BucketSourceOption.userProject(projectId)); } else { LOGGER.warning("Unable to delete bucket due to previous failed object deletes"); @@ -64,6 +73,122 @@ public static void doCleanup(String bucketName, Storage s) { } } + public static void doCleanup(String bucketName, Storage s, StorageControlClient ctrl) { + LOGGER.warning("Starting bucket cleanup: " + bucketName); + String projectId = s.getOptions().getProjectId(); + try { + // TODO: probe bucket existence, a bad test could have deleted the bucket + Page page1 = + s.list( + bucketName, + BlobListOption.userProject(projectId), + BlobListOption.versions(true), + BlobListOption.fields(BlobField.NAME)); + + List objectResults = + StreamSupport.stream(page1.iterateAll().spliterator(), false) + .map( + b -> + new DeleteResult( + b.getName(), + s.delete(b.getBlobId(), BlobSourceOption.userProject(projectId)))) + .collect(Collectors.toList()); + boolean anyFailedObjectDelete = getIfAnyFailedAndReport(bucketName, objectResults, "object"); + boolean anyFailedFolderDelete = false; + boolean anyFailedManagedFolderDelete = false; + + if (!anyFailedObjectDelete) { + BucketName parent = BucketName.of("_", bucketName); + StorageLayout storageLayout = + ctrl.getStorageLayout(StorageLayoutName.of(parent.getProject(), parent.getBucket())); + List folderDeletes; + if (storageLayout.hasHierarchicalNamespace() + && storageLayout.getHierarchicalNamespace().getEnabled()) { + folderDeletes = + StreamSupport.stream(ctrl.listFolders(parent).iterateAll().spliterator(), false) + .collect(Collectors.toList()) + .stream() + .sorted(Collections.reverseOrder(Comparator.comparing(Folder::getName))) + .map( + folder -> { + LOGGER.warning(String.format("folder = %s", folder.getName())); + boolean success = true; + try { + ctrl.deleteFolder(folder.getName()); + } catch (ApiException e) { + success = false; + } + return new DeleteResult(folder.getName(), success); + }) + .collect(Collectors.toList()); + } else { + folderDeletes = ImmutableList.of(); + } + + List managedFolderDeletes; + try { + managedFolderDeletes = + StreamSupport.stream( + ctrl.listManagedFolders(parent).iterateAll().spliterator(), false) + .map( + managedFolder -> { + LOGGER.warning( + String.format("managedFolder = %s", managedFolder.getName())); + boolean success = true; + try { + ctrl.deleteFolder(managedFolder.getName()); + } catch (ApiException e) { + success = false; + } + return new DeleteResult(managedFolder.getName(), success); + }) + .collect(Collectors.toList()); + } catch (FailedPreconditionException fpe) { + // FAILED_PRECONDITION: Uniform bucket-level access is required to be enabled on the + // bucket in order to perform this operation. Read more at + // https://cloud.google.com/storage/docs/uniform-bucket-level-access + managedFolderDeletes = ImmutableList.of(); + } + + anyFailedFolderDelete = getIfAnyFailedAndReport(bucketName, folderDeletes, "folder"); + anyFailedManagedFolderDelete = + getIfAnyFailedAndReport(bucketName, managedFolderDeletes, "managed folder"); + } + + List failed = + Stream.of( + anyFailedObjectDelete ? "object" : "", + anyFailedFolderDelete ? "folder" : "", + anyFailedManagedFolderDelete ? "managed folder" : "") + .filter(ss -> !ss.isEmpty()) + .collect(Collectors.toList()); + + if (!anyFailedObjectDelete && !anyFailedFolderDelete && !anyFailedManagedFolderDelete) { + s.delete(bucketName, BucketSourceOption.userProject(projectId)); + } else { + LOGGER.warning( + String.format( + "Unable to delete bucket %s due to previous failed %s deletes", + bucketName, failed)); + } + + LOGGER.warning("Bucket cleanup complete: " + bucketName); + } catch (Exception e) { + LOGGER.log(Level.SEVERE, e, () -> "Error during bucket cleanup."); + } + } + + private static boolean getIfAnyFailedAndReport( + String bucketName, List deleteResults, String resourceType) { + List failedDeletes = + deleteResults.stream().filter(r -> !r.success).collect(Collectors.toList()); + failedDeletes.forEach( + r -> + LOGGER.warning( + String.format("Failed to delete %s %s/%s", resourceType, bucketName, r.name))); + return !failedDeletes.isEmpty(); + } + private static final class DeleteResult { private final String name; private final boolean success; diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/TemporaryBucket.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/TemporaryBucket.java index 54ccfd92d9..b44ce44689 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/TemporaryBucket.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/TemporaryBucket.java @@ -23,22 +23,26 @@ import com.google.cloud.storage.Storage; import com.google.cloud.storage.conformance.retry.CleanupStrategy; import com.google.common.base.Preconditions; +import com.google.storage.control.v2.StorageControlClient; import java.time.Duration; public final class TemporaryBucket implements AutoCloseable { private final BucketInfo bucket; private final Storage storage; + private final StorageControlClient ctrl; private final Duration cleanupTimeout; private final CleanupStrategy cleanupStrategy; private TemporaryBucket( BucketInfo bucket, Storage storage, + StorageControlClient ctrl, Duration cleanupTimeout, CleanupStrategy cleanupStrategy) { this.bucket = bucket; this.storage = storage; + this.ctrl = ctrl; this.cleanupTimeout = cleanupTimeout; this.cleanupStrategy = cleanupStrategy; } @@ -51,7 +55,7 @@ public BucketInfo getBucket() { @Override public void close() throws Exception { if (cleanupStrategy == CleanupStrategy.ALWAYS) { - BucketCleaner.doCleanup(bucket.getName(), storage); + BucketCleaner.doCleanup(bucket.getName(), storage, ctrl); } } @@ -65,6 +69,7 @@ public static final class Builder { private Duration cleanupTimeoutDuration; private BucketInfo bucketInfo; private Storage storage; + private StorageControlClient ctrl; private Builder() { this.cleanupStrategy = CleanupStrategy.ALWAYS; @@ -91,14 +96,20 @@ public Builder setStorage(Storage storage) { return this; } + public Builder setStorageControl(StorageControlClient ctrl) { + this.ctrl = ctrl; + return this; + } + public TemporaryBucket build() { Preconditions.checkArgument( cleanupStrategy != CleanupStrategy.ONLY_ON_SUCCESS, "Unable to detect success."); Storage s = requireNonNull(storage, "storage must be non null"); + StorageControlClient c = requireNonNull(ctrl, "ctrl must be non null"); Bucket b = s.create(requireNonNull(bucketInfo, "bucketInfo must be non null")); // intentionally drop from Bucket to BucketInfo to ensure not leaking the Storage instance - return new TemporaryBucket(b.asBucketInfo(), s, cleanupTimeoutDuration, cleanupStrategy); + return new TemporaryBucket(b.asBucketInfo(), s, c, cleanupTimeoutDuration, cleanupStrategy); } } } diff --git a/samples/install-without-bom/pom.xml b/samples/install-without-bom/pom.xml index 9ee07c3e70..9f3574bc86 100644 --- a/samples/install-without-bom/pom.xml +++ b/samples/install-without-bom/pom.xml @@ -22,6 +22,7 @@ 1.8 1.8 UTF-8 + com.example.storage.ITVerboseBucketCleanupTest @@ -32,6 +33,13 @@ google-cloud-storage 2.42.0 + + com.google.cloud + google-cloud-storage + 2.42.0 + tests + test + com.google.cloud google-cloud-storage-control diff --git a/samples/pom.xml b/samples/pom.xml index 9f17328f88..d0a71d9f8c 100644 --- a/samples/pom.xml +++ b/samples/pom.xml @@ -29,9 +29,9 @@ - install-without-bom + snapshot - snippets + diff --git a/samples/snapshot/pom.xml b/samples/snapshot/pom.xml index 84cfc58129..37f33a152b 100644 --- a/samples/snapshot/pom.xml +++ b/samples/snapshot/pom.xml @@ -22,6 +22,7 @@ 1.8 1.8 UTF-8 + com.example.storage.ITVerboseBucketCleanupTest @@ -30,6 +31,13 @@ google-cloud-storage 2.42.1-SNAPSHOT + + com.google.cloud + google-cloud-storage + 2.42.1-SNAPSHOT + tests + test + com.google.cloud google-cloud-storage-control diff --git a/samples/snippets/pom.xml b/samples/snippets/pom.xml index 470b6c6990..22d98708b2 100644 --- a/samples/snippets/pom.xml +++ b/samples/snippets/pom.xml @@ -43,6 +43,13 @@ com.google.cloud google-cloud-storage + + com.google.cloud + google-cloud-storage + 2.42.0 + tests + test + com.google.cloud google-cloud-storage-control diff --git a/samples/snippets/src/test/java/com/example/storage/ITVerboseBucketCleanupTest.java b/samples/snippets/src/test/java/com/example/storage/ITVerboseBucketCleanupTest.java new file mode 100644 index 0000000000..8f8f6719d5 --- /dev/null +++ b/samples/snippets/src/test/java/com/example/storage/ITVerboseBucketCleanupTest.java @@ -0,0 +1,77 @@ +package com.example.storage; + +import com.google.cloud.storage.Storage; +import com.google.cloud.storage.Storage.BucketField; +import com.google.cloud.storage.Storage.BucketListOption; +import com.google.cloud.storage.StorageOptions; +import com.google.cloud.storage.it.BucketCleaner; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.google.storage.control.v2.StorageControlClient; +import java.time.Instant; +import java.time.OffsetDateTime; +import java.time.ZoneOffset; +import java.util.List; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.junit.Test; + +public final class ITVerboseBucketCleanupTest { + private static final Logger LOGGER = Logger.getLogger(ITVerboseBucketCleanupTest.class.getName()); + + @Test + public void verboseBucketCleanup() throws Exception { + + ThreadFactory threadFactory = new ThreadFactoryBuilder() + .setDaemon(true) + .setNameFormat("cp-pool-%03d") + .build(); + + ListeningExecutorService exec = MoreExecutors.listeningDecorator( + Executors.newFixedThreadPool(4 * Runtime.getRuntime().availableProcessors(), threadFactory) + ); + + OffsetDateTime now = Instant.now().atOffset(ZoneOffset.UTC); + OffsetDateTime _3DaysAgo = now.minusDays(3); + LOGGER.info(String.format("_3DaysAgo = %s", _3DaysAgo)); + + try (Storage s = StorageOptions.http().build().getService(); + StorageControlClient ctrl = StorageControlClient.create()) { + List> deleteFutures = s.list( + BucketListOption.fields(BucketField.NAME, BucketField.TIME_CREATED)) + .streamAll() + .map(bucket -> { + String name = bucket.getName(); + OffsetDateTime ctime = bucket.getCreateTimeOffsetDateTime(); + String action = null; + try { + if (ctime.isBefore(_3DaysAgo)) { + action = "Cleaning up"; + return exec.submit(() -> { + BucketCleaner.doCleanup(name, s, ctrl); + return null; + }); + } else { + action = "Skipping"; + return Futures.immediateVoidFuture(); + } + } finally { + LOGGER.warning(String.format("%11s bucket {name: '%s', ctime: '%s'}", action, name, ctime)); + } + }) + .collect(Collectors.toList()); + + ListenableFuture> allFuture = Futures.allAsList(deleteFutures); + + allFuture.get(15, TimeUnit.MINUTES); + } + + } +}