fix(check-pre-condition): CheckPrecondition doesn't evaluate expressi…

…on correctly after upstream stages get restarted (#4682) (#4718) Co-authored-by: ysaxena <[email protected]> (cherry picked from commit 613427f) Co-authored-by: Yash Saxena <[email protected]>
spinnaker · May 7, 2024 · b426314 · b426314
1 parent 7799f04
commit b426314
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 7 deletions.
diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperator.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperator.java
@@ -141,15 +141,15 @@ public PipelineExecution restartStage(
 
   private PipelineExecution updatePreconditionStageExpression(
       Map restartDetails, PipelineExecution execution) {
-    List<Map> preconditionList = getPreconditionsFromStage(restartDetails);
-    if (preconditionList.isEmpty()) {
+    Map<String, List<Map>> preconditionMap = getPreconditionsFromStage(restartDetails);
+    if (preconditionMap.isEmpty()) {
       return execution;
     }
 
     for (StageExecution stage : execution.getStages()) {
       if (stage.getType() != null && stage.getType().equalsIgnoreCase("checkPreconditions")) {
         if (stage.getContext().get("preconditions") != null) {
-          stage.getContext().replace("preconditions", preconditionList);
+          stage.getContext().replace("preconditions", preconditionMap.get(stage.getRefId()));
           repository.storeStage(stage);
           log.info("Updated preconditions for CheckPreconditions stage");
         }
@@ -158,8 +158,8 @@ private PipelineExecution updatePreconditionStageExpression(
     return execution;
   }
 
-  private List<Map> getPreconditionsFromStage(Map restartDetails) {
-    List<Map> preconditionList = new ArrayList();
+  private Map<String, List<Map>> getPreconditionsFromStage(Map restartDetails) {
+    Map<String, List<Map>> preconditionMap = new HashMap<>();
     Map pipelineConfigMap = new HashMap(restartDetails);
 
     List<String> keysToRetain = new ArrayList();
@@ -173,12 +173,13 @@ private List<Map> getPreconditionsFromStage(Map restartDetails) {
       List<Map> pipelineStageList = pipelineStageMap.get(keysToRetain.get(0));
       for (Map stageMap : pipelineStageList) {
         if (stageMap.get("type").toString().equalsIgnoreCase("checkPreconditions")) {
-          preconditionList = (List<Map>) stageMap.get("preconditions");
+          preconditionMap.put(
+              (String) stageMap.get("refId"), (List<Map>) stageMap.get("preconditions"));
           log.info("Retrieved preconditions for CheckPreconditions stage");
         }
       }
     }
-    return preconditionList;
+    return preconditionMap;
   }
 
   private PipelineExecution doInternal(

diff --git a/...core/src/test/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperatorTest.java b/...core/src/test/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperatorTest.java
@@ -41,6 +41,9 @@ final class CompoundExecutionOperatorTest {
   private static final String PIPELINE = "mypipeline";
   private static final String EXECUTION_ID = "EXECUTION_ID";
   private static final String STAGE_ID = "STAGE_ID";
+  private static final String UPSTREAM_STAGE = "UPSTREAM_STAGE";
+  private static final String UPSTREAM_STAGE_ID = "UPSTREAM_STAGE_ID";
+  private static final int PRECONDITION_STAGE_LIST_SIZE = 2;
   private final ExecutionRepository repository = mock(ExecutionRepository.class);
   private final ExecutionRunner runner = mock(ExecutionRunner.class);
   private final RetrySupport retrySupport = mock(RetrySupport.class);
@@ -76,6 +79,46 @@ void restartStageWithValidExpression() {
     assertEquals(APPLICATION, updatedExecution.getApplication());
   }
 
+  @Test
+  void restartUpstreamStageWithMultiplePreconditionStages() {
+    // ARRANGE
+    StageExecution upstreamStage = buildUpstreamStage();
+    List<StageExecution> preconditionStages = new ArrayList<>(PRECONDITION_STAGE_LIST_SIZE);
+    List<StageExecution> executionStages = execution.getStages();
+    executionStages.add(upstreamStage);
+
+    for (int i = 0; i < PRECONDITION_STAGE_LIST_SIZE; i++) {
+      StageExecution preconditionStage =
+          buildPreconditionStage("expression_" + i, "precondition_" + i, "precondition_stage_" + i);
+      preconditionStages.add(preconditionStage);
+      executionStages.add(preconditionStage);
+    }
+
+    Map<String, Object> restartDetails =
+        Map.of(
+            "application", APPLICATION,
+            "name", PIPELINE,
+            "executionId", EXECUTION_ID,
+            "stages", buildExpectedRestartStageList(preconditionStages, upstreamStage));
+
+    when(repository.retrieve(any(), anyString())).thenReturn(execution);
+    when(repository.handlesPartition(execution.getPartition())).thenReturn(true);
+
+    // ACT
+    PipelineExecution updatedExecution =
+        executionOperator.restartStage(EXECUTION_ID, STAGE_ID, restartDetails);
+
+    // ASSERT
+    assertEquals(APPLICATION, updatedExecution.getApplication());
+    for (int i = 0, size = preconditionStages.size(); i < size; i++) {
+      StageExecution originalPreconditionStage = preconditionStages.get(i);
+      StageExecution updatedPreconditionStage = updatedExecution.getStages().get(i + 1);
+      assertEquals(originalPreconditionStage.getContext(), updatedPreconditionStage.getContext());
+      assertEquals(originalPreconditionStage.getType(), updatedPreconditionStage.getType());
+      assertEquals(originalPreconditionStage.getName(), updatedPreconditionStage.getName());
+    }
+  }
+
   @Test
   void restartStageWithNoPreconditions() {
 
@@ -135,6 +178,55 @@ private PipelineExecution buildExpectedExecution(
     return execution;
   }
 
+  private List<Map<String, Object>> buildExpectedRestartStageList(
+      List<StageExecution> preconditionStages, StageExecution upstreamStage) {
+    List<Map<String, Object>> restartStageList = new ArrayList<>(preconditionStages.size() + 1);
+
+    Map<String, Object> upstreamStageMap =
+        Map.of(
+            "name", upstreamStage.getName(),
+            "type", upstreamStage.getType());
+    restartStageList.add(upstreamStageMap);
+
+    for (StageExecution stage : preconditionStages) {
+      Map<String, Object> stageMap =
+          Map.of(
+              "name", stage.getName(),
+              "type", stage.getType(),
+              "preconditions", stage.getContext().get("preconditions"));
+      restartStageList.add(stageMap);
+    }
+    return restartStageList;
+  }
+
+  private StageExecution buildPreconditionStage(
+      String stageStatus, String stageId, String preConditionStageName) {
+    StageExecution preconditionStage = new StageExecutionImpl();
+    preconditionStage.setId(stageId);
+    preconditionStage.setType("checkPreconditions");
+    preconditionStage.setName(preConditionStageName);
+    Map<String, Object> contextMap = new HashMap<>();
+    contextMap.put("stageName", UPSTREAM_STAGE);
+    contextMap.put("stageStatus", stageStatus);
+    List<Map<String, Object>> preconditionList = new ArrayList<>();
+    Map<String, Object> preconditionMap = new HashMap<>();
+    preconditionMap.put("context", contextMap);
+    preconditionMap.put("failPipeline", true);
+    preconditionMap.put("type", "stageStatus");
+    preconditionList.add(preconditionMap);
+    contextMap.put("preconditions", preconditionList);
+    preconditionStage.setContext(contextMap);
+    return preconditionStage;
+  }
+
+  private StageExecution buildUpstreamStage() {
+    StageExecution upstreamStage = new StageExecutionImpl();
+    upstreamStage.setId(UPSTREAM_STAGE_ID);
+    upstreamStage.setType("manualJudgment");
+    upstreamStage.setName(UPSTREAM_STAGE);
+    return upstreamStage;
+  }
+
   private Map buildExpectedRestartDetailsMap(String expression) {
     Map restartDetails = new HashMap<>();
     List<Map> pipelineStageList = new ArrayList();