k8ssandra / k8ssandra-operator

The Kubernetes operator for K8ssandra
https://k8ssandra.io/
Apache License 2.0
170 stars 78 forks source link

Cronjob for purging backup is not configurable #1278

Open c3-clement opened 7 months ago

c3-clement commented 7 months ago

What happened?

I noticed that in release 1.12, a cronjob that purges Medusa backup has been added. See https://github.com/k8ssandra/k8ssandra-operator/pull/1167

It would be great to be able to disable the cronjob creation from K8ssandra CR. In our use-case, we are handling backup purging in a different way.

Also, most of the cronjob spec fields are hardcoded and it's not possible to change them from configuration. It would be great to be able to configure the image, cron schedule, etc..

Also, I'm curious about the technical choices behind this implementation. Instead of creating a cronjob that performs a kubectl apply, why not use something like MedusaBackupSchedule? This would allow the operator to automatically create MedusaBackupJob instances according to the defined cron schedule. For purging and sync, we can imagine a new CR MedusaTaskSchedule that would create MedusaTask. This would have avoided the need of managing an additional container and cronjob.

Did you expect to see something different? I expect to be able to disable this cronjob, and to be able to configure it's spec.

How to reproduce it (as minimally and precisely as possible):

Deploy a k8ssandra cluster with Medusa ebnabled.

Environment

┆Issue is synchronized with this Jira Story by Unito ┆Issue Number: K8OP-32

c3-jharbaugh commented 6 months ago

@adejanovski - any chance this will be patched in 1.12.x?

ericsmalling commented 6 months ago

This patch against main / v1.15.0 will add a purgeBackups: field to k8ssandraCluster CRD:

  medusa:
    purgeBackups: false

Set to false, the cronjob will not be created Set to true (or omitted) the cronjob will be created

It probably should have some tests added and might need tweaks if you want to back-port it to 1.12 but it works on 1.14 and 1.15. cron-config.patch

ericsmalling commented 6 months ago

Text of the patch (for those not wanting to dl the file):

diff --git a/apis/medusa/v1alpha1/medusa_types.go b/apis/medusa/v1alpha1/medusa_types.go
index 1dc2d63..d709940 100644
--- a/apis/medusa/v1alpha1/medusa_types.go
+++ b/apis/medusa/v1alpha1/medusa_types.go
@@ -181,11 +181,4 @@ type MedusaClusterTemplate struct {
    // Define the liveness probe settings to use for the Medusa containers.
    // +optional
    LivenessProbe *corev1.Probe `json:"livenessProbe,omitempty"`
-
-   // Should medusa backups be purged or not
-   // Defaults to true.
-   // +kubebuilder:default=true
-   // +optional
-
-   PurgeBackups bool `json:"purgeBackups,omitempty"`
 }
diff --git a/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml b/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
index 06d650e..d22595b 100644
--- a/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
+++ b/config/crd/bases/k8ssandra.io_k8ssandraclusters.yaml
@@ -25180,7 +25180,6 @@ spec:
                             type: string
                         type: object
                     type: object
-                  purgeBackups: false
                   storageProperties:
                     description: Provides all storage backend related properties for
                       backups.
diff --git a/controllers/k8ssandra/medusa_reconciler.go b/controllers/k8ssandra/medusa_reconciler.go
index c2b372d..b23bb0c 100644
--- a/controllers/k8ssandra/medusa_reconciler.go
+++ b/controllers/k8ssandra/medusa_reconciler.go
@@ -138,23 +138,21 @@ func (r *K8ssandraClusterReconciler) reconcileMedusa(
            return result.RequeueSoon(r.DefaultDelay)
        }
        // Create a cron job to purge Medusa backups
-       logger.Info("Checking if Medusa backups should be purged, PurgeBackups: " + fmt.Sprintf("%t", medusaSpec.PurgeBackups))
-       if medusaSpec.PurgeBackups {
-           operatorNamespace := r.getOperatorNamespace()
-           purgeCronJob, err := medusa.PurgeCronJob(dcConfig, kc.SanitizedName(), operatorNamespace, logger)
-           if err != nil {
-               logger.Info("Failed to create Medusa purge backups cronjob", "error", err)
-               return result.Error(err)
-           }
-           purgeCronJob.SetLabels(labels.CleanedUpByLabels(kcKey))
-           recRes = reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *purgeCronJob)
-           switch {
-           case recRes.IsError():
-               return recRes
-           case recRes.IsRequeue():
-               return recRes
-           }
+       operatorNamespace := r.getOperatorNamespace()
+       purgeCronJob, err := medusa.PurgeCronJob(dcConfig, kc.SanitizedName(), operatorNamespace, logger)
+       if err != nil {
+           logger.Info("Failed to create Medusa purge backups cronjob", "error", err)
+           return result.Error(err)
+       }
+       purgeCronJob.SetLabels(labels.CleanedUpByLabels(kcKey))
+       recRes = reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *purgeCronJob)
+       switch {
+       case recRes.IsError():
+           return recRes
+       case recRes.IsRequeue():
+           return recRes
        }
+
    } else {
        logger.Info("Medusa is not enabled")
    }