move the accepted info from annotations to DU/DD CR

Signed-off-by: Lyndon-Li <[email protected]>
vmware-tanzu · Dec 9, 2024 · 86082eb · 86082eb
1 parent 11f100f
commit 86082eb
Show file tree

Hide file tree

Showing 13 changed files with 94 additions and 42 deletions.
diff --git a/changelogs/unreleased/8498-Lyndon-Li b/changelogs/unreleased/8498-Lyndon-Li
@@ -0,0 +1 @@
+Related to issue #8485, move the acceptedByNode and acceptedTimestamp to Status of DU/DD CRD
diff --git a/config/crd/v2alpha1/bases/velero.io_datadownloads.yaml b/config/crd/v2alpha1/bases/velero.io_datadownloads.yaml
@@ -136,6 +136,16 @@ spec:
           status:
             description: DataDownloadStatus is the current status of a DataDownload.
             properties:
+              acceptedByNode:
+                description: Node is name of the node where the DataUpload is prepared.
+                type: string
+              acceptedTimestamp:
+                description: |-
+                  AcceptedTimestamp records the time the DataUpload is to be prepared.
+                  The server's time is used for AcceptedTimestamp
+                format: date-time
+                nullable: true
+                type: string
               completionTimestamp:
                 description: |-
                   CompletionTimestamp records the time a restore was completed.

diff --git a/config/crd/v2alpha1/bases/velero.io_datauploads.yaml b/config/crd/v2alpha1/bases/velero.io_datauploads.yaml
@@ -143,6 +143,16 @@ spec:
           status:
             description: DataUploadStatus is the current status of a DataUpload.
             properties:
+              acceptedByNode:
+                description: Node is name of the node where the DataUpload is prepared.
+                type: string
+              acceptedTimestamp:
+                description: |-
+                  AcceptedTimestamp records the time the DataUpload is to be prepared.
+                  The server's time is used for AcceptedTimestamp
+                format: date-time
+                nullable: true
+                type: string
               completionTimestamp:
                 description: |-
                   CompletionTimestamp records the time a backup was completed.

diff --git a/config/crd/v2alpha1/crds/crds.go b/config/crd/v2alpha1/crds/crds.go
diff --git a/pkg/apis/velero/v2alpha1/data_download_types.go b/pkg/apis/velero/v2alpha1/data_download_types.go
@@ -115,6 +115,16 @@ type DataDownloadStatus struct {
 	// Node is name of the node where the DataDownload is processed.
 	// +optional
 	Node string `json:"node,omitempty"`
+
+	// Node is name of the node where the DataUpload is prepared.
+	// +optional
+	AcceptedByNode string `json:"acceptedByNode,omitempty"`
+
+	// AcceptedTimestamp records the time the DataUpload is to be prepared.
+	// The server's time is used for AcceptedTimestamp
+	// +optional
+	// +nullable
+	AcceptedTimestamp *metav1.Time `json:"acceptedTimestamp,omitempty"`
 }
 
 // TODO(2.0) After converting all resources to use the runtime-controller client, the genclient and k8s:deepcopy markers will no longer be needed and should be removed.

diff --git a/pkg/apis/velero/v2alpha1/data_upload_types.go b/pkg/apis/velero/v2alpha1/data_upload_types.go
@@ -144,6 +144,15 @@ type DataUploadStatus struct {
 	// Node is name of the node where the DataUpload is processed.
 	// +optional
 	Node string `json:"node,omitempty"`
+	// Node is name of the node where the DataUpload is prepared.
+	// +optional
+	AcceptedByNode string `json:"acceptedByNode,omitempty"`
+
+	// AcceptedTimestamp records the time the DataUpload is to be prepared.
+	// The server's time is used for AcceptedTimestamp
+	// +optional
+	// +nullable
+	AcceptedTimestamp *metav1.Time `json:"acceptedTimestamp,omitempty"`
 }
 
 // TODO(2.0) After converting all resources to use the runttime-controller client,

diff --git a/pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go b/pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go
diff --git a/pkg/builder/data_download_builder.go b/pkg/builder/data_download_builder.go
@@ -147,3 +147,15 @@ func (d *DataDownloadBuilder) Node(node string) *DataDownloadBuilder {
 	d.object.Status.Node = node
 	return d
 }
+
+// AcceptedByNode sets the DataDownload's AcceptedByNode.
+func (d *DataDownloadBuilder) AcceptedByNode(node string) *DataDownloadBuilder {
+	d.object.Status.AcceptedByNode = node
+	return d
+}
+
+// AcceptedTimestamp sets the DataDownload's AcceptedTimestamp.
+func (d *DataDownloadBuilder) AcceptedTimestamp(acceptedTimestamp *metav1.Time) *DataDownloadBuilder {
+	d.object.Status.AcceptedTimestamp = acceptedTimestamp
+	return d
+}
diff --git a/pkg/builder/data_upload_builder.go b/pkg/builder/data_upload_builder.go
@@ -150,3 +150,15 @@ func (d *DataUploadBuilder) Node(node string) *DataUploadBuilder {
 	d.object.Status.Node = node
 	return d
 }
+
+// AcceptedByNode sets the DataUpload's AcceptedByNode.
+func (d *DataUploadBuilder) AcceptedByNode(node string) *DataUploadBuilder {
+	d.object.Status.AcceptedByNode = node
+	return d
+}
+
+// AcceptedTimestamp sets the DataUpload's AcceptedTimestamp.
+func (d *DataUploadBuilder) AcceptedTimestamp(acceptedTimestamp *metav1.Time) *DataUploadBuilder {
+	d.object.Status.AcceptedTimestamp = acceptedTimestamp
+	return d
+}
diff --git a/pkg/controller/data_download_controller.go b/pkg/controller/data_download_controller.go
@@ -223,11 +223,9 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		} else if peekErr := r.restoreExposer.PeekExposed(ctx, getDataDownloadOwnerObject(dd)); peekErr != nil {
 			r.tryCancelAcceptedDataDownload(ctx, dd, fmt.Sprintf("found a dataupload %s/%s with expose error: %s. mark it as cancel", dd.Namespace, dd.Name, peekErr))
 			log.Errorf("Cancel dd %s/%s because of expose error %s", dd.Namespace, dd.Name, peekErr)
-		} else if at, found := dd.Annotations[acceptTimeAnnoKey]; found {
-			if t, err := time.Parse(time.RFC3339, at); err == nil {
-				if time.Since(t) >= r.preparingTimeout {
-					r.onPrepareTimeout(ctx, dd)
-				}
+		} else if dd.Status.AcceptedTimestamp != nil {
+			if time.Since(dd.Status.AcceptedTimestamp.Time) >= r.preparingTimeout {
+				r.onPrepareTimeout(ctx, dd)
 			}
 		}
 
@@ -636,13 +634,8 @@ func (r *DataDownloadReconciler) acceptDataDownload(ctx context.Context, dd *vel
 
 	updateFunc := func(datadownload *velerov2alpha1api.DataDownload) {
 		datadownload.Status.Phase = velerov2alpha1api.DataDownloadPhaseAccepted
-		annotations := datadownload.GetAnnotations()
-		if annotations == nil {
-			annotations = make(map[string]string)
-		}
-		annotations[acceptNodeAnnoKey] = r.nodeName
-		annotations[acceptTimeAnnoKey] = r.Clock.Now().Format(time.RFC3339)
-		datadownload.SetAnnotations(annotations)
+		datadownload.Status.AcceptedByNode = r.nodeName
+		datadownload.Status.AcceptedTimestamp = &metav1.Time{Time: r.Clock.Now()}
 	}
 
 	succeeded, err := r.exclusiveUpdateDataDownload(ctx, updated, updateFunc)

diff --git a/pkg/controller/data_download_controller_test.go b/pkg/controller/data_download_controller_test.go
@@ -349,7 +349,7 @@ func TestDataDownloadReconcile(t *testing.T) {
 		},
 		{
 			name:     "prepare timeout",
-			dd:       dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Annotations(map[string]string{acceptTimeAnnoKey: (time.Now().Add(-time.Minute * 5)).Format(time.RFC3339)}).Result(),
+			dd:       dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).AcceptedTimestamp(&metav1.Time{Time: time.Now().Add(-time.Minute * 5)}).Result(),
 			expected: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseFailed).Result(),
 		},
 		{
@@ -1003,23 +1003,19 @@ func TestAttemptDataDownloadResume(t *testing.T) {
 		},
 		{
 			name:                   "accepted DataDownload in the current node",
-			dd:                     dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Annotations(map[string]string{acceptNodeAnnoKey: "node-1"}).Result(),
+			dd:                     dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).AcceptedByNode("node-1").Result(),
 			cancelledDataDownloads: []string{dataDownloadName},
 			acceptedDataDownloads:  []string{dataDownloadName},
 		},
 		{
-			name: "accepted DataDownload with dd label but is canceled",
-			dd: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Cancel(true).Annotations(map[string]string{
-				acceptNodeAnnoKey: "node-1",
-			}).Result(),
+			name:                   "accepted DataDownload with dd label but is canceled",
+			dd:                     dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Cancel(true).AcceptedByNode("node-1").Result(),
 			acceptedDataDownloads:  []string{dataDownloadName},
 			cancelledDataDownloads: []string{dataDownloadName},
 		},
 		{
-			name: "accepted DataDownload with dd label but cancel fail",
-			dd: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Annotations(map[string]string{
-				acceptNodeAnnoKey: "node-1",
-			}).Result(),
+			name:                  "accepted DataDownload with dd label but cancel fail",
+			dd:                    dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).AcceptedByNode("node-1").Result(),
 			needErrs:              []bool{false, false, true, false, false, false},
 			acceptedDataDownloads: []string{dataDownloadName},
 		},

diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go
@@ -55,8 +55,6 @@ import (
 
 const (
 	dataUploadDownloadRequestor = "snapshot-data-upload-download"
-	acceptNodeAnnoKey           = "velero.io/accepted-by"
-	acceptTimeAnnoKey           = "velero.io/accepted-at"
 	DataUploadDownloadFinalizer = "velero.io/data-upload-download-finalizer"
 	preparingMonitorFrequency   = time.Minute
 )
@@ -256,11 +254,9 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		} else if peekErr := ep.PeekExposed(ctx, getOwnerObject(du)); peekErr != nil {
 			r.tryCancelAcceptedDataUpload(ctx, du, fmt.Sprintf("found a dataupload %s/%s with expose error: %s. mark it as cancel", du.Namespace, du.Name, peekErr))
 			log.Errorf("Cancel du %s/%s because of expose error %s", du.Namespace, du.Name, peekErr)
-		} else if at, found := du.Annotations[acceptTimeAnnoKey]; found {
-			if t, err := time.Parse(time.RFC3339, at); err == nil {
-				if time.Since(t) >= r.preparingTimeout {
-					r.onPrepareTimeout(ctx, du)
-				}
+		} else if du.Status.AcceptedTimestamp != nil {
+			if time.Since(du.Status.AcceptedTimestamp.Time) >= r.preparingTimeout {
+				r.onPrepareTimeout(ctx, du)
 			}
 		}
 
@@ -704,13 +700,8 @@ func (r *DataUploadReconciler) acceptDataUpload(ctx context.Context, du *velerov
 
 	updateFunc := func(dataUpload *velerov2alpha1api.DataUpload) {
 		dataUpload.Status.Phase = velerov2alpha1api.DataUploadPhaseAccepted
-		annotations := dataUpload.GetAnnotations()
-		if annotations == nil {
-			annotations = make(map[string]string)
-		}
-		annotations[acceptNodeAnnoKey] = r.nodeName
-		annotations[acceptTimeAnnoKey] = r.Clock.Now().Format(time.RFC3339)
-		dataUpload.SetAnnotations(annotations)
+		dataUpload.Status.AcceptedByNode = r.nodeName
+		dataUpload.Status.AcceptedTimestamp = &metav1.Time{Time: r.Clock.Now()}
 	}
 
 	succeeded, err := r.exclusiveUpdateDataUpload(ctx, updated, updateFunc)

diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go
@@ -475,7 +475,7 @@ func TestReconcile(t *testing.T) {
 		},
 		{
 			name:     "prepare timeout",
-			du:       dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).SnapshotType(fakeSnapshotType).Annotations(map[string]string{acceptTimeAnnoKey: (time.Now().Add(-time.Minute * 5)).Format(time.RFC3339)}).Result(),
+			du:       dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).SnapshotType(fakeSnapshotType).AcceptedTimestamp(&metav1.Time{Time: time.Now().Add(-time.Minute * 5)}).Result(),
 			expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseFailed).Result(),
 		},
 		{
@@ -1071,19 +1071,19 @@ func TestAttemptDataUploadResume(t *testing.T) {
 		},
 		{
 			name:                 "accepted DataUpload in the current node",
-			du:                   dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).Annotations(map[string]string{acceptNodeAnnoKey: "node-1"}).Result(),
+			du:                   dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).AcceptedByNode("node-1").Result(),
 			cancelledDataUploads: []string{dataUploadName},
 			acceptedDataUploads:  []string{dataUploadName},
 		},
 		{
 			name:                 "accepted DataUpload in the current node but canceled",
-			du:                   dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).Annotations(map[string]string{acceptNodeAnnoKey: "node-1"}).Cancel(true).Result(),
+			du:                   dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).AcceptedByNode("node-1").Cancel(true).Result(),
 			cancelledDataUploads: []string{dataUploadName},
 			acceptedDataUploads:  []string{dataUploadName},
 		},
 		{
 			name:                "accepted DataUpload in the current node but update error",
-			du:                  dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).Annotations(map[string]string{acceptNodeAnnoKey: "node-1"}).Result(),
+			du:                  dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).AcceptedByNode("node-1").Result(),
 			needErrs:            []bool{false, false, true, false, false, false},
 			acceptedDataUploads: []string{dataUploadName},
 		},
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Related to issue #8485, move the acceptedByNode and acceptedTimestamp to Status of DU/DD CRD