EVEREST-1316 Backups repos reconciliation fix (#477)

controllers/common/helper.go

@@ -601,24 +601,24 @@ func IsDatabaseClusterRestoreRunning(
 	return false, nil
 }
 
-// GetBackupStorageIndexInPGBackrestRepo returns the index of the backup storage in the pgbackrest repo list.
-func GetBackupStorageIndexInPGBackrestRepo(
+// GetRepoNameByBackupStorage returns the name of the repo that corresponds to the given backup storage.
+func GetRepoNameByBackupStorage(
 	backupStorage *everestv1alpha1.BackupStorage,
 	repos []crunchyv1beta1.PGBackRestRepo,
-) int {
-	for idx, repo := range repos {
+) string {
+	for _, repo := range repos {
 		if repo.S3 != nil &&
 			repo.S3.Bucket == backupStorage.Spec.Bucket &&
 			repo.S3.Region == backupStorage.Spec.Region &&
 			repo.S3.Endpoint == backupStorage.Spec.EndpointURL {
-			return idx
+			return repo.Name
 		}
 
 		if repo.Azure != nil && repo.Azure.Container == backupStorage.Spec.Bucket {
-			return idx
+			return repo.Name
 		}
 	}
-	return -1
+	return ""
 }
 
 // HandleUpstreamClusterCleanup handles the cleanup of the psdmb objects.

controllers/databaseclusterbackup_controller.go

@@ -845,8 +845,8 @@ func (r *DatabaseClusterBackupReconciler) reconcilePG(
 
 	// If the backup storage is not defined in the PerconaPGCluster CR, we
 	// cannot proceed
-	repoIdx := common.GetBackupStorageIndexInPGBackrestRepo(backupStorage, pgDBCR.Spec.Backups.PGBackRest.Repos)
-	if repoIdx == -1 {
+	repoName := common.GetRepoNameByBackupStorage(backupStorage, pgDBCR.Spec.Backups.PGBackRest.Repos)
+	if repoName == "" {
 		return false, ErrBackupStorageUndefined
 	}
 
@@ -865,7 +865,7 @@ func (r *DatabaseClusterBackupReconciler) reconcilePG(
 				BlockOwnerDeletion: pointer.ToBool(true),
 			}})
 
-			pgCR.Spec.RepoName = pgDBCR.Spec.Backups.PGBackRest.Repos[repoIdx].Name
+			pgCR.Spec.RepoName = repoName
 			pgCR.Spec.Options = []string{
 				"--type=full",
 			}

controllers/databaseclusterrestore_controller.go

@@ -465,8 +465,8 @@ func (r *DatabaseClusterRestoreReconciler) restorePG(ctx context.Context, restor
 	// PerconaPGCluster CR. If not, we requeue the restore to give the
 	// DatabaseCluster controller a chance to update the PG cluster CR.
 	// Otherwise, the restore will fail.
-	repoIdx := common.GetBackupStorageIndexInPGBackrestRepo(backupStorage, pgDBCR.Spec.Backups.PGBackRest.Repos)
-	if repoIdx == -1 {
+	repoName := common.GetRepoNameByBackupStorage(backupStorage, pgDBCR.Spec.Backups.PGBackRest.Repos)
+	if repoName == "" {
 		logger.Info(
 			fmt.Sprintf("Backup storage %s is not defined in the pg cluster %s, requeuing",
 				backupStorageName,
@@ -486,7 +486,7 @@ func (r *DatabaseClusterRestoreReconciler) restorePG(ctx context.Context, restor
 	}
 	_, err = controllerutil.CreateOrUpdate(ctx, r.Client, pgCR, func() error {
 		pgCR.Spec.PGCluster = restore.Spec.DBClusterName
-		pgCR.Spec.RepoName = pgDBCR.Spec.Backups.PGBackRest.Repos[repoIdx].Name
+		pgCR.Spec.RepoName = repoName
 		pgCR.Spec.Options, err = getPGRestoreOptions(restore.Spec.DataSource, backupBaseName)
 		return err
 	})

controllers/providers/pg/pg_repos_reconciler.go

@@ -20,6 +20,7 @@ import (
 	"container/list"
 	"errors"
 	"fmt"
+	"sort"
 	"strconv"
 
 	"github.com/AlekSi/pointer"
@@ -151,9 +152,34 @@ func (p *pgReposReconciler) reconcileBackups(
 			return fmt.Errorf("unknown backup storage %s", backup.Spec.BackupStorageName)
 		}
 
-		repoName, err := p.extractFirstAvailableRepoName()
-		if err != nil {
-			return err
+		// try to find the appropriate repo in the existing repos
+		var repoName string
+		var erNext *list.Element
+		for er := p.reposToBeReconciled.Front(); er != nil; er = erNext {
+			// Save the next element because we might remove the current one
+			erNext = er.Next()
+
+			repo, ok := er.Value.(crunchyv1beta1.PGBackRestRepo)
+			if !ok {
+				return fmt.Errorf("failed to cast repo %v", er.Value)
+			}
+			repoBackupStorageName := backupStorageNameFromRepo(backupStorages, repo)
+			if backup.Spec.BackupStorageName != repoBackupStorageName {
+				continue
+			}
+			repoName = repo.Name
+			p.removeAvailableRepoName(repo.Name)
+			// remove the repo from the reconciled list bc it will be re-added below with the schedule string
+			p.reposReconciled.Remove(er)
+			break
+		}
+		// if there is no repo for this backup - create a new one
+		if repoName == "" {
+			var err error
+			repoName, err = p.extractFirstAvailableRepoName()
+			if err != nil {
+				return err
+			}
 		}
 
 		repo, err := genPGBackrestRepo(repoName, backupStorage, nil)
@@ -279,7 +305,7 @@ func (p *pgReposReconciler) reconcileRepos(
 	return nil
 }
 
-func (p *pgReposReconciler) addNewSchedules(
+func (p *pgReposReconciler) addNewSchedules( //nolint:gocognit
 	backupStorages map[string]everestv1alpha1.BackupStorageSpec,
 	backupStoragesSecrets map[string]*corev1.Secret,
 	db *everestv1alpha1.DatabaseCluster,
@@ -295,9 +321,53 @@ func (p *pgReposReconciler) addNewSchedules(
 			return fmt.Errorf("unknown backup storage %s", backupSchedule.BackupStorageName)
 		}
 
-		repoName, err := p.extractFirstAvailableRepoName()
-		if err != nil {
-			return err
+		// try to reuse the repos already used by backups
+		var repoName string
+		var erNext *list.Element
+		for er := p.reposToBeReconciled.Front(); er != nil; er = erNext {
+			// Save the next element because we might remove the current one
+			erNext = er.Next()
+
+			repo, ok := er.Value.(crunchyv1beta1.PGBackRestRepo)
+			if !ok {
+				return fmt.Errorf("failed to cast repo %v", er.Value)
+			}
+			repoBackupStorageName := backupStorageNameFromRepo(backupStorages, repo)
+			if backupSchedule.BackupStorageName != repoBackupStorageName ||
+				repo.BackupSchedules != nil {
+				continue
+			}
+			repoName = repo.Name
+			p.removeAvailableRepoName(repo.Name)
+			break
+		}
+		// check the freshly reconciled repos if there is already such a repo there
+		for er := p.reposReconciled.Front(); er != nil; er = erNext {
+			// Save the next element because we might remove the current one
+			erNext = er.Next()
+
+			repo, ok := er.Value.(crunchyv1beta1.PGBackRestRepo)
+			if !ok {
+				return fmt.Errorf("failed to cast repo %v", er.Value)
+			}
+			repoBackupStorageName := backupStorageNameFromRepo(backupStorages, repo)
+			if backupSchedule.BackupStorageName != repoBackupStorageName ||
+				repo.BackupSchedules != nil {
+				continue
+			}
+			repoName = repo.Name
+			p.removeAvailableRepoName(repo.Name)
+			// remove the repo from the reconciled list bc we need to update it by adding a schedule
+			p.reposReconciled.Remove(er)
+			break
+		}
+		// if no existing repos can be reused - create a new one
+		if repoName == "" {
+			var err error
+			repoName, err = p.extractFirstAvailableRepoName()
+			if err != nil {
+				return err
+			}
 		}
 
 		repo, err := genPGBackrestRepo(repoName, backupStorage, &backupSchedule.Schedule)
@@ -360,9 +430,16 @@ func (p *pgReposReconciler) addDefaultRepo(engineStorage everestv1alpha1.Storage
 		}
 		newRepos = append(newRepos, repo)
 	}
+	sortByName(newRepos)
 	return newRepos, nil
 }
 
+func sortByName(repos []crunchyv1beta1.PGBackRestRepo) {
+	sort.Slice(repos, func(i, j int) bool {
+		return repos[i].Name < repos[j].Name
+	})
+}
+
 func updatePGIni(
 	pgBackRestSecretIni *ini.File,
 	secret *corev1.Secret,