Fix #6118: Do not persist VolumeSnapshot and VolumeSnapshotContent for snapshot DataMover case. (#6366)

1. Because VolumeSnapshot and VolumeSnapshotContent CRs are not kept after backup completed,
   don't persist them in the backup metadata.
2. Add some builder methods needed by CSI plugin.

Signed-off-by: Xun Jiang <jxun@vmware.com>
This commit is contained in:
Xun Jiang/Bruce Jiang
2023-06-08 17:05:17 +08:00
committed by GitHub
parent d7181fba55
commit 9743a7ce56
7 changed files with 237 additions and 6 deletions

View File

@@ -0,0 +1 @@
Do not persist VolumeSnapshot and VolumeSnapshotContent for snapshot DataMover case.

View File

@@ -67,3 +67,9 @@ func (b *PersistentVolumeClaimBuilder) StorageClass(name string) *PersistentVolu
b.object.Spec.StorageClassName = &name
return b
}
// Phase sets the PersistentVolumeClaim's status Phase.
func (b *PersistentVolumeClaimBuilder) Phase(phase corev1api.PersistentVolumeClaimPhase) *PersistentVolumeClaimBuilder {
b.object.Status.Phase = phase
return b
}

View File

@@ -81,3 +81,9 @@ func ForStorageClassSlice(names ...string) *StorageClassBuilder {
func (b *StorageClassBuilder) SliceResult() []*storagev1api.StorageClass {
return b.objectSlice
}
// Provisioner sets StorageClass's provisioner.
func (b *StorageClassBuilder) Provisioner(provisioner string) *StorageClassBuilder {
b.object.Provisioner = provisioner
return b
}

View File

@@ -0,0 +1,62 @@
/*
Copyright the Velero contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package builder
import (
snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
// VolumeSnapshotClassBuilder builds VolumeSnapshotClass objects.
type VolumeSnapshotClassBuilder struct {
object *snapshotv1api.VolumeSnapshotClass
}
// ForVolumeSnapshotClass is the constructor of VolumeSnapshotClassBuilder.
func (b *VolumeSnapshotClassBuilder) ForVolumeSnapshotClass(name string) *VolumeSnapshotClassBuilder {
return &VolumeSnapshotClassBuilder{
object: &snapshotv1api.VolumeSnapshotClass{
TypeMeta: metav1.TypeMeta{
Kind: "VolumeSnapshotClass",
APIVersion: snapshotv1api.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
},
}
}
// Result returns the built VolumeSnapshotClass.
func (b *VolumeSnapshotClassBuilder) Result() *snapshotv1api.VolumeSnapshotClass {
return b.object
}
// Driver sets the driver of built VolumeSnapshotClass.
func (b *VolumeSnapshotClassBuilder) Driver(driver string) *VolumeSnapshotClassBuilder {
b.object.Driver = driver
return b
}
// ObjectMeta applies functional options to the VolumeSnapshotClass's ObjectMeta.
func (b *VolumeSnapshotClassBuilder) ObjectMeta(opts ...ObjectMetaOpt) *VolumeSnapshotClassBuilder {
for _, opt := range opts {
opt(b.object)
}
return b
}

View File

@@ -648,7 +648,9 @@ func (b *backupReconciler) runBackup(backup *pkgbackup.Request) error {
var volumeSnapshots []snapshotv1api.VolumeSnapshot
var volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent
var volumeSnapshotClasses []snapshotv1api.VolumeSnapshotClass
if features.IsEnabled(velerov1api.CSIFeatureFlag) {
if boolptr.IsSetToTrue(backup.Spec.SnapshotMoveData) {
backupLog.Info("backup SnapshotMoveData is set to true, skip VolumeSnapshot resource persistence.")
} else if features.IsEnabled(velerov1api.CSIFeatureFlag) {
selector := label.NewSelectorForBackup(backup.Name)
vscList := &snapshotv1api.VolumeSnapshotContentList{}

View File

@@ -26,6 +26,9 @@ import (
"testing"
"time"
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"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
@@ -44,6 +47,7 @@ import (
pkgbackup "github.com/vmware-tanzu/velero/pkg/backup"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/discovery"
"github.com/vmware-tanzu/velero/pkg/features"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
"github.com/vmware-tanzu/velero/pkg/metrics"
"github.com/vmware-tanzu/velero/pkg/persistence"
@@ -581,6 +585,7 @@ func TestProcessBackupCompletions(t *testing.T) {
expectedResult *velerov1api.Backup
backupExists bool
existenceCheckError error
volumeSnapshot *snapshotv1api.VolumeSnapshot
}{
// Finalizing
{
@@ -1000,16 +1005,139 @@ func TestProcessBackupCompletions(t *testing.T) {
},
},
},
{
name: "backup with snapshot data movement when CSI feature is enabled",
backup: defaultBackup().SnapshotMoveData(true).Result(),
backupLocation: defaultBackupLocation,
defaultVolumesToFsBackup: false,
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: "velero.io/v1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "backup-1",
Annotations: map[string]string{
"velero.io/source-cluster-k8s-major-version": "1",
"velero.io/source-cluster-k8s-minor-version": "16",
"velero.io/source-cluster-k8s-gitversion": "v1.16.4",
"velero.io/resource-timeout": "0s",
},
Labels: map[string]string{
"velero.io/storage-location": "loc-1",
},
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.True(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
Expiration: &timestamp,
CSIVolumeSnapshotsAttempted: 0,
CSIVolumeSnapshotsCompleted: 0,
},
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
{
name: "backup with snapshot data movement set to false when CSI feature is enabled",
backup: defaultBackup().SnapshotMoveData(false).Result(),
//backup: defaultBackup().Result(),
backupLocation: defaultBackupLocation,
defaultVolumesToFsBackup: false,
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: "velero.io/v1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "backup-1",
Annotations: map[string]string{
"velero.io/source-cluster-k8s-major-version": "1",
"velero.io/source-cluster-k8s-minor-version": "16",
"velero.io/source-cluster-k8s-gitversion": "v1.16.4",
"velero.io/resource-timeout": "0s",
},
Labels: map[string]string{
"velero.io/storage-location": "loc-1",
},
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
SnapshotMoveData: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
Expiration: &timestamp,
CSIVolumeSnapshotsAttempted: 1,
CSIVolumeSnapshotsCompleted: 0,
},
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
{
name: "backup with snapshot data movement not set when CSI feature is enabled",
backup: defaultBackup().Result(),
backupLocation: defaultBackupLocation,
defaultVolumesToFsBackup: false,
expectedResult: &velerov1api.Backup{
TypeMeta: metav1.TypeMeta{
Kind: "Backup",
APIVersion: "velero.io/v1",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "backup-1",
Annotations: map[string]string{
"velero.io/source-cluster-k8s-major-version": "1",
"velero.io/source-cluster-k8s-minor-version": "16",
"velero.io/source-cluster-k8s-gitversion": "v1.16.4",
"velero.io/resource-timeout": "0s",
},
Labels: map[string]string{
"velero.io/storage-location": "loc-1",
},
},
Spec: velerov1api.BackupSpec{
StorageLocation: defaultBackupLocation.Name,
DefaultVolumesToFsBackup: boolptr.False(),
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFinalizing,
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
Expiration: &timestamp,
CSIVolumeSnapshotsAttempted: 1,
CSIVolumeSnapshotsCompleted: 0,
},
},
volumeSnapshot: builder.ForVolumeSnapshot("velero", "testVS").ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, "backup-1")).Result(),
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
formatFlag := logging.FormatText
var (
logger = logging.DefaultLogger(logrus.DebugLevel, formatFlag)
pluginManager = new(pluginmocks.Manager)
backupStore = new(persistencemocks.BackupStore)
backupper = new(fakeBackupper)
logger = logging.DefaultLogger(logrus.DebugLevel, formatFlag)
pluginManager = new(pluginmocks.Manager)
backupStore = new(persistencemocks.BackupStore)
backupper = new(fakeBackupper)
snapshotClient = snapshotfake.NewSimpleClientset()
sharedInformer = snapshotinformers.NewSharedInformerFactory(snapshotClient, 0)
snapshotLister = sharedInformer.Snapshot().V1().VolumeSnapshots().Lister()
)
var fakeClient kbclient.Client
@@ -1020,6 +1148,12 @@ func TestProcessBackupCompletions(t *testing.T) {
fakeClient = velerotest.NewFakeControllerRuntimeClient(t)
}
if test.volumeSnapshot != nil {
snapshotClient.SnapshotV1().VolumeSnapshots(test.volumeSnapshot.Namespace).Create(context.Background(), test.volumeSnapshot, metav1.CreateOptions{})
sharedInformer.Snapshot().V1().VolumeSnapshots().Informer().GetStore().Add(test.volumeSnapshot)
sharedInformer.WaitForCacheSync(make(chan struct{}))
}
apiServer := velerotest.NewAPIServer(t)
apiServer.DiscoveryClient.FakedServerVersion = &version.Info{
@@ -1050,6 +1184,8 @@ func TestProcessBackupCompletions(t *testing.T) {
backupStoreGetter: NewFakeSingleObjectBackupStoreGetter(backupStore),
backupper: backupper,
formatFlag: formatFlag,
volumeSnapshotClient: snapshotClient,
volumeSnapshotLister: snapshotLister,
}
pluginManager.On("GetBackupItemActionsV2").Return(nil, nil)
@@ -1079,10 +1215,20 @@ func TestProcessBackupCompletions(t *testing.T) {
// add the default backup storage location to the clientset and the informer/lister store
require.NoError(t, fakeClient.Create(context.Background(), defaultBackupLocation))
// Enable CSI feature flag for SnapshotDataMovement test.
if strings.Contains(test.name, "backup with snapshot data movement") {
features.Enable(velerov1api.CSIFeatureFlag)
}
actualResult, err := c.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace, Name: test.backup.Name}})
assert.Equal(t, actualResult, ctrl.Result{})
assert.Nil(t, err)
// Disable CSI feature to not impact other test cases.
if strings.Contains(test.name, "backup with snapshot data movement") {
features.Disable(velerov1api.CSIFeatureFlag)
}
res := &velerov1api.Backup{}
err = c.kbClient.Get(context.Background(), kbclient.ObjectKey{Namespace: test.backup.Namespace, Name: test.backup.Name}, res)
require.NoError(t, err)

View File

@@ -460,7 +460,15 @@ func TestRestorePodVolumes(t *testing.T) {
assert.Equal(t, test.errs[i].err, errMsg)
} else {
assert.EqualError(t, errs[i], test.errs[i].err)
for i := 0; i < len(errs); i++ {
j := 0
for ; j < len(test.errs); j++ {
if errs[i].Error() == test.errs[j].err {
break
}
}
assert.Equal(t, true, j < len(test.errs))
}
}
}
}