vmware-labs / reconciler-runtime

⚠️ Maintenance suspended. Please, migrate to the active fork reconciler.io/runtime. See https://github.com/reconcilerio/runtime/releases/tag/v0.20.0 for instructions. This repository will be archived eventually.
Other
81 stars 18 forks source link

CastResource does not keep its status update on error #473

Closed LittleBaiBai closed 5 months ago

LittleBaiBai commented 5 months ago

Describe the bug

When the reconciliation of the CastResource is erroring, and when the wrapped reconciler needs to update any field or - more commonly needed - set error condition on the resource status, the error condition does not get updated on the original resource. This is the section of code that is causing issue where updating the castOriginal happen after the error checks: cast.go

Reproduction steps

Use the CastResource with a subreconciler that marks conditions to false on error, then trigger that error intentionally. With the current behavior the condition on the original resource will not be updated to the error condition.

Expected behavior

"return subreconciler err, preserves result and status update": {
    Resource: resource.DieReleasePtr(),
    Metadata: map[string]interface{}{
        "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
            return &reconcilers.CastResource[*resources.TestResource, *appsv1.Deployment]{
                Reconciler: &reconcilers.SyncReconciler[*appsv1.Deployment]{
                    SyncWithResult: func(ctx context.Context, resource *appsv1.Deployment) (reconcilers.Result, error) {
                        resource.Status.Conditions[0] = appsv1.DeploymentCondition{
                            Type:    apis.ConditionReady,
                            Status:  corev1.ConditionFalse,
                            Reason:  "Failed",
                            Message: "expected error",
                        }
                        return reconcilers.Result{Requeue: true}, fmt.Errorf("subreconciler error")
                    },
                },
            }
        },
    },
    ExpectedResult: reconcilers.Result{Requeue: true},
    ExpectResource: resource.
        StatusDie(func(d *dies.TestResourceStatusDie) {
            d.ConditionsDie(
                diemetav1.ConditionBlank.Type(apis.ConditionReady).Status(metav1.ConditionFalse).Reason("Failed").Message("expected error"),
            )
        }).
        DieReleasePtr(),
    ShouldErr: true,
},

Additional context

No response