Skip to content

Commit

Permalink
refactor: make shouldUseSSA work with types instead of instances + te…
Browse files Browse the repository at this point in the history 
…sts (#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]>
  • Loading branch information
@metacosm
metacosm authored Sep 24, 2024
1 parent d7ce03e commit 3bb8172
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 61 deletions.
42 changes: 33 additions & 9 deletions ...ework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,24 +367,48 @@ default boolean ssaBasedCreateUpdateMatchForDependentResources() {
* not.
*
* @param dependentResource the {@link KubernetesDependentResource} under consideration
* @return {@code true} if SSA should be used for
* @param <R> the dependent resource type
* @param <P> the primary resource type
* @return {@code true} if SSA should be used for
* @since 4.9.4
*/
default <R extends HasMetadata, P extends HasMetadata> boolean shouldUseSSA(
KubernetesDependentResource<R, P> dependentResource) {
if (dependentResource instanceof ResourceUpdaterMatcher) {
return shouldUseSSA(dependentResource.getClass(), dependentResource.resourceType(),
dependentResource.configuration().orElse(null));
}

/**
* This is mostly useful as an integration point for downstream projects to be able to reuse the
* logic used to determine whether a given {@link KubernetesDependentResource} type should use SSA
* or not.
*
* @param dependentResourceType the {@link KubernetesDependentResource} type under consideration
* @param resourceType the resource type associated with the considered dependent resource type
* @return {@code true} if SSA should be used for specified dependent resource type, {@code false}
* otherwise
* @since 4.9.5
*/
@SuppressWarnings("rawtypes")
default boolean shouldUseSSA(Class<? extends KubernetesDependentResource> dependentResourceType,
Class<? extends HasMetadata> resourceType,
KubernetesDependentResourceConfig<? extends HasMetadata> config) {
if (ResourceUpdaterMatcher.class.isAssignableFrom(dependentResourceType)) {
return false;
}
Optional<Boolean> useSSAConfig = dependentResource.configuration()
.flatMap(KubernetesDependentResourceConfig::useSSA);
// don't use SSA for certain resources by default, only if explicitly overriden
if (useSSAConfig.isEmpty()
&& defaultNonSSAResources().contains(dependentResource.resourceType())) {
return false;
Boolean useSSAConfig = Optional.ofNullable(config)
.flatMap(KubernetesDependentResourceConfig::useSSA)
.orElse(null);
// don't use SSA for certain resources by default, only if explicitly overridden
if (useSSAConfig == null) {
if (defaultNonSSAResources().contains(resourceType)) {
return false;
} else {
return ssaBasedCreateUpdateMatchForDependentResources();
}
} else {
return useSSAConfig;
}
return useSSAConfig.orElse(ssaBasedCreateUpdateMatchForDependentResources());
}

/**
Expand Down
3 changes: 2 additions & 1 deletion ...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P

protected boolean useSSA(Context<P> context) {
if (useSSA == null) {
useSSA = context.getControllerConfiguration().getConfigurationService().shouldUseSSA(this);
useSSA = context.getControllerConfiguration().getConfigurationService()
.shouldUseSSA(getClass(), resourceType(), configuration().orElse(null));
}
return useSSA;
}
Expand Down
6 changes: 3 additions & 3 deletions operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
import io.fabric8.kubernetes.client.dsl.base.PatchType;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSACustomResource;
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler;
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec;
import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource;
import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -86,8 +86,8 @@ void testMatchingAndUpdate() {
});
}

public DependnetSSACustomResource testResource() {
DependnetSSACustomResource resource = new DependnetSSACustomResource();
public DependentSSACustomResource testResource() {
DependentSSACustomResource resource = new DependentSSACustomResource();
resource.setMetadata(new ObjectMetaBuilder()
.withName(TEST_RESOURCE_NAME)
.build());
Expand Down
20 changes: 10 additions & 10 deletions operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSACustomResource;
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler;
import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec;
import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource;
import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -33,7 +33,7 @@ class DependentSSAMigrationIT {
@BeforeEach
void setup(TestInfo testInfo) {
SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0);
LocallyRunOperatorExtension.applyCrd(DependnetSSACustomResource.class, client);
LocallyRunOperatorExtension.applyCrd(DependentSSACustomResource.class, client);
testInfo.getTestMethod().ifPresent(method -> {
namespace = KubernetesResourceUtil.sanitizeName(method.getName());
cleanup();
Expand All @@ -53,7 +53,7 @@ void cleanup() {
@Test
void migratesFromLegacyToWorksAndBack() {
var legacyOperator = createOperator(client, true, null);
DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
DependentSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);

var operator = createOperator(client, false, null);
testResource = reconcileWithNewApproach(testResource, operator);
Expand All @@ -66,7 +66,7 @@ void migratesFromLegacyToWorksAndBack() {
@Test
void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() {
var legacyOperator = createOperator(client, true, null);
DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
DependentSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);

var operator = createOperator(client, false,
FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER);
Expand All @@ -83,7 +83,7 @@ void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() {
}

private void reconcileAgainWithLegacy(Operator legacyOperator,
DependnetSSACustomResource testResource) {
DependentSSACustomResource testResource) {
legacyOperator.start();

testResource.getSpec().setValue(INITIAL_VALUE);
Expand All @@ -98,8 +98,8 @@ private void reconcileAgainWithLegacy(Operator legacyOperator,
legacyOperator.stop();
}

private DependnetSSACustomResource reconcileWithNewApproach(
DependnetSSACustomResource testResource, Operator operator) {
private DependentSSACustomResource reconcileWithNewApproach(
DependentSSACustomResource testResource, Operator operator) {
operator.start();

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

private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) {
private DependentSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) {
legacyOperator.start();

var testResource = client.resource(testResource()).create();
Expand Down Expand Up @@ -155,8 +155,8 @@ private Operator createOperator(KubernetesClient client, boolean legacyDependent
return operator;
}

public DependnetSSACustomResource testResource() {
DependnetSSACustomResource resource = new DependnetSSACustomResource();
public DependentSSACustomResource testResource() {
DependentSSACustomResource resource = new DependentSSACustomResource();
resource.setMetadata(new ObjectMetaBuilder()
.withNamespace(namespace)
.withName(TEST_RESOURCE_NAME)
Expand Down
Loading

0 comments on commit 3bb8172

Please sign in to comment.