Skip to content

Commit de27382

Browse files
Patch metadata and spec when handling errors (#92)
Description of changes: If any update hook code tried to return a requeue error, the reconciler would short circuit and never patch the metadata and spec. As a result, fields returned by the API server were never being persisted back into the K8s API - which in turn lead to the delta finding differences every loop. This is the root cause for aws-controllers-k8s/community#1251. This pull request adds patching into the `HandleReconcileError` method, which sits at the end of the call stack where we handle the re-queuing errors. Therefore, even if an error is returned, as long as the error is not terminal, the spec will be patched. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent fb55374 commit de27382

File tree

1 file changed

+8
-1
lines changed

1 file changed

+8
-1
lines changed

pkg/runtime/reconciler.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,8 @@ func (r *resourceReconciler) HandleReconcileError(
832832
err error,
833833
) (ctrlrt.Result, error) {
834834
if ackcompare.IsNotNil(latest) {
835+
// Create a copy so we don't override the spec
836+
latestCopy := latest.DeepCopy()
835837
// The reconciliation loop may have returned an error, but if latest is
836838
// not nil, there may be some changes available in the CR's Status
837839
// struct (example: Conditions), and we want to make sure we save those
@@ -845,13 +847,18 @@ func (r *resourceReconciler) HandleReconcileError(
845847
//
846848
// TODO(jaypipes): We ignore error handling here but I don't know if
847849
// there is a more robust way to handle failures in the patch operation
848-
_ = r.patchResourceStatus(ctx, desired, latest)
850+
_ = r.patchResourceStatus(ctx, desired, latestCopy)
849851
}
850852
if err == nil || err == ackerr.Terminal {
851853
return ctrlrt.Result{}, nil
852854
}
853855
rlog := ackrtlog.FromContext(ctx)
854856

857+
// Ensure that we are patching any changes to the annotations/metadata and
858+
// the Spec that may have been set by the resource manager's successful
859+
// Create call above.
860+
_ = r.patchResourceMetadataAndSpec(ctx, desired, latest)
861+
855862
var requeueNeededAfter *requeue.RequeueNeededAfter
856863
if errors.As(err, &requeueNeededAfter) {
857864
after := requeueNeededAfter.Duration()

0 commit comments

Comments
 (0)