spinnaker · jeyrschabu · Apr 23, 2019 · Apr 22, 2019
diff --git a/...om/netflix/spinnaker/orca/clouddriver/pipeline/ConditionAwareDeployStagePreprocessor.java b/...om/netflix/spinnaker/orca/clouddriver/pipeline/ConditionAwareDeployStagePreprocessor.java
@@ -16,29 +16,37 @@
 
 package com.netflix.spinnaker.orca.clouddriver.pipeline;
 
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.Condition;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.ConditionSupplier;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.WaitForConditionStage;
 import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies.DeployStagePreProcessor;
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData;
-import com.netflix.spinnaker.orca.pipeline.WaitForConditionStage;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.stereotype.Component;
 
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
+import java.util.stream.Collectors;
 
 @Component
+@ConditionalOnBean(ConditionSupplier.class)
 @ConditionalOnExpression("${tasks.evaluateCondition.enabled:false}")
 public class ConditionAwareDeployStagePreprocessor implements DeployStagePreProcessor {
+  private final Logger log = LoggerFactory.getLogger(ConditionAwareDeployStagePreprocessor.class);
   private final WaitForConditionStage waitForConditionStage;
+  private final List<ConditionSupplier> conditionSuppliers;
 
   @Autowired
   public ConditionAwareDeployStagePreprocessor(
-    WaitForConditionStage waitForConditionStage
+    WaitForConditionStage waitForConditionStage,
+    List<ConditionSupplier> conditionSuppliers
   ) {
     this.waitForConditionStage = waitForConditionStage;
+    this.conditionSuppliers = conditionSuppliers;
   }
 
   @Override
@@ -48,15 +56,35 @@ public boolean supports(Stage stage) {
 
   @Override
   public List<StageDefinition> beforeStageDefinitions(Stage stage) {
-    final StageData stageData = stage.mapTo(StageData.class);
-    Map<String, Object> ctx = new HashMap<>();
-    ctx.put("region", stageData.getRegion());
-    ctx.put("cluster", stageData.getCluster());
-
-    StageDefinition stageDefinition = new StageDefinition();
-    stageDefinition.name = "Wait For Condition";
-    stageDefinition.context = ctx;
-    stageDefinition.stageDefinitionBuilder = waitForConditionStage;
-    return Collections.singletonList(stageDefinition);
+    try {
+      final StageData stageData = stage.mapTo(StageData.class);
+      Set<Condition> conditions = conditionSuppliers
+        .stream()
+        .flatMap(supplier -> supplier.getConditions(
+          stageData.getCluster(),
+          stageData.getRegion(),
+          stageData.getAccount()
+        ).stream()).filter(Objects::nonNull)
+        .collect(Collectors.toSet());
+      if (conditions.isEmpty()) {
+        // do no inject the stage if there are no active conditions
+        return Collections.emptyList();
+      }
+
+      Map<String, Object> ctx = new HashMap<>();
+      // defines what is required by condition suppliers
+      ctx.put("region", stageData.getRegion());
+      ctx.put("cluster", stageData.getCluster());
+      ctx.put("account", stageData.getAccount());
+      StageDefinition stageDefinition = new StageDefinition();
+      stageDefinition.name = "Wait For Condition";
+      stageDefinition.context = ctx;
+      stageDefinition.stageDefinitionBuilder = waitForConditionStage;
+      return Collections.singletonList(stageDefinition);
+    } catch (Exception e) {
+      log.error("Error determining active conditions. Proceeding with execution {}", stage.getExecution().getId());
+    }
+
+    return Collections.emptyList();
   }
 }
diff --git a/.../spinnaker/orca/conditions/Condition.java → ...driver/pipeline/conditions/Condition.java b/.../spinnaker/orca/conditions/Condition.java → ...driver/pipeline/conditions/Condition.java
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.conditions;
+package com.netflix.spinnaker.orca.clouddriver.pipeline.conditions;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;

diff --git a/...ons/ConditionConfigurationProperties.java → ...ons/ConditionConfigurationProperties.java b/...ons/ConditionConfigurationProperties.java → ...ons/ConditionConfigurationProperties.java
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.conditions;
+package com.netflix.spinnaker.orca.clouddriver.pipeline.conditions;
 
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService;
 import org.springframework.beans.factory.annotation.Autowired;

diff --git a/...er/orca/conditions/ConditionSupplier.java → ...ipeline/conditions/ConditionSupplier.java b/...er/orca/conditions/ConditionSupplier.java → ...ipeline/conditions/ConditionSupplier.java
@@ -14,9 +14,8 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.conditions;
+package com.netflix.spinnaker.orca.clouddriver.pipeline.conditions;
 
-import com.netflix.spinnaker.orca.pipeline.model.Stage;
 import java.util.List;
 
 /**
@@ -26,5 +25,5 @@ public interface ConditionSupplier {
   /**
    * returns a list of currently unmet conditions.
    */
-  List<Condition> getConditions(Stage stage);
+  List<Condition> getConditions(String cluster, String region, String account);
 }
diff --git a/...ConfigurationBackedConditionSupplier.java → ...ConfigurationBackedConditionSupplier.java b/...ConfigurationBackedConditionSupplier.java → ...ConfigurationBackedConditionSupplier.java
@@ -14,10 +14,8 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.conditions;
+package com.netflix.spinnaker.orca.clouddriver.pipeline.conditions;
 
-import com.netflix.spinnaker.orca.pipeline.WaitForConditionStage.WaitForConditionContext;
-import com.netflix.spinnaker.orca.pipeline.model.Stage;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.stereotype.Component;
@@ -41,16 +39,15 @@ public ConfigurationBackedConditionSupplier(ConditionConfigurationProperties con
   }
 
   @Override
-  public List<Condition> getConditions(Stage stage) {
-    final WaitForConditionContext ctx = stage.mapTo(WaitForConditionContext.class);
+  public List<Condition> getConditions(String cluster, String region, String account) {
     final List<String> clusters = conditionsConfigurationProperties.getClusters();
     final List<String> activeConditions = conditionsConfigurationProperties.getActiveConditions();
 
     if (clusters == null || clusters.isEmpty() || activeConditions == null || activeConditions.isEmpty()) {
       return Collections.emptyList();
     }
 
-    if (!clusters.contains(ctx.getCluster())) {
+    if (!clusters.contains(cluster)) {
       return Collections.emptyList();
     }
 

diff --git a/.../orca/pipeline/WaitForConditionStage.java → ...ine/conditions/WaitForConditionStage.java b/.../orca/pipeline/WaitForConditionStage.java → ...ine/conditions/WaitForConditionStage.java
@@ -14,12 +14,14 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.pipeline;
+package com.netflix.spinnaker.orca.clouddriver.pipeline.conditions;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.netflix.spinnaker.orca.clouddriver.tasks.conditions.EvaluateConditionTask;
+import com.netflix.spinnaker.orca.pipeline.StageDefinitionBuilder;
+import com.netflix.spinnaker.orca.pipeline.TaskNode;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import com.netflix.spinnaker.orca.pipeline.tasks.EvaluateConditionTask;
 import org.jetbrains.annotations.NotNull;
 import org.springframework.stereotype.Component;
 
@@ -38,16 +40,19 @@ public static final class WaitForConditionContext {
     private Status status;
     private String region;
     private String cluster;
+    private String account;
 
     @JsonCreator
     public WaitForConditionContext(
       @JsonProperty("status") Status status,
       @JsonProperty("region") @Nullable String region,
-      @JsonProperty("cluster") @Nullable String cluster
+      @JsonProperty("cluster") @Nullable String cluster,
+      @JsonProperty("account") @Nullable String account
     ) {
       this.status = status;
       this.region = region;
       this.cluster = cluster;
+      this.account = account;
     }
 
     public enum Status {
@@ -77,5 +82,13 @@ public String getCluster() {
     public void setCluster(String cluster) {
       this.cluster = cluster;
     }
+
+    public String getAccount() {
+      return account;
+    }
+
+    public void setAccount(String account) {
+      this.account = account;
+    }
   }
 }
diff --git a/...pipeline/tasks/EvaluateConditionTask.java → ...sks/conditions/EvaluateConditionTask.java b/...pipeline/tasks/EvaluateConditionTask.java → ...sks/conditions/EvaluateConditionTask.java
@@ -14,19 +14,21 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.pipeline.tasks;
+package com.netflix.spinnaker.orca.clouddriver.tasks.conditions;
 
 import com.netflix.spinnaker.orca.ExecutionStatus;
 import com.netflix.spinnaker.orca.RetryableTask;
 import com.netflix.spinnaker.orca.TaskResult;
-import com.netflix.spinnaker.orca.conditions.ConditionConfigurationProperties;
-import com.netflix.spinnaker.orca.conditions.Condition;
-import com.netflix.spinnaker.orca.conditions.ConditionSupplier;
-import com.netflix.spinnaker.orca.pipeline.WaitForConditionStage.WaitForConditionContext;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.Condition;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.ConditionConfigurationProperties;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.ConditionSupplier;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.WaitForConditionStage.WaitForConditionContext;
+import com.netflix.spinnaker.orca.clouddriver.pipeline.conditions.WaitForConditionStage.WaitForConditionContext.Status;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.stereotype.Component;
 
@@ -38,20 +40,20 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
-import static com.netflix.spinnaker.orca.pipeline.WaitForConditionStage.WaitForConditionContext.*;
 
 @Component
+@ConditionalOnBean(ConditionSupplier.class)
 @ConditionalOnExpression("${tasks.evaluateCondition.enabled:false}")
 public class EvaluateConditionTask implements RetryableTask {
   private static final Logger log = LoggerFactory.getLogger(EvaluateConditionTask.class);
   private final ConditionConfigurationProperties conditionsConfigurationProperties;
-  private final Optional<List<ConditionSupplier>> suppliers;
+  private final List<ConditionSupplier> suppliers;
   private final Clock clock;
 
   @Autowired
   public EvaluateConditionTask(
     ConditionConfigurationProperties conditionsConfigurationProperties,
-    Optional<List<ConditionSupplier>> suppliers,
+    List<ConditionSupplier> suppliers,
     Clock clock
   ) {
     this.conditionsConfigurationProperties = conditionsConfigurationProperties;
@@ -73,7 +75,7 @@ public long getTimeout() {
   @Override
   public TaskResult execute(@Nonnull Stage stage) {
     final WaitForConditionContext ctx = stage.mapTo(WaitForConditionContext.class);
-    if (ctx.getStatus() == Status.SKIPPED || !suppliers.isPresent()) {
+    if (ctx.getStatus() == Status.SKIPPED) {
       return new TaskResult(ExecutionStatus.SUCCEEDED, Collections.singletonMap("status", Status.SKIPPED));
     }
 
@@ -88,9 +90,13 @@ public TaskResult execute(@Nonnull Stage stage) {
     }
 
     try {
-      Set<Condition> conditions = suppliers.get()
+      Set<Condition> conditions = suppliers
         .stream()
-        .flatMap(supplier -> supplier.getConditions(stage).stream()).filter(Objects::nonNull)
+        .flatMap(supplier -> supplier.getConditions(
+          ctx.getCluster(),
+          ctx.getRegion(),
+          ctx.getAccount()
+        ).stream()).filter(Objects::nonNull)
         .collect(Collectors.toSet());
 
       log.info("Found conditions: {}", conditions);

diff --git a/...urationBackedConditionSupplierSpec.groovy → ...urationBackedConditionSupplierSpec.groovy b/...urationBackedConditionSupplierSpec.groovy → ...urationBackedConditionSupplierSpec.groovy
@@ -14,17 +14,14 @@
  * limitations under the License.
  */
 
-package com.netflix.spinnaker.orca.conditions
+package com.netflix.spinnaker.orca.clouddriver.pipeline.conditions
 
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
-import com.netflix.spinnaker.orca.pipeline.WaitForConditionStage
 import com.netflix.spinnaker.orca.time.MutableClock
 import spock.lang.Specification
 import spock.lang.Subject
 import spock.lang.Unroll
 
-import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage
-
 class ConfigurationBackedConditionSupplierSpec extends Specification {
   def configService = Stub(DynamicConfigService) {
     getConfig(_ as Class, _ as String, _ as Object) >> { type, name, defaultValue -> return defaultValue }
@@ -40,29 +37,21 @@ class ConfigurationBackedConditionSupplierSpec extends Specification {
   @Unroll
   def "should return configured conditions"() {
     given:
-    def stage = stage {
-      refId = "1"
-      type = WaitForConditionStage.STAGE_TYPE
-      startTime = clock.millis()
-      context = ctx
-    }
-
-    and:
     conditionsConfigurationProperties.setClusters(clusters)
     conditionsConfigurationProperties.setActiveConditions(activeConditions)
 
     when:
-    def result = conditionSupplier.getConditions(stage)
+    def result = conditionSupplier.getConditions(cluster, "region", "account")
 
     then:
     result.size() == numberOfResultingConditions
 
     where:
-    ctx                 | clusters        | activeConditions     | numberOfResultingConditions
-    [cluster: "foo"]    | []              | []                   | 0
-    [cluster: "foo"]    | ["foo", "bar"]  | []                   | 0
-    [cluster: "foo"]    | ["bar"]         | [ "c1", "c2"]        | 0
-    [cluster: "foo"]    | ["foo", "bar"]  | [ "c1", "c2"]        | 2
-    [cluster: "foo"]    | []              | [ "c1", "c2"]        | 0
+    cluster                 | clusters        | activeConditions     | numberOfResultingConditions
+    "foo"                   | []              | []                   | 0
+    "foo"                   | ["foo", "bar"]  | []                   | 0
+    "foo"                   | ["bar"]         | [ "c1", "c2"]        | 0
+    "foo"                   | ["foo", "bar"]  | [ "c1", "c2"]        | 2
+    "foo"                   | []              | [ "c1", "c2"]        | 0
   }
 }