fix(destroy): add disable stage to destroy server group stage (spinna…

…ker#3848)

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.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

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)

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) {