Skip to content

improve: naming reconcile pre-condition is just condition #2306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/documentation/v5-0-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ permalink: /docs/v5-0-migration
Also, the related part of a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
4. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.
5. Reconcile pre-condition is renamed simply to `Condition` in ( @Dependent annotation, and related builders)
just rename related attributes
6 changes: 3 additions & 3 deletions docs/documentation/workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ reconciliation process.
- **Dependent resource** (DR) - are the resources being managed in a given reconciliation logic.
- **Depends-on relation** - a `B` DR depends on another `A` DR if `B` needs to be reconciled
after `A`.
- **Reconcile precondition** - is a condition on a given DR that needs to be become true before the
- **Condition** - is a condition on a given DR that needs to be become true before the
DR is reconciled. This also allows to define optional resources that would, for example, only be
created if a flag in a custom resource `.spec` has some specific value.
- **Ready postcondition** - is a condition on a given DR to prevent the workflow from
Expand Down Expand Up @@ -76,7 +76,7 @@ will only consider the `ConfigMap` deleted until that post-condition becomes `tr
@Dependent(name = DEPLOYMENT_NAME, type = DeploymentDependentResource.class,
readyPostcondition = DeploymentReadyCondition.class),
@Dependent(type = ConfigMapDependentResource.class,
reconcilePrecondition = ConfigMapReconcileCondition.class,
condition = ConfigMapCondition.class,
deletePostcondition = ConfigMapDeletePostCondition.class,
activationCondition = ConfigMapActivationCondition.class,
dependsOn = DEPLOYMENT_NAME)
Expand Down Expand Up @@ -252,7 +252,7 @@ stateDiagram-v2
- If `2`'s reconciliation resulted in an error, `4` would not be reconciled, but `3`
would be (and `1` as well, of course).

#### Sample with Reconcile Precondition
#### Sample with Condition

<div class="mermaid" markdown="0">

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ private static List<DependentResourceSpec> dependentResources(
spec = new DependentResourceSpec(dependentType, dependentName,
Set.of(dependent.dependsOn()),
Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
Utils.instantiate(dependent.condition(), Condition.class, context),
Utils.instantiate(dependent.deletePostcondition(), Condition.class, context),
Utils.instantiate(dependent.activationCondition(), Condition.class, context),
eventSourceName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class DependentResourceSpec<R, P extends HasMetadata> {

private final Condition<?, ?> readyCondition;

private final Condition<?, ?> reconcileCondition;
private final Condition<?, ?> condition;

private final Condition<?, ?> deletePostCondition;

Expand All @@ -28,13 +28,13 @@ public class DependentResourceSpec<R, P extends HasMetadata> {

public DependentResourceSpec(Class<? extends DependentResource<R, P>> dependentResourceClass,
String name, Set<String> dependsOn, Condition<?, ?> readyCondition,
Condition<?, ?> reconcileCondition, Condition<?, ?> deletePostCondition,
Condition<?, ?> condition, Condition<?, ?> deletePostCondition,
Condition<?, ?> activationCondition, String useEventSourceWithName) {
this.dependentResourceClass = dependentResourceClass;
this.name = name;
this.dependsOn = dependsOn;
this.readyCondition = readyCondition;
this.reconcileCondition = reconcileCondition;
this.condition = condition;
this.deletePostCondition = deletePostCondition;
this.activationCondition = activationCondition;
this.useEventSourceWithName = useEventSourceWithName;
Expand Down Expand Up @@ -81,8 +81,8 @@ public Condition getReadyCondition() {
}

@SuppressWarnings("rawtypes")
public Condition getReconcileCondition() {
return reconcileCondition;
public Condition getCondition() {
return condition;
}

@SuppressWarnings("rawtypes")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@
/**
* The condition (if it exists) that needs to become true before the associated
* {@link DependentResource} is reconciled. Note that if this condition is set and the condition
* doesn't hold true, the associated secondary will be deleted.
* doesn't hold true, the associated secondary will be deleted (if exists).
*
* @return a {@link Condition} implementation, defaulting to the interface itself if no value is
* set
*/
Class<? extends Condition> reconcilePrecondition() default Condition.class;
Class<? extends Condition> condition() default Condition.class;

/**
* The condition (if it exists) that needs to become true before further reconciliation of
Expand All @@ -54,12 +54,11 @@
* <p>
* A condition that needs to become true for the dependent to even be considered as part of the
* workflow. This is useful for dependents that represent optional resources on the cluster and
* might not be present. In this case, a reconcile pre-condition is not enough because in that
* situation, the associated informer will still be registered. With this activation condition,
* the associated event source will only be registered if the condition is met. Otherwise, this
* behaves like a reconcile pre-condition in the sense that dependents, that depend on this one,
* will only get created if the condition is met and will get deleted if the condition becomes
* false.
* might not be present. In this case, a condition is not enough because in that situation, the
* associated informer will still be registered. With this activation condition, the associated
* event source will only be registered if the condition is met. Otherwise, this behaves like a
* reconcile pre-condition in the sense that dependents, that depend on this one, will only get
* created if the condition is met and will get deleted if the condition becomes false.
* </p>
* <p>
* As other conditions, this gets evaluated at the beginning of every reconciliation, which means
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
* {@link io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource}
* to express that the resource deletion is handled by the controller during
* {@link DependentResource#reconcile(HasMetadata, Context)}. This takes effect during a
* reconciliation workflow, but not during a cleanup workflow, when a {@code reconcilePrecondition}
* is not met for the resource. In this case, {@link #delete(HasMetadata, Context)} is called.
* During a cleanup workflow, however, {@link #delete(HasMetadata, Context)} is not called, letting
* the Kubernetes garbage collector do its work instead (using owner references).
* reconciliation workflow, but not during a cleanup workflow, when a {@code condition} is not met
* for the resource. In this case, {@link #delete(HasMetadata, Context)} is called. During a cleanup
* workflow, however, {@link #delete(HasMetadata, Context)} is not called, letting the Kubernetes
* garbage collector do its work instead (using owner references).
* </p>
* <p>
* If a dependent resource implement this interface, an owner reference pointing to the associated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public Workflow<P> resolve(KubernetesClient client,
for (DependentResourceSpec spec : orderedSpecs) {
final var dependentResource = resolve(spec, client, configuration);
final var node = new DependentResourceNode(
spec.getReconcileCondition(),
spec.getCondition(),
spec.getDeletePostCondition(),
spec.getReadyCondition(),
spec.getActivationCondition(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class DependentResourceNode<R, P extends HasMetadata> {
private final List<DependentResourceNode> dependsOn = new LinkedList<>();
private final List<DependentResourceNode> parents = new LinkedList<>();

private Condition<R, P> reconcilePrecondition;
private Condition<R, P> condition;
private Condition<R, P> deletePostcondition;
private Condition<R, P> readyPostcondition;
private Condition<R, P> activationCondition;
Expand All @@ -23,10 +23,10 @@ class DependentResourceNode<R, P extends HasMetadata> {
this(null, null, null, null, dependentResource);
}

public DependentResourceNode(Condition<R, P> reconcilePrecondition,
public DependentResourceNode(Condition<R, P> condition,
Condition<R, P> deletePostcondition, Condition<R, P> readyPostcondition,
Condition<R, P> activationCondition, DependentResource<R, P> dependentResource) {
this.reconcilePrecondition = reconcilePrecondition;
this.condition = condition;
this.deletePostcondition = deletePostcondition;
this.readyPostcondition = readyPostcondition;
this.activationCondition = activationCondition;
Expand All @@ -50,8 +50,8 @@ public List<DependentResourceNode> getParents() {
return parents;
}

public Optional<Condition<R, P>> getReconcilePrecondition() {
return Optional.ofNullable(reconcilePrecondition);
public Optional<Condition<R, P>> getCondition() {
return Optional.ofNullable(condition);
}

public Optional<Condition<R, P>> getDeletePostcondition() {
Expand All @@ -62,8 +62,8 @@ public Optional<Condition<R, P>> getActivationCondition() {
return Optional.ofNullable(activationCondition);
}

void setReconcilePrecondition(Condition<R, P> reconcilePrecondition) {
this.reconcilePrecondition = reconcilePrecondition;
void setCondition(Condition<R, P> condition) {
this.condition = condition;
}

void setDeletePostcondition(Condition<R, P> cleanupCondition) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public WorkflowBuilder<P> dependsOn(DependentResource... dependentResources) {
return this;
}

public WorkflowBuilder<P> withReconcilePrecondition(Condition reconcilePrecondition) {
currentNode.setReconcilePrecondition(reconcilePrecondition);
public WorkflowBuilder<P> withCondition(Condition condition) {
currentNode.setCondition(condition);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ private synchronized <R> void handleReconcile(DependentResourceNode<R, P> depend
dependentResourceNode.getDependentResource());
registerOrDeregisterEventSourceBasedOnActivation(activationConditionMet, dependentResourceNode);

boolean reconcileConditionMet = true;
boolean condition = true;
if (activationConditionMet) {
reconcileConditionMet = isConditionMet(dependentResourceNode.getReconcilePrecondition(),
condition = isConditionMet(dependentResourceNode.getCondition(),
dependentResourceNode.getDependentResource());
}
if (!reconcileConditionMet || !activationConditionMet) {
if (!condition || !activationConditionMet) {
handleReconcileOrActivationConditionNotMet(dependentResourceNode, activationConditionMet);
} else {
submit(dependentResourceNode, new NodeReconcileExecutor<>(dependentResourceNode), RECONCILE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,11 @@ void onlyOneDependsOnErroredResourceNotReconciled() {
}

@Test
void simpleReconcileCondition() {
void simpleCondition() {
var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1).withReconcilePrecondition(notMetCondition)
.addDependentResource(dr2).withReconcilePrecondition(metCondition)
.addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(dr1).withCondition(notMetCondition)
.addDependentResource(dr2).withCondition(metCondition)
.addDependentResource(drDeleter).withCondition(notMetCondition)
.build();

var res = workflow.reconcile(new TestCustomResource(), mockContext);
Expand All @@ -202,7 +202,7 @@ void triangleOnceConditionNotMet() {
var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1)
.addDependentResource(dr2).dependsOn(dr1)
.addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(drDeleter).withCondition(notMetCondition)
.dependsOn(dr1)
.build();

Expand All @@ -215,17 +215,17 @@ void triangleOnceConditionNotMet() {
}

@Test
void reconcileConditionTransitiveDelete() {
void conditionTransitiveDelete() {
TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2");

var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1)
.addDependentResource(dr2).dependsOn(dr1)
.withReconcilePrecondition(notMetCondition)
.withCondition(notMetCondition)
.addDependentResource(drDeleter).dependsOn(dr2)
.withReconcilePrecondition(metCondition)
.withCondition(metCondition)
.addDependentResource(drDeleter2).dependsOn(drDeleter)
.withReconcilePrecondition(metCondition)
.withCondition(metCondition)
.build();

var res = workflow.reconcile(new TestCustomResource(), mockContext);
Expand All @@ -241,14 +241,14 @@ void reconcileConditionTransitiveDelete() {
}

@Test
void reconcileConditionAlsoErrorDependsOn() {
void conditionAlsoErrorDependsOn() {
TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2");

var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(drError)
.addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(drDeleter).withCondition(notMetCondition)
.addDependentResource(drDeleter2).dependsOn(drError, drDeleter)
.withReconcilePrecondition(metCondition)
.withCondition(metCondition)
.withThrowExceptionFurther(false)
.build();

Expand All @@ -269,7 +269,7 @@ void reconcileConditionAlsoErrorDependsOn() {
void oneDependsOnConditionNotMet() {
var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1)
.addDependentResource(dr2).withReconcilePrecondition(notMetCondition)
.addDependentResource(dr2).withCondition(notMetCondition)
.addDependentResource(drDeleter).dependsOn(dr1, dr2)
.build();

Expand All @@ -284,12 +284,12 @@ void oneDependsOnConditionNotMet() {
}

@Test
void deletedIfReconcileConditionNotMet() {
void deletedIfConditionNotMet() {
TestDeleterDependent drDeleter2 = new TestDeleterDependent("DR_DELETER_2");
var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1)
.addDependentResource(drDeleter).dependsOn(dr1)
.withReconcilePrecondition(notMetCondition)
.withCondition(notMetCondition)
.addDependentResource(drDeleter2).dependsOn(dr1, drDeleter)
.build();

Expand All @@ -312,7 +312,7 @@ void deleteDoneInReverseOrder() {

var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1)
.addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(drDeleter).withCondition(notMetCondition)
.dependsOn(dr1)
.addDependentResource(drDeleter2).dependsOn(drDeleter)
.addDependentResource(drDeleter3).dependsOn(drDeleter)
Expand All @@ -338,7 +338,7 @@ void diamondDeleteWithPostConditionInMiddle() {
TestDeleterDependent drDeleter4 = new TestDeleterDependent("DR_DELETER_4");

var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(drDeleter).withCondition(notMetCondition)
.addDependentResource(drDeleter2).dependsOn(drDeleter)
.addDependentResource(drDeleter3).dependsOn(drDeleter)
.withDeletePostcondition(this.notMetCondition)
Expand All @@ -362,7 +362,7 @@ void diamondDeleteErrorInMiddle() {
TestDeleterDependent drDeleter3 = new TestDeleterDependent("DR_DELETER_3");

var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(drDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(drDeleter).withCondition(notMetCondition)
.addDependentResource(drDeleter2).dependsOn(drDeleter)
.addDependentResource(errorDD).dependsOn(drDeleter)
.addDependentResource(drDeleter3).dependsOn(errorDD, drDeleter2)
Expand Down Expand Up @@ -452,9 +452,9 @@ void diamondShareWithReadyCondition() {
}

@Test
void garbageCollectedResourceIsDeletedIfReconcilePreconditionDoesNotHold() {
void garbageCollectedResourceIsDeletedIfConditionDoesNotHold() {
var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(gcDeleter).withReconcilePrecondition(notMetCondition)
.addDependentResource(gcDeleter).withCondition(notMetCondition)
.build();

var res = workflow.reconcile(new TestCustomResource(), mockContext);
Expand All @@ -464,9 +464,9 @@ void garbageCollectedResourceIsDeletedIfReconcilePreconditionDoesNotHold() {
}

@Test
void garbageCollectedDeepResourceIsDeletedIfReconcilePreconditionDoesNotHold() {
void garbageCollectedDeepResourceIsDeletedIfConditionDoesNotHold() {
var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1).withReconcilePrecondition(notMetCondition)
.addDependentResource(dr1).withCondition(notMetCondition)
.addDependentResource(gcDeleter).dependsOn(dr1)
.build();

Expand Down Expand Up @@ -521,18 +521,18 @@ void readyConditionNotCheckedOnNonActiveDependent() {
}

@Test
void reconcilePreconditionNotCheckedOnNonActiveDependent() {
var precondition = mock(Condition.class);
void conditionNotCheckedOnNonActiveDependent() {
var condition = mock(Condition.class);

var workflow = new WorkflowBuilder<TestCustomResource>()
.addDependentResource(dr1)
.withActivationCondition(notMetCondition)
.withReconcilePrecondition(precondition)
.withCondition(condition)
.build();

workflow.reconcile(new TestCustomResource(), mockContext);

verify(precondition, never()).isMet(any(), any(), any());
verify(condition, never()).isMet(any(), any(), any());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler;
import io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentSpec;

import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.ConfigMapCondition.DO_NOT_RECONCILE;
import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.ConfigMapDependent.TEST_CONFIG_MAP_NAME;
import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.ConfigMapReconcilePrecondition.DO_NOT_RECONCILE;
import static io.javaoperatorsdk.operator.sample.primarytosecondaydependent.PrimaryToSecondaryDependentReconciler.DATA_KEY;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void configMapNotReconciledUntilDeploymentReady() {


@Test
void configMapNotReconciledIfReconcileConditionNotMet() {
void configMapNotReconciledIfConditionNotMet() {
var resource = operator.create(customResource(false));

await().atMost(ONE_MINUTE).untilAsserted(() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class BulkDependentWithConditionIT {
.build();

@Test
void handlesBulkDependentWithPrecondition() {
void handlesBulkDependentWithCondition() {
var resource = testResource();
extension.create(resource);

Expand Down
Loading
Loading