Skip to content

Commit

Permalink
perf(ecs): ECS alarms to be cached/searched with EcsClusterName id
Browse files Browse the repository at this point in the history
  • Loading branch information
@christosarvanitis
christosarvanitis committed Sep 12, 2024
1 parent 6d6ec79 commit 3e714e2
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 75 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
3 changes: 1 addition & 2 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 @@ -66,8 +66,7 @@ public Collection<T> getAll(String account, String region) {
}

public Collection<T> getAll(Collection<String> identifiers) {
Collection<CacheData> allData = cacheView.getAll(keyNamespace, identifiers);
return convertAll(allData);
return convertAll(cacheView.getAll(keyNamespace, identifiers));
}

/**
Expand Down
33 changes: 10 additions & 23 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,33 +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<>();
// we can filter more here.

Collection<EcsMetricAlarm> allMetricAlarms = getAll(accountName, region);
String glob = Keys.getAlarmKey(accountName, region, "*", ecsClusterName);
Collection<String> metricAlarmsIds = filterIdentifiers(glob);
Collection<EcsMetricAlarm> allMetricAlarms = getAll(metricAlarmsIds);

outLoop:
for (EcsMetricAlarm metricAlarm : allMetricAlarms) {
for (String action : metricAlarm.getAlarmActions()) {
if (action.contains(serviceName)) {
metricAlarms.add(metricAlarm);
continue outLoop;
}
}

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;
}
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
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
100 changes: 61 additions & 39 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,7 +120,8 @@ private Map<String, Set<EcsServerCluster>> findClustersForRegion(
Map<String, Set<EcsServerCluster>> clusterMap,
AmazonCredentials credentials,
AmazonCredentials.AWSRegion awsRegion,
String application) {
String application,
boolean includeDetails) {

String glob =
application != null
Expand Down Expand Up @@ -172,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 @@ -313,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 @@ -373,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 @@ -461,12 +482,12 @@ private AmazonCredentials getEcsCredentials(String account) {

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

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

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

public Map<String, Set<EcsServerCluster>> getClusters0(String application) {
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, application);
clusterMap = findClusters(clusterMap, credentials, application, includeDetails);
}
return clusterMap;
}
Expand All @@ -493,7 +515,7 @@ public Map<String, Set<EcsServerCluster>> getClusters0(String application) {
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 @@ -544,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
1 change: 0 additions & 1 deletion ...-ecs/src/main/java/com/netflix/spinnaker/clouddriver/ecs/view/EcsApplicationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public EcsApplicationProvider(

@Override
public Application getApplication(String name) {
name = name.toLowerCase();
String glob = Keys.getServiceKey("*", "*", name + "*");
Collection<String> ecsServices = serviceCacheClient.filterIdentifiers(glob);
for (Application application : populateApplicationSet(ecsServices, true)) {
Expand Down
3 changes: 2 additions & 1 deletion ...oovy/com/netflix/spinnaker/clouddriver/ecs/cache/EcsCloudWatchAlarmCacheClientSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ class EcsCloudWatchAlarmCacheClientSpec extends Specification {
given:
def accountName = 'test-account-1'
def region = 'us-west-1'
def ecsClusterName = 'my-cluster'
def metricAlarm = new EcsMetricAlarm().withAlarmName("alarm-name").withAlarmArn("alarmArn").withRegion(region).withAccountName(accountName)
def key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn())
def key = Keys.getAlarmKey(accountName, region, metricAlarm.getAlarmArn(), ecsClusterName)
def attributes = EcsCloudMetricAlarmCachingAgent.convertMetricAlarmToAttributes(metricAlarm, accountName, region)

when:
Expand Down
2 changes: 1 addition & 1 deletion ...y/com/netflix/spinnaker/clouddriver/ecs/provider/view/EcsServerClusterProviderSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down
4 changes: 2 additions & 2 deletions ...st/groovy/com/netflix/spinnaker/clouddriver/ecs/services/EcsCloudMetricServiceSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,10 +413,10 @@ 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(_)
Expand Down

0 comments on commit 3e714e2

Please sign in to comment.