[improvement] Implement owner refs for linode resources (#746)

* Update CONTROLLER_GEN path to use CACHE_BIN in Makefile

* Implement owner reference setting for Linode resources

* Add comprehensive tests for setting owner references in LinodeCluster

This commit introduces a new test function, TestSetOwnerReferenceToLinodeCluster, which validates the functionality of setting owner references for LinodeCluster resources. The tests cover various scenarios, including successful cases and error handling.

Additionally, the import statements in helpers.go have been reorganized to fix lint error

* Add Resource Ownership documentation

This commit introduces a new document on Resource Ownership in Kubernetes, detailing the use of `ownerReferences` within the Cluster API Provider Linode (CAPL). It explains the implications of ownership for resource lifecycle management, including automatic garbage collection of dependent resources when a `LinodeCluster` is deleted. Additionally, the SUMMARY.md file is updated to include a link to this new documentation.

* Refactor TestSetOwnerReferenceToLinodeCluster to initialize mock controller and client within the test function

* Enhance error handling in Linode controllers to manage cluster retrieval failures

This commit updates the Linode controllers to improve error handling when fetching clusters from metadata. If a cluster is not found, the controllers now log an informational message and continue with the deletion process instead of returning an error. This change ensures smoother resource management during deletion scenarios. Additionally, comments have been added to clarify the handling of cases where the cluster may not be found.

Author: komer3

Makefile

@@ -347,7 +347,7 @@ CTLPTL         ?= $(LOCALBIN)/ctlptl
 CLUSTERCTL     ?= $(LOCALBIN)/clusterctl
 CRD_REF_DOCS   ?= $(CACHE_BIN)/crd-ref-docs
 KUBEBUILDER    ?= $(LOCALBIN)/kubebuilder
-CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
+CONTROLLER_GEN ?= $(CACHE_BIN)/controller-gen
 CONVERSION_GEN ?= $(CACHE_BIN)/conversion-gen
 TILT           ?= $(LOCALBIN)/tilt
 KIND           ?= $(LOCALBIN)/kind

cmd/main.go

@@ -328,7 +328,6 @@ func setupControllers(mgr manager.Manager, flags flagVars, linodeClientConfig, d
 	// LinodeObjectStorageKey Controller
 	if err := (&controller.LinodeObjectStorageKeyReconciler{
 		Client:             mgr.GetClient(),
-		Scheme:             mgr.GetScheme(),
 		Logger:             ctrl.Log.WithName("LinodeObjectStorageKeyReconciler"),
 		Recorder:           mgr.GetEventRecorderFor("LinodeObjectStorageKeyReconciler"),
 		WatchFilterValue:   flags.objectStorageKeyWatchFilter,

docs/src/SUMMARY.md

@@ -31,6 +31,7 @@
     - [VPC](./topics/vpc.md)
     - [Firewalling](./topics/firewalling.md)
     - [Placement Groups](./topics/placement-groups.md)
+    - [Resource Ownership](./topics/resource-ownership.md)
     - [Cluster Object Store](./topics/cluster-object-store.md)
     - [Linode Cloud Controller Manager](./topics/linode-cloud-controller-manager.md)
 - [Development](./developers/development.md)

docs/src/topics/resource-ownership.md

@@ -0,0 +1,51 @@
+# Resource Ownership and Lifecycle
+
+In Kubernetes, `ownerReferences` are a mechanism to specify the relationship between objects, where one object (the owner) owns another object (the dependent). This is crucial for managing the lifecycle of related resources.
+
+## Owner References in Cluster API Provider Linode (CAPL)
+
+Cluster API Provider Linode (CAPL) utilizes `ownerReferences` to link various Linode-specific resources to their parent `LinodeCluster`. This means that the `LinodeCluster` acts as the owner for resources such as:
+
+*   `LinodeFirewall`
+*   `LinodeObjectStorageBucket`
+*   `LinodeObjectStorageKey`
+*   `LinodePlacementGroup`
+*   `LinodeVPC`
+
+When a `LinodeCluster` is created, and these associated resources are also created as part of the cluster definition or by controllers, CAPL automatically sets an `ownerReference` on these dependent resources, pointing back to the `LinodeCluster`.
+
+## Implications of Ownership
+
+The primary implication of this ownership model is **garbage collection**. When the `LinodeCluster` object is deleted, the Kubernetes garbage collector will automatically delete all the resources that are owned by it. This simplifies cluster teardown and helps prevent orphaned resources in your Linode account.
+
+For example, if you delete a `LinodeCluster`:
+*   Any `LinodeVPC` created for that cluster will be deleted.
+*   Any `LinodeFirewall` associated with that cluster will be deleted.
+*   Any `LinodeObjectStorageBucket` used by that cluster (and owned by it) will be deleted.
+*   And so on for other owned resources.
+
+This ensures that the lifecycle of these infrastructure components is tightly coupled with the lifecycle of the Kubernetes cluster itself, as managed by Cluster API.
+
+## Verifying Ownership
+
+You can inspect the `ownerReferences` of a resource using `kubectl describe` or `kubectl get <resource> <name> -o yaml`. Look for the `metadata.ownerReferences` field.
+
+```yaml
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+kind: LinodeVPC
+metadata:
+  name: my-cluster-vpc
+  namespace: default
+  ownerReferences:
+  - apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
+    blockOwnerDeletion: true
+    controller: true
+    kind: LinodeCluster
+    name: my-cluster
+    uid: <uid-of-linodecluster>
+# ... other fields
+```
+
+In the example above, the `LinodeVPC` named `my-cluster-vpc` is owned by the `LinodeCluster` named `my-cluster`.
+
+Understanding these ownership relationships is key to effectively managing your cluster resources with CAPL. 

internal/controller/linodefirewall_controller.go

@@ -84,15 +84,21 @@ func (r *LinodeFirewallReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	if _, ok := linodeFirewall.Labels[clusterv1.ClusterNameLabel]; ok {
 		cluster, err = kutil.GetClusterFromMetadata(ctx, r.TracedClient(), linodeFirewall.ObjectMeta)
 		if err != nil {
-			// If we're deleting and cluster isn't found, that's okay
-			if !linodeFirewall.DeletionTimestamp.IsZero() && apierrors.IsNotFound(err) {
-				log.Info("Cluster not found but LinodeFirewall is being deleted, continuing with deletion")
-			} else {
+			if client.IgnoreNotFound(err) != nil {
 				log.Error(err, "failed to fetch cluster from metadata")
 				return ctrl.Result{}, err
 			}
+			log.Info("Cluster not found but LinodeFirewall is being deleted, continuing with deletion")
+		}
+
+		// Set ownerRef to LinodeCluster
+		// It will handle the case where the cluster is not found
+		if err := util.SetOwnerReferenceToLinodeCluster(ctx, r.TracedClient(), cluster, linodeFirewall, r.Scheme()); err != nil {
+			log.Error(err, "Failed to set owner reference to LinodeCluster")
+			return ctrl.Result{}, err
 		}
 	}
+
 	// Create the firewall scope.
 	fwScope, err := scope.NewFirewallScope(
 		ctx,

internal/controller/linodeobjectstoragebucket_controller.go

@@ -88,6 +88,23 @@ func (r *LinodeObjectStorageBucketReconciler) Reconcile(ctx context.Context, req
 		return ctrl.Result{}, err
 	}
 
+	if _, ok := objectStorageBucket.Labels[clusterv1.ClusterNameLabel]; ok {
+		cluster, err := kutil.GetClusterFromMetadata(ctx, r.TracedClient(), objectStorageBucket.ObjectMeta)
+		if err != nil {
+			if client.IgnoreNotFound(err) != nil {
+				logger.Error(err, "failed to fetch cluster from metadata")
+				return ctrl.Result{}, err
+			}
+			logger.Info("Cluster not found but LinodeObjectStorageBucket is being deleted, continuing with deletion")
+		}
+
+		// It will handle the case where the cluster is not found
+		if err := util.SetOwnerReferenceToLinodeCluster(ctx, r.TracedClient(), cluster, objectStorageBucket, r.Scheme()); err != nil {
+			logger.Error(err, "Failed to set owner reference to LinodeCluster")
+			return ctrl.Result{}, err
+		}
+	}
+
 	bScope, err := scope.NewObjectStorageBucketScope(
 		ctx,
 		r.LinodeClientConfig,

internal/controller/linodeobjectstoragekey_controller.go

@@ -27,7 +27,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -58,7 +57,6 @@ type LinodeObjectStorageKeyReconciler struct {
 	Recorder           record.EventRecorder
 	LinodeClientConfig scope.ClientConfig
 	WatchFilterValue   string
-	Scheme             *runtime.Scheme
 	ReconcileTimeout   time.Duration
 }
 
@@ -95,6 +93,23 @@ func (r *LinodeObjectStorageKeyReconciler) Reconcile(ctx context.Context, req ct
 		return ctrl.Result{}, err
 	}
 
+	if _, ok := objectStorageKey.Labels[clusterv1.ClusterNameLabel]; ok {
+		cluster, err := kutil.GetClusterFromMetadata(ctx, r.TracedClient(), objectStorageKey.ObjectMeta)
+		if err != nil {
+			if client.IgnoreNotFound(err) != nil {
+				logger.Error(err, "failed to fetch cluster from metadata")
+				return ctrl.Result{}, err
+			}
+			logger.Info("Cluster not found but LinodeObjectStorageKey is being deleted, continuing with deletion")
+		}
+
+		// It will handle the case where the cluster is not found
+		if err := util.SetOwnerReferenceToLinodeCluster(ctx, r.TracedClient(), cluster, objectStorageKey, r.Scheme()); err != nil {
+			logger.Error(err, "Failed to set owner reference to LinodeCluster")
+			return ctrl.Result{}, err
+		}
+	}
+
 	keyScope, err := scope.NewObjectStorageKeyScope(
 		ctx,
 		r.LinodeClientConfig,

internal/controller/linodeplacementgroup_controller.go

@@ -89,13 +89,18 @@ func (r *LinodePlacementGroupReconciler) Reconcile(ctx context.Context, req ctrl
 	if _, ok := linodeplacementgroup.Labels[clusterv1.ClusterNameLabel]; ok {
 		cluster, err = kutil.GetClusterFromMetadata(ctx, r.TracedClient(), linodeplacementgroup.ObjectMeta)
 		if err != nil {
-			// If we're deleting and cluster isn't found, that's okay
-			if !linodeplacementgroup.DeletionTimestamp.IsZero() && apierrors.IsNotFound(err) {
-				log.Info("Cluster not found but LinodePlacementGroup is being deleted, continuing with deletion")
-			} else {
+			if client.IgnoreNotFound(err) != nil {
 				log.Error(err, "failed to fetch cluster from metadata")
 				return ctrl.Result{}, err
 			}
+			log.Info("Cluster not found but LinodePlacementGroup is being deleted, continuing with deletion")
+		}
+
+		// Set ownerRef to LinodeCluster
+		// It will handle the case where the cluster is not found
+		if err := util.SetOwnerReferenceToLinodeCluster(ctx, r.TracedClient(), cluster, linodeplacementgroup, r.Scheme()); err != nil {
+			log.Error(err, "Failed to set owner reference to LinodeCluster")
+			return ctrl.Result{}, err
 		}
 	}
 

internal/controller/linodevpc_controller.go

@@ -27,7 +27,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -58,7 +57,6 @@ type LinodeVPCReconciler struct {
 	Recorder           record.EventRecorder
 	LinodeClientConfig scope.ClientConfig
 	WatchFilterValue   string
-	Scheme             *runtime.Scheme
 	ReconcileTimeout   time.Duration
 }
 
@@ -89,23 +87,28 @@ func (r *LinodeVPCReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		if err = client.IgnoreNotFound(err); err != nil {
 			log.Error(err, "Failed to fetch LinodeVPC")
 		}
-
 		return ctrl.Result{}, err
 	}
+
 	var cluster *clusterv1.Cluster
 	var err error
 	if _, ok := linodeVPC.Labels[clusterv1.ClusterNameLabel]; ok {
 		cluster, err = kutil.GetClusterFromMetadata(ctx, r.TracedClient(), linodeVPC.ObjectMeta)
 		if err != nil {
-			// If we're deleting and cluster isn't found, that's okay
-			if !linodeVPC.DeletionTimestamp.IsZero() && apierrors.IsNotFound(err) {
-				log.Info("Cluster not found but LinodeVPC is being deleted, continuing with deletion")
-			} else {
+			if client.IgnoreNotFound(err) != nil {
 				log.Error(err, "failed to fetch cluster from metadata")
 				return ctrl.Result{}, err
 			}
+			log.Info("Cluster not found but LinodeVPC is being deleted, continuing with deletion")
+		}
+
+		// It will handle the case where the cluster is not found
+		if err := util.SetOwnerReferenceToLinodeCluster(ctx, r.TracedClient(), cluster, linodeVPC, r.Scheme()); err != nil {
+			log.Error(err, "Failed to set owner reference to LinodeCluster")
+			return ctrl.Result{}, err
 		}
 	}
+
 	vpcScope, err := scope.NewVPCScope(
 		ctx,
 		r.LinodeClientConfig,
@@ -117,7 +120,6 @@ func (r *LinodeVPCReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	)
 	if err != nil {
 		log.Error(err, "Failed to create VPC scope")
-
 		return ctrl.Result{}, fmt.Errorf("failed to create VPC scope: %w", err)
 	}
 

util/helpers.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"context"
 	"errors"
 	"io"
 	"net"
@@ -10,6 +11,14 @@ import (
 	"strings"
 
 	"github.com/linode/linodego"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
 )
 
 // Pointer returns the pointer of any type
@@ -83,3 +92,39 @@ func IsLinodePrivateIP(ipAddress string) bool {
 	// Check if the IP is contained in the Linode private network
 	return linodePrivateNet.Contains(ip)
 }
+
+// SetOwnerReferenceToLinodeCluster fetches the LinodeCluster and sets it as the owner reference of a given object.
+func SetOwnerReferenceToLinodeCluster(ctx context.Context, k8sclient client.Client, cluster *clusterv1.Cluster, obj client.Object, scheme *runtime.Scheme) error {
+	logger := log.Log.WithName("SetOwnerReferenceToLinodeCluster")
+
+	if cluster == nil || cluster.Spec.InfrastructureRef == nil {
+		logger.Info("the Cluster or InfrastructureRef is nil, cannot fetch LinodeCluster")
+		return nil
+	}
+
+	var linodeCluster infrav1alpha2.LinodeCluster
+	key := types.NamespacedName{
+		Namespace: cluster.Spec.InfrastructureRef.Namespace,
+		Name:      cluster.Spec.InfrastructureRef.Name,
+	}
+	if err := k8sclient.Get(ctx, key, &linodeCluster); err != nil {
+		if client.IgnoreNotFound(err) != nil {
+			logger.Error(err, "Failed to fetch LinodeCluster")
+			return err
+		}
+		logger.Info("LinodeCluster not found, skipping owner reference setting")
+		return nil
+	}
+
+	if err := controllerutil.SetControllerReference(&linodeCluster, obj, scheme); err != nil {
+		logger.Error(err, "Failed to set owner reference to LinodeCluster")
+		return err
+	}
+
+	if err := k8sclient.Update(ctx, obj); err != nil {
+		logger.Error(err, "Failed to update object")
+		return err
+	}
+
+	return nil
+}