Skip to content

fix: minor fixes #767

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

Merged
merged 9 commits into from
Dec 20, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@

/**
* If the custom resource's status implements this interface, the observed generation will be
* automatically handled. The last observed generation will be updated on status when the status is
* instructed to be updated (see below). In addition to that, controller configuration will be
* checked if is set to generation aware. If generation aware config is turned off, this interface
* is ignored.
*
* In order to work the status object returned by CustomResource.getStatus() should not be null. In
* addition to that from the controller that the {@link UpdateControl#updateStatus(HasMetadata)} or
* {@link UpdateControl#updateResourceAndStatus(HasMetadata)} should be returned. The observed
* generation is not updated in other cases.
*
* automatically handled. The last observed generation will be updated on status. In addition to
* that, controller configuration will be checked if is set to be generation aware. If generation
* aware config is turned off, this interface is ignored.
* <p>
* In order for this automatic handling to work the status object returned by
* {@link CustomResource#getStatus()} should not be null.
* <p>
* The observed generation is updated even when {@link UpdateControl#noUpdate()} or
* {@link UpdateControl#updateResource(HasMetadata)} is called. Although those results call normally
* does not result in a status update, there will be a subsequent status update Kubernetes API call
* in this case.
*
* @see ObservedGenerationAwareStatus
*/
public interface ObservedGenerationAware {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ public interface ErrorStatusHandler<T extends HasMetadata> {

/**
* <p>
* Reconcile can implement this interface in order to update the status sub-resource in the case
* when the last reconciliation retry attempt is failed on the Reconciler. In that case the
* updateErrorStatus is called automatically.
* Reconciler can implement this interface in order to update the status sub-resource in the case
* an exception in thrown. In that case
* {@link #updateErrorStatus(HasMetadata, RetryInfo, RuntimeException)} is called automatically.
* <p>
* The result of the method call is used to make a status update on the custom resource. This is
* always a sub-resource update request, so no update on custom resource itself (like spec of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,30 @@
public interface Reconciler<R extends HasMetadata> {

/**
* Note that this method is used in combination of finalizers. If automatic finalizer handling is
* turned off for the controller, this method is not called.
* The implementation of this operation is required to be idempotent. Always use the UpdateControl
* object to make updates on custom resource if possible.
*
* @param resource the resource that has been created or updated
* @param context the context with which the operation is executed
* @return UpdateControl to manage updates on the custom resource (usually the status) after
* reconciliation.
*/
UpdateControl<R> reconcile(R resource, Context context);

/**
* Note that this method is used in combination with finalizers. If automatic finalizer handling
* is turned off for the controller, this method is not called.
*
* The implementation should delete the associated component(s). Note that this is method is
* called when an object is marked for deletion. After it's executed the custom resource finalizer
* is automatically removed by the framework; unless the return value is
* The implementation should delete the associated component(s). This method is called when an
* object is marked for deletion. After it's executed the custom resource finalizer is
* automatically removed by the framework; unless the return value is
* {@link DeleteControl#noFinalizerRemoval()}, which indicates that the controller has determined
* that the resource should not be deleted yet. This is usually a corner case, when a cleanup is
* tried again eventually.
*
* <p>
* It's important that this method be idempotent, as it could be called several times, depending
* on the conditions and the controller's configuration (for example, if the controller is
* configured to not use a finalizer but the resource does have finalizers, it might be be
* "offered" again for deletion several times until the finalizers are all removed.
* It's important for implementations of this method to be idempotent, since it can be called
* several times.
*
* @param resource the resource that is marked for deletion
* @param context the context with which the operation is executed
Expand All @@ -32,20 +41,4 @@ default DeleteControl cleanup(R resource, Context context) {
return DeleteControl.defaultDelete();
}

/**
* The implementation of this operation is required to be idempotent. Always use the UpdateControl
* object to make updates on custom resource if possible. Also always use the custom resource
* parameter (not the custom resource that might be in the events)
*
* @param resource the resource that has been created or updated
* @param context the context with which the operation is executed
* @return The resource is updated in api server if the return value is not
* {@link UpdateControl#noUpdate()}. This the common use cases. However in cases, for
* example the operator is restarted, and we don't want to have an update call to k8s api
* to be made unnecessarily, by returning {@link UpdateControl#noUpdate()} this update can
* be skipped. <b>However we will always call an update if there is no finalizer on object
* and it's not marked for deletion.</b>
*/
UpdateControl<R> reconcile(R resource, Context context);

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;

/**
* Event handler that makes sure that events are processed in a "single threaded" way per resource
* UID, while buffering events which are received during an execution.
*/
class EventProcessor<R extends HasMetadata> implements EventHandler, LifecycleAware {

private static final Logger log = LoggerFactory.getLogger(EventProcessor.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ private PostExecutionControl<R> handleDispatch(ExecutionScope<R> executionScope)
private boolean shouldNotDispatchToDelete(R resource) {
// we don't dispatch to delete if the controller is configured to use a finalizer but that
// finalizer is not present (which means it's already been removed)
return configuration().useFinalizer() && !resource.hasFinalizer(configuration().getFinalizer());
return !configuration().useFinalizer() || (configuration().useFinalizer()
&& !resource.hasFinalizer(configuration().getFinalizer()));
}

private PostExecutionControl<R> handleReconcile(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;

/**
* This is a special case since is not bound to a single custom resource
*/
public class ControllerResourceEventSource<T extends HasMetadata> extends AbstractEventSource
implements ResourceEventHandler<T> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@
public interface RetryExecution extends RetryInfo {

/**
* Calculates the delay for the next execution. This method should return 0, when called first
* time;
*
* @return the time to wait until the next execution in millisecondsz
* @return the time to wait until the next execution in milliseconds
*/
Optional<Long> nextDelay();
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,14 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
verify(reconciler, times(1)).cleanup(eq(testCustomResource), any());
}

/**
* Note that there could be more finalizers. Out of our control.
*/
@Test
void callDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() {
void doesNotCallDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() {
configureToNotUseFinalizer();
markForDeletion(testCustomResource);

reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));

verify(reconciler).cleanup(eq(testCustomResource), any());
verify(reconciler, times(0)).cleanup(eq(testCustomResource), any());
}

@Test
Expand Down