From 5f5db2eacae4c859f0aec6eb934508a9ac0b5267 Mon Sep 17 00:00:00 2001 From: danfengl Date: Wed, 26 Apr 2023 02:33:21 +0000 Subject: [PATCH] Fix context issues for several E2E tests 1. Fix context issues produced by previous PR, increase timeout or add case scpoed global timeout param to make backup/restore command timeout configurable. 2. Add global param for storage class name using by test cases; 3. Fix param DefaultVolumesToFsBackup usage issue: set DefaultVolumesToFsBackup to false in backup CLI in case it was set to true in install CLI. 4. Make namespace names of each namespace mapping test unique from being interfered by each other. Signed-off-by: danfengl --- test/e2e/backup/backup.go | 14 ++++++++++---- test/e2e/backups/sync_backups.go | 11 +++++------ test/e2e/backups/ttl.go | 8 ++++---- test/e2e/basic/namespace-mapping.go | 6 +++--- test/e2e/basic/storage-class-changing.go | 2 +- test/e2e/pv-backup/pv-backup-filter.go | 2 +- test/e2e/scale/multiple_namespaces.go | 4 +++- test/e2e/schedule/schedule.go | 2 +- test/e2e/test/test.go | 17 +++++++++++++---- test/e2e/types.go | 2 ++ test/e2e/util/kibishii/kibishii_utils.go | 18 ++++++++++++------ test/e2e/util/velero/velero_utils.go | 14 ++++++++++++++ 12 files changed, 69 insertions(+), 31 deletions(-) diff --git a/test/e2e/backup/backup.go b/test/e2e/backup/backup.go index 7532c22e4..68e0c55cd 100644 --- a/test/e2e/backup/backup.go +++ b/test/e2e/backup/backup.go @@ -58,7 +58,7 @@ func BackupRestoreTest(useVolumeSnapshots bool) { var err error flag.Parse() UUIDgen, err = uuid.NewRandom() - kibishiiNamespace = "kibishii-workload" + UUIDgen.String() + kibishiiNamespace = "k-" + UUIDgen.String() Expect(err).To(Succeed()) }) @@ -87,6 +87,9 @@ func BackupRestoreTest(useVolumeSnapshots bool) { if useVolumeSnapshots { //Install node agent also veleroCfg.UseNodeAgent = useVolumeSnapshots + // DefaultVolumesToFsBackup should be mutually exclusive with useVolumeSnapshots in installation CLI, + // otherwise DefaultVolumesToFsBackup need to be set to false in backup CLI when taking volume snapshot + // Make sure DefaultVolumesToFsBackup was set to false in backup CLI veleroCfg.DefaultVolumesToFsBackup = useVolumeSnapshots } else { veleroCfg.DefaultVolumesToFsBackup = !useVolumeSnapshots @@ -121,9 +124,11 @@ func BackupRestoreTest(useVolumeSnapshots bool) { if veleroCfg.InstallVelero { if useVolumeSnapshots { veleroCfg.DefaultVolumesToFsBackup = !useVolumeSnapshots - } else { + } else { //FS volume backup // Install VolumeSnapshots also veleroCfg.UseVolumeSnapshots = !useVolumeSnapshots + // DefaultVolumesToFsBackup is false in installation CLI here, + // so must set DefaultVolumesToFsBackup to be true in backup CLI come after veleroCfg.DefaultVolumesToFsBackup = useVolumeSnapshots } @@ -144,7 +149,7 @@ func BackupRestoreTest(useVolumeSnapshots bool) { Expect(CreateSecretFromFiles(context.TODO(), *veleroCfg.ClientToInstallVelero, veleroCfg.VeleroNamespace, secretName, files)).To(Succeed()) // Create additional BSL using credential - additionalBsl := fmt.Sprintf("bsl-%s", UUIDgen) + additionalBsl := "add-bsl" Expect(VeleroCreateBackupLocation(context.TODO(), veleroCfg.VeleroCLI, veleroCfg.VeleroNamespace, @@ -169,7 +174,8 @@ func BackupRestoreTest(useVolumeSnapshots bool) { restoreName = fmt.Sprintf("%s-%s", restoreName, UUIDgen) } veleroCfg.ProvideSnapshotsVolumeParam = !provideSnapshotVolumesParmInBackup - Expect(RunKibishiiTests(veleroCfg, backupName, restoreName, bsl, kibishiiNamespace, useVolumeSnapshots, !useVolumeSnapshots)).To(Succeed(), + workloadNmespace := kibishiiNamespace + bsl + Expect(RunKibishiiTests(veleroCfg, backupName, restoreName, bsl, workloadNmespace, useVolumeSnapshots, !useVolumeSnapshots)).To(Succeed(), "Failed to successfully backup and restore Kibishii namespace using BSL %s", bsl) } }) diff --git a/test/e2e/backups/sync_backups.go b/test/e2e/backups/sync_backups.go index c8b230528..1bfea90b2 100644 --- a/test/e2e/backups/sync_backups.go +++ b/test/e2e/backups/sync_backups.go @@ -50,8 +50,6 @@ func (b *SyncBackups) Init() { func BackupsSyncTest() { test := new(SyncBackups) - ctx, ctxCancel := context.WithCancel(context.Background()) - defer ctxCancel() var ( err error ) @@ -79,7 +77,8 @@ func BackupsSyncTest() { It("Backups in object storage should be synced to a new Velero successfully", func() { test.Init() - + ctx, ctxCancel := context.WithTimeout(context.Background(), 30*time.Minute) + defer ctxCancel() By(fmt.Sprintf("Prepare workload as target to backup by creating namespace %s namespace", test.testNS)) Expect(CreateNamespace(ctx, *VeleroCfg.ClientToInstallVelero, test.testNS)).To(Succeed(), fmt.Sprintf("Failed to create %s namespace", test.testNS)) @@ -114,12 +113,13 @@ func BackupsSyncTest() { }) By("Check all backups in object storage are synced to Velero", func() { - Expect(test.IsBackupsSynced()).To(Succeed(), fmt.Sprintf("Failed to sync backup %s from object storage", test.backupName)) + Expect(test.IsBackupsSynced(ctx, ctxCancel)).To(Succeed(), fmt.Sprintf("Failed to sync backup %s from object storage", test.backupName)) }) }) It("Deleted backups in object storage are synced to be deleted in Velero", func() { test.Init() + ctx, ctxCancel := context.WithTimeout(context.Background(), 30*time.Minute) defer ctxCancel() By(fmt.Sprintf("Prepare workload as target to backup by creating namespace in %s namespace", test.testNS), func() { Expect(CreateNamespace(ctx, *VeleroCfg.ClientToInstallVelero, test.testNS)).To(Succeed(), @@ -163,8 +163,7 @@ func BackupsSyncTest() { }) } -func (b *SyncBackups) IsBackupsSynced() error { - ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Minute) +func (b *SyncBackups) IsBackupsSynced(ctx context.Context, ctxCancel context.CancelFunc) error { defer ctxCancel() return WaitForBackupToBeCreated(ctx, VeleroCfg.VeleroCLI, b.backupName, 10*time.Minute) } diff --git a/test/e2e/backups/ttl.go b/test/e2e/backups/ttl.go index 3dc0ec0e2..037c0a2b5 100644 --- a/test/e2e/backups/ttl.go +++ b/test/e2e/backups/ttl.go @@ -56,8 +56,6 @@ func (b *TTL) Init() { } func TTLTest() { - ctx, contextCancel := context.WithCancel(context.Background()) - defer contextCancel() var err error var veleroCfg VeleroConfig useVolumeSnapshots := true @@ -65,8 +63,6 @@ func TTLTest() { veleroCfg = VeleroCfg client := *veleroCfg.ClientToInstallVelero - //Expect(err).To(Succeed(), "Failed to instantiate cluster client for backup tests") - BeforeEach(func() { flag.Parse() veleroCfg = VeleroCfg @@ -87,12 +83,16 @@ func TTLTest() { if veleroCfg.InstallVelero { Expect(VeleroUninstall(context.Background(), veleroCfg.VeleroCLI, veleroCfg.VeleroNamespace)).To(Succeed()) } + ctx, ctxCancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer ctxCancel() Expect(DeleteNamespace(ctx, client, test.testNS, false)).To(Succeed(), fmt.Sprintf("Failed to delete the namespace %s", test.testNS)) } }) It("Backups in object storage should be synced to a new Velero successfully", func() { test.Init() + ctx, ctxCancel := context.WithTimeout(context.Background(), 1*time.Hour) + defer ctxCancel() By(fmt.Sprintf("Prepare workload as target to backup by creating namespace %s namespace", test.testNS), func() { Expect(CreateNamespace(ctx, client, test.testNS)).To(Succeed(), fmt.Sprintf("Failed to create %s namespace", test.testNS)) diff --git a/test/e2e/basic/namespace-mapping.go b/test/e2e/basic/namespace-mapping.go index 0de695305..40e2767e5 100644 --- a/test/e2e/basic/namespace-mapping.go +++ b/test/e2e/basic/namespace-mapping.go @@ -24,9 +24,9 @@ type NamespaceMapping struct { const NamespaceBaseName string = "ns-mp-" var OneNamespaceMappingResticTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "1"}, UseVolumeSnapshots: false}}) -var MultiNamespacesMappingResticTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "1", NamespaceBaseName + "2"}, UseVolumeSnapshots: false}}) -var OneNamespaceMappingSnapshotTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "1"}, UseVolumeSnapshots: true}}) -var MultiNamespacesMappingSnapshotTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "1", NamespaceBaseName + "2"}, UseVolumeSnapshots: true}}) +var MultiNamespacesMappingResticTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "2", NamespaceBaseName + "3"}, UseVolumeSnapshots: false}}) +var OneNamespaceMappingSnapshotTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "4"}, UseVolumeSnapshots: true}}) +var MultiNamespacesMappingSnapshotTest func() = TestFunc(&NamespaceMapping{TestCase: TestCase{NSBaseName: NamespaceBaseName, NSIncluded: &[]string{NamespaceBaseName + "5", NamespaceBaseName + "6"}, UseVolumeSnapshots: true}}) func (n *NamespaceMapping) Init() error { n.VeleroCfg = VeleroCfg diff --git a/test/e2e/basic/storage-class-changing.go b/test/e2e/basic/storage-class-changing.go index ee931588a..d05ffcd80 100644 --- a/test/e2e/basic/storage-class-changing.go +++ b/test/e2e/basic/storage-class-changing.go @@ -49,7 +49,7 @@ func (s *StorageClasssChanging) Init() error { s.BackupName = "backup-sc-" + UUIDgen.String() s.RestoreName = "restore-" + UUIDgen.String() s.srcStorageClass = "default" - s.desStorageClass = "e2e-storage-class" + s.desStorageClass = StorageClassName s.labels = map[string]string{"velero.io/change-storage-class": "RestoreItemAction", "velero.io/plugin-config": ""} s.data = map[string]string{s.srcStorageClass: s.desStorageClass} diff --git a/test/e2e/pv-backup/pv-backup-filter.go b/test/e2e/pv-backup/pv-backup-filter.go index a144a7a7e..cb319a189 100644 --- a/test/e2e/pv-backup/pv-backup-filter.go +++ b/test/e2e/pv-backup/pv-backup-filter.go @@ -99,7 +99,7 @@ func (p *PVBackupFiltering) CreateResources() error { podName := fmt.Sprintf("pod-%d", i) pods = append(pods, podName) By(fmt.Sprintf("Create pod %s in namespace %s", podName, ns), func() { - pod, err := CreatePod(p.Client, ns, podName, "e2e-storage-class", "", volumes, nil, nil) + pod, err := CreatePod(p.Client, ns, podName, StorageClassName, "", volumes, nil, nil) Expect(err).To(Succeed()) ann := map[string]string{ p.annotation: volumesToAnnotation, diff --git a/test/e2e/scale/multiple_namespaces.go b/test/e2e/scale/multiple_namespaces.go index d63880c6b..d17f37f6b 100644 --- a/test/e2e/scale/multiple_namespaces.go +++ b/test/e2e/scale/multiple_namespaces.go @@ -17,8 +17,10 @@ limitations under the License. package scale import ( + "time" + basic "github.com/vmware-tanzu/velero/test/e2e/basic/resources-check" . "github.com/vmware-tanzu/velero/test/e2e/test" ) -var MultiNSBackupRestore func() = TestFunc(&basic.MultiNSBackup{IsScalTest: true}) +var MultiNSBackupRestore func() = TestFunc(&basic.MultiNSBackup{IsScalTest: true, TestCase: TestCase{Timeout: time.Hour}}) diff --git a/test/e2e/schedule/schedule.go b/test/e2e/schedule/schedule.go index bcd7e1f91..92100880e 100644 --- a/test/e2e/schedule/schedule.go +++ b/test/e2e/schedule/schedule.go @@ -92,7 +92,7 @@ func (n *ScheduleBackup) Backup() error { return nil } func (n *ScheduleBackup) Destroy() error { - ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Minute) + ctx, ctxCancel := context.WithTimeout(context.Background(), 60*time.Minute) defer ctxCancel() By(fmt.Sprintf("Schedule %s is created without any delay\n", n.ScheduleName), func() { creationTimestamp, err := GetSchedule(context.Background(), VeleroCfg.VeleroNamespace, n.ScheduleName) diff --git a/test/e2e/test/test.go b/test/e2e/test/test.go index 20c90dbbd..3d1c06017 100644 --- a/test/e2e/test/test.go +++ b/test/e2e/test/test.go @@ -72,12 +72,18 @@ type TestCase struct { UseVolumeSnapshots bool VeleroCfg VeleroConfig RestorePhaseExpect velerov1api.RestorePhase + Timeout time.Duration } func TestFunc(test VeleroBackupRestoreTest) func() { return func() { Expect(test.Init()).To(Succeed(), "Failed to instantiate test cases") veleroCfg := test.GetTestCase().VeleroCfg + // If TestCase.Timeout is not set, then make 10 minutes as default value for backup + // or restore CLI + if test.GetTestCase().Timeout == 0 { + test.GetTestCase().Timeout = 10 * time.Minute + } BeforeEach(func() { flag.Parse() veleroCfg := test.GetTestCase().VeleroCfg @@ -110,6 +116,9 @@ func TestFuncWithMultiIt(tests []VeleroBackupRestoreTest) func() { var veleroCfg VeleroConfig for k := range tests { Expect(tests[k].Init()).To(Succeed(), fmt.Sprintf("Failed to instantiate test %s case", tests[k].GetTestMsg().Desc)) + if tests[k].GetTestCase().Timeout == 0 { + tests[k].GetTestCase().Timeout = 10 * time.Minute + } veleroCfg = tests[k].GetTestCase().VeleroCfg useVolumeSnapshots = tests[k].GetTestCase().UseVolumeSnapshots } @@ -158,7 +167,7 @@ func (t *TestCase) StartRun() error { } func (t *TestCase) Backup() error { - ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Minute) + ctx, ctxCancel := context.WithTimeout(context.Background(), t.Timeout) defer ctxCancel() veleroCfg := t.GetTestCase().VeleroCfg if err := VeleroBackupExec(ctx, veleroCfg.VeleroCLI, veleroCfg.VeleroNamespace, t.BackupName, t.BackupArgs); err != nil { @@ -169,7 +178,7 @@ func (t *TestCase) Backup() error { } func (t *TestCase) Destroy() error { - ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Minute) + ctx, ctxCancel := context.WithTimeout(context.Background(), 60*time.Minute) defer ctxCancel() By(fmt.Sprintf("Start to destroy namespace %s......", t.NSBaseName), func() { Expect(CleanupNamespacesWithPoll(ctx, t.Client, t.NSBaseName)).To(Succeed(), "Could cleanup retrieve namespaces") @@ -178,7 +187,7 @@ func (t *TestCase) Destroy() error { } func (t *TestCase) Restore() error { - ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Minute) + ctx, ctxCancel := context.WithTimeout(context.Background(), t.Timeout) defer ctxCancel() veleroCfg := t.GetTestCase().VeleroCfg // the snapshots of AWS may be still in pending status when do the restore, wait for a while @@ -206,7 +215,7 @@ func (t *TestCase) Verify() error { } func (t *TestCase) Clean() error { - ctx, ctxCancel := context.WithTimeout(context.Background(), 10*time.Minute) + ctx, ctxCancel := context.WithTimeout(context.Background(), 60*time.Minute) defer ctxCancel() veleroCfg := t.GetTestCase().VeleroCfg if !veleroCfg.Debug { diff --git a/test/e2e/types.go b/test/e2e/types.go index 5c03be932..99f4c23cb 100644 --- a/test/e2e/types.go +++ b/test/e2e/types.go @@ -24,6 +24,8 @@ import ( . "github.com/vmware-tanzu/velero/test/e2e/util/k8s" ) +const StorageClassName = "e2e-storage-class" + var UUIDgen uuid.UUID var VeleroCfg VeleroConfig diff --git a/test/e2e/util/kibishii/kibishii_utils.go b/test/e2e/util/kibishii/kibishii_utils.go index 2f103508a..f14e702ec 100644 --- a/test/e2e/util/kibishii/kibishii_utils.go +++ b/test/e2e/util/kibishii/kibishii_utils.go @@ -54,6 +54,7 @@ var KibishiiPodNameList = []string{"kibishii-deployment-0", "kibishii-deployment // RunKibishiiTests runs kibishii tests on the provider. func RunKibishiiTests(veleroCfg VeleroConfig, backupName, restoreName, backupLocation, kibishiiNamespace string, useVolumeSnapshots, defaultVolumesToFsBackup bool) error { + pvCount := len(KibishiiPodNameList) client := *veleroCfg.ClientToInstallVelero oneHourTimeout, ctxCancel := context.WithTimeout(context.Background(), time.Minute*60) defer ctxCancel() @@ -101,9 +102,12 @@ func RunKibishiiTests(veleroCfg VeleroConfig, backupName, restoreName, backupLoc var snapshotCheckPoint SnapshotCheckPoint var err error pvbs, err := GetPVB(oneHourTimeout, veleroCfg.VeleroNamespace, kibishiiNamespace) + if err != nil { + return errors.Wrapf(err, "failed to get PVB for namespace %s", kibishiiNamespace) + } if useVolumeSnapshots { - if err != nil || len(pvbs) != 0 { - return errors.Wrapf(err, "failed to get PVB for namespace %s", kibishiiNamespace) + if len(pvbs) != 0 { + return errors.New(fmt.Sprintln("PVB count should be 0 in namespace %s", kibishiiNamespace)) } if providerName == "vsphere" { // Wait for uploads started by the Velero Plug-in for vSphere to complete @@ -124,8 +128,8 @@ func RunKibishiiTests(veleroCfg VeleroConfig, backupName, restoreName, backupLoc return errors.Wrap(err, "exceed waiting for snapshot created in cloud") } } else { - if err != nil || len(pvbs) != 2 { - return errors.Wrapf(err, "failed to get PVB for namespace %s", kibishiiNamespace) + if len(pvbs) != pvCount { + return errors.New(fmt.Sprintf("PVB count %d should be %d in namespace %s", len(pvbs), pvCount, kibishiiNamespace)) } if providerName == "vsphere" { // Wait for uploads started by the Velero Plug-in for vSphere to complete @@ -174,8 +178,10 @@ func RunKibishiiTests(veleroCfg VeleroConfig, backupName, restoreName, backupLoc } if !useVolumeSnapshots { pvrs, err := GetPVR(oneHourTimeout, veleroCfg.VeleroNamespace, kibishiiNamespace) - if err != nil || len(pvrs) != 2 { - return errors.Wrapf(err, "failed to get PVB for namespace %s", kibishiiNamespace) + if err != nil { + return errors.Wrapf(err, "failed to get PVR for namespace %s", kibishiiNamespace) + } else if len(pvrs) != pvCount { + return errors.New(fmt.Sprintf("PVR count %d is not as expected %d", len(pvrs), pvCount)) } } diff --git a/test/e2e/util/velero/velero_utils.go b/test/e2e/util/velero/velero_utils.go index 5e5929d72..a07fc3899 100644 --- a/test/e2e/util/velero/velero_utils.go +++ b/test/e2e/util/velero/velero_utils.go @@ -336,6 +336,20 @@ func VeleroBackupNamespace(ctx context.Context, veleroCLI, veleroNamespace strin if backupCfg.ProvideSnapshotsVolumeParam && !backupCfg.UseVolumeSnapshots { args = append(args, "--snapshot-volumes=false") } // if "--snapshot-volumes" is not provide, snapshot should be taken as default behavior. + } else { // DefaultVolumesToFsBackup is false + // Althrough DefaultVolumesToFsBackup is false, but probably DefaultVolumesToFsBackup + // was set to true in installation CLI in snapshot volume test, so set DefaultVolumesToFsBackup + // to false specifically to make sure volume snapshot was taken + if backupCfg.UseVolumeSnapshots { + if backupCfg.UseResticIfFSBackup { + args = append(args, "--default-volumes-to-restic=false") + } else { + args = append(args, "--default-volumes-to-fs-backup=false") + } + } + // Also Althrough DefaultVolumesToFsBackup is false, but probably DefaultVolumesToFsBackup + // was set to true in installation CLI in FS volume backup test, so do nothing here, no DefaultVolumesToFsBackup + // appear in backup CLI } if backupCfg.BackupLocation != "" { args = append(args, "--storage-location", backupCfg.BackupLocation)