Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Finalizer helper library #1453

Closed
rashmigottipati opened this issue Mar 25, 2021 · 7 comments · Fixed by #1481
Closed

[Feature Request] Finalizer helper library #1453

rashmigottipati opened this issue Mar 25, 2021 · 7 comments · Fixed by #1481
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@rashmigottipati
Copy link
Contributor

rashmigottipati commented Mar 25, 2021

In Kubernetes garbage collection model, OwnerReferences are allowed within a namespace wherein the "dependent" object has anOwnerReference field referencing the "parent" object. If the parent object is being deleted, Kubernetes garbage collector will delete all of its dependent objects. As cross-namespace owner references are not allowed by design; a Finalizer should be used to perform cleanup.

Finalizers are more powerful and broadly applicable than owner references because they permit customizable deletion steps for a controller to define how an object is cleaned up, ex. updating a CR's status during deletion with a particular message. This gives Operator authors control over CR deletion proceeds.

Operator projects would benefit from a finalizer helper library in controller-runtime which will make it easy for users to define their own finalizer struct implementations and use the abstractions we include to register the Finalizers and hand the Finalizers to their Reconciler and call Finalize function.

Please see @joelanford's high level thought on the implementation details: kubernetes-sigs/kubebuilder#2010 (comment).

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 25, 2021
@rashmigottipati
Copy link
Contributor Author

/assign

@Adirio
Copy link
Contributor

Adirio commented Mar 25, 2021

Pinging @joelanford as he suggested this approach.

@coderanger
Copy link
Contributor

https://github.com/coderanger/controller-utils/blob/main/core/reconciler.go#L277-L290 is my implementation of the general helper pattern for this. It checks on initialization (up above) for if the component implements the finalizer interface and if so, it will call Finalizer() on that component when needed.

@joelanford
Copy link
Member

joelanford commented Mar 25, 2021

My high level thought was to have something like this:

package finalizer

type Registerer interface {
	Register(key string, f Finalizer) error
}

type Finalizer interface {
	Finalize(context.Context, client.Object) (needsUpdate bool, err error)
}

type Finalizers interface {
	// implements Registerer and Finalizer to finalize
	// all registered finalizers if the provided object
	// has a deletion timestamp or set all registered
	// finalizers if it doesn't
	Registerer
	Finalizer
}

func NewFinalizers() Finalizers {
	return finalizers(map[string]Finalizer{})
}

type finalizers map[string]Finalizer

func (f finalizers) Register(key string, finalizer Finalizer) error {
	if _, ok := f[key]; ok {
		return fmt.Errorf("finalizer for key %q already registered")
	}
	f[key] = finalizer
	return nil
}

func (f finalizers) Finalize(ctx context.Context, obj client.Object) (bool, error) {
	needsUpdate := false
	for key, finalizer := range f {
		if obj.GetDeletionTimestamp() == nil && !controllerutil.ContainsFinalizer(obj, key) {
			controllerutil.AddFinalizer(obj, key)
			needsUpdate = true
		} else if obj.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(obj, key) {
			_, err := finalizer.Finalize(ctx, obj)
			if err != nil {
				return true, fmt.Errorf("finalize failed for key %q: %v", key, err)
			}
			controllerutil.RemoveFinalizer(obj, key)
			needsUpdate = true
		}
	}
	return needsUpdate, nil
}

With something like this users could:

  1. Define their own finalizer struct implementations

    const myFinalizerKey = "my-finalizer"
    type MyObjectFinalizer struct {
    	client.Client
    }
    
    func (f MyObjectFinalizer) Finalize(ctx context.Context, obj client.Object) (bool, error) {
        // use the client to delete some other stuff in the cluster, perhaps.
    }
  2. Register them with the Finalizers

    func main() {
    	...
    	myFinalziers := finalizer.NewFinalizers()
    	myFinalizers.Register(myFinalizerKey, &MyObjectFinalizer{mgr.GetClient()})
    	...
    }
  3. Hand the Finalizers to their reconciler:

    type MyReconciler struct {
        client.Client
        Log    logr.Logger
        Scheme *runtime.Scheme
        Finalizers finalizer.Finalizer
    }
    
    func main() {
        ...
        if err = (&controllers.MyReconciler{
      	  Client: mgr.GetClient(),
      	  Log: ctrl.Log.WithName("controllers").WithName("My")
      	  Scheme: mgr.GetScheme(),
      	  Finalizers: myFinalziers,
        }
        ...
    }
  4. And then the reconciler call is boiled down to something like this:

    func Reconcile(req ctrl.Request) (ctrl.Result, error) {
        obj := groupv1.MyObject
        _ = r.Client.Get(ctx, req, &obj)
        requeue, err := r.finalizers.Finalize(ctx, obj)
        if requeue || err != nil {
      	  r.Client.Update(ctx, &obj)
      	  return ctrl.Result{Requeue: requeue}, err
        }
    }

It looks like @coderanger has an actual working implementation of what the Finalizer function signature needs to look like to properly signal to the reconciler what it should do when the finalizer has completed. So take specifics of mine with a grain of salt.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 30, 2021
@rashmigottipati
Copy link
Contributor Author

Implemented and merged the finalizer helper library in #1481.
Closing this issue.

@austince
Copy link

austince commented Aug 12, 2021

Are there any examples that use this? I'm giving it a try but don't understand where the business logic of the finalizer should live. It feels like the Reconciler knows what should be deleted, not main. Should I define the finalizer logic inside the x_controller.go file, and just register that in the main?

And then, does it make sense to requeue if either the obj or its status was updated?

requeue, err := r.Finalizers.Finalize(ctx, &job)
if requeue.Updated || requeue.StatusUpdated || err != nil {
  if requeue.Updated {
	r.Client.Update(ctx, &job)
  }

  if requeue.StatusUpdated {
	r.Client.Status().Update(ctx, &job)
  }
  return ctrl.Result{Requeue: requeue.Updated || requeue.StatusUpdated}, err
}

Update: created a new issue instead of continuing to clutter this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants