Added backup routine removal flow

controllers/backup/reconciler.go

@@ -3,6 +3,7 @@ package backup
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -349,15 +350,15 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error {
 
 	r.Log.Info("Fetched backup service config", "config", config)
 
-	backupConfigMap := make(map[string]interface{})
+	specBackupConfigMap := make(map[string]interface{})
 
-	err = yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &backupConfigMap)
+	err = yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &specBackupConfigMap)
 	if err != nil {
 		return err
 	}
 
-	if backupConfigMap[common.AerospikeClusterKey] != nil {
-		cluster := backupConfigMap[common.AerospikeClusterKey].(map[string]interface{})
+	if specBackupConfigMap[common.AerospikeClusterKey] != nil {
+		cluster := specBackupConfigMap[common.AerospikeClusterKey].(map[string]interface{})
 
 		currentClusters, gErr := common.GetConfigSection(config, common.AerospikeClustersKey)
 		if gErr != nil {
@@ -367,9 +368,14 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error {
 		// TODO: Remove these API calls when hot reload is implemented
 		for name, clusterConfig := range cluster {
 			if _, ok := currentClusters[name]; ok {
-				err = serviceClient.PutCluster(name, clusterConfig)
-				if err != nil {
-					return err
+				// Only update if there is any change
+				if !reflect.DeepEqual(currentClusters[name], clusterConfig) {
+					r.Log.Info("Cluster config has been changed, updating it", "cluster", name)
+
+					err = serviceClient.PutCluster(name, clusterConfig)
+					if err != nil {
+						return err
+					}
 				}
 			} else {
 				err = serviceClient.AddCluster(name, clusterConfig)
@@ -380,8 +386,8 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error {
 		}
 	}
 
-	if backupConfigMap[common.BackupRoutinesKey] != nil {
-		routines := backupConfigMap[common.BackupRoutinesKey].(map[string]interface{})
+	if specBackupConfigMap[common.BackupRoutinesKey] != nil {
+		routines := specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{})
 
 		currentRoutines, gErr := common.GetConfigSection(config, common.BackupRoutinesKey)
 		if gErr != nil {
@@ -391,9 +397,14 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error {
 		// TODO: Remove these API calls when hot reload is implemented
 		for name, routine := range routines {
 			if _, ok := currentRoutines[name]; ok {
-				err = serviceClient.PutBackupRoutine(name, routine)
-				if err != nil {
-					return err
+				// Only update if there is any change
+				if !reflect.DeepEqual(currentRoutines[name], routine) {
+					r.Log.Info("Routine config has been changed, updating it", "routine", name)
+
+					err = serviceClient.PutBackupRoutine(name, routine)
+					if err != nil {
+						return err
+					}
 				}
 			} else {
 				err = serviceClient.AddBackupRoutine(name, routine)
@@ -404,6 +415,14 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error {
 		}
 	}
 
+	// If there are routines that are removed, unregister them
+	if len(r.aeroBackup.Status.Config.Raw) != 0 {
+		err = r.unregisterBackupRoutines(specBackupConfigMap, serviceClient)
+		if err != nil {
+			return err
+		}
+	}
+
 	// Apply the updated configuration for the changes to take effect
 	err = serviceClient.ApplyConfig()
 	if err != nil {
@@ -490,6 +509,32 @@ func (r *SingleBackupReconciler) unregisterBackup() error {
 	return nil
 }
 
+func (r *SingleBackupReconciler) unregisterBackupRoutines(
+	specBackupConfigMap map[string]interface{},
+	serviceClient *backup_service.Client,
+) error {
+	statusBackupConfigMap := make(map[string]interface{})
+
+	if err := yaml.Unmarshal(r.aeroBackup.Status.Config.Raw, &statusBackupConfigMap); err != nil {
+		return err
+	}
+
+	// Unregister the backup routines which are removed
+	for name := range statusBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) {
+		if _, ok := specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{})[name]; !ok {
+			r.Log.Info("Unregistering backup routine", "routine", name)
+
+			if err := serviceClient.DeleteBackupRoutine(name); err != nil {
+				return err
+			}
+
+			r.Log.Info("Unregistered backup routine", "routine", name)
+		}
+	}
+
+	return nil
+}
+
 func (r *SingleBackupReconciler) updateStatus() error {
 	r.aeroBackup.Status.BackupService = r.aeroBackup.Spec.BackupService
 	r.aeroBackup.Status.Config = r.aeroBackup.Spec.Config

test/backup/backup_test.go

@@ -282,6 +282,36 @@ var _ = Describe(
 				Expect(err).ToNot(HaveOccurred())
 			})
 
+			It("Should unregister backup-routines when removed from backup CR", func() {
+				backup, err = newBackup()
+				Expect(err).ToNot(HaveOccurred())
+				err = deployBackup(k8sClient, backup)
+				Expect(err).ToNot(HaveOccurred())
+
+				err = validateTriggeredBackup(k8sClient, backupServiceName, backupServiceNamespace, backup)
+				Expect(err).ToNot(HaveOccurred())
+
+				backup, err = getBackupObj(k8sClient, backup.Name, backup.Namespace)
+				Expect(err).ToNot(HaveOccurred())
+
+				By("Removing 1 backup-routine from backup CR")
+				backupConfig := getBackupConfigInMap()
+				routines := backupConfig[common.BackupRoutinesKey].(map[string]interface{})
+				delete(routines, "test-routine1")
+				backupConfig[common.BackupRoutinesKey] = routines
+
+				configBytes, mErr := json.Marshal(backupConfig)
+				Expect(mErr).ToNot(HaveOccurred())
+
+				backup.Spec.Config.Raw = configBytes
+
+				err = k8sClient.Update(testCtx, backup)
+				Expect(err).ToNot(HaveOccurred())
+
+				err = validateTriggeredBackup(k8sClient, backupServiceName, backupServiceNamespace, backup)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
 		})
 	},
 )

test/backup/test_utils.go

@@ -175,6 +175,14 @@ func getBackupConfigInMap() map[string]interface{} {
 				"source-cluster":     "test-cluster",
 				"storage":            "local",
 			},
+			"test-routine1": map[string]interface{}{
+				"backup-policy":      "test-policy",
+				"interval-cron":      "@daily",
+				"incr-interval-cron": "@hourly",
+				"namespaces":         []string{"test"},
+				"source-cluster":     "test-cluster",
+				"storage":            "local",
+			},
 		},
 	}
 }
@@ -271,22 +279,26 @@ func validateTriggeredBackup(k8sClient client.Client, backupServiceName, backupS
 
 	newCluster := desiredConfigMap[common.AerospikeClusterKey].(map[string]interface{})
 
-	for name := range newCluster {
-		if _, ok := config.AerospikeClusters[name]; !ok {
-			return fmt.Errorf("cluster %s not found in backup config", name)
+	for clusterName := range newCluster {
+		if _, ok := config.AerospikeClusters[clusterName]; !ok {
+			return fmt.Errorf("cluster %s not found in backup config", clusterName)
 		}
 	}
 
-	pkgLog.Info("Backup cluster info is found in backup config")
+	pkgLog.Info("Backup cluster info is found in backup service config")
 
 	routines := desiredConfigMap[common.BackupRoutinesKey].(map[string]interface{})
 
-	for name := range routines {
-		if _, ok := config.BackupRoutines[name]; !ok {
-			return fmt.Errorf("routine %s not found in backup config", name)
+	for routineName := range routines {
+		if _, ok := config.BackupRoutines[routineName]; !ok {
+			return fmt.Errorf("routine %s not found in backup service config", routineName)
 		}
 	}
 
+	if len(routines) != len(config.BackupRoutines) {
+		return fmt.Errorf("backup routine count mismatch")
+	}
+
 	pkgLog.Info("Backup routines info is found in backup config")
 
 	return nil

test/setup_test.go

@@ -0,0 +1,48 @@
+package test
+
+import (
+	goctx "context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/aerospike/aerospike-kubernetes-operator/controllers/common"
+)
+
+var _ = Describe(
+	"Backup Service Test", func() {
+
+		It("Should setup user RBAC", func() {
+			// Create SA for aerospike backup service
+			err := createServiceAccount(k8sClient, goctx.TODO(), common.AerospikeBackupService, namespace)
+			Expect(err).ToNot(HaveOccurred())
+
+			// Setup by user function
+			// test creating resource
+			// IN operator namespace
+			// Create aerospike-secret
+			// Create auth-secret (admin)
+			// Create auth-update (admin123)
+
+			// For test1
+			// Create aerospike-secret
+			// Create auth-secret (admin)
+
+			// For test2
+			// Create aerospike-secret
+			// Create auth-secret (admin)
+
+			// For aerospike
+			// Create aerospike-secret
+			// Create auth-secret (admin)
+
+			// For common
+			// Create namespace test1, test2, aerospike
+			// ServiceAccount: aerospike-cluster (operatorNs, test1, test2, aerospike)
+			// ClusterRole: aerospike-cluster
+			// ClusterRoleBinding: aerospike-cluster
+
+			err = setupByUser(k8sClient, goctx.TODO())
+			Expect(err).ToNot(HaveOccurred())
+		})
+	})

test/suite_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package test
 
 import (
-	goctx "context"
 	"testing"
 	"time"
 
@@ -37,7 +36,6 @@ import (
 	// +kubebuilder:scaffold:imports
 
 	asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
-	"github.com/aerospike/aerospike-kubernetes-operator/controllers/common"
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
@@ -91,39 +89,6 @@ var _ = BeforeSuite(
 		)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(k8sClient).NotTo(BeNil())
-
-		// Create SA for aerospike backup service
-		err = createServiceAccount(k8sClient, goctx.TODO(), common.AerospikeBackupService, namespace)
-		Expect(err).ToNot(HaveOccurred())
-
-		// Setup by user function
-		// test creating resource
-		// IN operator namespace
-		// Create aerospike-secret
-		// Create auth-secret (admin)
-		// Create auth-update (admin123)
-
-		// For test1
-		// Create aerospike-secret
-		// Create auth-secret (admin)
-
-		// For test2
-		// Create aerospike-secret
-		// Create auth-secret (admin)
-
-		// For aerospike
-		// Create aerospike-secret
-		// Create auth-secret (admin)
-
-		// For common
-		// Create namespace test1, test2, aerospike
-		// ServiceAccount: aerospike-cluster (operatorNs, test1, test2, aerospike)
-		// ClusterRole: aerospike-cluster
-		// ClusterRoleBinding: aerospike-cluster
-
-		// Need to create storageClass if not created already
-		err = setupByUser(k8sClient, goctx.TODO())
-		Expect(err).ToNot(HaveOccurred())
 	})
 
 var _ = AfterSuite(