fix(entitytags): Retry support when fetching entity tags from Front50

spinnaker · Dec 14, 2017 · 9dad8a4 · 9dad8a4
1 parent 812c61f
commit 9dad8a4
Show file tree

Hide file tree

Showing 9 changed files with 54 additions and 14 deletions.
diff --git a/.../main/java/com/netflix/spinnaker/clouddriver/elasticsearch/ElasticSearchEntityTagger.java b/.../main/java/com/netflix/spinnaker/clouddriver/elasticsearch/ElasticSearchEntityTagger.java
@@ -29,6 +29,7 @@
 import com.netflix.spinnaker.clouddriver.model.EntityTags;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider;
 import com.netflix.spinnaker.clouddriver.tags.EntityTagger;
+import com.netflix.spinnaker.kork.core.RetrySupport;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -48,13 +49,16 @@ public class ElasticSearchEntityTagger implements EntityTagger {
   private static final String NOTICE_TYPE = "notice";
   private static final String NOTICE_KEY_PREFIX = "spinnaker_ui_notice:";
 
+  private final RetrySupport retrySupport;
   private final Front50Service front50Service;
   private final AccountCredentialsProvider accountCredentialsProvider;
   private final ElasticSearchEntityTagsProvider entityTagsProvider;
 
-  public ElasticSearchEntityTagger(Front50Service front50Service,
+  public ElasticSearchEntityTagger(RetrySupport retrySupport,
+                                   Front50Service front50Service,
                                    AccountCredentialsProvider accountCredentialsProvider,
                                    ElasticSearchEntityTagsProvider entityTagsProvider) {
+    this.retrySupport = retrySupport;
     this.front50Service = front50Service;
     this.accountCredentialsProvider = accountCredentialsProvider;
     this.entityTagsProvider = entityTagsProvider;
@@ -199,6 +203,7 @@ private void upsertEntityTags(String type,
                                 String value,
                                 Long timestamp) {
     UpsertEntityTagsAtomicOperation upsertEntityTagsAtomicOperation = new UpsertEntityTagsAtomicOperation(
+      retrySupport,
       front50Service,
       accountCredentialsProvider,
       entityTagsProvider,

diff --git a/...er/clouddriver/elasticsearch/converters/BulkUpsertEntityTagsAtomicOperationConverter.java b/...er/clouddriver/elasticsearch/converters/BulkUpsertEntityTagsAtomicOperationConverter.java
@@ -24,6 +24,7 @@
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.clouddriver.security.AbstractAtomicOperationsCredentialsSupport;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider;
+import com.netflix.spinnaker.kork.core.RetrySupport;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -33,24 +34,27 @@
 public class BulkUpsertEntityTagsAtomicOperationConverter extends AbstractAtomicOperationsCredentialsSupport {
 
   private final ObjectMapper objectMapper;
+  private final RetrySupport retrySupport;
   private final Front50Service front50Service;
   private final AccountCredentialsProvider accountCredentialsProvider;
   private final ElasticSearchEntityTagsProvider entityTagsProvider;
 
   @Autowired
   public BulkUpsertEntityTagsAtomicOperationConverter(ObjectMapper objectMapper,
+                                                      RetrySupport retrySupport,
                                                       Front50Service front50Service,
                                                       AccountCredentialsProvider accountCredentialsProvider,
                                                       ElasticSearchEntityTagsProvider entityTagsProvider) {
     this.objectMapper = objectMapper;
+    this.retrySupport = retrySupport;
     this.front50Service = front50Service;
     this.accountCredentialsProvider = accountCredentialsProvider;
     this.entityTagsProvider = entityTagsProvider;
   }
 
   public AtomicOperation convertOperation(Map input) {
     return new BulkUpsertEntityTagsAtomicOperation(
-      front50Service, accountCredentialsProvider, entityTagsProvider, this.convertDescription(input)
+      retrySupport, front50Service, accountCredentialsProvider, entityTagsProvider, this.convertDescription(input)
     );
   }
 

diff --git a/...nnaker/clouddriver/elasticsearch/converters/UpsertEntityTagsAtomicOperationConverter.java b/...nnaker/clouddriver/elasticsearch/converters/UpsertEntityTagsAtomicOperationConverter.java
@@ -27,6 +27,7 @@
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.clouddriver.security.AbstractAtomicOperationsCredentialsSupport;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider;
+import com.netflix.spinnaker.kork.core.RetrySupport;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -36,27 +37,30 @@
 @Component("upsertEntityTags")
 public class UpsertEntityTagsAtomicOperationConverter extends AbstractAtomicOperationsCredentialsSupport {
   private final ObjectMapper objectMapper;
+  private final RetrySupport retrySupport;
   private final Front50Service front50Service;
   private final AccountCredentialsProvider accountCredentialsProvider;
   private final ElasticSearchEntityTagsProvider entityTagsProvider;
 
   @Autowired
   public UpsertEntityTagsAtomicOperationConverter(ObjectMapper objectMapper,
+                                                  RetrySupport retrySupport,
                                                   Front50Service front50Service,
                                                   AccountCredentialsProvider accountCredentialsProvider,
                                                   ElasticSearchEntityTagsProvider entityTagsProvider) {
     this.objectMapper = objectMapper
       .configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false)
       .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
 
+    this.retrySupport = retrySupport;
     this.front50Service = front50Service;
     this.accountCredentialsProvider = accountCredentialsProvider;
     this.entityTagsProvider = entityTagsProvider;
   }
 
   public AtomicOperation convertOperation(Map input) {
     return new UpsertEntityTagsAtomicOperation(
-      front50Service, accountCredentialsProvider, entityTagsProvider, this.convertDescription(input)
+      retrySupport, front50Service, accountCredentialsProvider, entityTagsProvider, this.convertDescription(input)
     );
   }
 

diff --git a/.../netflix/spinnaker/clouddriver/elasticsearch/ops/BulkUpsertEntityTagsAtomicOperation.java b/.../netflix/spinnaker/clouddriver/elasticsearch/ops/BulkUpsertEntityTagsAtomicOperation.java
@@ -27,6 +27,7 @@
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentials;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider;
+import com.netflix.spinnaker.kork.core.RetrySupport;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,15 +42,18 @@ public class BulkUpsertEntityTagsAtomicOperation implements AtomicOperation<Bulk
   private static final Logger log = LoggerFactory.getLogger(BulkUpsertEntityTagsAtomicOperation.class);
   private static final String BASE_PHASE = "ENTITY_TAGS";
 
+  private final RetrySupport retrySupport;
   private final Front50Service front50Service;
   private final AccountCredentialsProvider accountCredentialsProvider;
   private final ElasticSearchEntityTagsProvider entityTagsProvider;
   private final BulkUpsertEntityTagsDescription bulkUpsertEntityTagsDescription;
 
-  public BulkUpsertEntityTagsAtomicOperation(Front50Service front50Service,
+  public BulkUpsertEntityTagsAtomicOperation(RetrySupport retrySupport,
+                                             Front50Service front50Service,
                                              AccountCredentialsProvider accountCredentialsProvider,
                                              ElasticSearchEntityTagsProvider entityTagsProvider,
                                              BulkUpsertEntityTagsDescription bulkUpsertEntityTagsDescription) {
+    this.retrySupport = retrySupport;
     this.front50Service = front50Service;
     this.accountCredentialsProvider = accountCredentialsProvider;
     this.entityTagsProvider = entityTagsProvider;
@@ -90,9 +94,16 @@ public BulkUpsertEntityTagsAtomicOperationResult operate(List priorOutputs) {
   }
 
   private Map<String, EntityTags> retrieveExistingTags(List<EntityTags> entityTags) {
-    return front50Service.getAllEntityTagsById(
-      entityTags.stream().map(EntityTags::getId).collect(Collectors.toList())
-    ).stream().collect(Collectors.toMap(EntityTags::getId, Function.identity()));
+    List<String> ids = entityTags.stream().map(EntityTags::getId).collect(Collectors.toList());
+
+    try {
+      return retrySupport.retry(() -> front50Service.getAllEntityTagsById(ids)
+        .stream()
+        .collect(Collectors.toMap(EntityTags::getId, Function.identity())), 10, 2000, false);
+    } catch (Exception e) {
+      log.error("Unable to retrieve existing tags from Front50, reason: {} (ids: {})", e.getMessage(), ids);
+      throw e;
+    }
   }
 
   private void addTagIdsIfMissing(List<EntityTags> entityTags, BulkUpsertEntityTagsAtomicOperationResult result) {

diff --git a/.../com/netflix/spinnaker/clouddriver/elasticsearch/ops/UpsertEntityTagsAtomicOperation.java b/.../com/netflix/spinnaker/clouddriver/elasticsearch/ops/UpsertEntityTagsAtomicOperation.java
@@ -22,21 +22,24 @@
 import com.netflix.spinnaker.clouddriver.elasticsearch.model.ElasticSearchEntityTagsProvider;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider;
+import com.netflix.spinnaker.kork.core.RetrySupport;
 
 import java.util.Collections;
 import java.util.List;
 
 public class UpsertEntityTagsAtomicOperation implements AtomicOperation<Void> {
-
+  private final RetrySupport retrySupport;
   private final Front50Service front50Service;
   private final AccountCredentialsProvider accountCredentialsProvider;
   private final ElasticSearchEntityTagsProvider entityTagsProvider;
   private final UpsertEntityTagsDescription entityTagsDescription;
 
-  public UpsertEntityTagsAtomicOperation(Front50Service front50Service,
+  public UpsertEntityTagsAtomicOperation(RetrySupport retrySupport,
+                                         Front50Service front50Service,
                                          AccountCredentialsProvider accountCredentialsProvider,
                                          ElasticSearchEntityTagsProvider entityTagsProvider,
                                          UpsertEntityTagsDescription tagEntityDescription) {
+    this.retrySupport = retrySupport;
     this.front50Service = front50Service;
     this.accountCredentialsProvider = accountCredentialsProvider;
     this.entityTagsProvider = entityTagsProvider;
@@ -49,6 +52,7 @@ public Void operate(List priorOutputs) {
     bulkDescription.entityTags = Collections.singletonList(entityTagsDescription);
 
     new BulkUpsertEntityTagsAtomicOperation(
+      retrySupport,
       front50Service,
       accountCredentialsProvider,
       entityTagsProvider,

diff --git a/...ddriver-elasticsearch/src/main/java/com/netflix/spinnaker/config/ElasticSearchConfig.java b/...ddriver-elasticsearch/src/main/java/com/netflix/spinnaker/config/ElasticSearchConfig.java
@@ -20,6 +20,7 @@
 import com.netflix.spinnaker.clouddriver.elasticsearch.ElasticSearchEntityTagger;
 import com.netflix.spinnaker.clouddriver.elasticsearch.model.ElasticSearchEntityTagsProvider;
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider;
+import com.netflix.spinnaker.kork.core.RetrySupport;
 import io.searchbox.client.JestClient;
 import io.searchbox.client.JestClientFactory;
 import io.searchbox.client.config.HttpClientConfig;
@@ -46,9 +47,12 @@ JestClient jestClient(ElasticSearchConfigProperties elasticSearchConfigPropertie
   }
 
   @Bean
-  ElasticSearchEntityTagger elasticSearchEntityTagger(Front50Service front50Service,
+  ElasticSearchEntityTagger elasticSearchEntityTagger(RetrySupport retrySupport,
+                                                      Front50Service front50Service,
                                                       AccountCredentialsProvider accountCredentialsProvider,
                                                       ElasticSearchEntityTagsProvider elasticSearchEntityTagsProvider) {
-    return new ElasticSearchEntityTagger(front50Service, accountCredentialsProvider, elasticSearchEntityTagsProvider);
+    return new ElasticSearchEntityTagger(
+      retrySupport, front50Service, accountCredentialsProvider, elasticSearchEntityTagsProvider
+    );
   }
 }
diff --git a/...oovy/com/netflix/spinnaker/clouddriver/elasticsearch/ElasticSearchEntityTaggerSpec.groovy b/...oovy/com/netflix/spinnaker/clouddriver/elasticsearch/ElasticSearchEntityTaggerSpec.groovy
@@ -79,7 +79,7 @@ class ElasticSearchEntityTaggerSpec extends Specification {
   void "should only mutate threadLocalTask if null"() {
     given:
     Task threadLocalTask = null
-    def serverGroupTagger = new ElasticSearchEntityTagger(null, null, null) {
+    def serverGroupTagger = new ElasticSearchEntityTagger(null, null, null, null) {
       @Override
       protected void run(DeleteEntityTagsAtomicOperation deleteEntityTagsAtomicOperation) {
         threadLocalTask = TaskRepository.threadLocalTask.get()

diff --git a/.../clouddriver/elasticsearch/converters/UpsertEntityTagsAtomicOperationConverterSpec.groovy b/.../clouddriver/elasticsearch/converters/UpsertEntityTagsAtomicOperationConverterSpec.groovy
@@ -27,7 +27,9 @@ class UpsertEntityTagsAtomicOperationConverterSpec extends Specification {
   def objectMapper = new ObjectMapper()
 
   @Subject
-  def converter = new UpsertEntityTagsAtomicOperationConverter(objectMapper, null, null, null)
+  def converter = new UpsertEntityTagsAtomicOperationConverter(
+    objectMapper, null, null, null, null
+  )
 
   @Unroll
   def "should set valueType when converting from Map -> Description"() {

diff --git a/...ix/spinnaker/clouddriver/elasticsearch/ops/BulkUpsertEntityTagsAtomicOperationSpec.groovy b/...ix/spinnaker/clouddriver/elasticsearch/ops/BulkUpsertEntityTagsAtomicOperationSpec.groovy
@@ -29,6 +29,7 @@ import com.netflix.spinnaker.clouddriver.model.EntityTags.EntityTagMetadata
 import com.netflix.spinnaker.clouddriver.model.EntityTags.EntityTagValueType
 import com.netflix.spinnaker.clouddriver.security.AccountCredentials
 import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider
+import com.netflix.spinnaker.kork.core.RetrySupport
 import spock.lang.Specification
 
 class BulkUpsertEntityTagsAtomicOperationSpec extends Specification {
@@ -38,6 +39,9 @@ class BulkUpsertEntityTagsAtomicOperationSpec extends Specification {
     getName() >> { return "test" }
   }
 
+  def retrySupport = Spy(RetrySupport) {
+    _ * sleep(_) >> { /* do nothing */ }
+  }
   def front50Service = Mock(Front50Service)
   def accountCredentialsProvider = Mock(AccountCredentialsProvider)
   def entityTagsProvider = Mock(ElasticSearchEntityTagsProvider)
@@ -51,7 +55,9 @@ class BulkUpsertEntityTagsAtomicOperationSpec extends Specification {
 
   def setup() {
     description = new BulkUpsertEntityTagsDescription()
-    operation = new BulkUpsertEntityTagsAtomicOperation(front50Service, accountCredentialsProvider, entityTagsProvider, description)
+    operation = new BulkUpsertEntityTagsAtomicOperation(
+      retrySupport, front50Service, accountCredentialsProvider, entityTagsProvider, description
+    )
   }
 
   void "should perform bulk operation"() {