Skip to content

Commit

Permalink
refactor(kubernetes): Make cache keys immutable (#3826)
Browse files Browse the repository at this point in the history 
* refactor(kubernetes): Make cache keys immutable

We don't ever change cache keys after construction, but they
still support (unused) setters.  Make these keys immutable
so we can safely use them as keys in sets/maps.

* fix(kubernetes): Fix serialization of cache keys

Some cache key types don't have symmetry between serialization
and deserialization logic.  Fix them so that they do so we can
create a key and call toString() and get the same result as if
we directly created the string key.

* refactor(kubernetes): Move key constructors into inner classes

The top-level Keys class supports a number of methods for creating
a String representation of various key types. Move these methods
into the relevant inner classes and rename them createKey.

* perf(kubernetes): Short-circuit string replacement

As a performance improvement, don't allocate a new string for
every key part we're processing via the replaceAll(). Instead
check whether they key has a ':' first which is relatively cheap
compared to alwyas allocating a new string.
  • Loading branch information
@ezimanyi
ezimanyi authored Jun 28, 2019
1 parent b04520b commit a6f7fd3
Show file tree
Hide file tree
Showing 13 changed files with 147 additions and 116 deletions.
139 changes: 73 additions & 66 deletions ...ernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/Keys.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@
import static com.netflix.spinnaker.clouddriver.kubernetes.v2.caching.Keys.Kind.KUBERNETES_METRIC;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesManifest;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;

@Slf4j
Expand Down Expand Up @@ -101,41 +100,16 @@ public static LogicalKind fromString(String name) {

private static final String provider = "kubernetes.v2";

private static String createKey(Object... elems) {
private static String createKeyFromParts(Object... elems) {
List<String> components =
Arrays.stream(elems)
.map(s -> s == null ? "" : s.toString())
.map(s -> s.replaceAll(":", ";"))
.map(s -> s.contains(":") ? s.replaceAll(":", ";") : s)
.collect(Collectors.toList());
components.add(0, provider);
return String.join(":", components);
}

public static String artifact(String type, String name, String location, String version) {
return createKey(Kind.ARTIFACT, type, name, location, version);
}

public static String application(String name) {
return createKey(Kind.LOGICAL, LogicalKind.APPLICATIONS, name);
}

public static String cluster(String account, String application, String name) {
return createKey(Kind.LOGICAL, LogicalKind.CLUSTERS, account, application, name);
}

public static String infrastructure(
KubernetesKind kind, String account, String namespace, String name) {
return createKey(Kind.INFRASTRUCTURE, kind, account, namespace, name);
}

public static String infrastructure(KubernetesManifest manifest, String account) {
return infrastructure(manifest.getKind(), account, manifest.getNamespace(), manifest.getName());
}

public static String metric(KubernetesKind kind, String account, String namespace, String name) {
return createKey(KUBERNETES_METRIC, kind, account, namespace, name);
}

public static Optional<CacheKey> parseKey(String key) {
String[] parts = key.split(":", -1);

Expand Down Expand Up @@ -188,33 +162,30 @@ private static CacheKey parseLogicalKey(String[] parts) {
}
}

@Data
public abstract static class CacheKey {
private Kind kind;
private String provider = KubernetesCloudProvider.getID();
private String type;
private static final String provider = "kubernetes.v2";

public abstract String getGroup();

public abstract String getName();
}

@EqualsAndHashCode(callSuper = true)
@Data
@Getter
public abstract static class LogicalKey extends CacheKey {
private Kind kind = Kind.LOGICAL;
@Getter private static final Kind kind = Kind.LOGICAL;

public abstract LogicalKind getLogicalKind();
}

@EqualsAndHashCode(callSuper = true)
@Data
@Getter
public static class ArtifactCacheKey extends CacheKey {
private Kind kind = Kind.ARTIFACT;
private String type;
private String name;
private String location;
private String version;
@Getter private static final Kind kind = Kind.ARTIFACT;
private final String type;
private final String name;
private final String location;
private final String version;

public ArtifactCacheKey(String[] parts) {
if (parts.length != 6) {
Expand All @@ -227,9 +198,13 @@ public ArtifactCacheKey(String[] parts) {
version = parts[5];
}

public static String createKey(String type, String name, String location, String version) {
return createKeyFromParts(kind, type, name, location, version);
}

@Override
public String toString() {
return createKey(kind, type, name, version);
return createKeyFromParts(kind, type, name, location, version);
}

@Override
Expand All @@ -239,10 +214,10 @@ public String getGroup() {
}

@EqualsAndHashCode(callSuper = true)
@Data
@Getter
public static class ApplicationCacheKey extends LogicalKey {
private LogicalKind logicalKind = LogicalKind.APPLICATIONS;
private String name;
private static final LogicalKind logicalKind = LogicalKind.APPLICATIONS;
private final String name;

public ApplicationCacheKey(String[] parts) {
if (parts.length != 4) {
Expand All @@ -252,9 +227,18 @@ public ApplicationCacheKey(String[] parts) {
name = parts[3];
}

public static String createKey(String name) {
return createKeyFromParts(getKind(), logicalKind, name);
}

@Override
public LogicalKind getLogicalKind() {
return logicalKind;
}

@Override
public String toString() {
return createKey(getKind(), logicalKind, name);
return createKeyFromParts(getKind(), logicalKind, name);
}

@Override
Expand All @@ -264,12 +248,12 @@ public String getGroup() {
}

@EqualsAndHashCode(callSuper = true)
@Data
@Getter
public static class ClusterCacheKey extends LogicalKey {
private LogicalKind logicalKind = LogicalKind.CLUSTERS;
private String account;
private String application;
private String name;
private static final LogicalKind logicalKind = LogicalKind.CLUSTERS;
private final String account;
private final String application;
private final String name;

public ClusterCacheKey(String[] parts) {
if (parts.length != 6) {
Expand All @@ -281,9 +265,18 @@ public ClusterCacheKey(String[] parts) {
name = parts[5];
}

public static String createKey(String account, String application, String name) {
return createKeyFromParts(getKind(), logicalKind, account, application, name);
}

@Override
public LogicalKind getLogicalKind() {
return logicalKind;
}

@Override
public String toString() {
return createKey(getKind(), logicalKind, account, name);
return createKeyFromParts(getKind(), logicalKind, account, application, name);
}

@Override
Expand All @@ -293,13 +286,13 @@ public String getGroup() {
}

@EqualsAndHashCode(callSuper = true)
@Data
@Getter
public static class InfrastructureCacheKey extends CacheKey {
private Kind kind = Kind.INFRASTRUCTURE;
private KubernetesKind kubernetesKind;
private String account;
private String namespace;
private String name;
@Getter private static final Kind kind = Kind.INFRASTRUCTURE;
private final KubernetesKind kubernetesKind;
private final String account;
private final String namespace;
private final String name;

public InfrastructureCacheKey(String[] parts) {
if (parts.length != 6) {
Expand All @@ -313,9 +306,18 @@ public InfrastructureCacheKey(String[] parts) {
name = parts[5];
}

public static String createKey(
KubernetesKind kubernetesKind, String account, String namespace, String name) {
return createKeyFromParts(kind, kubernetesKind, account, namespace, name);
}

public static String createKey(KubernetesManifest manifest, String account) {
return createKey(manifest.getKind(), account, manifest.getNamespace(), manifest.getName());
}

@Override
public String toString() {
return createKey(kind, kubernetesKind, account, namespace, name);
return createKeyFromParts(kind, kubernetesKind, account, namespace, name);
}

@Override
Expand All @@ -325,13 +327,13 @@ public String getGroup() {
}

@EqualsAndHashCode(callSuper = true)
@Data
@Getter
public static class MetricCacheKey extends CacheKey {
private Kind kind = KUBERNETES_METRIC;
private KubernetesKind kubernetesKind;
private String account;
private String namespace;
private String name;
@Getter private static final Kind kind = KUBERNETES_METRIC;
private final KubernetesKind kubernetesKind;
private final String account;
private final String namespace;
private final String name;

public MetricCacheKey(String[] parts) {
if (parts.length != 6) {
Expand All @@ -344,9 +346,14 @@ public MetricCacheKey(String[] parts) {
name = parts[5];
}

public static String createKey(
KubernetesKind kubernetesKind, String account, String namespace, String name) {
return createKeyFromParts(kind, kubernetesKind, account, namespace, name);
}

@Override
public String toString() {
return createKey(kind, kubernetesKind, account, namespace, name);
return createKeyFromParts(kind, kubernetesKind, account, namespace, name);
}

@Override
Expand Down
31 changes: 18 additions & 13 deletions ...tflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCacheDataConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ public static CacheData convertAsArtifact(String account, KubernetesManifest man
Map<String, Collection<String>> cacheRelationships = new HashMap<>();

String key =
Keys.artifact(
Keys.ArtifactCacheKey.createKey(
artifact.getType(), artifact.getName(), artifact.getLocation(), artifact.getVersion());
String owner = Keys.infrastructure(manifest, account);
String owner = Keys.InfrastructureCacheKey.createKey(manifest, account);
cacheRelationships.put(manifest.getKind().toString(), Collections.singletonList(owner));

return defaultCacheData(key, logicalTtlSeconds, attributes, cacheRelationships);
Expand Down Expand Up @@ -180,10 +180,10 @@ public static CacheData convertPodMetric(
.put(
POD.toString(),
Collections.singletonList(
Keys.infrastructure(POD, account, namespace, podName)))
Keys.InfrastructureCacheKey.createKey(POD, account, namespace, podName)))
.build());

String id = Keys.metric(POD, account, namespace, podName);
String id = Keys.MetricCacheKey.createKey(POD, account, namespace, podName);

return defaultCacheData(id, infrastructureTtlSeconds, attributes, relationships);
}
Expand Down Expand Up @@ -247,7 +247,7 @@ public static CacheData convertAsResource(
ownerReferenceRelationships(account, namespace, manifest.getOwnerReferences()));
cacheRelationships.putAll(implicitRelationships(manifest, account, resourceRelationships));

String key = Keys.infrastructure(kind, account, namespace, name);
String key = Keys.InfrastructureCacheKey.createKey(kind, account, namespace, name);
return defaultCacheData(key, infrastructureTtlSeconds, attributes, cacheRelationships);
}

Expand Down Expand Up @@ -301,7 +301,7 @@ static Map<String, Collection<String>> annotatedRelationships(
cacheRelationships.put(
ARTIFACT.toString(),
Collections.singletonList(
Keys.artifact(
Keys.ArtifactCacheKey.createKey(
artifact.getType(),
artifact.getName(),
artifact.getLocation(),
Expand All @@ -310,12 +310,14 @@ static Map<String, Collection<String>> annotatedRelationships(

if (hasClusterRelationship) {
cacheRelationships.put(
APPLICATIONS.toString(), Collections.singletonList(Keys.application(application)));
APPLICATIONS.toString(),
Collections.singletonList(Keys.ApplicationCacheKey.createKey(application)));
String cluster = moniker.getCluster();
if (StringUtils.isNotEmpty(cluster)) {
cacheRelationships.put(
CLUSTERS.toString(),
Collections.singletonList(Keys.cluster(account, application, cluster)));
Collections.singletonList(
Keys.ClusterCacheKey.createKey(account, application, cluster)));
}
}

Expand All @@ -337,7 +339,7 @@ static void addSingleRelationship(
keys = new ArrayList<>();
}

keys.add(Keys.infrastructure(kind, account, namespace, name));
keys.add(Keys.InfrastructureCacheKey.createKey(kind, account, namespace, name));

relationships.put(kind.toString(), keys);
}
Expand All @@ -357,7 +359,7 @@ static Map<String, Collection<String>> implicitRelationships(
keys = new ArrayList<>();
}

keys.add(Keys.infrastructure(kind, account, namespace, name));
keys.add(Keys.InfrastructureCacheKey.createKey(kind, account, namespace, name));
relationships.put(kind.toString(), keys);
}

Expand All @@ -376,7 +378,7 @@ static Map<String, Collection<String>> ownerReferenceRelationships(
keys = new ArrayList<>();
}

keys.add(Keys.infrastructure(kind, account, namespace, name));
keys.add(Keys.InfrastructureCacheKey.createKey(kind, account, namespace, name));
relationships.put(kind.toString(), keys);
}

Expand Down Expand Up @@ -461,14 +463,17 @@ private static CacheData getApplicationClusterRelationships(
String account, String application, List<Moniker> monikers) {
Set<String> clusterRelationships =
monikers.stream()
.map(m -> Keys.cluster(account, application, m.getCluster()))
.map(m -> Keys.ClusterCacheKey.createKey(account, application, m.getCluster()))
.collect(Collectors.toSet());

Map<String, Object> attributes = new HashMap<>();
Map<String, Collection<String>> relationships = new HashMap<>();
relationships.put(CLUSTERS.toString(), clusterRelationships);
return defaultCacheData(
Keys.application(application), logicalTtlSeconds, attributes, relationships);
Keys.ApplicationCacheKey.createKey(application),
logicalTtlSeconds,
attributes,
relationships);
}

static void logStratifiedCacheData(
Expand Down
4 changes: 2 additions & 2 deletions ...x/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesV2OnDemandCachingAgent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public CacheResult loadData(ProviderCache providerCache) {
primaryResource.values().stream()
.flatMap(Collection::stream)
.map(rs -> objectMapper.convertValue(rs, KubernetesManifest.class))
.map(mf -> Keys.infrastructure(mf, accountName))
.map(mf -> Keys.InfrastructureCacheKey.createKey(mf, accountName))
.collect(Collectors.toList());

List<CacheData> keepInOnDemand = new ArrayList<>();
Expand Down Expand Up @@ -329,7 +329,7 @@ public OnDemandAgent.OnDemandResult handle(ProviderCache providerCache, Map<Stri
log.info("{}: Accepted on demand refresh of '{}'", getAgentType(), data);
OnDemandAgent.OnDemandResult result;
KubernetesManifest manifest = loadPrimaryResource(kind, namespace, name);
String resourceKey = Keys.infrastructure(kind, account, namespace, name);
String resourceKey = Keys.InfrastructureCacheKey.createKey(kind, account, namespace, name);
try {
result =
manifest == null
Expand Down
Loading

0 comments on commit a6f7fd3

Please sign in to comment.