From f7267389b503faf1ea4860e74592ac56def2502e Mon Sep 17 00:00:00 2001 From: Shelly Kagan Date: Tue, 30 Jan 2024 17:37:58 +0200 Subject: [PATCH] Generalize tests to check for specific resources in backup Instead of expecting a specific number of objects backed up we should check if the objects we care about were backed up, the check can also catch bugs that we could have missed in case the number didnt change but for some reason our desired object wasnt backed up. Signed-off-by: Shelly Kagan --- tests/framework/backup.go | 36 ++++ tests/framework/framework.go | 14 ++ tests/resource_filtering_test.go | 297 +++++++++++++++---------------- 3 files changed, 196 insertions(+), 151 deletions(-) diff --git a/tests/framework/backup.go b/tests/framework/backup.go index 215b1797..17431090 100644 --- a/tests/framework/backup.go +++ b/tests/framework/backup.go @@ -248,6 +248,42 @@ func WaitForBackupPhase(ctx context.Context, backupName string, backupNamespace return nil } +func DescribeBackup(ctx context.Context, backupName string) (map[string]interface{}, error) { + var result map[string]interface{} + checkCMD := exec.CommandContext(ctx, veleroCLI, "backup", "describe", "--details", "-o", "json", backupName) + + stdoutPipe, err := checkCMD.StdoutPipe() + if err != nil { + return result, err + } + + jsonBuf := make([]byte, 16*1024) + err = checkCMD.Start() + if err != nil { + return result, err + } + + bytesRead, err := io.ReadFull(stdoutPipe, jsonBuf) + if err != nil && err != io.ErrUnexpectedEOF { + return result, err + } + if bytesRead == len(jsonBuf) { + return result, errors.New("json returned bigger than max allowed") + } + + jsonBuf = jsonBuf[0:bytesRead] + err = checkCMD.Wait() + if err != nil { + return result, err + } + json.Unmarshal(jsonBuf, &result) + if err != nil { + return result, err + } + + return result, nil +} + func CreateSnapshotLocation(ctx context.Context, locationName, provider, region string, backupNamespace string) error { args := []string{ "snapshot-location", "create", locationName, diff --git a/tests/framework/framework.go b/tests/framework/framework.go index 181d6545..1f8c89d1 100644 --- a/tests/framework/framework.go +++ b/tests/framework/framework.go @@ -34,6 +34,7 @@ const ( backupNamespaceEnv = "KVP_BACKUP_NS" regionEnv = "KVP_REGION" storageClassEnv = "KVP_STORAGE_CLASS" + minioProxyURLEnv = "KVP_MINIO_PROXY_URL" defaultRegionName = "minio" defaultBackupNamespace = "velero" @@ -53,6 +54,7 @@ type Framework struct { BackupNamespace string StorageClass string Region string + MinioProxyURL string // Namespace provides a namespace for each test generated/unique ns per test Namespace *v1.Namespace namespacesToDelete []*v1.Namespace @@ -117,6 +119,17 @@ func getStorageClassFromEnv() string { return storageClass } +func getMinioProxyURLFromEnv() string { + minioProxyURL := os.Getenv(minioProxyURLEnv) + if minioProxyURL == "" { + fmt.Fprintf(os.Stderr, "defaulting to minio port 9000\n") + return "" + } + + fmt.Fprintf(ginkgo.GinkgoWriter, "MinioProxyURL [%s]\n", minioProxyURL) + return minioProxyURL +} + func getMaxFailsFromEnv() int { maxFailsEnv := os.Getenv("REPORTER_MAX_FAILS") if maxFailsEnv == "" { @@ -148,6 +161,7 @@ func NewFramework() *Framework { BackupNamespace: getBackupNamespaceFromEnv(), Region: getRegionFromEnv(), StorageClass: getStorageClassFromEnv(), + MinioProxyURL: getMinioProxyURLFromEnv(), Clients: ClientsInstance, reporter: reporter, } diff --git a/tests/resource_filtering_test.go b/tests/resource_filtering_test.go index 3f75c647..f49ecec3 100644 --- a/tests/resource_filtering_test.go +++ b/tests/resource_filtering_test.go @@ -3,6 +3,9 @@ package tests import ( "context" "fmt" + "os" + "os/exec" + "strings" "time" kubernetes "k8s.io/client-go/kubernetes" @@ -24,9 +27,14 @@ const ( includedDVName = "included-test-dv" otherDVName = "other-test-dv" includedVMName = "included-test-vm" + + localMinioProxyPort = 9000 + minioProxyPort = 9000 + minioProxyService = "svc/minio" ) var _ = Describe("Resource includes", func() { + var kubectlCmd *exec.Cmd var timeout context.Context var cancelFunc context.CancelFunc var backupName string @@ -39,6 +47,15 @@ var _ = Describe("Resource includes", func() { t := time.Now().UnixNano() backupName = fmt.Sprintf("test-backup-%d", t) restoreName = fmt.Sprintf("test-restore-%d", t) + if f.MinioProxyURL == "" { + By("Setting up minio port forwarding") + portMapping := fmt.Sprintf("%d:%d", localMinioProxyPort, minioProxyPort) + kubectlCmd = f.CreateKubectlCommand("port-forward", "-n", f.BackupNamespace, minioProxyService, portMapping) + By(fmt.Sprintf("port-forward command: [%s]", kubectlCmd.String())) + + err := kubectlCmd.Start() + Expect(err).ToNot(HaveOccurred()) + } }) AfterEach(func() { @@ -48,6 +65,11 @@ var _ = Describe("Resource includes", func() { fmt.Fprintf(GinkgoWriter, "Err: %s\n", err) } + if kubectlCmd != nil { + Expect(kubectlCmd.Process.Signal(os.Interrupt)).To(Succeed()) + Expect(kubectlCmd.Wait()).To(Succeed()) + } + cancelFunc() }) @@ -183,20 +205,15 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - // The backup should contains the following 5 items: - // - DataVolume - // - PVC - // - VolumeSnapshot - // - VolumeSnapshotContent - // - VolumeSnapshotClass - expectedItems := 5 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems -= 1 + expectedItems := map[string][]string{ + "DataVolume": []string{dvName}, + "PersistentVolumeClaim": []string{dvName}, } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") + } + checkBackupResources(timeout, backupName, expectedItems) By("Deleting DVs") err = framework.DeleteDataVolume(f.KvClient, f.Namespace.Name, dvIncluded.Name) @@ -238,21 +255,16 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - // The backup should contains the following items: - // - DataVolume - // - PVC - // - PV - // - VolumeSnapshot - // - VolumeSnapshotContent - // - a number of unbound PVs - expectedItems := 5 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems = 4 + expectedItems := map[string][]string{ + "DataVolume": []string{dvName}, + "PersistentVolumeClaim": []string{dvName}, + "PersistentVolume": []string{"pvc"}, } - Expect(backup.Status.Progress.ItemsBackedUp).To(BeNumerically(">=", expectedItems)) + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") + } + checkBackupResources(timeout, backupName, expectedItems) By("Deleting DVs") err = framework.DeleteDataVolume(f.KvClient, f.Namespace.Name, dvIncluded.Name) @@ -303,16 +315,14 @@ var _ = Describe("Resource includes", func() { backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) Expect(err).ToNot(HaveOccurred()) Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - // The backup should contains the following item: - // - DataVolume - expectedItems := 1 isDVGC := framework.IsDataVolumeGC(f.KvClient) - if isDVGC { - expectedItems = 0 - } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) if !isDVGC { + expectedItems := map[string][]string{ + "DataVolume": []string{dvName}, + } + checkBackupResources(timeout, backupName, expectedItems) + By("Deleting DVs") err = framework.DeleteDataVolume(f.KvClient, f.Namespace.Name, dvIncluded.Name) Expect(err).ToNot(HaveOccurred()) @@ -351,16 +361,11 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - // The backup should contains the following items: - // - PVC - // - PV - // - VolumeSnapshot - // - VolumeSnapshotContent - // - a number of unbound PVs - Expect(backup.Status.Progress.ItemsBackedUp).To(BeNumerically(">=", 4)) + expectedItems := map[string][]string{ + "PersistentVolumeClaim": []string{dvName}, + "PersistentVolume": []string{"pvc"}, + } + checkBackupResources(timeout, backupName, expectedItems) By("Deleting DVs") err = framework.DeleteDataVolume(f.KvClient, f.Namespace.Name, dvIncluded.Name) @@ -1295,27 +1300,17 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - - // The backup should contains the following 8 items: - // - DataVolume CRD - // - DataVolume - // - PVC - // - PV - // - VolumeSnapshot - // - VolumeSnapshotContent - // - VolumeSpapshotClass - // - Namespace - expectedItems := 8 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems -= 1 + expectedItems := map[string][]string{ + "CustomResourceDefinition": []string{"datavolumes"}, + "DataVolume": []string{includedDVName}, + "PersistentVolume": []string{"pvc"}, + "PersistentVolumeClaim": []string{includedDVName}, } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) - - err = framework.DeleteDataVolume(f.KvClient, f.Namespace.Name, dvSpec.Name) - Expect(err).ToNot(HaveOccurred()) + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") + } + checkBackupResources(timeout, backupName, expectedItems) }) }) @@ -1368,25 +1363,18 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - - // The backup should contain the following 12 items: - // - VirtualMachine - // - 2 DataVolume - // - 2 PVC - // - 2 PV - // - 2 VolumeSnapshot - // - 2 VolumeSnapshotContent - // - VolumeSpapshotClass - // - DataVolume CRD - // - Namespace - expectedItems := 14 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems -= 2 + expectedItems := map[string][]string{ + "CustomResourceDefinition": []string{"virtualmachines"}, + "VirtualMachine": []string{vm.Name}, + "DataVolume": []string{dvName, vm.Spec.DataVolumeTemplates[0].Name}, + "PersistentVolume": []string{"pvc"}, + "PersistentVolumeClaim": []string{dvName, vm.Spec.DataVolumeTemplates[0].Name}, + } + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) + checkBackupResources(timeout, backupName, expectedItems) }) }) Context("VM with DVTemplates", func() { @@ -1401,10 +1389,10 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) framework.EventuallyDVWith(f.KvClient, f.Namespace.Name, vmSpec.Spec.DataVolumeTemplates[0].Name, 180, HaveSucceeded()) - vmSpec = newVMSpecBlankDVTemplate("other-test-vm", "100Mi") - _, err = framework.CreateVirtualMachineFromDefinition(f.KvClient, f.Namespace.Name, vmSpec) + vmSpec2 := newVMSpecBlankDVTemplate("other-test-vm", "100Mi") + _, err = framework.CreateVirtualMachineFromDefinition(f.KvClient, f.Namespace.Name, vmSpec2) Expect(err).ToNot(HaveOccurred()) - framework.EventuallyDVWith(f.KvClient, f.Namespace.Name, vmSpec.Spec.DataVolumeTemplates[0].Name, 180, HaveSucceeded()) + framework.EventuallyDVWith(f.KvClient, f.Namespace.Name, vmSpec2.Spec.DataVolumeTemplates[0].Name, 180, HaveSucceeded()) By("Creating backup") err = framework.CreateBackupForSelector(timeout, backupName, "a.test.label=included", f.Namespace.Name, snapshotLocation, f.BackupNamespace, true) @@ -1413,25 +1401,18 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - - // The backup should contains the following 7 items: - // - VirtualMachine - // - DataVolume - // - PVC - // - PV - // - VolumeSnapshot - // - VolumeSnapshotContent - // - VolumeSpapshotClass - // - DataVolume CRD - // - Namespace - expectedItems := 9 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems -= 1 + expectedItems := map[string][]string{ + "CustomResourceDefinition": []string{"virtualmachines"}, + "VirtualMachine": []string{includedVMName}, + "DataVolume": []string{vmSpec.Spec.DataVolumeTemplates[0].Name}, + "PersistentVolume": []string{"pvc"}, + "PersistentVolumeClaim": []string{vmSpec.Spec.DataVolumeTemplates[0].Name}, } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") + } + checkBackupResources(timeout, backupName, expectedItems) }) It("[test_id:10211]Backup of a running VMs selected by label should include its DVs and PVCs, VMIs and Pods", func() { @@ -1461,27 +1442,20 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - - // The backup should contains the following 9 items: - // - VirtualMachine - // - VirtualMachineInstance - // - Launcher pod - // - DataVolume - // - PVC - // - PV - // - VolumeSnapshot - // - VolumeSnapshotContent - // - VolumeSpapshotClass - // - DataVolume CRD - // - Namespace - expectedItems := 11 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems -= 1 + expectedItems := map[string][]string{ + "CustomResourceDefinition": []string{"virtualmachines"}, + "VirtualMachine": []string{includedVMName}, + "VirtualMachineInstance": []string{includedVMName}, + "Pod": []string{"virt-launcher"}, + "DataVolume": []string{vmSpec.Spec.DataVolumeTemplates[0].Name}, + "PersistentVolume": []string{"pvc"}, + "PersistentVolumeClaim": []string{vmSpec.Spec.DataVolumeTemplates[0].Name}, } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") + } + checkBackupResources(timeout, backupName, expectedItems) }) }) @@ -1498,25 +1472,25 @@ var _ = Describe("Resource includes", func() { By(fmt.Sprintf("Creating DataVolume %s", dvSpec2.Name)) _, err = framework.CreateDataVolumeFromDefinition(f.KvClient, f.Namespace.Name, dvSpec2) Expect(err).ToNot(HaveOccurred()) - framework.EventuallyDVWith(f.KvClient, f.Namespace.Name, "test-dv-2", 180, HaveSucceeded()) + framework.EventuallyDVWith(f.KvClient, f.Namespace.Name, dvSpec2.Name, 180, HaveSucceeded()) By("Creating VirtualMachineInstance") vmiSpec := newBigVMISpecWithDV("test-vmi", dvName) pvcVolume := kvv1.VolumeSource{ PersistentVolumeClaim: &kvv1.PersistentVolumeClaimVolumeSource{ PersistentVolumeClaimVolumeSource: v1.PersistentVolumeClaimVolumeSource{ - ClaimName: "test-dv-2", + ClaimName: dvSpec2.Name, }}, } addVolumeToVMI(vmiSpec, pvcVolume, "pvc-volume") vmiSpec.Labels = map[string]string{ "a.test.label": "included", } - vm, err := framework.CreateVirtualMachineInstanceFromDefinition(f.KvClient, f.Namespace.Name, vmiSpec) + vmi, err := framework.CreateVirtualMachineInstanceFromDefinition(f.KvClient, f.Namespace.Name, vmiSpec) Expect(err).ToNot(HaveOccurred()) - err = framework.WaitForVirtualMachineInstancePhase(f.KvClient, f.Namespace.Name, vmiSpec.Name, kvv1.Running) + err = framework.WaitForVirtualMachineInstancePhase(f.KvClient, f.Namespace.Name, vmi.Name, kvv1.Running) Expect(err).ToNot(HaveOccurred()) - ok, err := framework.WaitForVirtualMachineInstanceCondition(f.KvClient, f.Namespace.Name, vm.Name, kvv1.VirtualMachineInstanceAgentConnected) + ok, err := framework.WaitForVirtualMachineInstanceCondition(f.KvClient, f.Namespace.Name, vmi.Name, kvv1.VirtualMachineInstanceAgentConnected) Expect(err).ToNot(HaveOccurred()) Expect(ok).To(BeTrue(), "VirtualMachineInstanceAgentConnected should be true") @@ -1527,30 +1501,19 @@ var _ = Describe("Resource includes", func() { Expect(err).ToNot(HaveOccurred()) By("Veryfing backup") - backup, err := framework.GetBackup(timeout, backupName, f.BackupNamespace) - Expect(err).ToNot(HaveOccurred()) - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(backup.Status.Progress.TotalItems)) - - // The backup should contains the following 13 items: - // - VirtualMachineInstance - // - Launcher pod - // - DataVolume - // - DV's PVC - // - DV's PVC's PV - // - standalone PVC - // - standaolne PVC's PV - // - VolumeSnapshot (DV) - // - VolumeSnapshotContent (DV) - // - VolumeSnapshot (PVC) - // - VolumeSnapshotContent (PVC) - // - VolumeSpapshotClass - // - DataVolume CRD - // - Namespace - expectedItems := 14 - if framework.IsDataVolumeGC(f.KvClient) { - expectedItems -= 1 + expectedItems := map[string][]string{ + "CustomResourceDefinition": []string{"virtualmachineinstances"}, + "VirtualMachineInstance": []string{vmi.Name}, + "Pod": []string{"virt-launcher"}, + "DataVolume": []string{dvName}, + "PersistentVolume": []string{"pvc"}, + "PersistentVolumeClaim": []string{dvName, dvSpec2.Name}, + } + isDVGC := framework.IsDataVolumeGC(f.KvClient) + if isDVGC { + delete(expectedItems, "DataVolume") } - Expect(backup.Status.Progress.ItemsBackedUp).To(Equal(expectedItems)) + checkBackupResources(timeout, backupName, expectedItems) }) }) }) @@ -3368,6 +3331,38 @@ var _ = Describe("Resource excludes", func() { }) }) +func checkBackupResources(timeout context.Context, backupName string, expectedRes map[string][]string) { + result, err := framework.DescribeBackup(timeout, backupName) + Expect(err).ToNot(HaveOccurred()) + fmt.Println(result) + resStatus := result["status"].(map[string]interface{}) + Expect(resStatus["itemsBackedUp"]).To(Equal(resStatus["totalItemsToBeBackedUp"])) + if resStatus["resourceList"] == nil { + Fail("No resources were backed up") + } + resourceList := resStatus["resourceList"].(map[string]interface{}) + Expect(len(expectedRes)).Should(BeNumerically("<=", len(resourceList))) + + for expType, expVals := range expectedRes { + for _, expVal := range expVals { + found := false + for resType, resVal := range resourceList { + if strings.Contains(resType, expType) { + for _, val := range resVal.([]interface{}) { + if strings.Contains(val.(string), expVal) { + found = true + break + } + } + } + } + if !found { + Fail(fmt.Sprintf("resource: %s-%s not found in backup resources: %+v", expType, expVal, resourceList)) + } + } + } +} + func FindLauncherPod(client *kubernetes.Clientset, namespace string, vmName string) v1.Pod { var pod v1.Pod pods, err := client.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{