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

…on correctly after upstream stages get restarted (#4682) (#4720) Co-authored-by: ysaxena <[email protected]> (cherry picked from commit 613427f) Co-authored-by: Yash Saxena <[email protected]>
spinnaker · May 7, 2024 · da0d5ca · da0d5ca
1 parent baedae5
commit da0d5ca
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
@@ -44,6 +44,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);
@@ -79,6 +82,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() {
 
@@ -138,6 +181,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();