From c14a58e202b7b0c43cd30a7fbe86b2afeeffa937 Mon Sep 17 00:00:00 2001 From: Asher Feldman Date: Thu, 23 May 2019 13:10:59 -0700 Subject: [PATCH] fix(clouddriver): WaitForUpInstances should block on Starting health providers (#2935) --- .../tasks/instance/WaitForUpInstanceHealthTask.groovy | 2 +- .../tasks/instance/WaitForUpInstancesTask.groovy | 4 ++-- .../spinnaker/orca/clouddriver/utils/HealthHelper.groovy | 9 ++++++--- .../rollingpush/WaitForNewUpInstancesLaunchTask.groovy | 3 +-- .../tasks/instance/WaitForUpInstancesTaskSpec.groovy | 3 +++ 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstanceHealthTask.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstanceHealthTask.groovy index eaea826774..fcc4aa9a95 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstanceHealthTask.groovy +++ b/orca-clouddriver/src/main/groovy/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 interestedHealthProviderNames) { - HealthHelper.someAreUpAndNoneAreDown(instance, interestedHealthProviderNames) + HealthHelper.someAreUpAndNoneAreDownOrStarting(instance, interestedHealthProviderNames) } } diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTask.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTask.groovy index b367471dc5..c5f0bf41cf 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTask.groovy +++ b/orca-clouddriver/src/main/groovy/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 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/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/HealthHelper.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/HealthHelper.groovy index 3912a33611..9b0f170b81 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/HealthHelper.groovy +++ b/orca-clouddriver/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 interestingHealthProviderNames) { + static boolean someAreUpAndNoneAreDownOrStarting(Map instance, Collection interestingHealthProviderNames) { List 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/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/rollingpush/WaitForNewUpInstancesLaunchTask.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/rollingpush/WaitForNewUpInstancesLaunchTask.groovy index 80cd017e3f..7d6d818b17 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/rollingpush/WaitForNewUpInstancesLaunchTask.groovy +++ b/orca-clouddriver/src/main/groovy/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 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/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTaskSpec.groovy b/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTaskSpec.groovy index 6c112b855f..5948f9489d 100644 --- a/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/instance/WaitForUpInstancesTaskSpec.groovy +++ b/orca-clouddriver/src/test/groovy/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"]]]]