From 850435b2f9652e6b0434ffb8487a38a0b10e9b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 11 Aug 2023 18:01:52 +0200 Subject: [PATCH] build: remove native image tests from required checks (#2584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: backoff and retry for RESOURCE_EXHAUSTED when creating test instance Do a backoff and retry if CreateInstance fails during integration tests. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: correctly unwrap SpannerException * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * test: back off more and retry more * fix: prevent -1 as sleep value * build: reuse test instance * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * build: try to get native build to pick up instance * build: remove native image tests from required checks * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * test: ignore creation error * test: use us-central config * fix: fall back to first config for emulator --------- Co-authored-by: Owl Bot --- .github/sync-repo-settings.yaml | 2 - .kokoro/build.sh | 6 +- README.md | 8 +- google-cloud-spanner/pom.xml | 3 + .../cloud/spanner/IntegrationTestEnv.java | 97 ++++++++++++------- .../cloud/spanner/IntegrationTestEnvTest.java | 54 +++++++++++ 6 files changed, 129 insertions(+), 41 deletions(-) create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnvTest.java diff --git a/.github/sync-repo-settings.yaml b/.github/sync-repo-settings.yaml index a6177d5b0e..5b8fc9acc0 100644 --- a/.github/sync-repo-settings.yaml +++ b/.github/sync-repo-settings.yaml @@ -19,8 +19,6 @@ branchProtectionRules: - compile (8) - compile (11) - OwlBot Post Processor - - 'Kokoro - Test: Java GraalVM Native Image' - - 'Kokoro - Test: Java 17 GraalVM Native Image' - pattern: 3.3.x isAdminEnforced: true requiredApprovingReviewCount: 1 diff --git a/.kokoro/build.sh b/.kokoro/build.sh index 00d8e29f6d..2a25656e25 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -88,6 +88,8 @@ integration) -Dclirr.skip=true \ -Denforcer.skip=true \ -Dmaven.main.skip=true \ + -Dspanner.gce.config.project_id=gcloud-devel \ + -Dspanner.testenv.instance=projects/gcloud-devel/instances/java-client-integration-tests \ -fae \ verify RETURN_CODE=$? @@ -126,12 +128,12 @@ integration-cloud-staging) ;; graalvm) # Run Unit and Integration Tests with Native Image - mvn test -Pnative -Penable-integration-tests + mvn test -Pnative -Penable-integration-tests -Dspanner.gce.config.project_id=gcloud-devel -Dspanner.testenv.instance=projects/gcloud-devel/instances/java-client-integration-tests RETURN_CODE=$? ;; graalvm17) # Run Unit and Integration Tests with Native Image - mvn test -Pnative -Penable-integration-tests + mvn test -Pnative -Penable-integration-tests -Dspanner.gce.config.project_id=gcloud-devel -Dspanner.testenv.instance=projects/gcloud-devel/instances/java-client-integration-tests RETURN_CODE=$? ;; slowtests) diff --git a/README.md b/README.md index 3d15b34d82..8c508d3f93 100644 --- a/README.md +++ b/README.md @@ -50,20 +50,20 @@ If you are using Maven without the BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.21.0') +implementation platform('com.google.cloud:libraries-bom:26.22.0') implementation 'com.google.cloud:google-cloud-spanner' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-spanner:6.44.0' +implementation 'com.google.cloud:google-cloud-spanner:6.45.0' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.44.0" +libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.45.0" ``` @@ -430,7 +430,7 @@ Java is a registered trademark of Oracle and/or its affiliates. [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java11.html [stability-image]: https://img.shields.io/badge/stability-stable-green [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-spanner.svg -[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.44.0 +[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.45.0 [authentication]: https://github.com/googleapis/google-cloud-java#authentication [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles diff --git a/google-cloud-spanner/pom.xml b/google-cloud-spanner/pom.xml index 24e086cc4d..1f2a7798eb 100644 --- a/google-cloud-spanner/pom.xml +++ b/google-cloud-spanner/pom.xml @@ -122,6 +122,9 @@ -Dspanner.testenv.config.class=${spanner.testenv.config.class} + -Dspanner.testenv.instance=${spanner.testenv.instance} + -Dspanner.gce.config.project_id=${spanner.gce.config.project_id} + -Dspanner.testenv.kms_key.name=${spanner.testenv.kms_key.name} diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java index 460b1000ce..2c59439ce7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkState; +import com.google.api.client.util.ExponentialBackOff; import com.google.api.gax.longrunning.OperationFuture; import com.google.api.gax.paging.Page; import com.google.cloud.Timestamp; @@ -25,8 +26,8 @@ import com.google.cloud.spanner.testing.RemoteSpannerHelper; import com.google.common.collect.Iterators; import com.google.spanner.admin.instance.v1.CreateInstanceMetadata; -import io.grpc.Status; import java.util.Random; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -52,6 +53,9 @@ public class IntegrationTestEnv extends ExternalResource { */ public static final String TEST_INSTANCE_PROPERTY = "spanner.testenv.instance"; + public static final String MAX_CREATE_INSTANCE_ATTEMPTS = + "spanner.testenv.max_create_instance_attempts"; + private static final Logger logger = Logger.getLogger(IntegrationTestEnv.class.getName()); private TestEnvConfig config; @@ -126,9 +130,14 @@ protected void after() { this.config.tearDown(); } - private void initializeInstance(InstanceId instanceId) { - InstanceConfig instanceConfig = - Iterators.get(instanceAdminClient.listInstanceConfigs().iterateAll().iterator(), 0, null); + private void initializeInstance(InstanceId instanceId) throws Exception { + InstanceConfig instanceConfig; + try { + instanceConfig = instanceAdminClient.getInstanceConfig("regional-us-central1"); + } catch (Throwable ignore) { + instanceConfig = + Iterators.get(instanceAdminClient.listInstanceConfigs().iterateAll().iterator(), 0, null); + } checkState(instanceConfig != null, "No instance configs found"); InstanceConfigId configId = instanceConfig.getId(); @@ -142,40 +151,62 @@ private void initializeInstance(InstanceId instanceId) { OperationFuture op = instanceAdminClient.createInstance(instance); Instance createdInstance; + int maxAttempts = 25; try { - createdInstance = op.get(); - } catch (Exception e) { - boolean cancelled = false; + maxAttempts = + Integer.parseInt( + System.getProperty(MAX_CREATE_INSTANCE_ATTEMPTS, String.valueOf(maxAttempts))); + } catch (NumberFormatException ignore) { + // Ignore and fall back to the default. + } + ExponentialBackOff backOff = + new ExponentialBackOff.Builder() + .setInitialIntervalMillis(5_000) + .setMaxIntervalMillis(500_000) + .setMultiplier(2.0) + .build(); + int attempts = 0; + while (true) { try { - // Try to cancel the createInstance operation. - instanceAdminClient.cancelOperation(op.getName()); - com.google.longrunning.Operation createOperation = - instanceAdminClient.getOperation(op.getName()); - cancelled = - createOperation.hasError() - && createOperation.getError().getCode() == Status.CANCELLED.getCode().value(); - if (cancelled) { - logger.info("Cancelled the createInstance operation because the operation failed"); - } else { - logger.info( - "Tried to cancel the createInstance operation because the operation failed, but the operation could not be cancelled. Current status: " - + createOperation.getError().getCode()); - } - } catch (Throwable t) { - logger.log(Level.WARNING, "Failed to cancel the createInstance operation", t); - } - if (!cancelled) { - try { - instanceAdminClient.deleteInstance(instanceId.getInstance()); - logger.info( - "Deleted the test instance because the createInstance operation failed and cancelling the operation did not succeed"); - } catch (Throwable t) { - logger.log(Level.WARNING, "Failed to delete the test instance", t); + createdInstance = op.get(); + } catch (Exception e) { + SpannerException spannerException = + (e instanceof ExecutionException && e.getCause() != null) + ? SpannerExceptionFactory.asSpannerException(e.getCause()) + : SpannerExceptionFactory.asSpannerException(e); + if (attempts < maxAttempts && isRetryableResourceExhaustedException(spannerException)) { + attempts++; + if (spannerException.getRetryDelayInMillis() > 0L) { + //noinspection BusyWait + Thread.sleep(spannerException.getRetryDelayInMillis()); + } else { + // The Math.max(...) prevents Backoff#STOP (=-1) to be used as the sleep value. + //noinspection BusyWait + Thread.sleep(Math.max(backOff.getMaxIntervalMillis(), backOff.nextBackOffMillis())); + } + continue; } + throw SpannerExceptionFactory.newSpannerException( + spannerException.getErrorCode(), + String.format( + "Could not create test instance and giving up after %d attempts: %s", + attempts, e.getMessage()), + e); } - throw SpannerExceptionFactory.newSpannerException(e); + logger.log(Level.INFO, "Created test instance: {0}", createdInstance.getId()); + break; + } + } + + static boolean isRetryableResourceExhaustedException(SpannerException exception) { + if (exception.getErrorCode() != ErrorCode.RESOURCE_EXHAUSTED) { + return false; } - logger.log(Level.INFO, "Created test instance: {0}", createdInstance.getId()); + return exception + .getMessage() + .contains( + "Quota exceeded for quota metric 'Instance create requests' and limit 'Instance create requests per minute'") + || exception.getMessage().matches(".*cannot add \\d+ nodes in region.*"); } private void cleanUpOldDatabases(InstanceId instanceId) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnvTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnvTest.java new file mode 100644 index 0000000000..8aa6d55051 --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnvTest.java @@ -0,0 +1,54 @@ +/* + * Copyright 2023 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 + * + * http://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.cloud.spanner; + +import static com.google.cloud.spanner.IntegrationTestEnv.isRetryableResourceExhaustedException; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class IntegrationTestEnvTest { + + @Test + public void testIsRetryableResourceExhaustedException() { + assertFalse( + isRetryableResourceExhaustedException( + SpannerExceptionFactory.newSpannerException(ErrorCode.INVALID_ARGUMENT, "test"))); + assertFalse( + isRetryableResourceExhaustedException( + SpannerExceptionFactory.newSpannerException(ErrorCode.RESOURCE_EXHAUSTED, "test"))); + assertTrue( + isRetryableResourceExhaustedException( + SpannerExceptionFactory.newSpannerException( + ErrorCode.RESOURCE_EXHAUSTED, + "Operation with name \"projects/my-project/instances/my-instance/operations/32bb3dccf4243afc\" failed with status = GrpcStatusCode{transportCode=RESOURCE_EXHAUSTED} and message = Project 123 cannot add 1 nodes in region ."))); + assertTrue( + isRetryableResourceExhaustedException( + SpannerExceptionFactory.newSpannerException( + ErrorCode.RESOURCE_EXHAUSTED, + "Operation with name \"projects/my-project/instances/my-instance/operations/32bb3dccf4243afc\" failed with status = GrpcStatusCode{transportCode=RESOURCE_EXHAUSTED} and message = Project 123 cannot add 99 nodes in region ."))); + assertTrue( + isRetryableResourceExhaustedException( + SpannerExceptionFactory.newSpannerException( + ErrorCode.RESOURCE_EXHAUSTED, + "Could not create instance. Quota exceeded for quota metric 'Instance create requests' and limit 'Instance create requests per minute'"))); + } +}