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
  • Loading branch information
christosarvanitis committed Aug 6, 2024
1 parent a5fc30d commit 36aa330
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ 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);
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 +115,8 @@ private Collection<CacheData> fetchFromCache(String account, String region) {

return allData;
}

public Collection<String> filterIdentifiers(String glob) {
return cacheView.filterIdentifiers(keyNamespace, glob);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ protected EcsMetricAlarm convert(CacheData cacheData) {
public List<EcsMetricAlarm> getMetricAlarms(
String serviceName, String accountName, String region) {
List<EcsMetricAlarm> metricAlarms = new LinkedList<>();
// we can filter more here.

Collection<EcsMetricAlarm> allMetricAlarms = getAll(accountName, region);

outLoop:
Expand Down
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;
}
}
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,13 @@ private Map<String, Set<EcsServerCluster>> findClustersForRegion(
AmazonCredentials.AWSRegion awsRegion,
String application) {

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 @@ -456,7 +461,7 @@ private AmazonCredentials getEcsCredentials(String account) {

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

@Override
Expand All @@ -480,6 +485,15 @@ public Map<String, Set<EcsServerCluster>> getClusters() {
return clusterMap;
}

public Map<String, Set<EcsServerCluster>> getClusters0(String application) {
Map<String, Set<EcsServerCluster>> clusterMap = new HashMap<>();

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

/** Gets Spinnaker clusters for a given Spinnaker application and ECS account. */
@Override
public Set<EcsServerCluster> getClusters(String application, String account) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -50,31 +51,32 @@ public EcsApplicationProvider(

@Override
public Application getApplication(String name) {

for (Application application : getApplications(true)) {
name = name.toLowerCase();
String glob = Keys.getServiceKey("*", "*", name + "*");
Collection<String> ecsServices = serviceCacheClient.filterIdentifiers(glob);
for (Application application : populateApplicationSet(ecsServices, true)) {
if (name.equals(application.getName())) {
return application;
}
}

return null;
}

@Override
public Set<Application> getApplications(boolean expand) {
Set<Application> applications = new HashSet<>();

public Set<EcsApplication> getApplications(boolean expand) {
Set<EcsApplication> applications = new HashSet<>();
for (NetflixECSCredentials credentials : credentialsRepository.getAll()) {
Set<Application> retrievedApplications = findApplicationsForAllRegions(credentials, expand);
Set<EcsApplication> retrievedApplications =
findApplicationsForAllRegions(credentials, expand);
applications.addAll(retrievedApplications);
}

return applications;
}

private Set<Application> findApplicationsForAllRegions(
private Set<EcsApplication> findApplicationsForAllRegions(
AmazonCredentials credentials, boolean expand) {
Set<Application> applications = new HashSet<>();
Set<EcsApplication> applications = new HashSet<>();

for (AmazonCredentials.AWSRegion awsRegion : credentials.getRegions()) {
applications.addAll(
Expand All @@ -84,16 +86,16 @@ private Set<Application> findApplicationsForAllRegions(
return applications;
}

private Set<Application> findApplicationsForRegion(
private Set<EcsApplication> findApplicationsForRegion(
String account, String region, boolean expand) {
HashMap<String, Application> applicationHashMap =
HashMap<String, EcsApplication> applicationHashMap =
populateApplicationMap(account, region, expand);
return transposeApplicationMapToSet(applicationHashMap);
}

private HashMap<String, Application> populateApplicationMap(
private HashMap<String, EcsApplication> populateApplicationMap(
String account, String region, boolean expand) {
HashMap<String, Application> applicationHashMap = new HashMap<>();
HashMap<String, EcsApplication> applicationHashMap = new HashMap<>();
Collection<Service> services = serviceCacheClient.getAll(account, region);

for (Service service : services) {
Expand All @@ -102,19 +104,32 @@ private HashMap<String, Application> populateApplicationMap(
return applicationHashMap;
}

private Set<Application> transposeApplicationMapToSet(
HashMap<String, Application> applicationHashMap) {
Set<Application> applications = new HashSet<>();
private Set<EcsApplication> populateApplicationSet(
Collection<String> identifiers, boolean expand) {
HashMap<String, EcsApplication> applicationHashMap = new HashMap<>();
Collection<Service> services = serviceCacheClient.getAll(identifiers);

for (Map.Entry<String, Application> entry : applicationHashMap.entrySet()) {
for (Service service : services) {
if (credentialsRepository.has(service.getAccount())) {
applicationHashMap = inferApplicationFromServices(applicationHashMap, service, expand);
}
}
return transposeApplicationMapToSet(applicationHashMap);
}

private Set<EcsApplication> transposeApplicationMapToSet(
HashMap<String, EcsApplication> applicationHashMap) {
Set<EcsApplication> applications = new HashSet<>();

for (Map.Entry<String, EcsApplication> entry : applicationHashMap.entrySet()) {
applications.add(entry.getValue());
}

return applications;
}

private HashMap<String, Application> inferApplicationFromServices(
HashMap<String, Application> applicationHashMap, Service service, boolean expand) {
private HashMap<String, EcsApplication> inferApplicationFromServices(
HashMap<String, EcsApplication> applicationHashMap, Service service, boolean expand) {

HashMap<String, String> attributes = new HashMap<>();
Moniker moniker = service.getMoniker();
Expand All @@ -125,18 +140,31 @@ private HashMap<String, Application> inferApplicationFromServices(
attributes.put("name", appName);

HashMap<String, Set<String>> clusterNames = new HashMap<>();
HashMap<String, Set<String>> 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());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,24 @@ class EcsClusterProviderSpec extends Specification {
Set<String> clusterNames = new HashSet<>()
Collection<CacheData> cacheData = new HashSet<>()
Collection<Cluster> clustersResponse = new ArrayList<>()
Collection<String> ecsClustersIdentifiers = new ArrayList<>()
clusterNames.add("example-app-test-Cluster-NSnYsTXmCfV2")
clusterNames.add("TestCluster")
clusterNames.add("spinnaker-deployment-cluster")

for (int x = 0; x < numberOfClusters; x++) {
String clusterKey = Keys.getClusterKey(ACCOUNT, REGION, clusterNames[x])
Map<String, Object> 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])
attributes.put("clusterName", clusterNames[x])

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()
Expand Down Expand Up @@ -102,12 +105,14 @@ class EcsClusterProviderSpec extends Specification {
Set<String> clusterNames = new HashSet<>()
Collection<CacheData> cacheData = new HashSet<>()
Collection<Cluster> clustersResponse = new ArrayList<>()
Collection<String> 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<String, Object> attributes = new HashMap<>()
attributes.put("account", ACCOUNT)
attributes.put("region", REGION)
Expand All @@ -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++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<String, Set<String>> clusterNames = new HashMap<>()
clusterNames.put(accountName, Collections.singleton(serviceName))
Map<String, Set<String>> 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,
Expand All @@ -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)
Expand Down
Loading

0 comments on commit 36aa330

Please sign in to comment.