Skip to content

Commit 43dae05

Browse files
committed
fix: use ownership instead of revision to guard teardown deletion
When an Owner is provided, check whether we are the controller via detectOwner rather than comparing revision annotations. Owner refs are the authoritative signal for ownership and correctly handle orphaning deletions where the revision annotation may have been updated by a different reconciliation pass. Callers without an Owner fall back to the existing revision comparison.
1 parent 42e572f commit 43dae05

1 file changed

Lines changed: 14 additions & 13 deletions

File tree

machinery/objects.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -151,19 +151,10 @@ func (e *ObjectEngine) Teardown(
151151
return false, fmt.Errorf("getting object before deletion: %w", err)
152152
}
153153

154-
// Check revision matches.
155-
actualRevision, err := e.getObjectRevision(actualObject)
156-
if err != nil {
157-
return false, fmt.Errorf("getting object revision: %w", err)
158-
}
159-
160-
// Object is not owned by this revision
161-
if actualRevision != revision {
162-
if options.Owner == nil {
163-
// No Owner to remove from the object, return.
164-
return true, nil
165-
}
166-
154+
if options.Owner != nil {
155+
// Check ownership instead of revision to determine if we should delete.
156+
// If we're not the controller, only remove our owner ref and leave the object in place.
157+
// A possible reason for this could be an orphaning deletion.
167158
ctrlSit, _ := e.detectOwner(options.Owner, options.OwnerStrategy, actualObject, nil)
168159
if ctrlSit != ctrlSituationIsController {
169160
// Remove us from owners list:
@@ -173,6 +164,16 @@ func (e *ObjectEngine) Teardown(
173164
// TODO should we check if the patch differs from actualObject before firing the request?
174165
return true, e.writer.Patch(ctx, patch, client.MergeFrom(actualObject))
175166
}
167+
} else {
168+
// No Owner to check against — fall back to revision comparison.
169+
actualRevision, err := e.getObjectRevision(actualObject)
170+
if err != nil {
171+
return false, fmt.Errorf("getting object revision: %w", err)
172+
}
173+
174+
if actualRevision != revision {
175+
return true, nil
176+
}
176177
}
177178

178179
// Actually delete the object.

0 commit comments

Comments
 (0)