Merge branch 'master' into lazy-loading-1-35

spinnaker · Jan 21, 2025 · e60ae17 · e60ae17
2 parents 26b8c61 + b2399f8
commit e60ae17
Show file tree

Hide file tree

Showing 13 changed files with 773 additions and 81 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1,5 +1,5 @@
 fiatVersion=1.53.0
-korkVersion=7.247.0
+korkVersion=7.248.0
 kotlinVersion=1.6.21
 org.gradle.parallel=true
 org.gradle.jvmargs=-Xmx4g

diff --git a/...ain/java/com/netflix/spinnaker/orca/clouddriver/config/LambdaConfigurationProperties.java b/...ain/java/com/netflix/spinnaker/orca/clouddriver/config/LambdaConfigurationProperties.java
@@ -22,9 +22,8 @@
 @Data
 @ConfigurationProperties(prefix = "lambda")
 public class LambdaConfigurationProperties {
+  private int cloudDriverPostTimeoutSeconds = 120;
 
-  private int cloudDriverReadTimeout = 60;
-  private int cloudDriverConnectTimeout = 15;
   private int cacheRefreshRetryWaitTime = 15;
   private int cacheOnDemandRetryWaitTime = 15;
   private int cloudDriverPostRequestRetries = 5;

diff --git a/...netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTask.java b/...netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTask.java
@@ -45,12 +45,25 @@
 public class LambdaCacheRefreshTask implements LambdaStageBaseTask {
   private static final Logger logger = LoggerFactory.getLogger(LambdaCacheRefreshTask.class);
   private static final String CLOUDDRIVER_REFRESH_CACHE_PATH = "/cache/aws/function";
+  private final OkHttpClient client;
 
-  @Autowired CloudDriverConfigurationProperties props;
+  CloudDriverConfigurationProperties props;
 
-  @Autowired private LambdaCloudDriverUtils utils;
+  private LambdaCloudDriverUtils utils;
 
-  @Autowired LambdaConfigurationProperties config;
+  LambdaConfigurationProperties config;
+
+  @Autowired
+  public LambdaCacheRefreshTask(
+      CloudDriverConfigurationProperties props,
+      LambdaCloudDriverUtils utils,
+      OkHttpClient client,
+      LambdaConfigurationProperties config) {
+    this.props = props;
+    this.utils = utils;
+    this.client = client;
+    this.config = config;
+  }
 
   private static final ObjectMapper objectMapper = new ObjectMapper();
 
@@ -98,11 +111,6 @@ private void forceCacheRefresh(StageExecution stage, int tries) {
         .retry(
             () -> {
               try {
-                OkHttpClient client =
-                    new OkHttpClient.Builder()
-                        .connectTimeout(Duration.ofSeconds(config.getCloudDriverConnectTimeout()))
-                        .readTimeout(Duration.ofSeconds(config.getCloudDriverReadTimeout()))
-                        .build();
                 Call call = client.newCall(request);
                 Response response = call.execute();
                 String respString = response.body().string();

diff --git a/...er/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaCloudDriverUtils.java b/...er/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaCloudDriverUtils.java
@@ -42,8 +42,16 @@
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
-import okhttp3.*;
+import okhttp3.Call;
+import okhttp3.Headers;
+import okhttp3.HttpUrl;
+import okhttp3.MediaType;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+import okhttp3.Response;
 import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.math.NumberUtils;
 import org.pf4j.util.StringUtils;
@@ -63,11 +71,24 @@ public class LambdaCloudDriverUtils {
     objectMapper.configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true);
   }
 
-  @Autowired LambdaConfigurationProperties config;
+  private final LambdaConfigurationProperties config;
 
-  @Autowired CloudDriverConfigurationProperties props;
+  private final CloudDriverConfigurationProperties props;
 
-  @Autowired OortService oort;
+  private final OortService oort;
+  private final OkHttpClient client;
+
+  @Autowired
+  public LambdaCloudDriverUtils(
+      LambdaConfigurationProperties config,
+      CloudDriverConfigurationProperties props,
+      OkHttpClient client,
+      OortService oort) {
+    this.config = config;
+    this.props = props;
+    this.client = client;
+    this.oort = oort;
+  }
 
   public LambdaCloudDriverResponse postToCloudDriver(String endPointUrl, String jsonString) {
     return postToCloudDriver(endPointUrl, jsonString, config.getCloudDriverPostRequestRetries());
@@ -82,8 +103,12 @@ public LambdaCloudDriverResponse postToCloudDriver(
         .retry(
             () -> {
               try {
-                OkHttpClient client = new OkHttpClient();
-                Call call = client.newCall(request);
+                Call call =
+                    client
+                        .newBuilder()
+                        .readTimeout(config.getCloudDriverPostTimeoutSeconds(), TimeUnit.SECONDS)
+                        .build()
+                        .newCall(request);
                 Response response = call.execute();
                 String respString = response.body().string();
 
@@ -184,11 +209,6 @@ public LambdaCloudDriverTaskResults verifyStatus(String endPoint) {
 
   public String getFromCloudDriver(String endPoint) {
     Request request = new Request.Builder().url(endPoint).headers(buildHeaders()).get().build();
-    OkHttpClient client =
-        new OkHttpClient.Builder()
-            .connectTimeout(Duration.ofSeconds(config.getCloudDriverConnectTimeout()))
-            .readTimeout(Duration.ofSeconds(config.getCloudDriverReadTimeout()))
-            .build();
     Call call = client.newCall(request);
     try {
       Response response = call.execute();
@@ -229,7 +249,6 @@ public LambdaDefinition retrieveLambdaFromCache(LambdaGetInput inp) {
     httpBuilder.addQueryParameter("functionName", fName);
     Request request =
         new Request.Builder().url(httpBuilder.build()).headers(buildHeaders()).build();
-    OkHttpClient client = new OkHttpClient();
     Call call = client.newCall(request);
     try {
       Response response = call.execute();

diff --git a/...lix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTaskTest.java b/...lix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTaskTest.java
@@ -34,10 +34,10 @@
 import java.util.List;
 import java.util.Map;
 import okhttp3.Headers;
+import okhttp3.OkHttpClient;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
@@ -49,7 +49,7 @@ public class LambdaCacheRefreshTaskTest {
 
   WireMockServer wireMockServer;
 
-  @InjectMocks private LambdaCacheRefreshTask lambdaCacheRefreshTask;
+  private LambdaCacheRefreshTask lambdaCacheRefreshTask;
 
   @Mock private CloudDriverConfigurationProperties propsMock;
 
@@ -86,6 +86,9 @@ void init(
         new ResponseDefinitionBuilder().withStatus(202).withBody(responseDefinitionBuilderJson);
 
     this.wireMockServer.stubFor(WireMock.post("/cache/aws/function").willReturn(mockResponse));
+    lambdaCacheRefreshTask =
+        new LambdaCacheRefreshTask(
+            propsMock, lambdaCloudDriverUtilsMock, new OkHttpClient(), config);
   }
 
   @Test

diff --git a/...rc/test/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaClouddriverUtilsTest.java b/...rc/test/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaClouddriverUtilsTest.java
@@ -16,12 +16,14 @@
 
 package com.netflix.spinnaker.orca.clouddriver.utils;
 
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
 import static org.junit.jupiter.api.Assertions.*;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.google.common.collect.ImmutableMap;
+import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
 import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution;
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import com.netflix.spinnaker.orca.clouddriver.config.CloudDriverConfigurationProperties;
@@ -31,16 +33,17 @@
 import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.lambda.model.LambdaDefinition;
 import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.lambda.model.input.LambdaDeploymentInput;
 import com.netflix.spinnaker.orca.clouddriver.tasks.providers.aws.lambda.model.input.LambdaGetInput;
+import java.net.SocketTimeoutException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import okhttp3.OkHttpClient;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
@@ -54,20 +57,47 @@ public class LambdaClouddriverUtilsTest {
 
   String CLOUDDRIVER_BASE_URL;
 
-  @InjectMocks private LambdaCloudDriverUtils lambdaCloudDriverUtils;
+  private LambdaCloudDriverUtils lambdaCloudDriverUtils;
 
   @Mock private CloudDriverConfigurationProperties propsMock;
 
-  @Mock private LambdaConfigurationProperties config;
+  private LambdaConfigurationProperties config;
 
   @BeforeEach
   void init(
       @WiremockResolver.Wiremock WireMockServer wireMockServer,
       @WiremockUriResolver.WiremockUri String uri) {
+    config = new LambdaConfigurationProperties();
     this.wireMockServer = wireMockServer;
     CLOUDDRIVER_BASE_URL = uri;
     MockitoAnnotations.initMocks(this);
     Mockito.when(propsMock.getCloudDriverBaseUrl()).thenReturn(uri);
+    lambdaCloudDriverUtils =
+        new LambdaCloudDriverUtils(config, propsMock, new OkHttpClient(), null);
+  }
+
+  @Test
+  public void handleTimeout() {
+
+    config.setCloudDriverPostTimeoutSeconds(1);
+    config.setCloudDriverPostRequestRetries(1);
+    this.wireMockServer.stubFor(
+        WireMock.post("/healthcheck")
+            .willReturn(aResponse().withStatus(200).withFixedDelay(20000)));
+    SpinnakerException exception =
+        assertThrows(
+            SpinnakerException.class,
+            () -> {
+              lambdaCloudDriverUtils.postToCloudDriver(
+                  CLOUDDRIVER_BASE_URL.concat("/healthcheck"), "{}");
+            });
+    exception.getCause().printStackTrace();
+    assertTrue(
+        exception.getCause() instanceof SocketTimeoutException,
+        "Should have been socket timeout... real cause was "
+            + exception.getCause().getClass()
+            + ", "
+            + exception.getCause().getMessage());
   }
 
   @Test

diff --git a/...core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/StageExecutionInternals.java b/...core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/StageExecutionInternals.java
@@ -22,6 +22,7 @@
 import com.netflix.spinnaker.orca.api.pipeline.SyntheticStageOwner;
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 
@@ -41,15 +42,17 @@ static List<StageExecution> getAncestorsImpl(
       StageExecution stage, Set<String> visited, boolean directParentOnly) {
     visited.add(stage.getRefId());
 
-    if (!stage.getRequisiteStageRefIds().isEmpty() && !directParentOnly) {
+    if (!directParentOnly && !stage.getRequisiteStageRefIds().isEmpty()) {
       // Get stages this stage depends on via requisiteStageRefIds:
+      Collection<String> requisiteStageRefIds = stage.getRequisiteStageRefIds();
       List<StageExecution> previousStages =
           stage.getExecution().getStages().stream()
-              .filter(it -> stage.getRequisiteStageRefIds().contains(it.getRefId()))
               .filter(it -> !visited.contains(it.getRefId()))
+              .filter(it -> requisiteStageRefIds.contains(it.getRefId()))
               .collect(toList());
       List<StageExecution> syntheticStages =
           stage.getExecution().getStages().stream()
+              .filter(s -> s.getSyntheticStageOwner() != null)
               .filter(
                   s ->
                       previousStages.stream()

diff --git a/.../src/main/groovy/com/netflix/spinnaker/orca/echo/spring/EchoNotifyingStageListener.groovy b/.../src/main/groovy/com/netflix/spinnaker/orca/echo/spring/EchoNotifyingStageListener.groovy
@@ -41,6 +41,7 @@ import static com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType.ORCHE
 @Slf4j
 class EchoNotifyingStageListener implements StageListener {
   public static final String INCLUDE_FULL_EXECUTION_PROPERTY = "echo.events.includeFullExecution"
+  public static final String IGNORE_TASK_EVENTS_PROPERTY = "echo.events.ignoreTaskEvents"
   private final EchoService echoService
   private final ContextParameterProcessor contextParameterProcessor
   private final DynamicConfigService dynamicConfigService
@@ -56,7 +57,9 @@ class EchoNotifyingStageListener implements StageListener {
 
   @Override
   void beforeTask(StageExecution stage, TaskExecution task) {
-    recordEvent('task', 'starting', stage, task)
+    if (!dynamicConfigService.getConfig(Boolean, IGNORE_TASK_EVENTS_PROPERTY, false)) {
+      recordEvent('task', 'starting', stage, task)
+    }
   }
 
   @Override
@@ -69,7 +72,7 @@ class EchoNotifyingStageListener implements StageListener {
   void afterTask(StageExecution stage,
                  TaskExecution task) {
     ExecutionStatus status = task.getStatus()
-    if (status == RUNNING) {
+    if (dynamicConfigService.getConfig(Boolean, IGNORE_TASK_EVENTS_PROPERTY, false) || status == RUNNING) {
       return
     }