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

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

Hide file tree

Showing 5 changed files with 44 additions and 1 deletion.
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
@@ -178,6 +180,10 @@ class BuildController {
           if (buildService.metaClass.respondsTo(buildService, 'stopQueuedBuild')) {
             buildService.stopQueuedBuild(queuedBuild)
           }
+        } catch (SpinnakerServerException e) {
+          if (e instanceof SpinnakerHttpException && ((SpinnakerHttpException) e).responseCode != NOT_FOUND.value()) {
+            throw e
+          }
         } catch (RetrofitError e) {
           if (e.response?.status != 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;
@@ -215,6 +218,16 @@ private ScmDetails getGitDetails(String jobName, Integer buildNumber) {
         () -> {
           try {
             return jenkinsClient.getGitDetails(encode(jobName), buildNumber);
+          } catch (SpinnakerServerException e) {
+            // assuming that a conversion error is unlikely to succeed on retry
+            if (e instanceof SpinnakerConversionException) {
+              log.warn(
+                  "Unable to deserialize git details for build " + buildNumber + " of " + jobName,
+                  e);
+              return null;
+            } else {
+              throw e;
+            }
           } catch (RetrofitError e) {
             // assuming that a conversion error is unlikely to succeed on retry
             if (e.getKind() == RetrofitError.Kind.CONVERSION) {
@@ -240,6 +253,13 @@ public Build getLatestBuild(String jobName) {
   public QueuedJob queuedBuild(String master, int item) {
     try {
       return circuitBreaker.executeSupplier(() -> jenkinsClient.getQueuedItem(item));
+    } 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));
+      }
+      throw e;
     } catch (RetrofitError e) {
       if (e.getResponse() != null && e.getResponse().getStatus() == NOT_FOUND.value()) {
         throw new NotFoundException(
@@ -331,6 +351,19 @@ private Response getPropertyFile(String jobName, Integer buildNumber, String fil
         () -> {
           try {
             return jenkinsClient.getPropertyFile(encode(jobName), buildNumber, fileName);
+          } catch (SpinnakerNetworkException e) {
+            throw e;
+          } catch (SpinnakerHttpException e) {
+            if (e.getResponseCode() == 404 || e.getResponseCode() >= 500) {
+              throw e;
+            }
+            SpinnakerException ex = new SpinnakerException(e);
+            ex.setRetryable(false);
+            throw ex;
+          } catch (SpinnakerServerException e) {
+            SpinnakerException ex = new SpinnakerException(e);
+            ex.setRetryable(false);
+            throw ex;
           } catch (RetrofitError e) {
             // retry on network issue, 404 and 5XX
             if (e.getKind() == RetrofitError.Kind.NETWORK

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,7 +257,7 @@ 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'));
+        def retrofitEx = new SpinnakerServerException(RetrofitError.unexpectedError("http://retro.fit/mock/error", new Exception('mock root cause')));
         jenkinsService.getBuilds('job2') >> { throw new RuntimeException ("Mocked failure while fetching 'job2'", retrofitEx) }
 
         jenkinsService.getBuilds('job3') >> [