Skip to content

Commit

Permalink
chore: Temporarily move DirectPath tests back to the test endpoint. (g…
Browse files Browse the repository at this point in the history 
…oogleapis#44)

* Temporarily update DirectPath tests to use test env to unblock internal testing until the prod endpoint goes live

* add missing port

* format

* Rename ProdEnv -> CloudEnv to avoid confusion

* fix: handle recovery failures during stream reframing failure

This was discovered while debugging another issue. While deflaking
ReadRowRetryTest, this issue came up preventing me from seeing the
underlying issue.

ReframingResponseObserver#deliverUnsafe() should never fail. However if
does, it will try to cancel the upstream stream and notify the
downstream observer. However canceling the upstream can throw an
exception and prevent the downstram observer from being notified of any
error.

This fix will catch cancellation errors and add them as suppressed
exceptions to the original failure

* fix: fix maven test configs to make sure admin tests don't clobber data tests
  • Loading branch information
@igorbernstein2
igorbernstein2 authored Oct 28, 2019
1 parent 18576f5 commit 3a500af
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 17 deletions.
9 changes: 7 additions & 2 deletions google-cloud-bigtable/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@
</goals>
<configuration>
<systemPropertyVariables>
<bigtable.env>prod</bigtable.env>
<bigtable.env>cloud</bigtable.env>
</systemPropertyVariables>
<includes>
<include>com.google.cloud.bigtable.**.it.*IT</include>
Expand Down Expand Up @@ -262,9 +262,14 @@
</goals>
<configuration>
<systemPropertyVariables>
<bigtable.env>prod</bigtable.env>
<bigtable.env>cloud</bigtable.env>
<!-- TODO(igorbernstein): This property should be auto set by gax -->
<io.grpc.internal.DnsNameResolverProvider.enable_grpclb>true</io.grpc.internal.DnsNameResolverProvider.enable_grpclb>

<!-- TODO(igorbernstein): Remove overrides once the prod DirectPath endpoint is available. -->
<!-- Use test env until the prod endpoint is available -->
<bigtable.data-endpoint>testdirectpath-bigtable.sandbox.googleapis.com:443</bigtable.data-endpoint>
<bigtable.admin-endpoint>test-bigtableadmin.sandbox.googleapis.com:443</bigtable.admin-endpoint>
</systemPropertyVariables>
<!-- Enable directpath for bigtable -->
<environmentVariables>
Expand Down
56 changes: 45 additions & 11 deletions ...ud/bigtable/test_helpers/env/ProdEnv.java → ...d/bigtable/test_helpers/env/CloudEnv.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@
package com.google.cloud.bigtable.test_helpers.env;

import com.google.cloud.bigtable.admin.v2.BigtableInstanceAdminClient;
import com.google.cloud.bigtable.admin.v2.BigtableInstanceAdminSettings;
import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
import com.google.cloud.bigtable.admin.v2.BigtableTableAdminSettings;
import com.google.cloud.bigtable.data.v2.BigtableDataClient;
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
import com.google.common.base.MoreObjects;
import java.io.IOException;
import javax.annotation.Nullable;

/**
* Test environment that uses an existing bigtable table. The table must have a pre-existing family
Expand All @@ -31,7 +35,10 @@
* <li>{@code bigtable.table}
* </ul>
*/
class ProdEnv extends AbstractTestEnv {
class CloudEnv extends AbstractTestEnv {
private static final String DATA_ENDPOINT_PROPERTY_NAME = "bigtable.data-endpoint";
private static final String ADMIN_ENDPOINT_PROPERTY_NAME = "bigtable.admin-endpoint";

private static final String PROJECT_PROPERTY_NAME = "bigtable.project";
private static final String INSTANCE_PROPERTY_NAME = "bigtable.instance";
private static final String TABLE_PROPERTY_NAME = "bigtable.table";
Expand All @@ -40,37 +47,60 @@ class ProdEnv extends AbstractTestEnv {
private final String instanceId;
private final String tableId;

private final BigtableDataSettings dataSettings;
private final BigtableDataSettings.Builder dataSettings;
private final BigtableTableAdminSettings.Builder tableAdminSettings;
private final BigtableInstanceAdminSettings.Builder instanceAdminSettings;

private BigtableDataClient dataClient;
private BigtableTableAdminClient tableAdminClient;
private BigtableInstanceAdminClient instanceAdminClient;

static ProdEnv fromSystemProperties() {
return new ProdEnv(
static CloudEnv fromSystemProperties() {
return new CloudEnv(
getOptionalProperty(DATA_ENDPOINT_PROPERTY_NAME, "bigtable.googleapis.com:443"),
getOptionalProperty(ADMIN_ENDPOINT_PROPERTY_NAME, "bigtableadmin.googleapis.com:443"),
getRequiredProperty(PROJECT_PROPERTY_NAME),
getRequiredProperty(INSTANCE_PROPERTY_NAME),
getRequiredProperty(TABLE_PROPERTY_NAME));
}

private ProdEnv(String projectId, String instanceId, String tableId) {
private CloudEnv(
@Nullable String dataEndpoint,
@Nullable String adminEndpoint,
String projectId,
String instanceId,
String tableId) {
this.projectId = projectId;
this.instanceId = instanceId;
this.tableId = tableId;

this.dataSettings =
BigtableDataSettings.newBuilder().setProjectId(projectId).setInstanceId(instanceId).build();
BigtableDataSettings.newBuilder().setProjectId(projectId).setInstanceId(instanceId);
if (dataEndpoint != null) {
dataSettings.stubSettings().setEndpoint(dataEndpoint);
}

this.tableAdminSettings =
BigtableTableAdminSettings.newBuilder().setProjectId(projectId).setInstanceId(instanceId);
if (adminEndpoint != null) {
this.tableAdminSettings.stubSettings().setEndpoint(adminEndpoint);
}

this.instanceAdminSettings = BigtableInstanceAdminSettings.newBuilder().setProjectId(projectId);
if (adminEndpoint != null) {
this.instanceAdminSettings.stubSettings().setEndpoint(adminEndpoint);
}
}

@Override
void start() throws IOException {
dataClient = BigtableDataClient.create(dataSettings);
tableAdminClient = BigtableTableAdminClient.create(projectId, instanceId);
instanceAdminClient = BigtableInstanceAdminClient.create(projectId);
dataClient = BigtableDataClient.create(dataSettings.build());
tableAdminClient = BigtableTableAdminClient.create(tableAdminSettings.build());
instanceAdminClient = BigtableInstanceAdminClient.create(instanceAdminSettings.build());
}

@Override
void stop() throws Exception {
void stop() {
dataClient.close();
tableAdminClient.close();
instanceAdminClient.close();
Expand All @@ -93,7 +123,7 @@ public BigtableInstanceAdminClient getInstanceAdminClient() {

@Override
public BigtableDataSettings getDataClientSettings() {
return dataSettings;
return dataSettings.build();
}

@Override
Expand All @@ -111,6 +141,10 @@ public String getTableId() {
return tableId;
}

private static String getOptionalProperty(String prop, String defaultValue) {
return MoreObjects.firstNonNull(System.getProperty(prop), defaultValue);
}

private static String getRequiredProperty(String prop) {
String value = System.getProperty(prop);
if (value == null || value.isEmpty()) {
Expand Down
8 changes: 4 additions & 4 deletions ...-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
* <ul>
* <li>{@code emulator}: uses the cbtemulator component that can be installed by gcloud
* <li>{@code prod}: uses a pre-existing production table. The target table is defined using
* system properties listed in {@link ProdEnv} and application default credentials
* system properties listed in {@link CloudEnv} and application default credentials
* <li>{@code direct_path}: uses a pre-existing table in the direct path test environment. The
* target table is defined using system properties listed in {@link ProdEnv} and application
* target table is defined using system properties listed in {@link CloudEnv} and application
* default credentials
* </ul>
*
Expand Down Expand Up @@ -60,8 +60,8 @@ protected void before() throws Throwable {
case "emulator":
testEnv = EmulatorEnv.createBundled();
break;
case "prod":
testEnv = ProdEnv.fromSystemProperties();
case "cloud":
testEnv = CloudEnv.fromSystemProperties();
break;
default:
throw new IllegalArgumentException(
Expand Down

0 comments on commit 3a500af

Please sign in to comment.