From 08af6f3cfbd1b98aef5ac3fe48ef4f12fd0ea2e0 Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 13:23:28 -0400 Subject: [PATCH 01/11] add todos for later refactor PR --- pkg/controllers/backup/controller.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index 66c89c17..abacb4ee 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -39,7 +39,8 @@ type handler struct { dynamicClient dynamic.Interface defaultBackupMountPath string defaultS3BackupLocation *v1.S3ObjectStore - kubeSystemNS string + // TODO: rename to kubeSystemNamespaceUID; nit to improve clarity, it's not the string representation nor the NS resource + kubeSystemNS string } const DefaultRetentionCount = 10 @@ -79,6 +80,7 @@ func Register( // fatal log here, because we need the kube-system ns UID while creating any backup file logrus.Fatalf("Error getting namespace kube-system %v", err) } + // TODO: rename to kubeSystemNamespaceUID; nit to improve clarity, it's not the string representation nor the NS resource controller.kubeSystemNS = string(kubeSystemNS.UID) // Register handlers backups.OnChange(ctx, "backups", controller.OnBackupChange) @@ -94,6 +96,7 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error return h.setReconcilingCondition(backup, err) } + // Handle updates made on Backup CRs with existing backup files if backup.Status.LastSnapshotTS != "" { if backup.Spec.Schedule == "" { // Backup CR was meant for one-time backup, and the backup has been completed. Probably here from UpdateStatus call From 4c401856e1482fe1103b057ec1dc31b066d3e1ee Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 15:47:52 -0400 Subject: [PATCH 02/11] Add new annotation for tracking a backups origin cluster ID --- pkg/apis/resources.cattle.io/v1/types.go | 4 ++++ pkg/controllers/backup/controller.go | 15 ++++++++++++--- pkg/util/util.go | 9 +++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/apis/resources.cattle.io/v1/types.go b/pkg/apis/resources.cattle.io/v1/types.go index ec2f3359..3a7bf768 100644 --- a/pkg/apis/resources.cattle.io/v1/types.go +++ b/pkg/apis/resources.cattle.io/v1/types.go @@ -15,6 +15,10 @@ var ( RestoreConditionReady = "Ready" ) +const ( + BackupClusterOriginIndex = "field.cattle.io/originClusterId" +) + // +genclient // +genclient:nonNamespaced // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index abacb4ee..3b3496a6 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -75,13 +75,13 @@ func Register( } // Use the kube-system NS.UID as the unique ID for a cluster - kubeSystemNS, err := controller.namespaces.Get("kube-system", k8sv1.GetOptions{}) + kubeSystemNamespaceUID, err := util.FetchClusterUid(namespaces) if err != nil { // fatal log here, because we need the kube-system ns UID while creating any backup file logrus.Fatalf("Error getting namespace kube-system %v", err) } - // TODO: rename to kubeSystemNamespaceUID; nit to improve clarity, it's not the string representation nor the NS resource - controller.kubeSystemNS = string(kubeSystemNS.UID) + // TODO: rename to kubeSystemNamespaceUID + controller.kubeSystemNS = kubeSystemNamespaceUID // Register handlers backups.OnChange(ctx, "backups", controller.OnBackupChange) } @@ -176,6 +176,15 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error return h.setReconcilingCondition(backup, err) } } + + backupAnnotations := backup.GetAnnotations() + if backupAnnotations == nil { + backupAnnotations = map[string]string{} + } + backupAnnotations[v1.BackupClusterOriginIndex] = h.kubeSystemNS + backup.SetAnnotations(backupAnnotations) + _, err = h.backups.Update(backup) + storageLocationType := backup.Status.StorageLocation updateErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { var err error diff --git a/pkg/util/util.go b/pkg/util/util.go index c2538d8a..5bc9b9bb 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -74,3 +74,12 @@ func ErrList(e []error) error { } return nil } + +func FetchClusterUid(namespaces v1core.NamespaceController) (string, error) { + kubesystemNamespace, err := namespaces.Get("kube-system", k8sv1.GetOptions{}) + if err != nil { + return "", err + } + + return string(kubesystemNamespace.UID), nil +} From 140a194685fb509484d3015f1366dbdd15163c24 Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 16:28:39 -0400 Subject: [PATCH 03/11] Add new BackupConditionClusterOrigin and fix whitespace --- pkg/apis/resources.cattle.io/v1/types.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/apis/resources.cattle.io/v1/types.go b/pkg/apis/resources.cattle.io/v1/types.go index 3a7bf768..e7e06a81 100644 --- a/pkg/apis/resources.cattle.io/v1/types.go +++ b/pkg/apis/resources.cattle.io/v1/types.go @@ -6,13 +6,14 @@ import ( ) var ( - BackupConditionReady = "Ready" - BackupConditionUploaded = "Uploaded" - BackupConditionReconciling = "Reconciling" - BackupConditionStalled = "Stalled" - RestoreConditionReconciling = "Reconciling" - RestoreConditionStalled = "Stalled" - RestoreConditionReady = "Ready" + BackupConditionReady = "Ready" + BackupConditionUploaded = "Uploaded" + BackupConditionReconciling = "Reconciling" + BackupConditionClusterOrigin = "ClusterOrigin" + BackupConditionStalled = "Stalled" + RestoreConditionReconciling = "Reconciling" + RestoreConditionStalled = "Stalled" + RestoreConditionReady = "Ready" ) const ( From ad5b6ddfaba6067d889d02711c6ca2f91e227b66 Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 17:57:52 -0400 Subject: [PATCH 04/11] add new BackupConditionInPlaceRestore and fix whitespace --- pkg/apis/resources.cattle.io/v1/types.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/apis/resources.cattle.io/v1/types.go b/pkg/apis/resources.cattle.io/v1/types.go index e7e06a81..3f669309 100644 --- a/pkg/apis/resources.cattle.io/v1/types.go +++ b/pkg/apis/resources.cattle.io/v1/types.go @@ -6,14 +6,15 @@ import ( ) var ( - BackupConditionReady = "Ready" - BackupConditionUploaded = "Uploaded" - BackupConditionReconciling = "Reconciling" - BackupConditionClusterOrigin = "ClusterOrigin" - BackupConditionStalled = "Stalled" - RestoreConditionReconciling = "Reconciling" - RestoreConditionStalled = "Stalled" - RestoreConditionReady = "Ready" + BackupConditionReady = "Ready" + BackupConditionUploaded = "Uploaded" + BackupConditionReconciling = "Reconciling" + BackupConditionClusterOrigin = "HasClusterOrigin" + BackupConditionInPlaceRestore = "InPlaceRestore" + BackupConditionStalled = "Stalled" + RestoreConditionReconciling = "Reconciling" + RestoreConditionStalled = "Stalled" + RestoreConditionReady = "Ready" ) const ( From 8ceba36e86724fa28731fd2fcaa6aec539b99a4c Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 19:20:03 -0400 Subject: [PATCH 05/11] refactor to more expressive varname --- pkg/controllers/backup/controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index 3b3496a6..257ea3ad 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -102,17 +102,17 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error // Backup CR was meant for one-time backup, and the backup has been completed. Probably here from UpdateStatus call logrus.Infof("Backup CR %v has been processed for one-time backup, returning", backup.Name) // This could also mean backup CR was updated from recurring to one-time, in which case observedGeneration needs to be updated - updBackupStatus := false + shouldUpdateStatus := false if backup.Generation != backup.Status.ObservedGeneration { backup.Status.ObservedGeneration = backup.Generation - updBackupStatus = true + shouldUpdateStatus = true } // check if the backup-type needs to be changed too if backup.Status.BackupType != "One-time" { backup.Status.BackupType = "One-time" - updBackupStatus = true + shouldUpdateStatus = true } - if updBackupStatus { + if shouldUpdateStatus { return h.backups.UpdateStatus(backup) } return backup, nil From 4c9237ae35f58864d3ced8982ccd9bf3e73d29a7 Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 19:21:18 -0400 Subject: [PATCH 06/11] Add new Status conditions for the Cluster/InPlace conditions --- pkg/controllers/backup/cluster_origin.go | 91 ++++++++++++++++++++++++ pkg/controllers/backup/controller.go | 7 ++ 2 files changed, 98 insertions(+) create mode 100644 pkg/controllers/backup/cluster_origin.go diff --git a/pkg/controllers/backup/cluster_origin.go b/pkg/controllers/backup/cluster_origin.go new file mode 100644 index 00000000..6c5de3c3 --- /dev/null +++ b/pkg/controllers/backup/cluster_origin.go @@ -0,0 +1,91 @@ +package backup + +import ( + v1 "github.com/rancher/backup-restore-operator/pkg/apis/resources.cattle.io/v1" + "github.com/rancher/wrangler/v3/pkg/condition" +) + +type backupClusterOriginConditionMeta struct { + backupName string + hasClusterOriginID bool + clusterOriginID string + hasCurrentOriginCondition bool + currentOriginCondition bool + canInPlaceRestore bool + hasInPlaceRestoreCondition bool + currentInPlaceRestoreCondition bool +} + +func newBackupClusterOriginConditionMeta(controllerClusterId string, backup *v1.Backup) backupClusterOriginConditionMeta { + conditionMeta := backupClusterOriginConditionMeta{ + backupName: backup.Name, + hasClusterOriginID: false, + hasCurrentOriginCondition: false, + currentOriginCondition: false, + canInPlaceRestore: false, + hasInPlaceRestoreCondition: false, + currentInPlaceRestoreCondition: false, + } + + originAnnotationValue, ok := backup.GetAnnotations()[v1.BackupClusterOriginIndex] + conditionMeta.hasClusterOriginID = ok && originAnnotationValue != "" + if conditionMeta.hasClusterOriginID { + conditionMeta.clusterOriginID = originAnnotationValue + } + + currentOriginConditionString := condition.Cond(v1.BackupConditionClusterOrigin).GetStatus(backup) + conditionMeta.hasCurrentOriginCondition = currentOriginConditionString != "" + if !conditionMeta.hasCurrentOriginCondition { + conditionMeta.currentOriginCondition = currentOriginConditionString == "True" + } + + if conditionMeta.hasClusterOriginID { + conditionMeta.canInPlaceRestore = conditionMeta.clusterOriginID == controllerClusterId + } + + currentInPlaceRestoreString := condition.Cond(v1.BackupConditionInPlaceRestore).GetStatus(backup) + conditionMeta.hasInPlaceRestoreCondition = currentInPlaceRestoreString != "" + if !conditionMeta.hasInPlaceRestoreCondition { + conditionMeta.currentInPlaceRestoreCondition = currentInPlaceRestoreString == "True" + } + + return conditionMeta +} + +// prepareClusterOriginConditions helps set the cluster origin conditions and reports if anything changed in this part of status. +func (h *handler) prepareClusterOriginConditions(backup *v1.Backup) bool { + conditionChanged := false + conditionMeta := newBackupClusterOriginConditionMeta(h.kubeSystemNS, backup) + + // Fist pass we only care to set BackupConditionClusterOrigin based on if the context is there + if !conditionMeta.hasCurrentOriginCondition || conditionMeta.currentOriginCondition != conditionMeta.hasClusterOriginID { + conditionChanged = true + condition.Cond(v1.BackupConditionClusterOrigin).SetStatusBool(backup, conditionMeta.hasClusterOriginID) + + if conditionMeta.hasClusterOriginID { + condition.Cond(v1.BackupConditionClusterOrigin).Message(backup, "Backup has cluster UID attached.") + } else { + condition.Cond(v1.BackupConditionClusterOrigin).Message(backup, "No cluster UID attached to backup.") + } + } + + // Second pass, we care about the specifics of the ClusterOrigin to set the InPlaceRestore condition + if !conditionMeta.hasClusterOriginID { + // When annotation is missing, we'll mark as unable to determine + condition.Cond(v1.BackupConditionInPlaceRestore).SetStatusBool(backup, false) + condition.Cond(v1.BackupConditionInPlaceRestore).Message(backup, "Unable to determine if in-place Restore is viable.") + } + + if !conditionMeta.hasInPlaceRestoreCondition || conditionMeta.canInPlaceRestore != conditionMeta.currentInPlaceRestoreCondition { + conditionChanged = true + condition.Cond(v1.BackupConditionInPlaceRestore).SetStatusBool(backup, conditionMeta.canInPlaceRestore) + if conditionMeta.canInPlaceRestore { + condition.Cond(v1.BackupConditionInPlaceRestore).Message(backup, "In-place Restore appears viable.") + } else { + condition.Cond(v1.BackupConditionInPlaceRestore).Message(backup, "In-place Restore does not appear viable.") + } + } + + // When the annotation is present and not changed + return conditionChanged +} diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index 257ea3ad..73c6749d 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -112,6 +112,11 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error backup.Status.BackupType = "One-time" shouldUpdateStatus = true } + // check if the origin cluster status needs updating + clusterOriginChanged := h.prepareClusterOriginConditions(backup) + if clusterOriginChanged { + shouldUpdateStatus = true + } if shouldUpdateStatus { return h.backups.UpdateStatus(backup) } @@ -199,6 +204,8 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error condition.Cond(v1.BackupConditionReady).Message(backup, "Completed") condition.Cond(v1.BackupConditionUploaded).SetStatusBool(backup, true) + h.prepareClusterOriginConditions(backup) + backup.Status.LastSnapshotTS = time.Now().Format(time.RFC3339) if cronSchedule != nil { nextBackupAt := cronSchedule.Next(time.Now()) From e63ec612ab795cbc3d7219b610a8b09ff4d9cb7d Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 19:33:16 -0400 Subject: [PATCH 07/11] Add todo related to new feature --- pkg/controllers/backup/controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index 73c6749d..6a257030 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -123,6 +123,9 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error return backup, nil } if backup.Status.NextSnapshotAt != "" { + // TODO: Verify how recurring backups work after a migration today + // Then decide how/where to call prepareClusterOriginConditions for that. + currTime := time.Now().Format(time.RFC3339) logrus.Infof("Next snapshot is scheduled for: %v, current time: %v", backup.Status.NextSnapshotAt, currTime) From 6b62c5e1c85fd0a1e5154f170867777e12d095f8 Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 19:57:41 -0400 Subject: [PATCH 08/11] lint --- pkg/controllers/backup/cluster_origin.go | 4 ++-- pkg/controllers/backup/controller.go | 2 +- pkg/util/util.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/backup/cluster_origin.go b/pkg/controllers/backup/cluster_origin.go index 6c5de3c3..d35d82a5 100644 --- a/pkg/controllers/backup/cluster_origin.go +++ b/pkg/controllers/backup/cluster_origin.go @@ -16,7 +16,7 @@ type backupClusterOriginConditionMeta struct { currentInPlaceRestoreCondition bool } -func newBackupClusterOriginConditionMeta(controllerClusterId string, backup *v1.Backup) backupClusterOriginConditionMeta { +func newBackupClusterOriginConditionMeta(controllerClusterID string, backup *v1.Backup) backupClusterOriginConditionMeta { conditionMeta := backupClusterOriginConditionMeta{ backupName: backup.Name, hasClusterOriginID: false, @@ -40,7 +40,7 @@ func newBackupClusterOriginConditionMeta(controllerClusterId string, backup *v1. } if conditionMeta.hasClusterOriginID { - conditionMeta.canInPlaceRestore = conditionMeta.clusterOriginID == controllerClusterId + conditionMeta.canInPlaceRestore = conditionMeta.clusterOriginID == controllerClusterID } currentInPlaceRestoreString := condition.Cond(v1.BackupConditionInPlaceRestore).GetStatus(backup) diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index 6a257030..36ac2b72 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -75,7 +75,7 @@ func Register( } // Use the kube-system NS.UID as the unique ID for a cluster - kubeSystemNamespaceUID, err := util.FetchClusterUid(namespaces) + kubeSystemNamespaceUID, err := util.FetchClusterUID(namespaces) if err != nil { // fatal log here, because we need the kube-system ns UID while creating any backup file logrus.Fatalf("Error getting namespace kube-system %v", err) diff --git a/pkg/util/util.go b/pkg/util/util.go index 5bc9b9bb..f5740138 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -75,7 +75,7 @@ func ErrList(e []error) error { return nil } -func FetchClusterUid(namespaces v1core.NamespaceController) (string, error) { +func FetchClusterUID(namespaces v1core.NamespaceController) (string, error) { kubesystemNamespace, err := namespaces.Get("kube-system", k8sv1.GetOptions{}) if err != nil { return "", err From a84e5f9c873d4f069316f98b49f569c032244f2d Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 19:38:49 -0400 Subject: [PATCH 09/11] Add new status for originCluster reference --- .../rancher-backup-crd/templates/backup.yaml | 3 ++ pkg/apis/resources.cattle.io/v1/types.go | 5 +-- pkg/controllers/backup/cluster_origin.go | 23 +++++++++-- pkg/controllers/backup/controller.go | 32 +++++++-------- pkg/util/util.go | 39 +++++++++++++++++++ 5 files changed, 77 insertions(+), 25 deletions(-) diff --git a/charts/rancher-backup-crd/templates/backup.yaml b/charts/rancher-backup-crd/templates/backup.yaml index 75ad5bf7..029ca23d 100644 --- a/charts/rancher-backup-crd/templates/backup.yaml +++ b/charts/rancher-backup-crd/templates/backup.yaml @@ -127,6 +127,9 @@ spec: type: string observedGeneration: type: integer + originCluster: + nullable: true + type: string storageLocation: nullable: true type: string diff --git a/pkg/apis/resources.cattle.io/v1/types.go b/pkg/apis/resources.cattle.io/v1/types.go index 3f669309..25ca3a99 100644 --- a/pkg/apis/resources.cattle.io/v1/types.go +++ b/pkg/apis/resources.cattle.io/v1/types.go @@ -17,10 +17,6 @@ var ( RestoreConditionReady = "Ready" ) -const ( - BackupClusterOriginIndex = "field.cattle.io/originClusterId" -) - // +genclient // +genclient:nonNamespaced // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -43,6 +39,7 @@ type BackupSpec struct { type BackupStatus struct { Conditions []genericcondition.GenericCondition `json:"conditions"` + OriginCluster string `json:"originCluster,omitempty"` LastSnapshotTS string `json:"lastSnapshotTs"` NextSnapshotAt string `json:"nextSnapshotAt"` ObservedGeneration int64 `json:"observedGeneration"` diff --git a/pkg/controllers/backup/cluster_origin.go b/pkg/controllers/backup/cluster_origin.go index d35d82a5..5a8ae90e 100644 --- a/pkg/controllers/backup/cluster_origin.go +++ b/pkg/controllers/backup/cluster_origin.go @@ -27,10 +27,10 @@ func newBackupClusterOriginConditionMeta(controllerClusterID string, backup *v1. currentInPlaceRestoreCondition: false, } - originAnnotationValue, ok := backup.GetAnnotations()[v1.BackupClusterOriginIndex] - conditionMeta.hasClusterOriginID = ok && originAnnotationValue != "" + originalValue := backup.Status.OriginCluster + conditionMeta.hasClusterOriginID = originalValue != "" if conditionMeta.hasClusterOriginID { - conditionMeta.clusterOriginID = originAnnotationValue + conditionMeta.clusterOriginID = originalValue } currentOriginConditionString := condition.Cond(v1.BackupConditionClusterOrigin).GetStatus(backup) @@ -55,6 +55,23 @@ func newBackupClusterOriginConditionMeta(controllerClusterID string, backup *v1. // prepareClusterOriginConditions helps set the cluster origin conditions and reports if anything changed in this part of status. func (h *handler) prepareClusterOriginConditions(backup *v1.Backup) bool { conditionChanged := false + if !h.canUseClusterOriginStatus { + currentOriginConditionString := condition.Cond(v1.BackupConditionClusterOrigin).GetStatus(backup) + if currentOriginConditionString != "False" { + condition.Cond(v1.BackupConditionClusterOrigin).SetStatusBool(backup, false) + condition.Cond(v1.BackupConditionClusterOrigin).Message(backup, "CRD not updated to include cluster UID yet.") + conditionChanged = true + } + currentInPlaceRestoreString := condition.Cond(v1.BackupConditionInPlaceRestore).GetStatus(backup) + if currentInPlaceRestoreString != "False" { + condition.Cond(v1.BackupConditionInPlaceRestore).SetStatusBool(backup, false) + condition.Cond(v1.BackupConditionInPlaceRestore).Message(backup, "Cannot determine if in-place Restore is viable.") + conditionChanged = true + } + + return conditionChanged + } + conditionMeta := newBackupClusterOriginConditionMeta(h.kubeSystemNS, backup) // Fist pass we only care to set BackupConditionClusterOrigin based on if the context is there diff --git a/pkg/controllers/backup/controller.go b/pkg/controllers/backup/controller.go index 36ac2b72..ca80581e 100644 --- a/pkg/controllers/backup/controller.go +++ b/pkg/controllers/backup/controller.go @@ -40,7 +40,8 @@ type handler struct { defaultBackupMountPath string defaultS3BackupLocation *v1.S3ObjectStore // TODO: rename to kubeSystemNamespaceUID; nit to improve clarity, it's not the string representation nor the NS resource - kubeSystemNS string + kubeSystemNS string + canUseClusterOriginStatus bool } const DefaultRetentionCount = 10 @@ -57,15 +58,16 @@ func Register( defaultS3 *v1.S3ObjectStore) { controller := &handler{ - ctx: ctx, - backups: backups, - resourceSets: resourceSets, - secrets: secrets, - namespaces: namespaces, - discoveryClient: clientSet.Discovery(), - dynamicClient: dynamicInterface, - defaultBackupMountPath: defaultLocalBackupLocation, - defaultS3BackupLocation: defaultS3, + ctx: ctx, + backups: backups, + resourceSets: resourceSets, + secrets: secrets, + namespaces: namespaces, + discoveryClient: clientSet.Discovery(), + dynamicClient: dynamicInterface, + defaultBackupMountPath: defaultLocalBackupLocation, + defaultS3BackupLocation: defaultS3, + canUseClusterOriginStatus: util.VerifyBackupCrdHasClusterStatus(clientSet.ApiextensionsV1()), } if controller.defaultBackupMountPath != "" { logrus.Infof("Default location for storing backups is %v", controller.defaultBackupMountPath) @@ -185,14 +187,6 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error } } - backupAnnotations := backup.GetAnnotations() - if backupAnnotations == nil { - backupAnnotations = map[string]string{} - } - backupAnnotations[v1.BackupClusterOriginIndex] = h.kubeSystemNS - backup.SetAnnotations(backupAnnotations) - _, err = h.backups.Update(backup) - storageLocationType := backup.Status.StorageLocation updateErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { var err error @@ -200,6 +194,8 @@ func (h *handler) OnBackupChange(_ string, backup *v1.Backup) (*v1.Backup, error if err != nil { return err } + // Set the Cluster origin reference on backup + backup.Status.OriginCluster = h.kubeSystemNS // reset conditions to remove the reconciling condition, because as per kstatus lib its presence is considered an error backup.Status.Conditions = []genericcondition.GenericCondition{} diff --git a/pkg/util/util.go b/pkg/util/util.go index f5740138..512a533a 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -6,9 +6,13 @@ import ( "os" "reflect" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsClientSetv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" + v1core "github.com/rancher/wrangler/v3/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" k8sv1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/server/options/encryptionconfig" "k8s.io/apiserver/pkg/storage/value" @@ -83,3 +87,38 @@ func FetchClusterUID(namespaces v1core.NamespaceController) (string, error) { return string(kubesystemNamespace.UID), nil } + +// Define the GroupVersionResource for CRDs +var crdGVR = schema.GroupVersionResource{ + Group: "apiextensions.k8s.io", + Version: "v1", + Resource: "customresourcedefinitions", +} + +func getCRDDefinition(dynamicClient apiextensionsClientSetv1.ApiextensionsV1Interface, crdName string) (*apiextensionsv1.CustomResourceDefinition, error) { + crd, err := dynamicClient.CustomResourceDefinitions().Get(context.TODO(), crdName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + return crd, nil +} + +func VerifyBackupCrdHasClusterStatus(client apiextensionsClientSetv1.ApiextensionsV1Interface) bool { + crdName := "backups.resources.cattle.io" + + crd, err := getCRDDefinition(client, crdName) + if err != nil { + logrus.Infof("Error fetching CRD: %v", err) + return false + } + + // Inspect the status schema, for example + _, found := crd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["status"].Properties["originCluster"] + if found { + logrus.Debugf("Status schema contains `originCluster` on CRD `%s`.\n", crdName) + return true + } + + logrus.Debugf("`originCluster` not found on status schema for CRD `%s`.\n", crdName) + return false +} From 9ac7ecbe647a5e3b7c5d4200dd8f4138806bdf21 Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 21:29:36 -0400 Subject: [PATCH 10/11] add TODO about potential enhancement --- pkg/controllers/backup/cluster_origin.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controllers/backup/cluster_origin.go b/pkg/controllers/backup/cluster_origin.go index 5a8ae90e..7e373dc7 100644 --- a/pkg/controllers/backup/cluster_origin.go +++ b/pkg/controllers/backup/cluster_origin.go @@ -72,6 +72,7 @@ func (h *handler) prepareClusterOriginConditions(backup *v1.Backup) bool { return conditionChanged } + // TODO: We could add a fallback mode that uses filenames (and/or the annotation) when the CRD is not updated conditionMeta := newBackupClusterOriginConditionMeta(h.kubeSystemNS, backup) // Fist pass we only care to set BackupConditionClusterOrigin based on if the context is there From dfc646aec3e92e342a1c14cf4ab72667603ac0ad Mon Sep 17 00:00:00 2001 From: Dan Pock Date: Thu, 31 Oct 2024 22:00:17 -0400 Subject: [PATCH 11/11] Add "In-Place" column to kubectl get backups --- charts/rancher-backup-crd/templates/backup.yaml | 3 +++ pkg/crds/crd.go | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/charts/rancher-backup-crd/templates/backup.yaml b/charts/rancher-backup-crd/templates/backup.yaml index 029ca23d..21904624 100644 --- a/charts/rancher-backup-crd/templates/backup.yaml +++ b/charts/rancher-backup-crd/templates/backup.yaml @@ -29,6 +29,9 @@ spec: - jsonPath: .status.conditions[?(@.type=="Ready")].message name: Status type: string + - jsonPath: .status.conditions[?(@.type=="InPlaceRestore")].status + name: In-Place + type: string name: v1 schema: openAPIV3Schema: diff --git a/pkg/crds/crd.go b/pkg/crds/crd.go index b04b1156..813a3791 100644 --- a/pkg/crds/crd.go +++ b/pkg/crds/crd.go @@ -59,7 +59,8 @@ func List() []crd.CRD { WithColumn("Latest-Backup", ".status.filename"). WithColumn("ResourceSet", ".spec.resourceSetName"). WithCustomColumn(apiext.CustomResourceColumnDefinition{Name: "Age", Type: "date", JSONPath: ".metadata.creationTimestamp"}). - WithColumn("Status", ".status.conditions[?(@.type==\"Ready\")].message") + WithColumn("Status", ".status.conditions[?(@.type==\"Ready\")].message"). + WithColumn("In-Place", ".status.conditions[?(@.type==\"InPlaceRestore\")].status") }), newCRD(&resources.Restore{}, func(c crd.CRD) crd.CRD { return c.