Skip to content

Commit 3bb8172

Browse files
metacosmmetacosm
authored
refactor: make shouldUseSSA work with types instead of instances + tests (#2538)
* refactor: make shouldUseSSA work with types instead of instances + tests Signed-off-by: Chris Laprun <[email protected]> * fix: shouldUseSSA should also work with unmanaged configuration Signed-off-by: Chris Laprun <[email protected]> --------- Signed-off-by: Chris Laprun <[email protected]>
1 parent d7ce03e commit 3bb8172

File tree

8 files changed

+189
-61
lines changed

8 files changed

+189
-61
lines changed

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

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -367,24 +367,48 @@ default boolean ssaBasedCreateUpdateMatchForDependentResources() {
367367
* not.
368368
*
369369
* @param dependentResource the {@link KubernetesDependentResource} under consideration
370-
* @return {@code true} if SSA should be used for
371370
* @param <R> the dependent resource type
372371
* @param <P> the primary resource type
372+
* @return {@code true} if SSA should be used for
373373
* @since 4.9.4
374374
*/
375375
default <R extends HasMetadata, P extends HasMetadata> boolean shouldUseSSA(
376376
KubernetesDependentResource<R, P> dependentResource) {
377-
if (dependentResource instanceof ResourceUpdaterMatcher) {
377+
return shouldUseSSA(dependentResource.getClass(), dependentResource.resourceType(),
378+
dependentResource.configuration().orElse(null));
379+
}
380+
381+
/**
382+
* This is mostly useful as an integration point for downstream projects to be able to reuse the
383+
* logic used to determine whether a given {@link KubernetesDependentResource} type should use SSA
384+
* or not.
385+
*
386+
* @param dependentResourceType the {@link KubernetesDependentResource} type under consideration
387+
* @param resourceType the resource type associated with the considered dependent resource type
388+
* @return {@code true} if SSA should be used for specified dependent resource type, {@code false}
389+
* otherwise
390+
* @since 4.9.5
391+
*/
392+
@SuppressWarnings("rawtypes")
393+
default boolean shouldUseSSA(Class<? extends KubernetesDependentResource> dependentResourceType,
394+
Class<? extends HasMetadata> resourceType,
395+
KubernetesDependentResourceConfig<? extends HasMetadata> config) {
396+
if (ResourceUpdaterMatcher.class.isAssignableFrom(dependentResourceType)) {
378397
return false;
379398
}
380-
Optional<Boolean> useSSAConfig = dependentResource.configuration()
381-
.flatMap(KubernetesDependentResourceConfig::useSSA);
382-
// don't use SSA for certain resources by default, only if explicitly overriden
383-
if (useSSAConfig.isEmpty()
384-
&& defaultNonSSAResources().contains(dependentResource.resourceType())) {
385-
return false;
399+
Boolean useSSAConfig = Optional.ofNullable(config)
400+
.flatMap(KubernetesDependentResourceConfig::useSSA)
401+
.orElse(null);
402+
// don't use SSA for certain resources by default, only if explicitly overridden
403+
if (useSSAConfig == null) {
404+
if (defaultNonSSAResources().contains(resourceType)) {
405+
return false;
406+
} else {
407+
return ssaBasedCreateUpdateMatchForDependentResources();
408+
}
409+
} else {
410+
return useSSAConfig;
386411
}
387-
return useSSAConfig.orElse(ssaBasedCreateUpdateMatchForDependentResources());
388412
}
389413

390414
/**

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P
193193

194194
protected boolean useSSA(Context<P> context) {
195195
if (useSSA == null) {
196-
useSSA = context.getControllerConfiguration().getConfigurationService().shouldUseSSA(this);
196+
useSSA = context.getControllerConfiguration().getConfigurationService()
197+
.shouldUseSSA(getClass(), resourceType(), configuration().orElse(null));
197198
}
198199
return useSSA;
199200
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
1313
import io.fabric8.kubernetes.client.dsl.base.PatchType;
1414
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
15+
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSACustomResource;
1516
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler;
1617
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec;
17-
import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource;
1818
import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent;
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
@@ -86,8 +86,8 @@ void testMatchingAndUpdate() {
8686
});
8787
}
8888

89-
public DependnetSSACustomResource testResource() {
90-
DependnetSSACustomResource resource = new DependnetSSACustomResource();
89+
public DependentSSACustomResource testResource() {
90+
DependentSSACustomResource resource = new DependentSSACustomResource();
9191
resource.setMetadata(new ObjectMetaBuilder()
9292
.withName(TEST_RESOURCE_NAME)
9393
.build());

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
1313
import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
1414
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
15+
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSACustomResource;
1516
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler;
1617
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec;
17-
import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource;
1818
import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent;
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
@@ -33,7 +33,7 @@ class DependentSSAMigrationIT {
3333
@BeforeEach
3434
void setup(TestInfo testInfo) {
3535
SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0);
36-
LocallyRunOperatorExtension.applyCrd(DependnetSSACustomResource.class, client);
36+
LocallyRunOperatorExtension.applyCrd(DependentSSACustomResource.class, client);
3737
testInfo.getTestMethod().ifPresent(method -> {
3838
namespace = KubernetesResourceUtil.sanitizeName(method.getName());
3939
cleanup();
@@ -53,7 +53,7 @@ void cleanup() {
5353
@Test
5454
void migratesFromLegacyToWorksAndBack() {
5555
var legacyOperator = createOperator(client, true, null);
56-
DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
56+
DependentSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
5757

5858
var operator = createOperator(client, false, null);
5959
testResource = reconcileWithNewApproach(testResource, operator);
@@ -66,7 +66,7 @@ void migratesFromLegacyToWorksAndBack() {
6666
@Test
6767
void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() {
6868
var legacyOperator = createOperator(client, true, null);
69-
DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
69+
DependentSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
7070

7171
var operator = createOperator(client, false,
7272
FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER);
@@ -83,7 +83,7 @@ void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() {
8383
}
8484

8585
private void reconcileAgainWithLegacy(Operator legacyOperator,
86-
DependnetSSACustomResource testResource) {
86+
DependentSSACustomResource testResource) {
8787
legacyOperator.start();
8888

8989
testResource.getSpec().setValue(INITIAL_VALUE);
@@ -98,8 +98,8 @@ private void reconcileAgainWithLegacy(Operator legacyOperator,
9898
legacyOperator.stop();
9999
}
100100

101-
private DependnetSSACustomResource reconcileWithNewApproach(
102-
DependnetSSACustomResource testResource, Operator operator) {
101+
private DependentSSACustomResource reconcileWithNewApproach(
102+
DependentSSACustomResource testResource, Operator operator) {
103103
operator.start();
104104

105105
await().untilAsserted(() -> {
@@ -124,7 +124,7 @@ private ConfigMap getDependentConfigMap() {
124124
return client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get();
125125
}
126126

127-
private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) {
127+
private DependentSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) {
128128
legacyOperator.start();
129129

130130
var testResource = client.resource(testResource()).create();
@@ -155,8 +155,8 @@ private Operator createOperator(KubernetesClient client, boolean legacyDependent
155155
return operator;
156156
}
157157

158-
public DependnetSSACustomResource testResource() {
159-
DependnetSSACustomResource resource = new DependnetSSACustomResource();
158+
public DependentSSACustomResource testResource() {
159+
DependentSSACustomResource resource = new DependentSSACustomResource();
160160
resource.setMetadata(new ObjectMetaBuilder()
161161
.withNamespace(namespace)
162162
.withName(TEST_RESOURCE_NAME)

0 commit comments

Comments
 (0)