Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cronjob for purging backup is not configurable #1278

Open
c3-clement opened this issue Apr 11, 2024 · 3 comments
Open

Cronjob for purging backup is not configurable #1278

c3-clement opened this issue Apr 11, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@c3-clement
Copy link
Contributor

c3-clement commented Apr 11, 2024

What happened?

I noticed that in release 1.12, a cronjob that purges Medusa backup has been added. See #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

  • K8ssandra Operator version:
    1.12
    Insert image tag or Git SHA here
  • Kubernetes version information:
    1.29

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

@c3-clement c3-clement added the bug Something isn't working label Apr 11, 2024
@c3-jharbaugh
Copy link

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

@ericsmalling
Copy link
Contributor

ericsmalling commented Apr 19, 2024

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
Copy link
Contributor

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")
 	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: No status
Development

No branches or pull requests

3 participants