Skip to content

Commit

Permalink
fix(provider/cf): Adjust lookup of lb's per spinnaker app to avoid re…
Browse files Browse the repository at this point in the history
…trieving all lb cache (#4582) (#4589)

(cherry picked from commit dc9337e)

Co-authored-by: Zach Smith <33258732+zachsmith1@users.noreply.github.com>
Co-authored-by: Ethan Rogers <ethanfrogers@users.noreply.github.com>
  • Loading branch information
3 people committed May 18, 2020
1 parent 8905bea commit 28c205d
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,36 @@ private CloudFoundryLoadBalancer loadBalancerFromCacheData(CacheData lbData, Det
findServerGroupsByKeys(lbData.getRelationships().get(SERVER_GROUPS.getNs()), Detail.NONE));
}

public Set<CloudFoundryLoadBalancer> findLoadBalancersByClusterKeys(
Collection<String> keys, Detail detail) {
Set<String> serverGroupKeys =
cacheView.getAll(CLUSTERS.getNs(), keys).stream()
.flatMap(cl -> cl.getRelationships().get(SERVER_GROUPS.getNs()).stream())
.collect(toSet());

Set<String> loadBalancerKeys =
cacheView.getAll(SERVER_GROUPS.getNs(), serverGroupKeys).stream()
.flatMap(
sg ->
sg.getRelationships().get(LOAD_BALANCERS.getNs()).stream()
.map(
lb ->
Keys.getLoadBalancerKey(
objectMapper
.convertValue(
sg.getAttributes().get("resource"),
CloudFoundryServerGroup.class)
.getAccount(),
lb)))
.collect(toSet());

return findLoadBalancersByKeys(
loadBalancerKeys.stream()
.flatMap(lb -> cacheView.filterIdentifiers(LOAD_BALANCERS.getNs(), lb).stream())
.collect(toSet()),
detail);
}

public Set<CloudFoundryInstance> findInstancesByKeys(Collection<String> keys) {
return cacheView.getAll(INSTANCES.getNs(), keys).stream()
.map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ public static String getLoadBalancerKey(String account, CloudFoundryLoadBalancer
+ lb.getRegion();
}

public static String getLoadBalancerKey(String account, String guid) {
return ID + ":" + Namespace.LOAD_BALANCERS + ":" + account + ":" + guid + ":*";
}

public static String getLoadBalancerKey(String account, String uri, String region) {
Pattern VALID_ROUTE_REGEX =
Pattern.compile("^([a-zA-Z0-9_-]+)\\.([a-zA-Z0-9_.-]+)(:[0-9]+)?([/a-zA-Z0-9_-]+)?$");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
package com.netflix.spinnaker.clouddriver.cloudfoundry.provider.agent;

import static com.netflix.spinnaker.cats.agent.AgentDataType.Authority.AUTHORITATIVE;
import static com.netflix.spinnaker.clouddriver.cloudfoundry.cache.Keys.Namespace.*;
import static com.netflix.spinnaker.clouddriver.core.provider.agent.Namespace.LOAD_BALANCERS;
import static com.netflix.spinnaker.clouddriver.core.provider.agent.Namespace.ON_DEMAND;
import static com.netflix.spinnaker.clouddriver.core.provider.agent.Namespace.SERVER_GROUPS;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static java.util.stream.Collectors.toSet;
Expand All @@ -39,7 +41,6 @@
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
import com.netflix.spinnaker.clouddriver.cloudfoundry.provider.CloudFoundryProvider;
import com.netflix.spinnaker.clouddriver.cloudfoundry.security.CloudFoundryCredentials;
import io.vavr.collection.HashMap;
import java.util.*;
import javax.annotation.Nullable;
import lombok.Getter;
Expand Down Expand Up @@ -84,12 +85,34 @@ public CacheResult loadData(ProviderCache providerCache) {
}
});

Map<String, CacheData> loadBalancersByServerGroupIds = new HashMap<>();
loadBalancers.stream()
.forEach(
lb ->
lb.getMappedApps().stream()
.forEach(
sg ->
loadBalancersByServerGroupIds
.computeIfAbsent(
sg.getId(),
(s) ->
new ResourceCacheData(
Keys.getServerGroupKey(
sg.getAccount(), sg.getName(), sg.getRegion()),
emptyMap(),
new java.util.HashMap<>()))
.getRelationships()
.computeIfAbsent(LOAD_BALANCERS.getNs(), k -> new HashSet<>())
.add(lb.getId())));

Map<String, Collection<CacheData>> results =
HashMap.<String, Collection<CacheData>>of(
io.vavr.collection.HashMap.of(
LOAD_BALANCERS.getNs(),
loadBalancers.stream()
.map(lb -> setCacheData(toKeep, lb, loadDataStart))
.collect(toSet()))
.collect(toSet()),
SERVER_GROUPS.getNs(),
loadBalancersByServerGroupIds.values())
.toJavaMap();

onDemandCacheData.forEach(this::processOnDemandCacheData);
Expand Down Expand Up @@ -205,7 +228,7 @@ public Collection<Map> pendingOnDemandRequests(ProviderCache providerCache) {
Map<String, String> details = Keys.parse(loadbalancerId).orElse(emptyMap());
Map<String, Object> attributes = it.getAttributes();

return HashMap.of(
return io.vavr.collection.HashMap.of(
"id",
loadbalancerId,
"details",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@

import static com.netflix.spinnaker.clouddriver.cloudfoundry.cache.CacheRepository.Detail.FULL;
import static com.netflix.spinnaker.clouddriver.cloudfoundry.cache.CacheRepository.Detail.NAMES_ONLY;
import static com.netflix.spinnaker.clouddriver.cloudfoundry.cache.Keys.Namespace.CLUSTERS;
import static com.netflix.spinnaker.clouddriver.cloudfoundry.cache.Keys.Namespace.LOAD_BALANCERS;
import static java.util.stream.Collectors.toSet;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.netflix.frigga.Names;
import com.netflix.spinnaker.cats.cache.Cache;
import com.netflix.spinnaker.clouddriver.cloudfoundry.CloudFoundryCloudProvider;
import com.netflix.spinnaker.clouddriver.cloudfoundry.cache.CacheRepository;
Expand Down Expand Up @@ -76,18 +75,9 @@ public List<CloudFoundryLoadBalancerDetail> byAccountAndRegionAndName(
*/
@Override
public Set<CloudFoundryLoadBalancer> getApplicationLoadBalancers(String application) {
return repository
.findLoadBalancersByKeys(
cacheView.filterIdentifiers(LOAD_BALANCERS.getNs(), Keys.getAllLoadBalancers()),
NAMES_ONLY)
.stream()
.filter(
lb ->
lb.getServerGroups().stream()
.anyMatch(
serverGroup ->
application.equals(Names.parseName(serverGroup.getName()).getApp())))
.collect(toSet());
return repository.findLoadBalancersByClusterKeys(
cacheView.filterIdentifiers(CLUSTERS.getNs(), Keys.getClusterKey("*", application, "*")),
NAMES_ONLY);
}

private Map<String, CloudFoundryLoadBalancerSummary> summarizeLoadBalancers(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ void loadDataShouldReturnCacheResultWithUpdatedData() {
LOAD_BALANCERS.getNs(),
HashSet.of(loadBalancerCacheData1, loadBalancerCacheData2).toJavaSet(),
ON_DEMAND.getNs(),
emptySet(),
SERVER_GROUPS.getNs(),
emptySet())
.toJavaMap();
CacheResult expectedCacheResult =
Expand All @@ -291,6 +293,80 @@ void loadDataShouldReturnCacheResultWithUpdatedData() {
assertThat(result).isEqualToComparingFieldByFieldRecursively(expectedCacheResult);
}

@Test
void loadDataShouldReturnCacheResultWithUpdatedDataAndServerGroups() {

CloudFoundryInstance instance1 = CloudFoundryInstance.builder().appGuid("ap-guid-1").build();

CloudFoundryServerGroup serverGroup1 =
CloudFoundryServerGroup.builder()
.account(accountName)
.id("sg-guid-1")
.name("demo")
.space(cloudFoundrySpace)
.instances(HashSet.of(instance1).toJavaSet())
.build();

CloudFoundryLoadBalancer loadBalancer1 =
CloudFoundryLoadBalancer.builder()
.account(accountName)
.id("lb-guid-1")
.domain(CloudFoundryDomain.builder().name("domain-name").build())
.mappedApps(HashSet.of(serverGroup1).toJavaSet())
.build();

when(mockProviderCache.getAll(any(), anyCollection())).thenReturn(emptySet());

Routes mockRoutes = mock(Routes.class);

when(mockRoutes.all()).thenReturn(List.of(loadBalancer1).toJavaList());

when(cloudFoundryClient.getRoutes()).thenReturn(mockRoutes);

CacheData serverGroupCacheData1 =
new ResourceCacheData(
Keys.getServerGroupKey(
serverGroup1.getAccount(), serverGroup1.getName(), cloudFoundrySpace.getRegion()),
emptyMap(),
Collections.singletonMap(
LOAD_BALANCERS.getNs(), HashSet.of(loadBalancer1.getId()).toJavaList()));

Map<String, CacheData> loadBalancersByServerGroupIds =
HashMap.of("1", serverGroupCacheData1).toJavaMap();

CacheData loadBalancerCacheData1 =
new ResourceCacheData(
Keys.getLoadBalancerKey(accountName, loadBalancer1),
cacheView(loadBalancer1),
Collections.singletonMap(
SERVER_GROUPS.getNs(),
HashSet.of(
Keys.getServerGroupKey(
serverGroup1.getAccount(),
serverGroup1.getName(),
cloudFoundrySpace.getRegion()))
.toJavaSet()));

Map<String, Collection<CacheData>> cacheResults =
HashMap.<String, Collection<CacheData>>of(
LOAD_BALANCERS.getNs(),
HashSet.of(loadBalancerCacheData1).toJavaSet(),
ON_DEMAND.getNs(),
emptySet(),
SERVER_GROUPS.getNs(),
loadBalancersByServerGroupIds.values())
.toJavaMap();

CacheResult expectedCacheResult =
new DefaultCacheResult(
cacheResults,
HashMap.<String, Collection<String>>of(ON_DEMAND.getNs(), emptySet()).toJavaMap());

CacheResult result = cloudFoundryLoadBalancerCachingAgent.loadData(mockProviderCache);

assertThat(result).isEqualToComparingFieldByFieldRecursively(expectedCacheResult);
}

@Test
void loadDataShouldReturnCacheResultWithDataFromOnDemandNamespace()
throws JsonProcessingException {
Expand Down Expand Up @@ -362,6 +438,8 @@ void loadDataShouldReturnCacheResultWithDataFromOnDemandNamespace()
LOAD_BALANCERS.getNs(),
HashSet.of(onDemandCacheResults).toJavaSet(),
ON_DEMAND.getNs(),
emptySet(),
SERVER_GROUPS.getNs(),
emptySet())
.toJavaMap();

Expand Down

0 comments on commit 28c205d

Please sign in to comment.