From da0d5caa7bc77b6e1286c74e1e9de4e5eb93f30f Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 7 May 2024 00:09:56 +0000 Subject: [PATCH] fix(check-pre-condition): CheckPrecondition doesn't evaluate expression correctly after upstream stages get restarted (#4682) (#4720) Co-authored-by: ysaxena (cherry picked from commit 613427f41bb16461a7ee8bb0f31212b438d75458) Co-authored-by: Yash Saxena <41922677+yashsaxena1503@users.noreply.github.com> --- .../pipeline/CompoundExecutionOperator.java | 15 +-- .../CompoundExecutionOperatorTest.java | 92 +++++++++++++++++++ 2 files changed, 100 insertions(+), 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 index dce150404d..2e0b45a543 100644 --- 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 preconditionList = getPreconditionsFromStage(restartDetails); - if (preconditionList.isEmpty()) { + 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 getPreconditionsFromStage(Map restartDetails) { - List preconditionList = new ArrayList(); + private Map> getPreconditionsFromStage(Map restartDetails) { + Map> preconditionMap = new HashMap<>(); Map pipelineConfigMap = new HashMap(restartDetails); List keysToRetain = new ArrayList(); @@ -173,12 +173,13 @@ private List getPreconditionsFromStage(Map restartDetails) { List pipelineStageList = pipelineStageMap.get(keysToRetain.get(0)); for (Map stageMap : pipelineStageList) { if (stageMap.get("type").toString().equalsIgnoreCase("checkPreconditions")) { - preconditionList = (List) stageMap.get("preconditions"); + preconditionMap.put( + (String) stageMap.get("refId"), (List) stageMap.get("preconditions")); log.info("Retrieved preconditions for CheckPreconditions stage"); } } } - return preconditionList; + return preconditionMap; } private PipelineExecution doInternal( diff --git a/orca-core/src/test/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperatorTest.java b/orca-core/src/test/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperatorTest.java index 46322a41bf..347fe9f4fb 100644 --- a/orca-core/src/test/java/com/netflix/spinnaker/orca/pipeline/CompoundExecutionOperatorTest.java +++ b/orca-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 preconditionStages = new ArrayList<>(PRECONDITION_STAGE_LIST_SIZE); + List 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 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> buildExpectedRestartStageList( + List preconditionStages, StageExecution upstreamStage) { + List> restartStageList = new ArrayList<>(preconditionStages.size() + 1); + + Map upstreamStageMap = + Map.of( + "name", upstreamStage.getName(), + "type", upstreamStage.getType()); + restartStageList.add(upstreamStageMap); + + for (StageExecution stage : preconditionStages) { + Map 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 contextMap = new HashMap<>(); + contextMap.put("stageName", UPSTREAM_STAGE); + contextMap.put("stageStatus", stageStatus); + List> preconditionList = new ArrayList<>(); + Map 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 pipelineStageList = new ArrayList();