From 3cdf32e46b6bafc2647ff9f79eb74b23fb6c5af2 Mon Sep 17 00:00:00 2001 From: Christos Arvanitis Date: Wed, 25 Sep 2024 21:08:32 +0300 Subject: [PATCH] perf(ecs): Narrowing the cache search for the ECS provider on views (#6256) * perf(ecs): Narrowing the cache search for the ECS provider on views * perf(ecs): ECS alarms to be cached/searched with EcsClusterName id --- .../spinnaker/clouddriver/ecs/cache/Keys.java | 4 +- .../ecs/cache/client/AbstractCacheClient.java | 12 + .../client/EcsCloudWatchAlarmCacheClient.java | 33 +-- .../ops/DestroyServiceAtomicOperation.java | 5 +- .../clouddriver/ecs/model/EcsApplication.java | 7 +- .../EcsCloudMetricAlarmCachingAgent.java | 8 +- .../ecs/provider/view/EcsClusterProvider.java | 5 +- .../view/EcsLoadBalancerProvider.java | 8 +- .../view/EcsServerClusterProvider.java | 116 ++++++---- .../ecs/services/EcsCloudMetricService.java | 5 +- .../ecs/view/EcsApplicationProvider.java | 69 ++++-- .../EcsCloudWatchAlarmCacheClientSpec.groovy | 216 +++++++++++++++++- ...EcsCloudMetricAlarmCachingAgentSpec.groovy | 38 ++- .../view/EcsClusterProviderSpec.groovy | 11 +- .../view/EcsLoadBalancerProviderSpec.groovy | 4 +- .../view/EcsServerClusterProviderSpec.groovy | 2 +- .../services/EcsCloudMetricServiceSpec.groovy | 6 +- .../view/EcsApplicationProviderSpec.groovy | 16 +- .../controllers/ClusterController.groovy | 8 +- 19 files changed, 456 insertions(+), 117 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 index 8aa80618132..0083edaf92f 100644 --- 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 @@ -201,8 +201,8 @@ public static String getTaskDefinitionKey( return buildKey(Namespace.TASK_DEFINITIONS.ns, account, region, taskDefinitionArn); } - public static String getAlarmKey(String account, String region, String alarmArn) { - return buildKey(Namespace.ALARMS.ns, account, region, alarmArn); + public static String getAlarmKey(String account, String region, String alarmArn, String cluster) { + return buildKey(Namespace.ALARMS.ns, account, region, alarmArn + SEPARATOR + cluster); } public static String getScalableTargetKey(String account, String region, String resourceId) { diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/AbstractCacheClient.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/AbstractCacheClient.java index 53c749e9c1d..d0686b544ca 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/AbstractCacheClient.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/AbstractCacheClient.java @@ -65,6 +65,14 @@ public Collection getAll(String account, String region) { return convertAll(data); } + public Collection getAll(Collection identifiers) { + Collection allData = cacheView.getAll(keyNamespace, identifiers); + if (allData == null) { + return Collections.emptyList(); + } + return convertAll(allData); + } + /** * @param key A key within the key namespace that will be used to retrieve the object. * @return An object of the generic type that is associated to the key. @@ -110,4 +118,8 @@ private Collection fetchFromCache(String account, String region) { return allData; } + + public Collection filterIdentifiers(String glob) { + return cacheView.filterIdentifiers(keyNamespace, glob); + } } diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsCloudWatchAlarmCacheClient.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsCloudWatchAlarmCacheClient.java index 0635329ef33..ec9c6659aab 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsCloudWatchAlarmCacheClient.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsCloudWatchAlarmCacheClient.java @@ -20,6 +20,7 @@ import com.netflix.spinnaker.cats.cache.Cache; import com.netflix.spinnaker.cats.cache.CacheData; +import com.netflix.spinnaker.clouddriver.ecs.cache.Keys; import com.netflix.spinnaker.clouddriver.ecs.cache.model.EcsMetricAlarm; import java.util.Collection; import java.util.Collections; @@ -71,31 +72,19 @@ protected EcsMetricAlarm convert(CacheData cacheData) { } public List getMetricAlarms( - String serviceName, String accountName, String region) { + String serviceName, String accountName, String region, String ecsClusterName) { List metricAlarms = new LinkedList<>(); - Collection allMetricAlarms = getAll(accountName, region); - outLoop: - for (EcsMetricAlarm metricAlarm : allMetricAlarms) { - for (String action : metricAlarm.getAlarmActions()) { - if (action.contains(serviceName)) { - metricAlarms.add(metricAlarm); - continue outLoop; - } - } + String glob = Keys.getAlarmKey(accountName, region, "*", ecsClusterName); + Collection metricAlarmsIds = filterIdentifiers(glob); + Collection allMetricAlarms = getAll(metricAlarmsIds); - for (String action : metricAlarm.getOKActions()) { - if (action.contains(serviceName)) { - metricAlarms.add(metricAlarm); - continue outLoop; - } - } - - for (String action : metricAlarm.getInsufficientDataActions()) { - if (action.contains(serviceName)) { - metricAlarms.add(metricAlarm); - continue outLoop; - } + for (EcsMetricAlarm metricAlarm : allMetricAlarms) { + if (metricAlarm.getAlarmActions().stream().anyMatch(action -> action.contains(serviceName)) + || metricAlarm.getOKActions().stream().anyMatch(action -> action.contains(serviceName)) + || metricAlarm.getInsufficientDataActions().stream() + .anyMatch(action -> action.contains(serviceName))) { + metricAlarms.add(metricAlarm); } } diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DestroyServiceAtomicOperation.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DestroyServiceAtomicOperation.java index 601c54c1931..f3a9e3d4e43 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DestroyServiceAtomicOperation.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DestroyServiceAtomicOperation.java @@ -45,7 +45,10 @@ public Void operate(List priorOutputs) { updateTaskStatus("Removing MetricAlarms from " + description.getServerGroupName() + "."); ecsCloudMetricService.deleteMetrics( - description.getServerGroupName(), description.getAccount(), description.getRegion()); + description.getServerGroupName(), + description.getAccount(), + description.getRegion(), + ecsClusterName); updateTaskStatus("Done removing MetricAlarms from " + description.getServerGroupName() + "."); UpdateServiceRequest updateServiceRequest = new UpdateServiceRequest(); diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/model/EcsApplication.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/model/EcsApplication.java index 73d1ec2c717..f710362c5d0 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/model/EcsApplication.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/model/EcsApplication.java @@ -26,11 +26,16 @@ public class EcsApplication implements Application { private String name; Map attributes; Map> clusterNames; + Map> clusterNameMetadata; public EcsApplication( - String name, Map attributes, Map> clusterNames) { + String name, + Map attributes, + Map> clusterNames, + Map> getClusterNameMetadata) { this.name = name; this.attributes = attributes; this.clusterNames = clusterNames; + this.clusterNameMetadata = getClusterNameMetadata; } } diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java index 00d7ed23876..4527fd2afbc 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java @@ -118,7 +118,13 @@ Map> generateFreshData(Set cacheableM Map> newDataMap = new HashMap<>(); for (MetricAlarm metricAlarm : cacheableMetricAlarm) { - String key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn()); + String cluster = + metricAlarm.getDimensions().stream() + .filter(t -> t.getName().equalsIgnoreCase("ClusterName")) + .map(t -> t.getValue()) + .collect(Collectors.joining()); + + String key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn(), cluster); Map attributes = convertMetricAlarmToAttributes(metricAlarm, accountName, region); diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProvider.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProvider.java index 81e57ca02fb..5b8a0a5a50f 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProvider.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProvider.java @@ -23,6 +23,7 @@ import com.google.common.collect.Lists; import com.netflix.spinnaker.cats.cache.Cache; import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider; +import com.netflix.spinnaker.clouddriver.ecs.cache.Keys; import com.netflix.spinnaker.clouddriver.ecs.cache.client.EcsClusterCacheClient; import com.netflix.spinnaker.clouddriver.ecs.cache.model.EcsCluster; import com.netflix.spinnaker.clouddriver.ecs.security.NetflixECSCredentials; @@ -55,9 +56,11 @@ public Collection getAllEcsClusters() { // TODO include[] input of Describe Cluster is not a part of this implementation, need to // implement in the future if additional properties are needed. public Collection getEcsClusterDescriptions(String account, String region) { + String glob = Keys.getClusterKey(account, region, "*"); + Collection ecsClustersIdentifiers = ecsClusterCacheClient.filterIdentifiers(glob); Collection clusters = new ArrayList<>(); List filteredEcsClusters = - ecsClusterCacheClient.getAll().stream() + ecsClusterCacheClient.getAll(ecsClustersIdentifiers).stream() .filter( cluster -> account.equals(cluster.getAccount()) && region.equals(cluster.getRegion())) diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProvider.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProvider.java index 24b0e6d384e..006b03684ae 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProvider.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProvider.java @@ -23,6 +23,7 @@ import com.netflix.spinnaker.clouddriver.aws.AmazonCloudProvider; import com.netflix.spinnaker.clouddriver.aws.data.ArnUtils; import com.netflix.spinnaker.clouddriver.ecs.EcsCloudProvider; +import com.netflix.spinnaker.clouddriver.ecs.cache.Keys; import com.netflix.spinnaker.clouddriver.ecs.cache.client.EcsLoadbalancerCacheClient; import com.netflix.spinnaker.clouddriver.ecs.cache.client.EcsTargetGroupCacheClient; import com.netflix.spinnaker.clouddriver.ecs.cache.client.ServiceCacheClient; @@ -118,8 +119,13 @@ public List
byAccountAndRegionAndName(String account, String region, St @Override public Set getApplicationLoadBalancers(String application) { // Find the load balancers currently in use by ECS services in this application + String glob = + application != null + ? Keys.getServiceKey("*", "*", application + "*") + : Keys.getServiceKey("*", "*", "*"); + Collection ecsServices = ecsServiceCacheClient.filterIdentifiers(glob); Set services = - ecsServiceCacheClient.getAll().stream() + ecsServiceCacheClient.getAll(ecsServices).stream() .filter(service -> service.getApplicationName().equals(application)) .collect(Collectors.toSet()); log.debug("Retrieved {} services for application '{}'", services.size(), application); diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java index 6fc1c978055..0867087d1c1 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java @@ -100,15 +100,17 @@ public EcsServerClusterProvider( private Map> findClusters( Map> clusterMap, AmazonCredentials credentials) { - return findClusters(clusterMap, credentials, null); + return findClusters(clusterMap, credentials, null, true); } private Map> findClusters( Map> clusterMap, AmazonCredentials credentials, - String application) { + String application, + boolean inludeDetails) { for (AmazonCredentials.AWSRegion awsRegion : credentials.getRegions()) { - clusterMap = findClustersForRegion(clusterMap, credentials, awsRegion, application); + clusterMap = + findClustersForRegion(clusterMap, credentials, awsRegion, application, inludeDetails); } return clusterMap; @@ -118,10 +120,16 @@ private Map> findClustersForRegion( Map> clusterMap, AmazonCredentials credentials, AmazonCredentials.AWSRegion awsRegion, - String application) { + String application, + boolean includeDetails) { - Collection services = - serviceCacheClient.getAll(credentials.getName(), awsRegion.getName()); + String glob = + application != null + ? Keys.getServiceKey(credentials.getName(), awsRegion.getName(), application + "*") + : Keys.getServiceKey(credentials.getName(), awsRegion.getName(), "*"); + + Collection ecsServices = serviceCacheClient.filterIdentifiers(glob); + Collection services = serviceCacheClient.getAll(ecsServices); Collection allTasks = taskCacheClient.getAll(credentials.getName(), awsRegion.getName()); for (Service service : services) { @@ -167,7 +175,8 @@ private Map> findClustersForRegion( service.getClusterName(), taskDefinition, service.getSubnets(), - service.getSecurityGroups()); + service.getSecurityGroups(), + includeDetails); if (ecsServerGroup == null) { continue; @@ -308,13 +317,9 @@ private EcsServerGroup buildEcsServerGroup( String ecsClusterName, com.amazonaws.services.ecs.model.TaskDefinition taskDefinition, List eniSubnets, - List eniSecurityGroups) { + List eniSecurityGroups, + boolean includeDetails) { ServerGroup.InstanceCounts instanceCounts = buildInstanceCount(instances); - TaskDefinition ecsTaskDefinition = buildTaskDefinition(taskDefinition); - EcsServerGroup.Image image = new EcsServerGroup.Image(); - image.setImageId(ecsTaskDefinition.getContainerImage()); - image.setName(ecsTaskDefinition.getContainerImage()); - String scalableTargetId = "service/" + ecsClusterName + "/" + serviceName; String scalableTargetKey = Keys.getScalableTargetKey(account, region, scalableTargetId); ScalableTarget scalableTarget = scalableTargetCacheClient.get(scalableTargetKey); @@ -368,31 +373,52 @@ private EcsServerGroup buildEcsServerGroup( } } } - Set metricAlarmNames = - ecsCloudWatchAlarmCacheClient.getMetricAlarms(serviceName, account, region).stream() + ecsCloudWatchAlarmCacheClient + .getMetricAlarms(serviceName, account, region, ecsClusterName) + .stream() .map(EcsMetricAlarm::getAlarmName) .collect(Collectors.toSet()); - - EcsServerGroup serverGroup = - new EcsServerGroup() - .setDisabled(capacity.getDesired() == 0) - .setName(serviceName) - .setCloudProvider(EcsCloudProvider.ID) - .setType(EcsCloudProvider.ID) - .setRegion(region) - .setInstances(instances) - .setCapacity(capacity) - .setImage(image) - .setInstanceCounts(instanceCounts) - .setCreatedTime(creationTime) - .setEcsCluster(ecsClusterName) - .setTaskDefinition(ecsTaskDefinition) - .setVpcId(vpcId) - .setSecurityGroups(securityGroups) - .setMetricAlarms(metricAlarmNames) - .setMoniker(moniker); - + EcsServerGroup serverGroup = new EcsServerGroup(); + if (includeDetails) { + TaskDefinition ecsTaskDefinition = buildTaskDefinition(taskDefinition); + EcsServerGroup.Image image = new EcsServerGroup.Image(); + image.setImageId(ecsTaskDefinition.getContainerImage()); + image.setName(ecsTaskDefinition.getContainerImage()); + serverGroup + .setDisabled(capacity.getDesired() == 0) + .setName(serviceName) + .setCloudProvider(EcsCloudProvider.ID) + .setType(EcsCloudProvider.ID) + .setRegion(region) + .setInstances(instances) + .setCapacity(capacity) + .setImage(image) + .setInstanceCounts(instanceCounts) + .setCreatedTime(creationTime) + .setEcsCluster(ecsClusterName) + .setTaskDefinition(ecsTaskDefinition) + .setVpcId(vpcId) + .setSecurityGroups(securityGroups) + .setMetricAlarms(metricAlarmNames) + .setMoniker(moniker); + } else { + serverGroup + .setDisabled(capacity.getDesired() == 0) + .setName(serviceName) + .setCloudProvider(EcsCloudProvider.ID) + .setType(EcsCloudProvider.ID) + .setRegion(region) + .setInstances(instances) + .setCapacity(capacity) + .setInstanceCounts(instanceCounts) + .setCreatedTime(creationTime) + .setEcsCluster(ecsClusterName) + .setVpcId(vpcId) + .setSecurityGroups(securityGroups) + .setMetricAlarms(metricAlarmNames) + .setMoniker(moniker); + } EcsServerGroup.AutoScalingGroup asg = new EcsServerGroup.AutoScalingGroup() .setDesiredCapacity(scalableTarget.getMaxCapacity()) @@ -456,26 +482,30 @@ private AmazonCredentials getEcsCredentials(String account) { @Override public Map> getClusterSummaries(String application) { - return getClusters(); + return getClusters0(application, false); } @Override public Map> getClusterDetails(String application) { + return getClusters0(application, true); + } + + @Override + public Map> getClusters() { Map> clusterMap = new HashMap<>(); for (AmazonCredentials credentials : getEcsCredentials()) { - clusterMap = findClusters(clusterMap, credentials, application); + clusterMap = findClusters(clusterMap, credentials); } - return clusterMap; } - @Override - public Map> getClusters() { + private Map> getClusters0( + String application, boolean includeDetails) { Map> clusterMap = new HashMap<>(); for (AmazonCredentials credentials : getEcsCredentials()) { - clusterMap = findClusters(clusterMap, credentials); + clusterMap = findClusters(clusterMap, credentials, application, includeDetails); } return clusterMap; } @@ -485,7 +515,7 @@ public Map> getClusters() { public Set getClusters(String application, String account) { try { AmazonCredentials credentials = getEcsCredentials(account); - return findClusters(new HashMap<>(), credentials, application).get(application); + return findClusters(new HashMap<>(), credentials, application, true).get(application); } catch (NoSuchElementException exception) { log.info("No ECS Credentials were found for account " + account); return null; @@ -536,7 +566,7 @@ public ServerGroup getServerGroup( AmazonCredentials credentials = getEcsCredentials(account); Moniker moniker = MonikerHelper.applicationNameToMoniker(serverGroupName); log.debug("App Name is: " + moniker.getApp()); - clusterMap = findClusters(clusterMap, credentials, moniker.getApp()); + clusterMap = findClusters(clusterMap, credentials, moniker.getApp(), true); } catch (NoSuchElementException exception) { /* This is ugly, but not sure how else to do it. If we don't have creds due * to not being an ECS account, there's nothing to do here, and we should diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricService.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricService.java index 90b45dbd8df..07af7889e05 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricService.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricService.java @@ -43,9 +43,10 @@ public class EcsCloudMetricService { private final Logger log = LoggerFactory.getLogger(getClass()); - public void deleteMetrics(String serviceName, String account, String region) { + public void deleteMetrics( + String serviceName, String account, String region, String ecsClusterName) { List metricAlarms = - metricAlarmCacheClient.getMetricAlarms(serviceName, account, region); + metricAlarmCacheClient.getMetricAlarms(ecsClusterName, serviceName, account, region); if (metricAlarms.isEmpty()) { return; diff --git a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java index e5ad5ddbc4c..0f219ed252c 100644 --- a/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java +++ b/clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java @@ -18,6 +18,7 @@ import com.google.common.collect.Sets; import com.netflix.spinnaker.clouddriver.aws.security.AmazonCredentials; +import com.netflix.spinnaker.clouddriver.ecs.cache.Keys; import com.netflix.spinnaker.clouddriver.ecs.cache.client.ServiceCacheClient; import com.netflix.spinnaker.clouddriver.ecs.cache.model.Service; import com.netflix.spinnaker.clouddriver.ecs.model.EcsApplication; @@ -50,31 +51,31 @@ public EcsApplicationProvider( @Override public Application getApplication(String name) { - - for (Application application : getApplications(true)) { + String glob = Keys.getServiceKey("*", "*", name + "*"); + Collection ecsServices = serviceCacheClient.filterIdentifiers(glob); + for (Application application : populateApplicationSet(ecsServices, true)) { if (name.equals(application.getName())) { return application; } } - return null; } @Override - public Set getApplications(boolean expand) { - Set applications = new HashSet<>(); - + public Set getApplications(boolean expand) { + Set applications = new HashSet<>(); for (NetflixECSCredentials credentials : credentialsRepository.getAll()) { - Set retrievedApplications = findApplicationsForAllRegions(credentials, expand); + Set retrievedApplications = + findApplicationsForAllRegions(credentials, expand); applications.addAll(retrievedApplications); } return applications; } - private Set findApplicationsForAllRegions( + private Set findApplicationsForAllRegions( AmazonCredentials credentials, boolean expand) { - Set applications = new HashSet<>(); + Set applications = new HashSet<>(); for (AmazonCredentials.AWSRegion awsRegion : credentials.getRegions()) { applications.addAll( @@ -84,16 +85,16 @@ private Set findApplicationsForAllRegions( return applications; } - private Set findApplicationsForRegion( + private Set findApplicationsForRegion( String account, String region, boolean expand) { - HashMap applicationHashMap = + HashMap applicationHashMap = populateApplicationMap(account, region, expand); return transposeApplicationMapToSet(applicationHashMap); } - private HashMap populateApplicationMap( + private HashMap populateApplicationMap( String account, String region, boolean expand) { - HashMap applicationHashMap = new HashMap<>(); + HashMap applicationHashMap = new HashMap<>(); Collection services = serviceCacheClient.getAll(account, region); for (Service service : services) { @@ -102,19 +103,32 @@ private HashMap populateApplicationMap( return applicationHashMap; } - private Set transposeApplicationMapToSet( - HashMap applicationHashMap) { - Set applications = new HashSet<>(); + private Set populateApplicationSet( + Collection identifiers, boolean expand) { + HashMap applicationHashMap = new HashMap<>(); + Collection services = serviceCacheClient.getAll(identifiers); - for (Map.Entry entry : applicationHashMap.entrySet()) { + for (Service service : services) { + if (credentialsRepository.has(service.getAccount())) { + applicationHashMap = inferApplicationFromServices(applicationHashMap, service, expand); + } + } + return transposeApplicationMapToSet(applicationHashMap); + } + + private Set transposeApplicationMapToSet( + HashMap applicationHashMap) { + Set applications = new HashSet<>(); + + for (Map.Entry entry : applicationHashMap.entrySet()) { applications.add(entry.getValue()); } return applications; } - private HashMap inferApplicationFromServices( - HashMap applicationHashMap, Service service, boolean expand) { + private HashMap inferApplicationFromServices( + HashMap applicationHashMap, Service service, boolean expand) { HashMap attributes = new HashMap<>(); Moniker moniker = service.getMoniker(); @@ -125,18 +139,31 @@ private HashMap inferApplicationFromServices( attributes.put("name", appName); HashMap> clusterNames = new HashMap<>(); + HashMap> clusterNamesMetadata = new HashMap<>(); + if (expand) { clusterNames.put(accountName, Sets.newHashSet(serviceName)); + clusterNamesMetadata.put(accountName, Sets.newHashSet(moniker.getCluster())); } - EcsApplication application = new EcsApplication(appName, attributes, clusterNames); + EcsApplication application = + new EcsApplication(appName, attributes, clusterNames, clusterNamesMetadata); if (!applicationHashMap.containsKey(appName)) { applicationHashMap.put(appName, application); } else { applicationHashMap.get(appName).getAttributes().putAll(application.getAttributes()); if (expand) { - applicationHashMap.get(appName).getClusterNames().get(accountName).add(serviceName); + applicationHashMap + .get(appName) + .getClusterNames() + .computeIfAbsent(accountName, k -> Sets.newHashSet()) + .add(serviceName); + applicationHashMap + .get(appName) + .getClusterNameMetadata() + .computeIfAbsent(accountName, k -> Sets.newHashSet()) + .add(moniker.getCluster()); } } diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/EcsCloudWatchAlarmCacheClientSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/EcsCloudWatchAlarmCacheClientSpec.groovy index de6503b4217..f4ffb21b683 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/EcsCloudWatchAlarmCacheClientSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/EcsCloudWatchAlarmCacheClientSpec.groovy @@ -16,27 +16,58 @@ package com.netflix.spinnaker.clouddriver.ecs.cache +import com.amazonaws.auth.AWSCredentialsProvider +import com.amazonaws.services.cloudwatch.AmazonCloudWatch +import com.amazonaws.services.cloudwatch.model.Dimension +import com.amazonaws.services.cloudwatch.model.MetricAlarm import com.netflix.spinnaker.cats.cache.Cache import com.netflix.spinnaker.cats.cache.DefaultCacheData +import com.netflix.spinnaker.cats.provider.ProviderCache +import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider import com.netflix.spinnaker.clouddriver.ecs.cache.client.EcsCloudWatchAlarmCacheClient import com.netflix.spinnaker.clouddriver.ecs.cache.model.EcsMetricAlarm +import com.netflix.spinnaker.clouddriver.ecs.provider.agent.CommonCachingAgent import com.netflix.spinnaker.clouddriver.ecs.provider.agent.EcsCloudMetricAlarmCachingAgent +import spock.lang.Shared import spock.lang.Specification import spock.lang.Subject class EcsCloudWatchAlarmCacheClientSpec extends Specification { - def cacheView = Mock(Cache) @Subject - EcsCloudWatchAlarmCacheClient client = new EcsCloudWatchAlarmCacheClient(cacheView) + EcsCloudWatchAlarmCacheClient client + + @Subject + EcsCloudMetricAlarmCachingAgent agent + + @Shared + String ACCOUNT = 'test-account' + + @Shared + String REGION = 'us-west-1' + + Cache cacheView + AmazonCloudWatch cloudWatch + AmazonClientProvider clientProvider + ProviderCache providerCache + AWSCredentialsProvider credentialsProvider + + def setup() { + cacheView = Mock(Cache) + client = new EcsCloudWatchAlarmCacheClient(cacheView) + cloudWatch = Mock(AmazonCloudWatch) + clientProvider = Mock(AmazonClientProvider) + providerCache = Mock(ProviderCache) + credentialsProvider = Mock(AWSCredentialsProvider) + agent = new EcsCloudMetricAlarmCachingAgent(CommonCachingAgent.netflixAmazonCredentials, REGION, clientProvider) + } def 'should convert cache data into object'() { given: - def accountName = 'test-account-1' - def region = 'us-west-1' - def metricAlarm = new EcsMetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn").withRegion(region).withAccountName(accountName) - def key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn()) - def attributes = EcsCloudMetricAlarmCachingAgent.convertMetricAlarmToAttributes(metricAlarm, accountName, region) + def ecsClusterName = 'my-cluster' + def metricAlarm = new EcsMetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn").withRegion(REGION).withAccountName(ACCOUNT) + def key = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarm.getAlarmArn(), ecsClusterName) + def attributes = EcsCloudMetricAlarmCachingAgent.convertMetricAlarmToAttributes(metricAlarm, ACCOUNT, REGION) when: def returnedMetricAlarm = client.get(key) @@ -44,4 +75,175 @@ class EcsCloudWatchAlarmCacheClientSpec extends Specification { cacheView.get(Keys.Namespace.ALARMS.ns, key) >> new DefaultCacheData(key, attributes, [:]) returnedMetricAlarm == metricAlarm } + + + def 'should return metric alarms for a service - single cluster'() { + given: + def serviceName = 'my-service' + def serviceName2 = 'not-matching-service' + + def ecsClusterName = 'my-cluster' + def metricAlarms = Set.of( + new MetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]), + new MetricAlarm().withAlarmName("alarm-name-2").withAlarmArn("alarmArn2") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]), + new MetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn3") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName2}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]) + ) + def keys = metricAlarms.collect { alarm -> + def key = Keys.getAlarmKey(ACCOUNT, REGION, alarm.getAlarmArn(), ecsClusterName) + def attributes = agent.convertMetricAlarmToAttributes(alarm, ACCOUNT, REGION) + [key, new DefaultCacheData(key, attributes, [:])] + } + + cacheView.filterIdentifiers(Keys.Namespace.ALARMS.ns, _) >> keys*.first() + cacheView.getAll(Keys.Namespace.ALARMS.ns, _) >> keys*.last() + + when: + def metricAlarmsReturned = client.getMetricAlarms(serviceName, ACCOUNT, REGION, ecsClusterName) + + then: + metricAlarmsReturned.size() == 2 + metricAlarmsReturned*.alarmName.containsAll(["alarm-name", "alarm-name-2"]) + metricAlarmsReturned*.alarmArn.containsAll(["alarmArn", "alarmArn2"]) + !metricAlarmsReturned*.alarmArn.contains(["alarmArn3"]) +} + + def 'should return metric alarms for a service - multiple clusters'() { + given: + def serviceName = 'my-service' + + def ecsClusterName = 'my-cluster' + def ecsClusterName2 = 'my-cluster-2' + def metricAlarm1 = new MetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]) + def metricAlarm2 = new MetricAlarm().withAlarmName("alarm-name-2").withAlarmArn("alarmArn2") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]) + def metricAlarm3 = new MetricAlarm().withAlarmName("alarm-name3").withAlarmArn("alarmArn3") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName2)]) + + def key1 = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarm1.getAlarmArn(), ecsClusterName) + def attributes1 = agent.convertMetricAlarmToAttributes(metricAlarm1, ACCOUNT, REGION) + def key2 = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarm2.getAlarmArn(), ecsClusterName) + def attributes2 = agent.convertMetricAlarmToAttributes(metricAlarm2, ACCOUNT, REGION) + def key3 = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarm3.getAlarmArn(), ecsClusterName2) + def attributes3 = agent.convertMetricAlarmToAttributes(metricAlarm3, ACCOUNT, REGION) + + + cacheView.filterIdentifiers(Keys.Namespace.ALARMS.ns, Keys.getAlarmKey(ACCOUNT, REGION, "*", ecsClusterName)) >> [key1,key2] + cacheView.filterIdentifiers(Keys.Namespace.ALARMS.ns, Keys.getAlarmKey(ACCOUNT, REGION, "*", ecsClusterName2)) >> [key3] + cacheView.getAll(Keys.Namespace.ALARMS.ns, [key1,key2]) >> [ + new DefaultCacheData(key1, attributes1, [:]), + new DefaultCacheData(key2, attributes2, [:]) + ] + cacheView.getAll(Keys.Namespace.ALARMS.ns, [key3]) >> [ + new DefaultCacheData(key3, attributes3, [:]) + ] + when: + def metricAlarmsReturned = client.getMetricAlarms(serviceName, ACCOUNT, REGION, ecsClusterName) + def metricAlarmsReturned2 = client.getMetricAlarms(serviceName, ACCOUNT, REGION, ecsClusterName2) + + then: + metricAlarmsReturned.size() == 2 + metricAlarmsReturned*.alarmName.containsAll(["alarm-name", "alarm-name-2"]) + metricAlarmsReturned*.alarmArn.containsAll(["alarmArn", "alarmArn2"]) + !metricAlarmsReturned*.alarmArn.contains(["alarmArn3"]) + metricAlarmsReturned2.size() == 1 + metricAlarmsReturned2*.alarmName.containsAll(["alarm-name3"]) + !metricAlarmsReturned2*.alarmArn.containsAll(["alarmArn", "alarmArn2"]) + metricAlarmsReturned2*.alarmArn.containsAll(["alarmArn3"]) + } + +def 'should return empty list if no metric alarms match the service'() { + given: + def serviceName = 'my-service' + + def ecsClusterName = 'my-cluster' + def metricAlarms = Set.of(new MetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)])) + def key = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarms[0].getAlarmArn(), ecsClusterName) + def attributes = agent.convertMetricAlarmToAttributes(metricAlarms[0], ACCOUNT, REGION) + + cacheView.filterIdentifiers(Keys.Namespace.ALARMS.ns, _) >> [key] + cacheView.getAll(Keys.Namespace.ALARMS.ns, _) >> [ + new DefaultCacheData(key, attributes, [:]) + ] + + when: + def metricAlarmsReturned = client.getMetricAlarms("some-other-service", ACCOUNT, REGION, ecsClusterName) + + then: + metricAlarmsReturned.isEmpty() +} + +def 'should return metric alarms with actions matching the service'() { + given: + def serviceName = 'my-service' + + def ecsClusterName = 'my-cluster' + def metricAlarms = Set.of( + new MetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn") + .withAlarmActions("arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions") + .withOKActions("arn:aws:sns:us-west-1:123456789012:${serviceName}-OKActions") + .withInsufficientDataActions("arn:aws:sns:us-west-1:123456789012:${serviceName}-InsufficientDataActions") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]), + new MetricAlarm().withAlarmName("alarm-name-2").withAlarmArn("alarmArn2") + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]), + new MetricAlarm().withAlarmName("alarm-name-3").withAlarmArn("alarmArn3") + .withAlarmActions( + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions1", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions2", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions3" + ) + .withOKActions( + "arn:aws:sns:us-west-1:123456789012:${serviceName}-OKActions1" + ) + .withInsufficientDataActions( + "arn:aws:sns:us-west-1:123456789012:${serviceName}-InsufficientDataActions1" + ) + .withDimensions([new Dimension().withName("ClusterName").withValue(ecsClusterName)]) + ) + def keys = metricAlarms.collect { alarm -> + def key = Keys.getAlarmKey(ACCOUNT, REGION, alarm.getAlarmArn(), ecsClusterName) + def attributes = agent.convertMetricAlarmToAttributes(alarm, ACCOUNT, REGION) + [key, new DefaultCacheData(key, attributes, [:])] + } + + cacheView.filterIdentifiers(Keys.Namespace.ALARMS.ns, _) >> keys*.first() + cacheView.getAll(Keys.Namespace.ALARMS.ns, _) >> keys*.last() + + when: + def metricAlarmsReturned = client.getMetricAlarms(serviceName, ACCOUNT, REGION, ecsClusterName) + + then: + metricAlarmsReturned.size() == 2 + metricAlarmsReturned*.alarmName.containsAll(["alarm-name", "alarm-name-3"]) + metricAlarmsReturned*.alarmArn.containsAll(["alarmArn", "alarmArn3"]) + metricAlarmsReturned*.alarmActions.flatten().size() == 4 + metricAlarmsReturned*.OKActions.flatten().size() == 2 + metricAlarmsReturned*.insufficientDataActions.flatten().size() == 2 + metricAlarmsReturned*.OKActions.flatten().sort() == List.of( + "arn:aws:sns:us-west-1:123456789012:${serviceName}-OKActions", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-OKActions1" + ) + metricAlarmsReturned*.alarmActions.flatten().sort() == List.of( + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions1", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions2", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-AlarmsActions3" + ) + metricAlarmsReturned*.insufficientDataActions.flatten().sort() == List.of( + "arn:aws:sns:us-west-1:123456789012:${serviceName}-InsufficientDataActions", + "arn:aws:sns:us-west-1:123456789012:${serviceName}-InsufficientDataActions1" + ) +} + } diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgentSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgentSpec.groovy index e7d0e8e10ac..05b5299f564 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgentSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgentSpec.groovy @@ -19,6 +19,9 @@ package com.netflix.spinnaker.clouddriver.ecs.provider.agent import com.amazonaws.auth.AWSCredentialsProvider import com.amazonaws.services.cloudwatch.AmazonCloudWatch import com.amazonaws.services.cloudwatch.model.DescribeAlarmsResult +import com.amazonaws.services.cloudwatch.model.Dimension +import com.amazonaws.services.cloudwatch.model.MetricAlarm +import com.netflix.spinnaker.cats.cache.DefaultCacheData import com.netflix.spinnaker.cats.provider.ProviderCache import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider import com.netflix.spinnaker.clouddriver.ecs.cache.Keys @@ -38,13 +41,15 @@ class EcsCloudMetricAlarmCachingAgentSpec extends Specification { AWSCredentialsProvider credentialsProvider @Subject - EcsCloudMetricAlarmCachingAgent agent = new EcsCloudMetricAlarmCachingAgent(CommonCachingAgent.netflixAmazonCredentials, REGION, clientProvider) + EcsCloudMetricAlarmCachingAgent agent def setup() { cloudWatch = Mock(AmazonCloudWatch) clientProvider = Mock(AmazonClientProvider) providerCache = Mock(ProviderCache) credentialsProvider = Mock(AWSCredentialsProvider) + agent = new EcsCloudMetricAlarmCachingAgent(CommonCachingAgent.netflixAmazonCredentials, 'us-west-1', clientProvider) + } def 'should get a list of cloud watch alarms'() { @@ -74,4 +79,35 @@ class EcsCloudMetricAlarmCachingAgentSpec extends Specification { metricAlarms*.accountName.containsAll(cacheData.get(Keys.Namespace.ALARMS.ns)*.getAttributes().accountName) metricAlarms*.region.containsAll(cacheData.get(Keys.Namespace.ALARMS.ns)*.getAttributes().region) } + + def 'should evict old keys when id is appended'() { + given: + def metricAlarm1 = new MetricAlarm().withAlarmName("alarm-name-1").withAlarmArn("alarmArn-1").withDimensions([new Dimension().withName("ClusterName").withValue("my-cluster")]) + def metricAlarm2 = new MetricAlarm().withAlarmName("alarm-name-2").withAlarmArn("alarmArn-2").withDimensions([new Dimension().withName("ClusterName").withValue("my-cluster")]) + def attributes1 = EcsCloudMetricAlarmCachingAgent.convertMetricAlarmToAttributes(metricAlarm1, ACCOUNT, REGION) + def attributes2 = EcsCloudMetricAlarmCachingAgent.convertMetricAlarmToAttributes(metricAlarm2, ACCOUNT, REGION) + def metricAlarms = [metricAlarm1, metricAlarm2] + def describeAlarmsResult = new DescribeAlarmsResult().withMetricAlarms(metricAlarms) + cloudWatch.describeAlarms(_) >> describeAlarmsResult + clientProvider.getAmazonCloudWatch(_, _, _) >> cloudWatch + + def oldKey1 = Keys.buildKey(Keys.Namespace.ALARMS.ns, ACCOUNT, REGION, metricAlarm1.getAlarmArn()) + def oldKey2 = Keys.buildKey(Keys.Namespace.ALARMS.ns, ACCOUNT, REGION, metricAlarm2.getAlarmArn()) + def oldData = [new DefaultCacheData(oldKey1, attributes1, [:]), new DefaultCacheData(oldKey2, attributes2, [:])] + providerCache.getAll(Keys.Namespace.ALARMS.ns) >> oldData + + def newKey1 = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarm1.getAlarmArn(), "my-cluster") + def newKey2 = Keys.getAlarmKey(ACCOUNT, REGION, metricAlarm2.getAlarmArn(), "my-cluster") + + when: + def cacheResult = agent.loadData(providerCache) + + then: + cacheResult.evictions[Keys.Namespace.ALARMS.ns].size() == 2 + cacheResult.evictions[Keys.Namespace.ALARMS.ns].containsAll([oldKey1, oldKey2]) + cacheResult.cacheResults[Keys.Namespace.ALARMS.ns].size() == 2 + cacheResult.cacheResults[Keys.Namespace.ALARMS.ns]*.id.containsAll([newKey1, newKey2]) + cacheResult.cacheResults[Keys.Namespace.ALARMS.ns]*.attributes.containsAll([attributes1, attributes2]) + } + } diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProviderSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProviderSpec.groovy index 8a4ea874594..173e7821572 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProviderSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProviderSpec.groovy @@ -47,6 +47,7 @@ class EcsClusterProviderSpec extends Specification { Set clusterNames = new HashSet<>() Collection cacheData = new HashSet<>() Collection clustersResponse = new ArrayList<>() + Collection ecsClustersIdentifiers = new ArrayList<>() clusterNames.add("example-app-test-Cluster-NSnYsTXmCfV2") clusterNames.add("TestCluster") clusterNames.add("spinnaker-deployment-cluster") @@ -54,6 +55,7 @@ class EcsClusterProviderSpec extends Specification { for (int x = 0; x < numberOfClusters; x++) { String clusterKey = Keys.getClusterKey(ACCOUNT, REGION, clusterNames[x]) Map attributes = new HashMap<>() + ecsClustersIdentifiers.add(Keys.getClusterKey(ACCOUNT, REGION, clusterNames[x])) attributes.put("account", ACCOUNT) attributes.put("region", REGION) attributes.put("clusterArn", "arn:aws:ecs:::cluster/" + clusterNames[x]) @@ -61,7 +63,8 @@ class EcsClusterProviderSpec extends Specification { cacheData.add(new DefaultCacheData(clusterKey, attributes, Collections.emptyMap())) } - cacheView.getAll(_) >> cacheData + cacheView.filterIdentifiers(_, _) >> ecsClustersIdentifiers + cacheView.getAll(_, ecsClustersIdentifiers) >> cacheData for (int x = 0; x < numberOfClusters; x++) { Cluster cluster = new Cluster() @@ -102,12 +105,14 @@ class EcsClusterProviderSpec extends Specification { Set clusterNames = new HashSet<>() Collection cacheData = new HashSet<>() Collection clustersResponse = new ArrayList<>() + Collection ecsClustersIdentifiers = new ArrayList<>() clusterNames.add("example-app-test-Cluster-NSnYsTXmCfV2") clusterNames.add("TestCluster") clusterNames.add("spinnaker-deployment-cluster") for (int x = 0; x < 2; x++) { String clusterKey = Keys.getClusterKey(ACCOUNT, REGION, clusterNames[x]) + ecsClustersIdentifiers.add(Keys.getClusterKey(ACCOUNT, REGION, clusterNames[x])) Map attributes = new HashMap<>() attributes.put("account", ACCOUNT) attributes.put("region", REGION) @@ -126,8 +131,8 @@ class EcsClusterProviderSpec extends Specification { cacheData.add(new DefaultCacheData(clusterKey, attributes, Collections.emptyMap())) - - cacheView.getAll(_) >> cacheData + cacheView.filterIdentifiers(_, _) >> ecsClustersIdentifiers + cacheView.getAll(_, ecsClustersIdentifiers) >> cacheData //Adding only two clusters in the response which belongs to the expected region. for (int x = 0; x < 2; x++) { diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProviderSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProviderSpec.groovy index ec4596f90f6..d73f782f413 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProviderSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProviderSpec.groovy @@ -174,7 +174,7 @@ class EcsLoadBalancerProviderSpec extends Specification { def loadBalancerList = provider.getApplicationLoadBalancers(applicationName) then: - mockServiceCache.getAll() >> Collections.singletonList(ecsService) + mockServiceCache.getAll(_) >> Collections.singletonList(ecsService) mockTargetGroupCache.getAllKeys() >> ['fake-tg-key-1', 'fake-tg-key-2'] mockTargetGroupCache.find(_) >> [ecsTg1, ecsTg2] mockLBCache.findWithTargetGroups(_) >> [ecsLoadBalancerCache1, ecsLoadBalancerCache2] @@ -249,7 +249,7 @@ class EcsLoadBalancerProviderSpec extends Specification { def loadBalancerList = provider.getApplicationLoadBalancers(applicationName) then: - mockServiceCache.getAll() >> [ecsService1, ecsService2] + mockServiceCache.getAll(_) >> [ecsService1, ecsService2] mockTargetGroupCache.getAllKeys() >> ['fake-tg-key-1'] mockTargetGroupCache.find(_) >> [ecsTg] mockLBCache.findWithTargetGroups(_) >> Collections.singletonList(ecsLoadBalancerCache) diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy index 6cb51ea7782..857a8660b70 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy @@ -191,7 +191,7 @@ class EcsServerClusterProviderSpec extends Specification { containerInformationService.getTaskZone(_, _, _) >> 'us-west-1a' taskDefinitionCacheClient.get(_) >> cachedTaskDefinition scalableTargetCacheClient.get(_) >> scalableTarget - ecsCloudWatchAlarmCacheClient.getMetricAlarms(_, _, _) >> [] + ecsCloudWatchAlarmCacheClient.getMetricAlarms(_, _,_ ,_) >> [] subnetSelector.getSubnetVpcIds(_, _, _) >> ['vpc-1234'] cacheView.filterIdentifiers(_, _) >> ['key'] diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricServiceSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricServiceSpec.groovy index fee4f351fc1..99b7ffc0458 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricServiceSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricServiceSpec.groovy @@ -413,12 +413,14 @@ class EcsCloudMetricServiceSpec extends Specification { ) } - metricAlarmCacheClient.getMetricAlarms(_, _, _) >> metricAlarms + metricAlarmCacheClient.getMetricAlarms(_, _,_ ,_) >> metricAlarms when: - service.deleteMetrics(targetServiceName, targetAccountName, targetRegion) + service.deleteMetrics(targetServiceName, targetAccountName, targetRegion, clusterName) then: 1 * targetCloudWatch.deleteAlarms(_) } + + } diff --git a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProviderSpec.groovy b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProviderSpec.groovy index f6a34981af9..c31e0d65450 100644 --- a/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProviderSpec.groovy +++ b/clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProviderSpec.groovy @@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.netflix.spinnaker.cats.cache.Cache import com.netflix.spinnaker.cats.cache.DefaultCacheData import com.netflix.spinnaker.clouddriver.ecs.TestCredential +import com.netflix.spinnaker.clouddriver.ecs.cache.Keys import com.netflix.spinnaker.clouddriver.ecs.cache.client.ServiceCacheClient import com.netflix.spinnaker.clouddriver.ecs.model.EcsApplication import com.netflix.spinnaker.clouddriver.ecs.provider.agent.ServiceCachingAgent @@ -45,15 +46,21 @@ class EcsApplicationProviderSpec extends Specification { def accountName = 'test-account' def credentials = new NetflixECSCredentials(TestCredential.named(accountName)) def appName = 'testapp' - def serviceName = appName + '-kcats-liated' + def serviceName = appName + '-kcats-liated-v001' + def monikerCluster = appName + '-kcats-liated' Map> clusterNames = new HashMap<>() clusterNames.put(accountName, Collections.singleton(serviceName)) + Map> clusterNameMetadata = new HashMap<>() + clusterNameMetadata.put(accountName, Collections.singleton(monikerCluster)) + + def givenApp = (Application) new EcsApplication(appName, [ name: appName ], - clusterNames) + clusterNames, + clusterNameMetadata) def service = new Service( serviceName: serviceName, @@ -67,8 +74,9 @@ class EcsApplicationProviderSpec extends Specification { credentials.getRegions()[0].getName()).convertServiceToAttributes(service) credentialsRepository.getAll() >> [credentials] - cache.filterIdentifiers(_, _) >> [] - cache.getAll(_, _) >> [new DefaultCacheData('key', attributes, [:])] + credentialsRepository.has(accountName) >> true + cache.filterIdentifiers(_, _) >> [Keys.getServiceKey(accountName,"us-east-1",serviceName)] + cache.getAll(_, _) >> [new DefaultCacheData(Keys.getServiceKey(accountName,"us-east-1",serviceName), attributes, [:])] when: def retrievedApp = provider.getApplication(appName) diff --git a/clouddriver-web/src/main/groovy/com/netflix/spinnaker/clouddriver/controllers/ClusterController.groovy b/clouddriver-web/src/main/groovy/com/netflix/spinnaker/clouddriver/controllers/ClusterController.groovy index f9244bd3adb..2ff3eaf56a6 100644 --- a/clouddriver-web/src/main/groovy/com/netflix/spinnaker/clouddriver/controllers/ClusterController.groovy +++ b/clouddriver-web/src/main/groovy/com/netflix/spinnaker/clouddriver/controllers/ClusterController.groovy @@ -16,6 +16,7 @@ package com.netflix.spinnaker.clouddriver.controllers +import com.netflix.spinnaker.clouddriver.ecs.model.EcsApplication import com.netflix.spinnaker.clouddriver.model.* import com.netflix.spinnaker.clouddriver.model.view.ClusterViewModelPostProcessor import com.netflix.spinnaker.clouddriver.model.view.ServerGroupViewModelPostProcessor @@ -85,9 +86,12 @@ class ClusterController { private Map> mergeClusters(List a) { Map> map = new HashMap<>() - a.stream() - .flatMap({ it.getClusterNames().entrySet().stream() }) + .flatMap({ + it instanceof EcsApplication + ? it.getClusterNameMetadata().entrySet().stream() + : it.getClusterNames().entrySet().stream() + }) .forEach({ entry -> map.computeIfAbsent(entry.getKey(), { new HashSet<>() }).addAll(entry.getValue()) })