Skip to content

Commit

Permalink
refactor: do not use BooleanWithUndefined outside of annotations
Browse files Browse the repository at this point in the history 
Signed-off-by: Chris Laprun <claprun@redhat.com>
  • Loading branch information
@metacosm
metacosm committed May 28, 2024
1 parent 5c59681 commit 8b6b321
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 20 deletions.
5 changes: 2 additions & 3 deletions ...avaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,11 @@ public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent confi
var createResourceOnlyIfNotExistingWithSSA =
DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA;

BooleanWithUndefined useSSA = BooleanWithUndefined.UNDEFINED;

Boolean useSSA = null;
if (configAnnotation != null) {
createResourceOnlyIfNotExistingWithSSA =
configAnnotation.createResourceOnlyIfNotExistingWithSSA();
useSSA = configAnnotation.useSSA();
useSSA = configAnnotation.useSSA().asBoolean();
}

var informerConfiguration = createInformerConfiguration(configAnnotation,
Expand Down
14 changes: 6 additions & 8 deletions ...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,16 @@ protected boolean useSSA(Context<P> context) {
if (usingCustomResourceUpdateMatcher) {
return false;
}
var useSSAConfig =
configuration().map(KubernetesDependentResourceConfig::useSSA)
.orElse(BooleanWithUndefined.UNDEFINED);
var useSSAConfig = configuration()
.map(KubernetesDependentResourceConfig::useSSA)
.orElse(null);
var configService = context.getControllerConfiguration().getConfigurationService();
// don't use SSA for certain resources by default, only if explicitly overriden
if (BooleanWithUndefined.UNDEFINED.equals(useSSAConfig)
&& configService.defaultNonSSAResource().contains(resourceType())) {
if (useSSAConfig == null && configService.defaultNonSSAResource().contains(resourceType())) {
return false;
}
return Optional.ofNullable(useSSAConfig.asBoolean())
.orElse(context.getControllerConfiguration().getConfigurationService()
.ssaBasedCreateUpdateMatchForDependentResources());
return Optional.ofNullable(useSSAConfig)
.orElse(configService.ssaBasedCreateUpdateMatchForDependentResources());
}

private boolean usePreviousAnnotation(Context<P> context) {
Expand Down
9 changes: 4 additions & 5 deletions ...eratorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ public class KubernetesDependentResourceConfig<R extends HasMetadata> {

public static final boolean DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA = true;

private final BooleanWithUndefined useSSA;
private final Boolean useSSA;
private final boolean createResourceOnlyIfNotExistingWithSSA;

private InformerConfiguration<R> informerConfiguration;
private final InformerConfiguration<R> informerConfiguration;

public KubernetesDependentResourceConfig(
BooleanWithUndefined useSSA,
Boolean useSSA,
boolean createResourceOnlyIfNotExistingWithSSA,
InformerConfiguration<R> informerConfiguration) {
this.useSSA = useSSA;
Expand All @@ -27,7 +26,7 @@ public boolean createResourceOnlyIfNotExistingWithSSA() {
return createResourceOnlyIfNotExistingWithSSA;
}

public BooleanWithUndefined useSSA() {
public Boolean useSSA() {
return useSSA;
}

Expand Down
4 changes: 2 additions & 2 deletions ...dk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfigBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
public final class KubernetesDependentResourceConfigBuilder<R extends HasMetadata> {

private boolean createResourceOnlyIfNotExistingWithSSA;
private BooleanWithUndefined useSSA = BooleanWithUndefined.UNDEFINED;

This comment has been minimized.

Sign in to view

Copy link
@csviri

csviri May 28, 2024

Collaborator

I don't think this is better this way, less explicit. Why not to use explicit model if there is already one with the UNDEFIEND.

Undefiend could be renamed actually to "defaults" or such.

This comment has been minimized.

Sign in to view

Copy link
@metacosm

metacosm May 28, 2024

Author Collaborator

Imo, the Boolean class fulfills this role outside of annotations and having a specific type makes things harder to read and deal with (I would also probably need to deal with it specifically in QOSDK) so I'd rather avoid it.

This comment has been minimized.

Sign in to view

Copy link
@csviri

csviri May 28, 2024

Collaborator

FIne, could you at least document it that null means system default in the builder pls?
private Boolean useSSA = null;

private InformerConfiguration<R> informerConfiguration;

Expand All @@ -23,7 +23,7 @@ public KubernetesDependentResourceConfigBuilder<R> withCreateResourceOnlyIfNotEx
return this;
}

public KubernetesDependentResourceConfigBuilder<R> withUseSSA(BooleanWithUndefined useSSA) {
public KubernetesDependentResourceConfigBuilder<R> withUseSSA(boolean useSSA) {
this.useSSA = useSSA;
return this;
}
Expand Down
3 changes: 1 addition & 2 deletions ...src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.javaoperatorsdk.operator.api.reconciler.EventSourceUtils;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.BooleanWithUndefined;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder;
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
Expand All @@ -29,7 +28,7 @@ public DependentSSAReconciler() {

public DependentSSAReconciler(boolean useSSA) {
ssaConfigMapDependent.configureWith(new KubernetesDependentResourceConfigBuilder<ConfigMap>()
.withUseSSA(useSSA ? BooleanWithUndefined.TRUE : BooleanWithUndefined.FALSE)
.withUseSSA(useSSA)
.build());
}

Expand Down

0 comments on commit 8b6b321

Please sign in to comment.