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

Preserve results when subreconcilers halt #390

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Preserve results when subreconcilers halt #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions reconcilers/reconcilers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,20 +903,20 @@ func (r *SyncReconciler[T]) Reconcile(ctx context.Context, resource T) (Result,

if resource.GetDeletionTimestamp() == nil || r.SyncDuringFinalization {
syncResult, err := r.sync(ctx, resource)
result = AggregateResults(result, syncResult)
if err != nil {
log.Error(err, "unable to sync")
return Result{}, err
return result, err
}
result = AggregateResults(result, syncResult)
}

if resource.GetDeletionTimestamp() != nil {
finalizeResult, err := r.finalize(ctx, resource)
result = AggregateResults(result, finalizeResult)
if err != nil {
log.Error(err, "unable to finalize")
return Result{}, err
return result, err
}
result = AggregateResults(result, finalizeResult)
}

return result, nil
Expand Down Expand Up @@ -1299,10 +1299,10 @@ func (r Sequence[T]) Reconcile(ctx context.Context, resource T) (Result, error)
ctx = logr.NewContext(ctx, log)

result, err := reconciler.Reconcile(ctx, resource)
aggregateResult = AggregateResults(result, aggregateResult)
if err != nil {
return Result{}, err
return result, err
}
aggregateResult = AggregateResults(result, aggregateResult)
}

return aggregateResult, nil
Expand Down Expand Up @@ -1397,7 +1397,7 @@ func (r *CastResource[T, CT]) Reconcile(ctx context.Context, resource T) (Result
castOriginal := castResource.DeepCopyObject().(client.Object)
result, err := r.Reconciler.Reconcile(ctx, castResource)
if err != nil {
return Result{}, err
return result, err
}
if !equality.Semantic.DeepEqual(castResource, castOriginal) {
// patch the reconciled resource with the updated duck values
Expand Down
135 changes: 132 additions & 3 deletions reconcilers/reconcilers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,47 @@ func TestResourceReconciler(t *testing.T) {
}),
},
},
"sub reconciler halted with result": {
Request: testRequest,
StatusSubResourceTypes: []client.Object{
&resources.TestResource{},
},
GivenObjects: []client.Object{
resource,
},
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
return reconcilers.Sequence[*resources.TestResource]{
&reconcilers.SyncReconciler[*resources.TestResource]{
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
resource.Status.Fields = map[string]string{
"want": "this to run",
}
return reconcilers.Result{ Requeue: true }, reconcilers.HaltSubReconcilers
},
},
&reconcilers.SyncReconciler[*resources.TestResource]{
Sync: func(ctx context.Context, resource *resources.TestResource) error {
resource.Status.Fields = map[string]string{
"don't want": "this to run",
}
return fmt.Errorf("reconciler error")
},
},
}
},
},
ExpectEvents: []rtesting.Event{
rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated",
`Updated status`),
},
ExpectStatusUpdates: []client.Object{
resource.StatusDie(func(d *dies.TestResourceStatusDie) {
d.AddField("want", "this to run")
}),
},
ExpectedResult: reconcilers.Result{ Requeue: true },
},
"status update failed": {
Request: testRequest,
StatusSubResourceTypes: []client.Object{
Expand Down Expand Up @@ -1335,6 +1376,36 @@ func TestAggregateReconciler(t *testing.T) {
AddData("foo", "bar"),
},
},
"reconcile halted with result": {
Request: request,
Metadata: map[string]interface{}{
"Reconciler": func(t *testing.T, c reconcilers.Config) reconcile.Reconciler {
r := defaultAggregateReconciler(c)
r.Reconciler = reconcilers.Sequence[*corev1.ConfigMap]{
&reconcilers.SyncReconciler[*corev1.ConfigMap]{
SyncWithResult: func(ctx context.Context, resource *corev1.ConfigMap) (reconcilers.Result, error) {
return reconcilers.Result { Requeue: true }, reconcilers.HaltSubReconcilers
},
},
&reconcilers.SyncReconciler[*corev1.ConfigMap]{
Sync: func(ctx context.Context, resource *corev1.ConfigMap) error {
return fmt.Errorf("test error")
},
},
}
return r
},
},
ExpectEvents: []rtesting.Event{
rtesting.NewEvent(configMapGiven, scheme, corev1.EventTypeNormal, "Created",
`Created ConfigMap %q`, testName),
},
ExpectCreates: []client.Object{
configMapCreate.
AddData("foo", "bar"),
},
ExpectedResult: reconcilers.Result { Requeue: true },
},
"context is stashable": {
Request: request,
GivenObjects: []client.Object{
Expand Down Expand Up @@ -1478,6 +1549,20 @@ func TestSyncReconciler(t *testing.T) {
},
},
},
"sync with result halted": {
Resource: resource.DieReleasePtr(),
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
return &reconcilers.SyncReconciler[*resources.TestResource]{
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
return reconcilers.Result { Requeue: true }, reconcilers.HaltSubReconcilers
},
}
},
},
ExpectedResult: reconcilers.Result { Requeue: true },
ShouldErr: true,
},
"sync error": {
Resource: resource.DieReleasePtr(),
Metadata: map[string]interface{}{
Expand Down Expand Up @@ -1541,6 +1626,29 @@ func TestSyncReconciler(t *testing.T) {
},
ExpectedResult: reconcile.Result{RequeueAfter: 3 * time.Hour},
},
"finalize can halt subreconcilers": {
Resource: resource.
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
d.DeletionTimestamp(&now)
d.Finalizers(testFinalizer)
}).
DieReleasePtr(),
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
return &reconcilers.SyncReconciler[*resources.TestResource]{
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
t.Errorf("reconciler should not call sync for deleted resources")
return reconcilers.Result{RequeueAfter: 2 * time.Hour}, nil
},
FinalizeWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
return reconcilers.Result{RequeueAfter: 3 * time.Hour}, reconcilers.HaltSubReconcilers
},
}
},
},
ExpectedResult: reconcile.Result{RequeueAfter: 3 * time.Hour},
ShouldErr: true,
},
"should finalize and sync deleted resources when asked to": {
Resource: resource.
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
Expand Down Expand Up @@ -3640,6 +3748,26 @@ func TestSequence(t *testing.T) {
},
ShouldErr: true,
},
"preserves result, sub reconciler halted": {
Resource: resource.DieReleasePtr(),
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
return reconcilers.Sequence[*resources.TestResource]{
&reconcilers.SyncReconciler[*resources.TestResource]{
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
return reconcilers.Result{RequeueAfter: 1 * time.Minute}, nil
},
},&reconcilers.SyncReconciler[*resources.TestResource]{
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
return reconcilers.Result{RequeueAfter: 2 * time.Minute}, reconcilers.HaltSubReconcilers
},
},
}
},
},
ExpectedResult: reconcilers.Result{RequeueAfter: 1 * time.Minute},
ShouldErr: true,
},
"preserves result, Requeue": {
Resource: resource.DieReleasePtr(),
Metadata: map[string]interface{}{
Expand Down Expand Up @@ -3941,19 +4069,20 @@ func TestCastResource(t *testing.T) {
},
ExpectedResult: reconcilers.Result{Requeue: true},
},
"return subreconciler err": {
"return subreconciler err, preserves result": {
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]{
Sync: func(ctx context.Context, resource *appsv1.Deployment) error {
return fmt.Errorf("subreconciler error")
SyncWithResult: func(ctx context.Context, resource *appsv1.Deployment) (reconcilers.Result, error) {
return reconcilers.Result{ Requeue: true }, fmt.Errorf("subreconciler error")
},
},
}
},
},
ExpectedResult: reconcilers.Result{Requeue: true},
ShouldErr: true,
},
"marshal error": {
Expand Down