Skip to content

Commit

Permalink
refactor(gce): Convert GoogleRegionalServerGroupCachingAgent to java (#…
Browse files Browse the repository at this point in the history
…4007)

* refactor(gce): remove an unnecessary checkState()

We can survive this condition just fine.

* test(gce): Add a test about on-demand caching behavior

This tests the behavior discussed on the review of #4002. I should have
added it there, but I didn't think about it until now.

* refactor(gce): fix an import

* refactor(gce): remove `static` from a method

* refactor(gce): use getAccountName() consistently

* docs(gce): remove a TODO

It actually does seem to be true that an autoscaler's name always equals
that of its associated instance group. This _might_ even be a GCE rule,
but it's definitely something we always do in Spinnaker (see
UpsertGoogleAutoscalingPolicyAtomicOperation and
GCEUtil#buildAutoscaler).

* refactor(gce): Prepare zonal caching agent for an abstract superclass

About  90% of the code from GoogleZonalServerGroupCachingAgent was just
copied into GoogleRegionalServerGroupCachingAgent. I'm going to extract
all the common code into an abstract base class.

Pretty much all that will be left after I do that are the methods
marked `// @Override`. Those methods will be the abstract ones in the
superclass.

* refactor(gce): Create an AbstractGoogleServerGroupCachingAgent

The code is just copy-pasted directly from
GoogleZonalServerGroupCachingAgent except that I replaced the methods
marked `// @Override` with abstract methods. No other changes were made.

* test(gce): Add tests for GoogleRegionalServerGroupCachingAgent

* refactor(gce): Convert GoogleRegionalServerGroupCachingAgent to java

* refactor(gce): Convert GoogleRegionalServerGroupCachingAgent to java

* test(gce): Move server property tests into a test of the abstract class

It seems somewhat pointless to have two copies of these. They're
essentially just data transformation tests, not really doing anything
fancy depending on the subclass.

A lot of the other tests could potentially also be merged but I think
that would lead to some pretty weird tests that are less useful, so I'd
rather keep those separate.

* fix(gce): fix a small bug in GoogleRegionalServerGroupCachingAgent

This bug is likely pretty innocuous since callers are looking for a
specific key amongst the pendingOnDemandRequests and returning an extra
one won't hurt anything, but I might as well fix it.

It looks like this bug likely exists in other caching agents, too, but
since it's very long-standing and the impact is negligible, I'm going to
ignore that.

* refactor(gce): convert anonymous test class to inner class

* refactor(gce): clean up some of the test code
  • Loading branch information
plumpy committed Sep 9, 2019
1 parent f4b00f2 commit 59f69b6
Show file tree
Hide file tree
Showing 12 changed files with 2,817 additions and 2,213 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ public InstanceTemplates createInstanceTemplates(GoogleNamedAccountCredentials c
return new InstanceTemplates(credentials, operationPoller, registry);
}

public RegionAutoscalers createRegionAutoscalers(GoogleNamedAccountCredentials credentials) {
return new RegionAutoscalers(credentials, operationPoller, registry);
}

public RegionInstanceGroupManagers createRegionInstanceGroupManagers(
GoogleNamedAccountCredentials credentials) {
return new RegionInstanceGroupManagers(credentials, operationPoller, registry);
}

public ZoneAutoscalers createZoneAutoscalers(GoogleNamedAccountCredentials credentials) {
return new ZoneAutoscalers(credentials, operationPoller, registry);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright 2019 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.netflix.spinnaker.clouddriver.google.compute;

import com.google.api.services.compute.Compute;
import com.google.api.services.compute.model.Autoscaler;
import com.google.api.services.compute.model.RegionAutoscalerList;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;
import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
import java.io.IOException;

public final class RegionAutoscalers {

private final Compute.RegionAutoscalers computeApi;
private final GoogleNamedAccountCredentials credentials;
private final RegionalGoogleComputeRequestFactory requestFactory;

RegionAutoscalers(
GoogleNamedAccountCredentials credentials,
GoogleOperationPoller operationPoller,
Registry registry) {
this.computeApi = credentials.getCompute().regionAutoscalers();
this.credentials = credentials;
this.requestFactory =
new RegionalGoogleComputeRequestFactory(
"regionAutoscalers", credentials, operationPoller, registry);
}

public GoogleComputeGetRequest<Compute.RegionAutoscalers.Get, Autoscaler> get(
String region, String name) throws IOException {
return requestFactory.wrapGetRequest(
computeApi.get(credentials.getProject(), region, name), "get", region);
}

public PaginatedComputeRequest<Compute.RegionAutoscalers.List, Autoscaler> list(String region) {
return new PaginatedComputeRequestImpl<>(
pageToken ->
requestFactory.wrapRequest(
computeApi.list(credentials.getProject(), region).setPageToken(pageToken),
"list",
region),
RegionAutoscalerList::getNextPageToken,
RegionAutoscalerList::getItems);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright 2019 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.netflix.spinnaker.clouddriver.google.compute;

import com.google.api.services.compute.Compute;
import com.google.api.services.compute.model.InstanceGroupManager;
import com.google.api.services.compute.model.RegionInstanceGroupManagerList;
import com.google.common.collect.ImmutableList;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;
import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
import java.io.IOException;
import java.util.Optional;

public final class RegionInstanceGroupManagers {

private final Compute.RegionInstanceGroupManagers computeApi;
private final GoogleNamedAccountCredentials credentials;
private final RegionalGoogleComputeRequestFactory requestFactory;

RegionInstanceGroupManagers(
GoogleNamedAccountCredentials credentials,
GoogleOperationPoller operationPoller,
Registry registry) {
this.computeApi = credentials.getCompute().regionInstanceGroupManagers();
this.credentials = credentials;
this.requestFactory =
new RegionalGoogleComputeRequestFactory(
"regionInstanceGroupManagers", credentials, operationPoller, registry);
}

public GoogleComputeGetRequest<Compute.RegionInstanceGroupManagers.Get, InstanceGroupManager> get(
String region, String name) throws IOException {
return requestFactory.wrapGetRequest(
computeApi.get(credentials.getProject(), region, name), "get", region);
}

public PaginatedComputeRequest<Compute.RegionInstanceGroupManagers.List, InstanceGroupManager>
list(String region) {
return new PaginatedComputeRequestImpl<>(
pageToken ->
requestFactory.wrapRequest(
computeApi.list(credentials.getProject(), region).setPageToken(pageToken),
"list",
region),
RegionInstanceGroupManagerList::getNextPageToken,
response -> Optional.ofNullable(response.getItems()).orElseGet(ImmutableList::of));
}
}
Loading

0 comments on commit 59f69b6

Please sign in to comment.