refactor(JenkinsClient): make changes to jenkinsclient to use Spinnak…

…er custom exception instead of RetrofitError using SpinnakerRetrofitErrorHandler
spinnaker · Aug 30, 2023 · 61f1883 · 61f1883
1 parent bada01e
commit 61f1883
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 18 deletions.
diff --git a/igor-web/igor-web.gradle b/igor-web/igor-web.gradle
@@ -12,6 +12,7 @@ dependencies {
     implementation project(":igor-monitor-artifactory")
     implementation project(":igor-monitor-plugins")
     implementation project(":igor-monitor-travis")
+    implementation "io.spinnaker.kork:kork-retrofit"
 
     implementation platform("io.spinnaker.kork:kork-bom:$korkVersion")
     compileOnly "org.projectlombok:lombok"

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/build/BuildController.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/build/BuildController.groovy
@@ -31,6 +31,8 @@ import com.netflix.spinnaker.igor.service.BuildOperations
 import com.netflix.spinnaker.igor.service.BuildProperties
 import com.netflix.spinnaker.igor.service.BuildServices
 import com.netflix.spinnaker.kork.artifacts.model.Artifact
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import com.netflix.spinnaker.kork.web.exceptions.InvalidRequestException
 import com.netflix.spinnaker.kork.web.exceptions.NotFoundException
 import com.netflix.spinnaker.security.AuthenticatedRequest
@@ -45,7 +47,6 @@ import org.springframework.web.bind.annotation.RequestMethod
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.RestController
 import org.springframework.web.servlet.HandlerMapping
-import retrofit.RetrofitError
 import retrofit.http.Query
 
 import javax.annotation.Nullable
@@ -178,8 +179,8 @@ class BuildController {
           if (buildService.metaClass.respondsTo(buildService, 'stopQueuedBuild')) {
             buildService.stopQueuedBuild(queuedBuild)
           }
-        } catch (RetrofitError e) {
-          if (e.response?.status != NOT_FOUND.value()) {
+        } catch (SpinnakerServerException e) {
+          if (e instanceof SpinnakerHttpException && ((SpinnakerHttpException) e).getResponseCode() != NOT_FOUND.value()) {
             throw e
           }
         }

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/JenkinsConfig.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/JenkinsConfig.groovy
@@ -31,6 +31,7 @@ import com.netflix.spinnaker.igor.config.client.JenkinsRetrofitRequestIntercepto
 import com.netflix.spinnaker.igor.jenkins.client.JenkinsClient
 import com.netflix.spinnaker.igor.jenkins.service.JenkinsService
 import com.netflix.spinnaker.igor.service.BuildServices
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
@@ -194,6 +195,7 @@ class JenkinsConfig {
             .setClient(new Ok3Client(clientBuilder.build()))
             .setConverter(new JacksonConverter(getObjectMapper()))
             .setLog(new Slf4jRetrofitLogger(JenkinsClient))
+            .setErrorHandler(SpinnakerRetrofitErrorHandler.getInstance())
             .build()
             .create(JenkinsClient)
     }

diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsService.java b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsService.java
@@ -44,6 +44,9 @@
 import com.netflix.spinnaker.igor.service.BuildProperties;
 import com.netflix.spinnaker.kork.core.RetrySupport;
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
+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.kork.web.exceptions.NotFoundException;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import io.github.resilience4j.circuitbreaker.CircuitBreaker;
@@ -92,10 +95,10 @@ public JenkinsService(
             CircuitBreakerConfig.custom()
                 .ignoreException(
                     (e) -> {
-                      if (e instanceof RetrofitError) {
-                        RetrofitError re = (RetrofitError) e;
-                        return re.getKind() == RetrofitError.Kind.HTTP
-                            && re.getResponse().getStatus() == 404;
+                      if (e instanceof SpinnakerServerException) {
+                        SpinnakerServerException re = (SpinnakerServerException) e;
+                        return re instanceof SpinnakerHttpException
+                            && ((SpinnakerHttpException) re).getResponseCode() == 404;
                       }
                       return false;
                     })
@@ -240,8 +243,9 @@ public Build getLatestBuild(String jobName) {
   public QueuedJob queuedBuild(String master, int item) {
     try {
       return circuitBreaker.executeSupplier(() -> jenkinsClient.getQueuedItem(item));
-    } catch (RetrofitError e) {
-      if (e.getResponse() != null && e.getResponse().getStatus() == NOT_FOUND.value()) {
+    } catch (SpinnakerServerException e) {
+      if (e instanceof SpinnakerHttpException
+          && ((SpinnakerHttpException) e).getResponseCode() == NOT_FOUND.value()) {
         throw new NotFoundException(
             String.format("Queued job '%s' not found for master '%s'.", item, master));
       }
@@ -331,12 +335,12 @@ private Response getPropertyFile(String jobName, Integer buildNumber, String fil
         () -> {
           try {
             return jenkinsClient.getPropertyFile(encode(jobName), buildNumber, fileName);
-          } catch (RetrofitError e) {
+          } catch (SpinnakerServerException e) {
             // retry on network issue, 404 and 5XX
-            if (e.getKind() == RetrofitError.Kind.NETWORK
-                || (e.getKind() == RetrofitError.Kind.HTTP
-                    && (e.getResponse().getStatus() == 404
-                        || e.getResponse().getStatus() >= 500))) {
+            if (e instanceof SpinnakerNetworkException
+                || (e instanceof SpinnakerHttpException
+                    && (((SpinnakerHttpException) e).getResponseCode() == 404
+                        || ((SpinnakerHttpException) e).getResponseCode() >= 500))) {
               throw e;
             }
             SpinnakerException ex = new SpinnakerException(e);

diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/JenkinsBuildMonitorSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/JenkinsBuildMonitorSpec.groovy
@@ -29,6 +29,7 @@ import com.netflix.spinnaker.igor.polling.PollContext
 import com.netflix.spinnaker.igor.service.BuildServices
 import com.netflix.spinnaker.kork.discovery.DiscoveryStatusListener
 import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException
 import org.slf4j.Logger
 import org.springframework.scheduling.TaskScheduler
 import retrofit.RetrofitError
@@ -256,8 +257,9 @@ class   JenkinsBuildMonitorSpec extends Specification {
             new Build(number: 1, timestamp: nowMinus30min, building: false, result: 'SUCCESS', duration: durationOf1min)
         ]
 
-        def retrofitEx = RetrofitError.unexpectedError("http://retro.fit/mock/error", new Exception('mock root cause'));
-        jenkinsService.getBuilds('job2') >> { throw new RuntimeException ("Mocked failure while fetching 'job2'", retrofitEx) }
+        def retrofitEx = new SpinnakerServerException(RetrofitError.unexpectedError("http://retro.fit/mock/error", new Exception('mock root cause')));
+        def runtimeException = new RuntimeException ("Mocked failure while fetching 'job2'", retrofitEx)
+        jenkinsService.getBuilds('job2') >> { throw runtimeException.getCause() }
 
         jenkinsService.getBuilds('job3') >> [
             new Build(number: 3, timestamp: nowMinus30min, building: false, result: 'SUCCESS', duration: durationOf1min)

diff --git a/...-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsServiceSpec.groovy b/...-web/src/test/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsServiceSpec.groovy
@@ -26,6 +26,7 @@ import com.netflix.spinnaker.igor.jenkins.client.model.Build
 import com.netflix.spinnaker.igor.jenkins.client.model.BuildArtifact
 import com.netflix.spinnaker.igor.jenkins.client.model.BuildsList
 import com.netflix.spinnaker.igor.jenkins.client.model.Project
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.squareup.okhttp.mockwebserver.MockResponse
 import com.squareup.okhttp.mockwebserver.MockWebServer
 import io.github.resilience4j.circuitbreaker.CircuitBreaker
@@ -765,12 +766,12 @@ class JenkinsServiceSpec extends Specification {
       build.number = buildNumber
       build.duration = 0L
       build.artifacts = [artifact]
-      def badRequestError = RetrofitError.httpError(
+      def badRequestError = new SpinnakerHttpException(RetrofitError.httpError(
         "http://my.jenkins.net",
         new Response("http://my.jenkins.net", 400, "bad request", [], null),
         null,
         null
-      )
+      ))
 
       when:
       def properties = service.getBuildProperties(jobName, build.genericBuild(jobName), artifact.fileName)