fix(provider/google): Wait for forwarding rule creation in LB upserts. (

#1622)
spinnaker · May 10, 2017 · f5f8c57 · f5f8c57
1 parent 45a2047
commit f5f8c57
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 49 deletions.
diff --git a/...ddriver/google/deploy/ops/loadbalancer/UpsertGoogleHttpLoadBalancerAtomicOperation.groovy b/...ddriver/google/deploy/ops/loadbalancer/UpsertGoogleHttpLoadBalancerAtomicOperation.groovy
@@ -474,10 +474,27 @@ class UpsertGoogleHttpLoadBalancerAtomicOperation extends UpsertGoogleLoadBalanc
         portRange: httpLoadBalancer.certificate ? "443" : httpLoadBalancer.portRange,
         target: targetProxyUrl,
       )
-      timeExecute(
-          compute.globalForwardingRules().insert(project, forwardingRule),
-          "compute.globalForwardingRules.insert",
-          TAG_SCOPE, SCOPE_GLOBAL)
+      Operation forwardingRuleOp = safeRetry.doRetry(
+          { timeExecute(
+              compute.globalForwardingRules().insert(project, forwardingRule),
+              "compute.globalForwardingRules.insert",
+              TAG_SCOPE, SCOPE_GLOBAL) },
+          "Global forwarding rule ${description.loadBalancerName}",
+          task,
+          [400, 403, 412],
+          [],
+          [action: "insert", phase: BASE_PHASE, operation: "compute.globalForwardingRules.insert", (TAG_SCOPE): SCOPE_GLOBAL],
+          registry
+      ) as Operation
+
+      // Orca's orchestration for upserting a Google load balancer does not contain a task
+      // to wait for the state of the platform to show that a load balancer was created (for good reason,
+      // that would be a complicated operation). Instead, Orca waits for Clouddriver to execute this operation
+      // and do a force cache refresh. We should wait for the whole load balancer to be created in the platform
+      // before we exit this upsert operation, so we wait for the forwarding rule to be created before continuing
+      // so we _know_ the state of the platform when we do a force cache refresh.
+      googleOperationPoller.waitForGlobalOperation(compute, project, forwardingRuleOp.getName(),
+          null, task, "forwarding rule " + httpLoadBalancerName, BASE_PHASE)
     }
     // NOTE: there is no update for forwarding rules because we support adding/deleting multiple listeners in the frontend.
     // Rotating or changing certificates updates the targetProxy only, so the forwarding rule doesn't need to change.

diff --git a/...ver/google/deploy/ops/loadbalancer/UpsertGoogleInternalLoadBalancerAtomicOperation.groovy b/...ver/google/deploy/ops/loadbalancer/UpsertGoogleInternalLoadBalancerAtomicOperation.groovy
@@ -232,7 +232,7 @@ class UpsertGoogleInternalLoadBalancerAtomicOperation extends GoogleAtomicOperat
         subnetwork: GCEUtil.buildSubnetworkUrl(project, region, description.subnet),
         ports: description.ports
       )
-      safeRetry.doRetry(
+      Operation forwardingRuleOp = safeRetry.doRetry(
         { timeExecute(
               compute.forwardingRules().insert(project, region, forwardingRule),
               "compute.forwardingRules.insert",
@@ -243,7 +243,16 @@ class UpsertGoogleInternalLoadBalancerAtomicOperation extends GoogleAtomicOperat
         [],
         [action: "insert", phase: BASE_PHASE, operation: "compute.forwardingRules.insert", (TAG_SCOPE): SCOPE_GLOBAL],
         registry
-      )
+      ) as Operation
+
+      // Orca's orchestration for upserting a Google load balancer does not contain a task
+      // to wait for the state of the platform to show that a load balancer was created (for good reason,
+      // that would be a complicated operation). Instead, Orca waits for Clouddriver to execute this operation
+      // and do a force cache refresh. We should wait for the whole load balancer to be created in the platform
+      // before we exit this upsert operation, so we wait for the forwarding rule to be created before continuing
+      // so we _know_ the state of the platform when we do a force cache refresh.
+      googleOperationPoller.waitForRegionalOperation(compute, project, region, forwardingRuleOp.getName(),
+          null, task, "forwarding rule " + description.loadBalancerName, BASE_PHASE)
     }
 
     task.updateStatus BASE_PHASE, "Done upserting load balancer $description.loadBalancerName in $region."

diff --git a/...clouddriver/google/deploy/ops/loadbalancer/UpsertGoogleLoadBalancerAtomicOperation.groovy b/...clouddriver/google/deploy/ops/loadbalancer/UpsertGoogleLoadBalancerAtomicOperation.groovy
@@ -31,6 +31,7 @@ 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.GoogleOperationPoller
+import com.netflix.spinnaker.clouddriver.google.deploy.SafeRetry
 import com.netflix.spinnaker.clouddriver.google.deploy.description.UpsertGoogleLoadBalancerDescription
 import com.netflix.spinnaker.clouddriver.google.deploy.exception.GoogleOperationException
 import com.netflix.spinnaker.clouddriver.google.deploy.ops.GoogleAtomicOperation
@@ -39,6 +40,9 @@ import org.springframework.beans.factory.annotation.Autowired
 class UpsertGoogleLoadBalancerAtomicOperation extends GoogleAtomicOperation<Map> {
   private static final String BASE_PHASE = "UPSERT_LOAD_BALANCER"
 
+  @Autowired
+  SafeRetry safeRetry
+
   private static Task getTask() {
     TaskRepository.threadLocalTask.get()
   }
@@ -425,11 +429,27 @@ class UpsertGoogleLoadBalancerAtomicOperation extends GoogleAtomicOperation<Map>
         portRange: description.portRange
       )
 
-      // We won't block on this operation since nothing else depends on its completion.
-      timeExecute(
-          compute.forwardingRules().insert(project, region, forwardingRule),
-          "compute.forwardingRules.insert",
-          TAG_SCOPE, SCOPE_REGIONAL, TAG_REGION, region)
+      Operation forwardingRuleOperation = safeRetry.doRetry(
+          { timeExecute(
+              compute.forwardingRules().insert(project, region, forwardingRule),
+              "compute.forwardingRules.insert",
+              TAG_SCOPE, SCOPE_REGIONAL, TAG_REGION, region) },
+          "Regional forwarding rule ${description.loadBalancerName}",
+          task,
+          [400, 403, 412],
+          [],
+          [action: "insert", phase: BASE_PHASE, operation: "compute.forwardingRules.insert", (TAG_SCOPE): SCOPE_GLOBAL],
+          registry
+      ) as Operation
+
+      // Orca's orchestration for upserting a Google load balancer does not contain a task
+      // to wait for the state of the platform to show that a load balancer was created (for good reason,
+      // that would be a complicated operation). Instead, Orca waits for Clouddriver to execute this operation
+      // and do a force cache refresh. We should wait for the whole load balancer to be created in the platform
+      // before we exit this upsert operation, so we wait for the forwarding rule to be created before continuing
+      // so we _know_ the state of the platform when we do a force cache refresh.
+      googleOperationPoller.waitForRegionalOperation(compute, project, region, forwardingRuleOperation.getName(),
+          null, task, "forwarding rule " + GCEUtil.getLocalName(targetPoolResourceLink), BASE_PHASE)
     }
   }
 }
diff --git a/...uddriver/google/deploy/ops/loadbalancer/UpsertGoogleSslLoadBalancerAtomicOperation.groovy b/...uddriver/google/deploy/ops/loadbalancer/UpsertGoogleSslLoadBalancerAtomicOperation.groovy
@@ -302,6 +302,13 @@ class UpsertGoogleSslLoadBalancerAtomicOperation extends UpsertGoogleLoadBalance
         [action: "insert", phase: BASE_PHASE, operation: "compute.globalForwardingRules.insert", (TAG_SCOPE): SCOPE_GLOBAL],
         registry
       ) as Operation
+
+      // Orca's orchestration for upserting a Google load balancer does not contain a task
+      // to wait for the state of the platform to show that a load balancer was created (for good reason,
+      // that would be a complicated operation). Instead, Orca waits for Clouddriver to execute this operation
+      // and do a force cache refresh. We should wait for the whole load balancer to be created in the platform
+      // before we exit this upsert operation, so we wait for the forwarding rule to be created before continuing
+      // so we _know_ the state of the platform when we do a force cache refresh.
       googleOperationPoller.waitForGlobalOperation(compute, project, ruleOp.getName(),
         null, task, "forwarding rule " + description.loadBalancerName, BASE_PHASE)
     }