-
Notifications
You must be signed in to change notification settings - Fork 220
ErrorStatusHandler - support for better error reporting in status #685
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
Changes from 4 commits
99464f8
e9cb07c
d32a41f
fcded0c
e99e184
e16e241
eb10689
fee947c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package io.javaoperatorsdk.operator.api.reconciler; | ||
|
||
import io.fabric8.kubernetes.client.CustomResource; | ||
|
||
public interface ErrorStatusHandler<T extends CustomResource<?, ?>> { | ||
|
||
/** | ||
* <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. | ||
* <p> | ||
* The result of the method call is used to make a status sub-resource update on the custom | ||
* resource. This is always a sub-resource update request, so no update on custom resource itself | ||
* (like spec of metadata) happens. Note that this update request will also produce an event, and | ||
* will result in a reconciliation if the controller is not generation aware. | ||
* <p> | ||
* Note that the scope of this feature is only the reconcile method of the reconciler, since there | ||
* should not be updates on custom resource after it is marked for deletion. | ||
* | ||
* @param resource to update the status on | ||
* @param e exception thrown from the reconciler | ||
* @return the updated resource | ||
*/ | ||
T updateErrorStatus(T resource, RuntimeException e); | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,12 +10,7 @@ | |
import io.fabric8.kubernetes.client.dsl.Resource; | ||
import io.javaoperatorsdk.operator.api.ObservedGenerationAware; | ||
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; | ||
import io.javaoperatorsdk.operator.api.reconciler.BaseControl; | ||
import io.javaoperatorsdk.operator.api.reconciler.Context; | ||
import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; | ||
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; | ||
import io.javaoperatorsdk.operator.api.reconciler.Reconciler; | ||
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; | ||
import io.javaoperatorsdk.operator.api.reconciler.*; | ||
|
||
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; | ||
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; | ||
|
@@ -109,27 +104,76 @@ private PostExecutionControl<R> handleCreateOrUpdate( | |
updateCustomResourceWithFinalizer(resource); | ||
return PostExecutionControl.onlyFinalizerAdded(); | ||
} else { | ||
log.debug( | ||
"Executing createOrUpdate for resource {} with version: {} with execution scope: {}", | ||
getName(resource), | ||
getVersion(resource), | ||
executionScope); | ||
try { | ||
var resourceForExecution = | ||
cloneResourceIfErrorStatusHandlerIfCouldBeCalled(resource, context); | ||
return createOrUpdateExecution(executionScope, resourceForExecution, context); | ||
} catch (RuntimeException e) { | ||
handleLastAttemptErrorStatusHandler(resource, context, e); | ||
throw e; | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Resource make sense only to clone for the ErrorStatusHandler. Otherwise, this operation can be | ||
* skipped since it can be memory and time-consuming. However, it needs to be cloned since it's | ||
* common that the custom resource is changed during an execution, and it's much cleaner to have | ||
* to original resource in place for status update. | ||
*/ | ||
private R cloneResourceIfErrorStatusHandlerIfCouldBeCalled(R resource, Context context) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method name doesn't make sense :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. :) not sure if much better |
||
if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { | ||
return (R) this.controller.getConfiguration().getConfigurationService().getResourceCloner() | ||
.clone(resource); | ||
} else { | ||
return resource; | ||
} | ||
} | ||
|
||
UpdateControl<R> updateControl = controller.reconcile(resource, context); | ||
R updatedCustomResource = null; | ||
if (updateControl.isUpdateCustomResourceAndStatusSubResource()) { | ||
updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); | ||
updateControl | ||
.getCustomResource() | ||
.getMetadata() | ||
.setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); | ||
updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); | ||
} else if (updateControl.isUpdateStatusSubResource()) { | ||
updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); | ||
} else if (updateControl.isUpdateCustomResource()) { | ||
updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); | ||
private PostExecutionControl<R> createOrUpdateExecution(ExecutionScope<R> executionScope, | ||
R resource, Context context) { | ||
log.debug( | ||
"Executing createOrUpdate for resource {} with version: {} with execution scope: {}", | ||
getName(resource), | ||
getVersion(resource), | ||
executionScope); | ||
|
||
UpdateControl<R> updateControl = controller.reconcile(resource, context); | ||
R updatedCustomResource = null; | ||
if (updateControl.isUpdateCustomResourceAndStatusSubResource()) { | ||
updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); | ||
updateControl | ||
.getCustomResource() | ||
.getMetadata() | ||
.setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion()); | ||
updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); | ||
} else if (updateControl.isUpdateStatusSubResource()) { | ||
updatedCustomResource = updateStatusGenerationAware(updateControl.getCustomResource()); | ||
} else if (updateControl.isUpdateCustomResource()) { | ||
updatedCustomResource = updateCustomResource(updateControl.getCustomResource()); | ||
} | ||
return createPostExecutionControl(updatedCustomResource, updateControl); | ||
} | ||
|
||
private void handleLastAttemptErrorStatusHandler(R resource, Context context, | ||
RuntimeException e) { | ||
if (isLastAttemptOfRetryAndErrorStatusHandlerPresent(context)) { | ||
try { | ||
var updatedResource = ((ErrorStatusHandler<R>) controller.getReconciler()) | ||
.updateErrorStatus(resource, e); | ||
customResourceFacade.updateStatus(updatedResource); | ||
} catch (RuntimeException ex) { | ||
log.error("Error during error status handling.", ex); | ||
} | ||
return createPostExecutionControl(updatedCustomResource, updateControl); | ||
} | ||
} | ||
|
||
private boolean isLastAttemptOfRetryAndErrorStatusHandlerPresent(Context context) { | ||
if (context.getRetryInfo().isPresent()) { | ||
return context.getRetryInfo().get().isLastAttempt() | ||
&& controller.getReconciler() instanceof ErrorStatusHandler; | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we have the
HasStatus
(or similar) interface, this probably should use that instead ofCustomResource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be moved to HasMetada if the other PR is merged.