diff --git a/Makefile b/Makefile index 646dcc44..ec6a8721 100644 --- a/Makefile +++ b/Makefile @@ -142,29 +142,25 @@ go-lint: golanci-lint ## Run golangci-lint against code. .PHONY: all-test all-test: manifests generate fmt vet envtest ## Run tests. - # KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -r --keep-going -coverprofile cover.out -show-node-events -v -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -coverprofile cover.out -r --keep-going -show-node-events -v -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} .PHONY: cluster-test cluster-test: manifests generate fmt vet envtest ## Run tests. - # KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -v . ./cluster -coverprofile cover.out -show-node-events -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -coverprofile cover.out --keep-separate-coverprofiles -v . ./cluster -show-node-events -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} .PHONY: backup-service-test backup-service-test: manifests generate fmt vet envtest ## Run tests. - # KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -v . ./backup_service -coverprofile cover.out -show-node-events -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -coverprofile cover.out --keep-separate-coverprofiles -v . ./backup_service -show-node-events -timeout=1h0m0s --junit-report="junit.xml" -- ${ARGS} .PHONY: backup-test backup-test: manifests generate fmt vet envtest ## Run tests. - # KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -v . ./backup -coverprofile cover.out -show-node-events -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -coverprofile cover.out --keep-separate-coverprofiles -v . ./backup -show-node-events -timeout=1h0m0s --junit-report="junit.xml" -- ${ARGS} .PHONY: restore-test restore-test: manifests generate fmt vet envtest ## Run tests. # KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -v . ./restore -coverprofile cover.out -show-node-events -timeout=12h0m0s --junit-report="junit.xml" -- ${ARGS} + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" cd $(shell pwd)/test; go run github.com/onsi/ginkgo/v2/ginkgo -coverprofile cover.out --keep-separate-coverprofiles -v . ./restore -show-node-events -timeout=1h0m0s --junit-report="junit.xml" -- ${ARGS} ##@ Build diff --git a/api/v1beta1/aerospikebackup_types.go b/api/v1beta1/aerospikebackup_types.go index 1bc37439..a54928a9 100644 --- a/api/v1beta1/aerospikebackup_types.go +++ b/api/v1beta1/aerospikebackup_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -37,7 +39,7 @@ type AerospikeBackupSpec struct { // OnDemandBackups is the configuration for on-demand backups. // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="On Demand Backups" // +kubebuilder:validation:MaxItems:=1 - OnDemandBackups []OnDemandBackupSpec `json:"OnDemandBackups,omitempty"` + OnDemandBackups []OnDemandBackupSpec `json:"onDemandBackups,omitempty"` } type BackupService struct { @@ -48,6 +50,10 @@ type BackupService struct { Namespace string `json:"namespace"` } +func (b *BackupService) String() string { + return fmt.Sprintf("%s/%s", b.Namespace, b.Name) +} + type OnDemandBackupSpec struct { // ID is the unique identifier for the on-demand backup. // +kubebuilder:validation:MinLength=1 @@ -70,7 +76,7 @@ type AerospikeBackupStatus struct { Config runtime.RawExtension `json:"config"` // OnDemandBackups is the configuration for on-demand backups. - OnDemandBackups []OnDemandBackupSpec `json:"OnDemandBackups,omitempty"` + OnDemandBackups []OnDemandBackupSpec `json:"onDemandBackups,omitempty"` // TODO: finalize the status and phase } diff --git a/api/v1beta1/aerospikebackup_webhook.go b/api/v1beta1/aerospikebackup_webhook.go index 05409598..8d114b62 100644 --- a/api/v1beta1/aerospikebackup_webhook.go +++ b/api/v1beta1/aerospikebackup_webhook.go @@ -64,7 +64,7 @@ func (r *AerospikeBackup) ValidateCreate() (admission.Warnings, error) { aerospikeBackupLog.Info("validate create", "name", r.Name) if len(r.Spec.OnDemandBackups) != 0 && r.Spec.Config.Raw != nil { - return nil, fmt.Errorf("onDemand and backup config cannot be specified together while creating backup") + return nil, fmt.Errorf("onDemand backups config cannot be specified while creating backup") } if err := r.validateBackupConfig(); err != nil { @@ -108,25 +108,6 @@ func (r *AerospikeBackup) ValidateDelete() (admission.Warnings, error) { } func (r *AerospikeBackup) validateBackupConfig() error { - var backupSvc AerospikeBackupService - - cl, gErr := getK8sClient() - if gErr != nil { - return gErr - } - - if err := cl.Get(context.TODO(), - types.NamespacedName{Name: r.Spec.BackupService.Name, Namespace: r.Spec.BackupService.Namespace}, - &backupSvc); err != nil { - return err - } - - var backupSvcConfig model.Config - - if err := yaml.Unmarshal(backupSvc.Spec.Config.Raw, &backupSvcConfig); err != nil { - return err - } - backupConfigMap := make(map[string]interface{}) if err := yaml.Unmarshal(r.Spec.Config.Raw, &backupConfigMap); err != nil { @@ -149,6 +130,25 @@ func (r *AerospikeBackup) validateBackupConfig() error { return fmt.Errorf("secret-agent field cannot be specified in backup config") } + var backupSvc AerospikeBackupService + + cl, gErr := getK8sClient() + if gErr != nil { + return gErr + } + + if err := cl.Get(context.TODO(), + types.NamespacedName{Name: r.Spec.BackupService.Name, Namespace: r.Spec.BackupService.Namespace}, + &backupSvc); err != nil { + return err + } + + var backupSvcConfig model.Config + + if err := yaml.Unmarshal(backupSvc.Spec.Config.Raw, &backupSvcConfig); err != nil { + return err + } + aeroClusters, err := r.validateAerospikeCluster(&backupSvcConfig, backupConfigMap) if err != nil { return err @@ -207,13 +207,13 @@ func getK8sClient() (client.Client, error) { } func (r *AerospikeBackup) validateAerospikeCluster(backupSvcConfig *model.Config, - backupSvcConfigMap map[string]interface{}, + backupConfigMap map[string]interface{}, ) (map[string]*model.AerospikeCluster, error) { - if _, ok := backupSvcConfigMap[common.AerospikeClusterKey]; !ok { - return nil, fmt.Errorf("aerospike-cluster field is required in backupSvcConfig") + if _, ok := backupConfigMap[common.AerospikeClusterKey]; !ok { + return nil, fmt.Errorf("aerospike-cluster field is required field in backup config") } - cluster, ok := backupSvcConfigMap[common.AerospikeClusterKey].(map[string]interface{}) + cluster, ok := backupConfigMap[common.AerospikeClusterKey].(map[string]interface{}) if !ok { return nil, fmt.Errorf("aerospike-cluster field is not in the right format") } @@ -238,7 +238,7 @@ func (r *AerospikeBackup) validateAerospikeCluster(backupSvcConfig *model.Config } for clusterName, aeroCluster := range aeroClusters { - if err := validateName(r.namePrefix(), clusterName); err != nil { + if err := validateName(r.NamePrefix(), clusterName); err != nil { return nil, fmt.Errorf("invalid cluster name %s, %s", clusterName, err.Error()) } @@ -249,6 +249,7 @@ func (r *AerospikeBackup) validateAerospikeCluster(backupSvcConfig *model.Config } func (r *AerospikeBackup) validateOnDemandBackupsUpdate(oldObj *AerospikeBackup) error { + // Check if backup config is updated along with onDemand backup add/update if !reflect.DeepEqual(r.Spec.OnDemandBackups, r.Status.OnDemandBackups) && !reflect.DeepEqual(r.Spec.Config.Raw, r.Status.Config.Raw) { return fmt.Errorf("can not add/update onDemand backup along with backup config change") @@ -300,7 +301,7 @@ func (r *AerospikeBackup) validateBackupRoutines(backupSvcConfig *model.Config, aeroClusters map[string]*model.AerospikeCluster, ) error { if _, ok := backupSvcConfigMap[common.BackupRoutinesKey]; !ok { - return fmt.Errorf("backup-routines field is required in backup onfig") + return fmt.Errorf("backup-routines field is required in backup config") } routines, ok := backupSvcConfigMap[common.BackupRoutinesKey].(map[string]interface{}) @@ -327,7 +328,7 @@ func (r *AerospikeBackup) validateBackupRoutines(backupSvcConfig *model.Config, // 1. if the correct format name is given // 2. if only correct aerospike cluster (the one referred in Backup CR) is used in backup routines for routineName, routine := range backupRoutines { - if err := validateName(r.namePrefix(), routineName); err != nil { + if err := validateName(r.NamePrefix(), routineName); err != nil { return fmt.Errorf("invalid backup routine name %s, %s", routineName, err.Error()) } @@ -347,7 +348,7 @@ func (r *AerospikeBackup) validateBackupRoutines(backupSvcConfig *model.Config, return nil } -func (r *AerospikeBackup) namePrefix() string { +func (r *AerospikeBackup) NamePrefix() string { return r.Namespace + "-" + r.Name } diff --git a/api/v1beta1/aerospikerestore_webhook.go b/api/v1beta1/aerospikerestore_webhook.go index e47d24d8..cbafa642 100644 --- a/api/v1beta1/aerospikerestore_webhook.go +++ b/api/v1beta1/aerospikerestore_webhook.go @@ -106,11 +106,11 @@ func (r *AerospikeRestore) validateRestoreConfig() error { var restoreRequest model.RestoreRequest if _, ok := restoreConfigInMap[common.RoutineKey]; ok { - return fmt.Errorf("routine key is not allowed in restore config for restore type %s", r.Spec.Type) + return fmt.Errorf("routine field is not allowed in restore config for restore type %s", r.Spec.Type) } if _, ok := restoreConfigInMap[common.TimeKey]; ok { - return fmt.Errorf("time key is not allowed in restore config for restore type %s", r.Spec.Type) + return fmt.Errorf("time field is not allowed in restore config for restore type %s", r.Spec.Type) } if err := yaml.Unmarshal(r.Spec.Config.Raw, &restoreRequest); err != nil { @@ -123,7 +123,7 @@ func (r *AerospikeRestore) validateRestoreConfig() error { var restoreRequest model.RestoreTimestampRequest if _, ok := restoreConfigInMap[common.SourceKey]; ok { - return fmt.Errorf("source key is not allowed in restore config for restore type %s", r.Spec.Type) + return fmt.Errorf("source field is not allowed in restore config for restore type %s", r.Spec.Type) } if err := yaml.Unmarshal(r.Spec.Config.Raw, &restoreRequest); err != nil { diff --git a/config/crd/bases/asdb.aerospike.com_aerospikebackups.yaml b/config/crd/bases/asdb.aerospike.com_aerospikebackups.yaml index 098a0f72..4bb22847 100644 --- a/config/crd/bases/asdb.aerospike.com_aerospikebackups.yaml +++ b/config/crd/bases/asdb.aerospike.com_aerospikebackups.yaml @@ -45,28 +45,6 @@ spec: description: AerospikeBackupSpec defines the desired state of AerospikeBackup for a given AerospikeCluster properties: - OnDemandBackups: - description: OnDemandBackups is the configuration for on-demand backups. - items: - properties: - delay: - description: Delay is the interval before starting the on-demand - backup. - type: string - id: - description: ID is the unique identifier for the on-demand backup. - minLength: 1 - type: string - routineName: - description: RoutineName is the routine name used to trigger - on-demand backup. - type: string - required: - - id - - routineName - type: object - maxItems: 1 - type: array backupService: description: BackupService is the backup service reference i.e. name and namespace. It is used to communicate to the backup service to @@ -88,14 +66,7 @@ spec: aerospike-cluster, backup-routines.' type: object x-kubernetes-preserve-unknown-fields: true - required: - - backupService - - config - type: object - status: - description: AerospikeBackupStatus defines the observed state of AerospikeBackup - properties: - OnDemandBackups: + onDemandBackups: description: OnDemandBackups is the configuration for on-demand backups. items: properties: @@ -115,7 +86,15 @@ spec: - id - routineName type: object + maxItems: 1 type: array + required: + - backupService + - config + type: object + status: + description: AerospikeBackupStatus defines the observed state of AerospikeBackup + properties: backupService: description: BackupService is the backup service reference i.e. name and namespace. @@ -136,6 +115,27 @@ spec: backup-routines.' type: object x-kubernetes-preserve-unknown-fields: true + onDemandBackups: + description: OnDemandBackups is the configuration for on-demand backups. + items: + properties: + delay: + description: Delay is the interval before starting the on-demand + backup. + type: string + id: + description: ID is the unique identifier for the on-demand backup. + minLength: 1 + type: string + routineName: + description: RoutineName is the routine name used to trigger + on-demand backup. + type: string + required: + - id + - routineName + type: object + type: array required: - backupService - config diff --git a/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml b/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml index 0f42acb2..3d5c7bc3 100644 --- a/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml @@ -25,9 +25,6 @@ spec: kind: AerospikeBackup name: aerospikebackups.asdb.aerospike.com specDescriptors: - - description: OnDemandBackups is the configuration for on-demand backups. - displayName: On Demand Backups - path: OnDemandBackups - description: BackupService is the backup service reference i.e. name and namespace. It is used to communicate to the backup service to trigger backups. This field is immutable @@ -38,6 +35,9 @@ spec: backup-routines.' displayName: Backup Config path: config + - description: OnDemandBackups is the configuration for on-demand backups. + displayName: On Demand Backups + path: onDemandBackups version: v1beta1 - description: AerospikeBackupService is the Schema for the aerospikebackupservices API diff --git a/config/samples/aerospikebackup.yaml b/config/samples/aerospikebackup.yaml index 0d1919a8..c0a743fa 100644 --- a/config/samples/aerospikebackup.yaml +++ b/config/samples/aerospikebackup.yaml @@ -15,7 +15,7 @@ spec: namespace: aerospike # onDemandBackups: # - id: first-ad-hoc-backup -# routineName: test-routine +# routineName: aerospike-aerospikebackup-test-routine config: aerospike-cluster: aerospike-aerospikebackup-test-cluster: diff --git a/controllers/backup/reconciler.go b/controllers/backup/reconciler.go index ab24da8e..492da642 100644 --- a/controllers/backup/reconciler.go +++ b/controllers/backup/reconciler.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "strings" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -113,33 +114,27 @@ func (r *SingleBackupReconciler) removeFinalizer(finalizerName string) error { } func (r *SingleBackupReconciler) reconcileConfigMap() error { - cm := &corev1.ConfigMap{} - - if err := r.Client.Get(context.TODO(), - types.NamespacedName{ - Namespace: r.aeroBackup.Spec.BackupService.Namespace, - Name: r.aeroBackup.Spec.BackupService.Name, - }, cm, - ); err != nil { - return fmt.Errorf("backup Service configMap not found, name: %s namespace: %s", - r.aeroBackup.Spec.BackupService.Name, r.aeroBackup.Spec.BackupService.Namespace) + cm, err := r.getBackupSvcConfigMap() + if err != nil { + return fmt.Errorf("backup Service configMap not found, name: %s", + r.aeroBackup.Spec.BackupService.String()) } r.Log.Info("Updating existing ConfigMap for Backup", - "name", r.aeroBackup.Spec.BackupService.Name, - "namespace", r.aeroBackup.Spec.BackupService.Namespace, + "name", r.aeroBackup.Spec.BackupService.String(), ) - specBackupConfigMap := make(map[string]interface{}) - backupSvcConfigMap := make(map[string]interface{}) - - if err := yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &specBackupConfigMap); err != nil { + specBackupConfigMap, err := r.getBackupConfigInMap() + if err != nil { return err } + backupSvcConfigMap := make(map[string]interface{}) + data := cm.Data[common.BackupServiceConfigYAML] - if err := yaml.Unmarshal([]byte(data), &backupSvcConfigMap); err != nil { + err = yaml.Unmarshal([]byte(data), &backupSvcConfigMap) + if err != nil { return err } @@ -152,10 +147,14 @@ func (r *SingleBackupReconciler) reconcileConfigMap() error { return fmt.Errorf("aerospike-clusters is not a map") } - newCluster := specBackupConfigMap[common.AerospikeClusterKey].(map[string]interface{}) + cluster := specBackupConfigMap[common.AerospikeClusterKey].(map[string]interface{}) + + var clusterName string - for name, cluster := range newCluster { - clusterMap[name] = cluster + // There will always be only one cluster in the backup config + for name, clusterInfo := range cluster { + clusterName = name + clusterMap[name] = clusterInfo } backupSvcConfigMap[common.AerospikeClustersKey] = clusterMap @@ -169,25 +168,18 @@ func (r *SingleBackupReconciler) reconcileConfigMap() error { return fmt.Errorf("backup-routines is not a map") } - newRoutines := specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) - - for name, routine := range newRoutines { - routineMap[name] = routine - } + routines := specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) // Remove the routines which are not in spec - if len(r.aeroBackup.Status.Config.Raw) != 0 { - statusBackupConfigMap := make(map[string]interface{}) + routinesToBeDeleted := r.routinesToDelete(routines, routineMap, clusterName) - if err := yaml.Unmarshal(r.aeroBackup.Status.Config.Raw, &statusBackupConfigMap); err != nil { - return err - } + for idx := range routinesToBeDeleted { + delete(routineMap, routinesToBeDeleted[idx]) + } - for name := range statusBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) { - if _, ok := specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{})[name]; !ok { - delete(routineMap, name) - } - } + // Add/update spec routines + for name, routine := range routines { + routineMap[name] = routine } backupSvcConfigMap[common.BackupRoutinesKey] = routineMap @@ -203,29 +195,24 @@ func (r *SingleBackupReconciler) reconcileConfigMap() error { context.TODO(), cm, common.UpdateOption, ); err != nil { return fmt.Errorf( - "failed to update Backup Service ConfigMap: %v", - err, + "failed to update Backup Service ConfigMap, name: %s, error %v", + r.aeroBackup.Spec.BackupService.String(), err, ) } r.Log.Info("Updated Backup Service ConfigMap for Backup", - "name", r.aeroBackup.Spec.BackupService.Name, - "namespace", r.aeroBackup.Spec.BackupService.Namespace, + "name", r.aeroBackup.Spec.BackupService.String(), ) return nil } func (r *SingleBackupReconciler) removeBackupInfoFromConfigMap() error { - cm := &corev1.ConfigMap{} - - if err := r.Client.Get(context.TODO(), - types.NamespacedName{ - Namespace: r.aeroBackup.Spec.BackupService.Namespace, - Name: r.aeroBackup.Spec.BackupService.Name, - }, cm, - ); err != nil { + cm, err := r.getBackupSvcConfigMap() + if err != nil { if errors.IsNotFound(err) { + r.Log.Info("Backup Service ConfigMap not found, skip updating", + "name", r.aeroBackup.Spec.BackupService.String()) return nil } @@ -233,46 +220,50 @@ func (r *SingleBackupReconciler) removeBackupInfoFromConfigMap() error { } r.Log.Info("Removing Backup info from existing ConfigMap", - "name", r.aeroBackup.Spec.BackupService.Name, - "namespace", r.aeroBackup.Spec.BackupService.Namespace, + "name", r.aeroBackup.Spec.BackupService.String(), ) - backupDataMap := make(map[string]interface{}) - cmDataMap := make(map[string]interface{}) - - if err := yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &backupDataMap); err != nil { + specBackupConfigMap, err := r.getBackupConfigInMap() + if err != nil { return err } + backupSvcConfigMap := make(map[string]interface{}) + data := cm.Data[common.BackupServiceConfigYAML] - if err := yaml.Unmarshal([]byte(data), &cmDataMap); err != nil { + err = yaml.Unmarshal([]byte(data), &backupSvcConfigMap) + if err != nil { return err } - if clusterIface, ok := cmDataMap[common.AerospikeClustersKey]; ok { + var clusterName string + + if clusterIface, ok := backupSvcConfigMap[common.AerospikeClustersKey]; ok { if clusterMap, ok := clusterIface.(map[string]interface{}); ok { - currentCluster := backupDataMap[common.AerospikeClusterKey].(map[string]interface{}) + currentCluster := specBackupConfigMap[common.AerospikeClusterKey].(map[string]interface{}) for name := range currentCluster { + clusterName = name delete(clusterMap, name) } - cmDataMap[common.AerospikeClustersKey] = clusterMap + backupSvcConfigMap[common.AerospikeClustersKey] = clusterMap } } - if routineIface, ok := cmDataMap[common.BackupRoutinesKey]; ok { + if routineIface, ok := backupSvcConfigMap[common.BackupRoutinesKey]; ok { if routineMap, ok := routineIface.(map[string]interface{}); ok { - currentRoutines := backupDataMap[common.BackupRoutinesKey].(map[string]interface{}) - for name := range currentRoutines { - delete(routineMap, name) + routinesToBeDelete := r.routinesToDelete(nil, routineMap, clusterName) + + for idx := range routinesToBeDelete { + delete(routineMap, routinesToBeDelete[idx]) } - cmDataMap[common.BackupRoutinesKey] = routineMap + backupSvcConfigMap[common.BackupRoutinesKey] = routineMap } } - updatedConfig, err := yaml.Marshal(cmDataMap) + updatedConfig, err := yaml.Marshal(backupSvcConfigMap) if err != nil { return err } @@ -283,20 +274,21 @@ func (r *SingleBackupReconciler) removeBackupInfoFromConfigMap() error { context.TODO(), cm, common.UpdateOption, ); err != nil { return fmt.Errorf( - "failed to update Backup Service ConfigMap: %v", - err, + "failed to update Backup Service ConfigMap, name: %s, error %v", + r.aeroBackup.Spec.BackupService.String(), err, ) } r.Log.Info("Removed Backup info from existing ConfigMap", - "name", r.aeroBackup.Spec.BackupService.Name, - "namespace", r.aeroBackup.Spec.BackupService.Namespace, + "name", r.aeroBackup.Spec.BackupService.String(), ) return nil } func (r *SingleBackupReconciler) scheduleOnDemandBackup() error { + r.Log.Info("Reconciling on-demand backup") + // There can be only one on-demand backup allowed right now. if len(r.aeroBackup.Status.OnDemandBackups) > 0 && r.aeroBackup.Spec.OnDemandBackups[0].ID == r.aeroBackup.Status.OnDemandBackups[0].ID { @@ -322,6 +314,8 @@ func (r *SingleBackupReconciler) scheduleOnDemandBackup() error { r.Log.Info("Scheduled on-demand backup", "ID", r.aeroBackup.Spec.OnDemandBackups[0].ID, "routine", r.aeroBackup.Spec.OnDemandBackups[0].RoutineName) + r.Log.Info("Reconciled scheduled backup") + return nil } @@ -334,38 +328,21 @@ func (r *SingleBackupReconciler) reconcileBackup() error { } func (r *SingleBackupReconciler) reconcileScheduledBackup() error { - specHash, err := utils.GetHash(string(r.aeroBackup.Spec.Config.Raw)) - if err != nil { - return err - } - - statusHash, err := utils.GetHash(string(r.aeroBackup.Status.Config.Raw)) - if err != nil { - return err - } - - if specHash == statusHash { - r.Log.Info("Backup config not changed") - return nil - } - - r.Log.Info("Registering backup") + r.Log.Info("Reconciling scheduled backup") serviceClient, err := backup_service.GetBackupServiceClient(r.Client, &r.aeroBackup.Spec.BackupService) if err != nil { return err } - config, err := serviceClient.GetBackupServiceConfig() + backupSvcConfigMap, err := serviceClient.GetBackupServiceConfig() if err != nil { return err } - r.Log.Info("Fetched backup service config", "config", config) - - specBackupConfigMap := make(map[string]interface{}) + r.Log.Info("Fetched backup service config", "config", backupSvcConfigMap) - err = yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &specBackupConfigMap) + specBackupConfigMap, err := r.getBackupConfigInMap() if err != nil { return err } @@ -373,7 +350,7 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error { if specBackupConfigMap[common.AerospikeClusterKey] != nil { cluster := specBackupConfigMap[common.AerospikeClusterKey].(map[string]interface{}) - currentClusters, gErr := common.GetConfigSection(config, common.AerospikeClustersKey) + currentClusters, gErr := common.GetConfigSection(backupSvcConfigMap, common.AerospikeClustersKey) if gErr != nil { return gErr } @@ -391,10 +368,14 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error { } } } else { + r.Log.Info("Adding new cluster", "cluster", name) + err = serviceClient.AddCluster(name, clusterConfig) if err != nil { return err } + + r.Log.Info("Added new cluster", "cluster", name) } } } @@ -402,7 +383,7 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error { if specBackupConfigMap[common.BackupRoutinesKey] != nil { routines := specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) - currentRoutines, gErr := common.GetConfigSection(config, common.BackupRoutinesKey) + currentRoutines, gErr := common.GetConfigSection(backupSvcConfigMap, common.BackupRoutinesKey) if gErr != nil { return gErr } @@ -420,20 +401,22 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error { } } } else { + r.Log.Info("Adding new backup routine", "routine", name) + err = serviceClient.AddBackupRoutine(name, routine) if err != nil { return err } + + r.Log.Info("Added new backup routine", "routine", name) } } } // 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 - } + err = r.unregisterBackupRoutines(serviceClient, backupSvcConfigMap, specBackupConfigMap) + if err != nil { + return err } // Apply the updated configuration for the changes to take effect @@ -442,7 +425,7 @@ func (r *SingleBackupReconciler) reconcileScheduledBackup() error { return err } - r.Log.Info("Registered backup") + r.Log.Info("Reconciled scheduled backup") return nil } @@ -465,40 +448,25 @@ func (r *SingleBackupReconciler) unregisterBackup() error { return err } - config, err := serviceClient.GetBackupServiceConfig() + backupSvcConfig, err := serviceClient.GetBackupServiceConfig() if err != nil { return err } - backupConfigMap := make(map[string]interface{}) - - err = yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &backupConfigMap) + specBackupConfigMap, err := r.getBackupConfigInMap() if err != nil { return err } - if backupConfigMap[common.BackupRoutinesKey] != nil { - routines := backupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) - - currentRoutines, gErr := common.GetConfigSection(config, common.BackupRoutinesKey) - if gErr != nil { - return gErr - } - - for name := range routines { - if _, ok := currentRoutines[name]; ok { - err = serviceClient.DeleteBackupRoutine(name) - if err != nil { - return err - } - } - } + err = r.unregisterBackupRoutines(serviceClient, backupSvcConfig, 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) + currentClusters, gErr := common.GetConfigSection(backupSvcConfig, common.AerospikeClustersKey) if gErr != nil { return gErr } @@ -523,26 +491,41 @@ func (r *SingleBackupReconciler) unregisterBackup() error { } func (r *SingleBackupReconciler) unregisterBackupRoutines( - specBackupConfigMap map[string]interface{}, serviceClient *backup_service.Client, + backupSvcConfigMap, + specBackupConfigMap map[string]interface{}, ) error { - statusBackupConfigMap := make(map[string]interface{}) - - if err := yaml.Unmarshal(r.aeroBackup.Status.Config.Raw, &statusBackupConfigMap); err != nil { + allRoutines, err := common.GetConfigSection(backupSvcConfigMap, common.BackupRoutinesKey) + if 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) + cluster := specBackupConfigMap[common.AerospikeClusterKey].(map[string]interface{}) - if err := serviceClient.DeleteBackupRoutine(name); err != nil { - return err - } + var clusterName string + + // There will always be only one cluster in the backup config + for name := range cluster { + clusterName = name + } + + specRoutines := make(map[string]interface{}) + + // Ignore routines from the spec if the backup is being deleted + if r.aeroBackup.DeletionTimestamp.IsZero() { + specRoutines = specBackupConfigMap[common.BackupRoutinesKey].(map[string]interface{}) + } - r.Log.Info("Unregistered backup routine", "routine", name) + routinesToBeDelete := r.routinesToDelete(specRoutines, allRoutines, clusterName) + + for idx := range routinesToBeDelete { + r.Log.Info("Unregistering backup routine", "routine", routinesToBeDelete[idx]) + + if err := serviceClient.DeleteBackupRoutine(routinesToBeDelete[idx]); err != nil { + return err } + + r.Log.Info("Unregistered backup routine", "routine", routinesToBeDelete[idx]) } return nil @@ -555,3 +538,49 @@ func (r *SingleBackupReconciler) updateStatus() error { return r.Client.Status().Update(context.Background(), r.aeroBackup) } + +func (r *SingleBackupReconciler) getBackupSvcConfigMap() (*corev1.ConfigMap, error) { + cm := &corev1.ConfigMap{} + + if err := r.Client.Get(context.TODO(), + types.NamespacedName{ + Namespace: r.aeroBackup.Spec.BackupService.Namespace, + Name: r.aeroBackup.Spec.BackupService.Name, + }, cm, + ); err != nil { + return nil, err + } + + return cm, nil +} + +func (r *SingleBackupReconciler) routinesToDelete( + specRoutines, allRoutines map[string]interface{}, clusterName string, +) []string { + var routinesTobeDeleted []string + + for name := range allRoutines { + if _, ok := specRoutines[name]; ok { + continue + } + + // Delete any dangling backup-routines related to this cluster + // Strict prefix check might fail for cases where the prefix is same. + if strings.HasPrefix(name, r.aeroBackup.NamePrefix()) && + allRoutines[name].(map[string]interface{})[common.SourceClusterKey].(string) == clusterName { + routinesTobeDeleted = append(routinesTobeDeleted, name) + } + } + + return routinesTobeDeleted +} + +func (r *SingleBackupReconciler) getBackupConfigInMap() (map[string]interface{}, error) { + backupConfigMap := make(map[string]interface{}) + + if err := yaml.Unmarshal(r.aeroBackup.Spec.Config.Raw, &backupConfigMap); err != nil { + return backupConfigMap, err + } + + return backupConfigMap, nil +} diff --git a/controllers/common/constant.go b/controllers/common/constant.go index b6ed628a..e863bf3b 100644 --- a/controllers/common/constant.go +++ b/controllers/common/constant.go @@ -9,6 +9,7 @@ const ( BackupRoutinesKey = "backup-routines" BackupPoliciesKey = "backup-policies" SecretAgentsKey = "secret-agent" + SourceClusterKey = "source-cluster" BackupServiceConfigYAML = "aerospike-backup-service.yml" ) diff --git a/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikebackups.asdb.aerospike.com.yaml b/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikebackups.asdb.aerospike.com.yaml index 098a0f72..4bb22847 100644 --- a/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikebackups.asdb.aerospike.com.yaml +++ b/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikebackups.asdb.aerospike.com.yaml @@ -45,28 +45,6 @@ spec: description: AerospikeBackupSpec defines the desired state of AerospikeBackup for a given AerospikeCluster properties: - OnDemandBackups: - description: OnDemandBackups is the configuration for on-demand backups. - items: - properties: - delay: - description: Delay is the interval before starting the on-demand - backup. - type: string - id: - description: ID is the unique identifier for the on-demand backup. - minLength: 1 - type: string - routineName: - description: RoutineName is the routine name used to trigger - on-demand backup. - type: string - required: - - id - - routineName - type: object - maxItems: 1 - type: array backupService: description: BackupService is the backup service reference i.e. name and namespace. It is used to communicate to the backup service to @@ -88,14 +66,7 @@ spec: aerospike-cluster, backup-routines.' type: object x-kubernetes-preserve-unknown-fields: true - required: - - backupService - - config - type: object - status: - description: AerospikeBackupStatus defines the observed state of AerospikeBackup - properties: - OnDemandBackups: + onDemandBackups: description: OnDemandBackups is the configuration for on-demand backups. items: properties: @@ -115,7 +86,15 @@ spec: - id - routineName type: object + maxItems: 1 type: array + required: + - backupService + - config + type: object + status: + description: AerospikeBackupStatus defines the observed state of AerospikeBackup + properties: backupService: description: BackupService is the backup service reference i.e. name and namespace. @@ -136,6 +115,27 @@ spec: backup-routines.' type: object x-kubernetes-preserve-unknown-fields: true + onDemandBackups: + description: OnDemandBackups is the configuration for on-demand backups. + items: + properties: + delay: + description: Delay is the interval before starting the on-demand + backup. + type: string + id: + description: ID is the unique identifier for the on-demand backup. + minLength: 1 + type: string + routineName: + description: RoutineName is the routine name used to trigger + on-demand backup. + type: string + required: + - id + - routineName + type: object + type: array required: - backupService - config diff --git a/test/backup/backup_test.go b/test/backup/backup_test.go index 5fea1451..839d4156 100644 --- a/test/backup/backup_test.go +++ b/test/backup/backup_test.go @@ -5,7 +5,9 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/yaml" asdbv1beta1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1beta1" "github.com/aerospike/aerospike-kubernetes-operator/controllers/common" @@ -147,7 +149,7 @@ var _ = Describe( It("Should fail when non-existing cluster is referred in Backup routine", func() { config := getBackupConfigInMap(namePrefix(backupNsNm)) routines := config[common.BackupRoutinesKey].(map[string]interface{}) - routines[namePrefix(backupNsNm)+"-"+"test-routine"].(map[string]interface{})["source-cluster"] = + routines[namePrefix(backupNsNm)+"-"+"test-routine"].(map[string]interface{})[common.SourceClusterKey] = "non-existing-cluster" config[common.BackupRoutinesKey] = routines @@ -174,17 +176,9 @@ var _ = Describe( Expect(err).To(HaveOccurred()) }) - It("Should fail when a different aerospike cluster is referred in Backup routine", func() { - config := getBackupConfigInMap(namePrefix(backupNsNm)) - routines := config[common.BackupRoutinesKey].(map[string]interface{}) - routines[namePrefix(backupNsNm)+"-"+"test-routine"].(map[string]interface{})["source-cluster"] = - "random-cluster" - config[common.BackupRoutinesKey] = routines - - configBytes, mErr := json.Marshal(config) - Expect(mErr).ToNot(HaveOccurred()) - - backup = newBackupWithConfig(backupNsNm, configBytes) + It("Should fail when empty backup config is given", func() { + backup = newBackupWithEmptyConfig(backupNsNm) + backup.Spec.Config.Raw = []byte("{}") err = CreateBackup(k8sClient, backup) Expect(err).To(HaveOccurred()) }) @@ -365,6 +359,78 @@ var _ = Describe( Expect(err).ToNot(HaveOccurred()) }) + It("Should delete dangling routines from the Backup service configMap", func() { + backup, err = NewBackup(backupNsNm) + Expect(err).ToNot(HaveOccurred()) + + err = CreateBackup(k8sClient, backup) + Expect(err).ToNot(HaveOccurred()) + + err = validateTriggeredBackup(k8sClient, backup) + Expect(err).ToNot(HaveOccurred()) + + By("Get Backup service configmap to update new dangling backup routine") + var cm corev1.ConfigMap + + err = k8sClient.Get(testCtx, + types.NamespacedName{Name: backupServiceName, Namespace: backupServiceNamespace}, + &cm) + Expect(err).ToNot(HaveOccurred()) + + // Add a routine to the configMap + data := cm.Data[common.BackupServiceConfigYAML] + backupSvcConfigMap := make(map[string]interface{}) + + err = yaml.Unmarshal([]byte(data), &backupSvcConfigMap) + Expect(err).ToNot(HaveOccurred()) + + backupRoutines := backupSvcConfigMap[common.BackupRoutinesKey].(map[string]interface{}) + // Add a new routine with a different name + newRoutineName := namePrefix(backupNsNm) + "-" + "test-routine1" + backupRoutines[newRoutineName] = + backupRoutines[namePrefix(backupNsNm)+"-"+"test-routine"] + + backupSvcConfigMap[common.BackupRoutinesKey] = backupRoutines + + newData, mErr := yaml.Marshal(backupSvcConfigMap) + Expect(mErr).ToNot(HaveOccurred()) + + cm.Data[common.BackupServiceConfigYAML] = string(newData) + + err = k8sClient.Update(testCtx, &cm) + Expect(err).ToNot(HaveOccurred()) + + By("Update backup CR to add on-demand backup") + backup, err = getBackupObj(k8sClient, backup.Name, backup.Namespace) + Expect(err).ToNot(HaveOccurred()) + + backup.Spec.OnDemandBackups = []asdbv1beta1.OnDemandBackupSpec{ + { + ID: "on-demand", + RoutineName: namePrefix(backupNsNm) + "-" + "test-routine", + }, + } + + err = updateBackup(k8sClient, backup) + Expect(err).ToNot(HaveOccurred()) + + By("Validate the routine is removed from the Backup service configMap") + err = k8sClient.Get(testCtx, + types.NamespacedName{Name: backupServiceName, Namespace: backupServiceNamespace}, + &cm) + Expect(err).ToNot(HaveOccurred()) + + data = cm.Data[common.BackupServiceConfigYAML] + backupSvcConfigMap = make(map[string]interface{}) + + err = yaml.Unmarshal([]byte(data), &backupSvcConfigMap) + Expect(err).ToNot(HaveOccurred()) + + backupRoutines = backupSvcConfigMap[common.BackupRoutinesKey].(map[string]interface{}) + _, ok := backupRoutines[namePrefix(backupNsNm)+"-"+"test-routine1"] + Expect(ok).To(BeFalse()) + }) + It("Should trigger on-demand backup when given", func() { backup, err = NewBackup(backupNsNm) Expect(err).ToNot(HaveOccurred()) diff --git a/test/backup_service/backup_service_test.go b/test/backup_service/backup_service_test.go index 01c7e3d8..17c19cb4 100644 --- a/test/backup_service/backup_service_test.go +++ b/test/backup_service/backup_service_test.go @@ -5,14 +5,13 @@ import ( "net/http" "time" - "github.com/aerospike/aerospike-kubernetes-operator/controllers/common" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" asdbv1beta1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1beta1" + "github.com/aerospike/aerospike-kubernetes-operator/controllers/common" ) var _ = Describe( diff --git a/test/cluster/suite_test.go b/test/cluster/suite_test.go index 8a6b4b9c..d119bc6b 100644 --- a/test/cluster/suite_test.go +++ b/test/cluster/suite_test.go @@ -57,7 +57,7 @@ var scheme = k8Runtime.NewScheme() func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Controller Suite") + RunSpecs(t, "Cluster Suite") } var _ = BeforeEach(func() { diff --git a/test/restore/restore_test.go b/test/restore/restore_test.go index cacbac9d..45ee7e02 100644 --- a/test/restore/restore_test.go +++ b/test/restore/restore_test.go @@ -90,16 +90,16 @@ var _ = Describe( Expect(err.Error()).To(ContainSubstring("restore point in time should be positive")) }) - It("Should fail when source is given for TimeStamp restore type", func() { + It("Should fail when source field is given for TimeStamp restore type", func() { restore, err = newRestore(restoreNsNm, asdbv1beta1.TimeStamp) Expect(err).ToNot(HaveOccurred()) err = createRestore(k8sClient, restore) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("source key is not allowed in restore config")) + Expect(err.Error()).To(ContainSubstring("source field is not allowed in restore config")) }) - It("Should fail when routine key is given for Full/Incremental restore type", func() { + It("Should fail when routine field is given for Full/Incremental restore type", func() { restoreConfig := getRestoreConfigInMap(backupDataPath) restoreConfig[common.RoutineKey] = "test-routine" @@ -110,10 +110,10 @@ var _ = Describe( err = createRestore(k8sClient, restore) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("routine key is not allowed in restore config")) + Expect(err.Error()).To(ContainSubstring("routine field is not allowed in restore config")) }) - It("Should fail when time key is given for Full/Incremental restore type", func() { + It("Should fail when time field is given for Full/Incremental restore type", func() { restoreConfig := getRestoreConfigInMap(backupDataPath) restoreConfig[common.TimeKey] = 1722408895094 @@ -124,7 +124,7 @@ var _ = Describe( err = createRestore(k8sClient, restore) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("time key is not allowed in restore config")) + Expect(err.Error()).To(ContainSubstring("time field is not allowed in restore config")) }) })