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

Commit

Permalink
Respect child desired controlling owner reference (#397)
Browse files Browse the repository at this point in the history 
When a desired child resource has a controlling owner reference defined,
we now respect that value and no longer attempt to redefine the
reference.

This can be useful when one of the default values needs to have a
different value, or the parent resource type is not defined in the
scheme.

Signed-off-by: Scott Andrews <andrewssc@vmware.com>
  • Loading branch information
@scothis
scothis committed Aug 2, 2023
1 parent b13a740 commit 04cfbf5
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 13 deletions.
2 changes: 1 addition & 1 deletion reconcilers/reconcilers.go
View file
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
View file
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

0 comments on commit 04cfbf5

Please sign in to comment.