fix(clouddriver): WaitForUpInstances should block on Starting health …

…providers (#2935)
spinnaker · May 23, 2019 · c14a58e · c14a58e
1 parent c7ededc
commit c14a58e
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 8 deletions.
diff --git a/.../com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstanceHealthTask.groovy b/.../com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstanceHealthTask.groovy
@@ -25,6 +25,6 @@ import org.springframework.stereotype.Component
 class WaitForUpInstanceHealthTask extends AbstractWaitForInstanceHealthChangeTask {
   @Override
   boolean hasSucceeded(Map instance, Collection<String> interestedHealthProviderNames) {
-    HealthHelper.someAreUpAndNoneAreDown(instance, interestedHealthProviderNames)
+    HealthHelper.someAreUpAndNoneAreDownOrStarting(instance, interestedHealthProviderNames)
   }
 }
diff --git a/...roovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTask.groovy b/...roovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTask.groovy
@@ -93,7 +93,7 @@ class WaitForUpInstancesTask extends AbstractWaitingForInstancesTask {
       }
 
       def healthyCount = instances.count { Map instance ->
-        HealthHelper.someAreUpAndNoneAreDown(instance, interestingHealthProviderNames)
+        HealthHelper.someAreUpAndNoneAreDownOrStarting(instance, interestingHealthProviderNames)
       }
 
       splainer.add("returning healthyCount=${healthyCount} >= targetDesiredSize=${targetDesiredSize}")
@@ -187,7 +187,7 @@ class WaitForUpInstancesTask extends AbstractWaitingForInstancesTask {
     }
     serverGroup.instances.each { Map instance ->
       List<Map> healths = HealthHelper.filterHealths(instance, interestingHealthProviderNames)
-      if (HealthHelper.someAreUpAndNoneAreDown(instance, interestingHealthProviderNames)) {
+      if (HealthHelper.someAreUpAndNoneAreDownOrStarting(instance, interestingHealthProviderNames)) {
         snapshot.up++
       } else if (someAreDown(instance, interestingHealthProviderNames)) {
         snapshot.down++

diff --git a/...uddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/HealthHelper.groovy b/...uddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/HealthHelper.groovy
@@ -102,13 +102,16 @@ class HealthHelper {
     return someAreDown && noneAreUp
   }
 
-  static boolean someAreUpAndNoneAreDown(Map instance, Collection<String> interestingHealthProviderNames) {
+  static boolean someAreUpAndNoneAreDownOrStarting(Map instance, Collection<String> interestingHealthProviderNames) {
     List<Map> healths = filterHealths(instance, interestingHealthProviderNames)
     boolean someAreUp = healths.any { Map health -> health.state == 'Up' }
     someAreUp = areSomeUpConsideringPlatformHealth(healths, interestingHealthProviderNames, someAreUp)
 
-    boolean noneAreDown = !healths.any { Map health -> health.state == 'Down' }
-    return someAreUp && noneAreDown
+    boolean noneAreDownOrStarting = !healths.any {
+      Map health -> health.state == 'Down' || health.state == 'Starting'
+    }
+
+    return someAreUp && noneAreDownOrStarting
   }
 
   static class HealthCountSnapshot {

diff --git a/.../com/netflix/spinnaker/orca/kato/tasks/rollingpush/WaitForNewUpInstancesLaunchTask.groovy b/.../com/netflix/spinnaker/orca/kato/tasks/rollingpush/WaitForNewUpInstancesLaunchTask.groovy
@@ -22,7 +22,6 @@ import com.netflix.spinnaker.orca.clouddriver.utils.HealthHelper
 import java.util.concurrent.TimeUnit
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.orca.ExecutionStatus
-import com.netflix.spinnaker.orca.RetryableTask
 import com.netflix.spinnaker.orca.TaskResult
 import com.netflix.spinnaker.orca.clouddriver.OortService
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData
@@ -60,7 +59,7 @@ class WaitForNewUpInstancesLaunchTask implements OverridableTimeoutRetryableTask
     Set<String> newUpInstanceIds = serverGroupInstances.findResults {
       String id = stageData.cloudProvider == 'titus' ? it.id : it.instanceId
       !knownInstanceIds.contains(id) &&
-        HealthHelper.someAreUpAndNoneAreDown(it, healthProviders) ? id : null
+        HealthHelper.someAreUpAndNoneAreDownOrStarting(it, healthProviders) ? id : null
     }
 
     int expectedNewInstances = (stage.context.instanceIds as List).size()

diff --git a/...y/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTaskSpec.groovy b/...y/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTaskSpec.groovy
@@ -456,6 +456,9 @@ class WaitForUpInstancesTaskSpec extends Specification {
     false        || 1               | ['b']               | [[health: [[type: 'a', state: "Up"]]]]
     true         || 1               | ['Amazon']          | [[health: [[type: 'Amazon', healthClass: 'platform', state: "Unknown"]]]]
     false        || 1               | ['Amazon']          | [[health: [[type: 'Amazon', state: "Down"]]]]
+    false        || 1               | null                | [[health: [[type: 'Amazon', state: "Up"], [type: 'targetGroup', state: "Starting"]]]]
+    false        || 1               | null                | [[health: [[type: 'Amazon', state: "Unknown"], [type: 'targetGroup', state: "Starting"], [type: 'd', state: "Up"]]]]
+    true         || 1               | ['Amazon']          | [[health: [[type: 'Amazon', state: "Up"], [type: 'targetGroup', state: "Starting"]]]]
     true         || 1               | ['GCE']             | [[health: [[type: 'GCE', healthClass: 'platform', state: "Unknown"]]]]
     false        || 1               | ['GCE']             | [[health: [[type: 'GCE', state: "Down"]]]]