From 6ade9ee4d8d3b8a8a29d405e283f2394a839c521 Mon Sep 17 00:00:00 2001 From: Bastian Krol Date: Tue, 6 Aug 2024 12:32:40 +0200 Subject: [PATCH] chore(controller): remove common controller This reverts some left-overs from commit abstraction layer for handling custom resources; and it is a follow up to #ad09e199cb1c6f2666808030de4cf3bd16439b31 which removed the second custom resource (BackendConnection) again. --- .../v1alpha1/dash0monitoring_types.go | 40 ----- .../controller/controller_util.go} | 143 ++++++++---------- internal/dash0/controller/dash0_controller.go | 14 +- 3 files changed, 64 insertions(+), 133 deletions(-) rename internal/{common/controller/controller_common.go => dash0/controller/controller_util.go} (62%) diff --git a/api/dash0monitoring/v1alpha1/dash0monitoring_types.go b/api/dash0monitoring/v1alpha1/dash0monitoring_types.go index 8dfebf4e..1ae9001f 100644 --- a/api/dash0monitoring/v1alpha1/dash0monitoring_types.go +++ b/api/dash0monitoring/v1alpha1/dash0monitoring_types.go @@ -8,10 +8,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/dash0hq/dash0-operator/internal/common/controller" ) const ( @@ -243,42 +239,6 @@ func (d *Dash0Monitoring) EnsureResourceIsMarkedAsDegraded( }) } -func (d *Dash0Monitoring) GetResourceTypeName() string { - return "Dash0MonitoringResource" -} -func (d *Dash0Monitoring) GetNaturalLanguageResourceTypeName() string { - return "Dash0 monitoring resource" -} -func (d *Dash0Monitoring) Get() client.Object { - return d -} -func (d *Dash0Monitoring) GetName() string { - return d.Name -} -func (d *Dash0Monitoring) GetUid() types.UID { - return d.UID -} -func (d *Dash0Monitoring) GetCreationTimestamp() metav1.Time { - return d.CreationTimestamp -} -func (d *Dash0Monitoring) GetReceiver() client.Object { - return &Dash0Monitoring{} -} -func (d *Dash0Monitoring) GetListReceiver() client.ObjectList { - return &Dash0MonitoringList{} -} -func (d *Dash0Monitoring) Items(list client.ObjectList) []client.Object { - items := list.(*Dash0MonitoringList).Items - result := make([]client.Object, len(items)) - for i := range items { - result[i] = &items[i] - } - return result -} -func (d *Dash0Monitoring) At(list client.ObjectList, index int) controller.CustomResource { - return &list.(*Dash0MonitoringList).Items[index] -} - //+kubebuilder:object:root=true // Dash0MonitoringList contains a list of Dash0Monitoring resources. diff --git a/internal/common/controller/controller_common.go b/internal/dash0/controller/controller_util.go similarity index 62% rename from internal/common/controller/controller_common.go rename to internal/dash0/controller/controller_util.go index 2c7172ce..a6ad2f72 100644 --- a/internal/common/controller/controller_common.go +++ b/internal/dash0/controller/controller_util.go @@ -12,32 +12,17 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" -) -type CustomResource interface { - GetResourceTypeName() string - GetNaturalLanguageResourceTypeName() string - Get() client.Object - GetName() string - GetUid() types.UID - GetCreationTimestamp() metav1.Time - GetReceiver() client.Object - GetListReceiver() client.ObjectList - Items(client.ObjectList) []client.Object - At(client.ObjectList, int) CustomResource - SetAvailableConditionToUnknown() - EnsureResourceIsMarkedAsDegraded(string, string) - IsMarkedForDeletion() bool -} + dash0v1alpha1 "github.com/dash0hq/dash0-operator/api/dash0monitoring/v1alpha1" +) -// CheckIfNamespaceExists checks if the given namespace (which is supposed to be the namespace from a reconcile request) +// checkIfNamespaceExists checks if the given namespace (which is supposed to be the namespace from a reconcile request) // exists in the cluster. If the namespace does not exist, it returns false, and this is supposed to stop the reconcile -func CheckIfNamespaceExists( +func checkIfNamespaceExists( ctx context.Context, clientset *kubernetes.Clientset, namespace string, @@ -55,12 +40,12 @@ func CheckIfNamespaceExists( return true, nil } -// VerifyUniqueDash0MonitoringResourceExists loads the resource that the current reconcile request applies to, if it exists. It -// also checks whether there is only one such resource (or, if there are multiple, if the currently reconciled one is -// the most recently created one). The bool returned has the meaning "stop the reconcile request", that is, if the -// function returns true, it expects the caller to stop the reconcile request immediately and not requeue it. If an -// error ocurrs during any of the checks (for example while talking to the Kubernetes API server), the function will -// return that error, the caller should then ignore the bool result and requeue the reconcile request. +// verifyUniqueDash0MonitoringResourceExists loads the resource that the current reconcile request applies to, if it +// exists. It also checks whether there is only one such resource (or, if there are multiple, if the currently +// reconciled one is the most recently created one). The bool returned has the meaning "stop the reconcile request", +// that is, if the function returns true, it expects the caller to stop the reconcile request immediately and not +// requeue it. If an error ocurrs during any of the checks (for example while talking to the Kubernetes API server), the +// function will return that error, the caller should then ignore the bool result and requeue the reconcile request. // // - If the resource does not exist, the function logs a message and returns (nil, true, nil) and expects the caller // to stop the reconciliation (without requeing it). @@ -71,20 +56,18 @@ func CheckIfNamespaceExists( // stopReconcile and the caller is expected to stop the reconcile and not requeue it. // - If any error is encountered when searching for resources etc., that error will be returned, the caller is // expected to ignore the bool result and requeue the reconcile request. -func VerifyUniqueDash0MonitoringResourceExists( +func verifyUniqueDash0MonitoringResourceExists( ctx context.Context, k8sClient client.Client, statusWriter client.SubResourceWriter, - customResourceReceiver CustomResource, updateStatusFailedMessage string, req ctrl.Request, logger logr.Logger, -) (CustomResource, bool, error) { - customResource, stopReconcile, err := verifyThatCustomResourceExists( +) (*dash0v1alpha1.Dash0Monitoring, bool, error) { + dash0MonitoringResource, stopReconcile, err := verifyThatCustomResourceExists( ctx, k8sClient, req, - customResourceReceiver, &logger, ) if err != nil || stopReconcile { @@ -96,11 +79,11 @@ func VerifyUniqueDash0MonitoringResourceExists( k8sClient, statusWriter, req, - customResource, + dash0MonitoringResource, updateStatusFailedMessage, &logger, ) - return customResource, stopReconcile, err + return dash0MonitoringResource, stopReconcile, err } // verifyThatCustomResourceExists loads the resource that the current reconcile request applies to. If that @@ -111,17 +94,15 @@ func verifyThatCustomResourceExists( ctx context.Context, k8sClient client.Client, req ctrl.Request, - customResourceReceiver CustomResource, logger *logr.Logger, -) (CustomResource, bool, error) { - resource := customResourceReceiver.GetReceiver() +) (*dash0v1alpha1.Dash0Monitoring, bool, error) { + resource := &dash0v1alpha1.Dash0Monitoring{} err := k8sClient.Get(ctx, req.NamespacedName, resource) if err != nil { if apierrors.IsNotFound(err) { logger.Info( - fmt.Sprintf( - "The %s has not been found, either it hasn't been installed or it has been deleted. Ignoring the reconcile request.", - customResourceReceiver.GetNaturalLanguageResourceTypeName())) + "The Dash0 monitoring resource has not been found, either it hasn't been installed or it has " + + "been deleted. Ignoring the reconcile request.") // stop the reconciliation, and do not requeue it (that is, return (ctrl.Result{}, nil)) return nil, true, nil } @@ -129,7 +110,7 @@ func verifyThatCustomResourceExists( // requeue the reconciliation (that is, return (ctrl.Result{}, err)) return nil, true, err } - return resource.(CustomResource), false, nil + return resource, false, nil } // verifyThatCustomResourceIsUniqe checks whether there are any additional resources of the same type in the namespace, @@ -146,11 +127,11 @@ func verifyThatCustomResourceIsUniqe( k8sClient client.Client, statusWriter client.SubResourceWriter, req ctrl.Request, - customResource CustomResource, + dash0MonitoringResource *dash0v1alpha1.Dash0Monitoring, updateStatusFailedMessage string, logger *logr.Logger, ) (bool, error) { - allCustomResourcesInNamespace := customResource.GetListReceiver() + allCustomResourcesInNamespace := &dash0v1alpha1.Dash0MonitoringList{} if err := k8sClient.List( ctx, allCustomResourcesInNamespace, @@ -160,14 +141,12 @@ func verifyThatCustomResourceIsUniqe( ); err != nil { logger.Error( err, - fmt.Sprintf( - "Failed to list all %ss, requeuing reconcile request.", - customResource.GetNaturalLanguageResourceTypeName(), - )) + "Failed to list all Dash0 monitoring resources, requeuing reconcile request.", + ) return true, err } - items := customResource.Items(allCustomResourcesInNamespace) + items := allCustomResourcesInNamespace.Items if len(items) > 1 { // There are multiple instances of the Dash0 monitoring resource in this namespace. If the resource that is // currently being reconciled is the one that has been most recently created, we assume that this is the source @@ -175,29 +154,28 @@ func verifyThatCustomResourceIsUniqe( // (they will be handled when they are being reconciled). If the currently reconciled resource is not the most // recent one, we set its status to degraded. sort.Sort(SortByCreationTimestamp(items)) - mostRecentResource := customResource.At(allCustomResourcesInNamespace, len(items)-1) - if mostRecentResource.GetUid() == customResource.GetUid() { - logger.Info(fmt.Sprintf( - "At least one other %[1]s exists in this namespace. This %[1]s resource is the most recent one. The state of the other resource(s) will be set to degraded.", - customResource.GetNaturalLanguageResourceTypeName(), - )) + mostRecentResource := items[len(items)-1] + if mostRecentResource.UID == dash0MonitoringResource.UID { + logger.Info( + "At least one other Dash0 monitoring resource exists in this namespace. This Dash0 monitoring " + + "resource is the most recent one. The state of the other resource(s) will be set to degraded.", + ) // continue with the reconcile request for this resource, let the reconcile requests for the other offending // resources handle the situation for those resources return false, nil } else { logger.Info( - fmt.Sprintf( - "At least one other %[1]s exists in this namespace, and at least one other %[1]s has been created more recently than this one. Setting the state of this resource to degraded.", - customResource.GetNaturalLanguageResourceTypeName(), - ), - fmt.Sprintf("most recently created %s", customResource.GetNaturalLanguageResourceTypeName()), - fmt.Sprintf("%s (%s)", mostRecentResource.GetName(), mostRecentResource.GetUid()), + "At least one other Dash0 monitoring resource exists in this namespace, and at least one other "+ + "Dash0 monitoring resource has been created more recently than this one. Setting the state of "+ + "this resource to degraded.", + "most recently created Dash0 monitoring resource", + fmt.Sprintf("%s (%s)", mostRecentResource.Name, mostRecentResource.UID), ) - customResource.EnsureResourceIsMarkedAsDegraded( + dash0MonitoringResource.EnsureResourceIsMarkedAsDegraded( "NewerResourceIsPresent", - fmt.Sprintf("There is a more recently created %s in this namespace, please remove all but one resource instance.", customResource.GetNaturalLanguageResourceTypeName()), + "There is a more recently created Dash0 monitoring resource in this namespace, please remove all but one resource instance.", ) - if err := statusWriter.Update(ctx, customResource.Get()); err != nil { + if err := statusWriter.Update(ctx, dash0MonitoringResource); err != nil { logger.Error(err, updateStatusFailedMessage) return true, err } @@ -208,7 +186,7 @@ func verifyThatCustomResourceIsUniqe( return false, nil } -type SortByCreationTimestamp []client.Object +type SortByCreationTimestamp []dash0v1alpha1.Dash0Monitoring func (s SortByCreationTimestamp) Len() int { return len(s) @@ -217,15 +195,15 @@ func (s SortByCreationTimestamp) Swap(i, j int) { s[i], s[j] = s[j], s[i] } func (s SortByCreationTimestamp) Less(i, j int) bool { - tsi := s[i].GetCreationTimestamp() - tsj := s[j].GetCreationTimestamp() + tsi := s[i].CreationTimestamp + tsj := s[j].CreationTimestamp return tsi.Before(&tsj) } -func InitStatusConditions( +func initStatusConditions( ctx context.Context, statusWriter client.SubResourceWriter, - customResource CustomResource, + dash0MonitoringResource *dash0v1alpha1.Dash0Monitoring, conditions []metav1.Condition, conditionTypeAvailable string, logger *logr.Logger, @@ -233,7 +211,7 @@ func InitStatusConditions( firstReconcile := false needsRefresh := false if len(conditions) == 0 { - customResource.SetAvailableConditionToUnknown() + dash0MonitoringResource.SetAvailableConditionToUnknown() firstReconcile = true needsRefresh = true } else if availableCondition := @@ -241,11 +219,11 @@ func InitStatusConditions( conditions, conditionTypeAvailable, ); availableCondition == nil { - customResource.SetAvailableConditionToUnknown() + dash0MonitoringResource.SetAvailableConditionToUnknown() needsRefresh = true } if needsRefresh { - err := updateResourceStatus(ctx, statusWriter, customResource, logger) + err := updateResourceStatus(ctx, statusWriter, dash0MonitoringResource, logger) if err != nil { // The error has already been logged in refreshStatus return firstReconcile, err @@ -257,44 +235,41 @@ func InitStatusConditions( func updateResourceStatus( ctx context.Context, statusWriter client.SubResourceWriter, - customResource CustomResource, + dash0MonitoringResource *dash0v1alpha1.Dash0Monitoring, logger *logr.Logger, ) error { - if err := statusWriter.Update(ctx, customResource.Get()); err != nil { - logger.Error(err, fmt.Sprintf("Cannot update the status of the %s.", customResource.GetNaturalLanguageResourceTypeName())) + if err := statusWriter.Update(ctx, dash0MonitoringResource); err != nil { + logger.Error(err, "Cannot update the status of the Dash0 monitoring resource.") return err } return nil } -func CheckImminentDeletionAndHandleFinalizers( +func checkImminentDeletionAndHandleFinalizers( ctx context.Context, k8sClient client.Client, - customResource CustomResource, + dash0MonitoringResource *dash0v1alpha1.Dash0Monitoring, finalizerId string, logger *logr.Logger, ) (bool, bool, error) { - isMarkedForDeletion := customResource.IsMarkedForDeletion() + isMarkedForDeletion := dash0MonitoringResource.IsMarkedForDeletion() if !isMarkedForDeletion { err := addFinalizerIfNecessary( ctx, k8sClient, - customResource, + dash0MonitoringResource, finalizerId, ) if err != nil { logger.Error( err, - fmt.Sprintf( - "Failed to add finalizer to %s, requeuing reconcile request.", - customResource.GetNaturalLanguageResourceTypeName(), - ), + "Failed to add finalizer to Dash0 monitoring resource, requeuing reconcile request.", ) return isMarkedForDeletion, false, err } return isMarkedForDeletion, false, nil } else { - if controllerutil.ContainsFinalizer(customResource.Get(), finalizerId) { + if controllerutil.ContainsFinalizer(dash0MonitoringResource, finalizerId) { return isMarkedForDeletion, true, nil } return isMarkedForDeletion, false, nil @@ -304,12 +279,12 @@ func CheckImminentDeletionAndHandleFinalizers( func addFinalizerIfNecessary( ctx context.Context, k8sClient client.Client, - customResource CustomResource, + dash0MonitoringResource *dash0v1alpha1.Dash0Monitoring, finalizerId string, ) error { - finalizerHasBeenAdded := controllerutil.AddFinalizer(customResource.Get(), finalizerId) + finalizerHasBeenAdded := controllerutil.AddFinalizer(dash0MonitoringResource, finalizerId) if finalizerHasBeenAdded { - return k8sClient.Update(ctx, customResource.Get()) + return k8sClient.Update(ctx, dash0MonitoringResource) } // The resource already had the finalizer, no update necessary. return nil diff --git a/internal/dash0/controller/dash0_controller.go b/internal/dash0/controller/dash0_controller.go index 17141432..10b863b8 100644 --- a/internal/dash0/controller/dash0_controller.go +++ b/internal/dash0/controller/dash0_controller.go @@ -26,7 +26,6 @@ import ( dash0v1alpha1 "github.com/dash0hq/dash0-operator/api/dash0monitoring/v1alpha1" "github.com/dash0hq/dash0-operator/internal/backendconnection" - "github.com/dash0hq/dash0-operator/internal/common/controller" "github.com/dash0hq/dash0-operator/internal/dash0/util" "github.com/dash0hq/dash0-operator/internal/dash0/workloads" ) @@ -144,11 +143,10 @@ func (r *Dash0Reconciler) InstrumentAtStartup() { Name: dash0MonitoringResource.Name, }, } - _, stop, err := controller.VerifyUniqueDash0MonitoringResourceExists( + _, stop, err := verifyUniqueDash0MonitoringResourceExists( ctx, r.Client, r.Status(), - &dash0v1alpha1.Dash0Monitoring{}, updateStatusFailedMessage, pseudoReconcileRequest, logger, @@ -200,7 +198,7 @@ func (r *Dash0Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl logger := log.FromContext(ctx) logger.Info("processing reconcile request for Dash0 monitoring resource") - namespaceStillExists, err := controller.CheckIfNamespaceExists(ctx, r.Clientset, req.Namespace, &logger) + namespaceStillExists, err := checkIfNamespaceExists(ctx, r.Clientset, req.Namespace, &logger) if err != nil { // The error has already been logged in checkIfNamespaceExists. return ctrl.Result{}, err @@ -210,11 +208,10 @@ func (r *Dash0Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, nil } - monitoringResource, stopReconcile, err := controller.VerifyUniqueDash0MonitoringResourceExists( + dash0MonitoringResource, stopReconcile, err := verifyUniqueDash0MonitoringResourceExists( ctx, r.Client, r.Status(), - &dash0v1alpha1.Dash0Monitoring{}, updateStatusFailedMessage, req, logger, @@ -224,9 +221,8 @@ func (r *Dash0Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } else if stopReconcile { return ctrl.Result{}, nil } - dash0MonitoringResource := monitoringResource.(*dash0v1alpha1.Dash0Monitoring) - isFirstReconcile, err := controller.InitStatusConditions( + isFirstReconcile, err := initStatusConditions( ctx, r.Status(), dash0MonitoringResource, @@ -239,7 +235,7 @@ func (r *Dash0Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, err } - isMarkedForDeletion, runCleanupActions, err := controller.CheckImminentDeletionAndHandleFinalizers( + isMarkedForDeletion, runCleanupActions, err := checkImminentDeletionAndHandleFinalizers( ctx, r.Client, dash0MonitoringResource,