From a93da2bdd7be7c9de7b3e593fc0031d721347d93 Mon Sep 17 00:00:00 2001 From: Ravi Shankar Date: Tue, 16 Jul 2024 17:56:54 -0700 Subject: [PATCH] incorporated comments during demo --- api/cloud-resources/v1beta1/labels.go | 3 + .../v1beta1/nfsbackupschedule_types.go | 20 +- .../v1beta1/zz_generated.deepcopy.go | 12 +- ...es.kyma-project.io_nfsbackupschedules.yaml | 178 +++++++++++------- ...es.kyma-project.io_nfsbackupschedules.yaml | 178 +++++++++++------- .../cloud-resources/nfsbackupschedule_test.go | 6 - pkg/skr/nfsbackupschedule/checkSuspension.go | 1 + pkg/skr/nfsbackupschedule/createNfsBackup.go | 8 +- .../nfsbackupschedule/createNfsBackup_test.go | 2 +- pkg/skr/nfsbackupschedule/deleteNfsBackup.go | 90 +++++---- .../nfsbackupschedule/deleteNfsBackup_test.go | 20 +- pkg/testinfra/dsl/nfsBackupSchedule.go | 14 -- 12 files changed, 316 insertions(+), 216 deletions(-) diff --git a/api/cloud-resources/v1beta1/labels.go b/api/cloud-resources/v1beta1/labels.go index 765529be5..aef1f4df8 100644 --- a/api/cloud-resources/v1beta1/labels.go +++ b/api/cloud-resources/v1beta1/labels.go @@ -9,4 +9,7 @@ const ( LabelRedisInstanceName = "cloud-resources.kyma-project.io/redisInstanceName" LabelRedisInstanceNamespace = "cloud-resources.kyma-project.io/redisInstanceNamespace" + + LabelScheduleName = "cloud-resources.kyma-project.io/scheduleName" + LabelScheduleNamespace = "cloud-resources.kyma-project.io/scheduleNamespace" ) diff --git a/api/cloud-resources/v1beta1/nfsbackupschedule_types.go b/api/cloud-resources/v1beta1/nfsbackupschedule_types.go index e5a03f862..d1a788501 100644 --- a/api/cloud-resources/v1beta1/nfsbackupschedule_types.go +++ b/api/cloud-resources/v1beta1/nfsbackupschedule_types.go @@ -32,6 +32,7 @@ const ( ReasonNfsVolumeNotFound = "NfsVolumeNotFound" ReasonNfsVolumeNotReady = "NfsVolumeNotReady" ReasonBackupCreateFailed = "BackupCreateFailed" + ReasonBackupListFailed = "BackupListFailed" ) // NfsBackupScheduleSpec defines the desired state of NfsBackupSchedule @@ -89,18 +90,31 @@ type NfsBackupScheduleStatus struct { // +listMapKey=type Conditions []metav1.Condition `json:"conditions,omitempty"` - // NextRunTimes contains the time when the next backup will be created + // NextRunTimes contains the times when the next backup will be created // +optional NextRunTimes []string `json:"nextRunTimes,omitempty"` + //NextDeleteTimes contains the map of backup objects and + //their expected deletion times (calculated based on MaxRetentionDays). + // +optional + NextDeleteTimes map[string]string `json:"nextDeleteTimes,omitempty"` + // LastCreateRun specifies the time when the last backup was created // +optional LastCreateRun *metav1.Time `json:"lastCreateRun,omitempty"` + // LastCreatedBackup contains the object reference of the backup object created during last run. + // +optional + LastCreatedBackup corev1.ObjectReference `json:"lastCreatedBackup,omitempty"` + // LastDeleteRun specifies the time when the backups exceeding the retention period were deleted // +optional LastDeleteRun *metav1.Time `json:"lastDeleteRun,omitempty"` + // LastDeletedBackups contains the object references of the backup object deleted during last run. + // +optional + LastDeletedBackups []corev1.ObjectReference `json:"lastDeletedBackups,omitempty"` + // Schedule specifies the cron expression of the current active schedule // +optional Schedule string `json:"schedule,omitempty"` @@ -108,10 +122,6 @@ type NfsBackupScheduleStatus struct { // BackupIndex specifies the current index of the backup created by this schedule // +kubebuilder:default=0 BackupIndex int `json:"backupIndex,omitempty"` - - // Backups specifies the list of backups created by this schedule - // +optional - Backups []corev1.ObjectReference `json:"backups,omitempty"` } //+kubebuilder:object:root=true diff --git a/api/cloud-resources/v1beta1/zz_generated.deepcopy.go b/api/cloud-resources/v1beta1/zz_generated.deepcopy.go index 0a352ffff..67876c6d6 100644 --- a/api/cloud-resources/v1beta1/zz_generated.deepcopy.go +++ b/api/cloud-resources/v1beta1/zz_generated.deepcopy.go @@ -1499,16 +1499,24 @@ func (in *NfsBackupScheduleStatus) DeepCopyInto(out *NfsBackupScheduleStatus) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.NextDeleteTimes != nil { + in, out := &in.NextDeleteTimes, &out.NextDeleteTimes + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.LastCreateRun != nil { in, out := &in.LastCreateRun, &out.LastCreateRun *out = (*in).DeepCopy() } + out.LastCreatedBackup = in.LastCreatedBackup if in.LastDeleteRun != nil { in, out := &in.LastDeleteRun, &out.LastDeleteRun *out = (*in).DeepCopy() } - if in.Backups != nil { - in, out := &in.Backups, &out.Backups + if in.LastDeletedBackups != nil { + in, out := &in.LastDeletedBackups, &out.LastDeletedBackups *out = make([]corev1.ObjectReference, len(*in)) copy(*out, *in) } diff --git a/config/crd/bases/cloud-resources.kyma-project.io_nfsbackupschedules.yaml b/config/crd/bases/cloud-resources.kyma-project.io_nfsbackupschedules.yaml index c7f132b67..f52433208 100644 --- a/config/crd/bases/cloud-resources.kyma-project.io_nfsbackupschedules.yaml +++ b/config/crd/bases/cloud-resources.kyma-project.io_nfsbackupschedules.yaml @@ -125,71 +125,6 @@ spec: description: BackupIndex specifies the current index of the backup created by this schedule type: integer - backups: - description: Backups specifies the list of backups created by this - schedule - items: - description: "ObjectReference contains enough information to let - you inspect or modify the referred object. --- New uses of this - type are discouraged because of difficulty describing its usage - when embedded in APIs. 1. Ignored fields. It includes many fields - which are not generally honored. For instance, ResourceVersion - and FieldPath are both very rarely valid in actual usage. 2. Invalid - usage help. It is impossible to add specific help for individual - usage. In most embedded usages, there are particular restrictions - like, \"must refer only to types A and B\" or \"UID not honored\" - or \"name must be restricted\". Those cannot be well described - when embedded. 3. Inconsistent validation. Because the usages - are different, the validation rules are different by usage, which - makes it hard for users to predict what will happen. 4. The fields - are both imprecise and overly precise. Kind is not a precise - mapping to a URL. This can produce ambiguity during interpretation - and require a REST mapping. In most cases, the dependency is - on the group,resource tuple and the version of the actual struct - is irrelevant. 5. We cannot easily change it. Because this type - is embedded in many locations, updates to this type will affect - numerous schemas. Don't make new APIs embed an underspecified - API type they do not control. \n Instead of using this type, create - a locally provided and used type that is well-focused on your - reference. For example, ServiceReferences for admission registration: - https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 - ." - properties: - apiVersion: - description: API version of the referent. - type: string - fieldPath: - description: 'If referring to a piece of an object instead of - an entire object, this string should contain a valid JSON/Go - field access statement, such as desiredState.manifest.containers[2]. - For example, if the object reference is to a container within - a pod, this would take on a value like: "spec.containers{name}" - (where "name" refers to the name of the container that triggered - the event) or if no container name is specified "spec.containers[2]" - (container with index 2 in this pod). This syntax is chosen - only to have some well-defined way of referencing a part of - an object. TODO: this design is not final and this field is - subject to change in the future.' - type: string - kind: - description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' - type: string - namespace: - description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' - type: string - resourceVersion: - description: 'Specific resourceVersion to which this reference - is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' - type: string - uid: - description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' - type: string - type: object - x-kubernetes-map-type: atomic - type: array conditions: description: List of status conditions items: @@ -267,14 +202,123 @@ spec: was created format: date-time type: string + lastCreatedBackup: + description: LastCreatedBackup contains the object reference of the + backup object created during last run. + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: 'If referring to a piece of an object instead of + an entire object, this string should contain a valid JSON/Go + field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container within + a pod, this would take on a value like: "spec.containers{name}" + (where "name" refers to the name of the container that triggered + the event) or if no container name is specified "spec.containers[2]" + (container with index 2 in this pod). This syntax is chosen + only to have some well-defined way of referencing a part of + an object. TODO: this design is not final and this field is + subject to change in the future.' + type: string + kind: + description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + namespace: + description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' + type: string + resourceVersion: + description: 'Specific resourceVersion to which this reference + is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' + type: string + uid: + description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + type: string + type: object + x-kubernetes-map-type: atomic lastDeleteRun: description: LastDeleteRun specifies the time when the backups exceeding the retention period were deleted format: date-time type: string + lastDeletedBackups: + description: LastDeletedBackups contains the object references of + the backup object deleted during last run. + items: + description: "ObjectReference contains enough information to let + you inspect or modify the referred object. --- New uses of this + type are discouraged because of difficulty describing its usage + when embedded in APIs. 1. Ignored fields. It includes many fields + which are not generally honored. For instance, ResourceVersion + and FieldPath are both very rarely valid in actual usage. 2. Invalid + usage help. It is impossible to add specific help for individual + usage. In most embedded usages, there are particular restrictions + like, \"must refer only to types A and B\" or \"UID not honored\" + or \"name must be restricted\". Those cannot be well described + when embedded. 3. Inconsistent validation. Because the usages + are different, the validation rules are different by usage, which + makes it hard for users to predict what will happen. 4. The fields + are both imprecise and overly precise. Kind is not a precise + mapping to a URL. This can produce ambiguity during interpretation + and require a REST mapping. In most cases, the dependency is + on the group,resource tuple and the version of the actual struct + is irrelevant. 5. We cannot easily change it. Because this type + is embedded in many locations, updates to this type will affect + numerous schemas. Don't make new APIs embed an underspecified + API type they do not control. \n Instead of using this type, create + a locally provided and used type that is well-focused on your + reference. For example, ServiceReferences for admission registration: + https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 + ." + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: 'If referring to a piece of an object instead of + an entire object, this string should contain a valid JSON/Go + field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container within + a pod, this would take on a value like: "spec.containers{name}" + (where "name" refers to the name of the container that triggered + the event) or if no container name is specified "spec.containers[2]" + (container with index 2 in this pod). This syntax is chosen + only to have some well-defined way of referencing a part of + an object. TODO: this design is not final and this field is + subject to change in the future.' + type: string + kind: + description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + namespace: + description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' + type: string + resourceVersion: + description: 'Specific resourceVersion to which this reference + is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' + type: string + uid: + description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + type: string + type: object + x-kubernetes-map-type: atomic + type: array + nextDeleteTimes: + additionalProperties: + type: string + description: NextDeleteTimes contains the map of backup objects and + their expected deletion times (calculated based on MaxRetentionDays). + type: object nextRunTimes: - description: NextRunTimes contains the time when the next backup will - be created + description: NextRunTimes contains the times when the next backup + will be created items: type: string type: array diff --git a/config/dist/skr/crd/bases/providers/gcp/cloud-resources.kyma-project.io_nfsbackupschedules.yaml b/config/dist/skr/crd/bases/providers/gcp/cloud-resources.kyma-project.io_nfsbackupschedules.yaml index c7f132b67..f52433208 100644 --- a/config/dist/skr/crd/bases/providers/gcp/cloud-resources.kyma-project.io_nfsbackupschedules.yaml +++ b/config/dist/skr/crd/bases/providers/gcp/cloud-resources.kyma-project.io_nfsbackupschedules.yaml @@ -125,71 +125,6 @@ spec: description: BackupIndex specifies the current index of the backup created by this schedule type: integer - backups: - description: Backups specifies the list of backups created by this - schedule - items: - description: "ObjectReference contains enough information to let - you inspect or modify the referred object. --- New uses of this - type are discouraged because of difficulty describing its usage - when embedded in APIs. 1. Ignored fields. It includes many fields - which are not generally honored. For instance, ResourceVersion - and FieldPath are both very rarely valid in actual usage. 2. Invalid - usage help. It is impossible to add specific help for individual - usage. In most embedded usages, there are particular restrictions - like, \"must refer only to types A and B\" or \"UID not honored\" - or \"name must be restricted\". Those cannot be well described - when embedded. 3. Inconsistent validation. Because the usages - are different, the validation rules are different by usage, which - makes it hard for users to predict what will happen. 4. The fields - are both imprecise and overly precise. Kind is not a precise - mapping to a URL. This can produce ambiguity during interpretation - and require a REST mapping. In most cases, the dependency is - on the group,resource tuple and the version of the actual struct - is irrelevant. 5. We cannot easily change it. Because this type - is embedded in many locations, updates to this type will affect - numerous schemas. Don't make new APIs embed an underspecified - API type they do not control. \n Instead of using this type, create - a locally provided and used type that is well-focused on your - reference. For example, ServiceReferences for admission registration: - https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 - ." - properties: - apiVersion: - description: API version of the referent. - type: string - fieldPath: - description: 'If referring to a piece of an object instead of - an entire object, this string should contain a valid JSON/Go - field access statement, such as desiredState.manifest.containers[2]. - For example, if the object reference is to a container within - a pod, this would take on a value like: "spec.containers{name}" - (where "name" refers to the name of the container that triggered - the event) or if no container name is specified "spec.containers[2]" - (container with index 2 in this pod). This syntax is chosen - only to have some well-defined way of referencing a part of - an object. TODO: this design is not final and this field is - subject to change in the future.' - type: string - kind: - description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - name: - description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' - type: string - namespace: - description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' - type: string - resourceVersion: - description: 'Specific resourceVersion to which this reference - is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' - type: string - uid: - description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' - type: string - type: object - x-kubernetes-map-type: atomic - type: array conditions: description: List of status conditions items: @@ -267,14 +202,123 @@ spec: was created format: date-time type: string + lastCreatedBackup: + description: LastCreatedBackup contains the object reference of the + backup object created during last run. + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: 'If referring to a piece of an object instead of + an entire object, this string should contain a valid JSON/Go + field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container within + a pod, this would take on a value like: "spec.containers{name}" + (where "name" refers to the name of the container that triggered + the event) or if no container name is specified "spec.containers[2]" + (container with index 2 in this pod). This syntax is chosen + only to have some well-defined way of referencing a part of + an object. TODO: this design is not final and this field is + subject to change in the future.' + type: string + kind: + description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + namespace: + description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' + type: string + resourceVersion: + description: 'Specific resourceVersion to which this reference + is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' + type: string + uid: + description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + type: string + type: object + x-kubernetes-map-type: atomic lastDeleteRun: description: LastDeleteRun specifies the time when the backups exceeding the retention period were deleted format: date-time type: string + lastDeletedBackups: + description: LastDeletedBackups contains the object references of + the backup object deleted during last run. + items: + description: "ObjectReference contains enough information to let + you inspect or modify the referred object. --- New uses of this + type are discouraged because of difficulty describing its usage + when embedded in APIs. 1. Ignored fields. It includes many fields + which are not generally honored. For instance, ResourceVersion + and FieldPath are both very rarely valid in actual usage. 2. Invalid + usage help. It is impossible to add specific help for individual + usage. In most embedded usages, there are particular restrictions + like, \"must refer only to types A and B\" or \"UID not honored\" + or \"name must be restricted\". Those cannot be well described + when embedded. 3. Inconsistent validation. Because the usages + are different, the validation rules are different by usage, which + makes it hard for users to predict what will happen. 4. The fields + are both imprecise and overly precise. Kind is not a precise + mapping to a URL. This can produce ambiguity during interpretation + and require a REST mapping. In most cases, the dependency is + on the group,resource tuple and the version of the actual struct + is irrelevant. 5. We cannot easily change it. Because this type + is embedded in many locations, updates to this type will affect + numerous schemas. Don't make new APIs embed an underspecified + API type they do not control. \n Instead of using this type, create + a locally provided and used type that is well-focused on your + reference. For example, ServiceReferences for admission registration: + https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 + ." + properties: + apiVersion: + description: API version of the referent. + type: string + fieldPath: + description: 'If referring to a piece of an object instead of + an entire object, this string should contain a valid JSON/Go + field access statement, such as desiredState.manifest.containers[2]. + For example, if the object reference is to a container within + a pod, this would take on a value like: "spec.containers{name}" + (where "name" refers to the name of the container that triggered + the event) or if no container name is specified "spec.containers[2]" + (container with index 2 in this pod). This syntax is chosen + only to have some well-defined way of referencing a part of + an object. TODO: this design is not final and this field is + subject to change in the future.' + type: string + kind: + description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names' + type: string + namespace: + description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/' + type: string + resourceVersion: + description: 'Specific resourceVersion to which this reference + is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency' + type: string + uid: + description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' + type: string + type: object + x-kubernetes-map-type: atomic + type: array + nextDeleteTimes: + additionalProperties: + type: string + description: NextDeleteTimes contains the map of backup objects and + their expected deletion times (calculated based on MaxRetentionDays). + type: object nextRunTimes: - description: NextRunTimes contains the time when the next backup will - be created + description: NextRunTimes contains the times when the next backup + will be created items: type: string type: array diff --git a/internal/controller/cloud-resources/nfsbackupschedule_test.go b/internal/controller/cloud-resources/nfsbackupschedule_test.go index 9ecc21d6b..81f07143f 100644 --- a/internal/controller/cloud-resources/nfsbackupschedule_test.go +++ b/internal/controller/cloud-resources/nfsbackupschedule_test.go @@ -167,12 +167,6 @@ var _ = Describe("Feature: SKR NfsBackupSchedule", func() { WithArguments( infra.Ctx(), infra.SKR().Client(), nfsBackupSchedule, WithNextRunTime(now), - WithNfsBackups( - &corev1.ObjectReference{ - Name: nfsBackup1Name, - Namespace: DefaultSkrNamespace, - }, - ), ). Should(Succeed()) }) diff --git a/pkg/skr/nfsbackupschedule/checkSuspension.go b/pkg/skr/nfsbackupschedule/checkSuspension.go index 34fe21365..22dbdac48 100644 --- a/pkg/skr/nfsbackupschedule/checkSuspension.go +++ b/pkg/skr/nfsbackupschedule/checkSuspension.go @@ -25,6 +25,7 @@ func checkSuspension(ctx context.Context, st composed.State) (error, context.Con logger.WithValues("NfsBackupSchedule :", schedule.Name).Info("Schedule is suspended. Stopping reconciliation.") schedule.Status.State = cloudresourcesv1beta1.JobStateSuspended schedule.Status.NextRunTimes = nil + schedule.Status.NextDeleteTimes = nil return composed.UpdateStatus(schedule). SuccessError(composed.StopAndForget). Run(ctx, state) diff --git a/pkg/skr/nfsbackupschedule/createNfsBackup.go b/pkg/skr/nfsbackupschedule/createNfsBackup.go index 73464a718..f54dd316c 100644 --- a/pkg/skr/nfsbackupschedule/createNfsBackup.go +++ b/pkg/skr/nfsbackupschedule/createNfsBackup.go @@ -69,11 +69,11 @@ func createNfsBackup(ctx context.Context, st composed.State) (error, context.Con schedule.Status.State = cloudresourcesv1beta1.JobStateActive schedule.Status.BackupIndex = index schedule.Status.LastCreateRun = &metav1.Time{Time: state.nextRunTime.UTC()} - schedule.Status.Backups = append(schedule.Status.Backups, corev1.ObjectReference{ + schedule.Status.LastCreatedBackup = corev1.ObjectReference{ Kind: backup.GetObjectKind().GroupVersionKind().Kind, Namespace: backup.GetNamespace(), Name: backup.GetName(), - }) + } return composed.UpdateStatus(schedule). SetExclusiveConditions(). SuccessError(composed.StopWithRequeue). @@ -88,8 +88,8 @@ func getProviderSpecificBackupObject(state *State, name string) (client.Object, Name: name, Namespace: schedule.Namespace, Labels: map[string]string{ - "nfs-backup-schedule": schedule.Name, - "nfs-backup-schedule-ns": schedule.Namespace, + cloudresourcesv1beta1.LabelScheduleName: schedule.Name, + cloudresourcesv1beta1.LabelScheduleNamespace: schedule.Namespace, }, } diff --git a/pkg/skr/nfsbackupschedule/createNfsBackup_test.go b/pkg/skr/nfsbackupschedule/createNfsBackup_test.go index 3f5ebe276..26ab7954b 100644 --- a/pkg/skr/nfsbackupschedule/createNfsBackup_test.go +++ b/pkg/skr/nfsbackupschedule/createNfsBackup_test.go @@ -125,7 +125,7 @@ func (suite *createNfsBackupSuite) testCreateBackup(scope *cloudcontrolv1beta1.S fromK8s) suite.Nil(err) suite.Equal(v1beta1.JobStateActive, fromK8s.Status.State) - suite.Equal(bkupName, fromK8s.Status.Backups[0].Name) + suite.Equal(bkupName, fromK8s.Status.LastCreatedBackup.Name) var bkup client.Object switch scope.Spec.Provider { diff --git a/pkg/skr/nfsbackupschedule/deleteNfsBackup.go b/pkg/skr/nfsbackupschedule/deleteNfsBackup.go index 944e802cb..f0c276ac5 100644 --- a/pkg/skr/nfsbackupschedule/deleteNfsBackup.go +++ b/pkg/skr/nfsbackupschedule/deleteNfsBackup.go @@ -3,14 +3,12 @@ package nfsbackupschedule import ( "context" "fmt" - cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1" cloudresourcesv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1" "github.com/kyma-project/cloud-manager/pkg/composed" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sort" "time" ) @@ -40,6 +38,8 @@ func deleteNfsBackup(ctx context.Context, st composed.State) (error, context.Con //If maxRetentionDays is not positive, requeue to update next run time if schedule.Spec.MaxRetentionDays <= 0 { schedule.Status.LastDeleteRun = &metav1.Time{Time: state.nextRunTime.UTC()} + schedule.Status.NextDeleteTimes = nil + schedule.Status.LastDeletedBackups = nil return composed.UpdateStatus(schedule). SetExclusiveConditions(). SuccessError(composed.StopWithRequeue). @@ -48,57 +48,67 @@ func deleteNfsBackup(ctx context.Context, st composed.State) (error, context.Con logger.WithValues("NfsBackupSchedule :", schedule.Name).Info("Deleting old File Backups") - temp := schedule.Status.Backups[:0] - for _, ref := range schedule.Status.Backups { - backup, err := getBackupObject(ctx, state, ref) - if err != nil { - logger.WithValues("Backup", ref.Name).Info("Error getting backup object") - if !errors.IsNotFound(err) { - //Adding it back to the list. Can try it next iteration - temp = append(temp, ref) - } - continue - } + list := &cloudresourcesv1beta1.GcpNfsVolumeBackupList{} + err := state.SkrCluster.K8sClient().List( + ctx, + list, + client.MatchingLabels{ + cloudresourcesv1beta1.LabelScheduleName: schedule.Name, + cloudresourcesv1beta1.LabelScheduleNamespace: schedule.Namespace, + }, + client.InNamespace(schedule.Namespace), + ) + + if err != nil { + return composed.UpdateStatus(schedule). + SetExclusiveConditions(metav1.Condition{ + Type: cloudresourcesv1beta1.ConditionTypeError, + Status: metav1.ConditionTrue, + Reason: cloudresourcesv1beta1.ReasonBackupListFailed, + Message: "Error listing backup(s)", + }). + SuccessError(composed.StopWithRequeue). + Run(ctx, state) + } + + //sort the list based on creationTime. + sort.Slice(list.Items, func(i, j int) bool { + return list.Items[i].CreationTimestamp.Before(&list.Items[j].CreationTimestamp) + }) + + nextDeleteTimes := map[string]string{} + var lastDeleted []corev1.ObjectReference + for _, backup := range list.Items { //Check if the backup object should be deleted toRetain := time.Duration(schedule.Spec.MaxRetentionDays) * 24 * time.Hour elapsed := time.Since(backup.GetCreationTimestamp().Time) if elapsed > toRetain { - logger.WithValues("Backup", ref.Name).Info("Deleting backup object") - err = state.Cluster().K8sClient().Delete(ctx, backup) - if err == nil { + logger.WithValues("Backup", backup.Name).Info("Deleting backup object") + err = state.Cluster().K8sClient().Delete(ctx, &backup) + if err != nil { + logger.Error(err, "Error deleting the backup object.") continue } + lastDeleted = append(lastDeleted, corev1.ObjectReference{ + Name: backup.Name, + Namespace: backup.Namespace, + }) + } + if len(nextDeleteTimes) < MaxSchedules { + backupName := fmt.Sprintf("%s/%s", backup.Namespace, backup.Name) + deleteTime := backup.CreationTimestamp.AddDate(0, 0, schedule.Spec.MaxRetentionDays) + nextDeleteTimes[backupName] = deleteTime.UTC().Format(time.RFC3339) } - temp = append(temp, ref) } //Update the status of the schedule with the list of available backups - schedule.Status.Backups = temp + //schedule.Status.Backups = temp schedule.Status.LastDeleteRun = &metav1.Time{Time: state.nextRunTime.UTC()} + schedule.Status.LastDeletedBackups = lastDeleted + schedule.Status.NextDeleteTimes = nextDeleteTimes return composed.UpdateStatus(schedule). SetExclusiveConditions(). SuccessError(composed.StopWithRequeue). Run(ctx, state) } - -func getBackupObject(ctx context.Context, state *State, ref corev1.ObjectReference) (client.Object, error) { - - key := types.NamespacedName{ - Name: ref.Name, - Namespace: ref.Namespace, - } - - var obj client.Object - - switch state.Scope.Spec.Provider { - case cloudcontrolv1beta1.ProviderGCP: - obj = &cloudresourcesv1beta1.GcpNfsVolumeBackup{} - case cloudcontrolv1beta1.ProviderAws: - obj = &cloudresourcesv1beta1.AwsNfsVolumeBackup{} - default: - return nil, fmt.Errorf("provider %s not supported", state.Scope.Spec.Provider) - } - err := state.SkrCluster.K8sClient().Get(ctx, key, obj) - return obj, err -} diff --git a/pkg/skr/nfsbackupschedule/deleteNfsBackup_test.go b/pkg/skr/nfsbackupschedule/deleteNfsBackup_test.go index 9987520dc..33a4002bf 100644 --- a/pkg/skr/nfsbackupschedule/deleteNfsBackup_test.go +++ b/pkg/skr/nfsbackupschedule/deleteNfsBackup_test.go @@ -6,7 +6,6 @@ import ( "github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1" "github.com/kyma-project/cloud-manager/pkg/composed" "github.com/stretchr/testify/suite" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -122,6 +121,8 @@ func (suite *deleteNfsBackupSuite) TestWhenNoMaxRetentionSet() { suite.Nil(err) suite.NotNil(fromK8s.Status.LastDeleteRun) suite.Equal(runTime.Unix(), fromK8s.Status.LastDeleteRun.Time.Unix()) + suite.Nil(fromK8s.Status.NextRunTimes) + suite.Nil(fromK8s.Status.LastDeletedBackups) } func (suite *deleteNfsBackupSuite) testDeleteBackup(scope *cloudcontrolv1beta1.Scope, createBackup2 bool) { @@ -148,11 +149,19 @@ func (suite *deleteNfsBackupSuite) testDeleteBackup(scope *cloudcontrolv1beta1.S Name: "test-backup-1", Namespace: nfsBackupSchedule.Namespace, CreationTimestamp: metav1.Time{Time: time.Now()}, + Labels: map[string]string{ + v1beta1.LabelScheduleName: nfsBackupSchedule.Name, + v1beta1.LabelScheduleNamespace: nfsBackupSchedule.Namespace, + }, } backup2Meta := metav1.ObjectMeta{ Name: "test-backup-2", Namespace: nfsBackupSchedule.Namespace, CreationTimestamp: metav1.Time{Time: time.Now().AddDate(0, 0, -2)}, + Labels: map[string]string{ + v1beta1.LabelScheduleName: nfsBackupSchedule.Name, + v1beta1.LabelScheduleNamespace: nfsBackupSchedule.Namespace, + }, } gcpSpec := v1beta1.GcpNfsVolumeBackupSpec{ Source: v1beta1.GcpNfsVolumeBackupSource{ @@ -200,13 +209,6 @@ func (suite *deleteNfsBackupSuite) testDeleteBackup(scope *cloudcontrolv1beta1.S suite.Nil(err) } - //Update the schedules backup list with the created backups - obj.Status.Backups = []corev1.ObjectReference{ - {Namespace: backup1.GetNamespace(), Name: backup1.GetName()}, - {Namespace: backup2.GetNamespace(), Name: backup2.GetName()}, - } - err = factory.skrCluster.K8sClient().Status().Update(ctx, obj) - //Invoke API under test err, _ = deleteNfsBackup(ctx, state) @@ -221,8 +223,6 @@ func (suite *deleteNfsBackupSuite) testDeleteBackup(scope *cloudcontrolv1beta1.S suite.Nil(err) suite.NotNil(fromK8s.Status.LastDeleteRun) suite.Equal(runTime.Unix(), fromK8s.Status.LastDeleteRun.Time.Unix()) - suite.Len(fromK8s.Status.Backups, 1) - suite.Equal(backup1.GetName(), fromK8s.Status.Backups[0].Name) var backup client.Object switch scope.Spec.Provider { diff --git a/pkg/testinfra/dsl/nfsBackupSchedule.go b/pkg/testinfra/dsl/nfsBackupSchedule.go index 4895b7d27..c421fde3d 100644 --- a/pkg/testinfra/dsl/nfsBackupSchedule.go +++ b/pkg/testinfra/dsl/nfsBackupSchedule.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" cloudresourcesv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "time" @@ -146,16 +145,3 @@ func WithNextRunTime(runTime time.Time) ObjStatusAction { }, } } - -func WithNfsBackups(refs ...*corev1.ObjectReference) ObjStatusAction { - return &objStatusAction{ - f: func(obj client.Object) { - if x, ok := obj.(*cloudresourcesv1beta1.NfsBackupSchedule); ok { - - for _, ref := range refs { - x.Status.Backups = append(x.Status.Backups, *ref) - } - } - }, - } -}