diff --git a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/config/LambdaConfigurationProperties.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/config/LambdaConfigurationProperties.java index 6413186bd9..c3e037168d 100644 --- a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/config/LambdaConfigurationProperties.java +++ b/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; diff --git a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTask.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTask.java index 642f0effa2..8f46452f4d 100644 --- a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTask.java +++ b/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(); diff --git a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaCloudDriverUtils.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaCloudDriverUtils.java index 9fef41529f..655d4614f6 100644 --- a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaCloudDriverUtils.java +++ b/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(); diff --git a/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTaskTest.java b/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTaskTest.java index f9a9dfee08..bbf12f3bed 100644 --- a/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/lambda/LambdaCacheRefreshTaskTest.java +++ b/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 diff --git a/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaClouddriverUtilsTest.java b/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaClouddriverUtilsTest.java index f0a6d61a78..41d5cb83d9 100644 --- a/orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/utils/LambdaClouddriverUtilsTest.java +++ b/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