Merge branch 'master' into log-execution-size

gradle.properties

@@ -1,5 +1,5 @@
-fiatVersion=1.43.0
-korkVersion=7.218.0
+fiatVersion=1.44.0
+korkVersion=7.221.0
 kotlinVersion=1.5.32
 org.gradle.parallel=true
 org.gradle.jvmargs=-Xmx6g

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(

orca-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();

orca-flex/orca-flex.gradle

@@ -24,4 +24,5 @@ dependencies {
   implementation("com.netflix.frigga:frigga")
   implementation("io.spinnaker.kork:kork-core")
   implementation("io.spinnaker.kork:kork-moniker")
+  implementation("io.spinnaker.kork:kork-retrofit")
 }

orca-flex/src/main/groovy/com/netflix/spinnaker/orca/flex/config/FlexConfiguration.groovy

@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.orca.flex.config
 
 import com.fasterxml.jackson.databind.ObjectMapper
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.orca.flex.FlexService
 import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration
 import com.netflix.spinnaker.orca.retrofit.logging.RetrofitSlf4jLog
@@ -59,6 +60,7 @@ class FlexConfiguration {
       .setEndpoint(flexEndpoint)
       .setClient(retrofitClient)
       .setLogLevel(retrofitLogLevel)
+      .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
       .setLog(new RetrofitSlf4jLog(FlexService))
       .setConverter(new JacksonConverter(mapper))
       .build()

orca-integrations-gremlin/orca-integrations-gremlin.gradle

@@ -6,6 +6,7 @@ dependencies {
   implementation(project(":orca-retrofit"))
   implementation("org.springframework.boot:spring-boot-autoconfigure")
   implementation("org.slf4j:slf4j-api")
+  implementation("io.spinnaker.kork:kork-retrofit")
 
   testImplementation(project(":orca-core-tck"))
 }

orca-integrations-gremlin/src/main/java/com/netflix/spinnaker/orca/config/GremlinConfiguration.kt

@@ -2,6 +2,7 @@ package com.netflix.spinnaker.orca.config
 
 import com.fasterxml.jackson.databind.PropertyNamingStrategy
 import com.fasterxml.jackson.databind.SerializationFeature
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.orca.gremlin.GremlinConverter
 import com.netflix.spinnaker.orca.gremlin.GremlinService
 import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
@@ -47,6 +48,7 @@ class GremlinConfiguration {
       .setEndpoint(gremlinEndpoint)
       .setClient(retrofitClient)
       .setLogLevel(RestAdapter.LogLevel.BASIC)
+      .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
       .setLog(RetrofitSlf4jLog(GremlinService::class.java))
       .setConverter(GremlinConverter(mapper))
       .build()

orca-kayenta/orca-kayenta.gradle

@@ -27,6 +27,7 @@ dependencies {
   implementation("com.netflix.frigga:frigga")
   implementation("io.spinnaker.kork:kork-moniker")
   implementation("com.fasterxml.jackson.module:jackson-module-kotlin")
+  implementation("io.spinnaker.kork:kork-retrofit")
 
   testImplementation(project(":orca-test-kotlin"))
   testImplementation("com.github.tomakehurst:wiremock-jre8-standalone")

orca-kayenta/src/main/kotlin/com/netflix/spinnaker/orca/kayenta/config/KayentaConfiguration.kt

@@ -18,6 +18,7 @@ package com.netflix.spinnaker.orca.kayenta.config
 
 import com.fasterxml.jackson.databind.SerializationFeature.WRITE_DATES_AS_TIMESTAMPS
 import com.netflix.spinnaker.kork.api.expressions.ExpressionFunctionProvider
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
 import com.netflix.spinnaker.orca.kayenta.KayentaService
 import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration
@@ -77,6 +78,7 @@ class KayentaConfiguration {
       .setLogLevel(retrofitLogLevel)
       .setLog(RetrofitSlf4jLog(KayentaService::class.java))
       .setConverter(JacksonConverter(mapper))
+      .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
       .build()
       .create(KayentaService::class.java)
   }

orca-retrofit/orca-retrofit.gradle

@@ -37,6 +37,8 @@ dependencies {
   testImplementation("org.junit.jupiter:junit-jupiter-params")
   testImplementation("org.mockito:mockito-core")
   testImplementation("org.springframework.boot:spring-boot-test")
+  testImplementation("com.squareup.okhttp3:mockwebserver")
+  testImplementation("com.squareup.retrofit2:converter-jackson")
   testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine")
   testRuntimeOnly("org.springframework:spring-test")
 

orca-retrofit/src/main/groovy/com/netflix/spinnaker/orca/retrofit/exceptions/BaseRetrofitExceptionHandler.groovy

@@ -24,6 +24,10 @@ import static java.net.HttpURLConnection.*
 
 abstract class BaseRetrofitExceptionHandler implements ExceptionHandler {
   boolean shouldRetry(Exception e, String kind, Integer responseCode) {
+    return shouldRetry(e, kind, null, responseCode)
+  }
+
+  boolean shouldRetry(Exception e, String kind, String httpMethod, Integer responseCode) {
     if (isMalformedRequest(kind, e.getMessage())) {
       return false
     }
@@ -33,7 +37,11 @@ abstract class BaseRetrofitExceptionHandler implements ExceptionHandler {
       return true
     }
 
-    return isIdempotentRequest(e) && (isNetworkError(kind) || isGatewayErrorCode(kind, responseCode) || isThrottle(kind, responseCode))
+    if(httpMethod == null) {
+      httpMethod = findHttpMethodAnnotation(e)
+    }
+
+    return isIdempotentRequest(httpMethod) && (isNetworkError(kind) || isGatewayErrorCode(kind, responseCode) || isThrottle(kind, responseCode))
   }
 
   private boolean isGatewayErrorCode(String kind, Integer responseCode) {
@@ -55,8 +63,8 @@ abstract class BaseRetrofitExceptionHandler implements ExceptionHandler {
     return "UNEXPECTED".equals(kind) && exceptionMessage?.contains("Path parameter")
   }
 
-  private static boolean isIdempotentRequest(Exception e) {
-    findHttpMethodAnnotation(e) in ["GET", "HEAD", "DELETE", "PUT"]
+  private static boolean isIdempotentRequest(String httpMethod) {
+    httpMethod in ["GET", "HEAD", "DELETE", "PUT"]
   }
 
   private static String findHttpMethodAnnotation(Exception exception) {

orca-retrofit/src/main/java/com/netflix/spinnaker/orca/retrofit/exceptions/SpinnakerServerExceptionHandler.java

@@ -43,6 +43,7 @@ public Response handle(String taskName, Exception exception) {
 
     String kind;
     Integer responseCode = null;
+    String httpMethod = ex.getHttpMethod();
 
     if (ex instanceof SpinnakerNetworkException) {
       kind = "NETWORK";
@@ -108,6 +109,12 @@ public Response handle(String taskName, Exception exception) {
 
     responseDetails.put("kind", kind);
 
+    // http method may be null if exception is created from RetrofitError
+    // so only include in responseDetails when value is valid
+    if (httpMethod != null) {
+      responseDetails.put("method", httpMethod);
+    }
+
     // Although Spinnaker*Exception has a retryable property that other parts of
     // spinnaker use, ignore it here for compatibility with
     // RetrofitExceptionHandler, specifically because that doesn't retry (most)
@@ -116,6 +123,6 @@ public Response handle(String taskName, Exception exception) {
         ex.getClass().getSimpleName(),
         taskName,
         responseDetails,
-        shouldRetry(ex, kind, responseCode));
+        shouldRetry(ex, kind, httpMethod, responseCode));
   }
 }