You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2021/11/03 20:29:35 UTC

[solr-operator] branch main updated: Remove Persistence option for SolrBackup. (#357)

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-operator.git


The following commit(s) were added to refs/heads/main by this push:
     new fb29d60  Remove Persistence option for SolrBackup. (#357)
fb29d60 is described below

commit fb29d602e216eb2a4c3e29a4fdc75fb3d4fc8269
Author: Houston Putman <ho...@apache.org>
AuthorDate: Wed Nov 3 16:29:30 2021 -0400

    Remove Persistence option for SolrBackup. (#357)
---
 api/v1beta1/solrbackup_types.go                   |  65 ++----
 api/v1beta1/solrbackup_with_defaults_test.go      |  45 ++++
 config/crd/bases/solr.apache.org_solrbackups.yaml |   4 +-
 config/rbac/role.yaml                             |  18 --
 controllers/solrbackup_controller.go              | 136 ++----------
 controllers/util/backup_util.go                   | 251 +---------------------
 controllers/util/solr_api/api.go                  |   2 +-
 docs/solr-backup/README.md                        |  19 +-
 docs/upgrade-notes.md                             |   6 +
 example/test_backup_managed_with_persistence.yaml |  31 ---
 example/test_solrcloud_backuprepos.yaml           |   4 +-
 helm/solr-operator/Chart.yaml                     |   7 +
 helm/solr-operator/crds/crds.yaml                 |   4 +-
 helm/solr-operator/templates/role.yaml            |  18 --
 14 files changed, 120 insertions(+), 490 deletions(-)

diff --git a/api/v1beta1/solrbackup_types.go b/api/v1beta1/solrbackup_types.go
index 895f9ee..e81ee9e 100644
--- a/api/v1beta1/solrbackup_types.go
+++ b/api/v1beta1/solrbackup_types.go
@@ -24,15 +24,6 @@ import (
 	"strings"
 )
 
-// EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
-// NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
-
-const (
-	DefaultAWSCliImageRepo    = "infrastructureascode/aws-cli"
-	DefaultAWSCliImageVersion = "1.16.204"
-	DefaultS3Retries          = 5
-)
-
 // SolrBackupSpec defines the desired state of SolrBackup
 type SolrBackupSpec struct {
 	// A reference to the SolrCloud to create a backup for
@@ -52,13 +43,17 @@ type SolrBackupSpec struct {
 	Location string `json:"location,omitempty"`
 
 	// Persistence is the specification on how to persist the backup data.
+	// This feature has been removed as of v0.5.0. Any options specified here will not be used.
+	//
 	// +optional
 	Persistence *PersistenceSource `json:"persistence,omitempty"`
 }
 
-func (spec *SolrBackupSpec) withDefaults(backupName string) (changed bool) {
+func (spec *SolrBackupSpec) withDefaults() (changed bool) {
+	// Remove any Persistence specs, since this feature was removed.
 	if spec.Persistence != nil {
-		changed = spec.Persistence.withDefaults(backupName) || changed
+		changed = true
+		spec.Persistence = nil
 	}
 
 	return changed
@@ -66,6 +61,8 @@ func (spec *SolrBackupSpec) withDefaults(backupName string) (changed bool) {
 
 // PersistenceSource defines the location and method of persisting the backup data.
 // Exactly one member must be specified.
+//
+// Deprecated: Will be unused as of v0.5.0
 type PersistenceSource struct {
 	// Persist to an s3 compatible endpoint
 	// +optional
@@ -76,19 +73,9 @@ type PersistenceSource struct {
 	Volume *VolumePersistenceSource `json:"volume,omitempty"`
 }
 
-func (spec *PersistenceSource) withDefaults(backupName string) (changed bool) {
-	if spec.Volume != nil {
-		changed = spec.Volume.withDefaults(backupName) || changed
-	}
-
-	if spec.S3 != nil {
-		changed = spec.S3.withDefaults(backupName) || changed
-	}
-
-	return changed
-}
-
 // S3PersistenceSource defines the specs for connecting to s3 for persistence
+//
+// Deprecated: Will be unused as of v0.5.0
 type S3PersistenceSource struct {
 	// The S3 compatible endpoint URL
 	// +optional
@@ -120,26 +107,9 @@ type S3PersistenceSource struct {
 	AWSCliImage ContainerImage `json:"AWSCliImage,omitempty"`
 }
 
-func (spec *S3PersistenceSource) withDefaults(backupName string) (changed bool) {
-	changed = spec.AWSCliImage.withDefaults(DefaultAWSCliImageRepo, DefaultAWSCliImageVersion, DefaultPullPolicy) || changed
-
-	if spec.Key == "" {
-		spec.Key = backupName + ".tgz"
-		changed = true
-	} else if strings.HasPrefix(spec.Key, "/") {
-		spec.Key = strings.TrimPrefix(spec.Key, "/")
-		changed = true
-	}
-	if spec.Retries == nil {
-		retries := int32(DefaultS3Retries)
-		spec.Retries = &retries
-		changed = true
-	}
-
-	return changed
-}
-
 // S3Secrets describes the secrets provided for accessing s3.
+//
+// Deprecated: Will be unused as of v0.5.0
 type S3Secrets struct {
 	// The name of the secrets object to use
 	Name string `json:"fromSecret"`
@@ -162,6 +132,8 @@ type S3Secrets struct {
 }
 
 // UploadSpec defines the location and method of uploading the backup data
+//
+// Deprecated: Will be unused as of v0.5.0
 type VolumePersistenceSource struct {
 	// The volume for persistence
 	VolumeSource corev1.VolumeSource `json:"source"`
@@ -180,6 +152,7 @@ type VolumePersistenceSource struct {
 	BusyBoxImage ContainerImage `json:"busyBoxImage,omitempty"`
 }
 
+// Deprecated: Will be unused as of v0.5.0
 func (spec *VolumePersistenceSource) withDefaults(backupName string) (changed bool) {
 	changed = spec.BusyBoxImage.withDefaults(DefaultBusyBoxImageRepo, DefaultBusyBoxImageVersion, DefaultPullPolicy) || changed
 
@@ -205,7 +178,9 @@ type SolrBackupStatus struct {
 	// +optional
 	CollectionBackupStatuses []CollectionBackupStatus `json:"collectionBackupStatuses,omitempty"`
 
-	// Whether the backups are in progress of being persisted
+	// Whether the backups are in progress of being persisted.
+	// This feature has been removed as of v0.5.0.
+	//
 	// +optional
 	PersistenceStatus *BackupPersistenceStatus `json:"persistenceStatus,omitempty"`
 
@@ -255,6 +230,8 @@ type CollectionBackupStatus struct {
 }
 
 // BackupPersistenceStatus defines the status of persisting Solr backup data
+//
+// Deprecated: Will be unused as of v0.5.0
 type BackupPersistenceStatus struct {
 	// Whether the collection is being backed up
 	// +optional
@@ -319,7 +296,7 @@ type SolrBackup struct {
 
 // WithDefaults set default values when not defined in the spec.
 func (sb *SolrBackup) WithDefaults() bool {
-	return sb.Spec.withDefaults(sb.Name)
+	return sb.Spec.withDefaults()
 }
 
 //+kubebuilder:object:root=true
diff --git a/api/v1beta1/solrbackup_with_defaults_test.go b/api/v1beta1/solrbackup_with_defaults_test.go
new file mode 100644
index 0000000..37702f6
--- /dev/null
+++ b/api/v1beta1/solrbackup_with_defaults_test.go
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package v1beta1
+
+import (
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"testing"
+)
+
+func TestRemoveBackupPersistence(t *testing.T) {
+	solrBackup := &SolrBackup{
+		ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
+		Spec:       SolrBackupSpec{},
+	}
+
+	// Set defaults for SolrCloud
+	assert.False(t, solrBackup.WithDefaults(), "Defaulting an empty backup should do nothing")
+
+	//No deprecated repository to move, no existing repos
+	assert.Nilf(t, solrBackup.Spec.Persistence, "No Persistence should exist after setting backup defaults")
+
+	solrBackup.Spec.Persistence = &PersistenceSource{}
+
+	// Set defaults for SolrCloud
+	assert.True(t, solrBackup.WithDefaults(), "Defaulting a backup with persistence should result in a change")
+
+	//No deprecated repository to move, no existing repos
+	assert.Nilf(t, solrBackup.Spec.Persistence, "No Persistence should exist after setting backup defaults")
+}
diff --git a/config/crd/bases/solr.apache.org_solrbackups.yaml b/config/crd/bases/solr.apache.org_solrbackups.yaml
index 53ef570..ddb9ffe 100644
--- a/config/crd/bases/solr.apache.org_solrbackups.yaml
+++ b/config/crd/bases/solr.apache.org_solrbackups.yaml
@@ -71,7 +71,7 @@ spec:
                 description: The location to store the backup in the specified backup repository.
                 type: string
               persistence:
-                description: Persistence is the specification on how to persist the backup data.
+                description: Persistence is the specification on how to persist the backup data. This feature has been removed as of v0.5.0. Any options specified here will not be used.
                 properties:
                   S3:
                     description: Persist to an s3 compatible endpoint
@@ -1106,7 +1106,7 @@ spec:
                 description: Whether the backup has finished
                 type: boolean
               persistenceStatus:
-                description: Whether the backups are in progress of being persisted
+                description: Whether the backups are in progress of being persisted. This feature has been removed as of v0.5.0.
                 properties:
                   finishTimestamp:
                     description: Time that the collection backup finished at
diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
index 406f5d6..a01adfa 100644
--- a/config/rbac/role.yaml
+++ b/config/rbac/role.yaml
@@ -135,24 +135,6 @@ rules:
   verbs:
   - get
 - apiGroups:
-  - batch
-  resources:
-  - jobs
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - patch
-  - update
-  - watch
-- apiGroups:
-  - batch
-  resources:
-  - jobs/status
-  verbs:
-  - get
-- apiGroups:
   - networking.k8s.io
   resources:
   - ingresses
diff --git a/controllers/solrbackup_controller.go b/controllers/solrbackup_controller.go
index 77373e3..cfee5d4 100644
--- a/controllers/solrbackup_controller.go
+++ b/controllers/solrbackup_controller.go
@@ -33,7 +33,6 @@ import (
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -49,8 +48,6 @@ type SolrBackupReconciler struct {
 
 //+kubebuilder:rbac:groups="",resources=pods/exec,verbs=create
 //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list
-//+kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups=batch,resources=jobs/status,verbs=get
 //+kubebuilder:rbac:groups=solr.apache.org,resources=solrclouds,verbs=get;list;watch
 //+kubebuilder:rbac:groups=solr.apache.org,resources=solrclouds/status,verbs=get
 //+kubebuilder:rbac:groups=solr.apache.org,resources=solrbackups,verbs=get;list;watch;create;update;patch;delete
@@ -91,48 +88,21 @@ func (r *SolrBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	// When working with the collection backups, auto-requeue after 5 seconds
 	// to check on the status of the async solr backup calls
-	requeueOrNot := reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}
+	requeueOrNot := reconcile.Result{}
 
-	solrCloud, allCollectionsComplete, collectionActionTaken, err := r.reconcileSolrCloudBackup(ctx, backup, logger)
+	solrCloud, _, err := r.reconcileSolrCloudBackup(ctx, backup, logger)
 	if err != nil {
 		// TODO Should we be failing the backup for some sub-set of errors here?
 		logger.Error(err, "Error while taking SolrCloud backup")
-	}
-	if allCollectionsComplete && collectionActionTaken {
-		// Requeue immediately to start the persisting job
-		// From here on in the backup lifecycle, requeueing will not happen for the backup.
-		requeueOrNot = reconcile.Result{RequeueAfter: time.Second * 10}
-	} else if solrCloud == nil {
-		requeueOrNot = reconcile.Result{}
-	} else {
-		// Only persist if the backup CRD is not finished (something bad happened)
-		// and the collection backups are all complete (not necessarily successful)
-		// Do not do this right after the collectionsBackup have been complete, wait till the next cycle
-		if allCollectionsComplete && !backup.Status.Finished {
-			if backup.Spec.Persistence != nil {
-				// We will count on the Job updates to be notified
-				requeueOrNot = reconcile.Result{}
-				err = r.persistSolrCloudBackups(ctx, backup, solrCloud, logger)
-				if err != nil {
-					logger.Error(err, "Error while persisting SolrCloud backup")
-				}
-			} else {
-				// Persistence not configured for this backup, mark as finished.
-				tru := true
-				backup.Status.Finished = true
-				backup.Status.Successful = &tru
-				now := metav1.Now()
-				backup.Status.FinishTime = &now
-			}
-		}
-	}
 
-	if backup.Status.Finished && backup.Status.FinishTime == nil {
+		// Requeue after 10 seconds for errors.
+		requeueOrNot = reconcile.Result{Requeue: true, RequeueAfter: time.Second * 10}
+	} else if solrCloud != nil && !backup.Status.Finished {
+		// Only requeue if the SolrCloud we are backing up exists and we are not finished with the backups.
+		requeueOrNot = reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}
+	} else if backup.Status.Finished && backup.Status.FinishTime == nil {
 		now := metav1.Now()
 		backup.Status.FinishTime = &now
-		if backup.Spec.Persistence != nil {
-			backup.Status.Successful = backup.Status.PersistenceStatus.Successful
-		}
 	}
 
 	if !reflect.DeepEqual(oldStatus, &backup.Status) {
@@ -140,39 +110,35 @@ func (r *SolrBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		err = r.Status().Update(ctx, backup)
 	}
 
-	if err != nil && backup.Status.Finished {
-		requeueOrNot = reconcile.Result{}
-	}
-
 	return requeueOrNot, err
 }
 
-func (r *SolrBackupReconciler) reconcileSolrCloudBackup(ctx context.Context, backup *solrv1beta1.SolrBackup, logger logr.Logger) (solrCloud *solrv1beta1.SolrCloud, collectionBackupsFinished bool, actionTaken bool, err error) {
+func (r *SolrBackupReconciler) reconcileSolrCloudBackup(ctx context.Context, backup *solrv1beta1.SolrBackup, logger logr.Logger) (solrCloud *solrv1beta1.SolrCloud, actionTaken bool, err error) {
 	// Get the solrCloud that this backup is for.
 	solrCloud = &solrv1beta1.SolrCloud{}
 
 	err = r.Get(ctx, types.NamespacedName{Namespace: backup.Namespace, Name: backup.Spec.SolrCloud}, solrCloud)
 	if err != nil && errors.IsNotFound(err) {
 		logger.Error(err, "Could not find cloud to backup", "solrCloud", backup.Spec.SolrCloud)
-		return nil, collectionBackupsFinished, actionTaken, err
+		return nil, actionTaken, err
 	} else if err != nil {
-		return nil, collectionBackupsFinished, actionTaken, err
+		return nil, actionTaken, err
 	}
 
 	// Add any additional values needed to Authn to Solr to the Context used when invoking the API
 	if solrCloud.Spec.SolrSecurity != nil {
 		ctx, err = util.AddAuthToContext(ctx, &r.Client, solrCloud.Spec.SolrSecurity, solrCloud.Namespace)
 		if err != nil {
-			return nil, collectionBackupsFinished, actionTaken, err
+			return nil, actionTaken, err
 		}
 	}
 
 	// First check if the collection backups have been completed
-	collectionBackupsFinished = util.CheckStatusOfCollectionBackups(backup)
+	collectionBackupsFinished := util.UpdateStatusOfCollectionBackups(backup)
 
 	// If the collectionBackups are complete, then nothing else has to be done here
 	if collectionBackupsFinished {
-		return solrCloud, collectionBackupsFinished, actionTaken, nil
+		return solrCloud, actionTaken, nil
 	}
 
 	actionTaken = true
@@ -181,7 +147,7 @@ func (r *SolrBackupReconciler) reconcileSolrCloudBackup(ctx context.Context, bac
 		err = fmt.Errorf("Unable to find backup repository to use for backup [%s] (which specified the repository"+
 			" [%s]).  solrcloud must define a repository matching that name (or have only 1 repository defined).",
 			backup.Name, backup.Spec.RepositoryName)
-		return solrCloud, collectionBackupsFinished, actionTaken, err
+		return solrCloud, actionTaken, err
 	}
 
 	// This should only occur before the backup processes have been started
@@ -189,7 +155,7 @@ func (r *SolrBackupReconciler) reconcileSolrCloudBackup(ctx context.Context, bac
 		// Prep the backup directory in the persistentVolume
 		err = util.EnsureDirectoryForBackup(solrCloud, backupRepository, backup, r.config)
 		if err != nil {
-			return solrCloud, collectionBackupsFinished, actionTaken, err
+			return solrCloud, actionTaken, err
 		}
 
 		// Make sure that all solr nodes are active and have the backupRestore shared volume mounted
@@ -197,7 +163,7 @@ func (r *SolrBackupReconciler) reconcileSolrCloudBackup(ctx context.Context, bac
 		cloudReady := solrCloud.Status.BackupRestoreReady && (solrCloud.Status.Replicas == solrCloud.Status.ReadyReplicas)
 		if !cloudReady {
 			logger.Info("Cloud not ready for backup backup", "solrCloud", solrCloud.Name)
-			return solrCloud, collectionBackupsFinished, actionTaken, errors.NewServiceUnavailable("Cloud is not ready for backups or restores")
+			return solrCloud, actionTaken, errors.NewServiceUnavailable("Cloud is not ready for backups or restores")
 		}
 
 		// Only set the solr version at the start of the backup. This shouldn't change throughout the backup.
@@ -206,13 +172,14 @@ func (r *SolrBackupReconciler) reconcileSolrCloudBackup(ctx context.Context, bac
 
 	// Go through each collection specified and reconcile the backup.
 	for _, collection := range backup.Spec.Collections {
+		// This will in-place update the CollectionBackupStatus in the backup object
 		_, err = reconcileSolrCollectionBackup(ctx, backup, solrCloud, backupRepository, collection, logger)
 	}
 
 	// First check if the collection backups have been completed
-	collectionBackupsFinished = util.CheckStatusOfCollectionBackups(backup)
+	util.UpdateStatusOfCollectionBackups(backup)
 
-	return solrCloud, collectionBackupsFinished, actionTaken, err
+	return solrCloud, actionTaken, err
 }
 
 func reconcileSolrCollectionBackup(ctx context.Context, backup *solrv1beta1.SolrBackup, solrCloud *solrv1beta1.SolrCloud, backupRepository *solrv1beta1.SolrBackupRepository, collection string, logger logr.Logger) (finished bool, err error) {
@@ -272,69 +239,6 @@ func reconcileSolrCollectionBackup(ctx context.Context, backup *solrv1beta1.Solr
 	return collectionBackupStatus.Finished, err
 }
 
-func (r *SolrBackupReconciler) persistSolrCloudBackups(ctx context.Context, backup *solrv1beta1.SolrBackup, solrCloud *solrv1beta1.SolrCloud, logger logr.Logger) (err error) {
-	if backup.Status.PersistenceStatus == nil {
-		backup.Status.PersistenceStatus = &solrv1beta1.BackupPersistenceStatus{}
-	}
-	if backup.Status.PersistenceStatus.Finished {
-		return nil
-	}
-	now := metav1.Now()
-
-	backupRepository := util.GetBackupRepositoryByName(solrCloud.Spec.BackupRepositories, backup.Spec.RepositoryName)
-	if backupRepository == nil {
-		err = fmt.Errorf("Unable to find backup repository to use for backup [%s] (which specified the repository"+
-			" [%s]).  solrcloud must define a repository matching that name (or have only 1 repository defined).",
-			backup.Name, backup.Spec.RepositoryName)
-		return err
-	}
-
-	if util.IsRepoManaged(backupRepository) {
-		persistenceJob := util.GenerateBackupPersistenceJobForCloud(backupRepository, backup, solrCloud)
-		if err := controllerutil.SetControllerReference(backup, persistenceJob, r.Scheme); err != nil {
-			return err
-		}
-
-		foundPersistenceJob := &batchv1.Job{}
-		err = r.Get(ctx, types.NamespacedName{Name: persistenceJob.Name, Namespace: persistenceJob.Namespace}, foundPersistenceJob)
-		if err == nil && !backup.Status.PersistenceStatus.InProgress {
-		} else if err != nil && errors.IsNotFound(err) {
-			logger.Info("Creating Persistence Job", "job", persistenceJob.Name)
-			err = r.Create(ctx, persistenceJob)
-			backup.Status.PersistenceStatus.InProgress = true
-			if backup.Status.PersistenceStatus.StartTime == nil {
-				backup.Status.PersistenceStatus.StartTime = &now
-			}
-		} else if err != nil {
-			return err
-		} else {
-			backup.Status.PersistenceStatus.FinishTime = foundPersistenceJob.Status.CompletionTime
-			tru := true
-			fals := false
-			numFailLimit := int32(0)
-			if foundPersistenceJob.Spec.BackoffLimit != nil {
-				numFailLimit = *foundPersistenceJob.Spec.BackoffLimit
-			}
-			if foundPersistenceJob.Status.Succeeded > 0 {
-				backup.Status.PersistenceStatus.Successful = &tru
-			} else if foundPersistenceJob.Status.Failed > numFailLimit {
-				backup.Status.PersistenceStatus.Successful = &fals
-			}
-
-			if backup.Status.PersistenceStatus.Successful != nil {
-				backup.Status.PersistenceStatus.InProgress = false
-				backup.Status.PersistenceStatus.Finished = true
-				backup.Status.PersistenceStatus.FinishTime = &now
-				backup.Status.Finished = true
-				backup.Status.Successful = backup.Status.PersistenceStatus.Successful
-			}
-		}
-		return err
-	} else {
-		return nil
-	}
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *SolrBackupReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	r.config = mgr.GetConfig()
diff --git a/controllers/util/backup_util.go b/controllers/util/backup_util.go
index 2670ca9..0119665 100644
--- a/controllers/util/backup_util.go
+++ b/controllers/util/backup_util.go
@@ -24,9 +24,7 @@ import (
 	solr "github.com/apache/solr-operator/api/v1beta1"
 	"github.com/apache/solr-operator/controllers/util/solr_api"
 	"github.com/go-logr/logr"
-	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -34,16 +32,6 @@ import (
 	"net/url"
 )
 
-const (
-	TarredFile       = "/var/solr/data/backup-restore/backup.tgz"
-	CleanupCommand   = " && rm -rf " + BaseBackupRestorePath + "/*"
-	BackupTarCommand = "cd " + BaseBackupRestorePath + " && tar -czf /tmp/backup.tgz * " + CleanupCommand + " && mv /tmp/backup.tgz " + TarredFile + " && chmod -R a+rwx " + TarredFile + " && cd - && "
-
-	AWSSecretDir = "/var/aws"
-
-	JobTTLSeconds = int32(60)
-)
-
 func GetBackupRepositoryByName(backupRepos []solr.SolrBackupRepository, repositoryName string) *solr.SolrBackupRepository {
 	// If no name is given and only 1 repo exists, return the repo
 	if repositoryName == "" && len(backupRepos) == 1 {
@@ -66,250 +54,23 @@ func AsyncIdForCollectionBackup(collection string, backupName string) string {
 	return fmt.Sprintf("%s-%s", backupName, collection)
 }
 
-func CheckStatusOfCollectionBackups(backup *solr.SolrBackup) (allFinished bool) {
-	fals := false
-
+func UpdateStatusOfCollectionBackups(backup *solr.SolrBackup) (allFinished bool) {
 	// Check if all collection backups have been completed, this is updated in the loop
 	allFinished = len(backup.Status.CollectionBackupStatuses) > 0
 
-	// Check if persistence should be skipped if no backup completed successfully
-	anySuccessful := false
+	allSuccessful := len(backup.Status.CollectionBackupStatuses) > 0
 
 	for _, collectionStatus := range backup.Status.CollectionBackupStatuses {
 		allFinished = allFinished && collectionStatus.Finished
-		anySuccessful = anySuccessful || (collectionStatus.Successful != nil && *collectionStatus.Successful)
+		allSuccessful = allSuccessful && (collectionStatus.Successful != nil && *collectionStatus.Successful)
 	}
-	if allFinished && !anySuccessful {
-		backup.Status.Finished = true
-		if backup.Status.Successful == nil {
-			backup.Status.Successful = &fals
-		}
+	backup.Status.Finished = allFinished
+	if allFinished && backup.Status.Successful == nil {
+		backup.Status.Successful = &allSuccessful
 	}
 	return
 }
 
-func GenerateBackupPersistenceJobForCloud(managedBackupRepository *solr.SolrBackupRepository, backup *solr.SolrBackup, solrCloud *solr.SolrCloud) *batchv1.Job {
-	backupVolume, _ := RepoVolumeSourceAndMount(managedBackupRepository, solrCloud.Name)
-	solrCloudBackupDirectoryOverride := managedBackupRepository.Managed.Directory
-	return GenerateBackupPersistenceJob(backup, backupVolume, BackupSubPathForCloud(solrCloudBackupDirectoryOverride, solrCloud.Name, backup.Name))
-}
-
-// GenerateBackupPersistenceJob creates a Job that will persist backup data and purge the backup from the solrBackupVolume
-func GenerateBackupPersistenceJob(solrBackup *solr.SolrBackup, solrBackupVolume *corev1.VolumeSource, backupSubPath string) *batchv1.Job {
-	copyLabels := solrBackup.GetLabels()
-	if copyLabels == nil {
-		copyLabels = map[string]string{}
-	}
-	labels := solrBackup.SharedLabelsWith(solrBackup.GetLabels())
-
-	// ttlSeconds := JobTTLSeconds
-
-	image, env, command, volume, volumeMount, numRetries := GeneratePersistenceOptions(solrBackup, solrBackupVolume)
-
-	volumes := []corev1.Volume{
-		{
-			Name:         "backup-data",
-			VolumeSource: *solrBackupVolume,
-		},
-	}
-	volumeMounts := []corev1.VolumeMount{
-		{
-			MountPath: BaseBackupRestorePath,
-			Name:      "backup-data",
-			SubPath:   backupSubPath,
-			ReadOnly:  false,
-		},
-	}
-	if volume != nil && volumeMount != nil {
-		volumes = append(volumes, *volume)
-		volumeMounts = append(volumeMounts, *volumeMount)
-	}
-
-	parallelismAndCompletions := int32(1)
-	solrGroup := int64(DefaultSolrGroup)
-	solrUser := int64(DefaultSolrUser)
-
-	job := &batchv1.Job{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      solrBackup.PersistenceJobName(),
-			Namespace: solrBackup.GetNamespace(),
-			Labels:    labels,
-		},
-		Spec: batchv1.JobSpec{
-			// TTLSecondsAfterFinished: &ttlSeconds,
-			BackoffLimit: numRetries,
-			Parallelism:  &parallelismAndCompletions,
-			Completions:  &parallelismAndCompletions,
-			Template: corev1.PodTemplateSpec{
-				ObjectMeta: metav1.ObjectMeta{
-					Labels: labels,
-				},
-				Spec: corev1.PodSpec{
-					Volumes: volumes,
-					Containers: []corev1.Container{
-						{
-							Name:            "backup-persistence",
-							Image:           image.ToImageName(),
-							ImagePullPolicy: image.PullPolicy,
-							VolumeMounts:    volumeMounts,
-							Env:             env,
-							Command:         command,
-						},
-					},
-					SecurityContext: &corev1.PodSecurityContext{
-						RunAsUser:  &solrUser,
-						RunAsGroup: &solrGroup,
-						FSGroup:    &solrGroup,
-					},
-					RestartPolicy: corev1.RestartPolicyNever,
-				},
-			},
-		},
-	}
-	return job
-}
-
-// GeneratePersistenceOptions creates options for a Job that will persist backup data
-func GeneratePersistenceOptions(solrBackup *solr.SolrBackup, solrBackupVolume *corev1.VolumeSource) (image solr.ContainerImage, envVars []corev1.EnvVar, command []string, volume *corev1.Volume, volumeMount *corev1.VolumeMount, numRetries *int32) {
-	// 'Persistence' expected to be non-nil
-	persistenceSource := solrBackup.Spec.Persistence
-	if persistenceSource.Volume != nil {
-		// Options for persisting to a volume
-		image = persistenceSource.Volume.BusyBoxImage
-		envVars = []corev1.EnvVar{
-			{
-				Name:  "FILE_NAME",
-				Value: persistenceSource.Volume.Filename,
-			},
-		}
-
-		finalLocation := BaseBackupRestorePath
-		// If the persistence volume is the same as the backup volume, we cannot mount the same volume twice.
-		if !DeepEqualWithNils(*solrBackupVolume, persistenceSource.Volume.VolumeSource) {
-			finalLocation = "/var/backup-persistence"
-			volume = &corev1.Volume{
-				Name:         "persistence",
-				VolumeSource: persistenceSource.Volume.VolumeSource,
-			}
-			volumeMount = &corev1.VolumeMount{
-				Name:      "persistence",
-				SubPath:   persistenceSource.Volume.Path,
-				ReadOnly:  false,
-				MountPath: finalLocation,
-			}
-		}
-		// Copy the information to the persistent storage, and delete it from the backup-restore volume.
-		command = []string{"sh", "-c", BackupTarCommand + "mv " + TarredFile + " \"" + finalLocation + "/${FILE_NAME}\""}
-
-		r := int32(1)
-		numRetries = &r
-	} else if persistenceSource.S3 != nil {
-		s3 := persistenceSource.S3
-		// Options for persisting to S3
-		image = s3.AWSCliImage
-		envVars = []corev1.EnvVar{
-			{
-				Name:  "BUCKET",
-				Value: s3.Bucket,
-			},
-			{
-				Name:  "KEY",
-				Value: s3.Key,
-			},
-			{
-				Name:  "ENDPOINT_URL",
-				Value: s3.EndpointUrl,
-			},
-		}
-		// Set up optional Environment variables
-		if s3.Region != "" {
-			envVars = append(envVars, corev1.EnvVar{
-				Name:  "AWS_DEFAULT_REGION",
-				Value: s3.Region,
-			})
-		}
-		if s3.Secrets.AccessKeyId != "" {
-			envVars = append(envVars, corev1.EnvVar{
-				Name: "AWS_ACCESS_KEY_ID",
-				ValueFrom: &corev1.EnvVarSource{
-					SecretKeyRef: &corev1.SecretKeySelector{
-						LocalObjectReference: corev1.LocalObjectReference{
-							Name: s3.Secrets.Name,
-						},
-						Key: s3.Secrets.AccessKeyId,
-					},
-				},
-			})
-		}
-		if s3.Secrets.SecretAccessKey != "" {
-			envVars = append(envVars, corev1.EnvVar{
-				Name: "AWS_SECRET_ACCESS_KEY",
-				ValueFrom: &corev1.EnvVarSource{
-					SecretKeyRef: &corev1.SecretKeySelector{
-						LocalObjectReference: corev1.LocalObjectReference{
-							Name: s3.Secrets.Name,
-						},
-						Key: s3.Secrets.SecretAccessKey,
-					},
-				},
-			})
-		}
-		if s3.Secrets.ConfigFile != "" {
-			envVars = append(envVars, corev1.EnvVar{
-				Name:  "AWS_CONFIG_FILE",
-				Value: AWSSecretDir + "/config",
-			})
-		}
-		if s3.Secrets.CredentialsFile != "" {
-			envVars = append(envVars, corev1.EnvVar{
-				Name:  "AWS_SHARED_CREDENTIALS_FILE",
-				Value: AWSSecretDir + "/credentials",
-			})
-		}
-
-		// If a config or credentials file is provided in the secrets, load them up in a volume
-		if s3.Secrets.ConfigFile != "" || s3.Secrets.CredentialsFile != "" {
-			readonly := int32(400)
-			volume = &corev1.Volume{
-				Name: "awsSecrets",
-				VolumeSource: corev1.VolumeSource{
-					Secret: &corev1.SecretVolumeSource{
-						SecretName: s3.Secrets.Name,
-						Items: []corev1.KeyToPath{
-							{
-								Key:  s3.Secrets.ConfigFile,
-								Path: "config",
-								Mode: &readonly,
-							},
-							{
-								Key:  s3.Secrets.CredentialsFile,
-								Path: "credentials",
-								Mode: &readonly,
-							},
-						},
-					},
-				},
-			}
-			volumeMount = &corev1.VolumeMount{
-				Name:      "awsSecrets",
-				ReadOnly:  true,
-				MountPath: AWSSecretDir,
-			}
-		}
-
-		// Only include the endpoint URL if it's provided
-		includeUrl := ""
-		if s3.EndpointUrl != "" {
-			includeUrl = "--endpoint-url \"${ENDPOINT_URL}\" "
-		}
-
-		command = []string{"sh", "-c", BackupTarCommand + "aws s3 cp " + includeUrl + TarredFile + " \"s3://${BUCKET}/${KEY}\""}
-		numRetries = persistenceSource.S3.Retries
-	}
-
-	return image, envVars, command, volume, volumeMount, numRetries
-}
-
 func GenerateQueryParamsForBackup(backupRepository *solr.SolrBackupRepository, backup *solr.SolrBackup, collection string) url.Values {
 	queryParams := url.Values{}
 	queryParams.Add("action", "BACKUP")
diff --git a/controllers/util/solr_api/api.go b/controllers/util/solr_api/api.go
index 51ce412..5e4baa1 100644
--- a/controllers/util/solr_api/api.go
+++ b/controllers/util/solr_api/api.go
@@ -105,7 +105,7 @@ func CallCollectionsApi(ctx context.Context, cloud *solr.SolrCloud, urlParams ur
 	}
 
 	if err == nil {
-		json.NewDecoder(resp.Body).Decode(&response)
+		err = json.NewDecoder(resp.Body).Decode(&response)
 	}
 
 	return err
diff --git a/docs/solr-backup/README.md b/docs/solr-backup/README.md
index e0e9bea..d5fa604 100644
--- a/docs/solr-backup/README.md
+++ b/docs/solr-backup/README.md
@@ -84,7 +84,7 @@ Now that there's a backup repository available to use, a backup can be triggered
 apiVersion: solr.apache.org/v1beta1
 kind: SolrBackup
 metadata:
-  name: local-backup-without-persistence
+  name: local-backup
   namespace: default
 spec:
   repositoryName: "local-collection-backups-1"
@@ -100,7 +100,7 @@ The status of our triggered backup can be checked with the command below.
 ```bash
 $ kubectl get solrbackups
 NAME                               CLOUD     FINISHED   SUCCESSFUL   AGE
-local-backup-without-persistence   example   true       true         72s
+local-backup   example   true       true         72s
 ```
 
 ## Deleting an example SolrBackup
@@ -108,7 +108,7 @@ local-backup-without-persistence   example   true       true         72s
 Once the operator completes a backup, the SolrBackup instance can be safely deleted.
 
 ```bash
-$ kubectl delete solrbackup local-backup-without-persistence
+$ kubectl delete solrbackup local-backup
 TODO command output
 ```
 
@@ -116,16 +116,17 @@ Note that deleting SolrBackup instances doesn't delete the backed up data, which
 In our example this data can still be found on the volume we created earlier
 
 ```bash
-$ kubectl exec example-solrcloud-0 -- ls -lh /var/solr/data/backup-restore-managed-local-collection-backups-1/backups/local-backup-without-persistence
+$ kubectl exec example-solrcloud-0 -- ls -lh /var/solr/data/backup-restore-managed-local-collection-backups-1/backups/
 total 8K
-drwxr-xr-x 3 solr solr 4.0K Sep 16 11:48 books
-drwxr-xr-x 3 solr solr 4.0K Sep 16 11:48 techproducts
+drwxr-xr-x 3 solr solr 4.0K Sep 16 11:48 local-backup-books
+drwxr-xr-x 3 solr solr 4.0K Sep 16 11:48 local-backup-techproducts
 ```
 
 Managed backup data, as in our example, can always be deleted using standard shell commands if desired:
 
 ```bash
-kubectl exec example-solrcloud-0 -- rm -r /var/solr/data/backup-restore-managed-local-collection-backups-1/backups/local-backup-without-persistence
+kubectl exec example-solrcloud-0 -- rm -r /var/solr/data/backup-restore/local-collection-backups-1/backups/local-backup-books
+kubectl exec example-solrcloud-0 -- rm -r /var/solr/data/backup-restore/local-collection-backups-1/backups/local-backup-techproducts
 ```
 
 ## Supported Repository Types
@@ -258,10 +259,6 @@ This may be addressed in future Solr versions, but for now use the same credenti
 _Since v0.5.0_
 
 Managed repositories store backup data "locally" on a Kubernetes volume mounted by each Solr pod.
-Managed repositories are so called because with the data stored in and managed by Kubernetes, the operator is able to offer a few advanced post-processing features that are unavailable for other repository types.
-
-The main example of this currently is the operator's "persistence" feature, which upon completion of the backup will compress the backup files and optionally relocate the archive to a more permanent volume.  See [the example here](../../example/test_backup_managed_with_persistence.yaml) for more details.
-
 An example of a SolrCloud spec with only one backup repository, with type Managed:
 
 ```yaml
diff --git a/docs/upgrade-notes.md b/docs/upgrade-notes.md
index 5428a53..57e67c9 100644
--- a/docs/upgrade-notes.md
+++ b/docs/upgrade-notes.md
@@ -123,6 +123,12 @@ _Note that the Helm chart version does not contain a `v` prefix, which the downl
   Because the backup name in Solr uses both the SolrBackup resource name and the collection name, there should be no collisions in this directory.
   However, this can be overridden using the `SolrBackup.spec.location` option.
 
+- The SolrBackup persistence option has been removed as of `v0.5.0`.
+  Users should plan to keep their backup data in the shared volume if using a MountedVolume Backup repository.
+  If `SolrBackup.spec.persistence` is provided, it will be removed and written back to Kubernetes.
+
+  Users using the S3 persistence option should try to use the [S3 backup repository](solr-backup/README.md#s3-backup-repositories) instead. This requires Solr 8.10 or higher.
+
 - Default ports when using TLS are now set to 443 instead of 80.
   This affects `solrCloud.Spec.SolrAddressability.CommonServicePort` and `solrCloud.Spec.SolrAddressability.CommonServicePort` field defaulting.
   Users already explicitly setting these values will not be affected.
diff --git a/example/test_backup_managed_with_persistence.yaml b/example/test_backup_managed_with_persistence.yaml
deleted file mode 100644
index c404a7f..0000000
--- a/example/test_backup_managed_with_persistence.yaml
+++ /dev/null
@@ -1,31 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one or more
-# contributor license agreements.  See the NOTICE file distributed with
-# this work for additional information regarding copyright ownership.
-# The ASF licenses this file to You under the Apache License, Version 2.0
-# (the "License"); you may not use this file except in compliance with
-# the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-# Assumes a 'solrcloud' whose 'backupRestoreOptions' match the "Multiple Backup Repositories" solrcloud example ('test_solrcloud_backuprepos.yaml')
-apiVersion: solr.apache.org/v1beta1
-kind: SolrBackup
-metadata:
-  name: local-backup-without-persistence
-  namespace: default
-spec:
-  repositoryName: "managed_repository_1"
-  solrCloud: multiple-backup-repositories-solr
-  collections:
-    - example
-  persistence:
-    volume:
-      source:
-        persistentVolumeClaim:
-          claimName: "pvc-test"
diff --git a/example/test_solrcloud_backuprepos.yaml b/example/test_solrcloud_backuprepos.yaml
index 498e07a..8de031c 100644
--- a/example/test_solrcloud_backuprepos.yaml
+++ b/example/test_solrcloud_backuprepos.yaml
@@ -20,7 +20,7 @@ metadata:
 spec:
   replicas: 1
   solrImage:
-    tag: 8.9.0
+    tag: 8.10
   backupRepositories:
     # "Managed" repositories store backup data in a Kubernetes volume, This
     # allows the operator to make several advanced features available, such as
@@ -48,7 +48,7 @@ spec:
 
     # Creates repositories that backup data to AWS S3.
     #
-    # Requires Solr >= 8.9.
+    # Requires Solr >= 8.10.
     - name: "s3-backup-repo"
       s3:
         region: "us-west-2"
diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml
index b33ca06..ae80f5b 100644
--- a/helm/solr-operator/Chart.yaml
+++ b/helm/solr-operator/Chart.yaml
@@ -162,6 +162,13 @@ annotations:
           url: https://github.com/apache/solr-operator/issues/277
         - name: Github PR
           url: https://github.com/apache/solr-operator/pull/363
+    - kind: removed
+      description: Removed "persistence" option for SolrBackups. Instead please use the S3 or GCP Backup Repositories (Solr 8.9+)
+      links:
+        - name: Github Issue
+          url: https://github.com/apache/solr-operator/issues/347
+        - name: Github PR
+          url: https://github.com/apache/solr-operator/pull/357
   artifacthub.io/images: |
     - name: solr-operator
       image: apache/solr-operator:v0.5.0-prerelease
diff --git a/helm/solr-operator/crds/crds.yaml b/helm/solr-operator/crds/crds.yaml
index 11a4fa1..409fca0 100644
--- a/helm/solr-operator/crds/crds.yaml
+++ b/helm/solr-operator/crds/crds.yaml
@@ -71,7 +71,7 @@ spec:
                 description: The location to store the backup in the specified backup repository.
                 type: string
               persistence:
-                description: Persistence is the specification on how to persist the backup data.
+                description: Persistence is the specification on how to persist the backup data. This feature has been removed as of v0.5.0. Any options specified here will not be used.
                 properties:
                   S3:
                     description: Persist to an s3 compatible endpoint
@@ -1106,7 +1106,7 @@ spec:
                 description: Whether the backup has finished
                 type: boolean
               persistenceStatus:
-                description: Whether the backups are in progress of being persisted
+                description: Whether the backups are in progress of being persisted. This feature has been removed as of v0.5.0.
                 properties:
                   finishTimestamp:
                     description: Time that the collection backup finished at
diff --git a/helm/solr-operator/templates/role.yaml b/helm/solr-operator/templates/role.yaml
index 0b29fc2..aca8ccd 100644
--- a/helm/solr-operator/templates/role.yaml
+++ b/helm/solr-operator/templates/role.yaml
@@ -139,24 +139,6 @@ rules:
   verbs:
   - get
 - apiGroups:
-  - batch
-  resources:
-  - jobs
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - patch
-  - update
-  - watch
-- apiGroups:
-  - batch
-  resources:
-  - jobs/status
-  verbs:
-  - get
-- apiGroups:
   - networking.k8s.io
   resources:
   - ingresses