Skip to content

Commit

Permalink
perf(kubernetes): Defer loading namespaces until first use (#4008)
Browse files Browse the repository at this point in the history
* refactor(kubernetes): Push namespace logic down to V1 and V2

A following commit will simplify the logic for handling namespaces
in the kubernetes caching agents. There is a lot of legacy V1 code
in groovy that depends on the particular implementation that I don't
want to refactor and test as part of this change. In order to restrict
the change to V2, push this logic down from the base caching agent to
the V1 and V2 caching agents, where it is for now duplicated but will
soon be changed in V2.

This also makes the KubernetesMetricCachingAgent extend the
KubernetesV2CachingAgent rather than the base KubernetesCachingAgent
so it can share the new namespace functionality.

* refactor(kubernetes): Make namespaces private

Change namespaces to be private, and have all the consumers use
the accessor (which can be protected instead of public). Also,
return an ImmutableList of namespaces rather than a list.

* perf(kubernetes): Defer loading namespaces until first use

The kubernetes v2 caching agents store a list of namespaces of
interest, and have a complex system of calling loadNamespaces()
to refresh this list then getNamespaces() to fetch them.

It turns out that we always just call loadNamespaces() immediately
before a call to getNamespaces() so the cache is doing nothing.
Remove the cached namespaces variable and all calls to
loadNamespaces(). Instead just have getNamespaces() directly look
up the namespaces (which was happening anyway before).

The main benefit is that now we can remove the call to loadNamespaces()
to prime the cache from the caching agent constructor. This means that
caching agents don't need to communicate with the cluster in their
constructor, so startup will not be blocked waiting for calls to
an unreachable cluster to time out.
  • Loading branch information
ezimanyi committed Sep 6, 2019
1 parent abaa9a3 commit a1c788c
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import com.netflix.spinnaker.cats.agent.CachingAgent;
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials;
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesNamedAccountCredentials;
import java.util.List;
import java.util.stream.Collectors;
import lombok.Getter;

public abstract class KubernetesCachingAgent<C extends KubernetesCredentials>
Expand All @@ -37,12 +35,6 @@ public abstract class KubernetesCachingAgent<C extends KubernetesCredentials>
protected final int agentIndex;
protected final int agentCount;

protected List<String> namespaces;

public List<String> getNamespaces() {
return namespaces;
}

protected KubernetesCachingAgent(
KubernetesNamedAccountCredentials<C> namedAccountCredentials,
ObjectMapper objectMapper,
Expand All @@ -56,20 +48,11 @@ protected KubernetesCachingAgent(

this.agentIndex = agentIndex;
this.agentCount = agentCount;

reloadNamespaces();
}

@Override
public String getAgentType() {
return String.format(
"%s/%s[%d/%d]", accountName, this.getClass().getSimpleName(), agentIndex + 1, agentCount);
}

protected void reloadNamespaces() {
namespaces =
credentials.getDeclaredNamespaces().stream()
.filter(n -> agentCount == 1 || Math.abs(n.hashCode() % agentCount) == agentIndex)
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesNamedAccountCredentials;
import com.netflix.spinnaker.clouddriver.kubernetes.v1.provider.KubernetesV1Provider;
import com.netflix.spinnaker.clouddriver.kubernetes.v1.security.KubernetesV1Credentials;
import java.util.List;
import java.util.stream.Collectors;
import lombok.Getter;

public abstract class KubernetesV1CachingAgent
extends KubernetesCachingAgent<KubernetesV1Credentials> {
@Getter protected final String providerName = KubernetesV1Provider.PROVIDER_NAME;
protected List<String> namespaces;

protected KubernetesV1CachingAgent(
KubernetesNamedAccountCredentials<KubernetesV1Credentials> namedAccountCredentials,
Expand All @@ -36,5 +39,17 @@ protected KubernetesV1CachingAgent(
int agentIndex,
int agentCount) {
super(namedAccountCredentials, objectMapper, registry, agentIndex, agentCount);
reloadNamespaces();
}

public List<String> getNamespaces() {
return namespaces;
}

protected void reloadNamespaces() {
namespaces =
credentials.getDeclaredNamespaces().stream()
.filter(n -> agentCount == 1 || Math.abs(n.hashCode() % agentCount) == agentIndex)
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
import com.netflix.spinnaker.cats.cache.CacheData;
import com.netflix.spinnaker.cats.provider.ProviderCache;
import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider;
import com.netflix.spinnaker.clouddriver.kubernetes.caching.KubernetesCachingAgent;
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesNamedAccountCredentials;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.KubernetesPodMetric;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.op.job.KubectlJobExecutor;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.security.KubernetesV2Credentials;
import java.util.Collection;
Expand All @@ -43,12 +43,10 @@
import lombok.extern.slf4j.Slf4j;

@Slf4j
public class KubernetesMetricCachingAgent extends KubernetesCachingAgent<KubernetesV2Credentials>
public class KubernetesMetricCachingAgent extends KubernetesV2CachingAgent
implements AgentIntervalAware {
@Getter protected String providerName = KubernetesCloudProvider.ID;

@Getter private final Long agentInterval;

@Getter
protected Collection<AgentDataType> providedDataTypes =
Collections.unmodifiableCollection(
Expand All @@ -61,17 +59,19 @@ protected KubernetesMetricCachingAgent(
int agentIndex,
int agentCount,
Long agentInterval) {
super(namedAccountCredentials, objectMapper, registry, agentIndex, agentCount);
this.agentInterval = agentInterval;
super(namedAccountCredentials, objectMapper, registry, agentIndex, agentCount, agentInterval);
}

@Override
protected List<KubernetesKind> primaryKinds() {
return Collections.emptyList();
}

@Override
public CacheResult loadData(ProviderCache providerCache) {
log.info(getAgentType() + ": agent is starting");
reloadNamespaces();

List<KubernetesPodMetric> podMetrics =
namespaces
getNamespaces()
.parallelStream()
.map(
n -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ public KubernetesNamespaceCachingAgent(

@Override
protected Map<KubernetesKind, List<KubernetesManifest>> loadPrimaryResourceList() {
reloadNamespaces();

// TODO perf: Only load desired namespaces rather than filter all.
Set<String> desired = new HashSet<>(this.namespaces);
Set<String> desired = new HashSet<>(this.getNamespaces());
return Collections.singletonMap(
KubernetesKind.NAMESPACE,
credentials.list(KubernetesKind.NAMESPACE, "").stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package com.netflix.spinnaker.clouddriver.kubernetes.v2.caching.agent;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.cats.agent.AgentIntervalAware;
import com.netflix.spinnaker.cats.agent.CacheResult;
Expand Down Expand Up @@ -66,7 +67,7 @@ protected KubernetesV2CachingAgent(

protected Map<String, Object> defaultIntrospectionDetails() {
Map<String, Object> result = new HashMap<>();
result.put("namespaces", namespaces);
result.put("namespaces", getNamespaces());
result.put("kinds", primaryKinds());
return result;
}
Expand All @@ -75,7 +76,7 @@ protected Map<String, Object> defaultIntrospectionDetails() {

protected Map<KubernetesKind, List<KubernetesManifest>> loadPrimaryResourceList() {
Map<KubernetesKind, List<KubernetesManifest>> result =
namespaces
getNamespaces()
.parallelStream()
.map(
n -> {
Expand Down Expand Up @@ -128,7 +129,6 @@ protected KubernetesManifest loadPrimaryResource(
@Override
public CacheResult loadData(ProviderCache providerCache) {
log.info(getAgentType() + ": agent is starting");
reloadNamespaces();
Map<String, Object> details = defaultIntrospectionDetails();

try {
Expand Down Expand Up @@ -193,4 +193,10 @@ protected Map<KubernetesManifest, List<KubernetesManifest>> loadSecondaryResourc
});
return result;
}

protected ImmutableList<String> getNamespaces() {
return credentials.getDeclaredNamespaces().stream()
.filter(n -> agentCount == 1 || Math.abs(n.hashCode() % agentCount) == agentIndex)
.collect(ImmutableList.toImmutableList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ protected KubernetesV2OnDemandCachingAgent(
@Override
public CacheResult loadData(ProviderCache providerCache) {
log.info(getAgentType() + ": agent is starting");
reloadNamespaces();
Map<String, Object> details = defaultIntrospectionDetails();

Long start = System.currentTimeMillis();
Expand Down Expand Up @@ -312,8 +311,7 @@ public OnDemandAgent.OnDemandResult handle(ProviderCache providerCache, Map<Stri
namespace = "";
}

reloadNamespaces();
if (!StringUtils.isEmpty(namespace) && !namespaces.contains(namespace)) {
if (!StringUtils.isEmpty(namespace) && !getNamespaces().contains(namespace)) {
return null;
}

Expand Down

0 comments on commit a1c788c

Please sign in to comment.