feat(timeouts): Configure post timeouts for clouddriver. Using standard okhttpclient (#4522)

Author: jasonmcintosh

orca-clouddriver/src/main/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;

orca-clouddriver/src/main/java/com/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();

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

orca-clouddriver/src/test/java/com/netflix/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

orca-clouddriver/src/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