Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

CastResource does not keep its status update on error #473

Closed
LittleBaiBai opened this issue Jan 23, 2024 · 0 comments · Fixed by #474
Closed

CastResource does not keep its status update on error #473

LittleBaiBai opened this issue Jan 23, 2024 · 0 comments · Fixed by #474
Labels
bug Something isn't working

Comments

@LittleBaiBai
Copy link
Contributor

LittleBaiBai commented Jan 23, 2024

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

@LittleBaiBai LittleBaiBai added the bug Something isn't working label Jan 23, 2024
LittleBaiBai added a commit to LittleBaiBai/reconciler-runtime that referenced this issue Jan 23, 2024
mamachanko pushed a commit that referenced this issue Jan 24, 2024
* Update original cast resource even on reconciliation error

Closes #473
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant