From 886e074b55c9c9bab15dda5cbd1fe0976f026760 Mon Sep 17 00:00:00 2001
From: Tiger Kaovilai <tkaovila@redhat.com>
Date: Wed, 1 Nov 2023 13:13:50 -0400
Subject: [PATCH] Add PatchResource unit test for backup status

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
---
 pkg/builder/backup_builder.go                 |  6 ++
 pkg/controller/backup_controller_test.go      | 67 +++++++++++++++++++
 .../backup_operations_controller.go           |  2 +
 3 files changed, 75 insertions(+)

diff --git a/pkg/builder/backup_builder.go b/pkg/builder/backup_builder.go
index 275852bd13..b689bbcae7 100644
--- a/pkg/builder/backup_builder.go
+++ b/pkg/builder/backup_builder.go
@@ -299,3 +299,9 @@ func (b *BackupBuilder) DataMover(name string) *BackupBuilder {
 	b.object.Spec.DataMover = name
 	return b
 }
+
+// WithStatus sets the Backup's status.
+func (b *BackupBuilder) WithStatus(status velerov1api.BackupStatus) *BackupBuilder {
+	b.object.Status = status
+	return b
+}
diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go
index f187877330..f4a1293705 100644
--- a/pkg/controller/backup_controller_test.go
+++ b/pkg/controller/backup_controller_test.go
@@ -21,11 +21,13 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"reflect"
 	"sort"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
 	snapshotfake "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/fake"
 	snapshotinformers "github.com/kubernetes-csi/external-snapshotter/client/v4/informers/externalversions"
@@ -41,8 +43,13 @@ import (
 	"k8s.io/utils/clock"
 	testclocks "k8s.io/utils/clock/testing"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 
+	kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
+
+	fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	pkgbackup "github.com/vmware-tanzu/velero/pkg/backup"
 	"github.com/vmware-tanzu/velero/pkg/builder"
@@ -1665,3 +1672,63 @@ func Test_getLastSuccessBySchedule(t *testing.T) {
 		})
 	}
 }
+
+// Unit tests to make sure that the backup's status is updated correctly during reconcile.
+// To clear up confusion whether status can be updated with Patch alone without status writer and not kbClient.Status().Patch()
+func TestPatchResourceWorksWithStatus(t *testing.T) {
+	type args struct {
+		original *velerov1api.Backup
+		updated  *velerov1api.Backup
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "patch backup status",
+			args: args{
+				original: defaultBackup().SnapshotMoveData(false).Result(),
+				updated: defaultBackup().SnapshotMoveData(false).WithStatus(velerov1api.BackupStatus{
+					CSIVolumeSnapshotsCompleted: 1,
+				}).Result(),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			scheme := runtime.NewScheme()
+			error := velerov1api.AddToScheme(scheme)
+			if error != nil {
+				t.Errorf("PatchResource() error = %v", error)
+			}
+			fakeClient := fakeClient.NewClientBuilder().WithScheme(scheme).WithObjects(tt.args.original).Build()
+			fromCluster := &velerov1api.Backup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tt.args.original.Name,
+					Namespace: tt.args.original.Namespace,
+				},
+			}
+			// check original exists
+			if err := fakeClient.Get(context.Background(), client.ObjectKeyFromObject(tt.args.updated), fromCluster); err != nil {
+				t.Errorf("PatchResource() error = %v", err)
+			}
+			// ignore resourceVersion
+			tt.args.updated.ResourceVersion = fromCluster.ResourceVersion
+			tt.args.original.ResourceVersion = fromCluster.ResourceVersion
+			if err := kubeutil.PatchResource(tt.args.original, tt.args.updated, fakeClient); (err != nil) != tt.wantErr {
+				t.Errorf("PatchResource() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			// check updated exists
+			if err := fakeClient.Get(context.Background(), client.ObjectKeyFromObject(tt.args.updated), fromCluster); err != nil {
+				t.Errorf("PatchResource() error = %v", err)
+			}
+
+			// check fromCluster is equal to updated
+			if !reflect.DeepEqual(fromCluster, tt.args.updated) {
+				t.Error(cmp.Diff(fromCluster, tt.args.updated))
+			}
+		})
+
+	}
+}
diff --git a/pkg/controller/backup_operations_controller.go b/pkg/controller/backup_operations_controller.go
index f00e9c2056..5e9a5cfd37 100644
--- a/pkg/controller/backup_operations_controller.go
+++ b/pkg/controller/backup_operations_controller.go
@@ -275,6 +275,8 @@ func (c *backupOperationsReconciler) updateBackupAndOperationsJSON(
 	return nil
 }
 
+// check progress of backupItemOperations
+// return: inProgressOperations, changes, completedCount, failedCount, errs
 func getBackupItemOperationProgress(
 	backup *velerov1api.Backup,
 	pluginManager clientmgmt.Manager,