Skip to content

Commit

Permalink
refactor(kubernetes): Create KubernetesKindRegistry (#3843)
Browse files Browse the repository at this point in the history
The KubernetesKind class is responsible for both providing information
about a given KubernetesKind and for keeping a registry of all
registered kinds.  The registry of all kinds is stored as a list which
is very inefficient to search.

In preparation for making this logic more efficient, create a
KubernetesKindRegistry that supports registering kind or getting
registered kinds. The implementation will be the exact same
(inefficient) one that is currently directly in KubernetesKind but
we have now defined an API and can write tests against it, and in
a future commit replace it with a more efficient implementation.
  • Loading branch information
ezimanyi committed Jul 3, 2019
1 parent e358c99 commit 8f3d5e5
Show file tree
Hide file tree
Showing 5 changed files with 274 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,9 @@ public Collection<AgentDataType> getProvidedDataTypes() {

@Override
protected List<KubernetesKind> primaryKinds() {
synchronized (KubernetesKind.getValues()) {
return KubernetesKind.getValues().stream()
.filter(credentials::isValidKind)
.filter(k -> !k.isDynamic())
.collect(Collectors.toList());
}
return KubernetesKind.getRegisteredKinds().stream()
.filter(credentials::isValidKind)
.filter(k -> !k.isDynamic())
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,8 @@

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand All @@ -37,9 +33,7 @@
@EqualsAndHashCode(onlyExplicitlyIncluded = true)
@Slf4j
public final class KubernetesKind {
@Getter
private static final List<KubernetesKind> values =
Collections.synchronizedList(new ArrayList<>());
private static final KubernetesKindRegistry kindRegistry = new KubernetesKindRegistry();

public static final KubernetesKind API_SERVICE =
createAndRegisterKind(
Expand Down Expand Up @@ -126,12 +120,12 @@ public final class KubernetesKind {
public static final KubernetesKind NONE =
createAndRegisterKind("none", KubernetesApiGroup.NONE, null, true, false);

@Nonnull private final String name;
@Getter @Nonnull private final String name;
@EqualsAndHashCode.Include @Nonnull private final String lcName;
@Nonnull private final KubernetesApiGroup apiGroup;
@Getter @Nonnull private final KubernetesApiGroup apiGroup;
@EqualsAndHashCode.Include @Nullable private final KubernetesApiGroup customApiGroup;

@Nullable private final String alias;
@Getter @Nullable private final String alias;
@Getter private final boolean isNamespaced;
// generally reserved for workloads, can be read as "does this belong to a spinnaker cluster?"
private final boolean hasClusterRelationship;
Expand All @@ -146,11 +140,9 @@ private static KubernetesKind createAndRegisterKind(
@Nullable String alias,
boolean isNamespaced,
boolean hasClusterRelationship) {
KubernetesKind kind =
return kindRegistry.registerKind(
new KubernetesKind(
name, apiGroup, alias, isNamespaced, hasClusterRelationship, false, true);
values.add(kind);
return kind;
name, apiGroup, alias, isNamespaced, hasClusterRelationship, false, true));
}

private KubernetesKind(
Expand Down Expand Up @@ -211,9 +203,11 @@ public static KubernetesKind fromString(@Nonnull final String name) {
return fromString(scopedKind.kindName, scopedKind.apiGroup);
}

@Nonnull
public static KubernetesKind fromString(
@Nonnull final String name, @Nullable final KubernetesApiGroup apiGroup) {
return getRegisteredKind(name, apiGroup)
return kindRegistry
.getRegisteredKind(name, apiGroup)
.orElseGet(
() ->
new KubernetesKind(
Expand All @@ -226,70 +220,30 @@ public static KubernetesKind fromString(
false));
}

private static Optional<KubernetesKind> getRegisteredKind(
@Nonnull final String name, @Nullable final KubernetesApiGroup apiGroup) {
if (StringUtils.isEmpty(name)) {
return Optional.of(KubernetesKind.NONE);
}

if (name.equalsIgnoreCase(KubernetesKind.NONE.toString())) {
throw new IllegalArgumentException("The 'NONE' kind cannot be read.");
}

Predicate<KubernetesKind> groupMatches =
kind -> {
// Exact match
if (Objects.equals(kind.apiGroup, apiGroup)) {
return true;
}

// If we have not specified an API group, default to finding a native kind that matches
if (apiGroup == null || apiGroup.isNativeGroup()) {
return kind.apiGroup.isNativeGroup();
}

return false;
};

// separate from the above chain to avoid concurrent modification of the values list
return values.stream()
.filter(
v ->
v.name.equalsIgnoreCase(name)
|| (v.alias != null && v.alias.equalsIgnoreCase(name)))
.filter(groupMatches)
.findAny();
}

@Nonnull
public static KubernetesKind getOrRegisterKind(
@Nonnull final String name,
final boolean registered,
final boolean namespaced,
@Nullable final KubernetesApiGroup apiGroup) {
synchronized (values) {
Optional<KubernetesKind> kindOptional = getRegisteredKind(name, apiGroup);
// separate from the above chain to avoid concurrent modification of the values list
return kindOptional.orElseGet(
() -> {
log.info(
"Dynamically registering {}, (namespaced: {}, registered: {})",
name,
namespaced,
registered);
KubernetesKind kind =
new KubernetesKind(
name,
Optional.ofNullable(apiGroup).orElse(KubernetesApiGroup.NONE),
null,
namespaced,
false,
true,
registered);
values.add(kind);
return kind;
});
}
return kindRegistry.getOrRegisterKind(
name,
apiGroup,
() -> {
log.info(
"Dynamically registering {}, (namespaced: {}, registered: {})",
name,
namespaced,
registered);
return new KubernetesKind(
name,
Optional.ofNullable(apiGroup).orElse(KubernetesApiGroup.NONE),
null,
namespaced,
false,
true,
registered);
});
}

@Nonnull
Expand All @@ -299,10 +253,16 @@ public static KubernetesKind getOrRegisterKind(
return getOrRegisterKind(scopedKind.kindName, true, isNamespaced, scopedKind.apiGroup);
}

public static List<KubernetesKind> getOrRegisterKinds(List<String> names) {
@Nonnull
public static List<KubernetesKind> getOrRegisterKinds(@Nonnull List<String> names) {
return names.stream().map(k -> getOrRegisterKind(k, true)).collect(Collectors.toList());
}

@Nonnull
public static List<KubernetesKind> getRegisteredKinds() {
return kindRegistry.getRegisteredKinds();
}

@RequiredArgsConstructor
private static class ScopedKind {
public final String kindName;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright 2019 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.v2.description.manifest;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.function.Supplier;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;

public class KubernetesKindRegistry {
private final List<KubernetesKind> values = Collections.synchronizedList(new ArrayList<>());

/** Registers a given {@link KubernetesKind} into the registry and returns the kind */
@Nonnull
public KubernetesKind registerKind(@Nonnull KubernetesKind kind) {
values.add(kind);
return kind;
}

/**
* Searches the registry for a {@link KubernetesKind} with the supplied name and apiGroup. If a
* kind is found, it is returned. If no kind is found, the provided {@link
* Supplier<KubernetesKind>} is invoked and the resulting kind is registered.
*
* <p>This method is guaranteed to atomically check and register the kind.
*/
@Nonnull
public synchronized KubernetesKind getOrRegisterKind(
@Nonnull final String name,
@Nullable final KubernetesApiGroup apiGroup,
@Nonnull Supplier<KubernetesKind> supplier) {
return getRegisteredKind(name, apiGroup).orElseGet(() -> registerKind(supplier.get()));
}

/**
* Searches the registry for a {@link KubernetesKind} with the supplied name and apiGroup. Returns
* an {@link Optional<KubernetesKind>} containing the kind, or an empty {@link Optional} if no
* kind is found.
*
* <p>Kinds whose API groups are different but are both is a native API groups (see {@link
* KubernetesApiGroup#isNativeGroup()}) are considered to match.
*/
@Nonnull
public Optional<KubernetesKind> getRegisteredKind(
@Nonnull final String name, @Nullable final KubernetesApiGroup apiGroup) {
if (StringUtils.isEmpty(name)) {
return Optional.of(KubernetesKind.NONE);
}

if (name.equalsIgnoreCase(KubernetesKind.NONE.toString())) {
throw new IllegalArgumentException("The 'NONE' kind cannot be read.");
}

Predicate<KubernetesKind> groupMatches =
kind -> {
// Exact match
if (Objects.equals(kind.getApiGroup(), apiGroup)) {
return true;
}

// If we have not specified an API group, default to finding a native kind that matches
if (apiGroup == null || apiGroup.isNativeGroup()) {
return kind.getApiGroup().isNativeGroup();
}

return false;
};

return values.stream()
.filter(
v ->
v.getName().equalsIgnoreCase(name)
|| (v.getAlias() != null && v.getAlias().equalsIgnoreCase(name)))
.filter(groupMatches)
.findAny();
}

/** Returns a list of all registered kinds */
@Nonnull
public List<KubernetesKind> getRegisteredKinds() {
return values;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ private void determineOmitKinds() {
// otherwise, checking all namespaces for all kinds is too expensive in large clusters (imagine
// a cluster with 100s of namespaces).
String checkNamespace = namespaces.get(0);
List<KubernetesKind> allKinds = KubernetesKind.getValues();
List<KubernetesKind> allKinds = KubernetesKind.getRegisteredKinds();

log.info(
"Checking permissions on configured kinds for account {}... {}", accountName, allKinds);
Expand Down
Loading

0 comments on commit 8f3d5e5

Please sign in to comment.