Skip to content

Commit

Permalink
chore: Fix flaky IT test cases (#8260)
Browse files Browse the repository at this point in the history
* chore: Busy wait java-container CREATE operation to finish

* chore: Delete any existing java-notebook instances

* chore: Use ListInstanceRequest with parent

* chore: Delete gapic compute instances

* chore: Use ZonedDateTime for RFC3339 with Zone

* chore: Setup the correct Request object field

* chore: Create new IT profile

* chore: Move shared deps profile to root pom.xml

* chore: Use test lifecycle phase for graalvm tests

* chore: Use old pom configs

* chore: Use correct path to vision resources

* chore: Remove unused pom.xml

* chore: Move busy wait after CREATE operation

* chore: Add specific names for IT compute instances

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: Clean up compute instances names

* chore: Clean up instances

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: Add checks for deletion

* chore: Add missing import

* chore: Add missing import for notebooks

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
lqiu96 and gcf-owl-bot[bot] authored Sep 12, 2022
1 parent db7c7d1 commit 8e4378d
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 57 deletions.
4 changes: 2 additions & 2 deletions .kokoro/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ case ${JOB_TYPE} in
-Penable-integration-tests \
-Pnative \
-fae \
verify
test
RETURN_CODE=$?
printf "Finished Unit and Integration Tests for GraalVM Native:\n%s\n" "${module_list}"
else
Expand All @@ -182,7 +182,7 @@ case ${JOB_TYPE} in
-Penable-integration-tests \
-Pnative \
-fae \
verify
test
RETURN_CODE=$?
printf "Finished Unit and Integration Tests for GraalVM Native 17:\n%s\n" "${module_list}"
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ public class BaseTest {
protected static final String DEFAULT_PROJECT = ServiceOptions.getDefaultProjectId();
protected static final String DEFAULT_ZONE = "us-central1-a";
protected static final String DEFAULT_REGION = "us-west1";
protected static final String COMPUTE_PREFIX = "it-test-compute";

public static String generateRandomName(String placeholder) {
return "gapic-" + placeholder + UUID.randomUUID().toString().substring(0, 8);
return COMPUTE_PREFIX + "-" + placeholder + "-" + UUID.randomUUID().toString().substring(0, 8);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ public static void setUp() throws IOException {
instances = new ArrayList<>();
InstancesSettings instanceSettings = InstancesSettings.newBuilder().build();
instancesClient = InstancesClient.create(instanceSettings);

Util.cleanUpComputeInstances(instancesClient, DEFAULT_PROJECT, DEFAULT_ZONE);
}

@Before
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.google.cloud.compute.v1.integration;

import com.google.cloud.compute.v1.DeleteInstanceRequest;
import com.google.cloud.compute.v1.Instance;
import com.google.cloud.compute.v1.InstancesClient;
import com.google.cloud.compute.v1.InstancesClient.ListPagedResponse;
import java.time.Instant;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;

public class Util {

// Cleans existing test resources if any.
private static final int DELETION_THRESHOLD_TIME_HOURS = 24;

/** Bring down any instances that are older than 24 hours */
public static void cleanUpComputeInstances(
InstancesClient instancesClient, String project, String zone) {
ListPagedResponse listPagedResponse = instancesClient.list(project, zone);
for (Instance instance : listPagedResponse.iterateAll()) {
if (isCreatedBeforeThresholdTime(
ZonedDateTime.parse(instance.getCreationTimestamp()).toInstant())
&& instance.getName().startsWith(BaseTest.COMPUTE_PREFIX)) {
instancesClient.deleteAsync(
DeleteInstanceRequest.newBuilder()
.setInstance(instance.getName())
.setProject(project)
.setZone(zone)
.build());
}
}
}

private static boolean isCreatedBeforeThresholdTime(Instant instant) {
return instant.isBefore(Instant.now().minus(DELETION_THRESHOLD_TIME_HOURS, ChronoUnit.HOURS));
}
}
4 changes: 2 additions & 2 deletions java-container/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ If you are using Maven with [BOM][libraries-bom], add this to your pom.xml file:
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>libraries-bom</artifactId>
<version>26.1.0</version>
<version>26.1.1</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down Expand Up @@ -49,7 +49,7 @@ If you are using Maven without 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.1.0')
implementation platform('com.google.cloud:libraries-bom:26.1.1')
implementation 'com.google.cloud:google-cloud-container'
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.container.v1.ListOperationsResponse;
import com.google.container.v1.NodePool;
import com.google.container.v1.Operation;
import com.google.container.v1.Operation.Status;
import com.google.container.v1.ServerConfig;
import java.util.List;
import java.util.UUID;
Expand All @@ -37,16 +38,18 @@

public class ITSystemTest {

protected static final String CONTAINER_PREFIX = "it-test-container";

private static ClusterManagerClient client;
private static Operation operation;

private static final Logger LOG = Logger.getLogger(ITSystemTest.class.getName());
private static final String PROJECT_ID = ServiceOptions.getDefaultProjectId();
private static final String ZONE = "us-central1-a";
private static final String CLUSTER_NAME =
"test-cluster-" + UUID.randomUUID().toString().substring(0, 8);
CONTAINER_PREFIX + "-cluster-" + UUID.randomUUID().toString().substring(0, 8);
private static final String NODE_POOL_NAME =
"test-node-pool-" + UUID.randomUUID().toString().substring(0, 8);
CONTAINER_PREFIX + "-node-pool-" + UUID.randomUUID().toString().substring(0, 8);
private static final String DETAIL = "test-detail";
private static final String STATUS_MESSAGE = "test-status-message";
private static final String SELF_LINK =
Expand All @@ -63,7 +66,7 @@ public static void beforeClass() throws Exception {
client = ClusterManagerClient.create();
Util.cleanUpExistingInstanceCluster(PROJECT_ID, ZONE, client);

/** create node pool* */
/* create node pool* */
NodePool nodePool =
NodePool.newBuilder()
.setInitialNodeCount(INITIAL_NODE_COUNT)
Expand All @@ -72,7 +75,7 @@ public static void beforeClass() throws Exception {
.setStatusMessage(STATUS_MESSAGE)
.build();

/** create cluster */
/* create cluster */
Cluster cluster =
Cluster.newBuilder()
.setName(CLUSTER_NAME)
Expand All @@ -84,13 +87,20 @@ public static void beforeClass() throws Exception {
.setNetwork(NETWORK)
.build();
operation = client.createCluster(PROJECT_ID, ZONE, cluster);

Operation response = client.getOperation(PROJECT_ID, ZONE, operation.getName());
// Busy Wait for one minute at a time until Cluster CREATE operation is complete
while (response.getStatus() != Status.DONE) {
LOG.info(String.format("Cluster CREATE Operation Status: %s", response.getStatus()));
Thread.sleep(TimeUnit.MINUTES.toMillis(1));
response = client.getOperation(PROJECT_ID, ZONE, operation.getName());
}
LOG.info(String.format("%s cluster created successfully.", CLUSTER_NAME));
LOG.info(String.format("%s node pool created successfully.", NODE_POOL_NAME));
}

@AfterClass
public static void afterClass() throws Exception {
Thread.sleep(TimeUnit.MINUTES.toMillis(5));
public static void afterClass() {
client.deleteCluster(PROJECT_ID, ZONE, CLUSTER_NAME);
LOG.info(String.format("%s cluster deleted successfully.", CLUSTER_NAME));
client.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,26 @@
import com.google.cloud.container.v1.ClusterManagerClient;
import com.google.container.v1.Cluster;
import com.google.container.v1.ListClustersResponse;
import java.io.IOException;
import java.time.Instant;
import java.time.OffsetDateTime;
import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.concurrent.ExecutionException;

public class Util {

// Cleans existing test resources if any.
private static final int DELETION_THRESHOLD_TIME_HOURS = 24;

/** tear down any clusters that are older than 24 hours * */
public static void cleanUpExistingInstanceCluster(
String projectId, String zone, ClusterManagerClient client)
throws IOException, ExecutionException, InterruptedException {
String projectId, String zone, ClusterManagerClient client) {

ListClustersResponse clustersResponse = client.listClusters(projectId, zone);
List<Cluster> clusters = clustersResponse.getClustersList();

for (Cluster cluster : clusters) {
if (isCreatedBeforeThresholdTime(cluster.getCreateTime())) {
if (isCreatedBeforeThresholdTime(cluster.getCreateTime())
&& cluster.getName().startsWith(ITSystemTest.CONTAINER_PREFIX)) {
client.deleteCluster(projectId, zone, cluster.getName());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,15 @@

public class ITNotebookServiceClientTest {

protected static final String NOTEBOOK_PREFIX = "it-test-notebook";

private static final String PROJECT_ID = ServiceOptions.getDefaultProjectId();
private static final String LOCATION = "us-central1-a";
private static final String PARENT = "projects/" + PROJECT_ID + "/locations/" + LOCATION;
private static NotebookServiceClient client;
private static final String ID = UUID.randomUUID().toString().substring(0, 8);
private static final String NOTEBOOK_INSTANCE_ID = "test-notebook-instance-id-" + ID;
private static final String ENVIRONMENT_ID = "test-environment-id-" + ID;
private static final String NOTEBOOK_INSTANCE_ID = NOTEBOOK_PREFIX + "-instance-id-" + ID;
private static final String ENVIRONMENT_ID = NOTEBOOK_PREFIX + "-environment-id-" + ID;
private static final String INSTANCE_NAME = PARENT + "/instances/" + NOTEBOOK_INSTANCE_ID;
private static final String ENVIRONMENT_NAME = PARENT + "/environments/" + ENVIRONMENT_ID;
private static Instance expectedNotebookInstance;
Expand All @@ -66,6 +68,8 @@ public class ITNotebookServiceClientTest {
public static void setUp() throws IOException, ExecutionException, InterruptedException {
// Create Test Notebook Instance
client = NotebookServiceClient.create();
Util.cleanUpNotebookInstances(client, PARENT);

ContainerImage containerImage =
ContainerImage.newBuilder().setRepository(FieldBehavior.OPTIONAL.name()).build();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.google.cloud.notebooks.v1beta1.it;

import com.google.cloud.notebooks.v1beta1.DeleteInstanceRequest;
import com.google.cloud.notebooks.v1beta1.Instance;
import com.google.cloud.notebooks.v1beta1.ListInstancesRequest;
import com.google.cloud.notebooks.v1beta1.NotebookServiceClient;
import com.google.cloud.notebooks.v1beta1.NotebookServiceClient.ListInstancesPagedResponse;
import com.google.protobuf.util.Timestamps;
import java.time.Instant;
import java.time.temporal.ChronoUnit;

public class Util {

// Cleans existing test resources if any.
private static final int DELETION_THRESHOLD_TIME_HOURS = 24;

/** Bring down any instances that are older than 24 hours */
public static void cleanUpNotebookInstances(NotebookServiceClient client, String parent) {
ListInstancesPagedResponse listInstancesPagedResponse =
client.listInstances(ListInstancesRequest.newBuilder().setParent(parent).build());
for (Instance instance : listInstancesPagedResponse.iterateAll()) {
if (isCreatedBeforeThresholdTime(
Instant.ofEpochMilli(Timestamps.toMillis(instance.getCreateTime())))
&& instance.getName().startsWith(ITNotebookServiceClientTest.NOTEBOOK_PREFIX)) {
client.deleteInstanceAsync(
DeleteInstanceRequest.newBuilder().setName(instance.getName()).build());
}
}
}

private static boolean isCreatedBeforeThresholdTime(Instant instant) {
return instant.isBefore(Instant.now().minus(DELETION_THRESHOLD_TIME_HOURS, ChronoUnit.HOURS));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ public class ITSystemTest {
private static final String ID = UUID.randomUUID().toString().substring(0, 8);
// GraalVM native-image test uses the project root as working directory, not google-cloud-vision
private static final String RESOURCES =
Files.exists(Paths.get("google-cloud-vision", "src", "test", "resources"))
? "google-cloud-vision/src/test/resources/"
Files.exists(Paths.get("java-vision","google-cloud-vision", "src", "test", "resources"))
? "java-vision/google-cloud-vision/src/test/resources/"
: "src/test/resources/";

private static final String GCS_BUCKET_ENV_VAR = "GOOGLE_CLOUD_TESTS_VISION_BUCKET";
Expand Down
74 changes: 37 additions & 37 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -155,42 +155,42 @@
</modules>

<build>
<plugins>
<!-- jacoco code coverage -->
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.7</version>
<executions>
<execution>
<goals>
<goal>prepare-agent</goal>
</goals>
</execution>
<execution>
<id>report</id>
<phase>test</phase>
<goals>
<goal>report</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skip>${skipUnitTests}</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<forkCount>1C</forkCount>
<reuseForks>true</reuseForks>
</configuration>
</plugin>
</plugins>
<plugins>
<!-- jacoco code coverage -->
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.7</version>
<executions>
<execution>
<goals>
<goal>prepare-agent</goal>
</goals>
</execution>
<execution>
<id>report</id>
<phase>test</phase>
<goals>
<goal>report</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skip>${skipUnitTests}</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<forkCount>1C</forkCount>
<reuseForks>true</reuseForks>
</configuration>
</plugin>
</plugins>
</build>
</project>

0 comments on commit 8e4378d

Please sign in to comment.