Move the progress messages to activities (#10552)

* move the progress messages to activities

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>

* update doc for activities in data mover CR

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>

---------

Signed-off-by: Lyndon-Li <lyonghui@vmware.com>

pkg/apis/velero/v1/pod_volume_backup_types.go

@@ -98,7 +98,7 @@ type PodVolumeBackupStatus struct {
 	// +optional
 	SnapshotID string `json:"snapshotID,omitempty"`
 
-	// Message is a message about the pod volume backup's status.
+	// Message is a message describing the pod volume backup when it reaches to a terminal status.
 	// +optional
 	Message string `json:"message,omitempty"`
 
@@ -144,6 +144,11 @@ type PodVolumeBackupStatus struct {
 
 	// FallbackFull indicates whether the incremental backup has fallen back to full backup
 	FallbackFull bool `json:"fallbackFull,omitempty"`
+
+	// Activities contains one or more messages about what have been done for this pod volume backup.
+	// +optional
+	// +nullable
+	Activities []string `json:"activities,omitempty"`
 }
 
 // TODO(2.0) After converting all resources to use the runttime-controller client,

pkg/apis/velero/v1/pod_volume_restore_type.go

@@ -88,7 +88,7 @@ type PodVolumeRestoreStatus struct {
 	// +optional
 	Phase PodVolumeRestorePhase `json:"phase,omitempty"`
 
-	// Message is a message about the pod volume restore's status.
+	// Message is a message describing the pod volume restore when it reaches to a terminal status.
 	// +optional
 	Message string `json:"message,omitempty"`
 
@@ -127,6 +127,11 @@ type PodVolumeRestoreStatus struct {
 
 	// FallbackFull indicates whether the incremental restore has fallen back to full restore
 	FallbackFull bool `json:"fallbackFull,omitempty"`
+
+	// Activities contains one or more messages about what have been done for this pod volume restore.
+	// +optional
+	// +nullable
+	Activities []string `json:"activities,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.

pkg/apis/velero/v2alpha1/data_download_types.go

@@ -109,7 +109,7 @@ type DataDownloadStatus struct {
 	// +optional
 	Phase DataDownloadPhase `json:"phase,omitempty"`
 
-	// Message is a message about the DataDownload's status.
+	// Message is a message describing the DataDownload when it reaches to a terminal status.
 	// +optional
 	Message string `json:"message,omitempty"`
 
@@ -152,6 +152,11 @@ type DataDownloadStatus struct {
 
 	// FallbackFull indicates whether the incremental restore has fallen back to full restore
 	FallbackFull bool `json:"fallbackFull,omitempty"`
+
+	// Activities contains one or more messages about what have been done for this DataDownload.
+	// +optional
+	// +nullable
+	Activities []string `json:"activities,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.

pkg/apis/velero/v2alpha1/data_upload_types.go

@@ -147,7 +147,7 @@ type DataUploadStatus struct {
 	// +nullable
 	DataMoverResult *map[string]string `json:"dataMoverResult,omitempty"`
 
-	// Message is a message about the DataUpload's status.
+	// Message is a message describing the DataUpload when it reaches to a terminal status.
 	// +optional
 	Message string `json:"message,omitempty"`
 
@@ -205,6 +205,11 @@ type DataUploadStatus struct {
 
 	// FallbackFull indicates whether the incremental backup has fallen back to full backup
 	FallbackFull bool `json:"fallbackFull,omitempty"`
+
+	// Activities contains one or more messages about what have been done for this DataUpload.
+	// +optional
+	// +nullable
+	Activities []string `json:"activities,omitempty"`
 }
 
 // TODO(2.0) After converting all resources to use the runttime-controller client,

pkg/controller/data_download_controller.go

@@ -621,9 +621,8 @@ func (r *DataDownloadReconciler) OnDataDownloadProgress(ctx context.Context, nam
 		}
 
 		if progress.Message != "" {
-			message := progress.Message + ";"
-			if !strings.HasSuffix(dd.Status.Message, message) {
-				dd.Status.Message += message
+			if len(dd.Status.Activities) == 0 || dd.Status.Activities[len(dd.Status.Activities)-1] != progress.Message {
+				dd.Status.Activities = append(dd.Status.Activities, progress.Message)
 			}
 		}
 

pkg/controller/data_download_controller_test.go

@@ -829,7 +829,7 @@ func TestOnDataDownloadProgress(t *testing.T) {
 
 			// Call the OnDataDownloadProgress function
 			r.OnDataDownloadProgress(ctx, namespace, duName, progress)
-			if len(test.needErrs) != 0 && !test.needErrs[0] {
+			if len(test.needErrs) == 0 {
 				// Get the updated DataDownload object from the fake client
 				updatedDd := &velerov2alpha1api.DataDownload{}
 				require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: duName, Namespace: namespace}, updatedDd))
@@ -845,7 +845,12 @@ func TestOnDataDownloadProgress(t *testing.T) {
 					assert.Equal(t, int64(0), updatedDd.Status.Progress.BytesDone) // assuming default or original value
 				}
 				if progress.Message != "" {
-					assert.Contains(t, updatedDd.Status.Message, progress.Message)
+					assert.Contains(t, updatedDd.Status.Activities, progress.Message)
+
+					// Call with the same message again to verify deduplication
+					r.OnDataDownloadProgress(ctx, namespace, duName, progress)
+					require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: duName, Namespace: namespace}, updatedDd))
+					assert.Equal(t, []string{progress.Message}, updatedDd.Status.Activities)
 				}
 			}
 		})

pkg/controller/data_upload_controller.go

@@ -648,9 +648,8 @@ func (r *DataUploadReconciler) OnDataUploadProgress(ctx context.Context, namespa
 		}
 
 		if progress.Message != "" {
-			message := progress.Message + ";"
-			if !strings.HasSuffix(du.Status.Message, message) {
-				du.Status.Message += message
+			if len(du.Status.Activities) == 0 || du.Status.Activities[len(du.Status.Activities)-1] != progress.Message {
+				du.Status.Activities = append(du.Status.Activities, progress.Message)
 			}
 		}
 

pkg/controller/data_upload_controller_test.go

@@ -851,7 +851,7 @@ func TestOnDataUploadProgress(t *testing.T) {
 
 			// Call the OnDataUploadProgress function
 			r.OnDataUploadProgress(ctx, namespace, duName, progress)
-			if len(test.needErrs) != 0 && !test.needErrs[0] {
+			if len(test.needErrs) == 0 {
 				// Get the updated DataUpload object from the fake client
 				updatedDu := &velerov2alpha1api.DataUpload{}
 				require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: duName, Namespace: namespace}, updatedDu))
@@ -867,7 +867,12 @@ func TestOnDataUploadProgress(t *testing.T) {
 					assert.Equal(t, int64(0), updatedDu.Status.Progress.BytesDone) // assuming default or original value
 				}
 				if progress.Message != "" {
-					assert.Contains(t, updatedDu.Status.Message, progress.Message)
+					assert.Contains(t, updatedDu.Status.Activities, progress.Message)
+
+					// Call with the same message again to verify deduplication
+					r.OnDataUploadProgress(ctx, namespace, duName, progress)
+					require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: duName, Namespace: namespace}, updatedDu))
+					assert.Equal(t, []string{progress.Message}, updatedDu.Status.Activities)
 				}
 			}
 		})

pkg/controller/pod_volume_backup_controller.go

@@ -641,9 +641,8 @@ func (r *PodVolumeBackupReconciler) OnDataPathProgress(ctx context.Context, name
 		}
 
 		if progress.Message != "" {
-			message := progress.Message + ";"
-			if !strings.HasSuffix(pvb.Status.Message, message) {
-				pvb.Status.Message += message
+			if len(pvb.Status.Activities) == 0 || pvb.Status.Activities[len(pvb.Status.Activities)-1] != progress.Message {
+				pvb.Status.Activities = append(pvb.Status.Activities, progress.Message)
 			}
 		}
 

pkg/controller/pod_volume_backup_controller_test.go

@@ -666,7 +666,7 @@ func TestOnPVBProgress(t *testing.T) {
 			progress := &test.progress
 
 			r.OnDataPathProgress(ctx, namespace, pvbName, progress)
-			if len(test.needErrs) != 0 && !test.needErrs[0] {
+			if len(test.needErrs) == 0 {
 				updatedPvb := &velerov1api.PodVolumeBackup{}
 				require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: pvbName, Namespace: namespace}, updatedPvb))
 				if progress.TotalBytes != -1 {
@@ -680,7 +680,12 @@ func TestOnPVBProgress(t *testing.T) {
 					assert.Equal(t, int64(0), updatedPvb.Status.Progress.BytesDone) // assuming default or original value
 				}
 				if progress.Message != "" {
-					assert.Contains(t, updatedPvb.Status.Message, progress.Message)
+					assert.Contains(t, updatedPvb.Status.Activities, progress.Message)
+
+					// Call with the same message again to verify deduplication
+					r.OnDataPathProgress(ctx, namespace, pvbName, progress)
+					require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: pvbName, Namespace: namespace}, updatedPvb))
+					assert.Equal(t, []string{progress.Message}, updatedPvb.Status.Activities)
 				}
 			}
 		})

pkg/controller/pod_volume_restore_controller.go

@@ -917,9 +917,8 @@ func (r *PodVolumeRestoreReconciler) OnDataPathProgress(ctx context.Context, nam
 		}
 
 		if progress.Message != "" {
-			message := progress.Message + ";"
-			if !strings.HasSuffix(pvr.Status.Message, message) {
-				pvr.Status.Message += message
+			if len(pvr.Status.Activities) == 0 || pvr.Status.Activities[len(pvr.Status.Activities)-1] != progress.Message {
+				pvr.Status.Activities = append(pvr.Status.Activities, progress.Message)
 			}
 		}
 

pkg/controller/pod_volume_restore_controller_test.go

@@ -1512,7 +1512,7 @@ func TestOnPodVolumeRestoreProgress(t *testing.T) {
 			progress := &test.progress
 
 			r.OnDataPathProgress(ctx, namespace, pvrName, progress)
-			if len(test.needErrs) != 0 && !test.needErrs[0] {
+			if len(test.needErrs) == 0 {
 				updatedPVR := &velerov1api.PodVolumeRestore{}
 				require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: pvrName, Namespace: namespace}, updatedPVR))
 				if progress.TotalBytes != -1 {
@@ -1526,7 +1526,12 @@ func TestOnPodVolumeRestoreProgress(t *testing.T) {
 					assert.Equal(t, int64(0), updatedPVR.Status.Progress.BytesDone) // assuming default or original value
 				}
 				if progress.Message != "" {
-					assert.Contains(t, updatedPVR.Status.Message, progress.Message)
+					assert.Contains(t, updatedPVR.Status.Activities, progress.Message)
+
+					// Call with the same message again to verify deduplication
+					r.OnDataPathProgress(ctx, namespace, pvrName, progress)
+					require.NoError(t, r.client.Get(ctx, types.NamespacedName{Name: pvrName, Namespace: namespace}, updatedPVR))
+					assert.Equal(t, []string{progress.Message}, updatedPVR.Status.Activities)
 				}
 			}
 		})