From 74bf03b2721fcd2e67e6d5a44f62740880fa7514 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Fri, 21 Jul 2023 12:45:35 +0800 Subject: [PATCH] data mover wrong bsl after sync Signed-off-by: Lyndon-Li --- changelogs/unreleased/6533-Lyndon-Li | 1 + pkg/cmd/server/plugin/plugin.go | 5 +- pkg/controller/backup_sync_controller.go | 1 + pkg/restore/dataupload_retrieve_action.go | 27 +++++--- .../dataupload_retrieve_action_test.go | 67 ++++++++++++++----- 5 files changed, 75 insertions(+), 26 deletions(-) create mode 100644 changelogs/unreleased/6533-Lyndon-Li diff --git a/changelogs/unreleased/6533-Lyndon-Li b/changelogs/unreleased/6533-Lyndon-Li new file mode 100644 index 000000000..ec1b71957 --- /dev/null +++ b/changelogs/unreleased/6533-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #6534, reset PVB CR's StorageLocation to the latest one during backup sync as same as the backup CR. Also fix similar problem with DataUploadResult for data mover restore. \ No newline at end of file diff --git a/pkg/cmd/server/plugin/plugin.go b/pkg/cmd/server/plugin/plugin.go index 45e45389a..aabdebb02 100644 --- a/pkg/cmd/server/plugin/plugin.go +++ b/pkg/cmd/server/plugin/plugin.go @@ -248,10 +248,11 @@ func newSecretRestoreItemAction(f client.Factory) plugincommon.HandlerInitialize func newDataUploadRetrieveAction(f client.Factory) plugincommon.HandlerInitializer { return func(logger logrus.FieldLogger) (interface{}, error) { - client, err := f.KubeClient() + client, err := f.KubebuilderClient() if err != nil { return nil, err } - return restore.NewDataUploadRetrieveAction(logger, client.CoreV1().ConfigMaps(f.Namespace())), nil + + return restore.NewDataUploadRetrieveAction(logger, client), nil } } diff --git a/pkg/controller/backup_sync_controller.go b/pkg/controller/backup_sync_controller.go index 0ffc0f977..b3d996d2f 100644 --- a/pkg/controller/backup_sync_controller.go +++ b/pkg/controller/backup_sync_controller.go @@ -208,6 +208,7 @@ func (b *backupSyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) podVolumeBackup.Namespace = backup.Namespace podVolumeBackup.ResourceVersion = "" + podVolumeBackup.Spec.BackupStorageLocation = location.Name err = b.client.Create(ctx, podVolumeBackup, &client.CreateOptions{}) switch { diff --git a/pkg/restore/dataupload_retrieve_action.go b/pkg/restore/dataupload_retrieve_action.go index a691d653c..c345e908f 100644 --- a/pkg/restore/dataupload_retrieve_action.go +++ b/pkg/restore/dataupload_retrieve_action.go @@ -25,7 +25,8 @@ import ( corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1" @@ -34,14 +35,14 @@ import ( ) type DataUploadRetrieveAction struct { - logger logrus.FieldLogger - configMapClient corev1client.ConfigMapInterface + logger logrus.FieldLogger + client client.Client } -func NewDataUploadRetrieveAction(logger logrus.FieldLogger, configMapClient corev1client.ConfigMapInterface) *DataUploadRetrieveAction { +func NewDataUploadRetrieveAction(logger logrus.FieldLogger, client client.Client) *DataUploadRetrieveAction { return &DataUploadRetrieveAction{ - logger: logger, - configMapClient: configMapClient, + logger: logger, + client: client, } } @@ -60,8 +61,18 @@ func (d *DataUploadRetrieveAction) Execute(input *velero.RestoreItemActionExecut return nil, errors.Wrap(err, "unable to convert unstructured item to DataUpload.") } + backup := &velerov1api.Backup{} + err := d.client.Get(context.Background(), types.NamespacedName{ + Namespace: input.Restore.Namespace, + Name: input.Restore.Spec.BackupName, + }, backup) + if err != nil { + d.logger.WithError(err).Errorf("Fail to get backup for restore %s.", input.Restore.Name) + return nil, errors.Wrapf(err, "error to get backup for restore %s", input.Restore.Name) + } + dataUploadResult := velerov2alpha1.DataUploadResult{ - BackupStorageLocation: dataUpload.Spec.BackupStorageLocation, + BackupStorageLocation: backup.Spec.StorageLocation, DataMover: dataUpload.Spec.DataMover, SnapshotID: dataUpload.Status.SnapshotID, SourceNamespace: dataUpload.Spec.SourceNamespace, @@ -93,7 +104,7 @@ func (d *DataUploadRetrieveAction) Execute(input *velero.RestoreItemActionExecut }, } - _, err = d.configMapClient.Create(context.Background(), &cm, metav1.CreateOptions{}) + err = d.client.Create(context.Background(), &cm, &client.CreateOptions{}) if err != nil { d.logger.Errorf("fail to create DataUploadResult ConfigMap %s/%s: %s", cm.Namespace, cm.Name, err.Error()) return nil, errors.Wrap(err, "fail to create DataUploadResult ConfigMap") diff --git a/pkg/restore/dataupload_retrieve_action_test.go b/pkg/restore/dataupload_retrieve_action_test.go index be8c65368..2624f0886 100644 --- a/pkg/restore/dataupload_retrieve_action_test.go +++ b/pkg/restore/dataupload_retrieve_action_test.go @@ -23,10 +23,11 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/kubernetes/fake" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1" @@ -37,31 +38,58 @@ import ( ) func TestDataUploadRetrieveActionExectue(t *testing.T) { + scheme := runtime.NewScheme() + velerov1.AddToScheme(scheme) + corev1.AddToScheme(scheme) + tests := []struct { name string dataUpload *velerov2alpha1.DataUpload restore *velerov1.Restore expectedDataUploadResult *corev1.ConfigMap expectedErr string + runtimeScheme *runtime.Scheme + veleroObjs []runtime.Object }{ { - name: "DataUploadRetrieve Action test", - dataUpload: builder.ForDataUpload("velero", "testDU").SourceNamespace("testNamespace").SourcePVC("testPVC").Result(), - restore: builder.ForRestore("velero", "testRestore").ObjectMeta(builder.WithUID("testingUID")).Result(), - expectedDataUploadResult: builder.ForConfigMap("velero", "").ObjectMeta(builder.WithGenerateName("testDU-"), builder.WithLabels(velerov1.PVCNamespaceNameLabel, "testNamespace.testPVC", velerov1.RestoreUIDLabel, "testingUID", velerov1.ResourceUsageLabel, string(velerov1.VeleroResourceUsageDataUploadResult))).Data("testingUID", `{"backupStorageLocation":"","sourceNamespace":"testNamespace"}`).Result(), + name: "error to find backup", + dataUpload: builder.ForDataUpload("velero", "testDU").SourceNamespace("testNamespace").SourcePVC("testPVC").Result(), + restore: builder.ForRestore("velero", "testRestore").ObjectMeta(builder.WithUID("testingUID")).Backup("testBackup").Result(), + runtimeScheme: scheme, + expectedErr: "error to get backup for restore testRestore: backups.velero.io \"testBackup\" not found", }, { - name: "Long source namespace and PVC name should also work", - dataUpload: builder.ForDataUpload("velero", "testDU").SourceNamespace("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1").SourcePVC("kibishii-data-kibishii-deployment-0").Result(), - restore: builder.ForRestore("velero", "testRestore").ObjectMeta(builder.WithUID("testingUID")).Result(), - expectedDataUploadResult: builder.ForConfigMap("velero", "").ObjectMeta(builder.WithGenerateName("testDU-"), builder.WithLabels(velerov1.PVCNamespaceNameLabel, "migre209d0da-49c7-45ba-8d5a-3e59fd591ec1.kibishii-data-ki152333", velerov1.RestoreUIDLabel, "testingUID", velerov1.ResourceUsageLabel, string(velerov1.VeleroResourceUsageDataUploadResult))).Data("testingUID", `{"backupStorageLocation":"","sourceNamespace":"migre209d0da-49c7-45ba-8d5a-3e59fd591ec1"}`).Result(), + name: "DataUploadRetrieve Action test", + dataUpload: builder.ForDataUpload("velero", "testDU").SourceNamespace("testNamespace").SourcePVC("testPVC").Result(), + restore: builder.ForRestore("velero", "testRestore").ObjectMeta(builder.WithUID("testingUID")).Backup("testBackup").Result(), + runtimeScheme: scheme, + veleroObjs: []runtime.Object{ + builder.ForBackup("velero", "testBackup").StorageLocation("testLocation").Result(), + }, + expectedDataUploadResult: builder.ForConfigMap("velero", "").ObjectMeta(builder.WithGenerateName("testDU-"), builder.WithLabels(velerov1.PVCNamespaceNameLabel, "testNamespace.testPVC", velerov1.RestoreUIDLabel, "testingUID", velerov1.ResourceUsageLabel, string(velerov1.VeleroResourceUsageDataUploadResult))).Data("testingUID", `{"backupStorageLocation":"testLocation","sourceNamespace":"testNamespace"}`).Result(), + }, + { + name: "Long source namespace and PVC name should also work", + dataUpload: builder.ForDataUpload("velero", "testDU").SourceNamespace("migre209d0da-49c7-45ba-8d5a-3e59fd591ec1").SourcePVC("kibishii-data-kibishii-deployment-0").Result(), + restore: builder.ForRestore("velero", "testRestore").ObjectMeta(builder.WithUID("testingUID")).Backup("testBackup").Result(), + runtimeScheme: scheme, + veleroObjs: []runtime.Object{ + builder.ForBackup("velero", "testBackup").StorageLocation("testLocation").Result(), + }, + expectedDataUploadResult: builder.ForConfigMap("velero", "").ObjectMeta(builder.WithGenerateName("testDU-"), builder.WithLabels(velerov1.PVCNamespaceNameLabel, "migre209d0da-49c7-45ba-8d5a-3e59fd591ec1.kibishii-data-ki152333", velerov1.RestoreUIDLabel, "testingUID", velerov1.ResourceUsageLabel, string(velerov1.VeleroResourceUsageDataUploadResult))).Data("testingUID", `{"backupStorageLocation":"testLocation","sourceNamespace":"migre209d0da-49c7-45ba-8d5a-3e59fd591ec1"}`).Result(), }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { logger := velerotest.NewLogger() - cmClient := fake.NewSimpleClientset() + + fakeClientBuilder := fake.NewClientBuilder() + if tc.runtimeScheme != nil { + fakeClientBuilder = fakeClientBuilder.WithScheme(tc.runtimeScheme) + } + + fakeClient := fakeClientBuilder.WithRuntimeObjects(tc.veleroObjs...).Build() var unstructuredDataUpload map[string]interface{} if tc.dataUpload != nil { @@ -74,21 +102,28 @@ func TestDataUploadRetrieveActionExectue(t *testing.T) { ItemFromBackup: &unstructured.Unstructured{Object: unstructuredDataUpload}, } - action := NewDataUploadRetrieveAction(logger, cmClient.CoreV1().ConfigMaps("velero")) + action := NewDataUploadRetrieveAction(logger, fakeClient) _, err := action.Execute(&input) if tc.expectedErr != "" { require.Equal(t, tc.expectedErr, err.Error()) + } else { + require.NoError(t, err) } - require.NoError(t, err) if tc.expectedDataUploadResult != nil { - cmList, err := cmClient.CoreV1().ConfigMaps("velero").List(context.Background(), metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s,%s=%s", velerov1.RestoreUIDLabel, "testingUID", velerov1.PVCNamespaceNameLabel, label.GetValidName(tc.dataUpload.Spec.SourceNamespace+"."+tc.dataUpload.Spec.SourcePVC)), + var cmList corev1.ConfigMapList + err := fakeClient.List(context.Background(), &cmList, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + velerov1.RestoreUIDLabel: "testingUID", + velerov1.PVCNamespaceNameLabel: label.GetValidName(tc.dataUpload.Spec.SourceNamespace + "." + tc.dataUpload.Spec.SourcePVC), + }), }) + require.NoError(t, err) // debug fmt.Printf("CM: %s\n", &cmList.Items[0]) - require.Equal(t, *tc.expectedDataUploadResult, cmList.Items[0]) + require.Equal(t, tc.expectedDataUploadResult.Labels, cmList.Items[0].Labels) + require.Equal(t, tc.expectedDataUploadResult.Data, cmList.Items[0].Data) } }) }