From ea14c1a7b8d2bcedaefa567f70c1481d7cf36642 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Wed, 4 Sep 2024 16:00:41 -0700 Subject: [PATCH 01/12] Refactor Stargate and Reaper cleanup --- controllers/k8ssandra/cleanup.go | 134 +----------------------------- controllers/k8ssandra/reaper.go | 43 ++-------- controllers/k8ssandra/stargate.go | 11 ++- 3 files changed, 19 insertions(+), 169 deletions(-) diff --git a/controllers/k8ssandra/cleanup.go b/controllers/k8ssandra/cleanup.go index b3acbe102..6d370f02d 100644 --- a/controllers/k8ssandra/cleanup.go +++ b/controllers/k8ssandra/cleanup.go @@ -8,8 +8,6 @@ import ( cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" - reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1" - stargateapi "github.com/k8ssandra/k8ssandra-operator/apis/stargate/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/annotations" "github.com/k8ssandra/k8ssandra-operator/pkg/k8ssandra" k8ssandralabels "github.com/k8ssandra/k8ssandra-operator/pkg/labels" @@ -39,7 +37,6 @@ func (r *K8ssandraClusterReconciler) checkDeletion(ctx context.Context, kc *api. logger.Info("Starting deletion") - kcKey := utils.GetKey(kc) hasErrors := false for _, dcTemplate := range kc.Spec.Cassandra.Datacenters { @@ -67,35 +64,6 @@ func (r *K8ssandraClusterReconciler) checkDeletion(ctx context.Context, kc *api. hasErrors = true } - selector := k8ssandralabels.CleanedUpByLabels(kcKey) - stargateList := &stargateapi.StargateList{} - options := client.ListOptions{ - Namespace: namespace, - LabelSelector: labels.SelectorFromSet(selector), - } - - err = remoteClient.List(ctx, stargateList, &options) - if err != nil { - logger.Error(err, "Failed to list Stargate objects", "Context", dcTemplate.K8sContext) - hasErrors = true - continue - } - - for _, sg := range stargateList.Items { - if err = remoteClient.Delete(ctx, &sg); err != nil { - key := client.ObjectKey{Namespace: namespace, Name: sg.Name} - if !errors.IsNotFound(err) { - logger.Error(err, "Failed to delete Stargate", "Stargate", key, - "Context", dcTemplate.K8sContext) - hasErrors = true - } - } - } - - if r.deleteReapers(ctx, kc, dcTemplate, namespace, remoteClient, logger) { - hasErrors = true - } - if r.deleteDeployments(ctx, kc, dcTemplate, namespace, remoteClient, logger) { hasErrors = true } @@ -174,29 +142,7 @@ func (r *K8ssandraClusterReconciler) checkDcDeletion(ctx context.Context, kc *ap func (r *K8ssandraClusterReconciler) deleteDc(ctx context.Context, kc *api.K8ssandraCluster, dcName string, cassDcName string, logger logr.Logger) result.ReconcileResult { kcKey := utils.GetKey(kc) - stargate, remoteClient, err := r.findStargateForDeletion(ctx, kcKey, cassDcName, nil) - if err != nil { - return result.Error(err) - } - - if stargate != nil { - if err = remoteClient.Delete(ctx, stargate); err != nil && !errors.IsNotFound(err) { - return result.Error(fmt.Errorf("failed to delete Stargate for dc (%s): %v", cassDcName, err)) - } - logger.Info("Deleted Stargate", "Stargate", utils.GetKey(stargate)) - } - - reaper, remoteClient, err := r.findReaperForDeletion(ctx, kcKey, cassDcName, remoteClient) - if err != nil { - return result.Error(err) - } - - if reaper != nil { - if err = remoteClient.Delete(ctx, reaper); err != nil && !errors.IsNotFound(err) { - return result.Error(fmt.Errorf("failed to delete Reaper for dc (%s): %v", cassDcName, err)) - } - logger.Info("Deleted Reaper", "Reaper", utils.GetKey(reaper)) - } + var remoteClient client.Client dc, remoteClient, err := r.findDcForDeletion(ctx, kcKey, dcName, remoteClient) if err != nil { @@ -235,84 +181,6 @@ func (r *K8ssandraClusterReconciler) deleteDc(ctx context.Context, kc *api.K8ssa return result.Continue() } -func (r *K8ssandraClusterReconciler) findStargateForDeletion( - ctx context.Context, - kcKey client.ObjectKey, - dcName string, - remoteClient client.Client) (*stargateapi.Stargate, client.Client, error) { - - selector := k8ssandralabels.CleanedUpByLabels(kcKey) - options := &client.ListOptions{LabelSelector: labels.SelectorFromSet(selector)} - stargateList := &stargateapi.StargateList{} - stargateName := kcKey.Name + "-" + dcName + "-stargate" - - if remoteClient == nil { - for _, remoteClient := range r.ClientCache.GetAllClients() { - err := remoteClient.List(ctx, stargateList, options) - if err != nil { - return nil, nil, fmt.Errorf("failed to find Stargate (%s) for DC (%s) deletion: %v", stargateName, dcName, err) - } - for _, stargate := range stargateList.Items { - if stargate.Name == stargateName { - return &stargate, remoteClient, nil - } - } - } - } else { - err := remoteClient.List(ctx, stargateList, options) - if err != nil { - return nil, nil, fmt.Errorf("failed to find Stargate (%s) for DC (%s) deletion: %v", stargateName, dcName, err) - } - - for _, stargate := range stargateList.Items { - if stargate.Name == stargateName { - return &stargate, remoteClient, nil - } - } - } - - return nil, nil, nil -} - -func (r *K8ssandraClusterReconciler) findReaperForDeletion( - ctx context.Context, - kcKey client.ObjectKey, - dcName string, - remoteClient client.Client) (*reaperapi.Reaper, client.Client, error) { - - selector := k8ssandralabels.CleanedUpByLabels(kcKey) - options := &client.ListOptions{LabelSelector: labels.SelectorFromSet(selector)} - reaperList := &reaperapi.ReaperList{} - reaperName := kcKey.Name + "-" + dcName + "-reaper" - - if remoteClient == nil { - for _, remoteClient := range r.ClientCache.GetAllClients() { - err := remoteClient.List(ctx, reaperList, options) - if err != nil { - return nil, nil, fmt.Errorf("failed to find Reaper (%s) for DC (%s) deletion: %v", reaperName, dcName, err) - } - for _, reaper := range reaperList.Items { - if reaper.Name == reaperName { - return &reaper, remoteClient, nil - } - } - } - } else { - err := remoteClient.List(ctx, reaperList, options) - if err != nil { - return nil, nil, fmt.Errorf("failed to find Reaper (%s) for DC (%s) deletion: %v", reaperName, dcName, err) - } - - for _, reaper := range reaperList.Items { - if reaper.Name == reaperName { - return &reaper, remoteClient, nil - } - } - } - - return nil, nil, nil -} - func (r *K8ssandraClusterReconciler) findDcForDeletion( ctx context.Context, kcKey client.ObjectKey, diff --git a/controllers/k8ssandra/reaper.go b/controllers/k8ssandra/reaper.go index c50852ea6..bc563d8ef 100644 --- a/controllers/k8ssandra/reaper.go +++ b/controllers/k8ssandra/reaper.go @@ -30,9 +30,9 @@ import ( "github.com/k8ssandra/k8ssandra-operator/pkg/reaper" "github.com/k8ssandra/k8ssandra-operator/pkg/result" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) func (r *K8ssandraClusterReconciler) reconcileReaperSchema( @@ -127,7 +127,10 @@ func (r *K8ssandraClusterReconciler) reconcileReaper( if err := remoteClient.Get(ctx, reaperKey, actualReaper); err != nil { if errors.IsNotFound(err) { logger.Info("Creating Reaper resource") - if err := remoteClient.Create(ctx, desiredReaper); err != nil { + if err := controllerutil.SetControllerReference(actualDc, desiredReaper, r.Scheme); err != nil { + logger.Error(err, "Failed to set controller reference on Reaper resource") + return result.Error(err) + } else if err := remoteClient.Create(ctx, desiredReaper); err != nil { logger.Error(err, "Failed to create Reaper resource") return result.Error(err) } else { @@ -151,7 +154,10 @@ func (r *K8ssandraClusterReconciler) reconcileReaper( resourceVersion := actualReaper.GetResourceVersion() desiredReaper.DeepCopyInto(actualReaper) actualReaper.SetResourceVersion(resourceVersion) - if err := remoteClient.Update(ctx, actualReaper); err != nil { + if err := controllerutil.SetControllerReference(actualDc, actualReaper, r.Scheme); err != nil { + logger.Error(err, "Failed to set controller reference on Reaper resource") + return result.Error(err) + } else if err := remoteClient.Update(ctx, actualReaper); err != nil { logger.Error(err, "Failed to update Reaper resource") return result.Error(err) } @@ -193,37 +199,6 @@ func (r *K8ssandraClusterReconciler) reconcileReaper( } } -func (r *K8ssandraClusterReconciler) deleteReapers( - ctx context.Context, - kc *api.K8ssandraCluster, - dcTemplate api.CassandraDatacenterTemplate, - namespace string, - remoteClient client.Client, - kcLogger logr.Logger, -) (hasErrors bool) { - selector := k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}) - reaperList := &reaperapi.ReaperList{} - options := client.ListOptions{ - Namespace: namespace, - LabelSelector: labels.SelectorFromSet(selector), - } - if err := remoteClient.List(ctx, reaperList, &options); err != nil { - kcLogger.Error(err, "Failed to list Reaper objects", "Context", dcTemplate.K8sContext) - return true - } - for _, rp := range reaperList.Items { - if err := remoteClient.Delete(ctx, &rp); err != nil { - key := client.ObjectKey{Namespace: namespace, Name: rp.Name} - if !errors.IsNotFound(err) { - kcLogger.Error(err, "Failed to delete Reaper", "Reaper", key, - "Context", dcTemplate.K8sContext) - hasErrors = true - } - } - } - return -} - func (r *K8ssandraClusterReconciler) setStatusForReaper(kc *api.K8ssandraCluster, reaper *reaperapi.Reaper, dcName string) error { if len(kc.Status.Datacenters) == 0 { kc.Status.Datacenters = make(map[string]api.K8ssandraStatus) diff --git a/controllers/k8ssandra/stargate.go b/controllers/k8ssandra/stargate.go index 9c1032dea..8e3f032a9 100644 --- a/controllers/k8ssandra/stargate.go +++ b/controllers/k8ssandra/stargate.go @@ -2,6 +2,7 @@ package k8ssandra import ( "context" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/go-logr/logr" cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" @@ -52,7 +53,10 @@ func (r *K8ssandraClusterReconciler) reconcileStargate( if err := remoteClient.Get(ctx, stargateKey, actualStargate); err != nil { if errors.IsNotFound(err) { logger.Info("Creating Stargate resource") - if err := remoteClient.Create(ctx, desiredStargate); err != nil { + if err := controllerutil.SetControllerReference(actualDc, desiredStargate, r.Scheme); err != nil { + logger.Error(err, "Failed to set controller reference on Stargate resource") + return result.Error(err) + } else if err := remoteClient.Create(ctx, desiredStargate); err != nil { logger.Error(err, "Failed to create Stargate resource") return result.Error(err) } else { @@ -72,7 +76,10 @@ func (r *K8ssandraClusterReconciler) reconcileStargate( resourceVersion := actualStargate.GetResourceVersion() desiredStargate.DeepCopyInto(actualStargate) actualStargate.SetResourceVersion(resourceVersion) - if err = remoteClient.Update(ctx, actualStargate); err == nil { + if err := controllerutil.SetControllerReference(actualDc, actualStargate, r.Scheme); err != nil { + logger.Error(err, "Failed to set controller reference on Stargate resource") + return result.Error(err) + } else if err = remoteClient.Update(ctx, actualStargate); err == nil { return result.RequeueSoon(r.DefaultDelay) } else { logger.Error(err, "Failed to update Stargate") From ab6aaab32c9794e1016924396a14e113fbd6cefe Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Thu, 5 Sep 2024 15:43:22 -0700 Subject: [PATCH 02/12] Add Vector cleanup --- controllers/k8ssandra/cleanup.go | 34 +++++++++++++++++++ .../k8ssandra/k8ssandracluster_controller.go | 2 ++ controllers/k8ssandra/vector.go | 22 ++++++++++-- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/controllers/k8ssandra/cleanup.go b/controllers/k8ssandra/cleanup.go index 6d370f02d..1bf3b2b94 100644 --- a/controllers/k8ssandra/cleanup.go +++ b/controllers/k8ssandra/cleanup.go @@ -3,6 +3,7 @@ package k8ssandra import ( "context" "fmt" + "k8s.io/apimachinery/pkg/runtime" "github.com/go-logr/logr" cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" @@ -344,3 +345,36 @@ func (r *K8ssandraClusterReconciler) deleteCronJobs( } return } + +// setDcOwnership loads the remote resource identified by controlledKey, sets dc as its owner, and writes it back. If +// the remote resource does not exist, this is a no-op. +func setDcOwnership[T client.Object]( + ctx context.Context, + dc *cassdcapi.CassandraDatacenter, + controlledKey client.ObjectKey, + controlled T, + remoteClient client.Client, + scheme *runtime.Scheme, + logger logr.Logger, +) result.ReconcileResult { + if err := remoteClient.Get(ctx, controlledKey, controlled); err != nil { + if errors.IsNotFound(err) { + return result.Continue() + } + logger.Error(err, "Failed to get controlled resource", "key", controlledKey) + return result.Error(err) + } + if controllerutil.HasControllerReference(controlled) { + // Assume this is us from a previous reconcile loop + return result.Continue() + } + if err := controllerutil.SetControllerReference(dc, controlled, scheme); err != nil { + logger.Error(err, "Failed to set DC owner reference", "key", controlledKey) + return result.Error(err) + } + if err := remoteClient.Update(ctx, controlled); err != nil { + logger.Error(err, "Failed to update controlled resource", "key", controlledKey) + return result.Error(err) + } + return result.Continue() +} diff --git a/controllers/k8ssandra/k8ssandracluster_controller.go b/controllers/k8ssandra/k8ssandracluster_controller.go index 36035e8f3..cd74668c0 100644 --- a/controllers/k8ssandra/k8ssandracluster_controller.go +++ b/controllers/k8ssandra/k8ssandracluster_controller.go @@ -181,6 +181,8 @@ func (r *K8ssandraClusterReconciler) afterCassandraReconciled(ctx context.Contex return recResult } else if recResult := r.reconcileReaper(ctx, kc, dcTemplate, dc, logger, remoteClient); recResult.Completed() { return recResult + } else if recResult := r.setupVectorCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { + return recResult } } return result.Continue() diff --git a/controllers/k8ssandra/vector.go b/controllers/k8ssandra/vector.go index 730d4a2ea..f694b8a99 100644 --- a/controllers/k8ssandra/vector.go +++ b/controllers/k8ssandra/vector.go @@ -2,17 +2,18 @@ package k8ssandra import ( "context" - "github.com/k8ssandra/k8ssandra-operator/pkg/shared" - "github.com/go-logr/logr" + cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/cassandra" "github.com/k8ssandra/k8ssandra-operator/pkg/labels" k8ssandralabels "github.com/k8ssandra/k8ssandra-operator/pkg/labels" "github.com/k8ssandra/k8ssandra-operator/pkg/reconciliation" "github.com/k8ssandra/k8ssandra-operator/pkg/result" + "github.com/k8ssandra/k8ssandra-operator/pkg/shared" "github.com/k8ssandra/k8ssandra-operator/pkg/telemetry" "github.com/k8ssandra/k8ssandra-operator/pkg/utils" + v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -57,3 +58,20 @@ func (r *K8ssandraClusterReconciler) reconcileVector( dcLogger.Info("Vector Agent ConfigMap successfully reconciled") return result.Continue() } + +// setupVectorCleanup adds owner references to ensure that the remote resources created by reconcileVector are correctly +// cleaned up when the CassandraDatacenter is deleted. We do that in a second pass because the CassandraDatacenter did +// not exist yet at the time those resources were created. +func (r *K8ssandraClusterReconciler) setupVectorCleanup( + ctx context.Context, + kc *k8ssandraapi.K8ssandraCluster, + dc *cassdcapi.CassandraDatacenter, + remoteClient client.Client, + logger logr.Logger, +) result.ReconcileResult { + configMapKey := client.ObjectKey{ + Namespace: dc.Namespace, + Name: telemetry.VectorAgentConfigMapName(kc.SanitizedName(), dc.SanitizedName()), + } + return setDcOwnership(ctx, dc, configMapKey, &v1.ConfigMap{}, remoteClient, r.Scheme, logger) +} From 025ed5c604c32f7f423d7fa5466fffc282cecc1d Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 6 Sep 2024 09:20:56 -0700 Subject: [PATCH 03/12] Add Medusa cleanup --- .../k8ssandra/k8ssandracluster_controller.go | 2 ++ controllers/k8ssandra/medusa_reconciler.go | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/controllers/k8ssandra/k8ssandracluster_controller.go b/controllers/k8ssandra/k8ssandracluster_controller.go index cd74668c0..5ce7fda3f 100644 --- a/controllers/k8ssandra/k8ssandracluster_controller.go +++ b/controllers/k8ssandra/k8ssandracluster_controller.go @@ -183,6 +183,8 @@ func (r *K8ssandraClusterReconciler) afterCassandraReconciled(ctx context.Contex return recResult } else if recResult := r.setupVectorCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { return recResult + } else if recResult := r.setupMedusaCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { + return recResult } } return result.Continue() diff --git a/controllers/k8ssandra/medusa_reconciler.go b/controllers/k8ssandra/medusa_reconciler.go index e55f7ca5e..06b6a45f7 100644 --- a/controllers/k8ssandra/medusa_reconciler.go +++ b/controllers/k8ssandra/medusa_reconciler.go @@ -3,6 +3,7 @@ package k8ssandra import ( "context" "fmt" + cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" batchv1 "k8s.io/api/batch/v1" "k8s.io/apimachinery/pkg/api/errors" "os" @@ -346,3 +347,34 @@ func (r *K8ssandraClusterReconciler) getOperatorNamespace() string { } return operatorNamespace } + +// setupMedusaCleanup adds owner references to ensure that the remote resources created by reconcileMedusa are correctly +// cleaned up when the CassandraDatacenter is deleted. We do that in a second pass because the CassandraDatacenter did +// not exist yet at the time those resources were created. +func (r *K8ssandraClusterReconciler) setupMedusaCleanup( + ctx context.Context, + kc *k8ssandraapi.K8ssandraCluster, + dc *cassdcapi.CassandraDatacenter, + remoteClient client.Client, + logger logr.Logger, +) result.ReconcileResult { + // Note: this ConfigMap is an edge case because it is not DC-specific. If two CassandraDatacenters are in the same + // namespace, they would share the same ConfigMap, so setting one of them as the owner is wrong. + // However, this is an unlikely scenario (DCs are usually in different contexts, let alone namespaces). Also, if we + // delete one DC and the other still needs the ConfigMap, it will be recreated. + configMapKey := client.ObjectKey{ + Namespace: dc.Namespace, + // see pgk/medusa/reconcile.go + Name: fmt.Sprintf("%s-medusa", kc.SanitizedName()), + } + result := setDcOwnership(ctx, dc, configMapKey, &corev1.ConfigMap{}, remoteClient, r.Scheme, logger) + if result.Completed() { + return result + } + + cronjobKey := client.ObjectKey{ + Namespace: dc.Namespace, + Name: medusa.MedusaPurgeCronJobName(kc.SanitizedName(), dc.SanitizedName()), + } + return setDcOwnership(ctx, dc, cronjobKey, &batchv1.CronJob{}, remoteClient, r.Scheme, logger) +} From aa228799c20fa02d1562a42b5b8ba987bc3e96f6 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 6 Sep 2024 10:51:31 -0700 Subject: [PATCH 04/12] Add per-node config cleanup --- .../k8ssandra/k8ssandracluster_controller.go | 2 ++ controllers/k8ssandra/per_node_config.go | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/controllers/k8ssandra/k8ssandracluster_controller.go b/controllers/k8ssandra/k8ssandracluster_controller.go index 5ce7fda3f..dba19572f 100644 --- a/controllers/k8ssandra/k8ssandracluster_controller.go +++ b/controllers/k8ssandra/k8ssandracluster_controller.go @@ -185,6 +185,8 @@ func (r *K8ssandraClusterReconciler) afterCassandraReconciled(ctx context.Contex return recResult } else if recResult := r.setupMedusaCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { return recResult + } else if recResult := r.setupPerNodeConfigurationCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { + return recResult } } return result.Continue() diff --git a/controllers/k8ssandra/per_node_config.go b/controllers/k8ssandra/per_node_config.go index 4b0b517c6..c56ff8871 100644 --- a/controllers/k8ssandra/per_node_config.go +++ b/controllers/k8ssandra/per_node_config.go @@ -3,8 +3,8 @@ package k8ssandra import ( "context" "errors" - "github.com/go-logr/logr" + cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/annotations" "github.com/k8ssandra/k8ssandra-operator/pkg/cassandra" @@ -172,3 +172,20 @@ func (r *K8ssandraClusterReconciler) reconcileUserProvidedPerNodeConfiguration( return result.Continue() } + +// setupMedusaCleanup adds an owner reference to ensure that the remote ConfigMap created by +// reconcileDefaultPerNodeConfiguration is correctly cleaned up when the CassandraDatacenter is deleted. We do that in a +// second pass because the CassandraDatacenter did not exist yet at the time the ConfigMap was created. +func (r *K8ssandraClusterReconciler) setupPerNodeConfigurationCleanup( + ctx context.Context, + kc *k8ssandraapi.K8ssandraCluster, + dc *cassdcapi.CassandraDatacenter, + remoteClient client.Client, + logger logr.Logger, +) result.ReconcileResult { + configMapKey := client.ObjectKey{ + Namespace: dc.Namespace, + Name: kc.SanitizedName() + "-" + dc.SanitizedName() + "-metrics-agent-config", + } + return setDcOwnership(ctx, dc, configMapKey, &corev1.ConfigMap{}, remoteClient, r.Scheme, logger) +} From 70f7979eb33308d2120bd3ed98114548e369fe1b Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 6 Sep 2024 16:22:19 -0700 Subject: [PATCH 05/12] Add telemetry cleanup --- .../k8ssandra/k8ssandracluster_controller.go | 2 ++ controllers/k8ssandra/telemetry_cleanup.go | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 controllers/k8ssandra/telemetry_cleanup.go diff --git a/controllers/k8ssandra/k8ssandracluster_controller.go b/controllers/k8ssandra/k8ssandracluster_controller.go index dba19572f..4b3177897 100644 --- a/controllers/k8ssandra/k8ssandracluster_controller.go +++ b/controllers/k8ssandra/k8ssandracluster_controller.go @@ -187,6 +187,8 @@ func (r *K8ssandraClusterReconciler) afterCassandraReconciled(ctx context.Contex return recResult } else if recResult := r.setupPerNodeConfigurationCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { return recResult + } else if recResult := r.setupTelemetryCleanup(ctx, kc, dc, remoteClient, logger); recResult.Completed() { + return recResult } } return result.Continue() diff --git a/controllers/k8ssandra/telemetry_cleanup.go b/controllers/k8ssandra/telemetry_cleanup.go new file mode 100644 index 000000000..eb869bc3f --- /dev/null +++ b/controllers/k8ssandra/telemetry_cleanup.go @@ -0,0 +1,29 @@ +package k8ssandra + +import ( + "context" + "github.com/go-logr/logr" + cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" + k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" + "github.com/k8ssandra/k8ssandra-operator/pkg/result" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// setupTelemetryCleanup adds an owner reference to ensure that the remote ConfigMap created by +// cassandra_agent.Configurator is correctly cleaned up when the CassandraDatacenter is deleted. We do that in a +// second pass because the CassandraDatacenter did not exist yet at the time the ConfigMap was created. +func (r *K8ssandraClusterReconciler) setupTelemetryCleanup( + ctx context.Context, + kc *k8ssandraapi.K8ssandraCluster, + dc *cassdcapi.CassandraDatacenter, + remoteClient client.Client, + logger logr.Logger, +) result.ReconcileResult { + // TODO this should be factored better with the rest of the telemetry code + configMapKey := client.ObjectKey{ + Namespace: dc.Namespace, + Name: kc.SanitizedName() + "-" + dc.SanitizedName() + "-per-node-config", + } + return setDcOwnership(ctx, dc, configMapKey, &corev1.ConfigMap{}, remoteClient, r.Scheme, logger) +} From 00f2e362e38a4f85570f20841d74b0c6b39c8913 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 6 Sep 2024 17:02:05 -0700 Subject: [PATCH 06/12] Remove stale cleanup code --- controllers/k8ssandra/cleanup.go | 147 -------------------------- controllers/k8ssandra/cleanup_test.go | 98 ----------------- controllers/k8ssandra/datacenters.go | 6 +- 3 files changed, 5 insertions(+), 246 deletions(-) delete mode 100644 controllers/k8ssandra/cleanup_test.go diff --git a/controllers/k8ssandra/cleanup.go b/controllers/k8ssandra/cleanup.go index 1bf3b2b94..589040b29 100644 --- a/controllers/k8ssandra/cleanup.go +++ b/controllers/k8ssandra/cleanup.go @@ -8,17 +8,13 @@ import ( "github.com/go-logr/logr" cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" - k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/annotations" "github.com/k8ssandra/k8ssandra-operator/pkg/k8ssandra" k8ssandralabels "github.com/k8ssandra/k8ssandra-operator/pkg/labels" "github.com/k8ssandra/k8ssandra-operator/pkg/result" "github.com/k8ssandra/k8ssandra-operator/pkg/utils" - appsv1 "k8s.io/api/apps/v1" - batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -64,22 +60,6 @@ func (r *K8ssandraClusterReconciler) checkDeletion(ctx context.Context, kc *api. logger.Error(err, "Failed to delete CassandraDatacenter", "CassandraDatacenter", dcKey, "Context", dcTemplate.K8sContext) hasErrors = true } - - if r.deleteDeployments(ctx, kc, dcTemplate, namespace, remoteClient, logger) { - hasErrors = true - } - - if r.deleteServices(ctx, kc, dcTemplate, namespace, remoteClient, logger) { - hasErrors = true - } - - if r.deleteK8ssandraConfigMaps(ctx, kc, dcTemplate, namespace, remoteClient, logger) { - hasErrors = true - } - - if r.deleteCronJobs(ctx, kc, dcTemplate, namespace, remoteClient, logger) { - hasErrors = true - } } if hasErrors { @@ -219,133 +199,6 @@ func (r *K8ssandraClusterReconciler) findDcForDeletion( return nil, nil, nil } -func (r *K8ssandraClusterReconciler) deleteK8ssandraConfigMaps( - ctx context.Context, - kc *k8ssandraapi.K8ssandraCluster, - dcTemplate k8ssandraapi.CassandraDatacenterTemplate, - namespace string, - remoteClient client.Client, - kcLogger logr.Logger, -) (hasErrors bool) { - selector := k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.SanitizedName()}) - configMaps := &corev1.ConfigMapList{} - options := client.ListOptions{ - Namespace: namespace, - LabelSelector: labels.SelectorFromSet(selector), - } - if err := remoteClient.List(ctx, configMaps, &options); err != nil { - kcLogger.Error(err, "Failed to list ConfigMap objects", "Context", dcTemplate.K8sContext) - return true - } - for _, rp := range configMaps.Items { - if err := remoteClient.Delete(ctx, &rp); err != nil { - key := client.ObjectKey{Namespace: namespace, Name: rp.Name} - if !apierrors.IsNotFound(err) { - kcLogger.Error(err, "Failed to delete configmap", "ConfigMap", key, - "Context", dcTemplate.K8sContext) - hasErrors = true - } - } - } - return -} - -func (r *K8ssandraClusterReconciler) deleteServices( - ctx context.Context, - kc *k8ssandraapi.K8ssandraCluster, - dcTemplate k8ssandraapi.CassandraDatacenterTemplate, - namespace string, - remoteClient client.Client, - kcLogger logr.Logger, -) (hasErrors bool) { - selector := k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}) - options := client.ListOptions{ - Namespace: namespace, - LabelSelector: labels.SelectorFromSet(selector), - } - serviceList := &corev1.ServiceList{} - if err := remoteClient.List(ctx, serviceList, &options); err != nil { - kcLogger.Error(err, "Failed to list K8ssandra services", "Context", dcTemplate.K8sContext) - return true - } - for _, rp := range serviceList.Items { - kcLogger.Info("Deleting service", "Service", utils.GetKey(&rp)) - if err := remoteClient.Delete(ctx, &rp); err != nil { - key := client.ObjectKey{Namespace: namespace, Name: rp.Name} - if !errors.IsNotFound(err) { - kcLogger.Error(err, "Failed to delete Service", "Service", key, - "Context", dcTemplate.K8sContext) - hasErrors = true - } - } - } - - return -} - -func (r *K8ssandraClusterReconciler) deleteDeployments( - ctx context.Context, - kc *k8ssandraapi.K8ssandraCluster, - dcTemplate k8ssandraapi.CassandraDatacenterTemplate, - namespace string, - remoteClient client.Client, - kcLogger logr.Logger, -) (hasErrors bool) { - selector := k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}) - options := client.ListOptions{ - Namespace: namespace, - LabelSelector: labels.SelectorFromSet(selector), - } - deploymentList := &appsv1.DeploymentList{} - if err := remoteClient.List(ctx, deploymentList, &options); err != nil { - kcLogger.Error(err, "Failed to list K8ssandra deployments", "Context", dcTemplate.K8sContext) - return true - } - for _, item := range deploymentList.Items { - kcLogger.Info("Deleting deployment", "Deployment", utils.GetKey(&item)) - if err := remoteClient.Delete(ctx, &item); err != nil { - key := client.ObjectKey{Namespace: namespace, Name: item.Name} - if !errors.IsNotFound(err) { - kcLogger.Error(err, "Failed to delete Deployment", "Deployment", key, - "Context", dcTemplate.K8sContext) - hasErrors = true - } - } - } - - return -} - -func (r *K8ssandraClusterReconciler) deleteCronJobs( - ctx context.Context, - kc *k8ssandraapi.K8ssandraCluster, - dcTemplate k8ssandraapi.CassandraDatacenterTemplate, - namespace string, - remoteClient client.Client, - kcLogger logr.Logger, -) (hasErrors bool) { - selector := k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.SanitizedName()}) - options := client.ListOptions{ - Namespace: namespace, - LabelSelector: labels.SelectorFromSet(selector), - } - cronJobList := &batchv1.CronJobList{} - if err := remoteClient.List(ctx, cronJobList, &options); err != nil { - kcLogger.Error(err, "Failed to list Medusa CronJobs", "Context", dcTemplate.K8sContext) - return true - } - for _, item := range cronJobList.Items { - kcLogger.Info("Deleting CronJob", "CronJob", utils.GetKey(&item)) - if err := remoteClient.Delete(ctx, &item); err != nil { - key := client.ObjectKey{Namespace: namespace, Name: item.Name} - if !errors.IsNotFound(err) { - kcLogger.Error(err, "Failed to delete CronJob", "CronJob", key, "Context", dcTemplate.K8sContext) - } - } - } - return -} - // setDcOwnership loads the remote resource identified by controlledKey, sets dc as its owner, and writes it back. If // the remote resource does not exist, this is a no-op. func setDcOwnership[T client.Object]( diff --git a/controllers/k8ssandra/cleanup_test.go b/controllers/k8ssandra/cleanup_test.go deleted file mode 100644 index 790e63de2..000000000 --- a/controllers/k8ssandra/cleanup_test.go +++ /dev/null @@ -1,98 +0,0 @@ -package k8ssandra - -import ( - "context" - "testing" - - testlogr "github.com/go-logr/logr/testing" - k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" - k8ssandralabels "github.com/k8ssandra/k8ssandra-operator/pkg/labels" - "github.com/stretchr/testify/require" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func TestK8ssandraClusterReconciler_DeleteServices(t *testing.T) { - k8sMock := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - ctx := context.Background() - logger := testlogr.NewTestLogger(t) - - kc := &k8ssandraapi.K8ssandraCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-namespace", - }, - } - - namespace := "test-namespace" - - service := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "service-1", - Namespace: "test-namespace", - Labels: k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}), - }, - } - - require.NoError(t, k8sMock.Create(ctx, service)) - - res := K8ssandraClusterReconciler{ - Client: k8sMock, - Scheme: scheme.Scheme, - } - - hasError := res.deleteServices(ctx, kc, k8ssandraapi.CassandraDatacenterTemplate{}, namespace, k8sMock, logger) - require.False(t, hasError, "Error while deleting services") - - err := k8sMock.Get(ctx, client.ObjectKeyFromObject(service), service) - require.Error(t, err) - require.True(t, errors.IsNotFound(err)) -} - -func TestK8ssandraClusterReconciler_DeleteDeployments(t *testing.T) { - k8sMock := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - ctx := context.Background() - logger := testlogr.NewTestLogger(t) - - kc := &k8ssandraapi.K8ssandraCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-namespace", - }, - } - - namespace := "test-namespace" - - deployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment-1", - Namespace: "test-namespace", - Labels: k8ssandralabels.CleanedUpByLabels(client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}), - }, - } - - require.NoError(t, k8sMock.Create(ctx, deployment)) - - res := K8ssandraClusterReconciler{ - Client: k8sMock, - Scheme: scheme.Scheme, - } - - hasError := res.deleteDeployments(ctx, kc, k8ssandraapi.CassandraDatacenterTemplate{}, namespace, k8sMock, logger) - - if hasError != false { - t.Errorf("Error while deleting deployments") - } - - err := k8sMock.Get(ctx, client.ObjectKeyFromObject(deployment), deployment) - - if err == nil || !errors.IsNotFound(err) { - t.Errorf("Deployment was not deleted: %v", err) - } - -} diff --git a/controllers/k8ssandra/datacenters.go b/controllers/k8ssandra/datacenters.go index 5961fccef..7d3202d52 100644 --- a/controllers/k8ssandra/datacenters.go +++ b/controllers/k8ssandra/datacenters.go @@ -3,6 +3,7 @@ package k8ssandra import ( "context" "fmt" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sort" "strconv" "strings" @@ -412,7 +413,10 @@ func (r *K8ssandraClusterReconciler) reconcileDcRebuild( } else { if errors.IsNotFound(err) { logger.Info("Creating rebuild task", "Task", taskKey) - if err = remoteClient.Create(ctx, desiredTask); err != nil { + if err = controllerutil.SetControllerReference(dc, desiredTask, r.Scheme); err != nil { + logger.Error(err, "Failed to set controller reference", "Task", taskKey) + return result.Error(err) + } else if err = remoteClient.Create(ctx, desiredTask); err != nil { logger.Error(err, "Failed to create rebuild task", "Task", taskKey) return result.Error(err) } From 13515757e7b99dd23be02bc1c91420c9c9529b81 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 6 Sep 2024 17:08:37 -0700 Subject: [PATCH 07/12] Add changelog --- CHANGELOG/CHANGELOG-1.21.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG/CHANGELOG-1.21.md b/CHANGELOG/CHANGELOG-1.21.md index 58ab84827..4da42ace5 100644 --- a/CHANGELOG/CHANGELOG-1.21.md +++ b/CHANGELOG/CHANGELOG-1.21.md @@ -14,3 +14,5 @@ Changelog for the K8ssandra Operator, new PRs should update the `unreleased` sec When cutting a new release, update the `unreleased` heading to the tag being generated and date, like `## vX.Y.Z - YYYY-MM-DD` and create a new placeholder section for `unreleased` entries. ## unreleased + +* [ENHANCEMENT] [#992](https://github.com/k8ssandra/k8ssandra-operator/issues/992) Use controller references to clean up DC components From 2b633cb7c194dac64b9a726a0897148a953c9b71 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 6 Sep 2024 19:09:21 -0700 Subject: [PATCH 08/12] More simplification --- controllers/k8ssandra/cleanup.go | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/controllers/k8ssandra/cleanup.go b/controllers/k8ssandra/cleanup.go index 589040b29..e28dbeae7 100644 --- a/controllers/k8ssandra/cleanup.go +++ b/controllers/k8ssandra/cleanup.go @@ -123,9 +123,7 @@ func (r *K8ssandraClusterReconciler) checkDcDeletion(ctx context.Context, kc *ap func (r *K8ssandraClusterReconciler) deleteDc(ctx context.Context, kc *api.K8ssandraCluster, dcName string, cassDcName string, logger logr.Logger) result.ReconcileResult { kcKey := utils.GetKey(kc) - var remoteClient client.Client - - dc, remoteClient, err := r.findDcForDeletion(ctx, kcKey, dcName, remoteClient) + dc, remoteClient, err := r.findDcForDeletion(ctx, kcKey, dcName) if err != nil { return result.Error(err) } @@ -166,29 +164,16 @@ func (r *K8ssandraClusterReconciler) findDcForDeletion( ctx context.Context, kcKey client.ObjectKey, dcName string, - remoteClient client.Client) (*cassdcapi.CassandraDatacenter, client.Client, error) { +) (*cassdcapi.CassandraDatacenter, client.Client, error) { selector := k8ssandralabels.CleanedUpByLabels(kcKey) options := &client.ListOptions{LabelSelector: labels.SelectorFromSet(selector)} dcList := &cassdcapi.CassandraDatacenterList{} - if remoteClient == nil { - for _, remoteClient := range r.ClientCache.GetAllClients() { - err := remoteClient.List(ctx, dcList, options) - if err != nil { - return nil, nil, fmt.Errorf("failed to CassandraDatacenter (%s) for DC (%s) deletion: %v", dcName, dcName, err) - } - for _, dc := range dcList.Items { - if dc.Name == dcName { - return &dc, remoteClient, nil - } - } - } - } else { + for _, remoteClient := range r.ClientCache.GetAllClients() { err := remoteClient.List(ctx, dcList, options) if err != nil { - return nil, nil, fmt.Errorf("failed to find CassandraDatacenter (%s) for deletion: %v", dcName, err) + return nil, nil, fmt.Errorf("failed to CassandraDatacenter (%s) for DC (%s) deletion: %v", dcName, dcName, err) } - for _, dc := range dcList.Items { if dc.Name == dcName { return &dc, remoteClient, nil From b581c3de5f5ce785b0837bc2ff9010a1ccd83f32 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Mon, 9 Sep 2024 16:55:26 -0700 Subject: [PATCH 09/12] Fix env tests --- controllers/k8ssandra/auth_test.go | 6 ------ .../k8ssandra/cassandra_metrics_agent_test.go | 17 +++++++---------- controllers/k8ssandra/reaper_test.go | 5 ++++- controllers/k8ssandra/remove_dc_test.go | 3 --- controllers/k8ssandra/vector.go | 1 + controllers/k8ssandra/vector_test.go | 5 ++--- .../cassandra_agent/cassandra_agent_config.go | 4 ++++ test/framework/framework.go | 18 ++++++++++++++++-- 8 files changed, 34 insertions(+), 25 deletions(-) diff --git a/controllers/k8ssandra/auth_test.go b/controllers/k8ssandra/auth_test.go index 3d33ce79d..e6e08ded3 100644 --- a/controllers/k8ssandra/auth_test.go +++ b/controllers/k8ssandra/auth_test.go @@ -127,8 +127,6 @@ func createSingleDcClusterNoAuth(t *testing.T, ctx context.Context, f *framework err = f.DeleteK8ssandraCluster(ctx, client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}, timeout, interval) require.NoError(t, err, "failed to delete K8ssandraCluster") f.AssertObjectDoesNotExist(ctx, t, dcKey, &cassdcapi.CassandraDatacenter{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, stargateKey, &stargateapi.Stargate{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, reaperKey, &reaperapi.Reaper{}, timeout, interval) } // createSingleDcClusterAuth verifies that it is possible to create an authenticated cluster with one DC and with @@ -240,8 +238,6 @@ func createSingleDcClusterAuth(t *testing.T, ctx context.Context, f *framework.F err = f.DeleteK8ssandraCluster(ctx, client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}, timeout, interval) require.NoError(t, err, "failed to delete K8ssandraCluster") f.AssertObjectDoesNotExist(ctx, t, dcKey, &cassdcapi.CassandraDatacenter{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, stargateKey, &stargateapi.Stargate{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, reaperKey, &reaperapi.Reaper{}, timeout, interval) } // createSingleDcClusterAuthExternalSecrets verifies that kubernetes secrets for credentials are not created when @@ -367,8 +363,6 @@ func createSingleDcClusterAuthExternalSecrets(t *testing.T, ctx context.Context, err = f.DeleteK8ssandraCluster(ctx, client.ObjectKey{Namespace: kc.Namespace, Name: kc.Name}, timeout, interval) require.NoError(t, err, "failed to delete K8ssandraCluster") f.AssertObjectDoesNotExist(ctx, t, dcKey, &cassdcapi.CassandraDatacenter{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, stargateKey, &stargateapi.Stargate{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, reaperKey, &reaperapi.Reaper{}, timeout, interval) } func createSingleDcClusterExternalInternode(t *testing.T, ctx context.Context, f *framework.Framework, namespace string) { diff --git a/controllers/k8ssandra/cassandra_metrics_agent_test.go b/controllers/k8ssandra/cassandra_metrics_agent_test.go index e5defbfdc..531b516b2 100644 --- a/controllers/k8ssandra/cassandra_metrics_agent_test.go +++ b/controllers/k8ssandra/cassandra_metrics_agent_test.go @@ -4,8 +4,6 @@ import ( "context" "testing" - "github.com/stretchr/testify/assert" - cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" telemetryapi "github.com/k8ssandra/k8ssandra-operator/apis/telemetry/v1alpha1" @@ -87,9 +85,13 @@ func createSingleDcClusterWithMetricsAgent(t *testing.T, ctx context.Context, f // check that we have the right ConfigMap agentCmKey := framework.ClusterKey{NamespacedName: types.NamespacedName{Name: "test-dc1" + "-metrics-agent-config", Namespace: namespace}, K8sContext: f.DataPlaneContexts[0]} agentCm := corev1.ConfigMap{} - if err := f.Get(ctx, agentCmKey, &agentCm); err != nil { - assert.Fail(t, "could not find expected metrics-agent-config configmap") - } + require.Eventually(func() bool { + if err := f.Get(ctx, agentCmKey, &agentCm); err != nil { + t.Log("could not find expected metrics-agent-config configmap") + return false + } + return f.IsOwnedByCassandraDatacenter(&agentCm) + }, timeout, interval) // Verify the ConfigMap is set to be mounted require.True(len(dc.Spec.StorageConfig.AdditionalVolumes) > 0) @@ -109,9 +111,4 @@ func createSingleDcClusterWithMetricsAgent(t *testing.T, ctx context.Context, f err = f.DeleteK8ssandraCluster(ctx, client.ObjectKey{Namespace: namespace, Name: kc.Name}, timeout, interval) require.NoError(err, "failed to delete K8ssandraCluster") f.AssertObjectDoesNotExist(ctx, t, dcKey, &cassdcapi.CassandraDatacenter{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, - agentCmKey, - &corev1.ConfigMap{}, - timeout, - interval) } diff --git a/controllers/k8ssandra/reaper_test.go b/controllers/k8ssandra/reaper_test.go index 3ed5c7945..3c8fd6b1e 100644 --- a/controllers/k8ssandra/reaper_test.go +++ b/controllers/k8ssandra/reaper_test.go @@ -254,7 +254,10 @@ func createMultiDcClusterWithControlPlaneReaper(t *testing.T, ctx context.Contex Name: "reaper"}, } t.Log("check that control plane reaper is created") - require.Eventually(f.ReaperExists(ctx, cpReaperKey), timeout, interval) + withReaper := f.NewWithReaper(ctx, cpReaperKey) + require.Eventually(withReaper(func(r *reaperapi.Reaper) bool { + return true + }), timeout, interval) kc := &api.K8ssandraCluster{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/k8ssandra/remove_dc_test.go b/controllers/k8ssandra/remove_dc_test.go index e9184139a..da73f36d5 100644 --- a/controllers/k8ssandra/remove_dc_test.go +++ b/controllers/k8ssandra/remove_dc_test.go @@ -9,7 +9,6 @@ import ( cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1" - stargateapi "github.com/k8ssandra/k8ssandra-operator/apis/stargate/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/annotations" "github.com/k8ssandra/k8ssandra-operator/pkg/cassandra" "github.com/k8ssandra/k8ssandra-operator/pkg/stargate" @@ -291,8 +290,6 @@ func deleteDcWithStargateAndReaper(ctx context.Context, t *testing.T, f *framewo assertDatacenterRemovedFromClusterStatus(ctx, t, f, kcKey, dc2Key) f.AssertObjectDoesNotExist(ctx, t, dc2Key, &cassdcapi.CassandraDatacenter{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, sg2Key, &stargateapi.Stargate{}, timeout, interval) - f.AssertObjectDoesNotExist(ctx, t, reaper2Key, &reaperapi.Reaper{}, timeout, interval) verifyReplicationOfInternalKeyspacesUpdated(t, mockMgmtApi, replication, updatedReplication) } diff --git a/controllers/k8ssandra/vector.go b/controllers/k8ssandra/vector.go index f694b8a99..3aebdad05 100644 --- a/controllers/k8ssandra/vector.go +++ b/controllers/k8ssandra/vector.go @@ -69,6 +69,7 @@ func (r *K8ssandraClusterReconciler) setupVectorCleanup( remoteClient client.Client, logger logr.Logger, ) result.ReconcileResult { + logger.Info("Setting up Vector Agent ConfigMap cleanup") configMapKey := client.ObjectKey{ Namespace: dc.Namespace, Name: telemetry.VectorAgentConfigMapName(kc.SanitizedName(), dc.SanitizedName()), diff --git a/controllers/k8ssandra/vector_test.go b/controllers/k8ssandra/vector_test.go index 343c68d1e..37949a2b9 100644 --- a/controllers/k8ssandra/vector_test.go +++ b/controllers/k8ssandra/vector_test.go @@ -152,7 +152,7 @@ func createSingleDcClusterWithVector(t *testing.T, ctx context.Context, f *frame assert.Fail(t, "error setting status ready", err) } - // Check that the Vector config map was created + // Check that the Vector config map was created and is owned by the DC vectorConfigMapKey := types.NamespacedName{Namespace: namespace, Name: telemetry.VectorAgentConfigMapName(kc.Name, dc1Key.Name)} vectorConfigMap := &corev1.ConfigMap{} require.Eventually(func() bool { @@ -161,7 +161,7 @@ func createSingleDcClusterWithVector(t *testing.T, ctx context.Context, f *frame t.Logf("failed to get Vector config map: %v", err) return false } - return true + return f.IsOwnedByCassandraDatacenter(vectorConfigMap) }, timeout, interval, "timed out waiting for Vector config map") // Check that Vector configuration was set to the SystemLoggerResources @@ -184,6 +184,5 @@ func createSingleDcClusterWithVector(t *testing.T, ctx context.Context, f *frame t.Log("deleting K8ssandraCluster") err = f.DeleteK8ssandraCluster(ctx, client.ObjectKey{Namespace: namespace, Name: kc.Name}, timeout, interval) require.NoError(err, "failed to delete K8ssandraCluster") - f.AssertObjectDoesNotExist(ctx, t, framework.ClusterKey{K8sContext: f.DataPlaneContexts[1], NamespacedName: vectorConfigMapKey}, &corev1.ConfigMap{}, timeout, interval) f.AssertObjectDoesNotExist(ctx, t, dcKey, &cassdcapi.CassandraDatacenter{}, timeout, interval) } diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go index c69062d80..1d163a134 100644 --- a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go +++ b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go @@ -3,6 +3,7 @@ package cassandra_agent import ( "context" "path/filepath" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" "github.com/adutra/goalesce" @@ -157,6 +158,9 @@ func (c Configurator) ReconcileTelemetryAgentConfig(dc *cassdcapi.CassandraDatac Namespace: c.Kluster.Namespace, } desiredCm.SetLabels(labels.CleanedUpByLabels(KlKey)) + if err := controllerutil.SetControllerReference(dc, desiredCm, c.RemoteClient.Scheme()); err != nil { + return result.Error(err) + } recRes := reconciliation.ReconcileObject(c.Ctx, c.RemoteClient, c.RequeueDelay, *desiredCm) switch { diff --git a/test/framework/framework.go b/test/framework/framework.go index fd1f58aed..580e8908e 100644 --- a/test/framework/framework.go +++ b/test/framework/framework.go @@ -704,7 +704,7 @@ func (f *Framework) withStargate(ctx context.Context, key ClusterKey, condition func (f *Framework) StargateExists(ctx context.Context, key ClusterKey) func() bool { withStargate := f.NewWithStargate(ctx, key) return withStargate(func(s *stargateapi.Stargate) bool { - return true + return f.IsOwnedByCassandraDatacenter(s) }) } @@ -735,7 +735,7 @@ func (f *Framework) withReaper(ctx context.Context, key ClusterKey, condition fu func (f *Framework) ReaperExists(ctx context.Context, key ClusterKey) func() bool { withReaper := f.NewWithReaper(ctx, key) return withReaper(func(r *reaperapi.Reaper) bool { - return true + return f.IsOwnedByCassandraDatacenter(r) }) } @@ -815,3 +815,17 @@ func (f *Framework) GetContactPointsService( } return service, endpoints, nil } + +// IsOwnedByCassandraDatacenter checks that the given resource has an owner reference to a CassandraDatacenter. +// We can't directly verify the deletion itself because controller-manager isn't actually running in EnvTest. +// See also: https://github.com/kubernetes-sigs/controller-runtime/issues/626 +func (f *Framework) IsOwnedByCassandraDatacenter(resource metav1.Object) bool { + for _, ref := range resource.GetOwnerReferences() { + // Ideally we'd want to verify that ref.UID matches the CassandraDatacenter UID, but it's always readily + // available in all env tests. This should be good enough. + if ref.Kind == "CassandraDatacenter" && *ref.Controller { + return true + } + } + return false +} From 5de3968ee9258596122624c5304199c09c934d8e Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Tue, 10 Sep 2024 11:41:30 -0700 Subject: [PATCH 10/12] Fix name mixup --- controllers/k8ssandra/per_node_config.go | 2 +- controllers/k8ssandra/telemetry_cleanup.go | 4 ++-- pkg/nodeconfig/generate.go | 6 +++++- pkg/telemetry/cassandra_agent/cassandra_agent_config.go | 6 +++++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/controllers/k8ssandra/per_node_config.go b/controllers/k8ssandra/per_node_config.go index c56ff8871..7287f1e3d 100644 --- a/controllers/k8ssandra/per_node_config.go +++ b/controllers/k8ssandra/per_node_config.go @@ -185,7 +185,7 @@ func (r *K8ssandraClusterReconciler) setupPerNodeConfigurationCleanup( ) result.ReconcileResult { configMapKey := client.ObjectKey{ Namespace: dc.Namespace, - Name: kc.SanitizedName() + "-" + dc.SanitizedName() + "-metrics-agent-config", + Name: nodeconfig.NewDefaultPerNodeConfigMapName(kc.CassClusterName(), dc.DatacenterName()), } return setDcOwnership(ctx, dc, configMapKey, &corev1.ConfigMap{}, remoteClient, r.Scheme, logger) } diff --git a/controllers/k8ssandra/telemetry_cleanup.go b/controllers/k8ssandra/telemetry_cleanup.go index eb869bc3f..fa7fce388 100644 --- a/controllers/k8ssandra/telemetry_cleanup.go +++ b/controllers/k8ssandra/telemetry_cleanup.go @@ -6,6 +6,7 @@ import ( cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/result" + "github.com/k8ssandra/k8ssandra-operator/pkg/telemetry/cassandra_agent" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -20,10 +21,9 @@ func (r *K8ssandraClusterReconciler) setupTelemetryCleanup( remoteClient client.Client, logger logr.Logger, ) result.ReconcileResult { - // TODO this should be factored better with the rest of the telemetry code configMapKey := client.ObjectKey{ Namespace: dc.Namespace, - Name: kc.SanitizedName() + "-" + dc.SanitizedName() + "-per-node-config", + Name: cassandra_agent.ConfigMapName(kc.CassClusterName(), dc.DatacenterName()), } return setDcOwnership(ctx, dc, configMapKey, &corev1.ConfigMap{}, remoteClient, r.Scheme, logger) } diff --git a/pkg/nodeconfig/generate.go b/pkg/nodeconfig/generate.go index ad4c3f301..9a0af0d85 100644 --- a/pkg/nodeconfig/generate.go +++ b/pkg/nodeconfig/generate.go @@ -39,11 +39,15 @@ func NewDefaultPerNodeConfigMap(kcKey types.NamespacedName, kc *k8ssandraapi.K8s func NewDefaultPerNodeConfigMapKey(kc *k8ssandraapi.K8ssandraCluster, dcConfig *cassandra.DatacenterConfig) types.NamespacedName { return types.NamespacedName{ - Name: cassdcapi.CleanupForKubernetes(kc.CassClusterName() + "-" + dcConfig.CassDcName() + "-per-node-config"), + Name: NewDefaultPerNodeConfigMapName(kc.CassClusterName(), dcConfig.CassDcName()), Namespace: utils.FirstNonEmptyString(dcConfig.Meta.Namespace, kc.Namespace), } } +func NewDefaultPerNodeConfigMapName(kcCqlName, dcCqlName string) string { + return cassdcapi.CleanupForKubernetes(kcCqlName + "-" + dcCqlName + "-per-node-config") +} + func newPerNodeConfigMap(kcKey, configKey types.NamespacedName) *corev1.ConfigMap { return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go index 1d163a134..c2ec9e6b2 100644 --- a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go +++ b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go @@ -144,7 +144,11 @@ func (c Configurator) GetTelemetryAgentConfigMap() (*corev1.ConfigMap, error) { } func (c Configurator) configMapName() string { - return cassdcapi.CleanupForKubernetes(c.Kluster.CassClusterName() + "-" + c.DcName + "-metrics-agent-config") + return ConfigMapName(c.Kluster.CassClusterName(), c.DcName) +} + +func ConfigMapName(kcCqlName, dcCqlName string) string { + return cassdcapi.CleanupForKubernetes(kcCqlName + "-" + dcCqlName + "-metrics-agent-config") } func (c Configurator) ReconcileTelemetryAgentConfig(dc *cassdcapi.CassandraDatacenter) result.ReconcileResult { From c6732999ddd6362c3785f22558edf495e12db113 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Tue, 10 Sep 2024 17:09:12 -0700 Subject: [PATCH 11/12] Remove premature telemetry reference --- pkg/telemetry/cassandra_agent/cassandra_agent_config.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go index c2ec9e6b2..1d951708a 100644 --- a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go +++ b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go @@ -3,7 +3,6 @@ package cassandra_agent import ( "context" "path/filepath" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" "github.com/adutra/goalesce" @@ -162,9 +161,6 @@ func (c Configurator) ReconcileTelemetryAgentConfig(dc *cassdcapi.CassandraDatac Namespace: c.Kluster.Namespace, } desiredCm.SetLabels(labels.CleanedUpByLabels(KlKey)) - if err := controllerutil.SetControllerReference(dc, desiredCm, c.RemoteClient.Scheme()); err != nil { - return result.Error(err) - } recRes := reconciliation.ReconcileObject(c.Ctx, c.RemoteClient, c.RequeueDelay, *desiredCm) switch { From 616cf6d8b89408d32d88d0ffc06ac12c732bd9ce Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Wed, 11 Sep 2024 08:35:57 -0700 Subject: [PATCH 12/12] Fix metrics agent test --- .../k8ssandra/cassandra_metrics_agent_test.go | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/controllers/k8ssandra/cassandra_metrics_agent_test.go b/controllers/k8ssandra/cassandra_metrics_agent_test.go index 531b516b2..ecc65ec88 100644 --- a/controllers/k8ssandra/cassandra_metrics_agent_test.go +++ b/controllers/k8ssandra/cassandra_metrics_agent_test.go @@ -76,6 +76,47 @@ func createSingleDcClusterWithMetricsAgent(t *testing.T, ctx context.Context, f t.Log("check that the datacenter was created") dcKey := framework.ClusterKey{NamespacedName: types.NamespacedName{Namespace: namespace, Name: "dc1"}, K8sContext: f.DataPlaneContexts[0]} require.Eventually(f.DatacenterExists(ctx, dcKey), timeout, interval) + + t.Log("update datacenter status to ready") + kcKey := framework.NewClusterKey(f.ControlPlaneContext, namespace, kc.Name) + err = f.PatchDatacenterStatus(ctx, dcKey, func(dc *cassdcapi.CassandraDatacenter) { + dc.Status.CassandraOperatorProgress = cassdcapi.ProgressReady + dc.SetCondition(cassdcapi.DatacenterCondition{ + Type: cassdcapi.DatacenterReady, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + }) + }) + require.NoError(err, "failed to update datacenter status to ready") + + t.Log("check that the K8ssandraCluster status is updated") + require.Eventually(func() bool { + kc := &api.K8ssandraCluster{} + err = f.Get(ctx, kcKey, kc) + + if err != nil { + t.Logf("failed to get K8ssandraCluster: %v", err) + return false + } + + if len(kc.Status.Datacenters) == 0 { + return false + } + + k8ssandraStatus, found := kc.Status.Datacenters[dcKey.Name] + if !found { + t.Logf("status for datacenter %s not found", dcKey) + return false + } + + condition := findDatacenterCondition(k8ssandraStatus.Cassandra, cassdcapi.DatacenterReady) + return condition != nil && condition.Status == corev1.ConditionTrue + }, timeout, interval, "timed out waiting for K8ssandraCluster status update") + + require.Eventually(func() bool { + return f.UpdateDatacenterGeneration(ctx, t, dcKey) + }, timeout, interval, "failed to update dc1 generation") + // Check that we have the right volumes and volume mounts. dc := &cassdcapi.CassandraDatacenter{} if err := f.Get(ctx, dcKey, dc); err != nil { @@ -83,7 +124,7 @@ func createSingleDcClusterWithMetricsAgent(t *testing.T, ctx context.Context, f } // check that we have the right ConfigMap - agentCmKey := framework.ClusterKey{NamespacedName: types.NamespacedName{Name: "test-dc1" + "-metrics-agent-config", Namespace: namespace}, K8sContext: f.DataPlaneContexts[0]} + agentCmKey := framework.ClusterKey{NamespacedName: types.NamespacedName{Name: "test-dc1-metrics-agent-config", Namespace: namespace}, K8sContext: f.DataPlaneContexts[0]} agentCm := corev1.ConfigMap{} require.Eventually(func() bool { if err := f.Get(ctx, agentCmKey, &agentCm); err != nil { @@ -112,3 +153,12 @@ func createSingleDcClusterWithMetricsAgent(t *testing.T, ctx context.Context, f require.NoError(err, "failed to delete K8ssandraCluster") f.AssertObjectDoesNotExist(ctx, t, dcKey, &cassdcapi.CassandraDatacenter{}, timeout, interval) } + +func findDatacenterCondition(status *cassdcapi.CassandraDatacenterStatus, condType cassdcapi.DatacenterConditionType) *cassdcapi.DatacenterCondition { + for _, condition := range status.Conditions { + if condition.Type == condType { + return &condition + } + } + return nil +}