Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(igor): use SpinnakerRetrofitErrorHandler with IgorService #4666

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
import com.jakewharton.retrofit.Ok3Client
import com.netflix.spinnaker.config.DefaultServiceEndpoint
import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
import com.netflix.spinnaker.orca.igor.IgorService
import com.netflix.spinnaker.orca.retrofit.RetrofitConfiguration
import com.netflix.spinnaker.orca.retrofit.logging.RetrofitSlf4jLog
Expand All @@ -34,7 +35,6 @@ import org.springframework.context.annotation.Import
import retrofit.Endpoint
import retrofit.RequestInterceptor
import retrofit.RestAdapter
import retrofit.client.Client
import retrofit.converter.JacksonConverter
import static retrofit.Endpoints.newFixedEndpoint

Expand All @@ -61,6 +61,7 @@ class IgorConfiguration {
.setEndpoint(igorEndpoint)
.setClient(new Ok3Client(clientProvider.getClient(new DefaultServiceEndpoint("igor", igorEndpoint.url))))
.setLogLevel(retrofitLogLevel)
.setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
.setRequestInterceptor(spinnakerRequestInterceptor)
.setLog(new RetrofitSlf4jLog(IgorService))
.setConverter(new JacksonConverter(mapper))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.netflix.spinnaker.orca.igor.tasks

import com.google.common.base.Enums
import com.netflix.spinnaker.kork.core.RetrySupport
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.OverridableTimeoutRetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
Expand All @@ -26,7 +27,6 @@ import com.netflix.spinnaker.orca.igor.BuildService
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError

import java.util.concurrent.TimeUnit

Expand Down Expand Up @@ -81,9 +81,9 @@ class MonitorJenkinsJobTask implements OverridableTimeoutRetryableTask {
}

return TaskResult.builder(taskStatus).context(outputs).outputs(outputs).build()
} catch (RetrofitError e) {
if ([503, 500, 404].contains(e.response?.status)) {
log.warn("Http ${e.response.status} received from `igor`, retrying...")
} catch (SpinnakerHttpException e) {
if ([503, 500, 404].contains(e.responseCode)) {
log.warn("Http ${e.responseCode} received from `igor`, retrying...")
return TaskResult.ofStatus(ExecutionStatus.RUNNING)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.igor.tasks

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.igor.IgorService
import org.springframework.beans.factory.annotation.Value
Expand All @@ -28,7 +29,6 @@ import com.netflix.spinnaker.orca.igor.BuildService
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError

@Slf4j
@Component
Expand Down Expand Up @@ -56,9 +56,9 @@ class MonitorQueuedJenkinsJobTask implements OverridableTimeoutRetryableTask {
createBacklink(stage, jenkinsController, jobName, build.number as Integer)
return TaskResult.builder(ExecutionStatus.SUCCEEDED).context([buildNumber: build.number]).build()
}
} catch (RetrofitError e) {
if ([503, 500, 404].contains(e.response?.status)) {
log.warn("Http ${e.response.status} received from `igor`, retrying...")
} catch (SpinnakerHttpException e) {
if ([503, 500, 404].contains(e.responseCode)) {
log.warn("Http ${e.responseCode} received from `igor`, retrying...")
return TaskResult.ofStatus(ExecutionStatus.RUNNING)
}
throw e
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
package com.netflix.spinnaker.orca.igor.tasks

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.OverridableTimeoutRetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
Expand All @@ -16,7 +17,6 @@ import com.netflix.spinnaker.orca.igor.BuildService
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.stereotype.Component
import retrofit.RetrofitError

import javax.annotation.Nonnull
import java.util.concurrent.TimeUnit
Expand All @@ -38,7 +38,6 @@ class MonitorWerckerJobStartedTask implements OverridableTimeoutRetryableTask {

try {
Map<String, Object> build = buildService.getBuild(buildNumber, master, job)
Map outputs = [:]
if ("not_built".equals(build?.result) || build?.number == null) {
//The build has not yet started, so the job started monitoring task needs to be re-run
return TaskResult.ofStatus(ExecutionStatus.RUNNING)
Expand All @@ -47,9 +46,9 @@ class MonitorWerckerJobStartedTask implements OverridableTimeoutRetryableTask {
return TaskResult.builder(ExecutionStatus.SUCCEEDED).context([buildNumber: build.number]).build()
}

} catch (RetrofitError e) {
if ([503, 500, 404].contains(e.response?.status)) {
log.warn("Http ${e.response.status} received from `igor`, retrying...")
} catch (SpinnakerHttpException e) {
if ([503, 500, 404].contains(e.responseCode)) {
log.warn("Http ${e.responseCode} received from `igor`, retrying...")
return TaskResult.ofStatus(ExecutionStatus.RUNNING)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,19 @@ package com.netflix.spinnaker.orca.igor.tasks

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.exceptions.SystemException
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.Task
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.exceptions.ExceptionHandler
import com.netflix.spinnaker.orca.igor.BuildService
import com.netflix.spinnaker.orca.retrofit.exceptions.RetrofitExceptionHandler
import com.netflix.spinnaker.orca.retrofit.exceptions.SpinnakerServerExceptionHandler
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.http.HttpStatus
import org.springframework.stereotype.Component
import retrofit.RetrofitError
import retrofit.client.Response

import javax.annotation.Nonnull
Expand All @@ -48,7 +47,7 @@ class StartJenkinsJobTask implements RetryableTask {
ObjectMapper objectMapper

@Autowired
RetrofitExceptionHandler retrofitExceptionHandler
SpinnakerServerExceptionHandler spinnakerServerExceptionHandler

@Nonnull
@Override
Expand All @@ -72,9 +71,9 @@ class StartJenkinsJobTask implements RetryableTask {
.build()
}
}
catch (RetrofitError e) {
catch (SpinnakerServerException e){
// This igor call is idempotent so we can retry despite it being PUT/POST
ExceptionHandler.Response exceptionResponse = retrofitExceptionHandler.handle("StartJenkinsJob", e)
ExceptionHandler.Response exceptionResponse = spinnakerServerExceptionHandler.handle("StartJenkinsJob", e)

if (exceptionResponse.shouldRetry) {
log.warn("Failure communicating with igor to start a jenkins job, will retry", e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ package com.netflix.spinnaker.orca.igor.tasks

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.exceptions.SystemException
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.exceptions.ExceptionHandler
import com.netflix.spinnaker.orca.igor.BuildService
import com.netflix.spinnaker.orca.retrofit.exceptions.RetrofitExceptionHandler
import com.netflix.spinnaker.orca.retrofit.exceptions.SpinnakerServerExceptionHandler
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.beans.factory.annotation.Value
import org.springframework.core.env.Environment
import org.springframework.http.HttpStatus
import org.springframework.stereotype.Component
import retrofit.RetrofitError
import retrofit.client.Response

import javax.annotation.Nonnull
Expand All @@ -49,7 +49,7 @@ class StartScriptTask implements RetryableTask {
ObjectMapper objectMapper

@Autowired
RetrofitExceptionHandler retrofitExceptionHandler
SpinnakerServerExceptionHandler spinnakerServerExceptionHandler

@Autowired
Environment environment
Expand Down Expand Up @@ -113,9 +113,9 @@ class StartScriptTask implements RetryableTask {
.build()
}
}
catch (RetrofitError e) {
catch (SpinnakerServerException e) {
// This igor call is idempotent so we can retry despite it being PUT/POST
ExceptionHandler.Response exceptionResponse = retrofitExceptionHandler.handle("StartJenkinsJob", e)
ExceptionHandler.Response exceptionResponse = spinnakerServerExceptionHandler.handle("StartJenkinsJob", e)

if (exceptionResponse.shouldRetry) {
log.warn("Failure communicating with igor to start a jenkins job, will retry", e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
package com.netflix.spinnaker.orca.igor.tasks;

import com.google.common.collect.ImmutableMap;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException;
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask;
import com.netflix.spinnaker.orca.api.pipeline.TaskResult;
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus;
Expand All @@ -28,7 +31,6 @@
import javax.annotation.Nonnull;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import retrofit.RetrofitError;

@RequiredArgsConstructor
@Slf4j
Expand All @@ -53,7 +55,7 @@ protected int getMaxConsecutiveErrors() {
try {
TaskResult result = tryExecute(stageDefinition);
return resetErrorCount(result);
} catch (RetrofitError e) {
} catch (SpinnakerServerException e) {
if (stageDefinition.getConsecutiveErrors() < getMaxConsecutiveErrors() && isRetryable(e)) {
return TaskResult.builder(ExecutionStatus.RUNNING)
.context(errorContext(errors + 1))
Expand Down Expand Up @@ -83,14 +85,13 @@ private Map<String, Integer> errorContext(int errors) {
return Collections.singletonMap("consecutiveErrors", errors);
}

private boolean isRetryable(RetrofitError retrofitError) {
if (retrofitError.getKind() == RetrofitError.Kind.NETWORK) {
private boolean isRetryable(SpinnakerServerException exception) {
if (exception instanceof SpinnakerNetworkException) {
log.warn("Failed to communicate with igor, retrying...");
return true;
}

if (retrofitError.getResponse() != null) {
int status = retrofitError.getResponse().getStatus();
} else if (exception instanceof SpinnakerHttpException) {
var httpException = (SpinnakerHttpException) exception;
int status = httpException.getResponseCode();
if (status == 500 || status == 503) {
log.warn(String.format("Received HTTP %s response from igor, retrying...", status));
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.igor.tasks

import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.igor.IgorService
Expand Down Expand Up @@ -73,14 +74,14 @@ class GetAwsCodeBuildArtifactsTaskSpec extends Specification {
TaskResult result = task.execute(stage)

then:
1 * igorService.getAwsCodeBuildArtifacts(ACCOUNT, BUILD_ID) >> { throw stubRetrofitError() }
1 * igorService.getAwsCodeBuildArtifacts(ACCOUNT, BUILD_ID) >> { throw stubNetworkError() }
0 * igorService._
result.getStatus() == ExecutionStatus.RUNNING
}

def stubRetrofitError() {
return Stub(RetrofitError) {
def stubNetworkError() {
return new SpinnakerNetworkException(Stub(RetrofitError) {
getKind() >> RetrofitError.Kind.NETWORK
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.netflix.spinnaker.orca.igor.tasks
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.kork.exceptions.ConfigurationException
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.igor.BuildService
Expand Down Expand Up @@ -120,9 +121,11 @@ class GetBuildPropertiesTaskSpec extends Specification {
getResponse() >> new Response("", 500, "", Collections.emptyList(), null)
}

def httpException = new SpinnakerHttpException(igorError)

and:
1 * buildService.getPropertyFile(BUILD_NUMBER, PROPERTY_FILE, MASTER, JOB) >>
{ throw igorError }
{ throw httpException }

when:
TaskResult result = task.execute(stage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,7 @@ class GetCommitsTaskSpec extends Specification {

then:
1 * scmService.compareCommits("stash", "projectKey", "repositorySlug", ['to': '186605b', 'from': 'a86305d', 'limit': 100]) >> {
throw new RetrofitError(null, null,
new Response("http://stash.com", 500, "test reason", [], null), null, null, null, null)
throw new SpinnakerHttpException(RetrofitError.httpError("http://stash.com", new Response("http://stash.com", 500, "test reason", [], null), null, null))
}
result.status == taskStatus

Expand Down Expand Up @@ -450,7 +449,7 @@ class GetCommitsTaskSpec extends Specification {
and:
task.scmService = Stub(ScmService) {
compareCommits("stash", "projectKey", "repositorySlug", ['to': '186605b', 'from': 'a86305d', 'limit': 100]) >> {
throw new RetrofitError(null, null, new Response("http://stash.com", 404, "test reason", [], null), null, null, null, null)
throw new SpinnakerHttpException(RetrofitError.httpError("http://stash.com", new Response("http://stash.com", 404, "test reason", [], null), null, null))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.igor.tasks

import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.igor.IgorService
Expand Down Expand Up @@ -72,14 +73,14 @@ class GetGoogleCloudBuildArtifactsTaskSpec extends Specification {
TaskResult result = task.execute(stage)

then:
1 * igorService.getGoogleCloudBuildArtifacts(ACCOUNT, BUILD_ID) >> { throw stubRetrofitError() }
1 * igorService.getGoogleCloudBuildArtifacts(ACCOUNT, BUILD_ID) >> { throw stubNetworkError() }
0 * igorService._
result.getStatus() == ExecutionStatus.RUNNING
}

def stubRetrofitError() {
return Stub(RetrofitError) {
def stubNetworkError() {
return new SpinnakerNetworkException(Stub(RetrofitError) {
getKind() >> RetrofitError.Kind.NETWORK
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.igor.tasks

import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.igor.IgorService
Expand Down Expand Up @@ -82,14 +83,14 @@ class MonitorAwsCodeBuildTaskSpec extends Specification {
TaskResult result = task.execute(stage)

then:
1 * igorService.getAwsCodeBuildExecution(ACCOUNT, BUILD_ID) >> { throw stubRetrofitError() }
1 * igorService.getAwsCodeBuildExecution(ACCOUNT, BUILD_ID) >> { throw stubNetworkError() }
0 * igorService._
result.getStatus() == ExecutionStatus.RUNNING
}

def stubRetrofitError() {
return Stub(RetrofitError) {
def stubNetworkError() {
return new SpinnakerNetworkException(Stub(RetrofitError) {
getKind() >> RetrofitError.Kind.NETWORK
}
})
}
}
Loading