samba-in-kubernetes / samba-operator

An operator for a Samba as a service on PVCs in kubernetes
Apache License 2.0
111 stars 24 forks source link

need to check if calls should use SetOwnerReference #105

Open phlogistonjohn opened 2 years ago

phlogistonjohn commented 2 years ago

Currently we use SetControllerReference all over the place, but this may not be the best choice.

anoopcs9 commented 2 years ago

Don't we need to add Owns(&corev1.ConfigMap{}), Owns(&corev1.Service{}) and Owns(&appsv1.StatefulSet) within SetupWithManager() as they are ControllerManagedBy generated objects? Then it completely makes sense to use SetControllerReference by all means.

Some excerpts from related docs:

SetControllerReference sets owner as a Controller OwnerReference on controlled. This is used for garbage collection of the controlled object and for reconciling the owner object on changes to controlled (with a Watch + EnqueueRequestForOwner). Since only one OwnerReference can be a controller, it returns an error if there is another OwnerReference with Controller flag set.



Note: **SetOwnerReference** doesn't have the notion of _controller_
synarete commented 2 years ago

And if needed, what kind of predicate should be used? (e.g., ocs-operator uses different predicates for its owned resources: https://github.com/red-hat-storage/ocs-operator/blob/main/controllers/storagecluster/storagecluster_controller.go#L124-L126)

phlogistonjohn commented 2 years ago

Let's start off with some reference links related to owner references:

https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/controller/controllerutil

https://github.com/kubernetes-sigs/controller-runtime/blob/v0.12.3/pkg/controller/controllerutil/controllerutil.go#L96 https://github.com/kubernetes-sigs/controller-runtime/blob/v0.12.3/pkg/controller/controllerutil/controllerutil.go#L59

https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#OwnerReference

https://stackoverflow.com/questions/51068026/when-exactly-do-i-set-an-ownerreferences-controller-field-to-true

https://github.com/kubernetes/design-proposals-archive/blob/main/api-machinery/controller-ref.md

And here's the owns function: https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.3/pkg/builder#Builder.Owns


The way I understand it is that the k8s API has a mechanism for tracking resource "ownership". This is part of the resource metadata and helps decide two things: (1) when should k8s garbage collect resources and (2) disambiguate what resource is supposed to have control over another.

Each resource can have a list (slice in the Go code) of owner references. The owner references are automatically removed from a resource when the "owning" resource is removed. If the last owner is being removed the resource can be garbage collected. In the owner references exactly 1 of the references can be the "controller" (not to be confused the the code executing a "controller"). If there's some sort of resource conflict the "contoller owner" gets final say.

If resource-A triggers resource-B to be created it is reasonable to create resource-B with resource-A as the controller-owner. If you inspect the metadata of resource-A you won't see any (direct) refrences to resource-B. Only resource-B will record the resources that own it.

One additional wrinkle is the controllers (the code) often want to trigger a reconcile loop when one of the owned objects is changed, not just when the owner changes. To have the controller-runtime framework watch these objects the Owns method can be used in conjunction with setting the objects as owned. The Owns method basically tells the controller to set a special watch for these "child" resources. The doc even says: "This is the equivalent of calling Watches(...). " [elision mine]. The options types that can follow Owns or Watches can act as filters so that certain events don't trigger the reconcile function. IIUC, you can define a predicate function so that only changes to an object's spec section trigger a reconcile but, for example, it may ignore a change to annotations.

A resource without a controller-owner can (should?) be "adopted" by another owner.

(Personally, I think the overuse of the words own & controller here are extremely confusing. IMO, in particular the Owns method could have been called something like WatchOwnedResource to be much clearer about what it does. )

phlogistonjohn commented 2 years ago

Slippery touchpads suck.