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

Respect desired child's controlling owner reference #397

Merged
merged 1 commit into from
Aug 2, 2023
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
2 changes: 1 addition & 1 deletion reconcilers/reconcilers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@ func (r *ChildReconciler[T, CT, CLT]) reconcile(ctx context.Context, resource T)
return nilCT, err
}
if !internal.IsNil(desired) {
if !r.SkipOwnerReference {
if !r.SkipOwnerReference && metav1.GetControllerOfNoCopy(desired) == nil {
if err := ctrl.SetControllerReference(resource, desired, c.Scheme()); err != nil {
return nilCT, err
}
Expand Down
83 changes: 71 additions & 12 deletions reconcilers/reconcilers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)
Expand Down Expand Up @@ -859,7 +860,7 @@ func TestResourceReconciler(t *testing.T) {
resource.Status.Fields = map[string]string{
"want": "this to run",
}
return reconcilers.Result{ Requeue: true }, reconcilers.HaltSubReconcilers
return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
},
},
&reconcilers.SyncReconciler[*resources.TestResource]{
Expand All @@ -882,7 +883,7 @@ func TestResourceReconciler(t *testing.T) {
d.AddField("want", "this to run")
}),
},
ExpectedResult: reconcilers.Result{ Requeue: true },
ExpectedResult: reconcilers.Result{Requeue: true},
},
"status update failed": {
Request: testRequest,
Expand Down Expand Up @@ -1384,7 +1385,7 @@ func TestAggregateReconciler(t *testing.T) {
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
return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
},
},
&reconcilers.SyncReconciler[*corev1.ConfigMap]{
Expand All @@ -1404,7 +1405,7 @@ func TestAggregateReconciler(t *testing.T) {
configMapCreate.
AddData("foo", "bar"),
},
ExpectedResult: reconcilers.Result { Requeue: true },
ExpectedResult: reconcilers.Result{Requeue: true},
},
"context is stashable": {
Request: request,
Expand Down Expand Up @@ -1555,13 +1556,13 @@ func TestSyncReconciler(t *testing.T) {
"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
return reconcilers.Result{Requeue: true}, reconcilers.HaltSubReconcilers
},
}
},
},
ExpectedResult: reconcilers.Result { Requeue: true },
ShouldErr: true,
ExpectedResult: reconcilers.Result{Requeue: true},
ShouldErr: true,
},
"sync error": {
Resource: resource.DieReleasePtr(),
Expand Down Expand Up @@ -1647,7 +1648,7 @@ func TestSyncReconciler(t *testing.T) {
},
},
ExpectedResult: reconcile.Result{RequeueAfter: 3 * time.Hour},
ShouldErr: true,
ShouldErr: true,
},
"should finalize and sync deleted resources when asked to": {
Resource: resource.
Expand Down Expand Up @@ -1922,6 +1923,64 @@ func TestChildReconciler(t *testing.T) {
configMapCreate,
},
},
"create child with custom owner reference": {
Resource: resource.
SpecDie(func(d *dies.TestResourceSpecDie) {
d.AddField("foo", "bar")
}).
DieReleasePtr(),
Metadata: map[string]interface{}{
"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
r := defaultChildReconciler(c)
desiredChild := r.DesiredChild
r.DesiredChild = func(ctx context.Context, resource *resources.TestResource) (*corev1.ConfigMap, error) {
child, err := desiredChild(ctx, resource)
if child != nil {
child.OwnerReferences = []metav1.OwnerReference{
{
APIVersion: resources.GroupVersion.String(),
Kind: "TestResource",
Name: resource.GetName(),
UID: resource.GetUID(),
Controller: pointer.Bool(true),
// the default controller ref is set to block
BlockOwnerDeletion: pointer.Bool(false),
},
}
}
return child, err
}
return r
},
},
ExpectEvents: []rtesting.Event{
rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "Created",
`Created ConfigMap %q`, testName),
},
ExpectResource: resourceReady.
SpecDie(func(d *dies.TestResourceSpecDie) {
d.AddField("foo", "bar")
}).
StatusDie(func(d *dies.TestResourceStatusDie) {
d.AddField("foo", "bar")
}).
DieReleasePtr(),
ExpectCreates: []client.Object{
configMapCreate.
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
d.OwnerReferences(
metav1.OwnerReference{
APIVersion: resources.GroupVersion.String(),
Kind: "TestResource",
Name: resource.GetName(),
UID: resource.GetUID(),
Controller: pointer.Bool(true),
BlockOwnerDeletion: pointer.Bool(false),
},
)
}),
},
},
"create child with finalizer": {
Resource: resource.
SpecDie(func(d *dies.TestResourceSpecDie) {
Expand Down Expand Up @@ -3757,7 +3816,7 @@ func TestSequence(t *testing.T) {
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
return reconcilers.Result{RequeueAfter: 1 * time.Minute}, nil
},
},&reconcilers.SyncReconciler[*resources.TestResource]{
}, &reconcilers.SyncReconciler[*resources.TestResource]{
SyncWithResult: func(ctx context.Context, resource *resources.TestResource) (reconcilers.Result, error) {
return reconcilers.Result{RequeueAfter: 2 * time.Minute}, reconcilers.HaltSubReconcilers
},
Expand All @@ -3766,7 +3825,7 @@ func TestSequence(t *testing.T) {
},
},
ExpectedResult: reconcilers.Result{RequeueAfter: 1 * time.Minute},
ShouldErr: true,
ShouldErr: true,
},
"preserves result, Requeue": {
Resource: resource.DieReleasePtr(),
Expand Down Expand Up @@ -4076,14 +4135,14 @@ func TestCastResource(t *testing.T) {
return &reconcilers.CastResource[*resources.TestResource, *appsv1.Deployment]{
Reconciler: &reconcilers.SyncReconciler[*appsv1.Deployment]{
SyncWithResult: func(ctx context.Context, resource *appsv1.Deployment) (reconcilers.Result, error) {
return reconcilers.Result{ Requeue: true }, fmt.Errorf("subreconciler error")
return reconcilers.Result{Requeue: true}, fmt.Errorf("subreconciler error")
},
},
}
},
},
ExpectedResult: reconcilers.Result{Requeue: true},
ShouldErr: true,
ShouldErr: true,
},
"marshal error": {
Resource: resource.
Expand Down