perf(aws): termination lifecycle eureka perf improvements (#1502)

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationConfigurationProperties.groovy

@@ -27,6 +27,8 @@ class InstanceTerminationConfigurationProperties {
   int visibilityTimeout = 30
   int waitTimeSeconds = 5
   int sqsMessageRetentionPeriodSeconds = 120
+  int eurekaFindApplicationRetryMax = 3
+  int eurekaUpdateStatusRetryMax = 3
 
   InstanceTerminationConfigurationProperties() {
     // default constructor
@@ -37,12 +39,16 @@ class InstanceTerminationConfigurationProperties {
                                              String topicARN,
                                              int visibilityTimeout,
                                              int waitTimeSeconds,
-                                             int sqsMessageRetentionPeriodSeconds) {
+                                             int sqsMessageRetentionPeriodSeconds,
+                                             int eurekaFindApplicationRetryMax,
+                                             int eurekaUpdateStatusRetryMax) {
     this.accountName = accountName
     this.queueARN = queueARN
     this.topicARN = topicARN
     this.visibilityTimeout = visibilityTimeout
     this.waitTimeSeconds = waitTimeSeconds
     this.sqsMessageRetentionPeriodSeconds = sqsMessageRetentionPeriodSeconds
+    this.eurekaFindApplicationRetryMax = eurekaFindApplicationRetryMax
+    this.eurekaUpdateStatusRetryMax = eurekaUpdateStatusRetryMax
   }
 }

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorker.java

@@ -212,7 +212,8 @@ private void handleMessage(LifecycleMessage message, Task task) {
     description.setInstanceIds(instanceIds);
 
     discoverySupport.get().updateDiscoveryStatusForInstances(
-      description, task, "handleLifecycleMessage", DiscoveryStatus.Disable, instanceIds
+      description, task, "handleLifecycleMessage", DiscoveryStatus.Disable, instanceIds,
+      properties.getEurekaFindApplicationRetryMax(), properties.getEurekaUpdateStatusRetryMax()
     );
 
     recordLag(message.time, queueARN.region, message.accountId, message.autoScalingGroupName, message.ec2InstanceId);

clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorkerProvider.java

@@ -24,7 +24,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.stereotype.Component;
 
 import javax.annotation.PostConstruct;
@@ -35,7 +35,7 @@
 import java.util.regex.Pattern;
 
 @Component
-@ConditionalOnProperty("aws.lifecycleSubscribers.instanceTermination.enabled")
+@ConditionalOnExpression("${aws.lifecycleSubscribers.instanceTermination.enabled:false} && ${caching.writeEnabled:true}")
 public class InstanceTerminationLifecycleWorkerProvider {
   private final static String REGION_TEMPLATE_PATTERN = Pattern.quote("{{region}}");
   private final static String ACCOUNT_ID_TEMPLATE_PATTERN = Pattern.quote("{{accountId}}");
@@ -87,7 +87,9 @@ public void start() {
             .replaceAll(ACCOUNT_ID_TEMPLATE_PATTERN, credentials.getAccountId()),
           properties.getVisibilityTimeout(),
           properties.getWaitTimeSeconds(),
-          properties.getSqsMessageRetentionPeriodSeconds()
+          properties.getSqsMessageRetentionPeriodSeconds(),
+          properties.getEurekaFindApplicationRetryMax(),
+          properties.getEurekaUpdateStatusRetryMax()
         ),
         discoverySupport,
         registry

clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/discovery/DiscoverySupportUnitSpec.groovy

@@ -107,6 +107,56 @@ class DiscoverySupportUnitSpec extends Specification {
     0 * eureka.updateInstanceStatus(*_)
   }
 
+  void "should short-circuit if application name is not in discovery and server group does not exist"() {
+    given:
+    def discoverySupport = new AwsEurekaSupport() {
+      {
+        clusterProviders = []
+      }
+
+      @Override
+      protected long getDiscoveryRetryMs() {
+        return 0
+      }
+
+      @Override
+      boolean verifyInstanceAndAsgExist(def credentials, String region, String instanceId, String asgName) {
+        return false
+      }
+    }
+    discoverySupport.regionScopedProviderFactory = Stub(RegionScopedProviderFactory) {
+      getAmazonClientProvider() >> {
+        return Stub(AmazonClientProvider)
+      }
+
+      forRegion(_, _) >> {
+        return Stub(RegionScopedProviderFactory.RegionScopedProvider) {
+          getEureka() >> eureka
+        }
+      }
+    }
+    discoverySupport.eurekaSupportConfigurationProperties = new EurekaSupportConfigurationProperties()
+
+    and:
+    def task = Mock(Task)
+    def description = new EnableDisableInstanceDiscoveryDescription(
+      asgName: 'myapp-test-v000',
+      region: 'us-east-1',
+      credentials: TestCredential.named('test', [discovery: 'http://{{region}}.discovery.netflix.net'])
+    )
+    def instances = ["i-123456"]
+
+    when:
+    discoverySupport.updateDiscoveryStatusForInstances(
+      description, task, "phase", AbstractEurekaSupport.DiscoveryStatus.Disable, instances
+    )
+
+    then:
+    discoverySupport.eurekaSupportConfigurationProperties.retryMax * task.getStatus() >> new DefaultTaskStatus(state: TaskState.STARTED)
+    0 * eureka.updateInstanceStatus(*_)
+    1 * task.updateStatus(_, "Could not find application name in Discovery or AWS, short-circuiting (asg: myapp-test-v000, region: us-east-1)")
+  }
+
   void "should enable each instance individually in discovery"() {
     given:
     def task = Mock(Task)

clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/lifecycle/InstanceTerminationLifecycleWorkerSpec.groovy

@@ -73,6 +73,8 @@ class InstanceTerminationLifecycleWorkerSpec extends Specification {
       topicARN.arn,
       -1,
       -1,
+      -1,
+      -1,
       -1
     ),
     awsEurekaSupportProvider,
@@ -140,7 +142,9 @@ class InstanceTerminationLifecycleWorkerSpec extends Specification {
       _ as Task,
       'handleLifecycleMessage',
       DiscoveryStatus.Disable,
-      ['i-1234']
+      ['i-1234'],
+      -1,
+      -1
     )
   }
 

clouddriver-eureka/src/main/groovy/com/netflix/spinnaker/clouddriver/eureka/deploy/ops/AbstractEurekaSupport.groovy

@@ -49,6 +49,19 @@ abstract class AbstractEurekaSupport {
                                          String phaseName,
                                          DiscoveryStatus discoveryStatus,
                                          List<String> instanceIds) {
+    updateDiscoveryStatusForInstances(
+      description, task, phaseName, discoveryStatus, instanceIds, eurekaSupportConfigurationProperties.retryMax, eurekaSupportConfigurationProperties.retryMax
+    )
+  }
+
+
+  void updateDiscoveryStatusForInstances(def description,
+                                         Task task,
+                                         String phaseName,
+                                         DiscoveryStatus discoveryStatus,
+                                         List<String> instanceIds,
+                                         int findApplicationNameRetryMax,
+                                         int updateEurekaRetryMax) {
 
     if (eurekaSupportConfigurationProperties == null) {
       throw new IllegalStateException("eureka configuration not supplied")
@@ -58,7 +71,7 @@ abstract class AbstractEurekaSupport {
     def random = new Random()
     def applicationName = null
     try {
-      applicationName = retry(task, phaseName, eurekaSupportConfigurationProperties.retryMax) { retryCount ->
+      applicationName = retry(task, phaseName, findApplicationNameRetryMax) { retryCount ->
         def instanceId = instanceIds[random.nextInt(instanceIds.size())]
         task.updateStatus phaseName, "Looking up discovery application name for instance $instanceId (attempt: $retryCount)"
 
@@ -75,6 +88,12 @@ abstract class AbstractEurekaSupport {
       }
     }
 
+    // In rare (delayed evented) cases, calls to update discovery status may happen against instances that no longer exist
+    if (applicationName == null) {
+      task.updateStatus phaseName, "Could not find application name in Discovery or AWS, short-circuiting (asg: ${description.asgName}, region: ${description.region})"
+      return
+    }
+
     def errors = [:]
     boolean shouldFail = false
     int index = 0
@@ -104,7 +123,7 @@ abstract class AbstractEurekaSupport {
       }
 
       try {
-        retry(task, phaseName, eurekaSupportConfigurationProperties.retryMax) { retryCount ->
+        retry(task, phaseName, updateEurekaRetryMax) { retryCount ->
           task.updateStatus phaseName, "Attempting to mark ${instanceId} as '${discoveryStatus.value}' in discovery (attempt: ${retryCount})."
 
           Response resp