Skip to content

Commit

Permalink
perf(ecs): Narrowing the cache search for the ECS provider on views (#…
Browse files Browse the repository at this point in the history 
…6256)

* perf(ecs): Narrowing the cache search for the ECS provider on views

* perf(ecs): ECS alarms to be cached/searched with EcsClusterName id
  • Loading branch information
@christosarvanitis
christosarvanitis authored Sep 25, 2024
1 parent 52c0dfe commit 3cdf32e
Show file tree
Hide file tree
Showing 19 changed files with 456 additions and 117 deletions.
4 changes: 2 additions & 2 deletions clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/Keys.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 12 additions & 0 deletions ...src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/AbstractCacheClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ public Collection<T> getAll(String account, String region) {
return convertAll(data);
}

public Collection<T> getAll(Collection<String> identifiers) {
Collection<CacheData> 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.
Expand Down Expand Up @@ -110,4 +118,8 @@ private Collection<CacheData> fetchFromCache(String account, String region) {

return allData;
}

public Collection<String> filterIdentifiers(String glob) {
return cacheView.filterIdentifiers(keyNamespace, glob);
}
}
33 changes: 11 additions & 22 deletions ...ava/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsCloudWatchAlarmCacheClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,31 +72,19 @@ protected EcsMetricAlarm convert(CacheData cacheData) {
}

public List<EcsMetricAlarm> getMetricAlarms(
String serviceName, String accountName, String region) {
String serviceName, String accountName, String region, String ecsClusterName) {
List<EcsMetricAlarm> metricAlarms = new LinkedList<>();
Collection<EcsMetricAlarm> 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<String> metricAlarmsIds = filterIdentifiers(glob);
Collection<EcsMetricAlarm> 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);
}
}

Expand Down
5 changes: 4 additions & 1 deletion .../java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DestroyServiceAtomicOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 6 additions & 1 deletion ...ddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/model/EcsApplication.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,16 @@ public class EcsApplication implements Application {
private String name;
Map<String, String> attributes;
Map<String, Set<String>> clusterNames;
Map<String, Set<String>> clusterNameMetadata;

public EcsApplication(
String name, Map<String, String> attributes, Map<String, Set<String>> clusterNames) {
String name,
Map<String, String> attributes,
Map<String, Set<String>> clusterNames,
Map<String, Set<String>> getClusterNameMetadata) {
this.name = name;
this.attributes = attributes;
this.clusterNames = clusterNames;
this.clusterNameMetadata = getClusterNameMetadata;
}
}
8 changes: 7 additions & 1 deletion ...com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,13 @@ Map<String, Collection<CacheData>> generateFreshData(Set<MetricAlarm> cacheableM
Map<String, Collection<CacheData>> 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<String, Object> attributes =
convertMetricAlarmToAttributes(metricAlarm, accountName, region);

Expand Down
5 changes: 4 additions & 1 deletion ...src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsClusterProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,9 +56,11 @@ public Collection<EcsCluster> 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<Cluster> getEcsClusterDescriptions(String account, String region) {
String glob = Keys.getClusterKey(account, region, "*");
Collection<String> ecsClustersIdentifiers = ecsClusterCacheClient.filterIdentifiers(glob);
Collection<Cluster> clusters = new ArrayList<>();
List<String> filteredEcsClusters =
ecsClusterCacheClient.getAll().stream()
ecsClusterCacheClient.getAll(ecsClustersIdentifiers).stream()
.filter(
cluster ->
account.equals(cluster.getAccount()) && region.equals(cluster.getRegion()))
Expand Down
8 changes: 7 additions & 1 deletion ...ain/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsLoadBalancerProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -118,8 +119,13 @@ public List<Details> byAccountAndRegionAndName(String account, String region, St
@Override
public Set<EcsLoadBalancer> 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<String> ecsServices = ecsServiceCacheClient.filterIdentifiers(glob);
Set<Service> 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);
Expand Down
116 changes: 73 additions & 43 deletions ...in/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,17 @@ public EcsServerClusterProvider(

private Map<String, Set<EcsServerCluster>> findClusters(
Map<String, Set<EcsServerCluster>> clusterMap, AmazonCredentials credentials) {
return findClusters(clusterMap, credentials, null);
return findClusters(clusterMap, credentials, null, true);
}

private Map<String, Set<EcsServerCluster>> findClusters(
Map<String, Set<EcsServerCluster>> 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;
Expand All @@ -118,10 +120,16 @@ private Map<String, Set<EcsServerCluster>> findClustersForRegion(
Map<String, Set<EcsServerCluster>> clusterMap,
AmazonCredentials credentials,
AmazonCredentials.AWSRegion awsRegion,
String application) {
String application,
boolean includeDetails) {

Collection<Service> 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<String> ecsServices = serviceCacheClient.filterIdentifiers(glob);
Collection<Service> services = serviceCacheClient.getAll(ecsServices);
Collection<Task> allTasks = taskCacheClient.getAll(credentials.getName(), awsRegion.getName());

for (Service service : services) {
Expand Down Expand Up @@ -167,7 +175,8 @@ private Map<String, Set<EcsServerCluster>> findClustersForRegion(
service.getClusterName(),
taskDefinition,
service.getSubnets(),
service.getSecurityGroups());
service.getSecurityGroups(),
includeDetails);

if (ecsServerGroup == null) {
continue;
Expand Down Expand Up @@ -308,13 +317,9 @@ private EcsServerGroup buildEcsServerGroup(
String ecsClusterName,
com.amazonaws.services.ecs.model.TaskDefinition taskDefinition,
List<String> eniSubnets,
List<String> eniSecurityGroups) {
List<String> 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);
Expand Down Expand Up @@ -368,31 +373,52 @@ private EcsServerGroup buildEcsServerGroup(
}
}
}

Set<String> 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())
Expand Down Expand Up @@ -456,26 +482,30 @@ private AmazonCredentials getEcsCredentials(String account) {

@Override
public Map<String, Set<EcsServerCluster>> getClusterSummaries(String application) {
return getClusters();
return getClusters0(application, false);
}

@Override
public Map<String, Set<EcsServerCluster>> getClusterDetails(String application) {
return getClusters0(application, true);
}

@Override
public Map<String, Set<EcsServerCluster>> getClusters() {
Map<String, Set<EcsServerCluster>> clusterMap = new HashMap<>();

for (AmazonCredentials credentials : getEcsCredentials()) {
clusterMap = findClusters(clusterMap, credentials, application);
clusterMap = findClusters(clusterMap, credentials);
}

return clusterMap;
}

@Override
public Map<String, Set<EcsServerCluster>> getClusters() {
private Map<String, Set<EcsServerCluster>> getClusters0(
String application, boolean includeDetails) {
Map<String, Set<EcsServerCluster>> clusterMap = new HashMap<>();

for (AmazonCredentials credentials : getEcsCredentials()) {
clusterMap = findClusters(clusterMap, credentials);
clusterMap = findClusters(clusterMap, credentials, application, includeDetails);
}
return clusterMap;
}
Expand All @@ -485,7 +515,7 @@ public Map<String, Set<EcsServerCluster>> getClusters() {
public Set<EcsServerCluster> 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;
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions ...s/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<EcsMetricAlarm> metricAlarms =
metricAlarmCacheClient.getMetricAlarms(serviceName, account, region);
metricAlarmCacheClient.getMetricAlarms(ecsClusterName, serviceName, account, region);

if (metricAlarms.isEmpty()) {
return;
Expand Down
Loading

0 comments on commit 3cdf32e

Please sign in to comment.