refactor(clouddriver): Switch to typed models in CloudDriverService w…

…here possible (cherry picked from commit 93f143da265e77d82200a8ee17481cff66eade45)
ziggyw · Apr 28, 2021 · 9e797f5 · 9e797f5
1 parent c542c00
commit 9e797f5
Show file tree

Hide file tree

Showing 21 changed files with 91 additions and 65 deletions.
diff --git a/...naker/orca/clouddriver/tasks/servergroup/TitusInterestingHealthProviderNamesSupplier.java b/...naker/orca/clouddriver/tasks/servergroup/TitusInterestingHealthProviderNamesSupplier.java
@@ -18,6 +18,7 @@
 
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
+import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup;
 import com.netflix.spinnaker.orca.kato.pipeline.support.SourceResolver;
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData;
 import java.util.Arrays;
@@ -69,7 +70,7 @@ public List<String> process(String cloudProvider, StageExecution stage) {
         String serverGroupName =
             source.getServerGroupName() != null ? source.getServerGroupName() : source.getAsgName();
 
-        Map<String, Object> serverGroup =
+        ServerGroup serverGroup =
             cloudDriverService.getServerGroupFromCluster(
                 stageData.getApplication(),
                 source.getAccount(),
@@ -78,12 +79,12 @@ public List<String> process(String cloudProvider, StageExecution stage) {
                 source.getRegion(),
                 cloudProvider);
 
-        Map titusServerGroupLabels = (Map) serverGroup.get("labels");
+        Map<String, String> titusServerGroupLabels = serverGroup.getLabels();
 
         if (titusServerGroupLabels != null
             && titusServerGroupLabels.containsKey(INTERESTING_HEALTH_PROVIDER_NAMES)) {
           String healthProviderNames =
-              (String) titusServerGroupLabels.get(INTERESTING_HEALTH_PROVIDER_NAMES);
+              titusServerGroupLabels.get(INTERESTING_HEALTH_PROVIDER_NAMES);
           return Arrays.asList(healthProviderNames.split(","));
         }
       }

diff --git a/...orca/clouddriver/tasks/servergroup/TitusInterestingHealthProviderNamesSupplierSpec.groovy b/...orca/clouddriver/tasks/servergroup/TitusInterestingHealthProviderNamesSupplierSpec.groovy
@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup
 
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
+import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup
 import com.netflix.spinnaker.orca.kato.pipeline.support.SourceResolver
 import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl
 import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl
@@ -52,12 +53,11 @@ class TitusInterestingHealthProviderNamesSupplierSpec extends Specification {
   def "should process interestingHealthNames by inspecting labels on titus serverGroup"() {
     given:
     def stage = new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "createServerGroup", stageContext)
-    def response = [
-      application: "app",
+    def response = new ServerGroup(
       region: "region",
       account: "test",
       labels: labels
-    ]
+    )
 
     and:
     1 * cloudDriverService.getServerGroupFromCluster(*_) >> response

diff --git a/...nnaker/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupCapacityStage.groovy b/...nnaker/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupCapacityStage.groovy
@@ -124,7 +124,7 @@ class ApplySourceServerGroupCapacityStage implements StageDefinitionBuilder, For
     }
 
     retrySupport.retry({
-      return oortService.getEntityTagsTyped([
+      return oortService.getEntityTags([
         ("tag:${PINNED_CAPACITY_TAG}".toString()): "*",
         entityId                                 : stage.context.serverGroupName,
         account                                  : stage.context.credentials,

diff --git a/...m/netflix/spinnaker/orca/kato/tasks/rollingpush/DetermineTerminationCandidatesTask.groovy b/...m/netflix/spinnaker/orca/kato/tasks/rollingpush/DetermineTerminationCandidatesTask.groovy
@@ -22,6 +22,8 @@ import com.netflix.spinnaker.orca.api.pipeline.TaskResult
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
+import com.netflix.spinnaker.orca.clouddriver.model.Instance
+import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup
 import com.netflix.spinnaker.orca.kato.pipeline.support.StageData
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
@@ -40,9 +42,9 @@ class DetermineTerminationCandidatesTask implements Task {
   @Override
   TaskResult execute(@Nonnull StageExecution stage) {
     def stageData = stage.mapTo(StageData)
-    def serverGroup = cloudDriverService.getServerGroupFromCluster(stageData.application, stageData.account, stageData.cluster, stage.context.asgName, stage.context.region, stage.context.cloudProvider ?: 'aws')
+    ServerGroup serverGroup = cloudDriverService.getServerGroupFromCluster(stageData.application, stageData.account, stageData.cluster, stage.context.asgName, stage.context.region, stage.context.cloudProvider ?: 'aws')
     boolean ascending = stage.context.termination?.order != 'newest'
-    def serverGroupInstances = serverGroup.instances.sort { ascending ? it.launchTime : -it.launchTime }
+    List<Instance> serverGroupInstances = serverGroup.instances.sort { ascending ? it.launchTime : -it.launchTime }
     // need to use id instead of instanceIds for titus as the titus API doesn't yet support this yet.
     def knownInstanceIds = getServerGroupInstanceIdCollector(stage)
         .flatMap { it.collect(serverGroupInstances) }

diff --git a/...-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/CloudDriverService.java b/...-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/CloudDriverService.java
@@ -67,7 +67,7 @@ public ServerGroup getServerGroup(ServerGroupDescriptor descriptor) {
     return readBody(response, ServerGroup.class);
   }
 
-  public Map<String, Object> getServerGroupFromCluster(
+  public ServerGroup getServerGroupFromCluster(
       String app,
       String account,
       String cluster,
@@ -77,15 +77,16 @@ public Map<String, Object> getServerGroupFromCluster(
     Response response =
         oortService.getServerGroupFromCluster(
             app, account, cluster, serverGroup, region, cloudProvider);
-    return readBody(response, JSON_MAP);
+    return readBody(response, ServerGroup.class);
   }
 
-  public List<Map<String, Object>> getEntityTags(
+  public List<EntityTags> getEntityTags(
       String cloudProvider, String entityType, String entityId, String account, String region) {
-    return oortService.getEntityTags(cloudProvider, entityType, entityId, account, region);
+    var response = oortService.getEntityTags(cloudProvider, entityType, entityId, account, region);
+    return objectMapper.convertValue(response, ENTITY_TAGS);
   }
 
-  public List<EntityTags> getEntityTagsTyped(Map parameters) {
+  public List<EntityTags> getEntityTags(Map parameters) {
     List<Map> response = oortService.getEntityTags(parameters);
     return objectMapper.convertValue(response, ENTITY_TAGS);
   }

diff --git a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/model/EntityTags.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/model/EntityTags.java
@@ -25,8 +25,10 @@ public class EntityTags {
   public EntityRef entityRef;
 
   public static class Tag {
+    public String namespace;
     public String name;
     public Object value;
+    public ValueType valueType;
   }
 
   public static class EntityRef {
@@ -38,4 +40,9 @@ public static class EntityRef {
     public String entityType;
     public String entityId;
   }
+
+  public enum ValueType {
+    literal, // number or string
+    object // map
+  }
 }
diff --git a/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/model/ServerGroup.java b/orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/model/ServerGroup.java
@@ -33,6 +33,8 @@ public class ServerGroup {
 
   public List<Instance> instances;
 
+  public Map<String, String> labels;
+
   /**
    * For some reason TargetServerGroup allows looking at 2 properties: {@link
    * TargetServerGroup#isDisabled()} } *

diff --git a/...pinnaker/orca/clouddriver/pipeline/servergroup/rollback/PreviousImageRollbackSupport.java b/...pinnaker/orca/clouddriver/pipeline/servergroup/rollback/PreviousImageRollbackSupport.java
@@ -21,6 +21,7 @@
 import com.netflix.spinnaker.kork.core.RetrySupport;
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
 import com.netflix.spinnaker.orca.clouddriver.FeaturesService;
+import com.netflix.spinnaker.orca.clouddriver.model.EntityTags;
 import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup.BuildInfo;
 import java.util.Collections;
 import java.util.List;
@@ -51,7 +52,7 @@ public PreviousImageRollbackSupport(
 
   public ImageDetails getImageDetailsFromEntityTags(
       String cloudProvider, String credentials, String region, String serverGroupName) {
-    List<Map<String, Object>> entityTags = null;
+    List<EntityTags> entityTags = null;
 
     try {
       entityTags =
@@ -83,11 +84,11 @@ public ImageDetails getImageDetailsFromEntityTags(
       return null;
     }
 
-    List<Map> tags = (List<Map>) entityTags.get(0).get("tags");
+    List<EntityTags.Tag> tags = entityTags.get(0).tags;
     PreviousServerGroup previousServerGroup =
         tags.stream()
-            .filter(t -> "spinnaker:metadata".equalsIgnoreCase((String) t.get("name")))
-            .map(t -> (Map<String, Object>) ((Map) t.get("value")).get("previousServerGroup"))
+            .filter(t -> "spinnaker:metadata".equalsIgnoreCase(t.name))
+            .map(t -> (Map<String, Object>) ((Map) t.value).get("previousServerGroup"))
             .filter(Objects::nonNull)
             .map(m -> objectMapper.convertValue(m, PreviousServerGroup.class))
             .findFirst()

diff --git a/...main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java b/...main/java/com/netflix/spinnaker/orca/clouddriver/pollers/EphemeralServerGroupsPoller.java
@@ -205,7 +205,7 @@ private List<EphemeralServerGroupTag> fetchEphemeralServerGroupTags() {
               () ->
                   retrySupport.retry(
                       () ->
-                          cloudDriverService.getEntityTagsTyped(
+                          cloudDriverService.getEntityTags(
                               Map.of("tag:" + TTL_TAG, "*", "entityType", "servergroup")),
                       15,
                       2000,

diff --git a/.../java/com/netflix/spinnaker/orca/clouddriver/pollers/RestorePinnedServerGroupsPoller.java b/.../java/com/netflix/spinnaker/orca/clouddriver/pollers/RestorePinnedServerGroupsPoller.java
@@ -203,7 +203,7 @@ public List<PinnedServerGroupTag> fetchPinnedServerGroupTags() {
             () ->
                 retrySupport.retry(
                     () ->
-                        cloudDriverService.getEntityTagsTyped(
+                        cloudDriverService.getEntityTags(
                             Map.of("tag:" + PINNED_CAPACITY_TAG, "*", "entityType", "servergroup")),
                     15,
                     2000,

diff --git a/...iver/src/main/java/com/netflix/spinnaker/orca/kato/tasks/rollingpush/CleanUpTagsTask.java b/...iver/src/main/java/com/netflix/spinnaker/orca/kato/tasks/rollingpush/CleanUpTagsTask.java
@@ -23,6 +23,8 @@
 import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
 import com.netflix.spinnaker.orca.clouddriver.KatoService;
+import com.netflix.spinnaker.orca.clouddriver.model.EntityTags;
+import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup;
 import com.netflix.spinnaker.orca.clouddriver.model.TaskId;
 import com.netflix.spinnaker.orca.clouddriver.utils.CloudProviderAware;
 import com.netflix.spinnaker.orca.clouddriver.utils.MonikerHelper;
@@ -58,7 +60,7 @@ public TaskResult execute(StageExecution stage) {
           Optional.ofNullable(source.getServerGroupName()).orElse(source.getAsgName());
       String cloudProvider = getCloudProvider(stage);
 
-      Map<String, Object> serverGroup =
+      ServerGroup serverGroup =
           cloudDriverService.getServerGroupFromCluster(
               monikerHelper.getAppNameFromStage(stage, serverGroupName),
               source.getAccount(),
@@ -67,9 +69,9 @@ public TaskResult execute(StageExecution stage) {
               source.getRegion(),
               cloudProvider);
 
-      String imageId = (String) ((Map) serverGroup.get("launchConfig")).get("imageId");
+      String imageId = (String) serverGroup.getLaunchConfig().get("imageId");
 
-      List<Map<String, Object>> tags =
+      List<EntityTags> tags =
           cloudDriverService.getEntityTags(
               cloudProvider,
               "servergroup",
@@ -79,10 +81,10 @@ public TaskResult execute(StageExecution stage) {
 
       List<String> tagsToDelete =
           tags.stream()
-              .flatMap(entityTag -> ((List<Map>) entityTag.get("tags")).stream())
-              .filter(tag -> "astrid_rules".equals(tag.get("namespace")))
+              .flatMap(entityTag -> entityTag.tags.stream())
+              .filter(tag -> "astrid_rules".equals(tag.namespace))
               .filter(hasNonMatchingImageId(imageId))
-              .map(t -> (String) t.get("name"))
+              .map(t -> t.name)
               .collect(Collectors.toList());
 
       log.info("found tags to delete {}", tagsToDelete);
@@ -91,7 +93,7 @@ public TaskResult execute(StageExecution stage) {
       }
 
       // All IDs should be the same; use the first one
-      String entityId = (String) tags.get(0).get("id");
+      String entityId = tags.get(0).id;
 
       TaskId taskId =
           katoService.requestOperations(cloudProvider, operations(entityId, tagsToDelete));
@@ -116,12 +118,12 @@ public TaskResult execute(StageExecution stage) {
     }
   }
 
-  private Predicate<Map> hasNonMatchingImageId(String imageId) {
+  private Predicate<EntityTags.Tag> hasNonMatchingImageId(String imageId) {
     return tag -> {
-      if (!"object".equals(tag.get("valueType"))) {
+      if (EntityTags.ValueType.object != tag.valueType) {
         return false;
       }
-      Map value = ((Map) tag.getOrDefault("value", Collections.EMPTY_MAP));
+      Map value = tag.value instanceof Map ? (Map) tag.value : Collections.emptyMap();
       return value.containsKey("imageId") && !value.get("imageId").equals(imageId);
     };
   }

diff --git a/...ava/com/netflix/spinnaker/orca/kato/tasks/rollingpush/ServerGroupInstanceIdCollector.java b/...ava/com/netflix/spinnaker/orca/kato/tasks/rollingpush/ServerGroupInstanceIdCollector.java
@@ -16,8 +16,8 @@
 package com.netflix.spinnaker.orca.kato.tasks.rollingpush;
 
 import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
+import com.netflix.spinnaker.orca.clouddriver.model.Instance;
 import java.util.List;
-import java.util.Map;
 import org.springframework.stereotype.Component;
 
 @Component
@@ -26,7 +26,7 @@ public interface ServerGroupInstanceIdCollector {
 
   boolean supports(String cloudProvider);
 
-  List<String> collect(List<Map<String, Object>> serverGroupInstances);
+  List<String> collect(List<Instance> serverGroupInstances);
 
-  String one(Map<String, Object> serverGroupInstance);
+  String one(Instance serverGroupInstance);
 }
diff --git a/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/ModelUtils.groovy b/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/ModelUtils.groovy
@@ -1,6 +1,7 @@
 package com.netflix.spinnaker.orca.clouddriver
 
 import com.netflix.spinnaker.orca.clouddriver.model.Cluster
+import com.netflix.spinnaker.orca.clouddriver.model.EntityTags
 import com.netflix.spinnaker.orca.clouddriver.model.Instance
 import com.netflix.spinnaker.orca.clouddriver.model.Instance.InstanceInfo
 import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup
@@ -23,4 +24,8 @@ class ModelUtils {
   static Cluster cluster(cluster) {
     OrcaObjectMapper.instance.convertValue(cluster, Cluster)
   }
+
+  static EntityTags tags(tags) {
+    OrcaObjectMapper.instance.convertValue(tags, EntityTags)
+  }
 }
diff --git a/...er/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupCapacityStageSpec.groovy b/...er/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupCapacityStageSpec.groovy
@@ -59,7 +59,7 @@ class ApplySourceServerGroupCapacityStageSpec extends Specification {
 
     then:
     1 * featuresService.areEntityTagsAvailable() >> { return false }
-    0 * cloudDriverService.getEntityTagsTyped(_)
+    0 * cloudDriverService.getEntityTags(_)
 
     afterStages.isEmpty()
   }
@@ -72,7 +72,7 @@ class ApplySourceServerGroupCapacityStageSpec extends Specification {
 
     then:
     1 * featuresService.areEntityTagsAvailable() >> { return true }
-    1 * cloudDriverService.getEntityTagsTyped(_) >> { return [] }
+    1 * cloudDriverService.getEntityTags(_) >> { return [] }
 
     afterStages.isEmpty()
   }
@@ -87,7 +87,7 @@ class ApplySourceServerGroupCapacityStageSpec extends Specification {
     notThrown(RuntimeException)
 
     1 * featuresService.areEntityTagsAvailable() >> { throw new RuntimeException("An Exception!") }
-    0 * cloudDriverService.getEntityTagsTyped(_)
+    0 * cloudDriverService.getEntityTags(_)
 
     afterStages.isEmpty()
   }
@@ -107,7 +107,7 @@ class ApplySourceServerGroupCapacityStageSpec extends Specification {
     notThrown(RuntimeException)
 
     1 * featuresService.areEntityTagsAvailable() >> { return true }
-    0 * cloudDriverService.getEntityTagsTyped(_)
+    0 * cloudDriverService.getEntityTags(_)
 
     afterStages.isEmpty()
 
@@ -129,7 +129,7 @@ class ApplySourceServerGroupCapacityStageSpec extends Specification {
 
     then:
     1 * featuresService.areEntityTagsAvailable() >> { return true }
-    1 * cloudDriverService.getEntityTagsTyped([
+    1 * cloudDriverService.getEntityTags([
       "tag:spinnaker:pinned_capacity": "*",
       "entityId"                     : "app-stack-details-v001",
       "account"                      : "test",

diff --git a/...spinnaker/orca/clouddriver/pipeline/servergroup/rollback/PreviousImageRollbackSpec.groovy b/...spinnaker/orca/clouddriver/pipeline/servergroup/rollback/PreviousImageRollbackSpec.groovy
@@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.kork.core.RetrySupport
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.clouddriver.FeaturesService
+import com.netflix.spinnaker.orca.clouddriver.ModelUtils
 import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.CloneServerGroupStage
 import com.netflix.spinnaker.orca.api.pipeline.SyntheticStageOwner
 import spock.lang.Specification
@@ -73,7 +74,7 @@ class PreviousImageRollbackSpec extends Specification {
     then:
     (imageName ? 0 : 1) * cloudDriverService.getEntityTags(*_) >> {
       // should only call `getEntityTags()` if an image was not explicitly provided to the rollback
-      return [[
+      return [ModelUtils.tags([
                 tags: [
                   [
                     name : "spinnaker:metadata",
@@ -85,7 +86,7 @@ class PreviousImageRollbackSpec extends Specification {
                     ]
                   ]
                 ]
-              ]]
+              ])]
 
     }
 

diff --git a/...er/orca/clouddriver/pipeline/servergroup/rollback/PreviousImageRollbackSupportSpec.groovy b/...er/orca/clouddriver/pipeline/servergroup/rollback/PreviousImageRollbackSupportSpec.groovy
@@ -20,6 +20,7 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.kork.core.RetrySupport
 import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
 import com.netflix.spinnaker.orca.clouddriver.FeaturesService
+import com.netflix.spinnaker.orca.clouddriver.model.EntityTags
 import spock.lang.Specification
 import spock.lang.Subject;
 
@@ -42,8 +43,8 @@ class PreviousImageRollbackSupportSpec extends Specification {
     1 * featuresService.areEntityTagsAvailable() >> { return true }
     1 * cloudDriverService.getEntityTags(*_) >> {
       return [
-        [id: "1"],
-        [id: "2"]
+        new EntityTags(id: "1"),
+        new EntityTags(id: "2")
       ]
     }