feat(dynamicRetry): Pull comma-delimited list of non-retryable except…

…ions from dynamic config store (#4089)
spinnaker · Oct 14, 2019 · 2b097fc · 2b097fc
1 parent a4d7d6f
commit 2b097fc
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 10 deletions.
diff --git a/...er-core/src/main/groovy/com/netflix/spinnaker/clouddriver/config/CloudDriverConfig.groovy b/...er-core/src/main/groovy/com/netflix/spinnaker/clouddriver/config/CloudDriverConfig.groovy
@@ -93,6 +93,7 @@ import com.netflix.spinnaker.clouddriver.security.MapBackedAccountCredentialsRep
 import com.netflix.spinnaker.clouddriver.security.config.SecurityConfig
 import com.netflix.spinnaker.fiat.shared.FiatPermissionEvaluator
 import com.netflix.spinnaker.kork.core.RetrySupport
+import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
 import com.netflix.spinnaker.kork.jackson.ObjectMapperSubtypeConfigurer
 import com.netflix.spinnaker.kork.jedis.RedisClientDelegate
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression
@@ -364,7 +365,8 @@ class CloudDriverConfig {
   }
 
   @Bean
-  ExceptionClassifier exceptionClassifier(ExceptionClassifierConfigurationProperties properties) {
-    return new ExceptionClassifier(properties)
+  ExceptionClassifier exceptionClassifier(ExceptionClassifierConfigurationProperties properties,
+                                          DynamicConfigService dynamicConfigService) {
+    return new ExceptionClassifier(properties, dynamicConfigService)
   }
 }
diff --git a/.../src/main/groovy/com/netflix/spinnaker/clouddriver/orchestration/ExceptionClassifier.java b/.../src/main/groovy/com/netflix/spinnaker/clouddriver/orchestration/ExceptionClassifier.java
@@ -15,30 +15,67 @@
  */
 package com.netflix.spinnaker.clouddriver.orchestration;
 
+import com.google.common.base.Splitter;
+import com.google.common.collect.Lists;
 import com.netflix.spinnaker.clouddriver.config.ExceptionClassifierConfigurationProperties;
+import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService;
 import com.netflix.spinnaker.kork.exceptions.SpinnakerException;
+import java.util.Collection;
+import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Nonnull;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.stereotype.Component;
 
 /**
  * Utility class to allow classifying non-SpinnakerException classes according to different
  * pre-determined characteristics.
  */
 @Component
+@Slf4j
 public class ExceptionClassifier {
 
   private final ExceptionClassifierConfigurationProperties properties;
+  private final DynamicConfigService dynamicConfigService;
 
-  public ExceptionClassifier(ExceptionClassifierConfigurationProperties properties) {
+  public ExceptionClassifier(
+      ExceptionClassifierConfigurationProperties properties,
+      DynamicConfigService dynamicConfigService) {
     this.properties = properties;
+    this.dynamicConfigService = dynamicConfigService;
   }
 
   /** Returns whether or not a given Exception is retryable or not. */
   public boolean isRetryable(@Nonnull Exception e) {
     if (e instanceof SpinnakerException) {
       return Optional.ofNullable(((SpinnakerException) e).getRetryable()).orElse(false);
     }
-    return !properties.getNonRetryableClasses().contains(e.getClass().getName());
+
+    try {
+      String dynamicNonRetryableClasses =
+          dynamicConfigService.getConfig(
+              String.class,
+              "clouddriver.exceptionClassifier.nonRetryableExceptions",
+              String.join(",", properties.getNonRetryableClasses()));
+
+      if (dynamicNonRetryableClasses != null) {
+        List<String> dynamicNonRetryableClassesList =
+            Lists.newArrayList(Splitter.on(",").split(dynamicNonRetryableClasses));
+
+        List<String> nonRetryableClasses =
+            Stream.of(dynamicNonRetryableClassesList, properties.getNonRetryableClasses())
+                .flatMap(Collection::stream)
+                .collect(Collectors.toList());
+
+        return !nonRetryableClasses.contains(e.getClass().getName());
+      }
+      return !properties.getNonRetryableClasses().contains(e.getClass().getName());
+
+    } catch (Exception caughtException) {
+      log.error("Unexpected exception while processing retryable classes", caughtException);
+      return false;
+    }
   }
 }
diff --git a/.../com/netflix/spinnaker/clouddriver/orchestration/DefaultOrchestrationProcessorSpec.groovy b/.../com/netflix/spinnaker/clouddriver/orchestration/DefaultOrchestrationProcessorSpec.groovy
@@ -22,6 +22,7 @@ import com.netflix.spinnaker.clouddriver.config.ExceptionClassifierConfiguration
 import com.netflix.spinnaker.clouddriver.data.task.DefaultTask
 import com.netflix.spinnaker.clouddriver.data.task.SagaId
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository
+import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
 import com.netflix.spinnaker.security.AuthenticatedRequest
 import org.slf4j.MDC
 import org.springframework.beans.factory.config.AutowireCapableBeanFactory
@@ -43,6 +44,8 @@ class DefaultOrchestrationProcessorSpec extends Specification {
 
   TaskRepository taskRepository
 
+  DynamicConfigService dynamicConfigService
+
   String taskKey
 
   def setup() {
@@ -51,6 +54,7 @@ class DefaultOrchestrationProcessorSpec extends Specification {
     taskRepository = Mock(TaskRepository)
     applicationContext = Mock(ApplicationContext)
     applicationContext.getAutowireCapableBeanFactory() >> Mock(AutowireCapableBeanFactory)
+    dynamicConfigService = Mock(DynamicConfigService)
 
     processor = new DefaultOrchestrationProcessor(
       taskRepository,
@@ -60,7 +64,7 @@ class DefaultOrchestrationProcessorSpec extends Specification {
       new ObjectMapper(),
       new ExceptionClassifier(new ExceptionClassifierConfigurationProperties(
         nonRetryableClasses: [NonRetryableException.class.getName()]
-      ))
+      ), dynamicConfigService)
     )
   }
 
@@ -81,6 +85,11 @@ class DefaultOrchestrationProcessorSpec extends Specification {
   @Unroll
   void "fail the task when exception is thrown (#exception.class.simpleName, #sagaId)"() {
     setup:
+    dynamicConfigService.getConfig(
+      String.class,
+      "clouddriver.exceptionClassifier.nonRetryableExceptions",
+      'com.netflix.spinnaker.clouddriver.orchestration.DefaultOrchestrationProcessorSpec$NonRetryableException'
+    ) >> { 'com.netflix.spinnaker.clouddriver.orchestration.DefaultOrchestrationProcessorSpec$SomeDynamicException,com.netflix.spinnaker.clouddriver.orchestration.DefaultOrchestrationProcessorSpec$AnotherDynamicException' }
     def task = new DefaultTask("1")
     if (sagaId) {
       task.sagaIdentifiers.add(sagaId)
@@ -97,11 +106,13 @@ class DefaultOrchestrationProcessorSpec extends Specification {
     task.status.retryable == retryable
 
     where:
-    exception                   | sagaId               || retryable
-    new RuntimeException()      | null                 || false
-    new NonRetryableException() | null                 || false
-    new RuntimeException()      | new SagaId("a", "a") || true
-    new NonRetryableException() | new SagaId("a", "a") || false
+    exception                     | sagaId               || retryable
+    new RuntimeException()        | null                 || false
+    new NonRetryableException()   | null                 || false
+    new RuntimeException()        | new SagaId("a", "a") || true
+    new NonRetryableException()   | new SagaId("a", "a") || false
+    new SomeDynamicException()    | new SagaId("a", "a") || false
+    new AnotherDynamicException() | new SagaId("a", "a") || false
   }
 
   void "failure should be logged in the result objects"() {
@@ -158,4 +169,6 @@ class DefaultOrchestrationProcessorSpec extends Specification {
   }
 
   private static class NonRetryableException extends RuntimeException {}
+  private static class SomeDynamicException extends RuntimeException {}
+  private static class AnotherDynamicException extends RuntimeException {}
 }