refactor(gce): introduce a WaitableComputeOperation (#3715)

* refactor(gce): move the GoogleServerGroupManagers and related classes * refactor(gce): introduce a WaitableComputeOperation
spinnaker · May 23, 2019 · 55b8da0 · 55b8da0
1 parent 4295373
commit 55b8da0
Show file tree

Hide file tree

Showing 17 changed files with 127 additions and 96 deletions.
diff --git a/...ps/AbstractGoogleServerGroupManagers.java → ...te/AbstractGoogleServerGroupManagers.java b/...ps/AbstractGoogleServerGroupManagers.java → ...te/AbstractGoogleServerGroupManagers.java
@@ -1,4 +1,4 @@
-package com.netflix.spinnaker.clouddriver.google.deploy.instancegroups;
+package com.netflix.spinnaker.clouddriver.google.compute;
 
 import com.google.api.client.googleapis.services.AbstractGoogleClientRequest;
 import com.google.api.services.compute.ComputeRequest;
@@ -26,16 +26,16 @@ public abstract class AbstractGoogleServerGroupManagers implements GoogleServerG
   }
 
   @Override
-  public Operation abandonInstances(List<String> instances) throws IOException {
-    return timeExecute(performAbandonInstances(instances), "abandonInstances");
+  public WaitableComputeOperation abandonInstances(List<String> instances) throws IOException {
+    return wrapOperation(timeExecute(performAbandonInstances(instances), "abandonInstances"));
   }
 
   abstract ComputeRequest<Operation> performAbandonInstances(List<String> instances)
       throws IOException;
 
   @Override
-  public Operation delete() throws IOException {
-    return timeExecute(performDelete(), "delete");
+  public WaitableComputeOperation delete() throws IOException {
+    return wrapOperation(timeExecute(performDelete(), "delete"));
   }
 
   abstract ComputeRequest<Operation> performDelete() throws IOException;
@@ -48,12 +48,14 @@ public InstanceGroupManager get() throws IOException {
   abstract ComputeRequest<InstanceGroupManager> performGet() throws IOException;
 
   @Override
-  public Operation update(InstanceGroupManager content) throws IOException {
-    return timeExecute(performUpdate(content), "update");
+  public WaitableComputeOperation update(InstanceGroupManager content) throws IOException {
+    return wrapOperation(timeExecute(performUpdate(content), "update"));
   }
 
   abstract ComputeRequest<Operation> performUpdate(InstanceGroupManager content) throws IOException;
 
+  abstract WaitableComputeOperation wrapOperation(Operation operation);
+
   private <T> T timeExecute(AbstractGoogleClientRequest<T> request, String api) throws IOException {
     return GoogleExecutor.timeExecute(
         registry,

diff --git a/...ancegroups/GoogleServerGroupManagers.java → ...le/compute/GoogleServerGroupManagers.java b/...ancegroups/GoogleServerGroupManagers.java → ...le/compute/GoogleServerGroupManagers.java
@@ -1,9 +1,8 @@
-package com.netflix.spinnaker.clouddriver.google.deploy.instancegroups;
+package com.netflix.spinnaker.clouddriver.google.compute;
 
 import com.google.api.services.compute.Compute.InstanceGroupManagers;
 import com.google.api.services.compute.Compute.RegionInstanceGroupManagers;
 import com.google.api.services.compute.model.InstanceGroupManager;
-import com.google.api.services.compute.model.Operation;
 import com.netflix.spinnaker.clouddriver.google.model.GoogleServerGroup;
 import java.io.IOException;
 import java.util.List;
@@ -14,13 +13,11 @@
  */
 public interface GoogleServerGroupManagers {
 
-  Operation abandonInstances(List<String> instances) throws IOException;
+  WaitableComputeOperation abandonInstances(List<String> instances) throws IOException;
 
-  Operation delete() throws IOException;
+  WaitableComputeOperation delete() throws IOException;
 
   InstanceGroupManager get() throws IOException;
 
-  Operation update(InstanceGroupManager content) throws IOException;
-
-  GoogleServerGroupOperationPoller getOperationPoller();
+  WaitableComputeOperation update(InstanceGroupManager content) throws IOException;
 }
diff --git a/...ups/GoogleServerGroupManagersFactory.java → ...ute/GoogleServerGroupManagersFactory.java b/...ups/GoogleServerGroupManagersFactory.java → ...ute/GoogleServerGroupManagersFactory.java
@@ -1,4 +1,4 @@
-package com.netflix.spinnaker.clouddriver.google.deploy.instancegroups;
+package com.netflix.spinnaker.clouddriver.google.compute;
 
 import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;

diff --git a/...ups/GoogleServerGroupOperationPoller.java → ...ute/GoogleServerGroupOperationPoller.java b/...ups/GoogleServerGroupOperationPoller.java → ...ute/GoogleServerGroupOperationPoller.java
@@ -1,4 +1,4 @@
-package com.netflix.spinnaker.clouddriver.google.deploy.instancegroups;
+package com.netflix.spinnaker.clouddriver.google.compute;
 
 import com.google.api.services.compute.model.Operation;
 import com.netflix.spinnaker.clouddriver.data.task.Task;

diff --git a/...oups/RegionGoogleServerGroupManagers.java → ...pute/RegionGoogleServerGroupManagers.java b/...oups/RegionGoogleServerGroupManagers.java → ...pute/RegionGoogleServerGroupManagers.java
@@ -1,4 +1,4 @@
-package com.netflix.spinnaker.clouddriver.google.deploy.instancegroups;
+package com.netflix.spinnaker.clouddriver.google.compute;
 
 import com.google.api.services.compute.Compute;
 import com.google.api.services.compute.ComputeRequest;
@@ -7,7 +7,6 @@
 import com.google.api.services.compute.model.RegionInstanceGroupManagersAbandonInstancesRequest;
 import com.google.common.collect.ImmutableList;
 import com.netflix.spectator.api.Registry;
-import com.netflix.spinnaker.clouddriver.data.task.Task;
 import com.netflix.spinnaker.clouddriver.google.GoogleExecutor;
 import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;
 import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
@@ -17,7 +16,7 @@
 class RegionGoogleServerGroupManagers extends AbstractGoogleServerGroupManagers {
 
   private final Compute.RegionInstanceGroupManagers managers;
-  private final RegionGoogleServerGroupOperationPoller operationPoller;
+  private final GoogleOperationPoller operationPoller;
   private final String region;
 
   RegionGoogleServerGroupManagers(
@@ -28,7 +27,7 @@ class RegionGoogleServerGroupManagers extends AbstractGoogleServerGroupManagers
       String region) {
     super(credentials, registry, instanceGroupName);
     this.managers = credentials.getCompute().regionInstanceGroupManagers();
-    this.operationPoller = new RegionGoogleServerGroupOperationPoller(operationPoller);
+    this.operationPoller = operationPoller;
     this.region = region;
   }
 
@@ -57,8 +56,8 @@ ComputeRequest<Operation> performUpdate(InstanceGroupManager content) throws IOE
   }
 
   @Override
-  public GoogleServerGroupOperationPoller getOperationPoller() {
-    return operationPoller;
+  WaitableComputeOperation wrapOperation(Operation operation) {
+    return new RegionalOperation(operation, getCredentials(), operationPoller);
   }
 
   @Override
@@ -74,25 +73,4 @@ List<String> getRegionOrZoneTags() {
         GoogleExecutor.getTAG_REGION(),
         region);
   }
-
-  private class RegionGoogleServerGroupOperationPoller extends GoogleServerGroupOperationPoller {
-
-    private RegionGoogleServerGroupOperationPoller(GoogleOperationPoller poller) {
-      super(poller);
-    }
-
-    @Override
-    public void waitForOperation(Operation operation, Long timeout, Task task, String phase) {
-      getPoller()
-          .waitForRegionalOperation(
-              getCredentials().getCompute(),
-              getProject(),
-              region,
-              operation.getName(),
-              timeout,
-              task,
-              "regional instance group " + getInstanceGroupName(),
-              phase);
-    }
-  }
 }
diff --git a/...e/src/main/groovy/com/netflix/spinnaker/clouddriver/google/compute/RegionalOperation.java b/...e/src/main/groovy/com/netflix/spinnaker/clouddriver/google/compute/RegionalOperation.java
@@ -0,0 +1,36 @@
+package com.netflix.spinnaker.clouddriver.google.compute;
+
+import com.google.api.services.compute.model.Operation;
+import com.netflix.spinnaker.clouddriver.data.task.Task;
+import com.netflix.spinnaker.clouddriver.google.deploy.GCEUtil;
+import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;
+import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
+
+class RegionalOperation implements WaitableComputeOperation {
+
+  private final Operation operation;
+  private final GoogleNamedAccountCredentials credentials;
+  private final GoogleOperationPoller poller;
+
+  RegionalOperation(
+      Operation operation,
+      GoogleNamedAccountCredentials credentials,
+      GoogleOperationPoller poller) {
+    this.operation = operation;
+    this.credentials = credentials;
+    this.poller = poller;
+  }
+
+  @Override
+  public Operation waitForDone(Task task, String phase) {
+    return poller.waitForRegionalOperation(
+        credentials.getCompute(),
+        credentials.getProject(),
+        operation.getRegion(),
+        operation.getName(),
+        /* timeoutSeconds= */ null,
+        task,
+        GCEUtil.getLocalName(operation.getTargetLink()),
+        phase);
+  }
+}
diff --git a/...ain/groovy/com/netflix/spinnaker/clouddriver/google/compute/WaitableComputeOperation.java b/...ain/groovy/com/netflix/spinnaker/clouddriver/google/compute/WaitableComputeOperation.java
@@ -0,0 +1,9 @@
+package com.netflix.spinnaker.clouddriver.google.compute;
+
+import com.google.api.services.compute.model.Operation;
+import com.netflix.spinnaker.clouddriver.data.task.Task;
+
+public interface WaitableComputeOperation {
+
+  Operation waitForDone(Task task, String phase);
+}
diff --git a/...ogle/src/main/groovy/com/netflix/spinnaker/clouddriver/google/compute/ZonalOperation.java b/...ogle/src/main/groovy/com/netflix/spinnaker/clouddriver/google/compute/ZonalOperation.java
@@ -0,0 +1,36 @@
+package com.netflix.spinnaker.clouddriver.google.compute;
+
+import com.google.api.services.compute.model.Operation;
+import com.netflix.spinnaker.clouddriver.data.task.Task;
+import com.netflix.spinnaker.clouddriver.google.deploy.GCEUtil;
+import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;
+import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
+
+class ZonalOperation implements WaitableComputeOperation {
+
+  private final Operation operation;
+  private final GoogleNamedAccountCredentials credentials;
+  private final GoogleOperationPoller poller;
+
+  ZonalOperation(
+      Operation operation,
+      GoogleNamedAccountCredentials credentials,
+      GoogleOperationPoller poller) {
+    this.operation = operation;
+    this.credentials = credentials;
+    this.poller = poller;
+  }
+
+  @Override
+  public Operation waitForDone(Task task, String phase) {
+    return poller.waitForZonalOperation(
+        credentials.getCompute(),
+        credentials.getProject(),
+        GCEUtil.getLocalName(operation.getZone()),
+        operation.getName(),
+        /* timeoutSeconds= */ null,
+        task,
+        GCEUtil.getLocalName(operation.getTargetLink()),
+        phase);
+  }
+}
diff --git a/...groups/ZoneGoogleServerGroupManagers.java → ...ompute/ZoneGoogleServerGroupManagers.java b/...groups/ZoneGoogleServerGroupManagers.java → ...ompute/ZoneGoogleServerGroupManagers.java
@@ -1,4 +1,4 @@
-package com.netflix.spinnaker.clouddriver.google.deploy.instancegroups;
+package com.netflix.spinnaker.clouddriver.google.compute;
 
 import com.google.api.services.compute.Compute;
 import com.google.api.services.compute.ComputeRequest;
@@ -7,7 +7,6 @@
 import com.google.api.services.compute.model.Operation;
 import com.google.common.collect.ImmutableList;
 import com.netflix.spectator.api.Registry;
-import com.netflix.spinnaker.clouddriver.data.task.Task;
 import com.netflix.spinnaker.clouddriver.google.GoogleExecutor;
 import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller;
 import com.netflix.spinnaker.clouddriver.google.security.GoogleNamedAccountCredentials;
@@ -17,7 +16,7 @@
 class ZoneGoogleServerGroupManagers extends AbstractGoogleServerGroupManagers {
 
   private final Compute.InstanceGroupManagers managers;
-  private final ZoneGoogleServerGroupOperationPoller operationPoller;
+  private final GoogleOperationPoller operationPoller;
   private final String zone;
 
   ZoneGoogleServerGroupManagers(
@@ -28,7 +27,7 @@ class ZoneGoogleServerGroupManagers extends AbstractGoogleServerGroupManagers {
       String zone) {
     super(credentials, registry, instanceGroupName);
     this.managers = credentials.getCompute().instanceGroupManagers();
-    this.operationPoller = new ZoneGoogleServerGroupOperationPoller(operationPoller);
+    this.operationPoller = operationPoller;
     this.zone = zone;
   }
 
@@ -57,8 +56,8 @@ ComputeRequest<Operation> performUpdate(InstanceGroupManager content) throws IOE
   }
 
   @Override
-  public GoogleServerGroupOperationPoller getOperationPoller() {
-    return operationPoller;
+  WaitableComputeOperation wrapOperation(Operation operation) {
+    return new ZonalOperation(operation, getCredentials(), operationPoller);
   }
 
   @Override
@@ -74,25 +73,4 @@ List<String> getRegionOrZoneTags() {
         GoogleExecutor.getTAG_ZONE(),
         zone);
   }
-
-  private class ZoneGoogleServerGroupOperationPoller extends GoogleServerGroupOperationPoller {
-
-    private ZoneGoogleServerGroupOperationPoller(GoogleOperationPoller poller) {
-      super(poller);
-    }
-
-    @Override
-    public void waitForOperation(Operation operation, Long timeout, Task task, String phase) {
-      getPoller()
-          .waitForZonalOperation(
-              getCredentials().getCompute(),
-              getProject(),
-              zone,
-              operation.getName(),
-              timeout,
-              task,
-              "zonal instance group " + getInstanceGroupName(),
-              phase);
-    }
-  }
 }
diff --git a/...innaker/clouddriver/google/deploy/converters/SetStatefulDiskAtomicOperationConverter.java b/...innaker/clouddriver/google/deploy/converters/SetStatefulDiskAtomicOperationConverter.java
@@ -1,8 +1,8 @@
 package com.netflix.spinnaker.clouddriver.google.deploy.converters;
 
 import com.netflix.spinnaker.clouddriver.google.GoogleOperation;
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagersFactory;
 import com.netflix.spinnaker.clouddriver.google.deploy.description.SetStatefulDiskDescription;
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagersFactory;
 import com.netflix.spinnaker.clouddriver.google.deploy.ops.SetStatefulDiskAtomicOperation;
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperations;

diff --git a/.../clouddriver/google/deploy/ops/AbandonAndDecrementGoogleServerGroupAtomicOperation.groovy b/.../clouddriver/google/deploy/ops/AbandonAndDecrementGoogleServerGroupAtomicOperation.groovy
@@ -20,7 +20,7 @@ package com.netflix.spinnaker.clouddriver.google.deploy.ops
 import com.netflix.spinnaker.clouddriver.data.task.Task
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository
 import com.netflix.spinnaker.clouddriver.google.deploy.GCEUtil
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagersFactory
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagersFactory
 import com.netflix.spinnaker.clouddriver.google.deploy.description.AbandonAndDecrementGoogleServerGroupDescription
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider
 import org.springframework.beans.factory.annotation.Autowired

diff --git a/...ix/spinnaker/clouddriver/google/deploy/ops/DestroyGoogleServerGroupAtomicOperation.groovy b/...ix/spinnaker/clouddriver/google/deploy/ops/DestroyGoogleServerGroupAtomicOperation.groovy
@@ -23,7 +23,7 @@ import com.netflix.spinnaker.clouddriver.google.deploy.GCEUtil
 import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller
 import com.netflix.spinnaker.clouddriver.google.deploy.SafeRetry
 import com.netflix.spinnaker.clouddriver.google.deploy.description.DestroyGoogleServerGroupDescription
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagersFactory
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagersFactory
 import com.netflix.spinnaker.clouddriver.google.model.GoogleServerGroup
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleLoadBalancerProvider
@@ -245,7 +245,7 @@ class DestroyGoogleServerGroupAtomicOperation extends GoogleAtomicOperation<Void
 
       task.updateStatus BASE_PHASE, "Waiting on delete operation for managed instance group..."
 
-      serverGroupManagers.operationPoller.waitForOperation(deleteOperation, /* timeout= */ null, task, BASE_PHASE)
+      deleteOperation.waitForDone(task, BASE_PHASE)
       null
     }
   }

diff --git a/...y/com/netflix/spinnaker/clouddriver/google/deploy/ops/SetStatefulDiskAtomicOperation.java b/...y/com/netflix/spinnaker/clouddriver/google/deploy/ops/SetStatefulDiskAtomicOperation.java
@@ -1,17 +1,17 @@
 package com.netflix.spinnaker.clouddriver.google.deploy.ops;
 
 import com.google.api.services.compute.model.InstanceGroupManager;
-import com.google.api.services.compute.model.Operation;
 import com.google.api.services.compute.model.StatefulPolicy;
 import com.google.api.services.compute.model.StatefulPolicyPreservedState;
 import com.google.api.services.compute.model.StatefulPolicyPreservedStateDiskDevice;
 import com.google.common.annotations.VisibleForTesting;
 import com.netflix.spinnaker.clouddriver.data.task.Task;
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository;
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagers;
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagersFactory;
+import com.netflix.spinnaker.clouddriver.google.compute.WaitableComputeOperation;
 import com.netflix.spinnaker.clouddriver.google.deploy.GCEUtil;
 import com.netflix.spinnaker.clouddriver.google.deploy.description.SetStatefulDiskDescription;
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagers;
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagersFactory;
 import com.netflix.spinnaker.clouddriver.google.model.GoogleServerGroup;
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider;
 import java.io.IOException;
@@ -77,14 +77,11 @@ public Void operate(List priorOutputs) {
 
       task.updateStatus(BASE_PHASE, "Storing updated instance group definition");
 
-      Operation operation = managers.update(instanceGroupManager);
+      WaitableComputeOperation operation = managers.update(instanceGroupManager);
 
       task.updateStatus(BASE_PHASE, "Waiting for update to complete");
 
-      managers
-          .getOperationPoller()
-          .waitForOperation(
-              operation, /* timeout= */ null, TaskRepository.threadLocalTask.get(), BASE_PHASE);
+      operation.waitForDone(TaskRepository.threadLocalTask.get(), BASE_PHASE);
 
       return null;
     } catch (IOException e) {

diff --git a/...ker/clouddriver/google/deploy/converters/SetStatefulDiskAtomicOperationConverterTest.java b/...ker/clouddriver/google/deploy/converters/SetStatefulDiskAtomicOperationConverterTest.java
@@ -6,8 +6,8 @@
 import static org.mockito.Mockito.when;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagersFactory;
 import com.netflix.spinnaker.clouddriver.google.deploy.description.SetStatefulDiskDescription;
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagersFactory;
 import com.netflix.spinnaker.clouddriver.google.deploy.ops.SetStatefulDiskAtomicOperation;
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider;
 import com.netflix.spinnaker.clouddriver.google.security.FakeGoogleCredentials;

diff --git a/...iver/google/deploy/ops/AbandonAndDecrementGoogleServerGroupAtomicOperationUnitSpec.groovy b/...iver/google/deploy/ops/AbandonAndDecrementGoogleServerGroupAtomicOperationUnitSpec.groovy
@@ -24,7 +24,7 @@ import com.netflix.spinnaker.clouddriver.data.task.TaskRepository
 import com.netflix.spinnaker.clouddriver.google.deploy.GCEUtil
 import com.netflix.spinnaker.clouddriver.google.deploy.GoogleOperationPoller
 import com.netflix.spinnaker.clouddriver.google.deploy.description.AbandonAndDecrementGoogleServerGroupDescription
-import com.netflix.spinnaker.clouddriver.google.deploy.instancegroups.GoogleServerGroupManagersFactory
+import com.netflix.spinnaker.clouddriver.google.compute.GoogleServerGroupManagersFactory
 import com.netflix.spinnaker.clouddriver.google.model.GoogleInstance
 import com.netflix.spinnaker.clouddriver.google.model.GoogleServerGroup
 import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider