From 3d90bdeae3637e4c6b2e16cf8cd622902ef9e07c Mon Sep 17 00:00:00 2001 From: Eric Zimanyi Date: Thu, 30 Jul 2020 15:39:59 -0400 Subject: [PATCH] refactor(kubernetes): Clean up KubernetesCacheUtils class (#4764) * refactor(kubernetes): Extract getHandler to a function This is a minor refactor to make the upcoming changes cleaner; just pull out a function. * refactor(kubernetes): Fix type safety and null safety resourceModelFromCacheData is very unsafe; it does an unchecked cast and can also return null. The goal of that function is to allow custom overrides for how to convert cache data into a ServerGroup or a ServerGroupManager. (We have not implemented the ability to customize other types.) The problem is that we are checking if the handler is of the generic ModelHandler instead of the specific type we are interested in. We already have ServerGroupHandler and ServerGroupManagerHandler to handle these two specific cases. Split the generic resourceModelFromCacheData into to specific ones to handle these two cases. By checking if the handler implements the specific case, we can avoid unchecked casts. Finally, rather than return null if the handler doesn't specify an override, just use the default implementation. The goal here is only to allow people to override this if they want; if they don't override, we can just use the default. * refactor(kubernetes): Clean up CustomKubernetesHandlerFactory This class implemented the (raw) ModelHandler, but only ever implemented the default implementations for the kind in question. As we now fall back to the default implementations in cases where there is no override, we don't need to explicitly override them here. * refactor(kubernetes): Remove ModelHandler class Now that we're using the more specific handler classes to detect custom implementations of fromCacheData, we don't need the ModelHandler interface at all anymore; remove it. * refactor(kubernetes): Add relationshipTypes helper function Minor refactor to abstract away translating a Spinnaker kind into its relationship types. * refactor(kubernetes): Add relationshipKeys function Minor refactor to pull common logic looking up relationships on a CacheData object into a function. * refactor(kubernetes): Rename to function Rename aggregateRelationshipsBySpinnakerKind as getRelationshipKeys, because it does the same thing as the relationshipKeys function we just created, but for a SpinnakerKind. Also collect to an ImmutableList and update the return type. Similarly remove the OfSpinnakerKind from getRelationships, becase the function signature implies what the input is. * refactor(kubernetes): Update return type of mapByRelationship It will now return a Map> instead of a Map> because it's not actually important to callers that the values are List. This will make the next commit possible. * refactor(kubernetes): mapByRelationship returns a MultiMap This is a much better data structure for mapping keys to multiple values. In particular, it has the property that .get(key) returns an empty list rather than null if there are no mappings, so we can avoid a lot of defaulting. * refactor(kubernetes): Inline a variable Inline the keys variable in loadRelationshipsFromCache; also collect the relationship keys to a Set rather than to a List. This cuts down the number of times we look up load balancers from 4 to 2; we'll still need to get that down to 1 but this is progress. * refactor(kubernetes): Rename getTransitiveRelationships Rename this function to something shorter and clearer; also update it to only accept a single CacheData as all callers are wrapping a single CacheData in a collection when calling it. * refactor(kubernetes): Require callers of to create the filter Simple refactor to push requiring the cache filter to be created to callers. (Actually caller as this is only used in one place.) * refactor(kubernetes): Require callers of getRelationships to pass data getRelationships is inherently ineffecient, as it encourages callers to require a lookup of data they already have. In 2 of the 3 callers we are either looking up the same data in a loop, or already have the data we're looking up. Require callers to first look up the cache data, then pass it in to get its relationships. * refactor(kubernetes): Rename loadRelationshipsFromCache Use getRelationships to be consistent with how we're naming other functions in this class. Also update the single-data-consuming version to call the list version with a singleton. * refactor(kubernetes): Remove loadRelationshipsFromCache This is identical to getRelationships. Also the whole class is non-null now, so move the annotation to the class level. * refactor(kubernetes): Cleanup of KubernetesCacheUtils Make the class package-private as it is only used in the package. Also add Javadoc to some methods. * refactor(kubernetes): Replace kindMap lookups with getRelationships We already have a function in cacheUtils that looks up all relationships for a SpinnakerKind; replace cases where callers map the SpinnakerKind to KubernetesKinds with just calling this function. * style(kubernetes): Fix comments Co-Authored-By: Maggie Neterval Co-authored-by: Maggie Neterval --- .../view/model/KubernetesV2Cluster.java | 6 +- .../view/model/KubernetesV2LoadBalancer.java | 7 +- .../view/provider/KubernetesCacheUtils.java | 150 ++++++++---------- .../provider/KubernetesV2ClusterProvider.java | 99 ++++++------ .../KubernetesV2LoadBalancerProvider.java | 50 +++--- .../KubernetesV2ManifestProvider.java | 8 +- ...ubernetesV2ServerGroupManagerProvider.java | 33 ++-- .../CustomKubernetesHandlerFactory.java | 24 +-- .../kubernetes/op/handler/ModelHandler.java | 25 --- .../op/handler/ServerGroupHandler.java | 2 +- .../op/handler/ServerGroupManagerHandler.java | 3 +- ...KubernetesDataProviderIntegrationTest.java | 2 +- 12 files changed, 168 insertions(+), 241 deletions(-) delete mode 100644 clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ModelHandler.java diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2Cluster.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2Cluster.java index 399086ec52d..9824a4dc47b 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2Cluster.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2Cluster.java @@ -22,8 +22,8 @@ import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys; import com.netflix.spinnaker.clouddriver.model.Cluster; import com.netflix.spinnaker.moniker.Moniker; +import java.util.Collection; import java.util.HashSet; -import java.util.List; import java.util.Set; import lombok.Value; @@ -43,8 +43,8 @@ public KubernetesV2Cluster(String rawKey) { public KubernetesV2Cluster( String rawKey, - List serverGroups, - List loadBalancers) { + Collection serverGroups, + Collection loadBalancers) { Keys.ClusterCacheKey key = (Keys.ClusterCacheKey) Keys.parseKey(rawKey).get(); this.name = key.getName(); this.accountName = key.getAccount(); diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2LoadBalancer.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2LoadBalancer.java index 8dd9efb29ca..b9513a015e2 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2LoadBalancer.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/model/KubernetesV2LoadBalancer.java @@ -18,6 +18,7 @@ package com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Multimap; import com.netflix.spinnaker.cats.cache.CacheData; import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys; import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.KubernetesCacheDataConverter; @@ -27,8 +28,6 @@ import com.netflix.spinnaker.clouddriver.model.LoadBalancerProvider; import com.netflix.spinnaker.clouddriver.model.LoadBalancerServerGroup; import java.util.Collection; -import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -54,8 +53,8 @@ private KubernetesV2LoadBalancer( public static KubernetesV2LoadBalancer fromCacheData( CacheData cd, - List serverGroupData, - Map> serverGroupToInstanceData) { + Collection serverGroupData, + Multimap serverGroupToInstanceData) { if (cd == null) { return null; } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesCacheUtils.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesCacheUtils.java index 1c410cd41dd..2c2dfbb7ab6 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesCacheUtils.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesCacheUtils.java @@ -17,35 +17,36 @@ package com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; + +import com.google.common.collect.ImmutableCollection; +import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; import com.netflix.spinnaker.cats.cache.Cache; import com.netflix.spinnaker.cats.cache.CacheData; import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter; import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys; import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.KubernetesCacheDataConverter; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.ManifestBasedModel; import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2CacheData; -import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesResourceProperties; import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesSpinnakerKindMap; import com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind; -import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest; +import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler; -import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.ModelHandler; -import java.util.ArrayList; +import com.netflix.spinnaker.kork.annotations.NonnullByDefault; import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @Component +@NonnullByDefault @Slf4j -public class KubernetesCacheUtils { +class KubernetesCacheUtils { private final Cache cache; private final KubernetesSpinnakerKindMap kindMap; private final KubernetesAccountResolver resourcePropertyResolver; @@ -60,115 +61,100 @@ public KubernetesCacheUtils( this.resourcePropertyResolver = resourcePropertyResolver; } - public Collection getAllKeys(String type) { + Collection getAllKeys(String type) { return cache.getAll(type); } - public Collection getAllKeysMatchingPattern(String type, String key) { + Collection getAllKeysMatchingPattern(String type, String key) { return cache.filterIdentifiers(type, key); } - public Collection getAllDataMatchingPattern(String type, String key) { + Collection getAllDataMatchingPattern(String type, String key) { return cache.getAll(type, getAllKeysMatchingPattern(type, key)); } - public Optional getSingleEntry(String type, String key) { + Optional getSingleEntry(String type, String key) { return Optional.ofNullable(cache.get(type, key)); } - public Optional getSingleEntryWithRelationships( - String type, String key, String... to) { - return Optional.ofNullable(cache.get(type, key, RelationshipCacheFilter.include(to))); + Optional getSingleEntryWithRelationships( + String type, String key, RelationshipCacheFilter cacheFilter) { + return Optional.ofNullable(cache.get(type, key, cacheFilter)); } - private Collection aggregateRelationshipsBySpinnakerKind( - CacheData source, SpinnakerKind kind) { - return kindMap.translateSpinnakerKind(kind).stream() - .map(g -> source.getRelationships().get(g.toString())) - .filter(Objects::nonNull) - .flatMap(Collection::stream) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + /** Gets the data for all relationships of a given Spinnaker kind for a CacheData item. */ + ImmutableCollection getRelationshipKeys( + CacheData cacheData, SpinnakerKind spinnakerKind) { + return relationshipTypes(spinnakerKind) + .flatMap(t -> relationshipKeys(cacheData, t)) + .collect(toImmutableList()); } - public Collection getTransitiveRelationship( - String from, List sourceKeys, String to) { - Collection sourceData = - cache.getAll(from, sourceKeys, RelationshipCacheFilter.include(to)); - return cache.getAll( - to, - sourceData.stream() - .map(CacheData::getRelationships) - .filter(Objects::nonNull) - .map(r -> r.get(to)) - .filter(Objects::nonNull) - .flatMap(Collection::stream) - .collect(Collectors.toList())); + /** Gets the data for all relationships of a given type for a CacheData item. */ + Collection getRelationships(CacheData cacheData, String relationshipType) { + return getRelationships(ImmutableSet.of(cacheData), relationshipType); } - public Collection getAllRelationshipsOfSpinnakerKind( + /** + * Gets the data for all relationships of a given Spinnaker kind for a collection of CacheData + * items. + */ + Collection getRelationships( Collection cacheData, SpinnakerKind spinnakerKind) { - return kindMap.translateSpinnakerKind(spinnakerKind).stream() - .map(kind -> loadRelationshipsFromCache(cacheData, kind.toString())) + return relationshipTypes(spinnakerKind) + .map(kind -> getRelationships(cacheData, kind)) .flatMap(Collection::stream) .collect(Collectors.toList()); } - public Collection loadRelationshipsFromCache( - CacheData source, String relationshipType) { - return loadRelationshipsFromCache(ImmutableSet.of(source), relationshipType); - } - - public Collection loadRelationshipsFromCache( + /** Gets the data for all relationships of a given type for a collection of CacheData items. */ + private Collection getRelationships( Collection sources, String relationshipType) { - List keys = + return cache.getAll( + relationshipType, sources.stream() - .map(CacheData::getRelationships) - .filter(Objects::nonNull) - .map(r -> r.get(relationshipType)) - .filter(Objects::nonNull) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - - return cache.getAll(relationshipType, keys); + .flatMap(cd -> relationshipKeys(cd, relationshipType)) + .collect(toImmutableSet())); } /* * Builds a map of all keys belonging to `sourceKind` that are related to any entries in `targetData` */ - public Map> mapByRelationship( + ImmutableMultimap mapByRelationship( Collection targetData, SpinnakerKind sourceKind) { - Map> result = new HashMap<>(); - - for (CacheData datum : targetData) { - Collection sourceKeys = aggregateRelationshipsBySpinnakerKind(datum, sourceKind); + ImmutableListMultimap.Builder builder = ImmutableListMultimap.builder(); + targetData.forEach( + datum -> + getRelationshipKeys(datum, sourceKind) + .forEach(sourceKey -> builder.put(sourceKey, datum))); + return builder.build(); + } - for (String sourceKey : sourceKeys) { - List storedData = result.getOrDefault(sourceKey, new ArrayList<>()); - storedData.add(datum); - result.put(sourceKey, storedData); - } + /** Returns a stream of all relationships of a given type for a given CacheData. */ + private Stream relationshipKeys(CacheData cacheData, String type) { + Collection relationships = cacheData.getRelationships().get(type); + // Avoiding creating an Optional here as this is deeply nested in performance-sensitive code. + if (relationships == null) { + return Stream.empty(); } + return relationships.stream(); + } - return result; + /** Given a spinnaker kind, returns a stream of the relationship types representing that kind. */ + private Stream relationshipTypes(SpinnakerKind spinnakerKind) { + return kindMap.translateSpinnakerKind(spinnakerKind).stream().map(KubernetesKind::toString); } - @SuppressWarnings("unchecked") - public T resourceModelFromCacheData( - KubernetesV2CacheData cacheData) { + KubernetesHandler getHandler(KubernetesV2CacheData cacheData) { Keys.InfrastructureCacheKey key = (Keys.InfrastructureCacheKey) Keys.parseKey(cacheData.primaryData().getId()).get(); - KubernetesManifest manifest = KubernetesCacheDataConverter.getManifest(cacheData.primaryData()); - - KubernetesResourceProperties properties = - resourcePropertyResolver - .getResourcePropertyRegistry(key.getAccount()) - .get(manifest.getKind()); - KubernetesHandler handler = properties.getHandler(); - if (handler instanceof ModelHandler) { - return (T) ((ModelHandler) handler).fromCacheData(cacheData); - } else { - return null; - } + // TODO(ezimanyi): The kind is also stored directly on the cache data; get it from there instead + // of reading it from the manifest. + KubernetesKind kind = + KubernetesCacheDataConverter.getManifest(cacheData.primaryData()).getKind(); + return resourcePropertyResolver + .getResourcePropertyRegistry(key.getAccount()) + .get(kind) + .getHandler(); } } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ClusterProvider.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ClusterProvider.java index 2c6406e22cd..7ba3c26390b 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ClusterProvider.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ClusterProvider.java @@ -28,7 +28,9 @@ import static java.util.stream.Collectors.toSet; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMultimap; import com.netflix.spinnaker.cats.cache.CacheData; +import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter; import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider; import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys; import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys.InfrastructureCacheKey; @@ -39,6 +41,8 @@ import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesSpinnakerKindMap; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest; +import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler; +import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.ServerGroupHandler; import com.netflix.spinnaker.clouddriver.model.ClusterProvider; import java.util.ArrayList; import java.util.Collection; @@ -49,6 +53,7 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import javax.annotation.Nonnull; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.tuple.Pair; import org.springframework.beans.factory.annotation.Autowired; @@ -76,8 +81,13 @@ public Map> getClusterSummaries(String applicat String applicationKey = Keys.ApplicationCacheKey.createKey(application); return groupByAccountName( loadClusterSummaries( - cacheUtils.getTransitiveRelationship( - APPLICATIONS.toString(), ImmutableList.of(applicationKey), CLUSTERS.toString()))); + cacheUtils + .getSingleEntryWithRelationships( + APPLICATIONS.toString(), + applicationKey, + RelationshipCacheFilter.include(CLUSTERS.toString())) + .map(d -> cacheUtils.getRelationships(d, CLUSTERS.toString())) + .orElseGet(ImmutableList::of))); } @Override @@ -139,31 +149,20 @@ public KubernetesV2ServerGroup getServerGroup( Optional serverGroupData = cacheUtils.getSingleEntryWithRelationships( - kind.toString(), key, relatedTypes.toArray(new String[0])); + kind.toString(), + key, + RelationshipCacheFilter.include(relatedTypes.toArray(new String[0]))); return serverGroupData .map( cd -> { - List instanceData = - kindMap.translateSpinnakerKind(INSTANCES).stream() - .map( - k -> - cacheUtils.loadRelationshipsFromCache( - ImmutableList.of(cd), k.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); + Collection instanceData = + cacheUtils.getRelationships(ImmutableList.of(cd), INSTANCES); - List loadBalancerKeys = - kindMap.translateSpinnakerKind(LOAD_BALANCERS).stream() - .map( - k -> - cacheUtils.loadRelationshipsFromCache( - ImmutableList.of(cd), k.toString())) - .flatMap(Collection::stream) - .map(CacheData::getId) - .collect(Collectors.toList()); + Collection loadBalancerKeys = + cacheUtils.getRelationshipKeys(cd, LOAD_BALANCERS); - return cacheUtils.resourceModelFromCacheData( + return serverGroupFromCacheData( KubernetesV2ServerGroupCacheData.builder() .serverGroupData(cd) .instanceData(instanceData) @@ -200,25 +199,10 @@ private Set loadClusterSummaries(Collection clus } private Set loadClusters(Collection clusterData) { - // TODO(lwander) possible optimization: store lb relationships in cluster object to cut down on - // number of loads here. - List serverGroupData = - kindMap.translateSpinnakerKind(SERVER_GROUPS).stream() - .map(kind -> cacheUtils.loadRelationshipsFromCache(clusterData, kind.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - - List loadBalancerData = - kindMap.translateSpinnakerKind(LOAD_BALANCERS).stream() - .map(kind -> cacheUtils.loadRelationshipsFromCache(serverGroupData, kind.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - - List instanceData = - kindMap.translateSpinnakerKind(INSTANCES).stream() - .map(kind -> cacheUtils.loadRelationshipsFromCache(serverGroupData, kind.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); + Collection serverGroupData = cacheUtils.getRelationships(clusterData, SERVER_GROUPS); + Collection loadBalancerData = + cacheUtils.getRelationships(serverGroupData, LOAD_BALANCERS); + Collection instanceData = cacheUtils.getRelationships(serverGroupData, INSTANCES); Map> clusterToServerGroups = new HashMap<>(); for (CacheData serverGroupDatum : serverGroupData) { @@ -246,11 +230,11 @@ private Set loadClusters(Collection clusterData) .collect(Collectors.toList())); } - Map> serverGroupToLoadBalancers = + ImmutableMultimap serverGroupToLoadBalancers = cacheUtils.mapByRelationship(loadBalancerData, SERVER_GROUPS); - Map> serverGroupToInstances = + ImmutableMultimap serverGroupToInstances = cacheUtils.mapByRelationship(instanceData, SERVER_GROUPS); - Map> loadBalancerToServerGroups = + ImmutableMultimap loadBalancerToServerGroups = cacheUtils.mapByRelationship(serverGroupData, LOAD_BALANCERS); return clusterData.stream() @@ -263,36 +247,30 @@ private Set loadClusters(Collection clusterData) clusterServerGroups.stream() .map( cd -> - cacheUtils.resourceModelFromCacheData( + serverGroupFromCacheData( KubernetesV2ServerGroupCacheData.builder() .serverGroupData(cd) - .instanceData( - serverGroupToInstances.getOrDefault( - cd.getId(), new ArrayList<>())) + .instanceData(serverGroupToInstances.get(cd.getId())) .loadBalancerKeys( - serverGroupToLoadBalancers - .getOrDefault(cd.getId(), new ArrayList<>()) - .stream() + serverGroupToLoadBalancers.get(cd.getId()).stream() .map(CacheData::getId) .collect(toImmutableList())) .serverGroupManagerKeys( serverGroupToServerGroupManagerKeys.getOrDefault( cd.getId(), new ArrayList<>())) .build())) - .filter(Objects::nonNull) .collect(Collectors.toList()); List loadBalancers = clusterServerGroups.stream() .map(CacheData::getId) - .map(id -> serverGroupToLoadBalancers.getOrDefault(id, new ArrayList<>())) + .map(serverGroupToLoadBalancers::get) .flatMap(Collection::stream) .map( cd -> KubernetesV2LoadBalancer.fromCacheData( cd, - loadBalancerToServerGroups.getOrDefault( - cd.getId(), new ArrayList<>()), + loadBalancerToServerGroups.get(cd.getId()), serverGroupToInstances)) .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -301,4 +279,17 @@ private Set loadClusters(Collection clusterData) }) .collect(toSet()); } + + private final ServerGroupHandler DEFAULT_SERVER_GROUP_HANDLER = new ServerGroupHandler() {}; + + @Nonnull + private KubernetesV2ServerGroup serverGroupFromCacheData( + @Nonnull KubernetesV2ServerGroupCacheData cacheData) { + KubernetesHandler handler = cacheUtils.getHandler(cacheData); + ServerGroupHandler serverGroupHandler = + handler instanceof ServerGroupHandler + ? (ServerGroupHandler) handler + : DEFAULT_SERVER_GROUP_HANDLER; + return serverGroupHandler.fromCacheData(cacheData); + } } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2LoadBalancerProvider.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2LoadBalancerProvider.java index b4eaaa13d06..5e7948e25d9 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2LoadBalancerProvider.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2LoadBalancerProvider.java @@ -17,12 +17,15 @@ package com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys.LogicalKind.APPLICATIONS; import static com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind.INSTANCES; import static com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind.LOAD_BALANCERS; import static com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind.SERVER_GROUPS; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.ImmutableSet; import com.netflix.spinnaker.cats.cache.CacheData; import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider; import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys; @@ -35,7 +38,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -102,46 +104,34 @@ public List byAccountAndRegionAndName( @Override public Set getApplicationLoadBalancers(String application) { - List loadBalancerData = - kindMap.translateSpinnakerKind(LOAD_BALANCERS).stream() - .map( - kind -> - cacheUtils.getTransitiveRelationship( - APPLICATIONS.toString(), - ImmutableList.of(ApplicationCacheKey.createKey(application)), - kind.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - - return fromLoadBalancerCacheData(loadBalancerData); + return cacheUtils + .getSingleEntry(APPLICATIONS.toString(), ApplicationCacheKey.createKey(application)) + .map( + applicationData -> + fromLoadBalancerCacheData( + kindMap.translateSpinnakerKind(LOAD_BALANCERS).stream() + .map(kind -> cacheUtils.getRelationships(applicationData, kind.toString())) + .flatMap(Collection::stream) + .collect(toImmutableList()))) + .orElseGet(ImmutableSet::of); } private Set fromLoadBalancerCacheData( List loadBalancerData) { - List serverGroupData = - kindMap.translateSpinnakerKind(SERVER_GROUPS).stream() - .map(kind -> cacheUtils.loadRelationshipsFromCache(loadBalancerData, kind.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - - List instanceData = - kindMap.translateSpinnakerKind(INSTANCES).stream() - .map(kind -> cacheUtils.loadRelationshipsFromCache(serverGroupData, kind.toString())) - .flatMap(Collection::stream) - .collect(Collectors.toList()); - - Map> loadBalancerToServerGroups = + Collection serverGroupData = + cacheUtils.getRelationships(loadBalancerData, SERVER_GROUPS); + Collection instanceData = cacheUtils.getRelationships(serverGroupData, INSTANCES); + + ImmutableMultimap loadBalancerToServerGroups = cacheUtils.mapByRelationship(serverGroupData, LOAD_BALANCERS); - Map> serverGroupToInstances = + ImmutableMultimap serverGroupToInstances = cacheUtils.mapByRelationship(instanceData, SERVER_GROUPS); return loadBalancerData.stream() .map( cd -> KubernetesV2LoadBalancer.fromCacheData( - cd, - loadBalancerToServerGroups.getOrDefault(cd.getId(), new ArrayList<>()), - serverGroupToInstances)) + cd, loadBalancerToServerGroups.get(cd.getId()), serverGroupToInstances)) .filter(Objects::nonNull) .collect(Collectors.toSet()); } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ManifestProvider.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ManifestProvider.java index f8ea0dd5be8..c68a05ca538 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ManifestProvider.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ManifestProvider.java @@ -113,7 +113,7 @@ public List getClusterAndSortAscending( .getSingleEntry(CLUSTERS.toString(), Keys.ClusterCacheKey.createKey(account, app, cluster)) .map( c -> - cacheUtils.loadRelationshipsFromCache(c, kind).stream() + cacheUtils.getRelationships(c, kind).stream() .map(cd -> fromCacheData(cd, credentials, false)) .filter(m -> m.getLocation().equals(location)) .sorted( @@ -129,14 +129,10 @@ private KubernetesV2Manifest fromCacheData( KubernetesManifest manifest = KubernetesCacheDataConverter.getManifest(data); String namespace = manifest.getNamespace(); KubernetesKind kind = manifest.getKind(); - String key = data.getId(); List events = includeEvents - ? cacheUtils - .getTransitiveRelationship( - kind.toString(), ImmutableList.of(key), KubernetesKind.EVENT.toString()) - .stream() + ? cacheUtils.getRelationships(data, KubernetesKind.EVENT.toString()).stream() .map(KubernetesCacheDataConverter::getManifest) .collect(Collectors.toList()) : ImmutableList.of(); diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ServerGroupManagerProvider.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ServerGroupManagerProvider.java index 96a11543ba5..cfab147baed 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ServerGroupManagerProvider.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesV2ServerGroupManagerProvider.java @@ -22,17 +22,18 @@ import static com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind.SERVER_GROUP_MANAGERS; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMultimap; import com.netflix.spinnaker.cats.cache.CacheData; import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys; import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.KubernetesV2ServerGroupManager; import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2ServerGroupManagerCacheData; +import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler; +import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.ServerGroupManagerHandler; import com.netflix.spinnaker.clouddriver.model.ServerGroupManagerProvider; -import java.util.ArrayList; import java.util.Collection; -import java.util.List; -import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import javax.annotation.Nonnull; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -59,23 +60,35 @@ public Set getServerGroupManagersByApplication( } Collection serverGroupManagerData = - cacheUtils.getAllRelationshipsOfSpinnakerKind( - ImmutableList.of(applicationDatum), SERVER_GROUP_MANAGERS); + cacheUtils.getRelationships(ImmutableList.of(applicationDatum), SERVER_GROUP_MANAGERS); Collection serverGroupData = - cacheUtils.getAllRelationshipsOfSpinnakerKind(serverGroupManagerData, SERVER_GROUPS); + cacheUtils.getRelationships(serverGroupManagerData, SERVER_GROUPS); - Map> managerToServerGroupMap = + ImmutableMultimap managerToServerGroupMap = cacheUtils.mapByRelationship(serverGroupData, SERVER_GROUP_MANAGERS); return serverGroupManagerData.stream() .map( cd -> - cacheUtils.resourceModelFromCacheData( + serverGroupManagerFromCacheData( KubernetesV2ServerGroupManagerCacheData.builder() .serverGroupManagerData(cd) - .serverGroupData( - managerToServerGroupMap.getOrDefault(cd.getId(), new ArrayList<>())) + .serverGroupData(managerToServerGroupMap.get(cd.getId())) .build())) .collect(Collectors.toSet()); } + + private final ServerGroupManagerHandler DEFAULT_SERVER_GROUP_MANAGER_HANDLER = + new ServerGroupManagerHandler() {}; + + @Nonnull + private KubernetesV2ServerGroupManager serverGroupManagerFromCacheData( + @Nonnull KubernetesV2ServerGroupManagerCacheData cacheData) { + KubernetesHandler handler = cacheUtils.getHandler(cacheData); + ServerGroupManagerHandler serverGroupManagerHandler = + handler instanceof ServerGroupManagerHandler + ? (ServerGroupManagerHandler) handler + : DEFAULT_SERVER_GROUP_MANAGER_HANDLER; + return serverGroupManagerHandler.fromCacheData(cacheData); + } } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CustomKubernetesHandlerFactory.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CustomKubernetesHandlerFactory.java index 17aee807390..a748ad8bbac 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CustomKubernetesHandlerFactory.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CustomKubernetesHandlerFactory.java @@ -22,12 +22,6 @@ import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.CustomKubernetesCachingAgentFactory; import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.KubernetesV2CachingAgent; import com.netflix.spinnaker.clouddriver.kubernetes.caching.agent.KubernetesV2CachingAgentFactory; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.KubernetesV2ServerGroup; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.KubernetesV2ServerGroupManager; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.ManifestBasedModel; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2CacheData; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2ServerGroupCacheData; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2ServerGroupManagerCacheData; import com.netflix.spinnaker.clouddriver.kubernetes.description.SpinnakerKind; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest; @@ -48,7 +42,7 @@ public static KubernetesHandler create( } @Slf4j - private static class Handler extends KubernetesHandler implements ModelHandler { + private static class Handler extends KubernetesHandler { private final KubernetesKind kubernetesKind; private final SpinnakerKind spinnakerKind; private final boolean versioned; @@ -113,21 +107,5 @@ private KubernetesV2CachingAgent buildCustomCachingAgent( agentCount, agentInterval); } - - @Override - public ManifestBasedModel fromCacheData(KubernetesV2CacheData cacheData) { - switch (spinnakerKind()) { - case SERVER_GROUPS: - return KubernetesV2ServerGroup.fromCacheData( - (KubernetesV2ServerGroupCacheData) cacheData); - case SERVER_GROUP_MANAGERS: - return KubernetesV2ServerGroupManager.fromCacheData( - (KubernetesV2ServerGroupManagerCacheData) cacheData); - default: - // TODO(dpeach): finish implementing for other SpinnakerKinds. - log.warn("No default cache data model mapping for Spinnaker kind " + spinnakerKind()); - return null; - } - } } } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ModelHandler.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ModelHandler.java deleted file mode 100644 index e71722e6bcb..00000000000 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ModelHandler.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2018 Google, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package com.netflix.spinnaker.clouddriver.kubernetes.op.handler; - -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.ManifestBasedModel; -import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2CacheData; - -public interface ModelHandler { - ManifestBasedModel fromCacheData(T cacheData); -} diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupHandler.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupHandler.java index 348564c7cd5..14eb76f2cae 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupHandler.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupHandler.java @@ -20,7 +20,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.KubernetesV2ServerGroup; import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2ServerGroupCacheData; -public interface ServerGroupHandler extends ModelHandler { +public interface ServerGroupHandler { default KubernetesV2ServerGroup fromCacheData(KubernetesV2ServerGroupCacheData cacheData) { return KubernetesV2ServerGroup.fromCacheData(cacheData); } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupManagerHandler.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupManagerHandler.java index fb050259850..44ac4bb44a9 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupManagerHandler.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/ServerGroupManagerHandler.java @@ -20,8 +20,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.model.KubernetesV2ServerGroupManager; import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.data.KubernetesV2ServerGroupManagerCacheData; -public interface ServerGroupManagerHandler - extends ModelHandler { +public interface ServerGroupManagerHandler { default KubernetesV2ServerGroupManager fromCacheData( KubernetesV2ServerGroupManagerCacheData cacheData) { return KubernetesV2ServerGroupManager.fromCacheData(cacheData); diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java index 42f1b523f07..cc6dd3499e4 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java @@ -544,7 +544,7 @@ private void assertFrontendCluster( assertFrontendServerGroups(softly, cluster.getServerGroups()); // TODO(ezimanyi): The same load balancer is being returned multiple times (likely due to // finding all relationships to it). This should be fixed to return it only once. - softly.assertThat(cluster.getLoadBalancers()).hasSize(4); + softly.assertThat(cluster.getLoadBalancers()).hasSize(2); if (!cluster.getLoadBalancers().isEmpty()) { assertFrontendLoadBalancer(softly, cluster.getLoadBalancers().iterator().next()); }