Skip to content

Commit 9e5cb13

Browse files
adam-sandoradam-sandor
authored
Merge pull request operator-framework#282 from java-operator-sdk/controller-name-fix
fix: clean up controller default name generation
2 parents fd310b0 + dc4b96f commit 9e5cb13

File tree

11 files changed

+100
-80
lines changed

11 files changed

+100
-80
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerUtils.java

+20
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.javaoperatorsdk.operator;
22

33
import io.fabric8.kubernetes.client.CustomResource;
4+
import io.javaoperatorsdk.operator.api.Controller;
45
import io.javaoperatorsdk.operator.api.ResourceController;
56
import java.util.Locale;
67

@@ -17,6 +18,24 @@ public static boolean hasGivenFinalizer(CustomResource resource, String finalize
1718
&& resource.getMetadata().getFinalizers().contains(finalizer);
1819
}
1920

21+
public static String getNameFor(Class<? extends ResourceController> controllerClass) {
22+
// if the controller annotation has a name attribute, use it
23+
final var annotation = controllerClass.getAnnotation(Controller.class);
24+
if (annotation != null) {
25+
final var name = annotation.name();
26+
if (!Controller.NULL.equals(name)) {
27+
return name;
28+
}
29+
}
30+
31+
// otherwise, use the lower-cased full class name
32+
return getDefaultNameFor(controllerClass);
33+
}
34+
35+
public static String getNameFor(ResourceController controller) {
36+
return getNameFor(controller.getClass());
37+
}
38+
2039
public static String getDefaultNameFor(ResourceController controller) {
2140
return getDefaultNameFor(controller.getClass());
2241
}
@@ -26,6 +45,7 @@ public static String getDefaultNameFor(Class<? extends ResourceController> contr
2645
}
2746

2847
public static String getDefaultResourceControllerName(String rcControllerClassName) {
48+
// if the name is fully qualified, extract the simple class name
2949
final var lastDot = rcControllerClassName.lastIndexOf('.');
3050
if (lastDot > 0) {
3151
rcControllerClassName = rcControllerClassName.substring(lastDot);

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ResourceController.java

-17
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import io.fabric8.kubernetes.client.CustomResource;
44
import io.javaoperatorsdk.operator.processing.event.EventSourceManager;
5-
import java.util.Locale;
65

76
public interface ResourceController<R extends CustomResource> {
87

@@ -38,20 +37,4 @@ public interface ResourceController<R extends CustomResource> {
3837
* @param eventSourceManager
3938
*/
4039
default void init(EventSourceManager eventSourceManager) {}
41-
42-
default String getName() {
43-
final var clazz = getClass();
44-
45-
// if the controller annotation has a name attribute, use it
46-
final var annotation = clazz.getAnnotation(Controller.class);
47-
if (annotation != null) {
48-
final var name = annotation.name();
49-
if (!Controller.NULL.equals(name)) {
50-
return name;
51-
}
52-
}
53-
54-
// otherwise, use the lower-cased full class name
55-
return clazz.getCanonicalName().toLowerCase(Locale.ROOT);
56-
}
5740
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package io.javaoperatorsdk.operator.api.config;
2+
3+
import io.fabric8.kubernetes.client.CustomResource;
4+
import io.javaoperatorsdk.operator.ControllerUtils;
5+
import io.javaoperatorsdk.operator.api.ResourceController;
6+
import java.util.Map;
7+
import java.util.concurrent.ConcurrentHashMap;
8+
9+
public abstract class AbstractConfigurationService implements ConfigurationService {
10+
11+
protected final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
12+
13+
protected <R extends CustomResource> void register(ControllerConfiguration<R> config) {
14+
final var name = config.getName();
15+
final var existing = configurations.get(name);
16+
if (existing != null) {
17+
throw new IllegalArgumentException(
18+
"Controller name '"
19+
+ name
20+
+ "' is used by both "
21+
+ existing.getAssociatedControllerClassName()
22+
+ " and "
23+
+ config.getAssociatedControllerClassName());
24+
}
25+
configurations.put(name, config);
26+
}
27+
28+
@Override
29+
public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
30+
ResourceController<R> controller) {
31+
return configurations.get(ControllerUtils.getNameFor(controller));
32+
}
33+
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java

+2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ public interface ControllerConfiguration<R extends CustomResource> {
2121

2222
Class<? extends CustomResourceDoneable<R>> getDoneableClass();
2323

24+
String getAssociatedControllerClassName();
25+
2426
default boolean isClusterScoped() {
2527
return false;
2628
}

‎operator-framework-quarkus-extension/deployment/src/main/java/io/javaoperatorsdk/quarkus/extension/deployment/QuarkusExtensionProcessor.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ private ControllerConfiguration createControllerConfiguration(
9595
.filter(t -> t.name().equals(RESOURCE_CONTROLLER))
9696
.findFirst()
9797
.map(Type::asParameterizedType)
98-
.orElseThrow(); // shouldn't happen since we're only dealing with ResourceController
99-
// implementors already
98+
// shouldn't happen since we're only dealing with ResourceController implementors
99+
// already
100+
.orElseThrow();
100101
final var crType = rcInterface.arguments().get(0).name().toString();
101102

102103
// create ResourceController bean
@@ -137,11 +138,13 @@ private ControllerConfiguration createControllerConfiguration(
137138
controllerAnnotation, "crdName", AnnotationValue::asString, EXCEPTION_SUPPLIER);
138139
final var configuration =
139140
new QuarkusControllerConfiguration(
141+
resourceControllerClassName,
140142
valueOrDefault(
141143
controllerAnnotation,
142144
"name",
143145
AnnotationValue::asString,
144-
() -> ControllerUtils.getDefaultResourceControllerName(info.simpleName())),
146+
() ->
147+
ControllerUtils.getDefaultResourceControllerName(resourceControllerClassName)),
145148
crdName,
146149
valueOrDefault(
147150
controllerAnnotation,

‎operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/QuarkusConfigurationService.java

+3-18
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,21 @@
11
package io.javaoperatorsdk.quarkus.extension;
22

33
import io.fabric8.kubernetes.client.Config;
4-
import io.fabric8.kubernetes.client.CustomResource;
54
import io.fabric8.kubernetes.client.KubernetesClient;
6-
import io.javaoperatorsdk.operator.api.ResourceController;
7-
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
5+
import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService;
86
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
9-
import java.util.Collections;
107
import java.util.List;
11-
import java.util.Map;
12-
import java.util.concurrent.ConcurrentHashMap;
138
import javax.inject.Inject;
149

15-
public class QuarkusConfigurationService implements ConfigurationService {
16-
private final Map<String, ControllerConfiguration> controllerConfigurations;
10+
public class QuarkusConfigurationService extends AbstractConfigurationService {
1711
@Inject KubernetesClient client;
1812

1913
public QuarkusConfigurationService(List<ControllerConfiguration> configurations) {
2014
if (configurations != null && !configurations.isEmpty()) {
21-
controllerConfigurations = new ConcurrentHashMap<>(configurations.size());
22-
configurations.forEach(c -> controllerConfigurations.put(c.getName(), c));
23-
} else {
24-
controllerConfigurations = Collections.emptyMap();
15+
configurations.forEach(this::register);
2516
}
2617
}
2718

28-
@Override
29-
public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
30-
ResourceController<R> controller) {
31-
return controllerConfigurations.get(controller.getName());
32-
}
33-
3419
@Override
3520
public Config getClientConfiguration() {
3621
return client.getConfiguration();

‎operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/QuarkusControllerConfiguration.java

+15-8
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
public class QuarkusControllerConfiguration<R extends CustomResource>
1212
implements ControllerConfiguration<R> {
13+
private final String associatedControllerClassName;
1314
private final String name;
1415
private final String crdName;
1516
private final String finalizer;
@@ -23,6 +24,7 @@ public class QuarkusControllerConfiguration<R extends CustomResource>
2324

2425
@RecordableConstructor
2526
public QuarkusControllerConfiguration(
27+
String associatedControllerClassName,
2628
String name,
2729
String crdName,
2830
String finalizer,
@@ -32,6 +34,7 @@ public QuarkusControllerConfiguration(
3234
String crClass,
3335
String doneableClassName,
3436
RetryConfiguration retryConfiguration) {
37+
this.associatedControllerClassName = associatedControllerClassName;
3538
this.name = name;
3639
this.crdName = crdName;
3740
this.finalizer = finalizer;
@@ -87,23 +90,27 @@ public boolean isGenerationAware() {
8790

8891
@Override
8992
public Class<R> getCustomResourceClass() {
90-
try {
91-
return (Class<R>) Thread.currentThread().getContextClassLoader().loadClass(crClass);
92-
} catch (ClassNotFoundException e) {
93-
throw new IllegalArgumentException("Couldn't find class " + crClass);
94-
}
93+
return (Class<R>) loadClass(crClass);
9594
}
9695

9796
@Override
9897
public Class<? extends CustomResourceDoneable<R>> getDoneableClass() {
98+
return (Class<? extends CustomResourceDoneable<R>>) loadClass(doneableClassName);
99+
}
100+
101+
private Class<?> loadClass(String className) {
99102
try {
100-
return (Class<CustomResourceDoneable<R>>)
101-
Thread.currentThread().getContextClassLoader().loadClass(doneableClassName);
103+
return Thread.currentThread().getContextClassLoader().loadClass(className);
102104
} catch (ClassNotFoundException e) {
103-
throw new IllegalArgumentException("Couldn't find class " + doneableClassName);
105+
throw new IllegalArgumentException("Couldn't find class " + className);
104106
}
105107
}
106108

109+
@Override
110+
public String getAssociatedControllerClassName() {
111+
return associatedControllerClassName;
112+
}
113+
107114
@Override
108115
public boolean isClusterScoped() {
109116
return clusterScoped;

‎operator-framework-spring-boot-starter/src/main/java/io/javaoperatorsdk/operator/springboot/starter/OperatorAutoConfiguration.java

+6-15
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,15 @@
66
import io.fabric8.kubernetes.client.DefaultKubernetesClient;
77
import io.fabric8.kubernetes.client.KubernetesClient;
88
import io.fabric8.openshift.client.DefaultOpenShiftClient;
9+
import io.javaoperatorsdk.operator.ControllerUtils;
910
import io.javaoperatorsdk.operator.Operator;
1011
import io.javaoperatorsdk.operator.api.ResourceController;
11-
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
12-
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
12+
import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService;
1313
import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
1414
import io.javaoperatorsdk.operator.config.runtime.AnnotationConfiguration;
1515
import java.util.List;
16-
import java.util.Map;
1716
import java.util.Optional;
1817
import java.util.Set;
19-
import java.util.concurrent.ConcurrentHashMap;
2018
import org.springframework.beans.factory.annotation.Autowired;
2119
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
2220
import org.springframework.boot.context.properties.EnableConfigurationProperties;
@@ -25,9 +23,8 @@
2523

2624
@Configuration
2725
@EnableConfigurationProperties({OperatorConfigurationProperties.class})
28-
public class OperatorAutoConfiguration implements ConfigurationService {
26+
public class OperatorAutoConfiguration extends AbstractConfigurationService {
2927
@Autowired private OperatorConfigurationProperties configuration;
30-
private final Map<String, ControllerConfiguration> controllers = new ConcurrentHashMap<>();
3128

3229
@Bean
3330
@ConditionalOnMissingBean
@@ -60,18 +57,12 @@ public Operator operator(
6057

6158
private ResourceController<?> processController(ResourceController<?> controller) {
6259
final var controllerPropertiesMap = configuration.getControllers();
63-
var controllerProps = controllerPropertiesMap.get(controller.getName());
64-
final var cfg = new ConfigurationWrapper(controller, controllerProps);
65-
this.controllers.put(controller.getName(), cfg);
60+
final var name = ControllerUtils.getNameFor(controller);
61+
var controllerProps = controllerPropertiesMap.get(name);
62+
register(new ConfigurationWrapper(controller, controllerProps));
6663
return controller;
6764
}
6865

69-
@Override
70-
public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
71-
ResourceController<R> controller) {
72-
return controllers.get(controller.getName());
73-
}
74-
7566
private static class ConfigurationWrapper<R extends CustomResource>
7667
extends AnnotationConfiguration<R> {
7768
private final Optional<ControllerProperties> properties;

‎operator-framework-spring-boot-starter/src/test/java/io/javaoperatorsdk/operator/springboot/starter/AutoConfigurationTest.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,7 @@ public void loadsKubernetesClientPropertiesProperly() {
3939
@Test
4040
public void loadsRetryPropertiesProperly() {
4141
final var retryProperties =
42-
config
43-
.getControllers()
44-
.get(ControllerUtils.getDefaultNameFor(TestController.class))
45-
.getRetry();
42+
config.getControllers().get(ControllerUtils.getNameFor(TestController.class)).getRetry();
4643
assertEquals(3, retryProperties.getMaxAttempts());
4744
assertEquals(1000, retryProperties.getInitialInterval());
4845
assertEquals(1.5, retryProperties.getIntervalMultiplier());

‎operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AnnotationConfiguration.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ public AnnotationConfiguration(ResourceController<R> controller) {
2121

2222
@Override
2323
public String getName() {
24-
final var name = annotation.name();
25-
return Controller.NULL.equals(name) ? ControllerUtils.getDefaultNameFor(controller) : name;
24+
return ControllerUtils.getNameFor(controller);
2625
}
2726

2827
@Override
@@ -63,4 +62,9 @@ public boolean isClusterScoped() {
6362
public Set<String> getNamespaces() {
6463
return Set.of(annotation.namespaces());
6564
}
65+
66+
@Override
67+
public String getAssociatedControllerClassName() {
68+
return controller.getClass().getCanonicalName();
69+
}
6670
}

‎operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationService.java

+8-13
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,13 @@
22

33
import io.fabric8.kubernetes.client.CustomResource;
44
import io.javaoperatorsdk.operator.api.ResourceController;
5+
import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService;
56
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
67
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
7-
import java.util.Map;
8-
import java.util.concurrent.ConcurrentHashMap;
98

10-
public class DefaultConfigurationService implements ConfigurationService {
9+
public class DefaultConfigurationService extends AbstractConfigurationService {
1110

1211
private static final ConfigurationService instance = new DefaultConfigurationService();
13-
private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
1412

1513
public static ConfigurationService instance() {
1614
return instance;
@@ -19,15 +17,12 @@ public static ConfigurationService instance() {
1917
@Override
2018
public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
2119
ResourceController<R> controller) {
22-
if (controller == null) {
23-
return null;
20+
var config = super.getConfigurationFor(controller);
21+
if (config == null) {
22+
// create the the configuration on demand and register it
23+
config = new AnnotationConfiguration(controller);
24+
register(config);
2425
}
25-
final var name = controller.getName();
26-
var configuration = configurations.get(name);
27-
if (configuration == null) {
28-
configuration = new AnnotationConfiguration(controller);
29-
configurations.put(name, configuration);
30-
}
31-
return configuration;
26+
return config;
3227
}
3328
}

0 commit comments

Comments
 (0)