From 98b060ce674fe1c2d30d5422c96a8501920d8ad9 Mon Sep 17 00:00:00 2001
From: Daniel Reynaud <dreynaud@netflix.com>
Date: Thu, 6 Aug 2020 15:20:33 -0700
Subject: [PATCH] fix(destroy): add disable stage to destroy server group stage

Let's add it as a beforeStage instead of duplicating the tasks in the disable stage.

In particular, the task graph of DestroyServerGroupStage was missing a determineHealthProviderTask
and using the now-deprecated WaitForAllInstancesNotUpTask.
---
 .../DestroyServerGroupStage.groovy            | 31 +++++++++++++------
 .../WaitForAllInstancesNotUpTask.groovy       |  4 +++
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DestroyServerGroupStage.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DestroyServerGroupStage.groovy
index 94f0838880..84fb65c37f 100644
--- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DestroyServerGroupStage.groovy
+++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/servergroup/DestroyServerGroupStage.groovy
@@ -17,12 +17,14 @@
 package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup
 
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
+import com.netflix.spinnaker.orca.api.pipeline.graph.TaskNode
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.clouddriver.ForceCacheRefreshAware
 import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroupLinearStageSupport
 import com.netflix.spinnaker.orca.clouddriver.tasks.MonitorKatoTask
-import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.*
-import com.netflix.spinnaker.orca.api.pipeline.graph.TaskNode
+import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.DestroyServerGroupTask
+import com.netflix.spinnaker.orca.clouddriver.tasks.servergroup.WaitForDestroyedServerGroupTask
+import com.netflix.spinnaker.orca.pipeline.graph.StageGraphBuilderImpl
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 
@@ -37,17 +39,26 @@ class DestroyServerGroupStage extends TargetServerGroupLinearStageSupport implem
     this.dynamicConfigService = dynamicConfigService
   }
 
+  private static void addDisableStage(Map<String, Object> context, StageGraphBuilderImpl graph) {
+    graph.add {
+      it.name = "disableServerGroup"
+      it.type = getType(DisableServerGroupStage)
+      it.context.putAll(context)
+    }
+  }
+
   @Override
-  protected void taskGraphInternal(StageExecution stage, TaskNode.Builder builder) {
-    builder
-      .withTask("disableServerGroup", DisableServerGroupTask)
-      .withTask("monitorServerGroup", MonitorKatoTask)
-      .withTask("waitForNotUpInstances", WaitForAllInstancesNotUpTask)
+  protected void preStatic(Map<String, Object> context, StageGraphBuilderImpl graph) {
+    addDisableStage(context, graph)
+  }
 
-    if (isForceCacheRefreshEnabled(dynamicConfigService)) {
-      builder.withTask("forceCacheRefresh", ServerGroupCacheForceRefreshTask)
-    }
+  @Override
+  protected void preDynamic(Map<String, Object> context, StageGraphBuilderImpl graph) {
+    addDisableStage(context, graph)
+  }
 
+  @Override
+  protected void taskGraphInternal(StageExecution stage, TaskNode.Builder builder) {
     builder
       .withTask("destroyServerGroup", DestroyServerGroupTask)
       .withTask("monitorServerGroup", MonitorKatoTask)
diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForAllInstancesNotUpTask.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForAllInstancesNotUpTask.groovy
index 1ef01daa95..b429544744 100644
--- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForAllInstancesNotUpTask.groovy
+++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForAllInstancesNotUpTask.groovy
@@ -25,6 +25,10 @@ import org.springframework.stereotype.Component
 
 @Slf4j
 @Component
+@Deprecated
+/**
+ * @deprecated this does not handle some corner cases (like the platformHealthOnly flag), use {@link WaitForRequiredInstancesDownTask} instead
+ */
 class WaitForAllInstancesNotUpTask extends AbstractWaitingForInstancesTask {
   @Override
   protected boolean hasSucceeded(StageExecution stage, Map serverGroup, List<Map> instances, Collection<String> interestingHealthProviderNames) {