Skip to content

Commit 3e714e2

Browse files
christosarvanitischristosarvanitis
committed
perf(ecs): ECS alarms to be cached/searched with EcsClusterName id
1 parent 6d6ec79 commit 3e714e2

File tree

11 files changed

+93
-75
lines changed

11 files changed

+93
-75
lines changed

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/Keys.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ public static String getTaskDefinitionKey(
201201
return buildKey(Namespace.TASK_DEFINITIONS.ns, account, region, taskDefinitionArn);
202202
}
203203

204-
public static String getAlarmKey(String account, String region, String alarmArn) {
205-
return buildKey(Namespace.ALARMS.ns, account, region, alarmArn);
204+
public static String getAlarmKey(String account, String region, String alarmArn, String cluster) {
205+
return buildKey(Namespace.ALARMS.ns, account, region, alarmArn + SEPARATOR + cluster);
206206
}
207207

208208
public static String getScalableTargetKey(String account, String region, String resourceId) {

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/AbstractCacheClient.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ public Collection<T> getAll(String account, String region) {
6666
}
6767

6868
public Collection<T> getAll(Collection<String> identifiers) {
69-
Collection<CacheData> allData = cacheView.getAll(keyNamespace, identifiers);
70-
return convertAll(allData);
69+
return convertAll(cacheView.getAll(keyNamespace, identifiers));
7170
}
7271

7372
/**

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/cache/client/EcsCloudWatchAlarmCacheClient.java‎

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.netflix.spinnaker.cats.cache.Cache;
2222
import com.netflix.spinnaker.cats.cache.CacheData;
23+
import com.netflix.spinnaker.clouddriver.ecs.cache.Keys;
2324
import com.netflix.spinnaker.clouddriver.ecs.cache.model.EcsMetricAlarm;
2425
import java.util.Collection;
2526
import java.util.Collections;
@@ -71,33 +72,19 @@ protected EcsMetricAlarm convert(CacheData cacheData) {
7172
}
7273

7374
public List<EcsMetricAlarm> getMetricAlarms(
74-
String serviceName, String accountName, String region) {
75+
String serviceName, String accountName, String region, String ecsClusterName) {
7576
List<EcsMetricAlarm> metricAlarms = new LinkedList<>();
76-
// we can filter more here.
7777

78-
Collection<EcsMetricAlarm> allMetricAlarms = getAll(accountName, region);
78+
String glob = Keys.getAlarmKey(accountName, region, "*", ecsClusterName);
79+
Collection<String> metricAlarmsIds = filterIdentifiers(glob);
80+
Collection<EcsMetricAlarm> allMetricAlarms = getAll(metricAlarmsIds);
7981

80-
outLoop:
8182
for (EcsMetricAlarm metricAlarm : allMetricAlarms) {
82-
for (String action : metricAlarm.getAlarmActions()) {
83-
if (action.contains(serviceName)) {
84-
metricAlarms.add(metricAlarm);
85-
continue outLoop;
86-
}
87-
}
88-
89-
for (String action : metricAlarm.getOKActions()) {
90-
if (action.contains(serviceName)) {
91-
metricAlarms.add(metricAlarm);
92-
continue outLoop;
93-
}
94-
}
95-
96-
for (String action : metricAlarm.getInsufficientDataActions()) {
97-
if (action.contains(serviceName)) {
98-
metricAlarms.add(metricAlarm);
99-
continue outLoop;
100-
}
83+
if (metricAlarm.getAlarmActions().stream().anyMatch(action -> action.contains(serviceName))
84+
|| metricAlarm.getOKActions().stream().anyMatch(action -> action.contains(serviceName))
85+
|| metricAlarm.getInsufficientDataActions().stream()
86+
.anyMatch(action -> action.contains(serviceName))) {
87+
metricAlarms.add(metricAlarm);
10188
}
10289
}
10390

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/deploy/ops/DestroyServiceAtomicOperation.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ public Void operate(List priorOutputs) {
4545

4646
updateTaskStatus("Removing MetricAlarms from " + description.getServerGroupName() + ".");
4747
ecsCloudMetricService.deleteMetrics(
48-
description.getServerGroupName(), description.getAccount(), description.getRegion());
48+
description.getServerGroupName(),
49+
description.getAccount(),
50+
description.getRegion(),
51+
ecsClusterName);
4952
updateTaskStatus("Done removing MetricAlarms from " + description.getServerGroupName() + ".");
5053

5154
UpdateServiceRequest updateServiceRequest = new UpdateServiceRequest();

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/agent/EcsCloudMetricAlarmCachingAgent.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,13 @@ Map<String, Collection<CacheData>> generateFreshData(Set<MetricAlarm> cacheableM
118118
Map<String, Collection<CacheData>> newDataMap = new HashMap<>();
119119

120120
for (MetricAlarm metricAlarm : cacheableMetricAlarm) {
121-
String key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn());
121+
String cluster =
122+
metricAlarm.getDimensions().stream()
123+
.filter(t -> t.getName().equalsIgnoreCase("ClusterName"))
124+
.map(t -> t.getValue())
125+
.collect(Collectors.joining());
126+
127+
String key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn(), cluster);
122128
Map<String, Object> attributes =
123129
convertMetricAlarmToAttributes(metricAlarm, accountName, region);
124130

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProvider.java‎

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,17 @@ public EcsServerClusterProvider(
100100

101101
private Map<String, Set<EcsServerCluster>> findClusters(
102102
Map<String, Set<EcsServerCluster>> clusterMap, AmazonCredentials credentials) {
103-
return findClusters(clusterMap, credentials, null);
103+
return findClusters(clusterMap, credentials, null, true);
104104
}
105105

106106
private Map<String, Set<EcsServerCluster>> findClusters(
107107
Map<String, Set<EcsServerCluster>> clusterMap,
108108
AmazonCredentials credentials,
109-
String application) {
109+
String application,
110+
boolean inludeDetails) {
110111
for (AmazonCredentials.AWSRegion awsRegion : credentials.getRegions()) {
111-
clusterMap = findClustersForRegion(clusterMap, credentials, awsRegion, application);
112+
clusterMap =
113+
findClustersForRegion(clusterMap, credentials, awsRegion, application, inludeDetails);
112114
}
113115

114116
return clusterMap;
@@ -118,7 +120,8 @@ private Map<String, Set<EcsServerCluster>> findClustersForRegion(
118120
Map<String, Set<EcsServerCluster>> clusterMap,
119121
AmazonCredentials credentials,
120122
AmazonCredentials.AWSRegion awsRegion,
121-
String application) {
123+
String application,
124+
boolean includeDetails) {
122125

123126
String glob =
124127
application != null
@@ -172,7 +175,8 @@ private Map<String, Set<EcsServerCluster>> findClustersForRegion(
172175
service.getClusterName(),
173176
taskDefinition,
174177
service.getSubnets(),
175-
service.getSecurityGroups());
178+
service.getSecurityGroups(),
179+
includeDetails);
176180

177181
if (ecsServerGroup == null) {
178182
continue;
@@ -313,13 +317,9 @@ private EcsServerGroup buildEcsServerGroup(
313317
String ecsClusterName,
314318
com.amazonaws.services.ecs.model.TaskDefinition taskDefinition,
315319
List<String> eniSubnets,
316-
List<String> eniSecurityGroups) {
320+
List<String> eniSecurityGroups,
321+
boolean includeDetails) {
317322
ServerGroup.InstanceCounts instanceCounts = buildInstanceCount(instances);
318-
TaskDefinition ecsTaskDefinition = buildTaskDefinition(taskDefinition);
319-
EcsServerGroup.Image image = new EcsServerGroup.Image();
320-
image.setImageId(ecsTaskDefinition.getContainerImage());
321-
image.setName(ecsTaskDefinition.getContainerImage());
322-
323323
String scalableTargetId = "service/" + ecsClusterName + "/" + serviceName;
324324
String scalableTargetKey = Keys.getScalableTargetKey(account, region, scalableTargetId);
325325
ScalableTarget scalableTarget = scalableTargetCacheClient.get(scalableTargetKey);
@@ -373,31 +373,52 @@ private EcsServerGroup buildEcsServerGroup(
373373
}
374374
}
375375
}
376-
377376
Set<String> metricAlarmNames =
378-
ecsCloudWatchAlarmCacheClient.getMetricAlarms(serviceName, account, region).stream()
377+
ecsCloudWatchAlarmCacheClient
378+
.getMetricAlarms(serviceName, account, region, ecsClusterName)
379+
.stream()
379380
.map(EcsMetricAlarm::getAlarmName)
380381
.collect(Collectors.toSet());
381-
382-
EcsServerGroup serverGroup =
383-
new EcsServerGroup()
384-
.setDisabled(capacity.getDesired() == 0)
385-
.setName(serviceName)
386-
.setCloudProvider(EcsCloudProvider.ID)
387-
.setType(EcsCloudProvider.ID)
388-
.setRegion(region)
389-
.setInstances(instances)
390-
.setCapacity(capacity)
391-
.setImage(image)
392-
.setInstanceCounts(instanceCounts)
393-
.setCreatedTime(creationTime)
394-
.setEcsCluster(ecsClusterName)
395-
.setTaskDefinition(ecsTaskDefinition)
396-
.setVpcId(vpcId)
397-
.setSecurityGroups(securityGroups)
398-
.setMetricAlarms(metricAlarmNames)
399-
.setMoniker(moniker);
400-
382+
EcsServerGroup serverGroup = new EcsServerGroup();
383+
if (includeDetails) {
384+
TaskDefinition ecsTaskDefinition = buildTaskDefinition(taskDefinition);
385+
EcsServerGroup.Image image = new EcsServerGroup.Image();
386+
image.setImageId(ecsTaskDefinition.getContainerImage());
387+
image.setName(ecsTaskDefinition.getContainerImage());
388+
serverGroup
389+
.setDisabled(capacity.getDesired() == 0)
390+
.setName(serviceName)
391+
.setCloudProvider(EcsCloudProvider.ID)
392+
.setType(EcsCloudProvider.ID)
393+
.setRegion(region)
394+
.setInstances(instances)
395+
.setCapacity(capacity)
396+
.setImage(image)
397+
.setInstanceCounts(instanceCounts)
398+
.setCreatedTime(creationTime)
399+
.setEcsCluster(ecsClusterName)
400+
.setTaskDefinition(ecsTaskDefinition)
401+
.setVpcId(vpcId)
402+
.setSecurityGroups(securityGroups)
403+
.setMetricAlarms(metricAlarmNames)
404+
.setMoniker(moniker);
405+
} else {
406+
serverGroup
407+
.setDisabled(capacity.getDesired() == 0)
408+
.setName(serviceName)
409+
.setCloudProvider(EcsCloudProvider.ID)
410+
.setType(EcsCloudProvider.ID)
411+
.setRegion(region)
412+
.setInstances(instances)
413+
.setCapacity(capacity)
414+
.setInstanceCounts(instanceCounts)
415+
.setCreatedTime(creationTime)
416+
.setEcsCluster(ecsClusterName)
417+
.setVpcId(vpcId)
418+
.setSecurityGroups(securityGroups)
419+
.setMetricAlarms(metricAlarmNames)
420+
.setMoniker(moniker);
421+
}
401422
EcsServerGroup.AutoScalingGroup asg =
402423
new EcsServerGroup.AutoScalingGroup()
403424
.setDesiredCapacity(scalableTarget.getMaxCapacity())
@@ -461,12 +482,12 @@ private AmazonCredentials getEcsCredentials(String account) {
461482

462483
@Override
463484
public Map<String, Set<EcsServerCluster>> getClusterSummaries(String application) {
464-
return getClusters0(application);
485+
return getClusters0(application, false);
465486
}
466487

467488
@Override
468489
public Map<String, Set<EcsServerCluster>> getClusterDetails(String application) {
469-
return getClusters0(application);
490+
return getClusters0(application, true);
470491
}
471492

472493
@Override
@@ -479,11 +500,12 @@ public Map<String, Set<EcsServerCluster>> getClusters() {
479500
return clusterMap;
480501
}
481502

482-
public Map<String, Set<EcsServerCluster>> getClusters0(String application) {
503+
private Map<String, Set<EcsServerCluster>> getClusters0(
504+
String application, boolean includeDetails) {
483505
Map<String, Set<EcsServerCluster>> clusterMap = new HashMap<>();
484506

485507
for (AmazonCredentials credentials : getEcsCredentials()) {
486-
clusterMap = findClusters(clusterMap, credentials, application);
508+
clusterMap = findClusters(clusterMap, credentials, application, includeDetails);
487509
}
488510
return clusterMap;
489511
}
@@ -493,7 +515,7 @@ public Map<String, Set<EcsServerCluster>> getClusters0(String application) {
493515
public Set<EcsServerCluster> getClusters(String application, String account) {
494516
try {
495517
AmazonCredentials credentials = getEcsCredentials(account);
496-
return findClusters(new HashMap<>(), credentials, application).get(application);
518+
return findClusters(new HashMap<>(), credentials, application, true).get(application);
497519
} catch (NoSuchElementException exception) {
498520
log.info("No ECS Credentials were found for account " + account);
499521
return null;
@@ -544,7 +566,7 @@ public ServerGroup getServerGroup(
544566
AmazonCredentials credentials = getEcsCredentials(account);
545567
Moniker moniker = MonikerHelper.applicationNameToMoniker(serverGroupName);
546568
log.debug("App Name is: " + moniker.getApp());
547-
clusterMap = findClusters(clusterMap, credentials, moniker.getApp());
569+
clusterMap = findClusters(clusterMap, credentials, moniker.getApp(), true);
548570
} catch (NoSuchElementException exception) {
549571
/* This is ugly, but not sure how else to do it. If we don't have creds due
550572
* to not being an ECS account, there's nothing to do here, and we should

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricService.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,10 @@ public class EcsCloudMetricService {
4343

4444
private final Logger log = LoggerFactory.getLogger(getClass());
4545

46-
public void deleteMetrics(String serviceName, String account, String region) {
46+
public void deleteMetrics(
47+
String serviceName, String account, String region, String ecsClusterName) {
4748
List<EcsMetricAlarm> metricAlarms =
48-
metricAlarmCacheClient.getMetricAlarms(serviceName, account, region);
49+
metricAlarmCacheClient.getMetricAlarms(ecsClusterName, serviceName, account, region);
4950

5051
if (metricAlarms.isEmpty()) {
5152
return;

‎clouddriver-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ public EcsApplicationProvider(
5151

5252
@Override
5353
public Application getApplication(String name) {
54-
name = name.toLowerCase();
5554
String glob = Keys.getServiceKey("*", "*", name + "*");
5655
Collection<String> ecsServices = serviceCacheClient.filterIdentifiers(glob);
5756
for (Application application : populateApplicationSet(ecsServices, true)) {

‎clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/cache/EcsCloudWatchAlarmCacheClientSpec.groovy‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ class EcsCloudWatchAlarmCacheClientSpec extends Specification {
3333
given:
3434
def accountName = 'test-account-1'
3535
def region = 'us-west-1'
36+
def ecsClusterName = 'my-cluster'
3637
def metricAlarm = new EcsMetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn").withRegion(region).withAccountName(accountName)
37-
def key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn())
38+
def key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn(), ecsClusterName)
3839
def attributes = EcsCloudMetricAlarmCachingAgent.convertMetricAlarmToAttributes(metricAlarm, accountName, region)
3940

4041
when:

‎clouddriver-ecs/src/test/groovy/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ class EcsServerClusterProviderSpec extends Specification {
191191
containerInformationService.getTaskZone(_, _, _) >> 'us-west-1a'
192192
taskDefinitionCacheClient.get(_) >> cachedTaskDefinition
193193
scalableTargetCacheClient.get(_) >> scalableTarget
194-
ecsCloudWatchAlarmCacheClient.getMetricAlarms(_, _, _) >> []
194+
ecsCloudWatchAlarmCacheClient.getMetricAlarms(_, _,_ ,_) >> []
195195
subnetSelector.getSubnetVpcIds(_, _, _) >> ['vpc-1234']
196196

197197
cacheView.filterIdentifiers(_, _) >> ['key']

0 commit comments

Comments
 (0)