Skip to content

Commit

Permalink
Track colliding child resources that are not owned (#569)
Browse files Browse the repository at this point in the history
When creating a child resource with a fixed name, it's possible there
already exists a resource with the same that blocks the creation of
the new resource. If this resource isn't owned by our resource, and owner
relationship are utilized, that creates a blind spot where changes to
that resource will not trigger a new reconcile.

We now track resources in this very specific case. When the target
child resource is deleted the parent resource will now be reconciled.

Signed-off-by: Scott Andrews <[email protected]>
  • Loading branch information
scothis authored Jan 3, 2025
1 parent 3c30843 commit fedae2e
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
5 changes: 5 additions & 0 deletions reconcilers/child.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ func (r *ChildReconciler[T, CT, CLT]) SetupWithManager(ctx context.Context, mgr

if !r.SkipOwnerReference {
bldr.Owns(r.ChildType)
bldr.Watches(r.ChildType, EnqueueTracked(ctx))
}

if err := r.ChildObjectManager.SetupWithManager(ctx, mgr, bldr); err != nil {
Expand Down Expand Up @@ -240,6 +241,10 @@ func (r *ChildReconciler[T, CT, CLT]) Reconcile(ctx context.Context, resource T)
return Result{}, err
}
log.Info("unable to reconcile child, not owned", "child", namespaceName(conflicted), "ownerRefs", conflicted.GetOwnerReferences())
if !r.SkipOwnerReference {
// manually track to watch for deletion since the existing resource is not owned by us
c.Tracker.TrackObject(conflicted, resource)
}
r.ReflectChildStatusOnParent(ctx, resource, child, err)
return Result{}, nil
case apierrs.IsInvalid(err):
Expand Down
18 changes: 18 additions & 0 deletions reconcilers/child_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,12 @@ func TestChildReconciler(t *testing.T) {
d.AddField("foo", "bar")
}).
DieReleasePtr(),
APIGivenObjects: []client.Object{
configMapGiven.
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
d.OwnerReferences()
}),
},
WithReactors: []rtesting.ReactionFunc{
rtesting.InduceFailure("create", "ConfigMap", rtesting.InduceFailureOpts{
Error: apierrs.NewAlreadyExists(schema.GroupResource{}, testName),
Expand All @@ -437,6 +443,9 @@ func TestChildReconciler(t *testing.T) {
ExpectCreates: []client.Object{
configMapCreate,
},
ExpectTracks: []rtesting.TrackRequest{
rtesting.NewTrackRequest(configMapGiven, resource, scheme),
},
},
"child name collision, stale informer cache": {
Resource: resourceReady.
Expand Down Expand Up @@ -908,6 +917,12 @@ func TestChildReconciler_Unstructured(t *testing.T) {
d.AddField("foo", "bar")
}).
DieReleaseUnstructured(),
APIGivenObjects: []client.Object{
configMapGiven.
MetadataDie(func(d *diemetav1.ObjectMetaDie) {
d.OwnerReferences()
}).DieReleaseUnstructured(),
},
WithReactors: []rtesting.ReactionFunc{
rtesting.InduceFailure("create", "ConfigMap", rtesting.InduceFailureOpts{
Error: apierrs.NewAlreadyExists(schema.GroupResource{}, testName),
Expand All @@ -932,6 +947,9 @@ func TestChildReconciler_Unstructured(t *testing.T) {
ExpectCreates: []client.Object{
configMapCreate.DieReleaseUnstructured(),
},
ExpectTracks: []rtesting.TrackRequest{
rtesting.NewTrackRequest(configMapGiven, resource, scheme),
},
},
"child name collision, stale informer cache": {
Resource: resourceReady.
Expand Down

0 comments on commit fedae2e

Please sign in to comment.