Skip to content

Commit

Permalink
Merge pull request #6533 from Lyndon-Li/data-mover-wrong-bsl-after-sync
Browse files Browse the repository at this point in the history 
Data mover wrong bsl after sync
  • Loading branch information
@Lyndon-Li
Lyndon-Li committed Jul 25, 2023
2 parents 4379b9a + 74bf03b commit 9c8275e
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 26 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/6533-Lyndon-Li
View file
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 3 additions & 2 deletions pkg/cmd/server/plugin/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
1 change: 1 addition & 0 deletions pkg/controller/backup_sync_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 19 additions & 8 deletions pkg/restore/dataupload_retrieve_action.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
}
}

Expand All @@ -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,
Expand Down Expand Up @@ -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")
Expand Down
67 changes: 51 additions & 16 deletions pkg/restore/dataupload_retrieve_action_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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: "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")).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: "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 {
Expand All @@ -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)
}
})
}
Expand Down

0 comments on commit 9c8275e

Please sign in to comment.