Skip to content

Commit 228b7e5

Browse files
claire921claire921
committed
Address comments in the PR and add integration tests.
1 parent 077e118 commit 228b7e5

4 files changed

Lines changed: 48 additions & 77 deletions

File tree

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -193,18 +193,6 @@ public Operation fromProto(Operation proto) {
193193
@Override
194194
public OperationFuture<Instance, CreateInstanceMetadata> createInstance(InstanceInfo instance)
195195
throws SpannerException {
196-
boolean valid_capacity_with_autoscaling =
197-
instance.getAutoscalingConfig() != null
198-
&& instance.getNodeCount() == 0
199-
&& instance.getProcessingUnits() == 0;
200-
boolean valid_capacity_without_autoscaling =
201-
(instance.getAutoscalingConfig() == null)
202-
&& (instance.getNodeCount() == 0 || instance.getProcessingUnits() == 0);
203-
204-
Preconditions.checkArgument(
205-
valid_capacity_with_autoscaling || valid_capacity_without_autoscaling,
206-
"Only one of nodeCount, processingUnits or autoscalingConfig can be set when creating a new instance");
207-
208196
String projectName = PROJECT_NAME_TEMPLATE.instantiate("project", projectId);
209197
OperationFuture<com.google.spanner.admin.instance.v1.Instance, CreateInstanceMetadata>
210198
rawOperationFuture =

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,7 @@ public Builder setProcessingUnits(int processingUnits) {
112112
* instance. Exactly one of processing units, node count, or autoscaling config must be set when
113113
* creating a new instance.
114114
*/
115-
public Builder setAutoscalingConfig(AutoscalingConfig autoscalingConfig) {
116-
throw new UnsupportedOperationException("Unimplemented");
117-
}
115+
public abstract Builder setAutoscalingConfig(AutoscalingConfig autoscalingConfig);
118116

119117
public abstract Builder setState(State state);
120118

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertTrue;
22-
import static org.junit.Assert.fail;
2322
import static org.mockito.Mockito.verify;
2423
import static org.mockito.Mockito.when;
2524
import static org.mockito.MockitoAnnotations.initMocks;
@@ -314,24 +313,6 @@ public void testCreateInstanceWithProcessingUnits() throws Exception {
314313
assertEquals(INSTANCE_NAME, operation.get().getId().getName());
315314
}
316315

317-
@Test
318-
public void testCreateInstanceWithBothNodeCountAndProcessingUnits() throws Exception {
319-
try {
320-
client.createInstance(
321-
InstanceInfo.newBuilder(InstanceId.of(PROJECT_ID, INSTANCE_ID))
322-
.setInstanceConfigId(InstanceConfigId.of(PROJECT_ID, CONFIG_ID))
323-
.setNodeCount(1)
324-
.setProcessingUnits(100)
325-
.build());
326-
fail("missing expected exception");
327-
} catch (IllegalArgumentException e) {
328-
assertTrue(
329-
e.getMessage()
330-
.contains(
331-
"Only one of nodeCount, processingUnits or autoscalingConfig can be set when creating a new instance"));
332-
}
333-
}
334-
335316
@Test
336317
public void testCreateInstanceWithAutoscalingConfig() throws Exception {
337318
OperationFuture<com.google.spanner.admin.instance.v1.Instance, CreateInstanceMetadata>
@@ -352,42 +333,6 @@ public void testCreateInstanceWithAutoscalingConfig() throws Exception {
352333
assertEquals(INSTANCE_NAME, operation.get().getId().getName());
353334
}
354335

355-
@Test
356-
public void testCreateInstanceWithBothNodeCountAndAutoscalingConfig() throws Exception {
357-
try {
358-
client.createInstance(
359-
InstanceInfo.newBuilder(InstanceId.of(PROJECT_ID, INSTANCE_ID))
360-
.setInstanceConfigId(InstanceConfigId.of(PROJECT_ID, CONFIG_ID))
361-
.setNodeCount(1)
362-
.setAutoscalingConfig(getAutoscalingConfigProto())
363-
.build());
364-
fail("missing expected exception");
365-
} catch (IllegalArgumentException e) {
366-
assertTrue(
367-
e.getMessage()
368-
.contains(
369-
"Only one of nodeCount, processingUnits or autoscalingConfig can be set when creating a new instance"));
370-
}
371-
}
372-
373-
@Test
374-
public void testCreateInstanceWithBothProcessingUnitsAndAutoscalingConfig() throws Exception {
375-
try {
376-
client.createInstance(
377-
InstanceInfo.newBuilder(InstanceId.of(PROJECT_ID, INSTANCE_ID))
378-
.setInstanceConfigId(InstanceConfigId.of(PROJECT_ID, CONFIG_ID))
379-
.setProcessingUnits(1000)
380-
.setAutoscalingConfig(getAutoscalingConfigProto())
381-
.build());
382-
fail("missing expected exception");
383-
} catch (IllegalArgumentException e) {
384-
assertTrue(
385-
e.getMessage()
386-
.contains(
387-
"Only one of nodeCount, processingUnits or autoscalingConfig can be set when creating a new instance"));
388-
}
389-
}
390-
391336
@Test
392337
public void testGetInstance() {
393338
when(rpc.getInstance(INSTANCE_NAME)).thenReturn(getInstanceProto());

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITInstanceAdminTest.java‎

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,9 @@
2121
import static org.junit.Assume.assumeFalse;
2222

2323
import com.google.api.gax.longrunning.OperationFuture;
24-
import com.google.cloud.spanner.Instance;
25-
import com.google.cloud.spanner.InstanceAdminClient;
26-
import com.google.cloud.spanner.InstanceConfig;
27-
import com.google.cloud.spanner.InstanceInfo;
28-
import com.google.cloud.spanner.IntegrationTestEnv;
29-
import com.google.cloud.spanner.Options;
30-
import com.google.cloud.spanner.ParallelIntegrationTest;
24+
import com.google.cloud.spanner.*;
3125
import com.google.common.collect.Iterators;
26+
import com.google.spanner.admin.instance.v1.AutoscalingConfig;
3227
import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata;
3328
import java.util.ArrayList;
3429
import java.util.List;
@@ -121,6 +116,51 @@ public void updateInstance() throws Exception {
121116
instanceClient.updateInstance(toUpdate, InstanceInfo.InstanceField.DISPLAY_NAME).get();
122117
}
123118

119+
@Test
120+
public void updateInstanceWithAutoscalingConfig() throws Exception {
121+
assumeFalse(
122+
"The emulator does not support updating instances with autoscaler", isUsingEmulator());
123+
124+
Instance instance =
125+
instanceClient.getInstance(env.getTestHelper().getInstanceId().getInstance());
126+
AutoscalingConfig autoscalingConfig =
127+
AutoscalingConfig.newBuilder()
128+
.setAutoscalingLimits(
129+
AutoscalingConfig.AutoscalingLimits.newBuilder()
130+
.setMinProcessingUnits(1000)
131+
.setMaxProcessingUnits(2000))
132+
.setAutoscalingTargets(
133+
AutoscalingConfig.AutoscalingTargets.newBuilder()
134+
.setHighPriorityCpuUtilizationPercent(65)
135+
.setStorageUtilizationPercent(95))
136+
.build();
137+
InstanceInfo toUpdate =
138+
InstanceInfo.newBuilder(env.getTestHelper().getInstanceId())
139+
.setNodeCount(0)
140+
.setAutoscalingConfig(autoscalingConfig)
141+
.build();
142+
OperationFuture<Instance, UpdateInstanceMetadata> op =
143+
instanceClient.updateInstance(toUpdate, InstanceInfo.InstanceField.AUTOSCALING_CONFIG);
144+
Instance newInstance = op.get();
145+
assertThat(newInstance.getAutoscalingConfig()).isEqualTo(autoscalingConfig);
146+
147+
Instance newInstanceFromGet =
148+
instanceClient.getInstance(env.getTestHelper().getInstanceId().getInstance());
149+
assertThat(newInstanceFromGet).isEqualTo(newInstance);
150+
151+
toUpdate =
152+
InstanceInfo.newBuilder(instance.getId())
153+
.setAutoscalingConfig(null)
154+
.setNodeCount(instance.getNodeCount())
155+
.build();
156+
instanceClient
157+
.updateInstance(
158+
toUpdate,
159+
InstanceInfo.InstanceField.AUTOSCALING_CONFIG,
160+
InstanceInfo.InstanceField.NODE_COUNT)
161+
.get();
162+
}
163+
124164
@Test
125165
public void updateInstanceViaEntity() throws Exception {
126166
assumeFalse("The emulator does not support updating instances", isUsingEmulator());

0 commit comments

Comments
 (0)