Skip to content
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
31 changes: 31 additions & 0 deletions docs/component.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ be passed without a guard.
| `component.BlockOnAbsence()` | Read-only only: a NotFound records a blocked status and short-circuits the remaining resources |
| `component.IgnoreIfAbsent()` | Read-only only: a NotFound is silently ignored and last-known state is preserved |
| `component.BlockOnForeignController()` | Managed only: records a blocked status that names the owner whose controller reference is on the live object, then skips the apply and the remaining resources |
| `component.DeleteOnlyIfOwned()` | Managed only: a deletion removes the live object only when it carries an owner reference to the owner; any other object is left in place |
| `component.SuppressGraceInconsistencyWarning()` | Suppresses the grace/convergence inconsistency warning |

A read-only resource is not owned by the component, so it is never deleted. `ReadOnly()` is mutually exclusive with
Expand Down Expand Up @@ -105,6 +106,36 @@ A delete of an object that the read found safe carries the observed UID and reso
owner claims the object between the read and the delete, the delete fails and the next reconcile reads the object again.
The option requires a managed resource. A combination with `ReadOnly()` is a build error.

`DeleteOnlyIfOwned()` protects an object that the component did not create from a deletion. Without it, every deletion
path removes the object by name, whoever created it. The only exception is an object that another owner controls, which
`BlockOnForeignController()` keeps. Use the option when the resource name is one that a user can also create by hand.
For example, a component registers `<name>-app` as a ServiceAccount behind `GatedBy()`. When the gate is off and the
user created their own `<name>-app`, the option keeps the object of the user.

When `Delete()`, `DeleteWhen()`, `GatedBy()`, a disabled component feature gate, or a suspension with
`DeleteOnSuspend()` asks for a deletion, the component first reads the live object, through `ReconcileContext.APIReader`
when it is set and `ReconcileContext.Client` otherwise. The component deletes the object only when the object has an
owner reference with the UID of the owner. This reference does not have to be the controller reference. The component
sets this reference on each object that it applies, with two exceptions: an `Unowned()` resource, and a cluster-scoped
resource of a namespace-scoped owner. The option never deletes an object of the second kind, so do not use it there. If
the object does not have the reference, the component does not write to the object and records no `ResourceDeleted`
event. It logs the skip, and the reconcile does not fail. The resource does not contribute to the component condition.
As with a deletion, a disabled component feature gate reports `Disabled`. During a suspension with `DeleteOnSuspend()`,
the component also skips the suspension apply for such an object, because that apply would set the owner reference. The
resource counts as suspended. While no deletion is asked for, the option has no effect.

The delete carries the observed UID and resourceVersion as preconditions. If the object changes, or a different object
takes the name, between the read and the delete, the API server rejects the delete with a conflict. The component then
keeps the object, logs the skip, and does not fail the reconcile. The next reconcile reads the object again. A
suspension with `DeleteOnSuspend()` binds its apply to the read in the same way, through the observed resourceVersion.
If the object changes between the read and that apply, the API server rejects the apply. The component does not write
the object, the resource reports `Suspending`, and the reconcile does not fail.

The option composes with `BlockOnForeignController()`. With both options, the component keeps an object that another
owner controls, even when the object also references this owner. A conflict on the delete then does not fail the
reconcile. `DeleteOnlyIfOwned()` requires a managed resource that carries the owner reference. A combination with
`ReadOnly()` or `Unowned()` is a build error.

Options compose. Gate a resource and exclude it from health aggregation in one call:

```go
Expand Down
11 changes: 7 additions & 4 deletions docs/guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -701,10 +701,13 @@ comp, _ := component.NewComponentBuilder().
Build()
```

A disabled `GatedBy` gate deletes the resource on the next reconcile. For an optional resource the component does
**not** own (a read-only Secret reference behind an optional spec field), use `IncludeWhen`, which omits the resource
without ever deleting it. The [IncludeWhen vs. GatedBy](component.md#includewhen-vs-gatedby) section covers the
distinction.
A disabled `GatedBy` gate deletes the resource on the next reconcile. The delete is by name, so it also removes an
object that a user created under that name. If users can create an object with the same name, for example a
ServiceAccount that a user brings instead of the one the operator makes, also register the resource with
[`component.DeleteOnlyIfOwned()`](component.md#resource-registration-options). The component then deletes only an object
that carries the owner reference of the owner. For an optional resource the component does **not** own (a read-only
Secret reference behind an optional spec field), use `IncludeWhen`, which omits the resource without ever deleting it.
The [IncludeWhen vs. GatedBy](component.md#includewhen-vs-gatedby) section covers the distinction.

## Provide a User-Override Escape Hatch as the Last Mutation

Expand Down
31 changes: 27 additions & 4 deletions pkg/component/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
)

// applyConfig holds configuration for an applyResources call.
type applyConfig struct {
resourceVersion string
}

// applyOption is a functional option for applyResources.
type applyOption func(*applyConfig)

// withResourceVersion makes the apply conditional: the API server rejects it
// with a conflict when the live object no longer has resourceVersion, because it
// changed or was replaced since it was read. An empty resourceVersion applies
// unconditionally.
func withResourceVersion(resourceVersion string) applyOption {
return func(cfg *applyConfig) {
cfg.resourceVersion = resourceVersion
}
}

// applyResource applies a single resource to the cluster using Server-Side Apply and
// collects its converging status. It returns the converging result (nil for static
// resources with no status interface) and any error encountered.
Expand All @@ -30,7 +48,7 @@ import (
// status-collection sequence.
func applyResource(
ctx context.Context, rec ReconcileContext, resource Resource, componentName string,
fieldOwner client.FieldOwner, mapper meta.RESTMapper, skipOwnerRef bool,
fieldOwner client.FieldOwner, mapper meta.RESTMapper, skipOwnerRef bool, resourceVersion string,
) (*reconcileResult, error) {
obj, err := resource.Object()
if err != nil {
Expand Down Expand Up @@ -86,6 +104,7 @@ func applyResource(
// server response (including these fields) because Mutate updates the internal
// pointer and Patch writes back into the same object.
clearServerFields(obj)
obj.SetResourceVersion(resourceVersion)

// Re-assert the GVK. A Mutate implementation that assigns the whole struct
// (*current = *desired) drops the TypeMeta set above, and SSA requires it.
Expand Down Expand Up @@ -247,15 +266,19 @@ const fieldManagerMaxLength = 128
// declares.
func applyResources(
ctx context.Context, rec ReconcileContext, entries []reconcileEntry,
componentName string, mapper meta.RESTMapper,
componentName string, mapper meta.RESTMapper, opts ...applyOption,
) ([]reconcileResult, error) {
var cfg applyConfig
for _, opt := range opts {
opt(&cfg)
}
fieldOwner := applyFieldOwner(rec.Owner, componentName)

var results []reconcileResult

for _, entry := range entries {
result, err := applyResource(
ctx, rec, entry.Resource, componentName, fieldOwner, mapper, entry.Options.Unowned,
ctx, rec, entry.Resource, componentName, fieldOwner, mapper, entry.Options.Unowned, cfg.resourceVersion,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -343,7 +366,7 @@ func reconcileResources(
result, err = readResource(ctx, rec, resource)
} else {
result, err = applyResource(
ctx, rec, resource, componentName, fieldOwner, mapper, entry.Options.Unowned,
ctx, rec, resource, componentName, fieldOwner, mapper, entry.Options.Unowned, "",
)
}
if err != nil {
Expand Down
62 changes: 41 additions & 21 deletions pkg/component/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,14 @@ func withDeletionReason(reason string) deleteOption {
// live object is controlled by another owner: the object is not this
// component's to delete, whichever path (a deletion flag, a disabled feature
// gate, suspension) asked for it. The skip is logged with the controlling owner.
// An entry registered with DeleteOnlyIfOwned is left in place, and logged,
// when the live object does not carry an owner reference to the owner.
//
// For each successful deletion, a "ResourceDeleted" event is recorded on the owner object.
// The opts parameter allows callers to customize the event message via functional options.
//
// Returns a joined error containing all encountered errors, or nil if all deletions
// were successful or resulted in "Not Found" errors.
// Returns the errors of all entries joined, or nil when no entry failed; an
// entry left in place is not an error.
func deleteResources(
ctx context.Context, rec ReconcileContext, entries []reconcileEntry, opts ...deleteOption,
) error {
Expand Down Expand Up @@ -92,22 +94,19 @@ func deleteResources(
// reports whether a deletion happened. An object that is already gone is not
// an error and reports false.
//
// For an entry registered with BlockOnForeignController the delete is bound to
// what was observed: the live object is read first (through the API reader
// when set), an absent object counts as deleted, an object another owner
// controls is left in place and logged, and an object observed as safe is
// deleted with its UID and resourceVersion as preconditions. An owner that
// claims the object between the read and the delete therefore makes the delete
// conflict instead of removing that owner's object; the error is returned so
// the next reconcile observes the object again. Unlike a lost apply race, a
// lost delete race is not repaired by the next reconcile, which is why the
// delete carries the precondition and the apply does not.
// For an entry registered with BlockOnForeignController or DeleteOnlyIfOwned
// the delete is bound to what was observed: the live object is read first
// (through the API reader when set), an absent object counts as deleted, an
// object another owner controls (BlockOnForeignController) or that does not
// reference the owner (DeleteOnlyIfOwned) is left in place and logged, and any
// other object is deleted with its UID and resourceVersion as preconditions.
func deleteEntry(
ctx context.Context, rec ReconcileContext, entry reconcileEntry, object client.Object,
) (bool, error) {
resource := entry.Resource
options := entry.Options

if !entry.Options.BlockOnForeignController {
if !options.BlockOnForeignController && !options.DeleteOnlyIfOwned {
if err := rec.Client.Delete(ctx, object); err != nil {
if apierrors.IsNotFound(err) {
return false, nil
Expand All @@ -124,23 +123,44 @@ func deleteEntry(
if live == nil {
return false, nil
}
if controller != nil {

if options.BlockOnForeignController && controller != nil {
log.FromContext(ctx).Info(
"skipping deletion of a resource another owner controls",
"resource", resource.Identity(), "controller", controller.Kind+" "+controller.Name,
)
return false, nil
}

if options.DeleteOnlyIfOwned && !ownedBy(live, rec.Owner.GetUID()) {
log.FromContext(ctx).Info(
"skipping deletion of a resource that does not reference the owner", "resource", resource.Identity(),
)
return false, nil
}

// An owner that claims the object, or a user that replaces it, between the
// read and the delete makes the delete conflict instead of removing an
// object the checks above did not see. Unlike a lost apply race, a lost
// delete race is not repaired by the next reconcile, which is why the
// delete carries the precondition and the apply does not.
uid, resourceVersion := live.GetUID(), live.GetResourceVersion()
err = rec.Client.Delete(ctx, live, client.Preconditions{UID: &uid, ResourceVersion: &resourceVersion})
if err != nil {
if apierrors.IsNotFound(err) {
return false, nil
}
return false, fmt.Errorf(
"failed to delete resource %s as observed: %w", resource.Identity(), err,
switch {
case err == nil:
return true, nil
case apierrors.IsNotFound(err):
return false, nil
case options.DeleteOnlyIfOwned && apierrors.IsConflict(err):
// The object changed after it was read. Leaving it is the safe outcome
// and needs no retry: the next reconcile reads it again.
log.FromContext(ctx).Info(
"skipping deletion of a resource that changed after it was read", "resource", resource.Identity(),
)
return false, nil
default:
// BlockOnForeignController alone returns the conflict so the reconcile
// fails and the object is observed again on the retry.
return false, fmt.Errorf("failed to delete resource %s as observed: %w", resource.Identity(), err)
}
return true, nil
}
Loading
Loading