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

Update original cast resource even on reconciliation error #474

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions reconcilers/cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"context"
"encoding/json"
"fmt"
"k8s.io/apimachinery/pkg/util/errors"
"reflect"
"sync"

Expand Down Expand Up @@ -111,22 +112,25 @@ func (r *CastResource[T, CT]) Reconcile(ctx context.Context, resource T) (Result
}
castOriginal := castResource.DeepCopyObject().(client.Object)
result, err := r.Reconciler.Reconcile(ctx, castResource)
var errs []error
if err != nil {
return result, err
errs = append(errs, err)
}
if !equality.Semantic.DeepEqual(castResource, castOriginal) {
// patch the reconciled resource with the updated duck values
patch, err := NewPatch(castOriginal, castResource)
if err != nil {
return Result{}, err
errs = append(errs, err)
return result, errors.NewAggregate(errs)
}
err = patch.Apply(resource)
if err != nil {
return Result{}, err
errs = append(errs, err)
return result, errors.NewAggregate(errs)
}

}
return result, nil
return result, errors.NewAggregate(errs)
}

func (r *CastResource[T, CT]) cast(ctx context.Context, resource T) (context.Context, CT, error) {
Expand Down
57 changes: 55 additions & 2 deletions reconcilers/cast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,21 +114,34 @@ func TestCastResource(t *testing.T) {
},
ExpectedResult: reconcilers.Result{Requeue: true},
},
"return subreconciler err, preserves result": {
"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},
ShouldErr: 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,
},
"marshal error": {
Resource: resource.
Expand Down Expand Up @@ -170,6 +183,46 @@ func TestCastResource(t *testing.T) {
},
ShouldErr: true,
},
"cast mutation patch error": {
Resource: resource.DieReleasePtr(),
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
return &reconcilers.CastResource[*resources.TestResource, *resources.TestResource]{
Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{
Sync: func(ctx context.Context, r *resources.TestResource) error {
r.Spec.ErrOnMarshal = true
return fmt.Errorf("subreconciler error")
},
},
}
},
},
ShouldErr: true,
},
"cast mutation patch apply error": {
Resource: resource.DieReleasePtr(),
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
return &reconcilers.CastResource[*resources.TestResource, *resources.TestResource]{
Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{
Sync: func(ctx context.Context, r *resources.TestResource) error {
r.Spec.ErrOnUnmarshal = true
return fmt.Errorf("subreconciler error")
},
},
}
},
},
ExpectResource: resource.
SpecDie(func(d *dies.TestResourceSpecDie) {
d.ErrOnUnmarshal(true)
}).
StatusDie(func(d *dies.TestResourceStatusDie) {
d.ConditionsDie() // The unmarshal error would result in losing the initializing Ready condition during applying the patch
}).
DieReleasePtr(),
ShouldErr: true,
},
}

rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase[*resources.TestResource], c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
Expand Down
Loading