fix(provider/ecs): ECS Caching Agents - Account/Region awareness (#2315)

* Caching agents are now account and region aware. * getCluster account aware. * Formatting fixed.
spinnaker · Jan 30, 2018 · 23abe22 · 23abe22
1 parent 51d6a22
commit 23abe22
Show file tree

Hide file tree

Showing 14 changed files with 126 additions and 39 deletions.
diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/Keys.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/Keys.java
@@ -61,6 +61,10 @@ public Map<String, String> parseKey(String key) {
 
   @Override
   public Boolean canParseType(String type) {
+    return canParse(type);
+  }
+
+  private static Boolean canParse(String type) {
     for (Namespace key : Namespace.values()) {
       if (key.toString().equals(type)) {
         return true;
@@ -81,6 +85,12 @@ public static Map<String, String> parse(String key) {
     result.put("type", parts[1]);
     result.put("account", parts[2]);
 
+    if(!canParse(parts[1]) && parts[1].equals(HEALTH.getNs())){
+      result.put("region", parts[3]);
+      result.put("taskId", parts[4]);
+      return result;
+    }
+
 
     Namespace namespace = Namespace.valueOf(CaseFormat.LOWER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, parts[1]));
 

diff --git a/...n/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsLoadbalancerCacheClient.java b/...n/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsLoadbalancerCacheClient.java
@@ -46,13 +46,24 @@ public EcsLoadbalancerCacheClient(Cache cacheView, ObjectMapper objectMapper) {
     this.objectMapper = objectMapper;
   }
 
+  public List<EcsLoadBalancerCache> find(String account, String region) {
+    Set<Map<String, Object>> loadbalancerAttributes = fetchFromCache(account, region);
+    return convertToLoadbalancer(loadbalancerAttributes);
+  }
+
   public List<EcsLoadBalancerCache> findAll() {
-    String searchKey = Keys.getLoadBalancerKey("*", "*", "*", "*", "*") + "*";
-    Collection<String> loadbalancerKeys = cacheView.filterIdentifiers(LOAD_BALANCERS.getNs(), searchKey);
+    return find("*", "*");
+  }
 
-    Set<Map<String, Object>> loadbalancerAttributes = fetchLoadBalancerAttributes(loadbalancerKeys);
+  private Set<Map<String, Object>> fetchFromCache(String account, String region) {
+    String accountFilter = account != null ? account : "*";
+    String regionFilter = region != null ? region : "*";
 
-    return convertToLoadbalancer(loadbalancerAttributes);
+    String searchKey = Keys.getLoadBalancerKey("*", accountFilter, regionFilter, "*", "*") + "*";
+
+    Collection<String> loadbalancerKeys = cacheView.filterIdentifiers(LOAD_BALANCERS.getNs(), searchKey);
+
+    return fetchLoadBalancerAttributes(loadbalancerKeys);
   }
 
   public Set<EcsLoadBalancerCache> findWithTargetGroups(Set<String> targetGroups) {

diff --git a/...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/AbstractEcsCachingAgent.java b/...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/AbstractEcsCachingAgent.java
@@ -28,6 +28,7 @@
 import com.netflix.spinnaker.cats.cache.CacheData;
 import com.netflix.spinnaker.cats.provider.ProviderCache;
 import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider;
+import com.netflix.spinnaker.clouddriver.ecs.cache.Keys;
 import com.netflix.spinnaker.clouddriver.ecs.provider.EcsProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -42,6 +43,7 @@
 
 import static com.netflix.spinnaker.cats.agent.AgentDataType.Authority.AUTHORITATIVE;
 import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.ECS_CLUSTERS;
+import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.IAM_ROLE;
 
 abstract class AbstractEcsCachingAgent<T> implements CachingAgent {
   private final Logger log = LoggerFactory.getLogger(getClass());
@@ -96,6 +98,8 @@ public CacheResult loadData(ProviderCache providerCache) {
    */
   Set<String> getClusters(AmazonECS ecs, ProviderCache providerCache) {
     Set<String> clusters = providerCache.getAll(ECS_CLUSTERS.toString()).stream()
+      .filter(cacheData ->  cacheData.getAttributes().get("region").equals(region) &&
+                            cacheData.getAttributes().get("account").equals(accountName))
       .map(cacheData -> (String) cacheData.getAttributes().get("clusterArn"))
       .collect(Collectors.toSet());
 
@@ -140,8 +144,10 @@ CacheResult buildCacheResult(String authoritativeKeyName, List<T> items, Provide
 
     Map<String, Collection<CacheData>> dataMap = generateFreshData(items);
 
+    //Old keys can come from different account/region, filter them to the current account/region.
     Set<String> oldKeys = providerCache.getAll(authoritativeKeyName).stream()
       .map(CacheData::getId)
+      .filter(key -> keyAccountRegionFilter(authoritativeKeyName, key))
       .collect(Collectors.toSet());
 
     Map<String, Collection<String>> evictions = computeEvictableData(dataMap.get(authoritativeKeyName), oldKeys);
@@ -159,24 +165,35 @@ CacheResult buildCacheResult(String authoritativeKeyName, List<T> items, Provide
    * @return Key collection associated to the key namespace the the caching agent is authoritative of.
    */
   private Map<String, Collection<String>> computeEvictableData(Collection<CacheData> newData, Collection<String> oldKeys) {
+    //New data can only come from the current account and region, no need to filter.
     Set<String> newKeys = newData.stream()
       .map(CacheData::getId)
       .collect(Collectors.toSet());
+
     Set<String> evictedKeys = oldKeys.stream()
       .filter(oldKey -> !newKeys.contains(oldKey))
       .collect(Collectors.toSet());
 
     Map<String, Collection<String>> evictionsByKey = new HashMap<>();
     evictionsByKey.put(getAuthoritativeKeyName(), evictedKeys);
+
     return evictionsByKey;
   }
 
+  protected boolean keyAccountRegionFilter(String authoritativeKeyName, String key) {
+    Map<String, String> keyParts = Keys.parse(key);
+    return keyParts != null &&
+           keyParts.get("account").equals(accountName) &&
+           //IAM role keys are not region specific, so it will be true. The region will be checked of other keys.
+           (authoritativeKeyName.equals(IAM_ROLE.ns) || keyParts.get("region").equals(region));
+  }
+
   /**
    * This method is to be overridden in order to add extra evictions.
    * @param evictions The existing eviction map.
    * @return Eviction map with addtional keys.
    */
-  protected Map<String, Collection<String>> addExtraEvictions(Map<String, Collection<String>> evictions){
+  protected Map<String, Collection<String>> addExtraEvictions(Map<String, Collection<String>> evictions) {
     return evictions;
   }
 }
diff --git a/...a/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ContainerInstanceCachingAgent.java b/...a/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ContainerInstanceCachingAgent.java
@@ -18,6 +18,7 @@
 
 import com.amazonaws.auth.AWSCredentialsProvider;
 import com.amazonaws.services.ecs.AmazonECS;
+import com.amazonaws.services.ecs.model.Attribute;
 import com.amazonaws.services.ecs.model.ContainerInstance;
 import com.amazonaws.services.ecs.model.DescribeContainerInstancesRequest;
 import com.amazonaws.services.ecs.model.ListContainerInstancesRequest;
@@ -56,7 +57,7 @@ public ContainerInstanceCachingAgent(String accountName, String region, AmazonCl
 
   @Override
   public String getAgentType() {
-    return ContainerInstanceCachingAgent.class.getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override
@@ -111,10 +112,15 @@ protected Map<String, Collection<CacheData>> generateFreshData(Collection<Contai
     return dataMap;
   }
 
-  public static  Map<String, Object> convertContainerInstanceToAttributes(ContainerInstance containerInstance){
+  public static Map<String, Object> convertContainerInstanceToAttributes(ContainerInstance containerInstance) {
     Map<String, Object> attributes = new HashMap<>();
     attributes.put("containerInstanceArn", containerInstance.getContainerInstanceArn());
     attributes.put("ec2InstanceId", containerInstance.getEc2InstanceId());
+    for (Attribute containerAttribute : containerInstance.getAttributes()) {
+      if (containerAttribute.getName().equals("ecs.availability-zone")) {
+        attributes.put("availabilityZone", containerAttribute.getValue());
+      }
+    }
     return attributes;
   }
 }
diff --git a/...com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java b/...com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java
@@ -92,7 +92,10 @@ public CacheResult loadData(ProviderCache providerCache) {
     Collection<CacheData> newData = newDataMap.get(ALARMS.toString());
 
     Set<String> oldKeys = providerCache.getAll(ALARMS.toString()).stream()
-      .map(CacheData::getId).collect(Collectors.toSet());
+      .map(CacheData::getId)
+      .filter(this::keyAccountRegionFilter)
+      .collect(Collectors.toSet());
+
     Map<String, Collection<String>> evictionsByKey = computeEvictableData(newData, oldKeys);
 
     return new DefaultCacheResult(newDataMap, evictionsByKey);
@@ -143,9 +146,16 @@ Set<MetricAlarm> fetchMetricAlarms(AmazonCloudWatch cloudWatch) {
     return cacheableMetricAlarm;
   }
 
+  private boolean keyAccountRegionFilter(String key) {
+    Map<String, String> keyParts = Keys.parse(key);
+    return keyParts != null &&
+      keyParts.get("account").equals(accountName) &&
+      keyParts.get("region").equals(region);
+  }
+
   @Override
   public String getAgentType() {
-    return getClass().getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override

diff --git a/...ain/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsClusterCachingAgent.java b/...ain/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsClusterCachingAgent.java
@@ -53,7 +53,7 @@ public EcsClusterCachingAgent(String accountName, String region, AmazonClientPro
 
   @Override
   public String getAgentType() {
-    return EcsClusterCachingAgent.class.getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override

diff --git a/...c/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/IamRoleCachingAgent.java b/...c/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/IamRoleCachingAgent.java
@@ -89,13 +89,14 @@ public CacheResult loadData(ProviderCache providerCache) {
     Collection<CacheData> newData = newDataMap.get(IAM_ROLE.toString());
 
     Set<String> oldKeys = providerCache.getAll(IAM_ROLE.toString()).stream()
-      .map(cache -> cache.getId()).collect(Collectors.toSet());
+      .map(CacheData::getId)
+      .filter(this::keyAccountFilter)
+      .collect(Collectors.toSet());
     Map<String, Collection<String>> evictionsByKey = computeEvictableData(newData, oldKeys);
 
     logUpcomingActions(newDataMap, evictionsByKey);
 
-    DefaultCacheResult cacheResult = new DefaultCacheResult(newDataMap, evictionsByKey);
-    return cacheResult;
+    return new DefaultCacheResult(newDataMap, evictionsByKey);
   }
 
   private void logUpcomingActions(Map<String, Collection<CacheData>> newDataMap, Map<String, Collection<String>> evictionsByKey) {
@@ -116,7 +117,9 @@ private void logUpcomingActions(Map<String, Collection<CacheData>> newDataMap, M
 
   private Map<String, Collection<String>> computeEvictableData(Collection<CacheData> newData, Collection<String> oldKeys) {
 
-    Set<String> newKeys = newData.stream().map(newKey -> newKey.getId()).collect(Collectors.toSet());
+    Set<String> newKeys = newData.stream()
+      .map(CacheData::getId)
+      .collect(Collectors.toSet());
 
     Set<String> evictedKeys = new HashSet<>();
     for (String oldKey : oldKeys) {
@@ -146,7 +149,7 @@ Map<String, Collection<CacheData>> generateFreshData(Set<IamRole> cacheableRoles
   }
 
   Set<IamRole> fetchIamRoles(AmazonIdentityManagement iam, String accountName) {
-    Set<IamRole> cacheableRoles = new HashSet();
+    Set<IamRole> cacheableRoles = new HashSet<>();
     String marker = null;
     do {
       ListRolesRequest request = new ListRolesRequest();
@@ -175,10 +178,16 @@ Set<IamRole> fetchIamRoles(AmazonIdentityManagement iam, String accountName) {
 
     return cacheableRoles;
   }
+
+  private boolean keyAccountFilter(String key) {
+    Map<String, String> keyParts = Keys.parse(key);
+    return keyParts != null &&
+      keyParts.get("account").equals(accountName);
+  }
 
   @Override
   public String getAgentType() {
-    return IamRoleCachingAgent.class.getSimpleName();
+    return accountName + "/" + getClass().getSimpleName();
   }
 
   @Override

diff --git a/...ava/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ScalableTargetsCachingAgent.java b/...ava/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ScalableTargetsCachingAgent.java
@@ -89,7 +89,10 @@ public CacheResult loadData(ProviderCache providerCache) {
     Collection<CacheData> newData = newDataMap.get(SCALABLE_TARGETS.toString());
 
     Set<String> oldKeys = providerCache.getAll(SCALABLE_TARGETS.toString()).stream()
-      .map(CacheData::getId).collect(Collectors.toSet());
+      .map(CacheData::getId)
+      .filter(this::keyAccountRegionFilter)
+      .collect(Collectors.toSet());
+
     Map<String, Collection<String>> evictionsByKey = computeEvictableData(newData, oldKeys);
 
     return new DefaultCacheResult(newDataMap, evictionsByKey);
@@ -140,9 +143,16 @@ Set<ScalableTarget> fetchScalableTargets(AWSApplicationAutoScaling autoScalingCl
     return scalableTargets;
   }
 
+  private boolean keyAccountRegionFilter(String key) {
+    Map<String, String> keyParts = Keys.parse(key);
+    return keyParts != null &&
+      keyParts.get("account").equals(accountName) &&
+      keyParts.get("region").equals(region);
+  }
+
   @Override
   public String getAgentType() {
-    return getClass().getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override

diff --git a/...c/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ServiceCachingAgent.java b/...c/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/ServiceCachingAgent.java
@@ -83,7 +83,7 @@ public static Map<String, Object> convertServiceToAttributes(String accountName,
 
   @Override
   public String getAgentType() {
-    return ServiceCachingAgent.class.getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override

diff --git a/.../src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskCachingAgent.java b/.../src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskCachingAgent.java
@@ -69,7 +69,7 @@ public Collection<AgentDataType> getProvidedDataTypes() {
 
   @Override
   public String getAgentType() {
-    return TaskCachingAgent.class.getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override
@@ -104,7 +104,8 @@ public Collection<Map> pendingOnDemandRequests(ProviderCache providerCache) {
     for (CacheData onDemand : allOnDemand) {
       Map<String, String> parsedKey = Keys.parse(onDemand.getId());
       if (parsedKey != null && parsedKey.get("type") != null &&
-        (parsedKey.get("type").equals(SERVICES.toString()) || parsedKey.get("type").equals(TASKS.toString()))) {
+        (parsedKey.get("type").equals(SERVICES.toString()) || parsedKey.get("type").equals(TASKS.toString()) &&
+          parsedKey.get("account").equals(accountName) && parsedKey.get("region").equals(region))) {
 
         parsedKey.put("type", "serverGroup");
         parsedKey.put("serverGroup", parsedKey.get("serviceName"));
@@ -165,7 +166,7 @@ protected Map<String, Collection<CacheData>> generateFreshData(Collection<Task>
     return dataMap;
   }
 
-  public static Map<String, Object> convertTaskToAttributes(Task task){
+  public static Map<String, Object> convertTaskToAttributes(Task task) {
     String taskId = StringUtils.substringAfterLast(task.getTaskArn(), "/");
 
     Map<String, Object> attributes = new HashMap<>();

diff --git a/...java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskDefinitionCachingAgent.java b/...java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskDefinitionCachingAgent.java
@@ -52,18 +52,24 @@ public class TaskDefinitionCachingAgent extends AbstractEcsOnDemandAgent<TaskDef
   private static final Collection<AgentDataType> types = Collections.unmodifiableCollection(Arrays.asList(
     AUTHORITATIVE.forType(TASK_DEFINITIONS.toString())
   ));
-  private ObjectMapper mapper;
   private final Logger log = LoggerFactory.getLogger(getClass());
 
-  public TaskDefinitionCachingAgent(String accountName, String region, AmazonClientProvider amazonClientProvider, AWSCredentialsProvider awsCredentialsProvider, Registry registry, ObjectMapper mapper) {
+  private ObjectMapper objectMapper;
+
+  public TaskDefinitionCachingAgent(String accountName, String region,
+                                    AmazonClientProvider amazonClientProvider,
+                                    AWSCredentialsProvider awsCredentialsProvider,
+                                    Registry registry,
+                                    ObjectMapper objectMapper) {
     super(accountName, region, amazonClientProvider, awsCredentialsProvider, registry);
-    this.mapper = mapper;
+    this.objectMapper = objectMapper;
   }
 
   public static Map<String, Object> convertTaskDefinitionToAttributes(TaskDefinition taskDefinition) {
     Map<String, Object> attributes = new HashMap<>();
     attributes.put("taskDefinitionArn", taskDefinition.getTaskDefinitionArn());
     attributes.put("containerDefinitions", taskDefinition.getContainerDefinitions());
+    attributes.put("taskRoleArn", taskDefinition.getTaskRoleArn());
     return attributes;
   }
 
@@ -74,13 +80,14 @@ public Collection<AgentDataType> getProvidedDataTypes() {
 
   @Override
   public String getAgentType() {
-    return TaskDefinitionCachingAgent.class.getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override
   protected List<TaskDefinition> getItems(AmazonECS ecs, ProviderCache providerCache) {
     List<TaskDefinition> taskDefinitionList = new LinkedList<>();
     Set<String> cachedArns = providerCache.getIdentifiers(TASK_DEFINITIONS.toString()).stream()
+      .filter(id -> keyAccountRegionFilter(TASK_DEFINITIONS.toString(), id))
       .map(id -> {
         Map<String, String> keyParts = Keys.parse(id);
         return keyParts.get("taskDefinitionArn");
@@ -123,8 +130,8 @@ protected List<TaskDefinition> getItems(AmazonECS ecs, ProviderCache providerCac
   }
 
   private Set<TaskDefinition> retrieveFromCache(Set<String> taskDefArns, ProviderCache providerCache) {
+    TaskDefinitionCacheClient taskDefinitionCacheClient = new TaskDefinitionCacheClient(providerCache, objectMapper);
     Set<TaskDefinition> taskDefs = new HashSet<>();
-    TaskDefinitionCacheClient taskDefinitionCacheClient = new TaskDefinitionCacheClient(providerCache, mapper);
 
     for (String taskDefArn : taskDefArns) {
       String key = Keys.getTaskDefinitionKey(accountName, region, taskDefArn);

diff --git a/...ain/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskHealthCachingAgent.java b/...ain/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/TaskHealthCachingAgent.java
@@ -102,7 +102,7 @@ protected List<TaskHealth> getItems(AmazonECS ecs, ProviderCache providerCache)
     serviceEvicitions = new LinkedList<>();
     taskDefEvicitions = new LinkedList<>();
 
-    Collection<Task> tasks = taskCacheClient.getAll();
+    Collection<Task> tasks = taskCacheClient.getAll(accountName, region);
     if (tasks != null) {
       for (Task task : tasks) {
         String containerInstanceCacheKey = Keys.getContainerInstanceKey(accountName, region, task.getContainerInstanceArn());
@@ -220,7 +220,7 @@ public Collection<AgentDataType> getProvidedDataTypes() {
 
   @Override
   public String getAgentType() {
-    return TaskHealthCachingAgent.class.getSimpleName();
+    return accountName + "/" + region + "/" + getClass().getSimpleName();
   }
 
   @Override