From 545aaada8e5315614f671c09dc71dca7a58740af Mon Sep 17 00:00:00 2001 From: Raghuram Devarakonda Date: Thu, 15 Jun 2023 13:14:58 -0400 Subject: [PATCH 01/14] Clarify that snapshot data is not uploaded to the object storage. Signed-off-by: Raghuram Devarakonda --- site/content/docs/main/csi.md | 4 +++- site/content/docs/v1.11/csi.md | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/site/content/docs/main/csi.md b/site/content/docs/main/csi.md index 80198c4c97..58899efc1c 100644 --- a/site/content/docs/main/csi.md +++ b/site/content/docs/main/csi.md @@ -56,7 +56,9 @@ From there, the CSI external-snapshotter controller will see the VolumeSnapshot The external-snapshotter plugin will call the CSI driver's snapshot method, and the driver will call the storage system's APIs to generate the snapshot. Once an ID is generated and the storage system marks the snapshot as usable for restore, the VolumeSnapshotContent object will be updated with a `status.snapshotHandle` and the `status.readyToUse` field will be set. -Velero will include the generated VolumeSnapshot and VolumeSnapshotContent objects in the backup tarball, as well as upload all VolumeSnapshots and VolumeSnapshotContents objects in a JSON file to the object storage system. +Velero will include the generated VolumeSnapshot and VolumeSnapshotContent objects in the backup tarball, as well as +upload all VolumeSnapshots and VolumeSnapshotContents objects in a JSON file to the object storage system. **Note that +only Kubernetes objects are uploaded to the object storage, not the data in snapshots.** When Velero synchronizes backups into a new cluster, VolumeSnapshotContent objects and the VolumeSnapshotClass that is chosen to take snapshot will be synced into the cluster as well, so that Velero can manage backup expiration appropriately. diff --git a/site/content/docs/v1.11/csi.md b/site/content/docs/v1.11/csi.md index 80198c4c97..58899efc1c 100644 --- a/site/content/docs/v1.11/csi.md +++ b/site/content/docs/v1.11/csi.md @@ -56,7 +56,9 @@ From there, the CSI external-snapshotter controller will see the VolumeSnapshot The external-snapshotter plugin will call the CSI driver's snapshot method, and the driver will call the storage system's APIs to generate the snapshot. Once an ID is generated and the storage system marks the snapshot as usable for restore, the VolumeSnapshotContent object will be updated with a `status.snapshotHandle` and the `status.readyToUse` field will be set. -Velero will include the generated VolumeSnapshot and VolumeSnapshotContent objects in the backup tarball, as well as upload all VolumeSnapshots and VolumeSnapshotContents objects in a JSON file to the object storage system. +Velero will include the generated VolumeSnapshot and VolumeSnapshotContent objects in the backup tarball, as well as +upload all VolumeSnapshots and VolumeSnapshotContents objects in a JSON file to the object storage system. **Note that +only Kubernetes objects are uploaded to the object storage, not the data in snapshots.** When Velero synchronizes backups into a new cluster, VolumeSnapshotContent objects and the VolumeSnapshotClass that is chosen to take snapshot will be synced into the cluster as well, so that Velero can manage backup expiration appropriately. From cc468873db2b567cda5c57daa1b105223800ab23 Mon Sep 17 00:00:00 2001 From: Raghuram Devarakonda Date: Fri, 7 Jul 2023 10:34:06 -0400 Subject: [PATCH 02/14] Clarify FSB restore pod status. Signed-off-by: Raghuram Devarakonda --- site/content/docs/main/file-system-backup.md | 6 +++--- site/content/docs/v1.11/file-system-backup.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/site/content/docs/main/file-system-backup.md b/site/content/docs/main/file-system-backup.md index 89c43ce7b7..7a9d9199f1 100644 --- a/site/content/docs/main/file-system-backup.md +++ b/site/content/docs/main/file-system-backup.md @@ -539,7 +539,7 @@ that it's backing up for the volumes to be backed up using FSB. 5. Meanwhile, each `PodVolumeBackup` is handled by the controller on the appropriate node, which: - has a hostPath volume mount of `/var/lib/kubelet/pods` to access the pod volume data - finds the pod volume's subdirectory within the above volume - - based on the path selection, Velero inokes restic or kopia for backup + - based on the path selection, Velero invokes restic or kopia for backup - updates the status of the custom resource to `Completed` or `Failed` 6. As each `PodVolumeBackup` finishes, the main Velero process adds it to the Velero backup in a file named `-podvolumebackups.json.gz`. This file gets uploaded to object storage alongside the backup tarball. @@ -556,7 +556,7 @@ It will be used for restores, as seen in the next section. 3. Velero adds an init container to the pod, whose job is to wait for all FSB restores for the pod to complete (more on this shortly) 4. Velero creates the pod, with the added init container, by submitting it to the Kubernetes API. Then, the Kubernetes -scheduler schedules this pod to a worker node, and the pod must be in a running state. If the pod fails to start for +scheduler schedules this pod to a worker node. If the pod fails to be scheduled for some reason (i.e. lack of cluster resources), the FSB restore will not be done. 5. Velero creates a `PodVolumeRestore` custom resource for each volume to be restored in the pod 6. The main Velero process now waits for each `PodVolumeRestore` resource to complete or fail @@ -564,7 +564,7 @@ some reason (i.e. lack of cluster resources), the FSB restore will not be done. - has a hostPath volume mount of `/var/lib/kubelet/pods` to access the pod volume data - waits for the pod to be running the init container - finds the pod volume's subdirectory within the above volume - - based on the path selection, Velero inokes restic or kopia for restore + - based on the path selection, Velero invokes restic or kopia for restore - on success, writes a file into the pod volume, in a `.velero` subdirectory, whose name is the UID of the Velero restore that this pod volume restore is for - updates the status of the custom resource to `Completed` or `Failed` diff --git a/site/content/docs/v1.11/file-system-backup.md b/site/content/docs/v1.11/file-system-backup.md index 881747895c..0fb442e118 100644 --- a/site/content/docs/v1.11/file-system-backup.md +++ b/site/content/docs/v1.11/file-system-backup.md @@ -539,7 +539,7 @@ that it's backing up for the volumes to be backed up using FSB. 5. Meanwhile, each `PodVolumeBackup` is handled by the controller on the appropriate node, which: - has a hostPath volume mount of `/var/lib/kubelet/pods` to access the pod volume data - finds the pod volume's subdirectory within the above volume - - based on the path selection, Velero inokes restic or kopia for backup + - based on the path selection, Velero invokes restic or kopia for backup - updates the status of the custom resource to `Completed` or `Failed` 6. As each `PodVolumeBackup` finishes, the main Velero process adds it to the Velero backup in a file named `-podvolumebackups.json.gz`. This file gets uploaded to object storage alongside the backup tarball. @@ -556,7 +556,7 @@ It will be used for restores, as seen in the next section. 3. Velero adds an init container to the pod, whose job is to wait for all FSB restores for the pod to complete (more on this shortly) 4. Velero creates the pod, with the added init container, by submitting it to the Kubernetes API. Then, the Kubernetes -scheduler schedules this pod to a worker node, and the pod must be in a running state. If the pod fails to start for +scheduler schedules this pod to a worker node. If the pod fails to be scheduled for some reason (i.e. lack of cluster resources), the FSB restore will not be done. 5. Velero creates a `PodVolumeRestore` custom resource for each volume to be restored in the pod 6. The main Velero process now waits for each `PodVolumeRestore` resource to complete or fail @@ -564,7 +564,7 @@ some reason (i.e. lack of cluster resources), the FSB restore will not be done. - has a hostPath volume mount of `/var/lib/kubelet/pods` to access the pod volume data - waits for the pod to be running the init container - finds the pod volume's subdirectory within the above volume - - based on the path selection, Velero inokes restic or kopia for restore + - based on the path selection, Velero invokes restic or kopia for restore - on success, writes a file into the pod volume, in a `.velero` subdirectory, whose name is the UID of the Velero restore that this pod volume restore is for - updates the status of the custom resource to `Completed` or `Failed` From bc8742566bfcdfc24758114dee1c51bc6e9033cc Mon Sep 17 00:00:00 2001 From: danfengl Date: Tue, 20 Jun 2023 10:24:11 +0000 Subject: [PATCH 03/14] Install plugin for datamover pipeline Signed-off-by: danfengl --- go.mod | 2 +- test/e2e/Makefile | 9 +- test/e2e/backup/backup.go | 9 +- test/e2e/backups/sync_backups.go | 4 +- test/e2e/backups/ttl.go | 2 +- .../api-group/enable_api_group_extentions.go | 4 +- .../api-group/enable_api_group_versions.go | 2 +- test/e2e/basic/namespace-mapping.go | 2 +- test/e2e/bsl-mgmt/deletion.go | 4 +- test/e2e/e2e_suite_test.go | 4 +- test/e2e/migration/migration.go | 116 ++++++++++++------ test/e2e/types.go | 4 + test/e2e/upgrade/upgrade.go | 4 +- test/e2e/util/k8s/common.go | 1 + test/e2e/util/k8s/statefulset.go | 39 ++++++ test/e2e/util/kibishii/kibishii_utils.go | 19 ++- test/e2e/util/velero/install.go | 38 +++--- test/e2e/util/velero/velero_utils.go | 111 +++++++++++++---- 18 files changed, 275 insertions(+), 99 deletions(-) create mode 100644 test/e2e/util/k8s/statefulset.go diff --git a/go.mod b/go.mod index 61885fe44c..ee43d5fa3d 100644 --- a/go.mod +++ b/go.mod @@ -35,6 +35,7 @@ require ( github.com/stretchr/testify v1.8.2 github.com/vmware-tanzu/crash-diagnostics v0.3.7 go.uber.org/zap v1.24.0 + golang.org/x/exp v0.0.0-20221028150844-83b7d23a625f golang.org/x/mod v0.10.0 golang.org/x/net v0.9.0 golang.org/x/oauth2 v0.7.0 @@ -139,7 +140,6 @@ require ( go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.8.0 // indirect - golang.org/x/exp v0.0.0-20221028150844-83b7d23a625f // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.7.0 // indirect golang.org/x/term v0.7.0 // indirect diff --git a/test/e2e/Makefile b/test/e2e/Makefile index f2d7594e1d..0fbf46e3ee 100644 --- a/test/e2e/Makefile +++ b/test/e2e/Makefile @@ -73,6 +73,7 @@ BSL_PREFIX ?= BSL_CONFIG ?= VSL_CONFIG ?= CLOUD_PROVIDER ?= +STANDBY_CLUSTER_CLOUD_PROVIDER ?= OBJECT_STORE_PROVIDER ?= INSTALL_VELERO ?= true REGISTRY_CREDENTIAL_FILE ?= @@ -99,6 +100,9 @@ STANDBY_CLUSTER ?= UPLOADER_TYPE ?= +SNAPSHOT_MOVE_DATA ?= false +DATA_MOVER_PLUGIN ?= + .PHONY:ginkgo ginkgo: # Make sure ginkgo is in $GOPATH/bin @@ -143,7 +147,10 @@ run: ginkgo -velero-server-debug-mode=$(VELERO_SERVER_DEBUG_MODE) \ -default-cluster=$(DEFAULT_CLUSTER) \ -standby-cluster=$(STANDBY_CLUSTER) \ - -uploader-type=$(UPLOADER_TYPE) + -uploader-type=$(UPLOADER_TYPE) \ + -snapshot-move-data=$(SNAPSHOT_MOVE_DATA) \ + -data-mover-plugin=$(DATA_MOVER_plugin) \ + -standby-cluster-cloud-provider=$(STANDBY_CLUSTER_CLOUD_PROVIDER) build: ginkgo mkdir -p $(OUTPUT_DIR) diff --git a/test/e2e/backup/backup.go b/test/e2e/backup/backup.go index 9bf694c001..8a1f2d40e7 100644 --- a/test/e2e/backup/backup.go +++ b/test/e2e/backup/backup.go @@ -87,7 +87,7 @@ func BackupRestoreTest(useVolumeSnapshots bool) { } else { veleroCfg.DefaultVolumesToFsBackup = !useVolumeSnapshots } - Expect(VeleroInstall(context.Background(), &veleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &veleroCfg, false)).To(Succeed()) } backupName = "backup-" + UUIDgen.String() restoreName = "restore-" + UUIDgen.String() @@ -125,12 +125,9 @@ func BackupRestoreTest(useVolumeSnapshots bool) { veleroCfg.DefaultVolumesToFsBackup = useVolumeSnapshots } - Expect(VeleroInstall(context.Background(), &veleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &veleroCfg, false)).To(Succeed()) } - - Expect(VeleroAddPluginsForProvider(context.TODO(), veleroCfg.VeleroCLI, - veleroCfg.VeleroNamespace, veleroCfg.AdditionalBSLProvider, - veleroCfg.AddBSLPlugins, veleroCfg.Features)).To(Succeed()) + Expect(VeleroAddPluginsForProvider(context.TODO(), veleroCfg.VeleroCLI, veleroCfg.VeleroNamespace, veleroCfg.AdditionalBSLProvider)).To(Succeed()) // Create Secret for additional BSL secretName := fmt.Sprintf("bsl-credentials-%s", UUIDgen) diff --git a/test/e2e/backups/sync_backups.go b/test/e2e/backups/sync_backups.go index 1bfea90b26..a12a270c82 100644 --- a/test/e2e/backups/sync_backups.go +++ b/test/e2e/backups/sync_backups.go @@ -59,7 +59,7 @@ func BackupsSyncTest() { if VeleroCfg.InstallVelero { veleroCfg := VeleroCfg veleroCfg.UseVolumeSnapshots = false - Expect(VeleroInstall(context.Background(), &VeleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &VeleroCfg, false)).To(Succeed()) } }) @@ -109,7 +109,7 @@ func BackupsSyncTest() { By("Install velero", func() { veleroCfg := VeleroCfg veleroCfg.UseVolumeSnapshots = false - Expect(VeleroInstall(ctx, &VeleroCfg)).To(Succeed()) + Expect(VeleroInstall(ctx, &VeleroCfg, false)).To(Succeed()) }) By("Check all backups in object storage are synced to Velero", func() { diff --git a/test/e2e/backups/ttl.go b/test/e2e/backups/ttl.go index 09afb23e8e..a60d95ea55 100644 --- a/test/e2e/backups/ttl.go +++ b/test/e2e/backups/ttl.go @@ -70,7 +70,7 @@ func TTLTest() { // Make sure GCFrequency is shorter than backup TTL veleroCfg.GCFrequency = "4m0s" veleroCfg.UseVolumeSnapshots = useVolumeSnapshots - Expect(VeleroInstall(context.Background(), &veleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &veleroCfg, false)).To(Succeed()) } }) diff --git a/test/e2e/basic/api-group/enable_api_group_extentions.go b/test/e2e/basic/api-group/enable_api_group_extentions.go index a3d6a3b88d..002833d55f 100644 --- a/test/e2e/basic/api-group/enable_api_group_extentions.go +++ b/test/e2e/basic/api-group/enable_api_group_extentions.go @@ -100,7 +100,7 @@ func APIExtensionsVersionsTest() { Expect(KubectlConfigUseContext(context.Background(), veleroCfg.DefaultCluster)).To(Succeed()) veleroCfg.Features = "EnableAPIGroupVersions" veleroCfg.UseVolumeSnapshots = false - Expect(VeleroInstall(context.Background(), &veleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &veleroCfg, false)).To(Succeed()) }) By(fmt.Sprintf("Install CRD of apiextenstions v1beta1 in cluster-A (%s)", veleroCfg.DefaultCluster), func() { @@ -129,7 +129,7 @@ func APIExtensionsVersionsTest() { By(fmt.Sprintf("Install Velero in cluster-B (%s) to restore workload", veleroCfg.StandbyCluster), func() { Expect(KubectlConfigUseContext(context.Background(), veleroCfg.StandbyCluster)).To(Succeed()) veleroCfg.ClientToInstallVelero = veleroCfg.StandbyClient - Expect(VeleroInstall(context.Background(), &veleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &veleroCfg, false)).To(Succeed()) }) By(fmt.Sprintf("Waiting for backups sync to Velero in cluster-B (%s)", veleroCfg.StandbyCluster), func() { diff --git a/test/e2e/basic/api-group/enable_api_group_versions.go b/test/e2e/basic/api-group/enable_api_group_versions.go index ba1d73f6e5..61d7ba08e3 100644 --- a/test/e2e/basic/api-group/enable_api_group_versions.go +++ b/test/e2e/basic/api-group/enable_api_group_versions.go @@ -75,7 +75,7 @@ func APIGropuVersionsTest() { if veleroCfg.InstallVelero { veleroCfg.Features = "EnableAPIGroupVersions" veleroCfg.UseVolumeSnapshots = false - err = VeleroInstall(context.Background(), &veleroCfg) + err = VeleroInstall(context.Background(), &veleroCfg, false) Expect(err).NotTo(HaveOccurred()) } testCaseNum = 4 diff --git a/test/e2e/basic/namespace-mapping.go b/test/e2e/basic/namespace-mapping.go index 8ea16d365f..ae9c0509e1 100644 --- a/test/e2e/basic/namespace-mapping.go +++ b/test/e2e/basic/namespace-mapping.go @@ -115,7 +115,7 @@ func (n *NamespaceMapping) Verify() error { func (n *NamespaceMapping) Clean() error { if !n.VeleroCfg.Debug { - if err := DeleteStorageClass(context.Background(), n.Client, "kibishii-storage-class"); err != nil { + if err := DeleteStorageClass(context.Background(), n.Client, KibishiiStorageClassName); err != nil { return err } for _, ns := range n.MappedNamespaceList { diff --git a/test/e2e/bsl-mgmt/deletion.go b/test/e2e/bsl-mgmt/deletion.go index e153073692..874cf53c78 100644 --- a/test/e2e/bsl-mgmt/deletion.go +++ b/test/e2e/bsl-mgmt/deletion.go @@ -104,9 +104,7 @@ func BslDeletionTest(useVolumeSnapshots bool) { } By(fmt.Sprintf("Add an additional plugin for provider %s", veleroCfg.AdditionalBSLProvider), func() { - Expect(VeleroAddPluginsForProvider(context.TODO(), veleroCfg.VeleroCLI, - veleroCfg.VeleroNamespace, veleroCfg.AdditionalBSLProvider, - veleroCfg.AddBSLPlugins, veleroCfg.Features)).To(Succeed()) + Expect(VeleroAddPluginsForProvider(context.TODO(), veleroCfg.VeleroCLI, veleroCfg.VeleroNamespace, veleroCfg.AdditionalBSLProvider)).To(Succeed()) }) additionalBsl := fmt.Sprintf("bsl-%s", UUIDgen) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 651affb0ec..df9d18ea4f 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -84,7 +84,9 @@ func init() { flag.StringVar(&VeleroCfg.StandbyCluster, "standby-cluster", "", "Standby cluster context for migration test.") flag.StringVar(&VeleroCfg.UploaderType, "uploader-type", "", "Identify persistent volume backup uploader.") flag.BoolVar(&VeleroCfg.VeleroServerDebugMode, "velero-server-debug-mode", false, "Identify persistent volume backup uploader.") - + flag.BoolVar(&VeleroCfg.SnapshotMoveData, "snapshot-move-data", false, "Install default plugin for data mover.") + flag.StringVar(&VeleroCfg.DataMoverPlugin, "data-mover-plugin", "", "Install customized plugin for data mover.") + flag.StringVar(&VeleroCfg.StandbyClusterCloudProvider, "standby-cluster-cloud-provider", "", "Install customized plugin for data mover.") } var _ = Describe("[APIGroup][APIVersion] Velero tests with various CRD API group versions", APIGropuVersionsTest) diff --git a/test/e2e/migration/migration.go b/test/e2e/migration/migration.go index 0a133f23eb..6b10ba9259 100644 --- a/test/e2e/migration/migration.go +++ b/test/e2e/migration/migration.go @@ -59,13 +59,14 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) BeforeEach(func() { veleroCfg = VeleroCfg UUIDgen, err = uuid.NewRandom() - migrationNamespace = "migration-workload-" + UUIDgen.String() + migrationNamespace = "migration-" + UUIDgen.String() if useVolumeSnapshots && veleroCfg.CloudProvider == "kind" { Skip("Volume snapshots not supported on kind") } - if useVolumeSnapshots && veleroCfg.CloudProvider == "aws" { + if useVolumeSnapshots && veleroCfg.CloudProvider == "aws" && !veleroCfg.SnapshotMoveData { Skip("Volume snapshots migration not supported on AWS provisioned by Sheperd public pool") } + if veleroCfg.DefaultCluster == "" && veleroCfg.StandbyCluster == "" { Skip("Migration test needs 2 clusters") } @@ -79,9 +80,10 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) }) AfterEach(func() { if !veleroCfg.Debug { - By("Clean backups after test", func() { - DeleteBackups(context.Background(), *veleroCfg.DefaultClient) - }) + // TODO: delete backup created by case self, not all + // By("Clean backups after test", func() { + // DeleteBackups(context.Background(), *veleroCfg.DefaultClient) + // }) if veleroCfg.InstallVelero { By(fmt.Sprintf("Uninstall Velero and delete sample workload namespace %s", migrationNamespace), func() { Expect(KubectlConfigUseContext(context.Background(), veleroCfg.DefaultCluster)).To(Succeed()) @@ -104,6 +106,16 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) }) When("kibishii is the sample workload", func() { It("should be successfully backed up and restored to the default BackupStorageLocation", func() { + + if veleroCfg.SnapshotMoveData { + if !useVolumeSnapshots { + Skip("FSB migration test is not needed in data mover scenario") + } + // TODO: remove this block once Velero version in cluster A is great than V1.11 for all migration path. + if veleroCLI2Version.VeleroVersion != "self" { + Skip(fmt.Sprintf("Only V1.12 support data mover scenario instead of %s", veleroCLI2Version.VeleroVersion)) + } + } oneHourTimeout, ctxCancel := context.WithTimeout(context.Background(), time.Minute*60) defer ctxCancel() flag.Parse() @@ -132,25 +144,20 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) OriginVeleroCfg.ClientToInstallVelero = OriginVeleroCfg.DefaultClient OriginVeleroCfg.UseVolumeSnapshots = useVolumeSnapshots OriginVeleroCfg.UseNodeAgent = !useVolumeSnapshots - // TODO: self means 1.10 and upper version - if veleroCLI2Version.VeleroVersion != "self" { + + // self represents v1.12 + if veleroCLI2Version.VeleroVersion == "self" { + if OriginVeleroCfg.SnapshotMoveData { + OriginVeleroCfg.UseNodeAgent = true + } + } else { Expect(err).To(Succeed()) fmt.Printf("Using default images address of Velero CLI %s\n", veleroCLI2Version.VeleroVersion) OriginVeleroCfg.VeleroImage = "" OriginVeleroCfg.RestoreHelperImage = "" OriginVeleroCfg.Plugins = "" - //TODO: Remove this once origin Velero version is 1.10 and upper - OriginVeleroCfg.UploaderType = "" - if supportUploaderType { - OriginVeleroCfg.UseRestic = false - OriginVeleroCfg.UseNodeAgent = !useVolumeSnapshots - } else { - OriginVeleroCfg.UseRestic = !useVolumeSnapshots - OriginVeleroCfg.UseNodeAgent = false - } } - - Expect(VeleroInstall(context.Background(), &OriginVeleroCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &OriginVeleroCfg, false)).To(Succeed()) if veleroCLI2Version.VeleroVersion != "self" { Expect(CheckVeleroVersion(context.Background(), OriginVeleroCfg.VeleroCLI, OriginVeleroCfg.MigrateFromVeleroVersion)).To(Succeed()) @@ -167,10 +174,15 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) fmt.Sprintf("Failed to create namespace %s to install Kibishii workload", migrationNamespace)) }) + KibishiiData := *DefaultKibishiiData By("Deploy sample workload of Kibishii", func() { + if OriginVeleroCfg.SnapshotMoveData { + KibishiiData.ExpectedNodes = 6 + } + Expect(KibishiiPrepareBeforeBackup(oneHourTimeout, *veleroCfg.DefaultClient, veleroCfg.CloudProvider, migrationNamespace, veleroCfg.RegistryCredentialFile, veleroCfg.Features, - veleroCfg.KibishiiDirectory, useVolumeSnapshots, DefaultKibishiiData)).To(Succeed()) + veleroCfg.KibishiiDirectory, useVolumeSnapshots, &KibishiiData)).To(Succeed()) }) By(fmt.Sprintf("Backup namespace %s", migrationNamespace), func() { @@ -178,6 +190,7 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) BackupStorageClassCfg.BackupName = backupScName BackupStorageClassCfg.IncludeResources = "StorageClass" BackupStorageClassCfg.IncludeClusterResources = true + //TODO Remove UseRestic parameter once minor version is 1.10 or upper BackupStorageClassCfg.UseResticIfFSBackup = !supportUploaderType Expect(VeleroBackupNamespace(context.Background(), OriginVeleroCfg.VeleroCLI, @@ -195,6 +208,7 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) BackupCfg.DefaultVolumesToFsBackup = !useVolumeSnapshots //TODO Remove UseRestic parameter once minor version is 1.10 or upper BackupCfg.UseResticIfFSBackup = !supportUploaderType + BackupCfg.SnapshotMoveData = OriginVeleroCfg.SnapshotMoveData Expect(VeleroBackupNamespace(context.Background(), OriginVeleroCfg.VeleroCLI, OriginVeleroCfg.VeleroNamespace, BackupCfg)).To(Succeed(), func() string { @@ -211,21 +225,27 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) migrationNamespace, 2)).To(Succeed()) }) } + var snapshotCheckPoint SnapshotCheckPoint snapshotCheckPoint.NamespaceBackedUp = migrationNamespace - By("Snapshot should be created in cloud object store", func() { - snapshotCheckPoint, err := GetSnapshotCheckPoint(*veleroCfg.DefaultClient, veleroCfg, 2, - migrationNamespace, backupName, KibishiiPVCNameList) - Expect(err).NotTo(HaveOccurred(), "Fail to get snapshot checkpoint") - Expect(SnapshotsShouldBeCreatedInCloud(veleroCfg.CloudProvider, - veleroCfg.CloudCredentialsFile, veleroCfg.BSLBucket, - veleroCfg.BSLConfig, backupName, snapshotCheckPoint)).To(Succeed()) - }) + + if !OriginVeleroCfg.SnapshotMoveData { + By("Snapshot should be created in cloud object store", func() { + snapshotCheckPoint, err := GetSnapshotCheckPoint(*veleroCfg.DefaultClient, veleroCfg, 2, + migrationNamespace, backupName, KibishiiPVCNameList) + Expect(err).NotTo(HaveOccurred(), "Fail to get snapshot checkpoint") + Expect(SnapshotsShouldBeCreatedInCloud(veleroCfg.CloudProvider, + veleroCfg.CloudCredentialsFile, veleroCfg.BSLBucket, + veleroCfg.BSLConfig, backupName, snapshotCheckPoint)).To(Succeed()) + }) + } else { + //TODO: checkpoint for datamover + } } - if useVolumeSnapshots && veleroCfg.CloudProvider == "azure" && strings.EqualFold(veleroCfg.Features, "EnableCSI") { - // Upgrade test is not running daily since no CSI plugin v1.0 released, because builds before - // v1.0 have issues to fail upgrade case. + if useVolumeSnapshots && veleroCfg.CloudProvider == "azure" && + strings.EqualFold(veleroCfg.Features, "EnableCSI") && + !OriginVeleroCfg.SnapshotMoveData { By("Sleep 5 minutes to avoid snapshot recreated by unknown reason ", func() { time.Sleep(5 * time.Minute) }) @@ -233,7 +253,7 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) // the snapshots of AWS may be still in pending status when do the restore, wait for a while // to avoid this https://github.com/vmware-tanzu/velero/issues/1799 // TODO remove this after https://github.com/vmware-tanzu/velero/issues/3533 is fixed - if veleroCfg.CloudProvider == "aws" && useVolumeSnapshots { + if veleroCfg.CloudProvider == "aws" && useVolumeSnapshots && !OriginVeleroCfg.SnapshotMoveData { fmt.Println("Waiting 5 minutes to make sure the snapshots are ready...") time.Sleep(5 * time.Minute) } @@ -253,7 +273,10 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) veleroCfg.ClientToInstallVelero = veleroCfg.StandbyClient veleroCfg.UseNodeAgent = !useVolumeSnapshots veleroCfg.UseRestic = false - Expect(VeleroInstall(context.Background(), &veleroCfg)).To(Succeed()) + if veleroCfg.SnapshotMoveData { + veleroCfg.UseNodeAgent = true + } + Expect(VeleroInstall(context.Background(), &veleroCfg, true)).To(Succeed()) }) By(fmt.Sprintf("Waiting for backups sync to Velero in cluster-B (%s)", veleroCfg.StandbyCluster), func() { @@ -262,12 +285,27 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) }) By(fmt.Sprintf("Restore %s", migrationNamespace), func() { - Expect(VeleroRestore(context.Background(), veleroCfg.VeleroCLI, - veleroCfg.VeleroNamespace, restoreScName, backupScName, "StorageClass")).To(Succeed(), func() string { - RunDebug(context.Background(), veleroCfg.VeleroCLI, - veleroCfg.VeleroNamespace, "", restoreName) - return "Fail to restore workload" - }) + if OriginVeleroCfg.SnapshotMoveData { + By(fmt.Sprintf("Create a storage class %s for restore PV provisioned by storage class %s on different cloud provider", StorageClassName, KibishiiStorageClassName), func() { + Expect(InstallStorageClass(context.Background(), fmt.Sprintf("testdata/storage-class/%s.yaml", veleroCfg.StandbyClusterCloudProvider))).To(Succeed()) + }) + configmaptName := "datamover-storage-class-config" + labels := map[string]string{"velero.io/change-storage-class": "RestoreItemAction", + "velero.io/plugin-config": ""} + data := map[string]string{KibishiiStorageClassName: StorageClassName} + + By(fmt.Sprintf("Create ConfigMap %s in namespace %s", configmaptName, veleroCfg.VeleroNamespace), func() { + _, err := CreateConfigMap(veleroCfg.StandbyClient.ClientGo, veleroCfg.VeleroNamespace, configmaptName, labels, data) + Expect(err).To(Succeed(), fmt.Sprintf("failed to create configmap in the namespace %q", veleroCfg.VeleroNamespace)) + }) + } else { + Expect(VeleroRestore(context.Background(), veleroCfg.VeleroCLI, + veleroCfg.VeleroNamespace, restoreScName, backupScName, "StorageClass")).To(Succeed(), func() string { + RunDebug(context.Background(), veleroCfg.VeleroCLI, + veleroCfg.VeleroNamespace, "", restoreName) + return "Fail to restore workload" + }) + } Expect(VeleroRestore(context.Background(), veleroCfg.VeleroCLI, veleroCfg.VeleroNamespace, restoreName, backupName, "")).To(Succeed(), func() string { RunDebug(context.Background(), veleroCfg.VeleroCLI, @@ -278,7 +316,7 @@ func MigrationTest(useVolumeSnapshots bool, veleroCLI2Version VeleroCLI2Version) By(fmt.Sprintf("Verify workload %s after restore ", migrationNamespace), func() { Expect(KibishiiVerifyAfterRestore(*veleroCfg.StandbyClient, migrationNamespace, - oneHourTimeout, DefaultKibishiiData)).To(Succeed(), "Fail to verify workload after restore") + oneHourTimeout, &KibishiiData)).To(Succeed(), "Fail to verify workload after restore") }) }) }) diff --git a/test/e2e/types.go b/test/e2e/types.go index 99f4c23cb6..4e3dcd1cd5 100644 --- a/test/e2e/types.go +++ b/test/e2e/types.go @@ -72,6 +72,9 @@ type VeleroConfig struct { DefaultVolumesToFsBackup bool UseVolumeSnapshots bool VeleroServerDebugMode bool + SnapshotMoveData bool + DataMoverPlugin string + StandbyClusterCloudProvider string } type SnapshotCheckPoint struct { @@ -98,6 +101,7 @@ type BackupConfig struct { OrderedResources string UseResticIfFSBackup bool DefaultVolumesToFsBackup bool + SnapshotMoveData bool } type VeleroCLI2Version struct { diff --git a/test/e2e/upgrade/upgrade.go b/test/e2e/upgrade/upgrade.go index 1e661d8235..58b8922554 100644 --- a/test/e2e/upgrade/upgrade.go +++ b/test/e2e/upgrade/upgrade.go @@ -136,7 +136,7 @@ func BackupUpgradeRestoreTest(useVolumeSnapshots bool, veleroCLI2Version VeleroC tmpCfgForOldVeleroInstall.UseNodeAgent = false } - Expect(VeleroInstall(context.Background(), &tmpCfgForOldVeleroInstall)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &tmpCfgForOldVeleroInstall, false)).To(Succeed()) Expect(CheckVeleroVersion(context.Background(), tmpCfgForOldVeleroInstall.VeleroCLI, tmpCfgForOldVeleroInstall.UpgradeFromVeleroVersion)).To(Succeed()) }) @@ -223,7 +223,7 @@ func BackupUpgradeRestoreTest(useVolumeSnapshots bool, veleroCLI2Version VeleroC tmpCfg.UseNodeAgent = !useVolumeSnapshots Expect(err).To(Succeed()) if supportUploaderType { - Expect(VeleroInstall(context.Background(), &tmpCfg)).To(Succeed()) + Expect(VeleroInstall(context.Background(), &tmpCfg, false)).To(Succeed()) Expect(CheckVeleroVersion(context.Background(), tmpCfg.VeleroCLI, tmpCfg.VeleroVersion)).To(Succeed()) } else { diff --git a/test/e2e/util/k8s/common.go b/test/e2e/util/k8s/common.go index cf18e7f0a8..020de0e282 100644 --- a/test/e2e/util/k8s/common.go +++ b/test/e2e/util/k8s/common.go @@ -200,6 +200,7 @@ func AddLabelToCRD(ctx context.Context, crd, label string) error { func KubectlApplyByFile(ctx context.Context, file string) error { args := []string{"apply", "-f", file, "--force=true"} + fmt.Println(args) return exec.CommandContext(ctx, "kubectl", args...).Run() } diff --git a/test/e2e/util/k8s/statefulset.go b/test/e2e/util/k8s/statefulset.go new file mode 100644 index 0000000000..e9a1e564d2 --- /dev/null +++ b/test/e2e/util/k8s/statefulset.go @@ -0,0 +1,39 @@ +/* +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 k8s + +import ( + "fmt" + "os/exec" + + "github.com/pkg/errors" + "golang.org/x/net/context" + + veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" +) + +func ScaleStatefulSet(ctx context.Context, namespace, name string, replicas int) error { + cmd := exec.CommandContext(ctx, "kubectl", "scale", "statefulsets", name, fmt.Sprintf("--replicas=%d", replicas), "-n", namespace) + fmt.Printf("Scale kibishii stateful set in namespace %s with CMD: %s", name, cmd.Args) + + _, stderr, err := veleroexec.RunCommand(cmd) + if err != nil { + return errors.Wrap(err, stderr) + } + + return nil +} diff --git a/test/e2e/util/kibishii/kibishii_utils.go b/test/e2e/util/kibishii/kibishii_utils.go index 2edb3acc0f..ea4e419a7d 100644 --- a/test/e2e/util/kibishii/kibishii_utils.go +++ b/test/e2e/util/kibishii/kibishii_utils.go @@ -48,8 +48,11 @@ type KibishiiData struct { ExpectedNodes int } -var DefaultKibishiiData = &KibishiiData{2, 10, 10, 1024, 1024, 0, 2} +var DefaultKibishiiWorkerCounts = 2 +var DefaultKibishiiData = &KibishiiData{2, 10, 10, 1024, 1024, 0, DefaultKibishiiWorkerCounts} + var KibishiiPVCNameList = []string{"kibishii-data-kibishii-deployment-0", "kibishii-data-kibishii-deployment-1"} +var KibishiiStorageClassName = "kibishii-storage-class" // RunKibishiiTests runs kibishii tests on the provider. func RunKibishiiTests(veleroCfg VeleroConfig, backupName, restoreName, backupLocation, kibishiiNamespace string, @@ -196,11 +199,15 @@ func RunKibishiiTests(veleroCfg VeleroConfig, backupName, restoreName, backupLoc } func installKibishii(ctx context.Context, namespace string, cloudPlatform, veleroFeatures, - kibishiiDirectory string, useVolumeSnapshots bool) error { + kibishiiDirectory string, useVolumeSnapshots bool, workerReplicas int) error { if strings.EqualFold(cloudPlatform, "azure") && strings.EqualFold(veleroFeatures, "EnableCSI") { cloudPlatform = "azure-csi" } + if strings.EqualFold(cloudPlatform, "aws") && + strings.EqualFold(veleroFeatures, "EnableCSI") { + cloudPlatform = "aws-csi" + } // We use kustomize to generate YAML for Kibishii from the checked-in yaml directories kibishiiInstallCmd := exec.CommandContext(ctx, "kubectl", "apply", "-n", namespace, "-k", kibishiiDirectory+cloudPlatform, "--timeout=90s") @@ -216,6 +223,12 @@ func installKibishii(ctx context.Context, namespace string, cloudPlatform, veler if err != nil { return errors.Wrapf(err, "failed to label namespace with PSA policy, stderr=%s", stderr) } + if workerReplicas != DefaultKibishiiWorkerCounts { + err = ScaleStatefulSet(ctx, namespace, "kibishii-deployment", workerReplicas) + if err != nil { + return errors.Wrapf(err, "failed to scale statefulset, stderr=%s", err.Error()) + } + } kibishiiSetWaitCmd := exec.CommandContext(ctx, "kubectl", "rollout", "status", "statefulset.apps/kibishii-deployment", "-n", namespace, "-w", "--timeout=30m") @@ -311,7 +324,7 @@ func KibishiiPrepareBeforeBackup(oneHourTimeout context.Context, client TestClie } if err := installKibishii(oneHourTimeout, kibishiiNamespace, providerName, veleroFeatures, - kibishiiDirectory, useVolumeSnapshots); err != nil { + kibishiiDirectory, useVolumeSnapshots, kibishiiData.ExpectedNodes); err != nil { return errors.Wrap(err, "Failed to install Kibishii workload") } // wait for kibishii pod startup diff --git a/test/e2e/util/velero/install.go b/test/e2e/util/velero/install.go index be50c95b2d..1830b15d3a 100644 --- a/test/e2e/util/velero/install.go +++ b/test/e2e/util/velero/install.go @@ -55,8 +55,23 @@ type installOptions struct { VeleroServerDebugMode bool } -func VeleroInstall(ctx context.Context, veleroCfg *VeleroConfig) error { +func VeleroInstall(ctx context.Context, veleroCfg *VeleroConfig, isStandbyCluster bool) error { fmt.Printf("Velero install %s\n", time.Now().Format("2006-01-02 15:04:05")) + // veleroCfg struct including a set of BSL params and a set of additional BSL params, + // additional BSL set is for additional BSL test only, so only default BSL set is effective + // for VeleroInstall(). + // + // veleroCfg struct including 2 sets of cluster setting, but VeleroInstall() only read + // default cluster settings, so if E2E test needs install on the standby cluster, default cluster + // setting should be reset to the value of standby cluster's. + // + // Some other setting might not needed by standby cluster installation like "snapshotMoveData", because in + // standby cluster only restore if performed, so CSI plugin is not needed, but it is installed due to + // the only one veleroCfg setting is provided as current design, since it will not introduce any issues as + // we can predict, so keep it intact for now. + if isStandbyCluster { + veleroCfg.CloudProvider = veleroCfg.StandbyClusterCloudProvider + } if veleroCfg.CloudProvider != "kind" { fmt.Printf("For cloud platforms, object store plugin provider will be set as cloud provider") // If ObjectStoreProvider is not provided, then using the value same as CloudProvider @@ -69,7 +84,7 @@ func VeleroInstall(ctx context.Context, veleroCfg *VeleroConfig) error { } } - providerPluginsTmp, err := getProviderPlugins(ctx, veleroCfg.VeleroCLI, veleroCfg.ObjectStoreProvider, veleroCfg.CloudProvider, veleroCfg.Plugins, veleroCfg.Features) + pluginsTmp, err := getPlugins(ctx, *veleroCfg) if err != nil { return errors.WithMessage(err, "Failed to get provider plugins") } @@ -91,22 +106,19 @@ func VeleroInstall(ctx context.Context, veleroCfg *VeleroConfig) error { } } - veleroInstallOptions, err := getProviderVeleroInstallOptions(veleroCfg, providerPluginsTmp) + veleroInstallOptions, err := getProviderVeleroInstallOptions(veleroCfg, pluginsTmp) if err != nil { return errors.WithMessagef(err, "Failed to get Velero InstallOptions for plugin provider %s", veleroCfg.ObjectStoreProvider) } veleroInstallOptions.UseVolumeSnapshots = veleroCfg.UseVolumeSnapshots - if !veleroCfg.UseRestic { - veleroInstallOptions.UseNodeAgent = veleroCfg.UseNodeAgent - } - veleroInstallOptions.UseRestic = veleroCfg.UseRestic + veleroInstallOptions.UseNodeAgent = veleroCfg.UseNodeAgent veleroInstallOptions.Image = veleroCfg.VeleroImage veleroInstallOptions.Namespace = veleroCfg.VeleroNamespace veleroInstallOptions.UploaderType = veleroCfg.UploaderType GCFrequency, _ := time.ParseDuration(veleroCfg.GCFrequency) veleroInstallOptions.GarbageCollectionFrequency = GCFrequency - err = installVeleroServer(ctx, veleroCfg.VeleroCLI, &installOptions{ + err = installVeleroServer(ctx, veleroCfg.VeleroCLI, veleroCfg.CloudProvider, &installOptions{ Options: veleroInstallOptions, RegistryCredentialFile: veleroCfg.RegistryCredentialFile, RestoreHelperImage: veleroCfg.RestoreHelperImage, @@ -176,7 +188,7 @@ func clearupvSpherePluginConfig(c clientset.Interface, ns, secretName, configMap return nil } -func installVeleroServer(ctx context.Context, cli string, options *installOptions) error { +func installVeleroServer(ctx context.Context, cli, cloudProvider string, options *installOptions) error { args := []string{"install"} namespace := "velero" if len(options.Namespace) > 0 { @@ -192,9 +204,6 @@ func installVeleroServer(ctx context.Context, cli string, options *installOption if options.DefaultVolumesToFsBackup { args = append(args, "--default-volumes-to-fs-backup") } - if options.UseRestic { - args = append(args, "--use-restic") - } if options.UseVolumeSnapshots { args = append(args, "--use-volume-snapshots") } @@ -219,10 +228,11 @@ func installVeleroServer(ctx context.Context, cli string, options *installOption if len(options.Plugins) > 0 { args = append(args, "--plugins", options.Plugins.String()) } + fmt.Println("Start to install Azure VolumeSnapshotClass ...") if len(options.Features) > 0 { args = append(args, "--features", options.Features) if strings.EqualFold(options.Features, "EnableCSI") && options.UseVolumeSnapshots { - if strings.EqualFold(options.ProviderName, "Azure") { + if strings.EqualFold(cloudProvider, "azure") { if err := KubectlApplyByFile(ctx, "util/csi/AzureVolumeSnapshotClass.yaml"); err != nil { return err } @@ -528,7 +538,7 @@ func PrepareVelero(ctx context.Context, caseName string) error { return nil } fmt.Printf("need to install velero for case %s \n", caseName) - return VeleroInstall(context.Background(), &VeleroCfg) + return VeleroInstall(context.Background(), &VeleroCfg, false) } func VeleroUninstall(ctx context.Context, cli, namespace string) error { diff --git a/test/e2e/util/velero/velero_utils.go b/test/e2e/util/velero/velero_utils.go index 7da65f945b..c5d1834c16 100644 --- a/test/e2e/util/velero/velero_utils.go +++ b/test/e2e/util/velero/velero_utils.go @@ -34,6 +34,7 @@ import ( "time" "github.com/pkg/errors" + "golang.org/x/exp/slices" "k8s.io/apimachinery/pkg/util/wait" kbclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -70,15 +71,16 @@ var pluginsMatrix = map[string]map[string][]string{ "csi": {"velero/velero-plugin-for-csi:v0.5.0"}, }, "main": { - "aws": {"velero/velero-plugin-for-aws:main"}, - "azure": {"velero/velero-plugin-for-microsoft-azure:main"}, - "vsphere": {"vsphereveleroplugin/velero-plugin-for-vsphere:v1.5.1"}, - "gcp": {"velero/velero-plugin-for-gcp:main"}, - "csi": {"velero/velero-plugin-for-csi:main"}, + "aws": {"velero/velero-plugin-for-aws:main"}, + "azure": {"velero/velero-plugin-for-microsoft-azure:main"}, + "vsphere": {"vsphereveleroplugin/velero-plugin-for-vsphere:v1.5.1"}, + "gcp": {"velero/velero-plugin-for-gcp:main"}, + "csi": {"velero/velero-plugin-for-csi:main"}, + "datamover": {"velero/velero-plugin-for-aws:main"}, }, } -func GetProviderPluginsByVersion(version, providerName, feature string) ([]string, error) { +func getPluginsByVersion(version, cloudProvider, objectStoreProvider, feature string, needDataMoverPlugin bool) ([]string, error) { var cloudMap map[string][]string arr := strings.Split(version, ".") if len(arr) >= 3 { @@ -92,17 +94,47 @@ func GetProviderPluginsByVersion(version, providerName, feature string) ([]strin } var pluginsForFeature []string - plugins, ok := cloudMap[providerName] + if cloudProvider == "kind" { + plugins, ok := cloudMap["aws"] + if !ok { + return nil, errors.Errorf("fail to get plugins by version: %s and provider %s", version, cloudProvider) + } + return plugins, nil + } + + plugins, ok := cloudMap[cloudProvider] if !ok { - return nil, errors.Errorf("fail to get plugins by version: %s and provider %s", version, providerName) + return nil, errors.Errorf("fail to get plugins by version: %s and provider %s", version, cloudProvider) } + + if objectStoreProvider != cloudProvider { + pluginsForObjectStoreProvider, ok := cloudMap[objectStoreProvider] + if !ok { + return nil, errors.Errorf("fail to get plugins by version: %s and object store provider %s", version, objectStoreProvider) + } + plugins = append(plugins, pluginsForObjectStoreProvider...) + } + if strings.EqualFold(feature, "EnableCSI") { pluginsForFeature, ok = cloudMap["csi"] if !ok { - return nil, errors.Errorf("fail to get plugins by version: %s and provider %s", version, providerName) + return nil, errors.Errorf("fail to get CSI plugins by version: %s ", version) } + plugins = append(plugins, pluginsForFeature...) } - return append(plugins, pluginsForFeature...), nil + if needDataMoverPlugin { + pluginsForDatamover, ok := cloudMap["datamover"] + if !ok { + return nil, errors.Errorf("fail to get plugins by for datamover") + } + for _, p := range pluginsForDatamover { + if !slices.Contains(plugins, p) { + plugins = append(plugins, pluginsForDatamover...) + } + } + + } + return plugins, nil } // getProviderVeleroInstallOptions returns Velero InstallOptions for the provider. @@ -280,6 +312,10 @@ func VeleroBackupNamespace(ctx context.Context, veleroCLI, veleroNamespace strin args = append(args, "--selector", backupCfg.Selector) } + if backupCfg.SnapshotMoveData { + args = append(args, "--snapshot-move-data") + } + if backupCfg.UseVolumeSnapshots { if backupCfg.ProvideSnapshotsVolumeParam { args = append(args, "--snapshot-volumes") @@ -516,36 +552,67 @@ func VeleroVersion(ctx context.Context, veleroCLI, veleroNamespace string) error return nil } -func getProviderPlugins(ctx context.Context, veleroCLI, objectStoreProvider, cloudProvider, providerPlugins, feature string) ([]string, error) { +// getProviderPlugins only provide plugin for specific cloud provider +func getProviderPlugins(ctx context.Context, veleroCLI string, cloudProvider string) ([]string, error) { + if cloudProvider == "" { + return []string{}, errors.New("CloudProvider should be provided") + } + + version, err := getVeleroVersion(ctx, veleroCLI, true) + if err != nil { + return nil, errors.WithMessage(err, "failed to get velero version") + } + + plugins, err := getPluginsByVersion(version, cloudProvider, cloudProvider, "", false) + if err != nil { + return nil, errors.WithMessagef(err, "Fail to get plugin by provider %s and version %s", cloudProvider, version) + } + + return plugins, nil +} + +// getPlugins will collect all kinds plugins for VeleroInstall, such as provider +// plugins(cloud provider/object store provider, if object store provider is not +// provided, it should be set to value as cloud provider's), feature plugins (CSI/Datamover) +func getPlugins(ctx context.Context, veleroCfg VeleroConfig) ([]string, error) { + veleroCLI := veleroCfg.VeleroCLI + cloudProvider := veleroCfg.CloudProvider + objectStoreProvider := veleroCfg.ObjectStoreProvider + providerPlugins := veleroCfg.Plugins + feature := veleroCfg.Features + needDataMoverPlugin := false + // Fetch the plugins for the provider before checking for the object store provider below. var plugins []string if len(providerPlugins) > 0 { plugins = strings.Split(providerPlugins, ",") } else { + if cloudProvider == "" { + return []string{}, errors.New("CloudProvider should be provided") + } + if objectStoreProvider == "" { + objectStoreProvider = cloudProvider + } version, err := getVeleroVersion(ctx, veleroCLI, true) if err != nil { return nil, errors.WithMessage(err, "failed to get velero version") } - plugins, err = GetProviderPluginsByVersion(version, objectStoreProvider, feature) + + if veleroCfg.SnapshotMoveData && veleroCfg.DataMoverPlugin == "" { + needDataMoverPlugin = true + } + plugins, err = getPluginsByVersion(version, cloudProvider, objectStoreProvider, feature, needDataMoverPlugin) if err != nil { return nil, errors.WithMessagef(err, "Fail to get plugin by provider %s and version %s", objectStoreProvider, version) } - if objectStoreProvider != "" && cloudProvider != "kind" && objectStoreProvider != cloudProvider { - pluginsTmp, err := GetProviderPluginsByVersion(version, cloudProvider, feature) - if err != nil { - return nil, errors.WithMessage(err, "failed to get velero version") - } - plugins = append(plugins, pluginsTmp...) - } } return plugins, nil } // VeleroAddPluginsForProvider determines which plugins need to be installed for a provider and // installs them in the current Velero installation, skipping over those that are already installed. -func VeleroAddPluginsForProvider(ctx context.Context, veleroCLI string, veleroNamespace string, provider string, addPlugins, feature string) error { - plugins, err := getProviderPlugins(ctx, veleroCLI, provider, provider, addPlugins, feature) - fmt.Printf("addPlugins cmd =%v\n", addPlugins) +func VeleroAddPluginsForProvider(ctx context.Context, veleroCLI string, veleroNamespace string, provider string) error { + plugins, err := getProviderPlugins(ctx, veleroCLI, provider) fmt.Printf("provider cmd = %v\n", provider) fmt.Printf("plugins cmd = %v\n", plugins) if err != nil { From 480fe445b1bb6a3faa28aa679b11b45b326f1899 Mon Sep 17 00:00:00 2001 From: Ming Qiu Date: Wed, 5 Jul 2023 15:55:42 +0800 Subject: [PATCH 04/14] Mark dataupload datadownload status failed when velero pod restart Signed-off-by: Ming Qiu --- pkg/cmd/cli/nodeagent/server.go | 64 +++++++++++++- pkg/cmd/server/server.go | 56 +++++++++++- pkg/controller/data_download_controller.go | 86 ++++++++++++++++--- .../data_download_controller_test.go | 10 ++- pkg/controller/data_upload_controller.go | 85 ++++++++++++++---- pkg/controller/data_upload_controller_test.go | 14 ++- 6 files changed, 277 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index 3f635602ed..19729e1c45 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -256,11 +256,15 @@ func (s *nodeAgentServer) run() { s.logger.WithError(err).Fatal("Unable to create the pod volume restore controller") } - if err = controller.NewDataUploadReconciler(s.mgr.GetClient(), s.kubeClient, s.csiSnapshotClient.SnapshotV1(), repoEnsurer, clock.RealClock{}, credentialGetter, s.nodeName, s.fileSystem, s.logger).SetupWithManager(s.mgr); err != nil { + dataUploadReconciler := controller.NewDataUploadReconciler(s.mgr.GetClient(), s.kubeClient, s.csiSnapshotClient.SnapshotV1(), repoEnsurer, clock.RealClock{}, credentialGetter, s.nodeName, s.fileSystem, s.logger) + s.markDataUploadsCancel(dataUploadReconciler) + if err = dataUploadReconciler.SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data upload controller") } - if err = controller.NewDataDownloadReconciler(s.mgr.GetClient(), s.kubeClient, repoEnsurer, credentialGetter, s.nodeName, s.logger).SetupWithManager(s.mgr); err != nil { + dataDownloadReconciler := controller.NewDataDownloadReconciler(s.mgr.GetClient(), s.kubeClient, repoEnsurer, credentialGetter, s.nodeName, s.logger) + s.markDataDownloadsCancel(dataDownloadReconciler) + if err = dataDownloadReconciler.SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data download controller") } @@ -333,6 +337,62 @@ func (s *nodeAgentServer) markInProgressCRsFailed() { s.markInProgressPVRsFailed(client) } +func (s *nodeAgentServer) markDataUploadsCancel(r *controller.DataUploadReconciler) { + // the function is called before starting the controller manager, the embedded client isn't ready to use, so create a new one here + client, err := ctrlclient.New(s.mgr.GetConfig(), ctrlclient.Options{Scheme: s.mgr.GetScheme()}) + if err != nil { + s.logger.WithError(errors.WithStack(err)).Error("failed to create client") + return + } + if dataUploads, err := r.FindDataUploads(s.ctx, client, s.namespace); err != nil { + s.logger.WithError(errors.WithStack(err)).Error("failed to find data downloads") + } else { + for i := range dataUploads { + du := dataUploads[i] + if du.Status.Phase == velerov2alpha1api.DataUploadPhaseAccepted || + du.Status.Phase == velerov2alpha1api.DataUploadPhasePrepared || + du.Status.Phase == velerov2alpha1api.DataUploadPhaseInProgress { + updated := du.DeepCopy() + updated.Spec.Cancel = true + updated.Status.Message = fmt.Sprintf("found a dataupload with status %q during the node-agent starting, mark it as cancel", du.Status.Phase) + if err := client.Patch(s.ctx, updated, ctrlclient.MergeFrom(&du)); err != nil { + s.logger.WithError(errors.WithStack(err)).Errorf("failed to mark datadownload %q cancel", du.GetName()) + continue + } + s.logger.WithField("dataupload", du.GetName()).Warn(du.Status.Message) + } + } + } +} + +func (s *nodeAgentServer) markDataDownloadsCancel(r *controller.DataDownloadReconciler) { + // the function is called before starting the controller manager, the embedded client isn't ready to use, so create a new one here + client, err := ctrlclient.New(s.mgr.GetConfig(), ctrlclient.Options{Scheme: s.mgr.GetScheme()}) + if err != nil { + s.logger.WithError(errors.WithStack(err)).Error("failed to create client") + return + } + if dataDownloads, err := r.FindDataDownloads(s.ctx, client, s.namespace); err != nil { + s.logger.WithError(errors.WithStack(err)).Error("failed to find data downloads") + } else { + for i := range dataDownloads { + dd := dataDownloads[i] + if dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseAccepted || + dd.Status.Phase == velerov2alpha1api.DataDownloadPhasePrepared || + dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseInProgress { + updated := dd.DeepCopy() + updated.Spec.Cancel = true + updated.Status.Message = fmt.Sprintf("found a datadownload with status %q during the node-agent starting, mark it as cancel", dd.Status.Phase) + if err := client.Patch(s.ctx, updated, ctrlclient.MergeFrom(dd)); err != nil { + s.logger.WithError(errors.WithStack(err)).Errorf("failed to mark datadownload %q cancel", dd.GetName()) + continue + } + s.logger.WithField("datadownload", dd.GetName()).Warn(dd.Status.Message) + } + } + } +} + func (s *nodeAgentServer) markInProgressPVBsFailed(client ctrlclient.Client) { pvbs := &velerov1api.PodVolumeBackupList{} if err := client.List(s.ctx, pvbs, &ctrlclient.MatchingFields{"metadata.namespace": s.namespace}); err != nil { diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 00a4456f0c..e0c774373e 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -1065,7 +1065,7 @@ func markInProgressBackupsFailed(ctx context.Context, client ctrlclient.Client, } for i, backup := range backups.Items { - if backup.Status.Phase != velerov1api.BackupPhaseInProgress { + if backup.Status.Phase != velerov1api.BackupPhaseInProgress && backup.Status.Phase != velerov1api.BackupPhaseWaitingForPluginOperations { log.Debugf("the status of backup %q is %q, skip", backup.GetName(), backup.Status.Phase) continue } @@ -1078,6 +1078,7 @@ func markInProgressBackupsFailed(ctx context.Context, client ctrlclient.Client, continue } log.WithField("backup", backup.GetName()).Warn(updated.Status.FailureReason) + markDataUploadsCancel(ctx, client, backup, log) } } @@ -1088,7 +1089,7 @@ func markInProgressRestoresFailed(ctx context.Context, client ctrlclient.Client, return } for i, restore := range restores.Items { - if restore.Status.Phase != velerov1api.RestorePhaseInProgress { + if restore.Status.Phase != velerov1api.RestorePhaseInProgress && restore.Status.Phase != velerov1api.RestorePhaseWaitingForPluginOperations { log.Debugf("the status of restore %q is %q, skip", restore.GetName(), restore.Status.Phase) continue } @@ -1101,5 +1102,56 @@ func markInProgressRestoresFailed(ctx context.Context, client ctrlclient.Client, continue } log.WithField("restore", restore.GetName()).Warn(updated.Status.FailureReason) + markDataDownloadsCancel(ctx, client, restore, log) + } +} + +func markDataUploadsCancel(ctx context.Context, client ctrlclient.Client, backup velerov1api.Backup, log logrus.FieldLogger) { + dataUploads := &velerov2alpha1api.DataUploadList{} + + if err := client.List(ctx, dataUploads, &ctrlclient.MatchingFields{"metadata.namespace": backup.GetNamespace()}, &ctrlclient.MatchingLabels{velerov1api.BackupUIDLabel: string(backup.GetUID())}); err != nil { + log.WithError(errors.WithStack(err)).Error("failed to list dataUploads") + return + } + + for i := range dataUploads.Items { + du := dataUploads.Items[i] + if du.Status.Phase == velerov2alpha1api.DataUploadPhaseAccepted || + du.Status.Phase == velerov2alpha1api.DataUploadPhasePrepared || + du.Status.Phase == velerov2alpha1api.DataUploadPhaseInProgress { + updated := du.DeepCopy() + updated.Spec.Cancel = true + updated.Status.Message = fmt.Sprintf("found a dataupload with status %q during the velero server starting, mark it as cancel", du.Status.Phase) + if err := client.Patch(ctx, updated, ctrlclient.MergeFrom(&du)); err != nil { + log.WithError(errors.WithStack(err)).Errorf("failed to mark dataupload %q cancel", du.GetName()) + continue + } + log.WithField("dataupload", du.GetName()).Warn(du.Status.Message) + } + } +} + +func markDataDownloadsCancel(ctx context.Context, client ctrlclient.Client, restore velerov1api.Restore, log logrus.FieldLogger) { + dataDownloads := &velerov2alpha1api.DataDownloadList{} + + if err := client.List(ctx, dataDownloads, &ctrlclient.MatchingFields{"metadata.namespace": restore.GetNamespace()}, &ctrlclient.MatchingLabels{velerov1api.RestoreUIDLabel: string(restore.GetUID())}); err != nil { + log.WithError(errors.WithStack(err)).Error("failed to list dataDownloads") + return + } + + for i := range dataDownloads.Items { + dd := dataDownloads.Items[i] + if dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseAccepted || + dd.Status.Phase == velerov2alpha1api.DataDownloadPhasePrepared || + dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseInProgress { + updated := dd.DeepCopy() + updated.Spec.Cancel = true + updated.Status.Message = fmt.Sprintf("found a datadownload with status %q during the velero server starting, mark it as cancel", dd.Status.Phase) + if err := client.Patch(ctx, updated, ctrlclient.MergeFrom(&dd)); err != nil { + log.WithError(errors.WithStack(err)).Errorf("failed to mark dataupload %q cancel", dd.GetName()) + continue + } + log.WithField("datadownload", dd.GetName()).Warn(dd.Status.Message) + } } } diff --git a/pkg/controller/data_download_controller.go b/pkg/controller/data_download_controller.go index 4750cb0edb..2bb6251d26 100644 --- a/pkg/controller/data_download_controller.go +++ b/pkg/controller/data_download_controller.go @@ -57,7 +57,7 @@ type DataDownloadReconciler struct { logger logrus.FieldLogger credentialGetter *credentials.CredentialGetter fileSystem filesystem.Interface - clock clock.WithTickerAndDelayedExecution + Clock clock.WithTickerAndDelayedExecution restoreExposer exposer.GenericRestoreExposer nodeName string repositoryEnsurer *repository.Ensurer @@ -72,7 +72,7 @@ func NewDataDownloadReconciler(client client.Client, kubeClient kubernetes.Inter logger: logger.WithField("controller", "DataDownload"), credentialGetter: credentialGetter, fileSystem: filesystem.NewFileSystem(), - clock: &clock.RealClock{}, + Clock: &clock.RealClock{}, nodeName: nodeName, repositoryEnsurer: repoEnsurer, restoreExposer: exposer.NewGenericRestoreExposer(kubeClient, logger), @@ -132,9 +132,15 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request log.Info("Data download is accepted") + if dd.Spec.Cancel { + log.Debugf("Data download is been canceled %s in Phase %s", dd.GetName(), dd.Status.Phase) + r.OnDataDownloadCancelled(ctx, dd.GetNamespace(), dd.GetName()) + return ctrl.Result{}, nil + } + hostingPodLabels := map[string]string{velerov1api.DataDownloadLabel: dd.Name} - // ep.Expose() will trigger to create one pod whose volume is restored by a given volume snapshot, + // Expose() will trigger to create one pod whose volume is restored by a given volume snapshot, // but the pod maybe is not in the same node of the current controller, so we need to return it here. // And then only the controller who is in the same node could do the rest work. err = r.restoreExposer.Expose(ctx, getDataDownloadOwnerObject(dd), dd.Spec.TargetVolume.PVC, dd.Spec.TargetVolume.Namespace, hostingPodLabels, dd.Spec.OperationTimeout.Duration) @@ -143,9 +149,22 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request } log.Info("Restore is exposed") + return ctrl.Result{}, nil + } else if dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseAccepted { + if dd.Spec.Cancel { + log.Debugf("Data download is been canceled %s in Phase %s", dd.GetName(), dd.Status.Phase) + r.OnDataDownloadCancelled(ctx, dd.GetNamespace(), dd.GetName()) + } return ctrl.Result{}, nil } else if dd.Status.Phase == velerov2alpha1api.DataDownloadPhasePrepared { log.Info("Data download is prepared") + + if dd.Spec.Cancel { + log.Debugf("Data download is been canceled %s in Phase %s", dd.GetName(), dd.Status.Phase) + r.OnDataDownloadCancelled(ctx, dd.GetNamespace(), dd.GetName()) + return ctrl.Result{}, nil + } + fsRestore := r.dataPathMgr.GetAsyncBR(dd.Name) if fsRestore != nil { @@ -184,7 +203,7 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request // Update status to InProgress original := dd.DeepCopy() dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseInProgress - dd.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + dd.Status.StartTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, dd, client.MergeFrom(original)); err != nil { log.WithError(err).Error("Unable to update status to in progress") return ctrl.Result{}, err @@ -271,7 +290,7 @@ func (r *DataDownloadReconciler) OnDataDownloadCompleted(ctx context.Context, na original := dd.DeepCopy() dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseCompleted - dd.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + dd.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, &dd, client.MergeFrom(original)); err != nil { log.WithError(err).Error("error updating data download status") } else { @@ -313,9 +332,9 @@ func (r *DataDownloadReconciler) OnDataDownloadCancelled(ctx context.Context, na original := dd.DeepCopy() dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseCanceled if dd.Status.StartTimestamp.IsZero() { - dd.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + dd.Status.StartTimestamp = &metav1.Time{Time: r.Clock.Now()} } - dd.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + dd.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, &dd, client.MergeFrom(original)); err != nil { log.WithError(err).Error("error updating data download status") } @@ -382,15 +401,13 @@ func (r *DataDownloadReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *DataDownloadReconciler) findSnapshotRestoreForPod(podObj client.Object) []reconcile.Request { pod := podObj.(*v1.Pod) - dd := &velerov2alpha1api.DataDownload{} - err := r.client.Get(context.Background(), types.NamespacedName{ - Namespace: pod.Namespace, - Name: pod.Labels[velerov1api.DataDownloadLabel], - }, dd) - + dd, err := findDataDownloadByPod(r.client, *pod) if err != nil { r.logger.WithField("Restore pod", pod.Name).WithError(err).Error("unable to get DataDownload") return []reconcile.Request{} + } else if dd == nil { + r.logger.WithField("Restore pod", pod.Name).Error("get empty DataDownload") + return []reconcile.Request{} } if dd.Status.Phase != velerov2alpha1api.DataDownloadPhaseAccepted { @@ -416,6 +433,30 @@ func (r *DataDownloadReconciler) findSnapshotRestoreForPod(podObj client.Object) return requests } +func (r *DataDownloadReconciler) FindDataDownloads(ctx context.Context, cli client.Client, ns string) ([]*velerov2alpha1api.DataDownload, error) { + pods := &v1.PodList{} + var dataDownloads []*velerov2alpha1api.DataDownload + if err := cli.List(ctx, pods, &client.ListOptions{Namespace: ns}); err != nil { + r.logger.WithError(errors.WithStack(err)).Error("failed to list pods on current node") + return nil, errors.Wrapf(err, "failed to list pods on current node") + } + + for _, pod := range pods.Items { + if pod.Spec.NodeName != r.nodeName { + r.logger.Debugf("Pod %s related data download will not handled by %s nodes", pod.GetName(), r.nodeName) + continue + } + dd, err := findDataDownloadByPod(cli, pod) + if err != nil { + r.logger.WithError(errors.WithStack(err)).Error("failed to get dataDownload by pod") + continue + } else if dd != nil { + dataDownloads = append(dataDownloads, dd) + } + } + return dataDownloads, nil +} + func (r *DataDownloadReconciler) patchDataDownload(ctx context.Context, req *velerov2alpha1api.DataDownload, mutate func(*velerov2alpha1api.DataDownload)) error { original := req.DeepCopy() mutate(req) @@ -442,7 +483,7 @@ func (r *DataDownloadReconciler) updateStatusToFailed(ctx context.Context, dd *v original := dd.DeepCopy() dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseFailed dd.Status.Message = errors.WithMessage(err, msg).Error() - dd.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + dd.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if patchErr := r.client.Patch(ctx, dd, client.MergeFrom(original)); patchErr != nil { log.WithError(patchErr).Error("error updating DataDownload status") @@ -491,3 +532,20 @@ func getDataDownloadOwnerObject(dd *velerov2alpha1api.DataDownload) v1.ObjectRef APIVersion: dd.APIVersion, } } + +func findDataDownloadByPod(client client.Client, pod v1.Pod) (*velerov2alpha1api.DataDownload, error) { + if label, exist := pod.Labels[velerov1api.DataDownloadLabel]; exist { + dd := &velerov2alpha1api.DataDownload{} + err := client.Get(context.Background(), types.NamespacedName{ + Namespace: pod.Namespace, + Name: label, + }, dd) + + if err != nil { + return nil, errors.Wrapf(err, "error to find DataDownload by pod %s/%s", pod.Namespace, pod.Name) + } + return dd, nil + } + + return nil, nil +} diff --git a/pkg/controller/data_download_controller_test.go b/pkg/controller/data_download_controller_test.go index 773112207a..9aeadd859a 100644 --- a/pkg/controller/data_download_controller_test.go +++ b/pkg/controller/data_download_controller_test.go @@ -550,6 +550,14 @@ func TestFindDataDownloadForPod(t *testing.T) { assert.Equal(t, du.Namespace, requests[0].Namespace) assert.Equal(t, du.Name, requests[0].Name) }, + }, { + name: "no selected label found for pod", + du: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Result(), + pod: builder.ForPod(velerov1api.DefaultNamespace, dataDownloadName).Result(), + checkFunc: func(du *velerov2alpha1api.DataDownload, requests []reconcile.Request) { + // Assert that the function returns a single request + assert.Empty(t, requests) + }, }, { name: "no matched pod", du: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).Result(), @@ -559,7 +567,7 @@ func TestFindDataDownloadForPod(t *testing.T) { }, }, { - name: "dataDownload not accepte", + name: "dataDownload not accept", du: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseInProgress).Result(), pod: builder.ForPod(velerov1api.DefaultNamespace, dataDownloadName).Labels(map[string]string{velerov1api.DataDownloadLabel: dataDownloadName}).Result(), checkFunc: func(du *velerov2alpha1api.DataDownload, requests []reconcile.Request) { diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index 735026cda2..daf9b40a07 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -61,7 +61,7 @@ type DataUploadReconciler struct { kubeClient kubernetes.Interface csiSnapshotClient snapshotter.SnapshotV1Interface repoEnsurer *repository.Ensurer - clock clocks.WithTickerAndDelayedExecution + Clock clocks.WithTickerAndDelayedExecution credentialGetter *credentials.CredentialGetter nodeName string fileSystem filesystem.Interface @@ -77,7 +77,7 @@ func NewDataUploadReconciler(client client.Client, kubeClient kubernetes.Interfa client: client, kubeClient: kubeClient, csiSnapshotClient: csiSnapshotClient, - clock: clock, + Clock: clock, credentialGetter: cred, nodeName: nodeName, fileSystem: fs, @@ -134,18 +134,34 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) log.Info("Data upload is accepted") + if du.Spec.Cancel { + r.OnDataUploadCancelled(ctx, du.GetNamespace(), du.GetName()) + return ctrl.Result{}, nil + } + exposeParam := r.setupExposeParam(&du) if err := ep.Expose(ctx, getOwnerObject(&du), exposeParam); err != nil { return r.errorOut(ctx, &du, err, "error to expose snapshot", log) } log.Info("Snapshot is exposed") - // ep.Expose() will trigger to create one pod whose volume is restored by a given volume snapshot, + // Expose() will trigger to create one pod whose volume is restored by a given volume snapshot, // but the pod maybe is not in the same node of the current controller, so we need to return it here. // And then only the controller who is in the same node could do the rest work. return ctrl.Result{}, nil + } else if du.Status.Phase == velerov2alpha1api.DataUploadPhaseAccepted { + if du.Spec.Cancel { + r.OnDataUploadCancelled(ctx, du.GetNamespace(), du.GetName()) + } + return ctrl.Result{}, nil } else if du.Status.Phase == velerov2alpha1api.DataUploadPhasePrepared { log.Info("Data upload is prepared") + + if du.Spec.Cancel { + r.OnDataUploadCancelled(ctx, du.GetNamespace(), du.GetName()) + return ctrl.Result{}, nil + } + fsBackup := r.dataPathMgr.GetAsyncBR(du.Name) if fsBackup != nil { log.Info("Cancellable data path is already started") @@ -183,7 +199,7 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Update status to InProgress original := du.DeepCopy() du.Status.Phase = velerov2alpha1api.DataUploadPhaseInProgress - du.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + du.Status.StartTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, &du, client.MergeFrom(original)); err != nil { return r.errorOut(ctx, &du, err, "error updating dataupload status", log) } @@ -277,7 +293,7 @@ func (r *DataUploadReconciler) OnDataUploadCompleted(ctx context.Context, namesp du.Status.Path = result.Backup.Source.ByPath du.Status.Phase = velerov2alpha1api.DataUploadPhaseCompleted du.Status.SnapshotID = result.Backup.SnapshotID - du.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + du.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if result.Backup.EmptySnapshot { du.Status.Message = "volume was empty so no data was upload" } @@ -331,9 +347,9 @@ func (r *DataUploadReconciler) OnDataUploadCancelled(ctx context.Context, namesp original := du.DeepCopy() du.Status.Phase = velerov2alpha1api.DataUploadPhaseCanceled if du.Status.StartTimestamp.IsZero() { - du.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + du.Status.StartTimestamp = &metav1.Time{Time: r.Clock.Now()} } - du.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + du.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, &du, client.MergeFrom(original)); err != nil { log.WithError(err).Error("error updating DataUpload status") } @@ -399,16 +415,13 @@ func (r *DataUploadReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *DataUploadReconciler) findDataUploadForPod(podObj client.Object) []reconcile.Request { pod := podObj.(*corev1.Pod) - - du := &velerov2alpha1api.DataUpload{} - err := r.client.Get(context.Background(), types.NamespacedName{ - Namespace: pod.Namespace, - Name: pod.Labels[velerov1api.DataUploadLabel], - }, du) - + du, err := findDataUploadByPod(r.client, *pod) if err != nil { r.logger.WithField("Backup pod", pod.Name).WithError(err).Error("unable to get dataupload") return []reconcile.Request{} + } else if du == nil { + r.logger.WithField("Backup pod", pod.Name).Error("get empty DataUpload") + return []reconcile.Request{} } if du.Status.Phase != velerov2alpha1api.DataUploadPhaseAccepted { @@ -430,6 +443,30 @@ func (r *DataUploadReconciler) findDataUploadForPod(podObj client.Object) []reco return []reconcile.Request{requests} } +func (r *DataUploadReconciler) FindDataUploads(ctx context.Context, cli client.Client, ns string) ([]velerov2alpha1api.DataUpload, error) { + pods := &corev1.PodList{} + var dataUploads []velerov2alpha1api.DataUpload + if err := cli.List(ctx, pods, &client.ListOptions{Namespace: ns}); err != nil { + r.logger.WithError(errors.WithStack(err)).Error("failed to list pods on current node") + return nil, errors.Wrapf(err, "failed to list pods on current node") + } + + for _, pod := range pods.Items { + if pod.Spec.NodeName != r.nodeName { + r.logger.Debugf("Pod %s related data upload will not handled by %s nodes", pod.GetName(), r.nodeName) + continue + } + du, err := findDataUploadByPod(cli, pod) + if err != nil { + r.logger.WithError(errors.WithStack(err)).Error("failed to get dataUpload by pod") + continue + } else if du != nil { + dataUploads = append(dataUploads, *du) + } + } + return dataUploads, nil +} + func (r *DataUploadReconciler) patchDataUpload(ctx context.Context, req *velerov2alpha1api.DataUpload, mutate func(*velerov2alpha1api.DataUpload)) error { original := req.DeepCopy() mutate(req) @@ -462,10 +499,10 @@ func (r *DataUploadReconciler) updateStatusToFailed(ctx context.Context, du *vel du.Status.Phase = velerov2alpha1api.DataUploadPhaseFailed du.Status.Message = errors.WithMessage(err, msg).Error() if du.Status.StartTimestamp.IsZero() { - du.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + du.Status.StartTimestamp = &metav1.Time{Time: r.Clock.Now()} } - du.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + du.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if patchErr := r.client.Patch(ctx, du, client.MergeFrom(original)); patchErr != nil { log.WithError(patchErr).Error("error updating DataUpload status") } @@ -535,3 +572,19 @@ func getOwnerObject(du *velerov2alpha1api.DataUpload) corev1.ObjectReference { APIVersion: du.APIVersion, } } + +func findDataUploadByPod(client client.Client, pod corev1.Pod) (*velerov2alpha1api.DataUpload, error) { + if label, exist := pod.Labels[velerov1api.DataUploadLabel]; exist { + du := &velerov2alpha1api.DataUpload{} + err := client.Get(context.Background(), types.NamespacedName{ + Namespace: pod.Namespace, + Name: label, + }, du) + + if err != nil { + return nil, errors.Wrapf(err, "error to find DataUpload by pod %s/%s", pod.Namespace, pod.Name) + } + return du, nil + } + return nil, nil +} diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go index 654e07531f..503c930f4f 100644 --- a/pkg/controller/data_upload_controller_test.go +++ b/pkg/controller/data_upload_controller_test.go @@ -334,7 +334,7 @@ func TestReconcile(t *testing.T) { name: "runCancelableDataUpload is concurrent limited", dataMgr: datapath.NewManager(0), pod: builder.ForPod(velerov1api.DefaultNamespace, dataUploadName).Volumes(&corev1.Volume{Name: "dataupload-1"}).Result(), - du: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhasePrepared).SnapshotType(fakeSnapshotType).Cancel(true).Result(), + du: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhasePrepared).SnapshotType(fakeSnapshotType).Result(), expectedProcessed: false, expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhasePrepared).Result(), expectedRequeue: ctrl.Result{Requeue: true, RequeueAfter: time.Minute}, @@ -369,7 +369,7 @@ func TestReconcile(t *testing.T) { } if test.du.Spec.SnapshotType == fakeSnapshotType { - r.snapshotExposerList = map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer{fakeSnapshotType: &fakeSnapshotExposer{r.client, r.clock}} + r.snapshotExposerList = map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer{fakeSnapshotType: &fakeSnapshotExposer{r.client, r.Clock}} } else if test.du.Spec.SnapshotType == velerov2alpha1api.SnapshotTypeCSI { r.snapshotExposerList = map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer{velerov2alpha1api.SnapshotTypeCSI: exposer.NewCSISnapshotExposer(r.kubeClient, r.csiSnapshotClient, velerotest.NewLogger())} } @@ -378,7 +378,7 @@ func TestReconcile(t *testing.T) { return &fakeDataUploadFSBR{ du: test.du, kubeClient: r.client, - clock: r.clock, + clock: r.Clock, } } @@ -569,6 +569,14 @@ func TestFindDataUploadForPod(t *testing.T) { assert.Equal(t, du.Namespace, requests[0].Namespace) assert.Equal(t, du.Name, requests[0].Name) }, + }, { + name: "no selected label found for pod", + du: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).Result(), + pod: builder.ForPod(velerov1api.DefaultNamespace, dataUploadName).Result(), + checkFunc: func(du *velerov2alpha1api.DataUpload, requests []reconcile.Request) { + // Assert that the function returns a single request + assert.Empty(t, requests) + }, }, { name: "no matched pod", du: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).Result(), From 9f5162ece39cbfdb30af6b586cb79ac3d5e8924b Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Fri, 7 Jul 2023 15:02:36 +0800 Subject: [PATCH 05/14] add wait timeout for expose prepare Signed-off-by: Lyndon-Li --- go.mod | 2 +- pkg/builder/data_download_builder.go | 6 + pkg/builder/data_upload_builder.go | 6 + pkg/cmd/cli/nodeagent/server.go | 18 +- pkg/controller/data_download_controller.go | 95 ++++++++--- .../data_download_controller_test.go | 129 +++++++++++++- pkg/controller/data_upload_controller.go | 109 +++++++++--- pkg/controller/data_upload_controller_test.go | 160 ++++++++++++++++-- 8 files changed, 462 insertions(+), 63 deletions(-) diff --git a/go.mod b/go.mod index 61885fe44c..3a2e3bfae9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/vmware-tanzu/velero -go 1.18 +go 1.20 require ( cloud.google.com/go/storage v1.30.1 diff --git a/pkg/builder/data_download_builder.go b/pkg/builder/data_download_builder.go index c564c80cff..9a85c79056 100644 --- a/pkg/builder/data_download_builder.go +++ b/pkg/builder/data_download_builder.go @@ -110,3 +110,9 @@ func (d *DataDownloadBuilder) ObjectMeta(opts ...ObjectMetaOpt) *DataDownloadBui return d } + +// StartTimestamp sets the DataDownload's StartTimestamp. +func (d *DataDownloadBuilder) StartTimestamp(startTime *metav1.Time) *DataDownloadBuilder { + d.object.Status.StartTimestamp = startTime + return d +} diff --git a/pkg/builder/data_upload_builder.go b/pkg/builder/data_upload_builder.go index a844ef6ef8..cb5d0b2de3 100644 --- a/pkg/builder/data_upload_builder.go +++ b/pkg/builder/data_upload_builder.go @@ -113,3 +113,9 @@ func (d *DataUploadBuilder) CSISnapshot(cSISnapshot *velerov2alpha1api.CSISnapsh d.object.Spec.CSISnapshot = cSISnapshot return d } + +// StartTimestamp sets the DataUpload's StartTimestamp. +func (d *DataUploadBuilder) StartTimestamp(startTime *metav1.Time) *DataUploadBuilder { + d.object.Status.StartTimestamp = startTime + return d +} diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index 3f635602ed..2762569453 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -71,20 +71,23 @@ const ( // files will be written to defaultCredentialsDirectory = "/tmp/credentials" - defaultResourceTimeout = 10 * time.Minute + defaultResourceTimeout = 10 * time.Minute + defaultDataMoverPrepareTimeout = 30 * time.Minute ) type nodeAgentServerConfig struct { - metricsAddress string - resourceTimeout time.Duration + metricsAddress string + resourceTimeout time.Duration + dataMoverPrepareTimeout time.Duration } func NewServerCommand(f client.Factory) *cobra.Command { logLevelFlag := logging.LogLevelFlag(logrus.InfoLevel) formatFlag := logging.NewFormatFlag() config := nodeAgentServerConfig{ - metricsAddress: defaultMetricsAddress, - resourceTimeout: defaultResourceTimeout, + metricsAddress: defaultMetricsAddress, + resourceTimeout: defaultResourceTimeout, + dataMoverPrepareTimeout: defaultDataMoverPrepareTimeout, } command := &cobra.Command{ @@ -110,6 +113,7 @@ func NewServerCommand(f client.Factory) *cobra.Command { command.Flags().Var(logLevelFlag, "log-level", fmt.Sprintf("The level at which to log. Valid values are %s.", strings.Join(logLevelFlag.AllowedValues(), ", "))) command.Flags().Var(formatFlag, "log-format", fmt.Sprintf("The format for log output. Valid values are %s.", strings.Join(formatFlag.AllowedValues(), ", "))) command.Flags().DurationVar(&config.resourceTimeout, "resource-timeout", config.resourceTimeout, "How long to wait for resource processes which are not covered by other specific timeout parameters. Default is 10 minutes.") + command.Flags().DurationVar(&config.dataMoverPrepareTimeout, "data-mover-prepare-timeout", config.dataMoverPrepareTimeout, "How long to wait for preparing a DataUpload/DataDownload. Default is 30 minutes.") return command } @@ -256,11 +260,11 @@ func (s *nodeAgentServer) run() { s.logger.WithError(err).Fatal("Unable to create the pod volume restore controller") } - if err = controller.NewDataUploadReconciler(s.mgr.GetClient(), s.kubeClient, s.csiSnapshotClient.SnapshotV1(), repoEnsurer, clock.RealClock{}, credentialGetter, s.nodeName, s.fileSystem, s.logger).SetupWithManager(s.mgr); err != nil { + if err = controller.NewDataUploadReconciler(s.mgr.GetClient(), s.kubeClient, s.csiSnapshotClient.SnapshotV1(), repoEnsurer, clock.RealClock{}, credentialGetter, s.nodeName, s.fileSystem, s.config.dataMoverPrepareTimeout, s.logger).SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data upload controller") } - if err = controller.NewDataDownloadReconciler(s.mgr.GetClient(), s.kubeClient, repoEnsurer, credentialGetter, s.nodeName, s.logger).SetupWithManager(s.mgr); err != nil { + if err = controller.NewDataDownloadReconciler(s.mgr.GetClient(), s.kubeClient, repoEnsurer, credentialGetter, s.nodeName, s.config.dataMoverPrepareTimeout, s.logger).SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data download controller") } diff --git a/pkg/controller/data_download_controller.go b/pkg/controller/data_download_controller.go index 51be1ab9be..d7e3e3e266 100644 --- a/pkg/controller/data_download_controller.go +++ b/pkg/controller/data_download_controller.go @@ -62,10 +62,11 @@ type DataDownloadReconciler struct { nodeName string repositoryEnsurer *repository.Ensurer dataPathMgr *datapath.Manager + preparingTimeout time.Duration } func NewDataDownloadReconciler(client client.Client, kubeClient kubernetes.Interface, - repoEnsurer *repository.Ensurer, credentialGetter *credentials.CredentialGetter, nodeName string, logger logrus.FieldLogger) *DataDownloadReconciler { + repoEnsurer *repository.Ensurer, credentialGetter *credentials.CredentialGetter, nodeName string, preparingTimeout time.Duration, logger logrus.FieldLogger) *DataDownloadReconciler { return &DataDownloadReconciler{ client: client, kubeClient: kubeClient, @@ -77,6 +78,7 @@ func NewDataDownloadReconciler(client client.Client, kubeClient kubernetes.Inter repositoryEnsurer: repoEnsurer, restoreExposer: exposer.NewGenericRestoreExposer(kubeClient, logger), dataPathMgr: datapath.NewManager(1), + preparingTimeout: preparingTimeout, } } @@ -143,6 +145,14 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request } log.Info("Restore is exposed") + return ctrl.Result{}, nil + } else if dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseAccepted { + if dd.Status.StartTimestamp != nil { + if time.Since(dd.Status.StartTimestamp.Time) >= r.preparingTimeout { + r.onPrepareTimeout(ctx, dd) + } + } + return ctrl.Result{}, nil } else if dd.Status.Phase == velerov2alpha1api.DataDownloadPhasePrepared { log.Info("Data download is prepared") @@ -184,7 +194,6 @@ func (r *DataDownloadReconciler) Reconcile(ctx context.Context, req ctrl.Request // Update status to InProgress original := dd.DeepCopy() dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseInProgress - dd.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} if err := r.client.Patch(ctx, dd, client.MergeFrom(original)); err != nil { log.WithError(err).Error("Unable to update status to in progress") return ctrl.Result{}, err @@ -345,8 +354,15 @@ func (r *DataDownloadReconciler) OnDataDownloadProgress(ctx context.Context, nam // re-enqueue the previous related request once the related pod is in running status to keep going on the rest logic. and below logic will avoid handling the unwanted // pod status and also avoid block others CR handling func (r *DataDownloadReconciler) SetupWithManager(mgr ctrl.Manager) error { + s := kube.NewPeriodicalEnqueueSource(r.logger, r.client, &velerov2alpha1api.DataDownloadList{}, preparingMonitorFrequency, kube.PeriodicalEnqueueSourceOption{}) + gp := kube.NewGenericEventPredicate(func(object client.Object) bool { + dd := object.(*velerov2alpha1api.DataDownload) + return (dd.Status.Phase == velerov2alpha1api.DataDownloadPhaseAccepted) + }) + return ctrl.NewControllerManagedBy(mgr). For(&velerov2alpha1api.DataDownload{}). + Watches(s, nil, builder.WithPredicates(gp)). Watches(&source.Kind{Type: &v1.Pod{}}, kube.EnqueueRequestsFromMapUpdateFunc(r.findSnapshotRestoreForPod), builder.WithPredicates(predicate.Funcs{ UpdateFunc: func(ue event.UpdateEvent) bool { @@ -400,9 +416,15 @@ func (r *DataDownloadReconciler) findSnapshotRestoreForPod(podObj client.Object) requests := make([]reconcile.Request, 1) r.logger.WithField("Restore pod", pod.Name).Infof("Preparing data download %s", dd.Name) - err = r.patchDataDownload(context.Background(), dd, r.prepareDataDownload) - if err != nil { - r.logger.WithField("Restore pod", pod.Name).WithError(err).Error("unable to patch data download") + + // we don't expect anyone else update the CR during the Prepare process + updated, err := r.exclusiveUpdateDataDownload(context.Background(), dd, r.prepareDataDownload) + if err != nil || !updated { + r.logger.WithFields(logrus.Fields{ + "Datadownload": dd.Name, + "Restore pod": pod.Name, + "updated": updated, + }).WithError(err).Warn("failed to patch datadownload, prepare will halt for this datadownload") return []reconcile.Request{} } @@ -416,16 +438,6 @@ func (r *DataDownloadReconciler) findSnapshotRestoreForPod(podObj client.Object) return requests } -func (r *DataDownloadReconciler) patchDataDownload(ctx context.Context, req *velerov2alpha1api.DataDownload, mutate func(*velerov2alpha1api.DataDownload)) error { - original := req.DeepCopy() - mutate(req) - if err := r.client.Patch(ctx, req, client.MergeFrom(original)); err != nil { - return errors.Wrap(err, "error patching data download") - } - - return nil -} - func (r *DataDownloadReconciler) prepareDataDownload(ssb *velerov2alpha1api.DataDownload) { ssb.Status.Phase = velerov2alpha1api.DataDownloadPhasePrepared ssb.Status.Node = r.nodeName @@ -453,17 +465,62 @@ func (r *DataDownloadReconciler) updateStatusToFailed(ctx context.Context, dd *v } func (r *DataDownloadReconciler) acceptDataDownload(ctx context.Context, dd *velerov2alpha1api.DataDownload) (bool, error) { - updated := dd.DeepCopy() - updated.Status.Phase = velerov2alpha1api.DataDownloadPhaseAccepted + r.logger.Infof("Accepting data download %s", dd.Name) - r.logger.Infof("Accepting snapshot restore %s", dd.Name) // For all data download controller in each node-agent will try to update download CR, and only one controller will success, // and the success one could handle later logic + succeeded, err := r.exclusiveUpdateDataDownload(ctx, dd, func(dd *velerov2alpha1api.DataDownload) { + dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseAccepted + dd.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + }) + + if err != nil { + return false, err + } + + if succeeded { + r.logger.WithField("DataDownload", dd.Name).Infof("This datadownload has been accepted by %s", r.nodeName) + return true, nil + } + + r.logger.WithField("DataDownload", dd.Name).Info("This datadownload has been accepted by others") + return false, nil +} + +func (r *DataDownloadReconciler) onPrepareTimeout(ctx context.Context, dd *velerov2alpha1api.DataDownload) { + log := r.logger.WithField("DataDownload", dd.Name) + + log.Info("Timeout happened for preparing datadownload") + + succeeded, err := r.exclusiveUpdateDataDownload(ctx, dd, func(dd *velerov2alpha1api.DataDownload) { + dd.Status.Phase = velerov2alpha1api.DataDownloadPhaseFailed + dd.Status.Message = "timeout on preparing data download" + }) + + if err != nil { + log.WithError(err).Warn("Failed to update datadownload") + return + } + + if !succeeded { + log.Warn("Dataupload has been updated by others") + return + } + + r.restoreExposer.CleanUp(ctx, getDataDownloadOwnerObject(dd)) + + log.Info("Dataupload has been cleaned up") +} + +func (r *DataDownloadReconciler) exclusiveUpdateDataDownload(ctx context.Context, dd *velerov2alpha1api.DataDownload, + updateFunc func(*velerov2alpha1api.DataDownload)) (bool, error) { + updated := dd.DeepCopy() + updateFunc(updated) + err := r.client.Update(ctx, updated) if err == nil { return true, nil } else if apierrors.IsConflict(err) { - r.logger.WithField("DataDownload", dd.Name).Error("This data download restore has been accepted by others") return false, nil } else { return false, err diff --git a/pkg/controller/data_download_controller_test.go b/pkg/controller/data_download_controller_test.go index 773112207a..8447a9eb36 100644 --- a/pkg/controller/data_download_controller_test.go +++ b/pkg/controller/data_download_controller_test.go @@ -29,6 +29,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" clientgofake "k8s.io/client-go/kubernetes/fake" @@ -65,6 +66,29 @@ func dataDownloadBuilder() *builder.DataDownloadBuilder { } func initDataDownloadReconciler(objects []runtime.Object, needError ...bool) (*DataDownloadReconciler, error) { + var errs []error = make([]error, 4) + if len(needError) == 4 { + if needError[0] { + errs[0] = fmt.Errorf("Get error") + } + + if needError[1] { + errs[1] = fmt.Errorf("Create error") + } + + if needError[2] { + errs[2] = fmt.Errorf("Update error") + } + + if needError[3] { + errs[3] = fmt.Errorf("Patch error") + } + } + + return initDataDownloadReconcilerWithError(objects, errs...) +} + +func initDataDownloadReconcilerWithError(objects []runtime.Object, needError ...error) (*DataDownloadReconciler, error) { scheme := runtime.NewScheme() err := velerov1api.AddToScheme(scheme) if err != nil { @@ -112,7 +136,7 @@ func initDataDownloadReconciler(objects []runtime.Object, needError ...bool) (*D if err != nil { return nil, err } - return NewDataDownloadReconciler(fakeClient, fakeKubeClient, nil, &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", velerotest.NewLogger()), nil + return NewDataDownloadReconciler(fakeClient, fakeKubeClient, nil, &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", time.Minute*5, velerotest.NewLogger()), nil } func TestDataDownloadReconcile(t *testing.T) { @@ -132,6 +156,7 @@ func TestDataDownloadReconcile(t *testing.T) { notMockCleanUp bool mockCancel bool mockClose bool + expected *velerov2alpha1api.DataDownload expectedStatusMsg string expectedResult *ctrl.Result }{ @@ -215,7 +240,7 @@ func TestDataDownloadReconcile(t *testing.T) { dd: builder.ForDataDownload("test-ns", dataDownloadName).Result(), targetPVC: builder.ForPersistentVolumeClaim("test-ns", "test-pvc").Result(), needErrs: []bool{true, false, false, false}, - expectedStatusMsg: "Create error", + expectedStatusMsg: "Get error", }, { name: "Unsupported dataDownload type", @@ -246,6 +271,11 @@ func TestDataDownloadReconcile(t *testing.T) { expectedStatusMsg: "Error to expose restore exposer", isExposeErr: true, }, + { + name: "prepare timeout", + dd: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseAccepted).StartTimestamp(&metav1.Time{Time: time.Now().Add(-time.Minute * 5)}).Result(), + expected: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseFailed).Result(), + }, } for _, test := range tests { @@ -345,6 +375,11 @@ func TestDataDownloadReconcile(t *testing.T) { Namespace: test.dd.Namespace, }, &dd) + if test.expected != nil { + require.NoError(t, err) + assert.Equal(t, dd.Status.Phase, test.expected.Status.Phase) + } + if test.isGetExposeErr { assert.Contains(t, dd.Status.Message, test.expectedStatusMsg) } @@ -580,3 +615,93 @@ func TestFindDataDownloadForPod(t *testing.T) { } } } + +func TestAcceptDataDownload(t *testing.T) { + tests := []struct { + name string + dd *velerov2alpha1api.DataDownload + needErrs []error + succeeded bool + expectedErr string + }{ + { + name: "update fail", + dd: dataDownloadBuilder().Result(), + needErrs: []error{nil, nil, fmt.Errorf("fake-update-error"), nil}, + expectedErr: "fake-update-error", + }, + { + name: "accepted by others", + dd: dataDownloadBuilder().Result(), + needErrs: []error{nil, nil, &fakeAPIStatus{metav1.StatusReasonConflict}, nil}, + }, + { + name: "succeed", + dd: dataDownloadBuilder().Result(), + needErrs: []error{nil, nil, nil, nil}, + succeeded: true, + }, + } + for _, test := range tests { + ctx := context.Background() + r, err := initDataDownloadReconcilerWithError(nil, test.needErrs...) + require.NoError(t, err) + + err = r.client.Create(ctx, test.dd) + require.NoError(t, err) + + succeeded, err := r.acceptDataDownload(ctx, test.dd) + assert.Equal(t, test.succeeded, succeeded) + if test.expectedErr == "" { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, test.expectedErr) + } + } +} + +func TestOnDdPrepareTimeout(t *testing.T) { + tests := []struct { + name string + dd *velerov2alpha1api.DataDownload + needErrs []error + expected *velerov2alpha1api.DataDownload + }{ + { + name: "update fail", + dd: dataDownloadBuilder().Result(), + needErrs: []error{nil, nil, fmt.Errorf("fake-update-error"), nil}, + expected: dataDownloadBuilder().Result(), + }, + { + name: "update interrupted", + dd: dataDownloadBuilder().Result(), + needErrs: []error{nil, nil, &fakeAPIStatus{metav1.StatusReasonConflict}, nil}, + expected: dataDownloadBuilder().Result(), + }, + { + name: "succeed", + dd: dataDownloadBuilder().Result(), + needErrs: []error{nil, nil, nil, nil}, + expected: dataDownloadBuilder().Phase(velerov2alpha1api.DataDownloadPhaseFailed).Result(), + }, + } + for _, test := range tests { + ctx := context.Background() + r, err := initDataDownloadReconcilerWithError(nil, test.needErrs...) + require.NoError(t, err) + + err = r.client.Create(ctx, test.dd) + require.NoError(t, err) + + r.onPrepareTimeout(ctx, test.dd) + + dd := velerov2alpha1api.DataDownload{} + _ = r.client.Get(ctx, kbclient.ObjectKey{ + Name: test.dd.Name, + Namespace: test.dd.Namespace, + }, &dd) + + assert.Equal(t, test.expected.Status.Phase, dd.Status.Phase) + } +} diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index 101c083704..49ca428e1a 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -55,6 +55,8 @@ import ( const dataMoverType string = "velero" const dataUploadDownloadRequestor string = "snapshot-data-upload-download" +const preparingMonitorFrequency time.Duration = time.Minute + // DataUploadReconciler reconciles a DataUpload object type DataUploadReconciler struct { client client.Client @@ -68,11 +70,12 @@ type DataUploadReconciler struct { logger logrus.FieldLogger snapshotExposerList map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer dataPathMgr *datapath.Manager + preparingTimeout time.Duration } func NewDataUploadReconciler(client client.Client, kubeClient kubernetes.Interface, csiSnapshotClient snapshotter.SnapshotV1Interface, repoEnsurer *repository.Ensurer, clock clocks.WithTickerAndDelayedExecution, - cred *credentials.CredentialGetter, nodeName string, fs filesystem.Interface, log logrus.FieldLogger) *DataUploadReconciler { + cred *credentials.CredentialGetter, nodeName string, fs filesystem.Interface, preparingTimeout time.Duration, log logrus.FieldLogger) *DataUploadReconciler { return &DataUploadReconciler{ client: client, kubeClient: kubeClient, @@ -85,6 +88,7 @@ func NewDataUploadReconciler(client client.Client, kubeClient kubernetes.Interfa repoEnsurer: repoEnsurer, snapshotExposerList: map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer{velerov2alpha1api.SnapshotTypeCSI: exposer.NewCSISnapshotExposer(kubeClient, csiSnapshotClient, log)}, dataPathMgr: datapath.NewManager(1), + preparingTimeout: preparingTimeout, } } @@ -143,6 +147,14 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) // ep.Expose() will trigger to create one pod whose volume is restored by a given volume snapshot, // but the pod maybe is not in the same node of the current controller, so we need to return it here. // And then only the controller who is in the same node could do the rest work. + return ctrl.Result{}, nil + } else if du.Status.Phase == velerov2alpha1api.DataUploadPhaseAccepted { + if du.Status.StartTimestamp != nil { + if time.Since(du.Status.StartTimestamp.Time) >= r.preparingTimeout { + r.onPrepareTimeout(ctx, &du) + } + } + return ctrl.Result{}, nil } else if du.Status.Phase == velerov2alpha1api.DataUploadPhasePrepared { log.Info("Data upload is prepared") @@ -183,7 +195,6 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Update status to InProgress original := du.DeepCopy() du.Status.Phase = velerov2alpha1api.DataUploadPhaseInProgress - du.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} if err := r.client.Patch(ctx, &du, client.MergeFrom(original)); err != nil { return r.errorOut(ctx, &du, err, "error updating dataupload status", log) } @@ -363,8 +374,15 @@ func (r *DataUploadReconciler) OnDataUploadProgress(ctx context.Context, namespa // re-enqueue the previous related request once the related pod is in running status to keep going on the rest logic. and below logic will avoid handling the unwanted // pod status and also avoid block others CR handling func (r *DataUploadReconciler) SetupWithManager(mgr ctrl.Manager) error { + s := kube.NewPeriodicalEnqueueSource(r.logger, r.client, &velerov2alpha1api.DataUploadList{}, preparingMonitorFrequency, kube.PeriodicalEnqueueSourceOption{}) + gp := kube.NewGenericEventPredicate(func(object client.Object) bool { + du := object.(*velerov2alpha1api.DataUpload) + return (du.Status.Phase == velerov2alpha1api.DataUploadPhaseAccepted) + }) + return ctrl.NewControllerManagedBy(mgr). For(&velerov2alpha1api.DataUpload{}). + Watches(s, nil, builder.WithPredicates(gp)). Watches(&source.Kind{Type: &corev1.Pod{}}, kube.EnqueueRequestsFromMapUpdateFunc(r.findDataUploadForPod), builder.WithPredicates(predicate.Funcs{ UpdateFunc: func(ue event.UpdateEvent) bool { @@ -416,8 +434,15 @@ func (r *DataUploadReconciler) findDataUploadForPod(podObj client.Object) []reco } r.logger.WithField("Backup pod", pod.Name).Infof("Preparing dataupload %s", du.Name) - if err := r.patchDataUpload(context.Background(), du, r.prepareDataUpload); err != nil { - r.logger.WithField("Backup pod", pod.Name).WithError(err).Error("failed to patch dataupload") + + // we don't expect anyone else update the CR during the Prepare process + updated, err := r.exclusiveUpdateDataUpload(context.Background(), du, r.prepareDataUpload) + if err != nil || !updated { + r.logger.WithFields(logrus.Fields{ + "Dataupload": du.Name, + "Backup pod": pod.Name, + "updated": updated, + }).WithError(err).Warn("failed to patch dataupload, prepare will halt for this dataupload") return []reconcile.Request{} } @@ -430,16 +455,6 @@ func (r *DataUploadReconciler) findDataUploadForPod(podObj client.Object) []reco return []reconcile.Request{requests} } -func (r *DataUploadReconciler) patchDataUpload(ctx context.Context, req *velerov2alpha1api.DataUpload, mutate func(*velerov2alpha1api.DataUpload)) error { - original := req.DeepCopy() - mutate(req) - if err := r.client.Patch(ctx, req, client.MergeFrom(original)); err != nil { - return errors.Wrap(err, "error patching DataUpload") - } - - return nil -} - func (r *DataUploadReconciler) prepareDataUpload(du *velerov2alpha1api.DataUpload) { du.Status.Phase = velerov2alpha1api.DataUploadPhasePrepared du.Status.Node = r.nodeName @@ -475,19 +490,73 @@ func (r *DataUploadReconciler) updateStatusToFailed(ctx context.Context, du *vel } func (r *DataUploadReconciler) acceptDataUpload(ctx context.Context, du *velerov2alpha1api.DataUpload) (bool, error) { - updated := du.DeepCopy() - updated.Status.Phase = velerov2alpha1api.DataUploadPhaseAccepted - - r.logger.Infof("Accepting snapshot backup %s", du.Name) + r.logger.Infof("Accepting data upload %s", du.Name) // For all data upload controller in each node-agent will try to update dataupload CR, and only one controller will success, // and the success one could handle later logic + succeeded, err := r.exclusiveUpdateDataUpload(ctx, du, func(du *velerov2alpha1api.DataUpload) { + du.Status.Phase = velerov2alpha1api.DataUploadPhaseAccepted + du.Status.StartTimestamp = &metav1.Time{Time: r.clock.Now()} + }) + + if err != nil { + return false, err + } + + if succeeded { + r.logger.WithField("Dataupload", du.Name).Infof("This datauplod has been accepted by %s", r.nodeName) + return true, nil + } + + r.logger.WithField("Dataupload", du.Name).Info("This datauplod has been accepted by others") + return false, nil +} + +func (r *DataUploadReconciler) onPrepareTimeout(ctx context.Context, du *velerov2alpha1api.DataUpload) { + log := r.logger.WithField("Dataupload", du.Name) + + log.Info("Timeout happened for preparing dataupload") + + succeeded, err := r.exclusiveUpdateDataUpload(ctx, du, func(du *velerov2alpha1api.DataUpload) { + du.Status.Phase = velerov2alpha1api.DataUploadPhaseFailed + du.Status.Message = "timeout on preparing data upload" + }) + + if err != nil { + log.WithError(err).Warn("Failed to update dataupload") + return + } + + if !succeeded { + log.Warn("Dataupload has been updated by others") + return + } + + ep, ok := r.snapshotExposerList[du.Spec.SnapshotType] + if !ok { + log.WithError(fmt.Errorf("%v type of snapshot exposer is not exist", du.Spec.SnapshotType)). + Warn("Failed to clean up resources on canceled") + } else { + var volumeSnapshotName string + if du.Spec.SnapshotType == velerov2alpha1api.SnapshotTypeCSI { // Other exposer should have another condition + volumeSnapshotName = du.Spec.CSISnapshot.VolumeSnapshot + } + + ep.CleanUp(ctx, getOwnerObject(du), volumeSnapshotName, du.Spec.SourceNamespace) + + log.Info("Dataupload has been cleaned up") + } +} + +func (r *DataUploadReconciler) exclusiveUpdateDataUpload(ctx context.Context, du *velerov2alpha1api.DataUpload, + updateFunc func(*velerov2alpha1api.DataUpload)) (bool, error) { + updated := du.DeepCopy() + updateFunc(updated) + err := r.client.Update(ctx, updated) if err == nil { - r.logger.WithField("Dataupload", du.Name).Infof("This datauplod backup has been accepted by %s", r.nodeName) return true, nil } else if apierrors.IsConflict(err) { - r.logger.WithField("Dataupload", du.Name).Info("This datauplod backup has been accepted by others") return false, nil } else { return false, err diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go index 654e07531f..e7a3b476f9 100644 --- a/pkg/controller/data_upload_controller_test.go +++ b/pkg/controller/data_upload_controller_test.go @@ -58,45 +58,68 @@ const fakeSnapshotType velerov2alpha1api.SnapshotType = "fake-snapshot" type FakeClient struct { kbclient.Client - getError bool - createError bool - updateError bool - patchError bool + getError error + createError error + updateError error + patchError error } func (c *FakeClient) Get(ctx context.Context, key kbclient.ObjectKey, obj kbclient.Object) error { - if c.getError { - return fmt.Errorf("Create error") + if c.getError != nil { + return c.getError } return c.Client.Get(ctx, key, obj) } func (c *FakeClient) Create(ctx context.Context, obj kbclient.Object, opts ...kbclient.CreateOption) error { - if c.createError { - return fmt.Errorf("Create error") + if c.createError != nil { + return c.createError } return c.Client.Create(ctx, obj, opts...) } func (c *FakeClient) Update(ctx context.Context, obj kbclient.Object, opts ...kbclient.UpdateOption) error { - if c.updateError { - return fmt.Errorf("Update error") + if c.updateError != nil { + return c.updateError } return c.Client.Update(ctx, obj, opts...) } func (c *FakeClient) Patch(ctx context.Context, obj kbclient.Object, patch kbclient.Patch, opts ...kbclient.PatchOption) error { - if c.patchError { - return fmt.Errorf("Patch error") + if c.patchError != nil { + return c.patchError } return c.Client.Patch(ctx, obj, patch, opts...) } func initDataUploaderReconciler(needError ...bool) (*DataUploadReconciler, error) { + var errs []error = make([]error, 4) + if len(needError) == 4 { + if needError[0] { + errs[0] = fmt.Errorf("Get error") + } + + if needError[1] { + errs[1] = fmt.Errorf("Create error") + } + + if needError[2] { + errs[2] = fmt.Errorf("Update error") + } + + if needError[3] { + errs[3] = fmt.Errorf("Patch error") + } + } + + return initDataUploaderReconcilerWithError(errs...) +} + +func initDataUploaderReconcilerWithError(needError ...error) (*DataUploadReconciler, error) { vscName := "fake-vsc" vsObject := &snapshotv1api.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ @@ -170,7 +193,7 @@ func initDataUploaderReconciler(needError ...bool) (*DataUploadReconciler, error return nil, err } return NewDataUploadReconciler(fakeClient, fakeKubeClient, fakeSnapshotClient.SnapshotV1(), nil, - testclocks.NewFakeClock(now), &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", fakeFS, velerotest.NewLogger()), nil + testclocks.NewFakeClock(now), &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", fakeFS, time.Minute*5, velerotest.NewLogger()), nil } func dataUploadBuilder() *builder.DataUploadBuilder { @@ -277,7 +300,7 @@ func TestReconcile(t *testing.T) { expectedProcessed: false, expected: nil, expectedRequeue: ctrl.Result{}, - expectedErrMsg: "getting DataUpload: Create error", + expectedErrMsg: "getting DataUpload: Get error", needErrs: []bool{true, false, false, false}, }, { name: "Unsupported data mover type", @@ -339,6 +362,11 @@ func TestReconcile(t *testing.T) { expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhasePrepared).Result(), expectedRequeue: ctrl.Result{Requeue: true, RequeueAfter: time.Minute}, }, + { + name: "prepare timeout", + du: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).SnapshotType(fakeSnapshotType).StartTimestamp(&metav1.Time{Time: time.Now().Add(-time.Minute * 5)}).Result(), + expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseFailed).Result(), + }, } for _, test := range tests { @@ -599,3 +627,107 @@ func TestFindDataUploadForPod(t *testing.T) { } } } + +type fakeAPIStatus struct { + reason metav1.StatusReason +} + +func (f *fakeAPIStatus) Status() metav1.Status { + return metav1.Status{ + Reason: f.reason, + } +} + +func (f *fakeAPIStatus) Error() string { + return string(f.reason) +} + +func TestAcceptDataUpload(t *testing.T) { + tests := []struct { + name string + du *velerov2alpha1api.DataUpload + needErrs []error + succeeded bool + expectedErr string + }{ + { + name: "update fail", + du: dataUploadBuilder().Result(), + needErrs: []error{nil, nil, fmt.Errorf("fake-update-error"), nil}, + expectedErr: "fake-update-error", + }, + { + name: "accepted by others", + du: dataUploadBuilder().Result(), + needErrs: []error{nil, nil, &fakeAPIStatus{metav1.StatusReasonConflict}, nil}, + }, + { + name: "succeed", + du: dataUploadBuilder().Result(), + needErrs: []error{nil, nil, nil, nil}, + succeeded: true, + }, + } + for _, test := range tests { + ctx := context.Background() + r, err := initDataUploaderReconcilerWithError(test.needErrs...) + require.NoError(t, err) + + err = r.client.Create(ctx, test.du) + require.NoError(t, err) + + succeeded, err := r.acceptDataUpload(ctx, test.du) + assert.Equal(t, test.succeeded, succeeded) + if test.expectedErr == "" { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, test.expectedErr) + } + } +} + +func TestOnDuPrepareTimeout(t *testing.T) { + tests := []struct { + name string + du *velerov2alpha1api.DataUpload + needErrs []error + expected *velerov2alpha1api.DataUpload + }{ + { + name: "update fail", + du: dataUploadBuilder().Result(), + needErrs: []error{nil, nil, fmt.Errorf("fake-update-error"), nil}, + expected: dataUploadBuilder().Result(), + }, + { + name: "update interrupted", + du: dataUploadBuilder().Result(), + needErrs: []error{nil, nil, &fakeAPIStatus{metav1.StatusReasonConflict}, nil}, + expected: dataUploadBuilder().Result(), + }, + { + name: "succeed", + du: dataUploadBuilder().Result(), + needErrs: []error{nil, nil, nil, nil}, + expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseFailed).Result(), + }, + } + for _, test := range tests { + ctx := context.Background() + r, err := initDataUploaderReconcilerWithError(test.needErrs...) + require.NoError(t, err) + + err = r.client.Create(ctx, test.du) + require.NoError(t, err) + + r.onPrepareTimeout(ctx, test.du) + + du := velerov2alpha1api.DataUpload{} + _ = r.client.Get(ctx, kbclient.ObjectKey{ + Name: test.du.Name, + Namespace: test.du.Namespace, + }, &du) + + assert.Equal(t, test.expected.Status.Phase, du.Status.Phase) + } +} From 05722876b90c8a49a9e264aaa6d2f956188cab9a Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 11 Jul 2023 17:45:39 -0400 Subject: [PATCH 06/14] Moving kopia logging to remove kopia from indirect dependency in velero plugins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit when running `go mod why -m github.com/kopia/kopia` in velero-plugins prior to this change you will see following ``` ❯ go mod why -m github.com/kopia/kopia github.com/konveyor/openshift-velero-plugin/velero-plugins github.com/vmware-tanzu/velero/pkg/plugin/framework github.com/vmware-tanzu/velero/pkg/util/logging github.com/kopia/kopia/repo/logging ``` after ``` ❯ go mod why -m github.com/kopia/kopia (main module does not need module github.com/kopia/kopia) ``` Signed-off-by: Tiger Kaovilai --- changelogs/unreleased/6484-kaovilai | 1 + pkg/{util/logging => kopia}/kopia_log.go | 4 +-- pkg/{util/logging => kopia}/kopia_log_test.go | 2 +- pkg/repository/udmrepo/kopialib/lib_repo.go | 28 +++++++++---------- pkg/uploader/kopia/snapshot.go | 6 ++-- 5 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/6484-kaovilai rename pkg/{util/logging => kopia}/kopia_log.go (99%) rename pkg/{util/logging => kopia}/kopia_log_test.go (99%) diff --git a/changelogs/unreleased/6484-kaovilai b/changelogs/unreleased/6484-kaovilai new file mode 100644 index 0000000000..764f43c6f4 --- /dev/null +++ b/changelogs/unreleased/6484-kaovilai @@ -0,0 +1 @@ +Velero Plugins no longer need kopia indirect dependency in their go.mod \ No newline at end of file diff --git a/pkg/util/logging/kopia_log.go b/pkg/kopia/kopia_log.go similarity index 99% rename from pkg/util/logging/kopia_log.go rename to pkg/kopia/kopia_log.go index 82dffc056d..ec08c81c0a 100644 --- a/pkg/util/logging/kopia_log.go +++ b/pkg/kopia/kopia_log.go @@ -1,3 +1,5 @@ +package kopia + /* Copyright the Velero contributors. @@ -14,8 +16,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -package logging - import ( "context" diff --git a/pkg/util/logging/kopia_log_test.go b/pkg/kopia/kopia_log_test.go similarity index 99% rename from pkg/util/logging/kopia_log_test.go rename to pkg/kopia/kopia_log_test.go index 848638c6d0..d50a4bba36 100644 --- a/pkg/util/logging/kopia_log_test.go +++ b/pkg/kopia/kopia_log_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package logging +package kopia import ( "testing" diff --git a/pkg/repository/udmrepo/kopialib/lib_repo.go b/pkg/repository/udmrepo/kopialib/lib_repo.go index 7802555fc7..a0a99d2b45 100644 --- a/pkg/repository/udmrepo/kopialib/lib_repo.go +++ b/pkg/repository/udmrepo/kopialib/lib_repo.go @@ -33,8 +33,8 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" + "github.com/vmware-tanzu/velero/pkg/kopia" "github.com/vmware-tanzu/velero/pkg/repository/udmrepo" - "github.com/vmware-tanzu/velero/pkg/util/logging" ) type kopiaRepoService struct { @@ -91,7 +91,7 @@ func NewKopiaRepoService(logger logrus.FieldLogger) udmrepo.BackupRepoService { } func (ks *kopiaRepoService) Init(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error { - repoCtx := logging.SetupKopiaLog(ctx, ks.logger) + repoCtx := kopia.SetupKopiaLog(ctx, ks.logger) if createNew { if err := CreateBackupRepo(repoCtx, repoOption); err != nil { @@ -113,7 +113,7 @@ func (ks *kopiaRepoService) Open(ctx context.Context, repoOption udmrepo.RepoOpt return nil, errors.Wrapf(err, "repo config %s doesn't exist", repoConfig) } - repoCtx := logging.SetupKopiaLog(ctx, ks.logger) + repoCtx := kopia.SetupKopiaLog(ctx, ks.logger) r, err := openKopiaRepo(repoCtx, repoConfig, repoOption.RepoPassword) if err != nil { @@ -156,7 +156,7 @@ func (ks *kopiaRepoService) Maintain(ctx context.Context, repoOption udmrepo.Rep return errors.Wrapf(err, "repo config %s doesn't exist", repoConfig) } - repoCtx := logging.SetupKopiaLog(ctx, ks.logger) + repoCtx := kopia.SetupKopiaLog(ctx, ks.logger) r, err := openKopiaRepo(repoCtx, repoConfig, repoOption.RepoPassword) if err != nil { @@ -206,7 +206,7 @@ func (ks *kopiaRepoService) DefaultMaintenanceFrequency() time.Duration { } func (km *kopiaMaintenance) runMaintenance(ctx context.Context, rep repo.DirectRepositoryWriter) error { - err := snapshotmaintenance.Run(logging.SetupKopiaLog(ctx, km.logger), rep, km.mode, false, maintenance.SafetyFull) + err := snapshotmaintenance.Run(kopia.SetupKopiaLog(ctx, km.logger), rep, km.mode, false, maintenance.SafetyFull) if err != nil { return errors.Wrapf(err, "error to run maintenance under mode %s", km.mode) } @@ -238,7 +238,7 @@ func (kr *kopiaRepository) OpenObject(ctx context.Context, id udmrepo.ID) (udmre return nil, errors.Wrapf(err, "error to parse object ID from %v", id) } - reader, err := kr.rawRepo.OpenObject(logging.SetupKopiaLog(ctx, kr.logger), objID) + reader, err := kr.rawRepo.OpenObject(kopia.SetupKopiaLog(ctx, kr.logger), objID) if err != nil { return nil, errors.Wrap(err, "error to open object") } @@ -253,7 +253,7 @@ func (kr *kopiaRepository) GetManifest(ctx context.Context, id udmrepo.ID, mani return errors.New("repo is closed or not open") } - metadata, err := kr.rawRepo.GetManifest(logging.SetupKopiaLog(ctx, kr.logger), manifest.ID(id), mani.Payload) + metadata, err := kr.rawRepo.GetManifest(kopia.SetupKopiaLog(ctx, kr.logger), manifest.ID(id), mani.Payload) if err != nil { return errors.Wrap(err, "error to get manifest") } @@ -268,7 +268,7 @@ func (kr *kopiaRepository) FindManifests(ctx context.Context, filter udmrepo.Man return nil, errors.New("repo is closed or not open") } - metadata, err := kr.rawRepo.FindManifests(logging.SetupKopiaLog(ctx, kr.logger), filter.Labels) + metadata, err := kr.rawRepo.FindManifests(kopia.SetupKopiaLog(ctx, kr.logger), filter.Labels) if err != nil { return nil, errors.Wrap(err, "error to find manifests") } @@ -286,7 +286,7 @@ func (kr *kopiaRepository) Time() time.Time { func (kr *kopiaRepository) Close(ctx context.Context) error { if kr.rawWriter != nil { - err := kr.rawWriter.Close(logging.SetupKopiaLog(ctx, kr.logger)) + err := kr.rawWriter.Close(kopia.SetupKopiaLog(ctx, kr.logger)) if err != nil { return errors.Wrap(err, "error to close repo writer") } @@ -295,7 +295,7 @@ func (kr *kopiaRepository) Close(ctx context.Context) error { } if kr.rawRepo != nil { - err := kr.rawRepo.Close(logging.SetupKopiaLog(ctx, kr.logger)) + err := kr.rawRepo.Close(kopia.SetupKopiaLog(ctx, kr.logger)) if err != nil { return errors.Wrap(err, "error to close repo") } @@ -311,7 +311,7 @@ func (kr *kopiaRepository) NewObjectWriter(ctx context.Context, opt udmrepo.Obje return nil } - writer := kr.rawWriter.NewObjectWriter(logging.SetupKopiaLog(ctx, kr.logger), object.WriterOptions{ + writer := kr.rawWriter.NewObjectWriter(kopia.SetupKopiaLog(ctx, kr.logger), object.WriterOptions{ Description: opt.Description, Prefix: index.IDPrefix(opt.Prefix), AsyncWrites: opt.AsyncWrites, @@ -332,7 +332,7 @@ func (kr *kopiaRepository) PutManifest(ctx context.Context, manifest udmrepo.Rep return "", errors.New("repo writer is closed or not open") } - id, err := kr.rawWriter.PutManifest(logging.SetupKopiaLog(ctx, kr.logger), manifest.Metadata.Labels, manifest.Payload) + id, err := kr.rawWriter.PutManifest(kopia.SetupKopiaLog(ctx, kr.logger), manifest.Metadata.Labels, manifest.Payload) if err != nil { return "", errors.Wrap(err, "error to put manifest") } @@ -345,7 +345,7 @@ func (kr *kopiaRepository) DeleteManifest(ctx context.Context, id udmrepo.ID) er return errors.New("repo writer is closed or not open") } - err := kr.rawWriter.DeleteManifest(logging.SetupKopiaLog(ctx, kr.logger), manifest.ID(id)) + err := kr.rawWriter.DeleteManifest(kopia.SetupKopiaLog(ctx, kr.logger), manifest.ID(id)) if err != nil { return errors.Wrap(err, "error to delete manifest") } @@ -358,7 +358,7 @@ func (kr *kopiaRepository) Flush(ctx context.Context) error { return errors.New("repo writer is closed or not open") } - err := kr.rawWriter.Flush(logging.SetupKopiaLog(ctx, kr.logger)) + err := kr.rawWriter.Flush(kopia.SetupKopiaLog(ctx, kr.logger)) if err != nil { return errors.Wrap(err, "error to flush repo") } diff --git a/pkg/uploader/kopia/snapshot.go b/pkg/uploader/kopia/snapshot.go index 033f304210..85000ad00b 100644 --- a/pkg/uploader/kopia/snapshot.go +++ b/pkg/uploader/kopia/snapshot.go @@ -28,9 +28,9 @@ import ( "github.com/sirupsen/logrus" + "github.com/vmware-tanzu/velero/pkg/kopia" "github.com/vmware-tanzu/velero/pkg/repository/udmrepo" "github.com/vmware-tanzu/velero/pkg/uploader" - "github.com/vmware-tanzu/velero/pkg/util/logging" "github.com/kopia/kopia/fs" "github.com/kopia/kopia/fs/localfs" @@ -121,7 +121,7 @@ func Backup(ctx context.Context, fsUploader SnapshotUploader, repoWriter repo.Re return nil, false, errors.Wrap(err, "Unable to get local filesystem entry") } - kopiaCtx := logging.SetupKopiaLog(ctx, log) + kopiaCtx := kopia.SetupKopiaLog(ctx, log) snapID, snapshotSize, err := SnapshotSource(kopiaCtx, repoWriter, fsUploader, sourceInfo, rootDir, forceFull, parentSnapshot, tags, log, "Kopia Uploader") if err != nil { return nil, false, err @@ -306,7 +306,7 @@ func findPreviousSnapshotManifest(ctx context.Context, rep repo.Repository, sour func Restore(ctx context.Context, rep repo.RepositoryWriter, progress *Progress, snapshotID, dest string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { log.Info("Start to restore...") - kopiaCtx := logging.SetupKopiaLog(ctx, log) + kopiaCtx := kopia.SetupKopiaLog(ctx, log) snapshot, err := snapshot.LoadSnapshot(kopiaCtx, rep, manifest.ID(snapshotID)) if err != nil { From 967152c4066d84bcd727ba549d04b2f09625c159 Mon Sep 17 00:00:00 2001 From: Anshul Ahuja Date: Fri, 14 Jul 2023 02:33:12 +0530 Subject: [PATCH 07/14] Proposal to add support for Resource Modifier (AKA JSON Substitutions) in Restore workflow (#5880) * Design proposal Signed-off-by: Anshul Ahuja * spell Signed-off-by: Anshul Ahuja * add kubectl reference Signed-off-by: Anshul Ahuja * patch order Signed-off-by: Anshul Ahuja --------- Signed-off-by: Anshul Ahuja Co-authored-by: Anshul Ahuja --- design/json-substitution-action-design.md | 160 ++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 design/json-substitution-action-design.md diff --git a/design/json-substitution-action-design.md b/design/json-substitution-action-design.md new file mode 100644 index 0000000000..3d5b0eb28b --- /dev/null +++ b/design/json-substitution-action-design.md @@ -0,0 +1,160 @@ +# Proposal to add support for Resource Modifiers (AKA JSON Substitutions) in Restore Workflow + +- [Proposal to add support for Resource Modifiers (AKA JSON Substitutions) in Restore Workflow](#proposal-to-add-support-for-resource-modifiers-aka-json-substitutions-in-restore-workflow) + - [Abstract](#abstract) + - [Goals](#goals) + - [Non Goals](#non-goals) + - [User Stories](#user-stories) + - [Scenario 1](#scenario-1) + - [Scenario 2](#scenario-2) + - [Detailed Design](#detailed-design) + - [Reference in velero API](#reference-in-velero-api) + - [ConfigMap Structure](#configmap-structure) + - [Operations supported by the JSON Patch library:](#operations-supported-by-the-json-patch-library) + - [Advance scenarios](#advance-scenarios) + - [Conditional patches using test operation](#conditional-patches-using-test-operation) + - [Alternatives Considered](#alternatives-considered) + - [Security Considerations](#security-considerations) + - [Compatibility](#compatibility) + - [Implementation](#implementation) + - [Future Enhancements](#future-enhancements) + - [Open Issues](#open-issues) + +## Abstract +Currently velero supports substituting certain values in the K8s resources during restoration like changing the namespace, changing the storage class, etc. This proposal is to add generic support for JSON substitutions in the restore workflow. This will allow the user specify filters for particular resources and then specify a JSON patch (operator, path, value) to apply on a resource. This will allow the user to substitute any value in the K8s resource without having to write a new RestoreItemAction plugin for each kind of substitution. + + + +## Goals +- Allow the user to specify a GroupKind, Name(optional), JSON patch for modification. +- Allow the user to specify multiple JSON patch. + +## Non Goals +- Deprecating the existing RestoreItemAction plugins for standard substitutions(like changing the namespace, changing the storage class, etc.) + +## User Stories + +### Scenario 1 +- Alice has a PVC which is encrypted using a DES(Disk Encryption Set - Azure example) mentioned in the PVC YAML through the StorageClass YAML. +- Alice wishes to restore this snapshot to a different cluster. The new cluster does not have access to the same DES to provision disk's out of the snapshot. +- She wishes to use a different DES for all the PVCs which use the certain DES. +- She can use this feature to substitute the DES in all StorageClass YAMLs with the new DES without having to create a fresh storageclass, or understanding the name of the storageclass. + +### Scenario 2 +- Bob has multi zone cluster where nodes are spread across zones. +- Bob has pinned certain pods to a particular zone using nodeSelector/ nodeaffinity on the pod spec. +- In case of zone outage of the cloudprovider, Bob wishes to restore the workload to a different namespace in the same cluster, but change the zone pinning of the workload. +- Bob can use this feature to substitute the nodeSelector/ nodeaffinity in the pod spec with the new zone pinning to quickly failover the workload to a different zone's nodes. + +## Detailed Design +- The design and approach is inspired from [kubectl patch command](https://github.com/kubernetes/kubectl/blob/0a61782351a027411b8b45b1443ec3dceddef421/pkg/cmd/patch/patch.go#L102C2-L104C1) +```bash +# Update a container's image using a json patch with positional arrays +kubectl patch pod valid-pod -type='json' -p='[{"op": "replace", "path": "/spec/containers/0/image", "value":"new image"}]' +``` +- The user is expected to create a configmap with the desired Resource Modifications. Then the reference of the configmap will be provided in the RestoreSpec. +- The core restore workflow before creating/updating a particular resource in the cluster will be checked against the filters provided and respective substitutions will be applied on it. + +### Reference in velero API +> Example of Reference to configmap in RestoreSpec +```yaml +apiVersion: velero.io/v1 +kind: Restore +metadata: + name: restore-1 +spec: + resourceModifier: + refType: Configmap + ref: resourcemodifierconfigmap +``` +> Example CLI Command +```bash +velero restore create --from-backup backup-1 --resource-modifier-configmap resourcemodifierconfigmap +``` + +### Resource Modifier ConfigMap Structure +- User first needs to provide details on which resources the JSON Substitutions need to be applied. + - For this the user will provide 4 inputs - Namespaces(for NS Scoped resources), GroupKind (kind.group format similar to includeResources field in velero) and Name Regex(optional). + - If the user does not provide the Name, the JSON Substitutions will be applied to all the resources of the given Group and Kind under the given namespaces. + +- Further the use will specify the JSON Patch using the structure of kubectl's "JSON Patch" based inputs. +- Sample data in ConfigMap +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupKind: persistentvolumeclaims + resourceNameRegex: "mysql.*" + namespaces: + - bar + - foo + patches: + - operation: replace + path: "/spec/storageClassName" + value: "premium" + - operation: remove + path: "/metadata/labels/test" +``` +- The above configmap will apply the JSON Patch to all the PVCs in the namespaces bar and foo with name starting with mysql. The JSON Patch will replace the storageClassName with "premium" and remove the label "test" from the PVCs. +- The user can specify multiple JSON Patches for a particular resource. The patches will be applied in the order specified in the configmap. A subsequent patch is applied in order and if multiple patches are specified for the same path, the last patch will override the previous patches. +- The user can specify multiple resourceModifierRules in the configmap. The rules will be applied in the order specified in the configmap. + +> Users need to create one configmap in Velero install namespace from a YAML file that defined resource modifiers. The creating command would be like the below: +```bash +kubectl create cm --from-file -n velero +``` + +### Operations supported by the JSON Patch library: +- add +- remove +- replace +- move +- copy +- test (covered below) + +### Advance scenarios +#### **Conditional patches using test operation** + The `test` operation can be used to check if a particular value is present in the resource. If the value is present, the patch will be applied. If the value is not present, the patch will not be applied. This can be used to apply a patch only if a particular value is present in the resource. For example, if the user wishes to change the storage class of a PVC only if the PVC is using a particular storage class, the user can use the following configmap. +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupKind: persistentvolumeclaims.storage.k8s.io + resourceNameRegex: ".*" + namespaces: + - bar + - foo + patches: + - operation: test + path: "/spec/storageClassName" + value: "premium" + - operation: replace + path: "/spec/storageClassName" + value: "standard" +``` + +## Alternatives Considered +1. JSON Path based addressal of json fields in the resource + - This was the initial planned approach, but there is no open source library which gives satisfactory edit functionality with support for all operators supported by the JsonPath RFC. + - We attempted modifying the [https://kubernetes.io/docs/reference/kubectl/jsonpath/](https://kubernetes.io/docs/reference/kubectl/jsonpath/) but given the complexity of the code it did not make sense to change it since it would become a long term maintainability problem. +1. RestoreItemAction for each kind of standard substitution + - Not an extensible design. If a new kind of substitution is required, a new RestoreItemAction needs to be written. +1. RIA for JSON Substitution: The approach of doing JSON Substitution through a RestoreItemAction plugin was considered. But it is likely to have performance implications as the plugin will be invoked for all the resources. + +## Security Considerations +No security impact. + +## Compatibility +Compatibility with existing StorageClass mapping RestoreItemAction and similar plugins needs to be evaluated. + +## Implementation +- Changes in Restore CRD. Add a new field to the RestoreSpec to reference the configmap. +- One example of where code will be modified: https://github.com/vmware-tanzu/velero/blob/eeee4e06d209df7f08bfabda326b27aaf0054759/pkg/restore/restore.go#L1266 On the obj before Creation, we can apply the conditions to check if the resource is filtered out using given parameters. Then using JsonPatch provided, we can update the resource. +- For Jsonpatch - https://github.com/evanphx/json-patch library is used. +- JSON Patch RFC https://datatracker.ietf.org/doc/html/rfc6902 + +## Future enhancements +- Additional features such as wildcard support in path, regex match support in value, etc. can be added in future. This would involve forking the https://github.com/evanphx/json-patch library and adding the required features, since those features are not supported by the library currently and are not part of jsonpatch RFC. + +## Open Issues +NA From eebb879278d4a7217928188ebc9eb8a1c1760385 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 13 Jul 2023 18:29:43 +0800 Subject: [PATCH 08/14] data mover restore abort for existing pvc Signed-off-by: Lyndon-Li --- pkg/exposer/generic_restore.go | 4 ++ pkg/exposer/generic_restore_test.go | 20 ++++++++++ pkg/util/kube/pvc_pv.go | 9 ++++- pkg/util/kube/pvc_pv_test.go | 60 +++++++++++++++++++++++++---- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index 3617c53a8f..f11ac27c85 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -78,6 +78,10 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1.O curLog.WithField("target PVC", targetPVCName).WithField("selected node", selectedNode).Info("Target PVC is consumed") + if kube.IsPVCBound(targetPVC) { + return errors.Errorf("Target PVC %s/%s has already been bound, abort", sourceNamespace, targetPVCName) + } + restorePod, err := e.createRestorePod(ctx, ownerObject, hostingPodLabels, selectedNode) if err != nil { return errors.Wrapf(err, "error to create restore pod") diff --git a/pkg/exposer/generic_restore_test.go b/pkg/exposer/generic_restore_test.go index 2b7f4f0a63..23dd3f4800 100644 --- a/pkg/exposer/generic_restore_test.go +++ b/pkg/exposer/generic_restore_test.go @@ -54,6 +54,16 @@ func TestRestoreExpose(t *testing.T) { }, } + targetPVCObjBound := &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-target-pvc", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "fake-pv", + }, + } + tests := []struct { name string kubeClientObj []runtime.Object @@ -70,6 +80,16 @@ func TestRestoreExpose(t *testing.T) { ownerRestore: restore, err: "error to wait target PVC consumed, fake-ns/fake-target-pvc: error to wait for PVC: error to get pvc fake-ns/fake-target-pvc: persistentvolumeclaims \"fake-target-pvc\" not found", }, + { + name: "target pvc is already bound", + targetPVCName: "fake-target-pvc", + sourceNamespace: "fake-ns", + ownerRestore: restore, + kubeClientObj: []runtime.Object{ + targetPVCObjBound, + }, + err: "Target PVC fake-ns/fake-target-pvc has already been bound, abort", + }, { name: "create restore pod fail", targetPVCName: "fake-target-pvc", diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index a24525894a..8e13b6b1c7 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -284,11 +284,11 @@ func WaitPVBound(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvN } if tmpPV.Spec.ClaimRef.Name != pvcName { - return false, nil + return false, errors.Errorf("pv has been bound by unexpected pvc %s/%s", tmpPV.Spec.ClaimRef.Namespace, tmpPV.Spec.ClaimRef.Name) } if tmpPV.Spec.ClaimRef.Namespace != pvcNamespace { - return false, nil + return false, errors.Errorf("pv has been bound by unexpected pvc %s/%s", tmpPV.Spec.ClaimRef.Namespace, tmpPV.Spec.ClaimRef.Name) } updated = tmpPV @@ -302,3 +302,8 @@ func WaitPVBound(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvN return updated, nil } } + +// IsPVCBound returns true if the specified PVC has been bound +func IsPVCBound(pvc *corev1api.PersistentVolumeClaim) bool { + return pvc.Spec.VolumeName != "" +} diff --git a/pkg/util/kube/pvc_pv_test.go b/pkg/util/kube/pvc_pv_test.go index d31c5d57d1..b883a3ea5d 100644 --- a/pkg/util/kube/pvc_pv_test.go +++ b/pkg/util/kube/pvc_pv_test.go @@ -741,9 +741,10 @@ func TestWaitPVBound(t *testing.T) { err: "error to wait for bound of PV: timed out waiting for the condition", }, { - name: "pvc claimRef pvc name mismatch", - pvName: "fake-pv", - pvcName: "fake-pvc", + name: "pvc claimRef pvc name mismatch", + pvName: "fake-pv", + pvcName: "fake-pvc", + pvcNamespace: "fake-ns", kubeClientObj: []runtime.Object{ &corev1api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ @@ -751,12 +752,14 @@ func TestWaitPVBound(t *testing.T) { }, Spec: corev1api.PersistentVolumeSpec{ ClaimRef: &corev1api.ObjectReference{ - Kind: "fake-kind", + Kind: "fake-kind", + Namespace: "fake-ns", + Name: "fake-pvc-1", }, }, }, }, - err: "error to wait for bound of PV: timed out waiting for the condition", + err: "error to wait for bound of PV: pv has been bound by unexpected pvc fake-ns/fake-pvc-1", }, { name: "pvc claimRef pvc namespace mismatch", @@ -770,13 +773,14 @@ func TestWaitPVBound(t *testing.T) { }, Spec: corev1api.PersistentVolumeSpec{ ClaimRef: &corev1api.ObjectReference{ - Kind: "fake-kind", - Name: "fake-pvc", + Kind: "fake-kind", + Namespace: "fake-ns-1", + Name: "fake-pvc", }, }, }, }, - err: "error to wait for bound of PV: timed out waiting for the condition", + err: "error to wait for bound of PV: pv has been bound by unexpected pvc fake-ns-1/fake-pvc", }, { name: "success", @@ -834,3 +838,43 @@ func TestWaitPVBound(t *testing.T) { }) } } + +func TestIsPVCBound(t *testing.T) { + tests := []struct { + name string + pvc *corev1api.PersistentVolumeClaim + expect bool + }{ + { + name: "expect bound", + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-pvc", + }, + Spec: corev1api.PersistentVolumeClaimSpec{ + VolumeName: "fake-volume", + }, + }, + expect: true, + }, + { + name: "expect not bound", + pvc: &corev1api.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "fake-ns", + Name: "fake-pvc", + }, + }, + expect: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := IsPVCBound(test.pvc) + + assert.Equal(t, test.expect, result) + }) + } +} From 084fd665866290c7acc3b363a9a8fee047f445a8 Mon Sep 17 00:00:00 2001 From: allenxu404 Date: Thu, 13 Jul 2023 19:38:13 +0800 Subject: [PATCH 09/14] add data upload/download metrics Signed-off-by: allenxu404 --- changelogs/unreleased/6493-allenxu404 | 1 + pkg/cmd/cli/nodeagent/server.go | 8 +- pkg/controller/data_download_controller.go | 12 +- .../data_download_controller_test.go | 3 +- pkg/controller/data_upload_controller.go | 15 ++- pkg/controller/data_upload_controller_test.go | 3 +- .../pod_volume_backup_controller_test.go | 2 +- pkg/metrics/metrics.go | 120 +++++++++++++++++- 8 files changed, 152 insertions(+), 12 deletions(-) create mode 100644 changelogs/unreleased/6493-allenxu404 diff --git a/changelogs/unreleased/6493-allenxu404 b/changelogs/unreleased/6493-allenxu404 new file mode 100644 index 0000000000..cc61aaa86d --- /dev/null +++ b/changelogs/unreleased/6493-allenxu404 @@ -0,0 +1 @@ +Add data upload and download metrics \ No newline at end of file diff --git a/pkg/cmd/cli/nodeagent/server.go b/pkg/cmd/cli/nodeagent/server.go index 51c0a45006..ffbb5ca74a 100644 --- a/pkg/cmd/cli/nodeagent/server.go +++ b/pkg/cmd/cli/nodeagent/server.go @@ -224,9 +224,9 @@ func (s *nodeAgentServer) run() { s.logger.Fatalf("Failed to start metric server for node agent at [%s]: %v", s.metricsAddress, err) } }() - s.metrics = metrics.NewPodVolumeMetrics() + s.metrics = metrics.NewNodeMetrics() s.metrics.RegisterAllMetrics() - s.metrics.InitPodVolumeMetricsForNode(s.nodeName) + s.metrics.InitMetricsForNode(s.nodeName) s.markInProgressCRsFailed() @@ -260,13 +260,13 @@ func (s *nodeAgentServer) run() { s.logger.WithError(err).Fatal("Unable to create the pod volume restore controller") } - dataUploadReconciler := controller.NewDataUploadReconciler(s.mgr.GetClient(), s.kubeClient, s.csiSnapshotClient.SnapshotV1(), repoEnsurer, clock.RealClock{}, credentialGetter, s.nodeName, s.fileSystem, s.config.dataMoverPrepareTimeout, s.logger) + dataUploadReconciler := controller.NewDataUploadReconciler(s.mgr.GetClient(), s.kubeClient, s.csiSnapshotClient.SnapshotV1(), repoEnsurer, clock.RealClock{}, credentialGetter, s.nodeName, s.fileSystem, s.config.dataMoverPrepareTimeout, s.logger, s.metrics) s.markDataUploadsCancel(dataUploadReconciler) if err = dataUploadReconciler.SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data upload controller") } - dataDownloadReconciler := controller.NewDataDownloadReconciler(s.mgr.GetClient(), s.kubeClient, repoEnsurer, credentialGetter, s.nodeName, s.config.dataMoverPrepareTimeout, s.logger) + dataDownloadReconciler := controller.NewDataDownloadReconciler(s.mgr.GetClient(), s.kubeClient, repoEnsurer, credentialGetter, s.nodeName, s.config.dataMoverPrepareTimeout, s.logger, s.metrics) s.markDataDownloadsCancel(dataDownloadReconciler) if err = dataDownloadReconciler.SetupWithManager(s.mgr); err != nil { s.logger.WithError(err).Fatal("Unable to create the data download controller") diff --git a/pkg/controller/data_download_controller.go b/pkg/controller/data_download_controller.go index 2243b17dec..d41a262863 100644 --- a/pkg/controller/data_download_controller.go +++ b/pkg/controller/data_download_controller.go @@ -44,6 +44,7 @@ import ( datamover "github.com/vmware-tanzu/velero/pkg/datamover" "github.com/vmware-tanzu/velero/pkg/datapath" "github.com/vmware-tanzu/velero/pkg/exposer" + "github.com/vmware-tanzu/velero/pkg/metrics" repository "github.com/vmware-tanzu/velero/pkg/repository" "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/filesystem" @@ -63,10 +64,11 @@ type DataDownloadReconciler struct { repositoryEnsurer *repository.Ensurer dataPathMgr *datapath.Manager preparingTimeout time.Duration + metrics *metrics.ServerMetrics } func NewDataDownloadReconciler(client client.Client, kubeClient kubernetes.Interface, - repoEnsurer *repository.Ensurer, credentialGetter *credentials.CredentialGetter, nodeName string, preparingTimeout time.Duration, logger logrus.FieldLogger) *DataDownloadReconciler { + repoEnsurer *repository.Ensurer, credentialGetter *credentials.CredentialGetter, nodeName string, preparingTimeout time.Duration, logger logrus.FieldLogger, metrics *metrics.ServerMetrics) *DataDownloadReconciler { return &DataDownloadReconciler{ client: client, kubeClient: kubeClient, @@ -79,6 +81,7 @@ func NewDataDownloadReconciler(client client.Client, kubeClient kubernetes.Inter restoreExposer: exposer.NewGenericRestoreExposer(kubeClient, logger), dataPathMgr: datapath.NewManager(1), preparingTimeout: preparingTimeout, + metrics: metrics, } } @@ -301,6 +304,7 @@ func (r *DataDownloadReconciler) OnDataDownloadCompleted(ctx context.Context, na log.WithError(err).Error("error updating data download status") } else { log.Infof("Data download is marked as %s", dd.Status.Phase) + r.metrics.RegisterDataDownloadSuccess(r.nodeName) } } @@ -343,6 +347,8 @@ func (r *DataDownloadReconciler) OnDataDownloadCancelled(ctx context.Context, na dd.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, &dd, client.MergeFrom(original)); err != nil { log.WithError(err).Error("error updating data download status") + } else { + r.metrics.RegisterDataDownloadCancel(r.nodeName) } } } @@ -497,6 +503,8 @@ func (r *DataDownloadReconciler) updateStatusToFailed(ctx context.Context, dd *v if patchErr := r.client.Patch(ctx, dd, client.MergeFrom(original)); patchErr != nil { log.WithError(patchErr).Error("error updating DataDownload status") + } else { + r.metrics.RegisterDataDownloadFailure(r.nodeName) } return err @@ -548,6 +556,8 @@ func (r *DataDownloadReconciler) onPrepareTimeout(ctx context.Context, dd *veler r.restoreExposer.CleanUp(ctx, getDataDownloadOwnerObject(dd)) log.Info("Dataupload has been cleaned up") + + r.metrics.RegisterDataDownloadFailure(r.nodeName) } func (r *DataDownloadReconciler) exclusiveUpdateDataDownload(ctx context.Context, dd *velerov2alpha1api.DataDownload, diff --git a/pkg/controller/data_download_controller_test.go b/pkg/controller/data_download_controller_test.go index f70706a7ef..a0a9b645f0 100644 --- a/pkg/controller/data_download_controller_test.go +++ b/pkg/controller/data_download_controller_test.go @@ -46,6 +46,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/datapath" datapathmockes "github.com/vmware-tanzu/velero/pkg/datapath/mocks" "github.com/vmware-tanzu/velero/pkg/exposer" + "github.com/vmware-tanzu/velero/pkg/metrics" velerotest "github.com/vmware-tanzu/velero/pkg/test" "github.com/vmware-tanzu/velero/pkg/uploader" @@ -136,7 +137,7 @@ func initDataDownloadReconcilerWithError(objects []runtime.Object, needError ... if err != nil { return nil, err } - return NewDataDownloadReconciler(fakeClient, fakeKubeClient, nil, &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", time.Minute*5, velerotest.NewLogger()), nil + return NewDataDownloadReconciler(fakeClient, fakeKubeClient, nil, &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", time.Minute*5, velerotest.NewLogger(), metrics.NewServerMetrics()), nil } func TestDataDownloadReconcile(t *testing.T) { diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index a20f009c9e..de54afd0dd 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -46,6 +46,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/datamover" "github.com/vmware-tanzu/velero/pkg/datapath" "github.com/vmware-tanzu/velero/pkg/exposer" + "github.com/vmware-tanzu/velero/pkg/metrics" "github.com/vmware-tanzu/velero/pkg/repository" "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/filesystem" @@ -71,11 +72,12 @@ type DataUploadReconciler struct { snapshotExposerList map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer dataPathMgr *datapath.Manager preparingTimeout time.Duration + metrics *metrics.ServerMetrics } func NewDataUploadReconciler(client client.Client, kubeClient kubernetes.Interface, csiSnapshotClient snapshotter.SnapshotV1Interface, repoEnsurer *repository.Ensurer, clock clocks.WithTickerAndDelayedExecution, - cred *credentials.CredentialGetter, nodeName string, fs filesystem.Interface, preparingTimeout time.Duration, log logrus.FieldLogger) *DataUploadReconciler { + cred *credentials.CredentialGetter, nodeName string, fs filesystem.Interface, preparingTimeout time.Duration, log logrus.FieldLogger, metrics *metrics.ServerMetrics) *DataUploadReconciler { return &DataUploadReconciler{ client: client, kubeClient: kubeClient, @@ -89,6 +91,7 @@ func NewDataUploadReconciler(client client.Client, kubeClient kubernetes.Interfa snapshotExposerList: map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer{velerov2alpha1api.SnapshotTypeCSI: exposer.NewCSISnapshotExposer(kubeClient, csiSnapshotClient, log)}, dataPathMgr: datapath.NewManager(1), preparingTimeout: preparingTimeout, + metrics: metrics, } } @@ -308,8 +311,10 @@ func (r *DataUploadReconciler) OnDataUploadCompleted(ctx context.Context, namesp if err := r.client.Patch(ctx, &du, client.MergeFrom(original)); err != nil { log.WithError(err).Error("error updating DataUpload status") + } else { + log.Info("Data upload completed") + r.metrics.RegisterDataUploadSuccess(r.nodeName) } - log.Info("Data upload completed") } func (r *DataUploadReconciler) OnDataUploadFailed(ctx context.Context, namespace string, duName string, err error) { @@ -360,6 +365,8 @@ func (r *DataUploadReconciler) OnDataUploadCancelled(ctx context.Context, namesp du.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if err := r.client.Patch(ctx, &du, client.MergeFrom(original)); err != nil { log.WithError(err).Error("error updating DataUpload status") + } else { + r.metrics.RegisterDataUploadCancel(r.nodeName) } } } @@ -518,6 +525,8 @@ func (r *DataUploadReconciler) updateStatusToFailed(ctx context.Context, du *vel du.Status.CompletionTimestamp = &metav1.Time{Time: r.Clock.Now()} if patchErr := r.client.Patch(ctx, du, client.MergeFrom(original)); patchErr != nil { log.WithError(patchErr).Error("error updating DataUpload status") + } else { + r.metrics.RegisterDataUploadFailure(r.nodeName) } return err @@ -580,6 +589,8 @@ func (r *DataUploadReconciler) onPrepareTimeout(ctx context.Context, du *velerov log.Info("Dataupload has been cleaned up") } + + r.metrics.RegisterDataUploadFailure(r.nodeName) } func (r *DataUploadReconciler) exclusiveUpdateDataUpload(ctx context.Context, du *velerov2alpha1api.DataUpload, diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go index 761a8d66db..3e0c046065 100644 --- a/pkg/controller/data_upload_controller_test.go +++ b/pkg/controller/data_upload_controller_test.go @@ -46,6 +46,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/datapath" "github.com/vmware-tanzu/velero/pkg/exposer" + "github.com/vmware-tanzu/velero/pkg/metrics" "github.com/vmware-tanzu/velero/pkg/repository" velerotest "github.com/vmware-tanzu/velero/pkg/test" "github.com/vmware-tanzu/velero/pkg/uploader" @@ -193,7 +194,7 @@ func initDataUploaderReconcilerWithError(needError ...error) (*DataUploadReconci return nil, err } return NewDataUploadReconciler(fakeClient, fakeKubeClient, fakeSnapshotClient.SnapshotV1(), nil, - testclocks.NewFakeClock(now), &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", fakeFS, time.Minute*5, velerotest.NewLogger()), nil + testclocks.NewFakeClock(now), &credentials.CredentialGetter{FromFile: credentialFileStore}, "test_node", fakeFS, time.Minute*5, velerotest.NewLogger(), metrics.NewServerMetrics()), nil } func dataUploadBuilder() *builder.DataUploadBuilder { diff --git a/pkg/controller/pod_volume_backup_controller_test.go b/pkg/controller/pod_volume_backup_controller_test.go index a7ec9f59c6..25fcae80a0 100644 --- a/pkg/controller/pod_volume_backup_controller_test.go +++ b/pkg/controller/pod_volume_backup_controller_test.go @@ -192,7 +192,7 @@ var _ = Describe("PodVolumeBackup Reconciler", func() { r := PodVolumeBackupReconciler{ Client: fakeClient, clock: testclocks.NewFakeClock(now), - metrics: metrics.NewPodVolumeMetrics(), + metrics: metrics.NewNodeMetrics(), credentialGetter: &credentials.CredentialGetter{FromFile: credentialFileStore}, nodeName: "test_node", fileSystem: fakeFS, diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 75d2e79d53..e6879f363c 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -66,6 +66,14 @@ const ( podVolumeOperationLatencySeconds = "pod_volume_operation_latency_seconds" podVolumeOperationLatencyGaugeSeconds = "pod_volume_operation_latency_seconds_gauge" + // data mover metrics + DataUploadSuccessTotal = "data_upload_success_total" + DataUploadFailureTotal = "data_upload_failure_total" + DataUploadCancelTotal = "data_upload_cancel_total" + DataDownloadSuccessTotal = "data_download_success_total" + DataDownloadFailureTotal = "data_download_failure_total" + DataDownloadCancelTotal = "data_download_cancel_total" + // Labels nodeMetricLabel = "node" podVolumeOperationLabel = "operation" @@ -319,7 +327,7 @@ func NewServerMetrics() *ServerMetrics { } } -func NewPodVolumeMetrics() *ServerMetrics { +func NewNodeMetrics() *ServerMetrics { return &ServerMetrics{ metrics: map[string]prometheus.Collector{ podVolumeBackupEnqueueTotal: prometheus.NewCounterVec( @@ -365,6 +373,54 @@ func NewPodVolumeMetrics() *ServerMetrics { }, []string{nodeMetricLabel, podVolumeOperationLabel, backupNameLabel, pvbNameLabel}, ), + DataUploadSuccessTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: podVolumeMetricsNamespace, + Name: DataUploadSuccessTotal, + Help: "Total number of successful uploaded snapshots", + }, + []string{nodeMetricLabel}, + ), + DataUploadFailureTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: podVolumeMetricsNamespace, + Name: DataUploadFailureTotal, + Help: "Total number of failed uploaded snapshots", + }, + []string{nodeMetricLabel}, + ), + DataUploadCancelTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: podVolumeMetricsNamespace, + Name: DataUploadCancelTotal, + Help: "Total number of canceled uploaded snapshots", + }, + []string{nodeMetricLabel}, + ), + DataDownloadSuccessTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: podVolumeMetricsNamespace, + Name: DataDownloadSuccessTotal, + Help: "Total number of successful downloaded snapshots", + }, + []string{nodeMetricLabel}, + ), + DataDownloadFailureTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: podVolumeMetricsNamespace, + Name: DataDownloadFailureTotal, + Help: "Total number of failed downloaded snapshots", + }, + []string{nodeMetricLabel}, + ), + DataDownloadCancelTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: podVolumeMetricsNamespace, + Name: DataDownloadCancelTotal, + Help: "Total number of canceled downloaded snapshots", + }, + []string{nodeMetricLabel}, + ), }, } } @@ -450,13 +506,31 @@ func (m *ServerMetrics) InitSchedule(scheduleName string) { } // InitSchedule initializes counter metrics for a node. -func (m *ServerMetrics) InitPodVolumeMetricsForNode(node string) { +func (m *ServerMetrics) InitMetricsForNode(node string) { if c, ok := m.metrics[podVolumeBackupEnqueueTotal].(*prometheus.CounterVec); ok { c.WithLabelValues(node).Add(0) } if c, ok := m.metrics[podVolumeBackupDequeueTotal].(*prometheus.CounterVec); ok { c.WithLabelValues(node).Add(0) } + if c, ok := m.metrics[DataUploadSuccessTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Add(0) + } + if c, ok := m.metrics[DataUploadFailureTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Add(0) + } + if c, ok := m.metrics[DataUploadCancelTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Add(0) + } + if c, ok := m.metrics[DataDownloadSuccessTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Add(0) + } + if c, ok := m.metrics[DataDownloadFailureTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Add(0) + } + if c, ok := m.metrics[DataDownloadCancelTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Add(0) + } } // RegisterPodVolumeBackupEnqueue records enqueuing of a PodVolumeBackup object. @@ -473,6 +547,48 @@ func (m *ServerMetrics) RegisterPodVolumeBackupDequeue(node string) { } } +// RegisterDataUploadSuccess records successful uploaded snapshots. +func (m *ServerMetrics) RegisterDataUploadSuccess(node string) { + if c, ok := m.metrics[DataUploadSuccessTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Inc() + } +} + +// RegisterDataUploadFailure records failed uploaded snapshots. +func (m *ServerMetrics) RegisterDataUploadFailure(node string) { + if c, ok := m.metrics[DataUploadFailureTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Inc() + } +} + +// RegisterDataUploadCancel records canceled uploaded snapshots. +func (m *ServerMetrics) RegisterDataUploadCancel(node string) { + if c, ok := m.metrics[DataUploadCancelTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Inc() + } +} + +// RegisterDataDownloadSuccess records successful downloaded snapshots. +func (m *ServerMetrics) RegisterDataDownloadSuccess(node string) { + if c, ok := m.metrics[DataDownloadSuccessTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Inc() + } +} + +// RegisterDataDownloadFailure records failed downloaded snapshots. +func (m *ServerMetrics) RegisterDataDownloadFailure(node string) { + if c, ok := m.metrics[DataDownloadFailureTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Inc() + } +} + +// RegisterDataDownloadCancel records canceled downloaded snapshots. +func (m *ServerMetrics) RegisterDataDownloadCancel(node string) { + if c, ok := m.metrics[DataDownloadCancelTotal].(*prometheus.CounterVec); ok { + c.WithLabelValues(node).Inc() + } +} + // ObservePodVolumeOpLatency records the number of seconds a pod volume operation took. func (m *ServerMetrics) ObservePodVolumeOpLatency(node, pvbName, opName, backupName string, seconds float64) { if h, ok := m.metrics[podVolumeOperationLatencySeconds].(*prometheus.HistogramVec); ok { From 89d3ad48643b90efa518222da99e537cfff9fcbb Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Wed, 5 Jul 2023 20:44:09 +0800 Subject: [PATCH 10/14] Remove dependency of the legacy client code from pkg/cmd directory. Signed-off-by: Xun Jiang --- changelogs/unreleased/6469-blackpiglet | 1 + pkg/client/factory.go | 37 ++++ pkg/client/mocks/Factory.go | 26 +++ pkg/cmd/cli/backup/create.go | 32 ++-- pkg/cmd/cli/backup/create_test.go | 160 ++++++++---------- pkg/cmd/cli/backup/delete.go | 28 ++- pkg/cmd/cli/backup/delete_test.go | 55 +++--- pkg/cmd/cli/backup/describe.go | 40 +++-- pkg/cmd/cli/backup/describe_test.go | 40 ++--- pkg/cmd/cli/backup/download.go | 7 +- pkg/cmd/cli/backup/download_test.go | 26 ++- pkg/cmd/cli/backuplocation/delete_test.go | 13 +- pkg/cmd/cli/backuplocation/get_test.go | 5 +- pkg/cmd/cli/delete_options.go | 6 +- pkg/cmd/cli/restore/delete.go | 28 ++- pkg/cmd/cli/restore/describe.go | 2 +- pkg/cmd/cli/schedule/delete.go | 22 ++- pkg/cmd/cli/version/version_test.go | 5 +- pkg/cmd/util/output/backup_describer.go | 6 +- .../output/backup_structured_describer.go | 6 +- pkg/cmd/util/output/restore_describer.go | 3 +- pkg/util/kube/list_watch.go | 41 +++++ 22 files changed, 332 insertions(+), 257 deletions(-) create mode 100644 changelogs/unreleased/6469-blackpiglet create mode 100644 pkg/util/kube/list_watch.go diff --git a/changelogs/unreleased/6469-blackpiglet b/changelogs/unreleased/6469-blackpiglet new file mode 100644 index 0000000000..b5349c5179 --- /dev/null +++ b/changelogs/unreleased/6469-blackpiglet @@ -0,0 +1 @@ +Remove dependency of the legacy client code from pkg/cmd directory \ No newline at end of file diff --git a/pkg/client/factory.go b/pkg/client/factory.go index e57ddc4fe5..5c1ffc545c 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -55,6 +55,10 @@ type Factory interface { // types to its scheme. It uses the following priority to specify the cluster // configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration. KubebuilderClient() (kbclient.Client, error) + // KubebuilderWatchClient returns a client with watcher for the controller runtime framework. + // It adds Kubernetes and Velero types to its scheme. It uses the following priority to specify the cluster + // configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration. + KubebuilderWatchClient() (kbclient.WithWatch, error) // SetBasename changes the basename for an already-constructed client. // This is useful for generating clients that require a different user-agent string below the root `velero` // command, such as the server subcommand. @@ -182,6 +186,39 @@ func (f *factory) KubebuilderClient() (kbclient.Client, error) { return kubebuilderClient, nil } +func (f *factory) KubebuilderWatchClient() (kbclient.WithWatch, error) { + clientConfig, err := f.ClientConfig() + if err != nil { + return nil, err + } + + scheme := runtime.NewScheme() + if err := velerov1api.AddToScheme(scheme); err != nil { + return nil, err + } + if err := velerov2alpha1api.AddToScheme(scheme); err != nil { + return nil, err + } + if err := k8scheme.AddToScheme(scheme); err != nil { + return nil, err + } + if err := apiextv1beta1.AddToScheme(scheme); err != nil { + return nil, err + } + if err := apiextv1.AddToScheme(scheme); err != nil { + return nil, err + } + kubebuilderWatchClient, err := kbclient.NewWithWatch(clientConfig, kbclient.Options{ + Scheme: scheme, + }) + + if err != nil { + return nil, err + } + + return kubebuilderWatchClient, nil +} + func (f *factory) SetBasename(name string) { f.baseName = name } diff --git a/pkg/client/mocks/Factory.go b/pkg/client/mocks/Factory.go index 27c588cf44..3bd0160476 100644 --- a/pkg/client/mocks/Factory.go +++ b/pkg/client/mocks/Factory.go @@ -157,6 +157,32 @@ func (_m *Factory) KubebuilderClient() (pkgclient.Client, error) { return r0, r1 } +// KubebuilderWatchClient provides a mock function with given fields: +func (_m *Factory) KubebuilderWatchClient() (pkgclient.WithWatch, error) { + ret := _m.Called() + + var r0 pkgclient.WithWatch + var r1 error + if rf, ok := ret.Get(0).(func() (pkgclient.WithWatch, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() pkgclient.WithWatch); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(pkgclient.WithWatch) + } + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Namespace provides a mock function with given fields: func (_m *Factory) Namespace() string { ret := _m.Called() diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 2b1a1c766b..f69a9a7fae 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -22,13 +22,11 @@ import ( "strings" "time" - kbclient "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/spf13/cobra" "github.com/spf13/pflag" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeerrs "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/cache" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" @@ -36,9 +34,8 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" "github.com/vmware-tanzu/velero/pkg/cmd/util/output" - veleroclient "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" - v1 "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions/velero/v1" "github.com/vmware-tanzu/velero/pkg/util/collections" + "github.com/vmware-tanzu/velero/pkg/util/kube" ) func NewCreateCommand(f client.Factory, use string) *cobra.Command { @@ -107,7 +104,7 @@ type CreateOptions struct { CSISnapshotTimeout time.Duration ItemOperationTimeout time.Duration ResPoliciesConfigmap string - client veleroclient.Interface + client kbclient.WithWatch } func NewCreateOptions() *CreateOptions { @@ -171,7 +168,7 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } - client, err := f.KubebuilderClient() + client, err := f.KubebuilderWatchClient() if err != nil { return err } @@ -203,7 +200,8 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto } for _, loc := range o.SnapshotLocations { - if _, err := o.client.VeleroV1().VolumeSnapshotLocations(f.Namespace()).Get(context.TODO(), loc, metav1.GetOptions{}); err != nil { + snapshotLocation := new(velerov1api.VolumeSnapshotLocation) + if err := o.client.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: loc}, snapshotLocation); err != nil { return err } } @@ -216,7 +214,7 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { if len(args) > 0 { o.Name = args[0] } - client, err := f.Client() + client, err := f.KubebuilderWatchClient() if err != nil { return err } @@ -238,7 +236,6 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { fmt.Println("Creating backup from schedule, all other filters are ignored.") } - var backupInformer cache.SharedIndexInformer var updates chan *velerov1api.Backup if o.Wait { stop := make(chan struct{}) @@ -246,12 +243,17 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { updates = make(chan *velerov1api.Backup) - backupInformer = v1.NewBackupInformer(o.client, f.Namespace(), 0, nil) - + lw := kube.InternalLW{ + Client: o.client, + Namespace: f.Namespace(), + ObjectList: new(velerov1api.BackupList), + } + backupInformer := cache.NewSharedInformer(&lw, &velerov1api.Backup{}, time.Second) backupInformer.AddEventHandler( cache.FilteringResourceEventHandler{ FilterFunc: func(obj interface{}) bool { backup, ok := obj.(*velerov1api.Backup) + if !ok { return false } @@ -275,10 +277,11 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { }, }, ) + go backupInformer.Run(stop) } - _, err = o.client.VeleroV1().Backups(backup.Namespace).Create(context.TODO(), backup, metav1.CreateOptions{}) + err = o.client.Create(context.TODO(), backup, &kbclient.CreateOptions{}) if err != nil { return err } @@ -341,7 +344,8 @@ func (o *CreateOptions) BuildBackup(namespace string) (*velerov1api.Backup, erro var backupBuilder *builder.BackupBuilder if o.FromSchedule != "" { - schedule, err := o.client.VeleroV1().Schedules(namespace).Get(context.TODO(), o.FromSchedule, metav1.GetOptions{}) + schedule := new(velerov1api.Schedule) + err := o.client.Get(context.TODO(), kbclient.ObjectKey{Namespace: namespace, Name: o.FromSchedule}, schedule) if err != nil { return nil, err } diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index bcff3a98c4..8d20cda18d 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -23,24 +23,21 @@ import ( "testing" "time" + flag "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - flag "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/runtime" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" - - clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake" - factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/fake" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" "github.com/vmware-tanzu/velero/pkg/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) func TestCreateOptions_BuildBackup(t *testing.T) { @@ -77,27 +74,31 @@ func TestCreateOptions_BuildBackup(t *testing.T) { func TestCreateOptions_BuildBackupFromSchedule(t *testing.T) { o := NewCreateOptions() o.FromSchedule = "test" - o.client = fake.NewSimpleClientset() + + scheme := runtime.NewScheme() + err := velerov1api.AddToScheme(scheme) + require.NoError(t, err) + o.client = velerotest.NewFakeControllerRuntimeClient(t).(controllerclient.WithWatch) t.Run("inexistent schedule", func(t *testing.T) { _, err := o.BuildBackup(cmdtest.VeleroNameSpace) - assert.Error(t, err) + require.Error(t, err) }) expectedBackupSpec := builder.ForBackup("test", cmdtest.VeleroNameSpace).IncludedNamespaces("test").Result().Spec schedule := builder.ForSchedule(cmdtest.VeleroNameSpace, "test").Template(expectedBackupSpec).ObjectMeta(builder.WithLabels("velero.io/test", "true"), builder.WithAnnotations("velero.io/test", "true")).Result() - o.client.VeleroV1().Schedules(cmdtest.VeleroNameSpace).Create(context.TODO(), schedule, metav1.CreateOptions{}) + o.client.Create(context.TODO(), schedule, &kbclient.CreateOptions{}) t.Run("existing schedule", func(t *testing.T) { backup, err := o.BuildBackup(cmdtest.VeleroNameSpace) - assert.NoError(t, err) + require.NoError(t, err) - assert.Equal(t, expectedBackupSpec, backup.Spec) - assert.Equal(t, map[string]string{ + require.Equal(t, expectedBackupSpec, backup.Spec) + require.Equal(t, map[string]string{ "velero.io/test": "true", velerov1api.ScheduleNameLabel: "test", }, backup.GetLabels()) - assert.Equal(t, map[string]string{ + require.Equal(t, map[string]string{ "velero.io/test": "true", }, backup.GetAnnotations()) }) @@ -145,6 +146,7 @@ func TestCreateCommand(t *testing.T) { args := []string{name} t.Run("create a backup create command with full options except fromSchedule and wait, then run by create option", func(t *testing.T) { + // create a factory f := &factorymocks.Factory{} @@ -203,81 +205,68 @@ func TestCreateCommand(t *testing.T) { flags.Parse([]string{"--data-mover", dataMover}) //flags.Parse([]string{"--wait"}) - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - backups.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) + client := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + f.On("Namespace").Return(mock.Anything) - f.On("KubebuilderClient").Return(nil, nil) + f.On("KubebuilderWatchClient").Return(client, nil) //Complete e := o.Complete(args, f) - assert.NoError(t, e) + require.NoError(t, e) //Validate e = o.Validate(cmd, args, f) - assert.Contains(t, e.Error(), "include-resources, exclude-resources and include-cluster-resources are old filter parameters") - assert.Contains(t, e.Error(), "include-cluster-scoped-resources, exclude-cluster-scoped-resources, include-namespace-scoped-resources and exclude-namespace-scoped-resources are new filter parameters.\nThey cannot be used together") + require.Contains(t, e.Error(), "include-resources, exclude-resources and include-cluster-resources are old filter parameters") + require.Contains(t, e.Error(), "include-cluster-scoped-resources, exclude-cluster-scoped-resources, include-namespace-scoped-resources and exclude-namespace-scoped-resources are new filter parameters.\nThey cannot be used together") //cmd e = o.Run(cmd, f) - assert.NoError(t, e) + require.NoError(t, e) //Execute cmd.SetArgs([]string{"bk-name-exe"}) e = cmd.Execute() - assert.NoError(t, e) + require.NoError(t, e) // verify all options are set as expected - assert.Equal(t, name, o.Name) - assert.Equal(t, includeNamespaces, o.IncludeNamespaces.String()) - assert.Equal(t, excludeNamespaces, o.ExcludeNamespaces.String()) - assert.Equal(t, includeResources, o.IncludeResources.String()) - assert.Equal(t, excludeResources, o.ExcludeResources.String()) - assert.Equal(t, includeClusterScopedResources, o.IncludeClusterScopedResources.String()) - assert.Equal(t, excludeClusterScopedResources, o.ExcludeClusterScopedResources.String()) - assert.Equal(t, includeNamespaceScopedResources, o.IncludeNamespaceScopedResources.String()) - assert.Equal(t, excludeNamespaceScopedResources, o.ExcludeNamespaceScopedResources.String()) - assert.Equal(t, true, test.CompareSlice(strings.Split(labels, ","), strings.Split(o.Labels.String(), ","))) - assert.Equal(t, storageLocation, o.StorageLocation) - assert.Equal(t, snapshotLocations, strings.Split(o.SnapshotLocations[0], ",")[0]) - assert.Equal(t, selector, o.Selector.String()) - assert.Equal(t, orderedResources, o.OrderedResources) - assert.Equal(t, csiSnapshotTimeout, o.CSISnapshotTimeout.String()) - assert.Equal(t, itemOperationTimeout, o.ItemOperationTimeout.String()) - assert.Equal(t, snapshotVolumes, o.SnapshotVolumes.String()) - assert.Equal(t, snapshotMoveData, o.SnapshotMoveData.String()) - assert.Equal(t, includeClusterResources, o.IncludeClusterResources.String()) - assert.Equal(t, defaultVolumesToFsBackup, o.DefaultVolumesToFsBackup.String()) - assert.Equal(t, resPoliciesConfigmap, o.ResPoliciesConfigmap) - assert.Equal(t, dataMover, o.DataMover) + require.Equal(t, name, o.Name) + require.Equal(t, includeNamespaces, o.IncludeNamespaces.String()) + require.Equal(t, excludeNamespaces, o.ExcludeNamespaces.String()) + require.Equal(t, includeResources, o.IncludeResources.String()) + require.Equal(t, excludeResources, o.ExcludeResources.String()) + require.Equal(t, includeClusterScopedResources, o.IncludeClusterScopedResources.String()) + require.Equal(t, excludeClusterScopedResources, o.ExcludeClusterScopedResources.String()) + require.Equal(t, includeNamespaceScopedResources, o.IncludeNamespaceScopedResources.String()) + require.Equal(t, excludeNamespaceScopedResources, o.ExcludeNamespaceScopedResources.String()) + require.Equal(t, true, test.CompareSlice(strings.Split(labels, ","), strings.Split(o.Labels.String(), ","))) + require.Equal(t, storageLocation, o.StorageLocation) + require.Equal(t, snapshotLocations, strings.Split(o.SnapshotLocations[0], ",")[0]) + require.Equal(t, selector, o.Selector.String()) + require.Equal(t, orderedResources, o.OrderedResources) + require.Equal(t, csiSnapshotTimeout, o.CSISnapshotTimeout.String()) + require.Equal(t, itemOperationTimeout, o.ItemOperationTimeout.String()) + require.Equal(t, snapshotVolumes, o.SnapshotVolumes.String()) + require.Equal(t, snapshotMoveData, o.SnapshotMoveData.String()) + require.Equal(t, includeClusterResources, o.IncludeClusterResources.String()) + require.Equal(t, defaultVolumesToFsBackup, o.DefaultVolumesToFsBackup.String()) + require.Equal(t, resPoliciesConfigmap, o.ResPoliciesConfigmap) + require.Equal(t, dataMover, o.DataMover) //assert.Equal(t, true, o.Wait) // verify oldAndNewFilterParametersUsedTogether mix := o.oldAndNewFilterParametersUsedTogether() - assert.Equal(t, true, mix) + require.Equal(t, true, mix) }) + t.Run("create a backup create command with specific storage-location setting", func(t *testing.T) { bsl := "bsl-1" // create a factory f := &factorymocks.Factory{} cmd := NewCreateCommand(f, "") - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - kbclient := clientfake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - - bk := &velerov1api.Backup{} - backups.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) + kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + f.On("Namespace").Return(mock.Anything) - f.On("KubebuilderClient").Return(kbclient, nil) + f.On("KubebuilderWatchClient").Return(kbclient, nil) flags := new(flag.FlagSet) o := NewCreateOptions() @@ -301,18 +290,14 @@ func TestCreateCommand(t *testing.T) { // create a factory f := &factorymocks.Factory{} cmd := NewCreateCommand(f, "") - vsls := &velerov1mocks.VolumeSnapshotLocationInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - kbclient := clientfake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - - vsl := &velerov1api.VolumeSnapshotLocation{} - vsls.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(vsl, nil) - veleroV1.On("VolumeSnapshotLocations", mock.Anything).Return(vsls, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) - f.On("KubebuilderClient").Return(kbclient, nil) + kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + + vsl := builder.ForVolumeSnapshotLocation(cmdtest.VeleroNameSpace, vslName).Result() + + kbclient.Create(cmd.Context(), vsl, &controllerclient.CreateOptions{}) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderWatchClient").Return(kbclient, nil) flags := new(flag.FlagSet) o := NewCreateOptions() @@ -343,22 +328,13 @@ func TestCreateCommand(t *testing.T) { fromSchedule := "schedule-name-1" flags.Parse([]string{"--from-schedule", fromSchedule}) - backups := &velerov1mocks.BackupInterface{} - bk := &velerov1api.Backup{} - schedules := &velerov1mocks.ScheduleInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - kbclient := clientfake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - sd := &velerov1api.Schedule{} - - backups.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - schedules.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(sd, nil) - veleroV1.On("Schedules", mock.Anything).Return(schedules, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) - f.On("KubebuilderClient").Return(kbclient, nil) + kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + + schedule := builder.ForSchedule(cmdtest.VeleroNameSpace, fromSchedule).Result() + kbclient.Create(context.Background(), schedule, &controllerclient.CreateOptions{}) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderWatchClient").Return(kbclient, nil) e := o.Complete(args, f) assert.NoError(t, e) diff --git a/pkg/cmd/cli/backup/delete.go b/pkg/cmd/cli/backup/delete.go index a3e816bbcc..5d235bd72f 100644 --- a/pkg/cmd/cli/backup/delete.go +++ b/pkg/cmd/cli/backup/delete.go @@ -25,12 +25,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" kubeerrs "k8s.io/apimachinery/pkg/util/errors" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "github.com/vmware-tanzu/velero/pkg/backup" + "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/cli" + "github.com/vmware-tanzu/velero/pkg/label" ) // NewDeleteCommand creates a new command that deletes a backup. @@ -82,7 +84,8 @@ func Run(o *cli.DeleteOptions) error { switch { case len(o.Names) > 0: for _, name := range o.Names { - backup, err := o.Client.VeleroV1().Backups(o.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) + backup := new(velerov1api.Backup) + err := o.Client.Get(context.TODO(), controllerclient.ObjectKey{Namespace: o.Namespace, Name: name}, backup) if err != nil { errs = append(errs, errors.WithStack(err)) continue @@ -91,17 +94,22 @@ func Run(o *cli.DeleteOptions) error { backups = append(backups, backup) } default: - selector := labels.Everything().String() + selector := labels.Everything() if o.Selector.LabelSelector != nil { - selector = o.Selector.String() + convertedSelector, err := metav1.LabelSelectorAsSelector(o.Selector.LabelSelector) + if err != nil { + return errors.WithStack(err) + } + selector = convertedSelector } - res, err := o.Client.VeleroV1().Backups(o.Namespace).List(context.TODO(), metav1.ListOptions{LabelSelector: selector}) + backupList := new(velerov1api.BackupList) + err := o.Client.List(context.TODO(), backupList, &controllerclient.ListOptions{LabelSelector: selector, Namespace: o.Namespace}) if err != nil { return errors.WithStack(err) } - for i := range res.Items { - backups = append(backups, &res.Items[i]) + for i := range backupList.Items { + backups = append(backups, &backupList.Items[i]) } } @@ -112,9 +120,11 @@ func Run(o *cli.DeleteOptions) error { // create a backup deletion request for each for _, b := range backups { - deleteRequest := backup.NewDeleteBackupRequest(b.Name, string(b.UID)) + deleteRequest := builder.ForDeleteBackupRequest(o.Namespace, "").BackupName(b.Name). + ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, label.GetValidName(b.Name), + velerov1api.BackupUIDLabel, string(b.UID)), builder.WithGenerateName(b.Name+"-")).Result() - if _, err := o.Client.VeleroV1().DeleteBackupRequests(o.Namespace).Create(context.TODO(), deleteRequest, metav1.CreateOptions{}); err != nil { + if err := o.Client.Create(context.TODO(), deleteRequest, &controllerclient.CreateOptions{}); err != nil { errs = append(errs, err) continue } diff --git a/pkg/cmd/cli/backup/delete_test.go b/pkg/cmd/cli/backup/delete_test.go index 7d479aaaca..0d34bc5a10 100644 --- a/pkg/cmd/cli/backup/delete_test.go +++ b/pkg/cmd/cli/backup/delete_test.go @@ -17,68 +17,60 @@ limitations under the License. package backup import ( + "context" "fmt" "os" "os/exec" "testing" flag "github.com/spf13/pflag" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" "github.com/vmware-tanzu/velero/pkg/cmd/cli" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" + velerotest "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" ) func TestDeleteCommand(t *testing.T) { - backupName := "backup-name-1" + backup1 := "backup-name-1" + backup2 := "backup-name-2" // create a factory f := &factorymocks.Factory{} - deleteBackupRequest := &velerov1mocks.DeleteBackupRequestInterface{} - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - dbr := &velerov1api.DeleteBackupRequest{} - backups.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - deleteBackupRequest.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(dbr, nil) - veleroV1.On("DeleteBackupRequests", mock.Anything).Return(deleteBackupRequest, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) + client := velerotest.NewFakeControllerRuntimeClient(t) + client.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, backup1).Result(), &controllerclient.CreateOptions{}) + client.Create(context.Background(), builder.ForBackup("default", backup2).Result(), &controllerclient.CreateOptions{}) + + f.On("KubebuilderClient").Return(client, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) // create command c := NewDeleteCommand(f, "velero backup delete") - c.SetArgs([]string{backupName}) - assert.Equal(t, "Delete backups", c.Short) + c.SetArgs([]string{backup1, backup2}) + require.Equal(t, "Delete backups", c.Short) o := cli.NewDeleteOptions("backup") flags := new(flag.FlagSet) o.BindFlags(flags) flags.Parse([]string{"--confirm"}) - args := []string{"bk1", "bk2"} + args := []string{backup1, backup2} - bk.Name = backupName e := o.Complete(f, args) - assert.Equal(t, e, nil) + require.Equal(t, nil, e) e = o.Validate(c, f, args) - assert.Equal(t, e, nil) + require.Equal(t, nil, e) - e = Run(o) - assert.Equal(t, e, nil) + Run(o) e = c.Execute() - assert.Equal(t, e, nil) + require.Equal(t, nil, e) if os.Getenv(cmdtest.CaptureFlag) == "1" { return @@ -87,10 +79,7 @@ func TestDeleteCommand(t *testing.T) { cmd := exec.Command(os.Args[0], []string{"-test.run=TestDeleteCommand"}...) cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) stdout, _, err := veleroexec.RunCommand(cmd) - - if err == nil { - assert.Contains(t, stdout, fmt.Sprintf("Request to delete backup \"%s\" submitted successfully.", backupName)) - return + if err != nil { + require.Contains(t, stdout, fmt.Sprintf("backups.velero.io \"%s\" not found.", backup2)) } - t.Fatalf("process ran with err %v, want backups by get()", err) } diff --git a/pkg/cmd/cli/backup/describe.go b/pkg/cmd/cli/backup/describe.go index e6af647dfc..7d26dd963e 100644 --- a/pkg/cmd/cli/backup/describe.go +++ b/pkg/cmd/cli/backup/describe.go @@ -21,14 +21,14 @@ import ( "fmt" "os" - "github.com/spf13/cobra" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1" snapshotv1client "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned" + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - pkgbackup "github.com/vmware-tanzu/velero/pkg/backup" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/output" @@ -54,9 +54,6 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { Use: use + " [NAME1] [NAME2] [NAME...]", Short: "Describe backups", Run: func(c *cobra.Command, args []string) { - veleroClient, err := f.Client() - cmd.CheckError(err) - kbClient, err := f.KubebuilderClient() cmd.CheckError(err) @@ -64,29 +61,37 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { cmd.CheckError(fmt.Errorf("invalid output format '%s'. valid value are 'plaintext, json'", outputFormat)) } - var backups *velerov1api.BackupList + backups := new(velerov1api.BackupList) if len(args) > 0 { - backups = new(velerov1api.BackupList) for _, name := range args { - backup, err := veleroClient.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{}) + backup := new(velerov1api.Backup) + err := kbClient.Get(context.TODO(), controllerclient.ObjectKey{Namespace: f.Namespace(), Name: name}, backup) cmd.CheckError(err) backups.Items = append(backups.Items, *backup) } } else { - backups, err = veleroClient.VeleroV1().Backups(f.Namespace()).List(context.TODO(), listOptions) + parsedSelector, err := labels.Parse(listOptions.LabelSelector) + cmd.CheckError(err) + err = kbClient.List(context.TODO(), backups, &controllerclient.ListOptions{LabelSelector: parsedSelector, Namespace: f.Namespace()}) cmd.CheckError(err) } first := true for i, backup := range backups.Items { - deleteRequestListOptions := pkgbackup.NewDeleteBackupRequestListOptions(backup.Name, string(backup.UID)) - deleteRequestList, err := veleroClient.VeleroV1().DeleteBackupRequests(f.Namespace()).List(context.TODO(), deleteRequestListOptions) + deleteRequestList := new(velerov1api.DeleteBackupRequestList) + err := kbClient.List(context.TODO(), deleteRequestList, &controllerclient.ListOptions{ + Namespace: f.Namespace(), + LabelSelector: labels.SelectorFromSet(map[string]string{velerov1api.BackupNameLabel: label.GetValidName(backup.Name), velerov1api.BackupUIDLabel: string(backup.UID)}), + }) if err != nil { fmt.Fprintf(os.Stderr, "error getting DeleteBackupRequests for backup %s: %v\n", backup.Name, err) } - opts := label.NewListOptionsForBackup(backup.Name) - podVolumeBackupList, err := veleroClient.VeleroV1().PodVolumeBackups(f.Namespace()).List(context.TODO(), opts) + podVolumeBackupList := new(velerov1api.PodVolumeBackupList) + err = kbClient.List(context.TODO(), podVolumeBackupList, &controllerclient.ListOptions{ + Namespace: f.Namespace(), + LabelSelector: labels.SelectorFromSet(map[string]string{velerov1api.BackupNameLabel: label.GetValidName(backup.Name)}), + }) if err != nil { fmt.Fprintf(os.Stderr, "error getting PodVolumeBackups for backup %s: %v\n", backup.Name, err) } @@ -101,6 +106,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { csiClient, err = snapshotv1client.NewForConfig(clientConfig) cmd.CheckError(err) + opts := label.NewListOptionsForBackup(backup.Name) vscList, err = csiClient.SnapshotV1().VolumeSnapshotContents().List(context.TODO(), opts) if err != nil { fmt.Fprintf(os.Stderr, "error getting VolumeSnapshotContent objects for backup %s: %v\n", backup.Name, err) @@ -110,10 +116,10 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { // structured output only applies to a single backup in case of OOM // To describe the list of backups in structured format, users could iterate over the list and describe backup one after another. if len(backups.Items) == 1 && outputFormat != "plaintext" { - s := output.DescribeBackupInSF(context.Background(), kbClient, &backups.Items[i], deleteRequestList.Items, podVolumeBackupList.Items, vscList.Items, details, veleroClient, insecureSkipTLSVerify, caCertFile, outputFormat) + s := output.DescribeBackupInSF(context.Background(), kbClient, &backups.Items[i], deleteRequestList.Items, podVolumeBackupList.Items, vscList.Items, details, insecureSkipTLSVerify, caCertFile, outputFormat) fmt.Print(s) } else { - s := output.DescribeBackup(context.Background(), kbClient, &backups.Items[i], deleteRequestList.Items, podVolumeBackupList.Items, vscList.Items, details, veleroClient, insecureSkipTLSVerify, caCertFile) + s := output.DescribeBackup(context.Background(), kbClient, &backups.Items[i], deleteRequestList.Items, podVolumeBackupList.Items, vscList.Items, details, insecureSkipTLSVerify, caCertFile) if first { first = false fmt.Print(s) diff --git a/pkg/cmd/cli/backup/describe_test.go b/pkg/cmd/cli/backup/describe_test.go index d90285dc03..3272c68009 100644 --- a/pkg/cmd/cli/backup/describe_test.go +++ b/pkg/cmd/cli/backup/describe_test.go @@ -17,55 +17,37 @@ limitations under the License. package backup import ( + "context" "fmt" "os" "os/exec" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "k8s.io/client-go/rest" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" "github.com/vmware-tanzu/velero/pkg/features" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" + "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" ) func TestNewDescribeCommand(t *testing.T) { // create a factory f := &factorymocks.Factory{} + backupName := "bk-describe-1" + testBackup := builder.ForBackup(cmdtest.VeleroNameSpace, backupName).Result() - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} clientConfig := rest.Config{} + kbClient := test.NewFakeControllerRuntimeClient(t) + kbClient.Create(context.Background(), testBackup, &controllerclient.CreateOptions{}) - deleteBackupRequest := &velerov1mocks.DeleteBackupRequestInterface{} - bk := &velerov1api.Backup{} - bkList := &velerov1api.BackupList{} - deleteBackupRequestList := &velerov1api.DeleteBackupRequestList{} - podVolumeBackups := &velerov1mocks.PodVolumeBackupInterface{} - podVolumeBackupList := &velerov1api.PodVolumeBackupList{} - - backupName := "bk-describe-1" - bk.Name = backupName - - backups.On("List", mock.Anything, mock.Anything).Return(bkList, nil) - backups.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - deleteBackupRequest.On("List", mock.Anything, mock.Anything).Return(deleteBackupRequestList, nil) - veleroV1.On("DeleteBackupRequests", mock.Anything).Return(deleteBackupRequest, nil) - podVolumeBackups.On("List", mock.Anything, mock.Anything).Return(podVolumeBackupList, nil) - veleroV1.On("PodVolumeBackups", mock.Anything, mock.Anything).Return(podVolumeBackups, nil) - client.On("VeleroV1").Return(veleroV1, nil) f.On("ClientConfig").Return(&clientConfig, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) - f.On("KubebuilderClient").Return(nil, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(kbClient, nil) // create command c := NewDescribeCommand(f, "velero backup describe") @@ -74,7 +56,7 @@ func TestNewDescribeCommand(t *testing.T) { features.NewFeatureFlagSet("EnableCSI") defer features.NewFeatureFlagSet() - c.SetArgs([]string{"bk1"}) + c.SetArgs([]string{backupName}) e := c.Execute() assert.NoError(t, e) diff --git a/pkg/cmd/cli/backup/download.go b/pkg/cmd/cli/backup/download.go index dec0864e02..5d2fec546c 100644 --- a/pkg/cmd/cli/backup/download.go +++ b/pkg/cmd/cli/backup/download.go @@ -26,7 +26,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -84,10 +84,11 @@ func (o *DownloadOptions) BindFlags(flags *pflag.FlagSet) { } func (o *DownloadOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { - veleroClient, err := f.Client() + kbClient, err := f.KubebuilderClient() cmd.CheckError(err) - if _, err := veleroClient.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), o.Name, metav1.GetOptions{}); err != nil { + backup := new(velerov1api.Backup) + if err := kbClient.Get(context.TODO(), controllerclient.ObjectKey{Namespace: f.Namespace(), Name: o.Name}, backup); err != nil { return err } diff --git a/pkg/cmd/cli/backup/download_test.go b/pkg/cmd/cli/backup/download_test.go index 02bb4ba30d..3a28b73f76 100644 --- a/pkg/cmd/cli/backup/download_test.go +++ b/pkg/cmd/cli/backup/download_test.go @@ -17,6 +17,7 @@ limitations under the License. package backup import ( + "context" "fmt" "os" "os/exec" @@ -25,35 +26,29 @@ import ( flag "github.com/spf13/pflag" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/vmware-tanzu/velero/pkg/builder" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" ) func TestNewDownloadCommand(t *testing.T) { - // create a factory f := &factorymocks.Factory{} - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} + backupName := "backup-1" kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + err := kbclient.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, backupName).Result()) + require.NoError(t, err) + err = kbclient.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, "bk-to-be-download").Result()) + require.NoError(t, err) - backups.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) f.On("KubebuilderClient").Return(kbclient, nil) // create command @@ -78,7 +73,6 @@ func TestNewDownloadCommand(t *testing.T) { flags.Parse([]string{fmt.Sprintf("--insecure-skip-tls-verify=%s", strconv.FormatBool(insecureSkipTlsVerify))}) flags.Parse([]string{"--cacert", cacert}) - backupName := "backup-1" args := []string{backupName, "arg2"} e := o.Complete(args) @@ -105,7 +99,7 @@ func TestNewDownloadCommand(t *testing.T) { _, stderr, err := veleroexec.RunCommand(cmd) if err != nil { - assert.Contains(t, stderr, "download request download url timeout") + require.Contains(t, stderr, "download request download url timeout") return } t.Fatalf("process ran with err %v, want backup delete successfully", err) diff --git a/pkg/cmd/cli/backuplocation/delete_test.go b/pkg/cmd/cli/backuplocation/delete_test.go index 4fe9dbc1aa..9736c9883a 100644 --- a/pkg/cmd/cli/backuplocation/delete_test.go +++ b/pkg/cmd/cli/backuplocation/delete_test.go @@ -26,14 +26,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" "github.com/vmware-tanzu/velero/pkg/cmd/cli" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" + velerotest "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" ) @@ -41,9 +38,7 @@ func TestNewDeleteCommand(t *testing.T) { // create a factory f := &factorymocks.Factory{} - client := &versionedmocks.Interface{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - f.On("Client").Return(client, nil) + kbclient := velerotest.NewFakeControllerRuntimeClient(t) f.On("Namespace").Return(mock.Anything) f.On("KubebuilderClient").Return(kbclient, nil) @@ -86,9 +81,7 @@ func TestDeleteFunctions(t *testing.T) { //t.Run("create the other create command with fromSchedule option for Run() other branches", func(t *testing.T) { // create a factory f := &factorymocks.Factory{} - client := &versionedmocks.Interface{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - f.On("Client").Return(client, nil) + kbclient := velerotest.NewFakeControllerRuntimeClient(t) f.On("Namespace").Return(mock.Anything) f.On("KubebuilderClient").Return(kbclient, nil) diff --git a/pkg/cmd/cli/backuplocation/get_test.go b/pkg/cmd/cli/backuplocation/get_test.go index 34781c1b56..2e4c5510cb 100644 --- a/pkg/cmd/cli/backuplocation/get_test.go +++ b/pkg/cmd/cli/backuplocation/get_test.go @@ -24,11 +24,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "sigs.k8s.io/controller-runtime/pkg/client/fake" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" + velerotest "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" ) @@ -36,7 +35,7 @@ func TestNewGetCommand(t *testing.T) { bkList := []string{"b1", "b2"} f := &factorymocks.Factory{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + kbclient := velerotest.NewFakeControllerRuntimeClient(t) f.On("Namespace").Return(mock.Anything) f.On("KubebuilderClient").Return(kbclient, nil) diff --git a/pkg/cmd/cli/delete_options.go b/pkg/cmd/cli/delete_options.go index c6197a428c..1a5d711dc5 100644 --- a/pkg/cmd/cli/delete_options.go +++ b/pkg/cmd/cli/delete_options.go @@ -25,16 +25,16 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/vmware-tanzu/velero/pkg/client" - clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" ) // DeleteOptions contains parameters used for deleting a restore. type DeleteOptions struct { *SelectOptions Confirm bool - Client clientset.Interface + Client controllerclient.Client Namespace string } @@ -47,7 +47,7 @@ func NewDeleteOptions(singularTypeName string) *DeleteOptions { // Complete fills in the correct values for all the options. func (o *DeleteOptions) Complete(f client.Factory, args []string) error { o.Namespace = f.Namespace() - client, err := f.Client() + client, err := f.KubebuilderClient() if err != nil { return err } diff --git a/pkg/cmd/cli/restore/delete.go b/pkg/cmd/cli/restore/delete.go index 42479ced28..cd180d507e 100644 --- a/pkg/cmd/cli/restore/delete.go +++ b/pkg/cmd/cli/restore/delete.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" kubeerrs "k8s.io/apimachinery/pkg/util/errors" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -76,7 +77,8 @@ func Run(o *cli.DeleteOptions) error { switch { case len(o.Names) > 0: for _, name := range o.Names { - restore, err := o.Client.VeleroV1().Restores(o.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) + restore := new(velerov1api.Restore) + err := o.Client.Get(context.TODO(), controllerclient.ObjectKey{Namespace: o.Namespace, Name: name}, restore) if err != nil { errs = append(errs, errors.WithStack(err)) continue @@ -84,27 +86,39 @@ func Run(o *cli.DeleteOptions) error { restores = append(restores, restore) } default: - selector := labels.Everything().String() + selector := labels.Everything() if o.Selector.LabelSelector != nil { - selector = o.Selector.String() + convertedSelector, err := metav1.LabelSelectorAsSelector(o.Selector.LabelSelector) + if err != nil { + return errors.WithStack(err) + } + selector = convertedSelector } - res, err := o.Client.VeleroV1().Restores(o.Namespace).List(context.TODO(), metav1.ListOptions{ + restoreList := new(velerov1api.RestoreList) + err := o.Client.List(context.TODO(), restoreList, &controllerclient.ListOptions{ + Namespace: o.Namespace, LabelSelector: selector, }) if err != nil { errs = append(errs, errors.WithStack(err)) } - for i := range res.Items { - restores = append(restores, &res.Items[i]) + for i := range restoreList.Items { + restores = append(restores, &restoreList.Items[i]) } } + + if len(errs) > 0 { + fmt.Println("errs: ", errs) + return kubeerrs.NewAggregate(errs) + } + if len(restores) == 0 { fmt.Println("No restores found") return nil } for _, r := range restores { - err := o.Client.VeleroV1().Restores(r.Namespace).Delete(context.TODO(), r.Name, metav1.DeleteOptions{}) + err := o.Client.Delete(context.TODO(), r, &controllerclient.DeleteOptions{}) if err != nil { errs = append(errs, errors.WithStack(err)) continue diff --git a/pkg/cmd/cli/restore/describe.go b/pkg/cmd/cli/restore/describe.go index 88514e8651..10aca7e025 100644 --- a/pkg/cmd/cli/restore/describe.go +++ b/pkg/cmd/cli/restore/describe.go @@ -75,7 +75,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { fmt.Fprintf(os.Stderr, "error getting PodVolumeRestores for restore %s: %v\n", restore.Name, err) } - s := output.DescribeRestore(context.Background(), kbClient, &restores.Items[i], podvolumeRestoreList.Items, details, veleroClient, insecureSkipTLSVerify, caCertFile) + s := output.DescribeRestore(context.Background(), kbClient, &restores.Items[i], podvolumeRestoreList.Items, details, insecureSkipTLSVerify, caCertFile) if first { first = false fmt.Print(s) diff --git a/pkg/cmd/cli/schedule/delete.go b/pkg/cmd/cli/schedule/delete.go index af7c7c28d4..883a3aaf32 100644 --- a/pkg/cmd/cli/schedule/delete.go +++ b/pkg/cmd/cli/schedule/delete.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" kubeerrs "k8s.io/apimachinery/pkg/util/errors" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -76,7 +77,8 @@ func Run(o *cli.DeleteOptions) error { switch { case len(o.Names) > 0: for _, name := range o.Names { - schedule, err := o.Client.VeleroV1().Schedules(o.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) + schedule := new(velerov1api.Schedule) + err := o.Client.Get(context.TODO(), controllerclient.ObjectKey{Namespace: o.Namespace, Name: name}, schedule) if err != nil { errs = append(errs, errors.WithStack(err)) continue @@ -84,19 +86,25 @@ func Run(o *cli.DeleteOptions) error { schedules = append(schedules, schedule) } default: - selector := labels.Everything().String() + selector := labels.Everything() if o.Selector.LabelSelector != nil { - selector = o.Selector.String() + convertedSelector, err := metav1.LabelSelectorAsSelector(o.Selector.LabelSelector) + if err != nil { + return errors.WithStack(err) + } + selector = convertedSelector } - res, err := o.Client.VeleroV1().Schedules(o.Namespace).List(context.TODO(), metav1.ListOptions{ + scheduleList := new(velerov1api.ScheduleList) + err := o.Client.List(context.TODO(), scheduleList, &controllerclient.ListOptions{ + Namespace: o.Namespace, LabelSelector: selector, }) if err != nil { errs = append(errs, errors.WithStack(err)) } - for i := range res.Items { - schedules = append(schedules, &res.Items[i]) + for i := range scheduleList.Items { + schedules = append(schedules, &scheduleList.Items[i]) } } if len(schedules) == 0 { @@ -105,7 +113,7 @@ func Run(o *cli.DeleteOptions) error { } for _, s := range schedules { - err := o.Client.VeleroV1().Schedules(s.Namespace).Delete(context.TODO(), s.Name, metav1.DeleteOptions{}) + err := o.Client.Delete(context.TODO(), s, &controllerclient.DeleteOptions{}) if err != nil { errs = append(errs, errors.WithStack(err)) continue diff --git a/pkg/cmd/cli/version/version_test.go b/pkg/cmd/cli/version/version_test.go index 90e6e811b9..355626802f 100644 --- a/pkg/cmd/cli/version/version_test.go +++ b/pkg/cmd/cli/version/version_test.go @@ -25,12 +25,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" kbclient "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" "github.com/vmware-tanzu/velero/pkg/builder" "github.com/vmware-tanzu/velero/pkg/buildinfo" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) func TestPrintVersion(t *testing.T) { @@ -83,7 +82,7 @@ func TestPrintVersion(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { var ( - kbClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + kbClient = velerotest.NewFakeControllerRuntimeClient(t) serverStatusGetter = new(mockServerStatusGetter) buf = new(bytes.Buffer) ) diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index 23ca389760..ec3878b626 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -36,7 +36,6 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest" "github.com/vmware-tanzu/velero/pkg/features" - clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" "github.com/vmware-tanzu/velero/pkg/itemoperation" "github.com/vmware-tanzu/velero/pkg/util/collections" @@ -53,7 +52,6 @@ func DescribeBackup( podVolumeBackups []velerov1api.PodVolumeBackup, volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent, details bool, - veleroClient clientset.Interface, insecureSkipTLSVerify bool, caCertFile string, ) string { @@ -106,7 +104,7 @@ func DescribeBackup( DescribeBackupSpec(d, backup.Spec) d.Println() - DescribeBackupStatus(ctx, kbClient, d, backup, details, veleroClient, insecureSkipTLSVerify, caCertFile) + DescribeBackupStatus(ctx, kbClient, d, backup, details, insecureSkipTLSVerify, caCertFile) if len(deleteRequests) > 0 { d.Println() @@ -300,7 +298,7 @@ func DescribeBackupSpec(d *Describer, spec velerov1api.BackupSpec) { } // DescribeBackupStatus describes a backup status in human-readable format. -func DescribeBackupStatus(ctx context.Context, kbClient kbclient.Client, d *Describer, backup *velerov1api.Backup, details bool, veleroClient clientset.Interface, insecureSkipTLSVerify bool, caCertPath string) { +func DescribeBackupStatus(ctx context.Context, kbClient kbclient.Client, d *Describer, backup *velerov1api.Backup, details bool, insecureSkipTLSVerify bool, caCertPath string) { status := backup.Status // Status.Version has been deprecated, use Status.FormatVersion diff --git a/pkg/cmd/util/output/backup_structured_describer.go b/pkg/cmd/util/output/backup_structured_describer.go index cecab22cbc..4a69fc057c 100644 --- a/pkg/cmd/util/output/backup_structured_describer.go +++ b/pkg/cmd/util/output/backup_structured_describer.go @@ -33,7 +33,6 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest" "github.com/vmware-tanzu/velero/pkg/features" - clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" "github.com/vmware-tanzu/velero/pkg/util/results" "github.com/vmware-tanzu/velero/pkg/volume" ) @@ -47,7 +46,6 @@ func DescribeBackupInSF( podVolumeBackups []velerov1api.PodVolumeBackup, volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent, details bool, - veleroClient clientset.Interface, insecureSkipTLSVerify bool, caCertFile string, outputFormat string, @@ -70,7 +68,7 @@ func DescribeBackupInSF( DescribeBackupSpecInSF(d, backup.Spec) - DescribeBackupStatusInSF(ctx, kbClient, d, backup, details, veleroClient, insecureSkipTLSVerify, caCertFile) + DescribeBackupStatusInSF(ctx, kbClient, d, backup, details, insecureSkipTLSVerify, caCertFile) if len(deleteRequests) > 0 { DescribeDeleteBackupRequestsInSF(d, deleteRequests) @@ -236,7 +234,7 @@ func DescribeBackupSpecInSF(d *StructuredDescriber, spec velerov1api.BackupSpec) } // DescribeBackupStatusInSF describes a backup status in structured format. -func DescribeBackupStatusInSF(ctx context.Context, kbClient kbclient.Client, d *StructuredDescriber, backup *velerov1api.Backup, details bool, veleroClient clientset.Interface, insecureSkipTLSVerify bool, caCertPath string) { +func DescribeBackupStatusInSF(ctx context.Context, kbClient kbclient.Client, d *StructuredDescriber, backup *velerov1api.Backup, details bool, insecureSkipTLSVerify bool, caCertPath string) { status := backup.Status backupStatusInfo := make(map[string]interface{}) diff --git a/pkg/cmd/util/output/restore_describer.go b/pkg/cmd/util/output/restore_describer.go index 8edd7c1846..13ac580b54 100644 --- a/pkg/cmd/util/output/restore_describer.go +++ b/pkg/cmd/util/output/restore_describer.go @@ -31,12 +31,11 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest" - clientset "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" "github.com/vmware-tanzu/velero/pkg/itemoperation" "github.com/vmware-tanzu/velero/pkg/util/results" ) -func DescribeRestore(ctx context.Context, kbClient kbclient.Client, restore *velerov1api.Restore, podVolumeRestores []velerov1api.PodVolumeRestore, details bool, veleroClient clientset.Interface, insecureSkipTLSVerify bool, caCertFile string) string { +func DescribeRestore(ctx context.Context, kbClient kbclient.Client, restore *velerov1api.Restore, podVolumeRestores []velerov1api.PodVolumeRestore, details bool, insecureSkipTLSVerify bool, caCertFile string) string { return Describe(func(d *Describer) { d.DescribeMetadata(restore.ObjectMeta) diff --git a/pkg/util/kube/list_watch.go b/pkg/util/kube/list_watch.go new file mode 100644 index 0000000000..5497d99ddd --- /dev/null +++ b/pkg/util/kube/list_watch.go @@ -0,0 +1,41 @@ +/* +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 kube + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/watch" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +type InternalLW struct { + Client kbclient.WithWatch + Namespace string + ObjectList kbclient.ObjectList +} + +func (lw *InternalLW) Watch(options metav1.ListOptions) (watch.Interface, error) { + return lw.Client.Watch(context.Background(), lw.ObjectList, &kbclient.ListOptions{Raw: &options, Namespace: lw.Namespace}) +} + +func (lw *InternalLW) List(options metav1.ListOptions) (runtime.Object, error) { + err := lw.Client.List(context.Background(), lw.ObjectList, &kbclient.ListOptions{Raw: &options, Namespace: lw.Namespace}) + return lw.ObjectList, err +} From b5bd55fc56788960dbebd5df4e0cc39d86014a3c Mon Sep 17 00:00:00 2001 From: lyndon <98304688+Lyndon-Li@users.noreply.github.com> Date: Sat, 15 Jul 2023 02:05:51 +0800 Subject: [PATCH 11/14] fix issue 6490 (#6491) Signed-off-by: Lyndon-Li --- changelogs/unreleased/6491-Lyndon-Li | 1 + pkg/controller/backup_operations_controller.go | 7 ++++--- pkg/controller/restore_operations_controller.go | 7 ++++--- 3 files changed, 9 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/6491-Lyndon-Li diff --git a/changelogs/unreleased/6491-Lyndon-Li b/changelogs/unreleased/6491-Lyndon-Li new file mode 100644 index 0000000000..afadb6bf02 --- /dev/null +++ b/changelogs/unreleased/6491-Lyndon-Li @@ -0,0 +1 @@ +Fix issue 6490, If a backup/restore has multiple async operations and one operation fails while others are still in-progress, when all the operations finish, the backup/restore will be set as Completed falsely \ No newline at end of file diff --git a/pkg/controller/backup_operations_controller.go b/pkg/controller/backup_operations_controller.go index d6d491c5fc..f00e9c2056 100644 --- a/pkg/controller/backup_operations_controller.go +++ b/pkg/controller/backup_operations_controller.go @@ -185,14 +185,15 @@ func (c *backupOperationsReconciler) Reconcile(ctx context.Context, req ctrl.Req operations.ChangesSinceUpdate = true } + if len(operations.ErrsSinceUpdate) > 0 { + backup.Status.Phase = velerov1api.BackupPhaseWaitingForPluginOperationsPartiallyFailed + } + // if stillInProgress is false, backup moves to finalize phase and needs update // if operations.ErrsSinceUpdate is not empty, then backup phase needs to change to // BackupPhaseWaitingForPluginOperationsPartiallyFailed and needs update // If the only changes are incremental progress, then no write is necessary, progress can remain in memory if !stillInProgress { - if len(operations.ErrsSinceUpdate) > 0 { - backup.Status.Phase = velerov1api.BackupPhaseWaitingForPluginOperationsPartiallyFailed - } if backup.Status.Phase == velerov1api.BackupPhaseWaitingForPluginOperations { log.Infof("Marking backup %s Finalizing", backup.Name) backup.Status.Phase = velerov1api.BackupPhaseFinalizing diff --git a/pkg/controller/restore_operations_controller.go b/pkg/controller/restore_operations_controller.go index 100fef2146..869c5acb1b 100644 --- a/pkg/controller/restore_operations_controller.go +++ b/pkg/controller/restore_operations_controller.go @@ -177,14 +177,15 @@ func (r *restoreOperationsReconciler) Reconcile(ctx context.Context, req ctrl.Re operations.ChangesSinceUpdate = true } + if len(operations.ErrsSinceUpdate) > 0 { + restore.Status.Phase = velerov1api.RestorePhaseWaitingForPluginOperationsPartiallyFailed + } + // if stillInProgress is false, restore moves to terminal phase and needs update // if operations.ErrsSinceUpdate is not empty, then restore phase needs to change to // RestorePhaseWaitingForPluginOperationsPartiallyFailed and needs update // If the only changes are incremental progress, then no write is necessary, progress can remain in memory if !stillInProgress { - if len(operations.ErrsSinceUpdate) > 0 { - restore.Status.Phase = velerov1api.RestorePhaseWaitingForPluginOperationsPartiallyFailed - } if restore.Status.Phase == velerov1api.RestorePhaseWaitingForPluginOperations { log.Infof("Marking restore %s completed", restore.Name) restore.Status.Phase = velerov1api.RestorePhaseCompleted From 9e515ac397541724f227ee644e219a8437871524 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Sat, 15 Jul 2023 14:57:30 +0800 Subject: [PATCH 12/14] Remove dependency of the legacy client code from pkg/cmd directory part 2. Signed-off-by: Xun Jiang --- changelogs/unreleased/6497-blackpiglet | 1 + pkg/client/factory_test.go | 4 + pkg/cmd/cli/backup/download_test.go | 8 +- pkg/cmd/cli/backup/get.go | 17 ++- pkg/cmd/cli/backup/get_test.go | 56 ++++--- pkg/cmd/cli/backup/logs.go | 94 ++++++++---- pkg/cmd/cli/backup/logs_test.go | 162 ++++++++++++++------ pkg/cmd/cli/backuplocation/create_test.go | 24 +-- pkg/cmd/cli/backuplocation/set_test.go | 36 +---- pkg/cmd/cli/restore/create.go | 39 +++-- pkg/cmd/cli/restore/create_test.go | 174 ++++++++++++++++++++++ pkg/cmd/cli/restore/restore_test.go | 34 +++++ pkg/util/kube/list_watch_test.go | 54 +++++++ 13 files changed, 536 insertions(+), 167 deletions(-) create mode 100644 changelogs/unreleased/6497-blackpiglet create mode 100644 pkg/cmd/cli/restore/create_test.go create mode 100644 pkg/cmd/cli/restore/restore_test.go create mode 100644 pkg/util/kube/list_watch_test.go diff --git a/changelogs/unreleased/6497-blackpiglet b/changelogs/unreleased/6497-blackpiglet new file mode 100644 index 0000000000..30337ae92b --- /dev/null +++ b/changelogs/unreleased/6497-blackpiglet @@ -0,0 +1 @@ +Remove dependency of the legacy client code from pkg/cmd directory part 2 \ No newline at end of file diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index e96b2b0449..547be6bff6 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -141,6 +141,10 @@ func TestFactory(t *testing.T) { kubebuilderClient, e := f.KubebuilderClient() assert.Contains(t, e.Error(), fmt.Sprintf("Get \"%s/api?timeout=", test.expectedHost)) assert.Nil(t, kubebuilderClient) + + kbClientWithWatch, e := f.KubebuilderWatchClient() + assert.Contains(t, e.Error(), fmt.Sprintf("Get \"%s/api?timeout=", test.expectedHost)) + assert.Nil(t, kbClientWithWatch) }) } } diff --git a/pkg/cmd/cli/backup/download_test.go b/pkg/cmd/cli/backup/download_test.go index 3a28b73f76..a709a814b8 100644 --- a/pkg/cmd/cli/backup/download_test.go +++ b/pkg/cmd/cli/backup/download_test.go @@ -27,14 +27,12 @@ import ( flag "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" + velerotest "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" - - factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" ) func TestNewDownloadCommand(t *testing.T) { @@ -42,7 +40,7 @@ func TestNewDownloadCommand(t *testing.T) { f := &factorymocks.Factory{} backupName := "backup-1" - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() + kbclient := velerotest.NewFakeControllerRuntimeClient(t) err := kbclient.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, backupName).Result()) require.NoError(t, err) err = kbclient.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, "bk-to-be-download").Result()) diff --git a/pkg/cmd/cli/backup/get.go b/pkg/cmd/cli/backup/get.go index f5ccc07e10..159fac30dd 100644 --- a/pkg/cmd/cli/backup/get.go +++ b/pkg/cmd/cli/backup/get.go @@ -21,6 +21,8 @@ import ( "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -38,19 +40,24 @@ func NewGetCommand(f client.Factory, use string) *cobra.Command { err := output.ValidateFlags(c) cmd.CheckError(err) - veleroClient, err := f.Client() + kbClient, err := f.KubebuilderClient() cmd.CheckError(err) - var backups *api.BackupList + backups := new(api.BackupList) if len(args) > 0 { - backups = new(api.BackupList) for _, name := range args { - backup, err := veleroClient.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{}) + backup := new(api.Backup) + err := kbClient.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: name}, backup) cmd.CheckError(err) backups.Items = append(backups.Items, *backup) } } else { - backups, err = veleroClient.VeleroV1().Backups(f.Namespace()).List(context.TODO(), listOptions) + parsedSelector, err := labels.Parse(listOptions.LabelSelector) + cmd.CheckError(err) + err = kbClient.List(context.TODO(), backups, &kbclient.ListOptions{ + LabelSelector: parsedSelector, + Namespace: f.Namespace(), + }) cmd.CheckError(err) } diff --git a/pkg/cmd/cli/backup/get_test.go b/pkg/cmd/cli/backup/get_test.go index fbbeb91913..af37953522 100644 --- a/pkg/cmd/cli/backup/get_test.go +++ b/pkg/cmd/cli/backup/get_test.go @@ -17,6 +17,7 @@ limitations under the License. package backup import ( + "context" "fmt" "os" "os/exec" @@ -24,15 +25,14 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" ) func TestNewGetCommand(t *testing.T) { @@ -41,18 +41,16 @@ func TestNewGetCommand(t *testing.T) { // create a factory f := &factorymocks.Factory{} - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - bkList := &velerov1api.BackupList{} + client := velerotest.NewFakeControllerRuntimeClient(t) - backups.On("List", mock.Anything, mock.Anything).Return(bkList, nil) - backups.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) + for _, backupName := range args { + backup := builder.ForBackup(cmdtest.VeleroNameSpace, backupName).ObjectMeta(builder.WithLabels("abc", "abc")).Result() + err := client.Create(context.Background(), backup, &kbclient.CreateOptions{}) + require.NoError(t, err) + } + + f.On("KubebuilderClient").Return(client, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) // create command c := NewGetCommand(f, "velero backup get") @@ -69,6 +67,28 @@ func TestNewGetCommand(t *testing.T) { cmd := exec.Command(os.Args[0], []string{"-test.run=TestNewGetCommand"}...) cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) stdout, _, err := veleroexec.RunCommand(cmd) + require.NoError(t, err) + + if err == nil { + output := strings.Split(stdout, "\n") + i := 0 + for _, line := range output { + if strings.Contains(line, "New") { + i++ + } + } + assert.Equal(t, len(args), i) + } + + d := NewGetCommand(f, "velero backup get") + c.SetArgs([]string{"-l", "abc=abc"}) + e = d.Execute() + assert.NoError(t, e) + + cmd = exec.Command(os.Args[0], []string{"-test.run=TestNewGetCommand"}...) + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) + stdout, _, err = veleroexec.RunCommand(cmd) + require.NoError(t, err) if err == nil { output := strings.Split(stdout, "\n") @@ -79,7 +99,5 @@ func TestNewGetCommand(t *testing.T) { } } assert.Equal(t, len(args), i) - return } - t.Fatalf("process ran with err %v, want backups by get()", err) } diff --git a/pkg/cmd/cli/backup/logs.go b/pkg/cmd/cli/backup/logs.go index 95c2387a31..0655eaa15f 100644 --- a/pkg/cmd/cli/backup/logs.go +++ b/pkg/cmd/cli/backup/logs.go @@ -23,8 +23,9 @@ import ( "time" "github.com/spf13/cobra" + "github.com/spf13/pflag" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -32,51 +33,84 @@ import ( "github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest" ) -func NewLogsCommand(f client.Factory) *cobra.Command { +type LogsOptions struct { + Timeout time.Duration + InsecureSkipTLSVerify bool + CaCertFile string + Client kbclient.Client + BackupName string +} + +func NewLogsOptions() LogsOptions { config, err := client.LoadConfig() if err != nil { fmt.Fprintf(os.Stderr, "WARNING: Error reading config file: %v\n", err) } - timeout := time.Minute - insecureSkipTLSVerify := false - caCertFile := config.CACertFile() + return LogsOptions{ + Timeout: time.Minute, + InsecureSkipTLSVerify: false, + CaCertFile: config.CACertFile(), + } +} + +func (l *LogsOptions) BindFlags(flags *pflag.FlagSet) { + flags.DurationVar(&l.Timeout, "timeout", l.Timeout, "How long to wait to receive logs.") + flags.BoolVar(&l.InsecureSkipTLSVerify, "insecure-skip-tls-verify", l.InsecureSkipTLSVerify, "If true, the object store's TLS certificate will not be checked for validity. This is insecure and susceptible to man-in-the-middle attacks. Not recommended for production.") + flags.StringVar(&l.CaCertFile, "cacert", l.CaCertFile, "Path to a certificate bundle to use when verifying TLS connections.") +} + +func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error { + backup := new(velerov1api.Backup) + err := l.Client.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: l.BackupName}, backup) + if apierrors.IsNotFound(err) { + return fmt.Errorf("backup %q does not exist", l.BackupName) + } else if err != nil { + return fmt.Errorf("error checking for backup %q: %v", l.BackupName, err) + } + + switch backup.Status.Phase { + case velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed, velerov1api.BackupPhaseFailed, velerov1api.BackupPhaseWaitingForPluginOperations, velerov1api.BackupPhaseWaitingForPluginOperationsPartiallyFailed: + // terminal and waiting for plugin operations phases, do nothing. + default: + return fmt.Errorf("logs for backup %q are not available until it's finished processing, please wait "+ + "until the backup has a phase of Completed or Failed and try again", l.BackupName) + } + + err = downloadrequest.Stream(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, os.Stdout, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile) + return err +} + +func (l *LogsOptions) Complete(args []string, f client.Factory) error { + if len(args) > 0 { + l.BackupName = args[0] + } + + kbClient, err := f.KubebuilderClient() + if err != nil { + return err + } + l.Client = kbClient + return nil +} + +func NewLogsCommand(f client.Factory) *cobra.Command { + l := NewLogsOptions() c := &cobra.Command{ Use: "logs BACKUP", Short: "Get backup logs", Args: cobra.ExactArgs(1), Run: func(c *cobra.Command, args []string) { - backupName := args[0] - - veleroClient, err := f.Client() + err := l.Complete(args, f) cmd.CheckError(err) - kbClient, err := f.KubebuilderClient() - cmd.CheckError(err) - - backup, err := veleroClient.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), backupName, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - cmd.Exit("Backup %q does not exist.", backupName) - } else if err != nil { - cmd.Exit("Error checking for backup %q: %v", backupName, err) - } - - switch backup.Status.Phase { - case velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed, velerov1api.BackupPhaseFailed, velerov1api.BackupPhaseWaitingForPluginOperations, velerov1api.BackupPhaseWaitingForPluginOperationsPartiallyFailed: - // terminal and waiting for plugin operations phases, do nothing. - default: - cmd.Exit("Logs for backup %q are not available until it's finished processing. Please wait "+ - "until the backup has a phase of Completed or Failed and try again.", backupName) - } - - err = downloadrequest.Stream(context.Background(), kbClient, f.Namespace(), backupName, velerov1api.DownloadTargetKindBackupLog, os.Stdout, timeout, insecureSkipTLSVerify, caCertFile) + err = l.Run(c, f) cmd.CheckError(err) }, } - c.Flags().DurationVar(&timeout, "timeout", timeout, "How long to wait to receive logs.") - c.Flags().BoolVar(&insecureSkipTLSVerify, "insecure-skip-tls-verify", insecureSkipTLSVerify, "If true, the object store's TLS certificate will not be checked for validity. This is insecure and susceptible to man-in-the-middle attacks. Not recommended for production.") - c.Flags().StringVar(&caCertFile, "cacert", caCertFile, "Path to a certificate bundle to use when verifying TLS connections.") + l.BindFlags(c.Flags()) + return c } diff --git a/pkg/cmd/cli/backup/logs_test.go b/pkg/cmd/cli/backup/logs_test.go index d4b98b8847..9bb95a7a8d 100644 --- a/pkg/cmd/cli/backup/logs_test.go +++ b/pkg/cmd/cli/backup/logs_test.go @@ -17,62 +17,130 @@ limitations under the License. package backup import ( + "context" "fmt" - "os" - "os/exec" + "strconv" "testing" + "time" + flag "github.com/spf13/pflag" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "sigs.k8s.io/controller-runtime/pkg/client/fake" + "github.com/stretchr/testify/require" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" - veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) func TestNewLogsCommand(t *testing.T) { - backupName := "bk-logs-1" - - // create a factory - f := &factorymocks.Factory{} - - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - bkList := &velerov1api.BackupList{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - - backups.On("List", mock.Anything, mock.Anything).Return(bkList, nil) - backups.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) - f.On("Namespace").Return(mock.Anything) - f.On("KubebuilderClient").Return(kbclient, nil) - - c := NewLogsCommand(f) - assert.Equal(t, "Get backup logs", c.Short) - - if os.Getenv(cmdtest.CaptureFlag) == "1" { - c.SetArgs([]string{backupName}) - e := c.Execute() - assert.NoError(t, e) - return - } - - cmd := exec.Command(os.Args[0], []string{"-test.run=TestNewLogsCommand"}...) - cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) - _, stderr, err := veleroexec.RunCommand(cmd) - - if err != nil { - assert.Contains(t, stderr, fmt.Sprintf("Logs for backup \"%s\" are not available until it's finished processing", backupName)) - return - } - t.Fatalf("process ran with err %v, want backup delete successfully", err) + t.Run("Flag test", func(t *testing.T) { + l := NewLogsOptions() + flags := new(flag.FlagSet) + l.BindFlags(flags) + + timeout := "1m0s" + insecureSkipTLSVerify := "true" + caCertFile := "testing" + + flags.Parse([]string{"--timeout", timeout}) + flags.Parse([]string{"--insecure-skip-tls-verify", insecureSkipTLSVerify}) + flags.Parse([]string{"--cacert", caCertFile}) + + require.Equal(t, timeout, l.Timeout.String()) + require.Equal(t, insecureSkipTLSVerify, strconv.FormatBool(l.InsecureSkipTLSVerify)) + require.Equal(t, caCertFile, l.CaCertFile) + }) + + t.Run("Backup not complete test", func(t *testing.T) { + backupName := "bk-logs-1" + + // create a factory + f := &factorymocks.Factory{} + + kbClient := velerotest.NewFakeControllerRuntimeClient(t) + backup := builder.ForBackup(cmdtest.VeleroNameSpace, backupName).Result() + err := kbClient.Create(context.Background(), backup, &kbclient.CreateOptions{}) + require.NoError(t, err) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(kbClient, nil) + + c := NewLogsCommand(f) + assert.Equal(t, "Get backup logs", c.Short) + + l := NewLogsOptions() + flags := new(flag.FlagSet) + l.BindFlags(flags) + err = l.Complete([]string{backupName}, f) + require.NoError(t, err) + + err = l.Run(c, f) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("logs for backup \"%s\" are not available until it's finished processing", backupName)) + }) + + t.Run("Backup not exist test", func(t *testing.T) { + backupName := "not-exist" + // create a factory + f := &factorymocks.Factory{} + + kbClient := velerotest.NewFakeControllerRuntimeClient(t) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(kbClient, nil) + + c := NewLogsCommand(f) + assert.Equal(t, "Get backup logs", c.Short) + + l := NewLogsOptions() + flags := new(flag.FlagSet) + l.BindFlags(flags) + err := l.Complete([]string{backupName}, f) + require.NoError(t, err) + + err = l.Run(c, f) + require.Error(t, err) + + require.Equal(t, fmt.Sprintf("backup \"%s\" does not exist", backupName), err.Error()) + }) + + t.Run("Normal backup log test", func(t *testing.T) { + backupName := "bk-logs-1" + + // create a factory + f := &factorymocks.Factory{} + + kbClient := velerotest.NewFakeControllerRuntimeClient(t) + backup := builder.ForBackup(cmdtest.VeleroNameSpace, backupName).Phase(velerov1api.BackupPhaseCompleted).Result() + err := kbClient.Create(context.Background(), backup, &kbclient.CreateOptions{}) + require.NoError(t, err) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(kbClient, nil) + + c := NewLogsCommand(f) + assert.Equal(t, "Get backup logs", c.Short) + + l := NewLogsOptions() + flags := new(flag.FlagSet) + l.BindFlags(flags) + err = l.Complete([]string{backupName}, f) + require.NoError(t, err) + + timeout := time.After(3 * time.Second) + done := make(chan bool) + go func() { + err = l.Run(c, f) + require.Error(t, err) + }() + + select { + case <-timeout: + t.Skip("Test didn't finish in time, because BSL is not in Available state.") + case <-done: + } + }) } diff --git a/pkg/cmd/cli/backuplocation/create_test.go b/pkg/cmd/cli/backuplocation/create_test.go index 34293fb889..dc2cb1aae0 100644 --- a/pkg/cmd/cli/backuplocation/create_test.go +++ b/pkg/cmd/cli/backuplocation/create_test.go @@ -29,16 +29,9 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - veleroflag "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" - "github.com/vmware-tanzu/velero/pkg/test" - - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" + veleroflag "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) func TestBuildBackupStorageLocationSetsNamespace(t *testing.T) { @@ -149,16 +142,7 @@ func TestCreateCommand_Run(t *testing.T) { args := []string{name, "arg2"} - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - - backups.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) + kbclient := velerotest.NewFakeControllerRuntimeClient(t) f.On("Namespace").Return(mock.Anything) f.On("KubebuilderClient").Return(kbclient, nil) @@ -179,7 +163,7 @@ func TestCreateCommand_Run(t *testing.T) { assert.Equal(t, backupSyncPeriod, o.BackupSyncPeriod.String()) assert.Equal(t, validationFrequency, o.ValidationFrequency.String()) assert.Equal(t, true, reflect.DeepEqual(bslConfig, o.Config)) - assert.Equal(t, true, test.CompareSlice(strings.Split(labels, ","), strings.Split(o.Labels.String(), ","))) + assert.Equal(t, true, velerotest.CompareSlice(strings.Split(labels, ","), strings.Split(o.Labels.String(), ","))) assert.Equal(t, caCertFile, o.CACertFile) assert.Equal(t, accessMode, o.AccessMode.String()) diff --git a/pkg/cmd/cli/backuplocation/set_test.go b/pkg/cmd/cli/backuplocation/set_test.go index d97327126c..b57f6a3208 100644 --- a/pkg/cmd/cli/backuplocation/set_test.go +++ b/pkg/cmd/cli/backuplocation/set_test.go @@ -27,16 +27,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" veleroflag "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" - - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" - versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks" - "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/scheme" - velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks" + velerotest "github.com/vmware-tanzu/velero/pkg/test" veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" ) @@ -44,16 +38,9 @@ func TestNewSetCommand(t *testing.T) { backupName := "arg2" // create a config for factory f := &factorymocks.Factory{} - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - - backups.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) + + kbclient := velerotest.NewFakeControllerRuntimeClient(t) + f.On("Namespace").Return(mock.Anything) f.On("KubebuilderClient").Return(kbclient, nil) @@ -97,16 +84,9 @@ func TestSetCommand_Execute(t *testing.T) { if os.Getenv(cmdtest.CaptureFlag) == "1" { // create a config for factory f := &factorymocks.Factory{} - backups := &velerov1mocks.BackupInterface{} - veleroV1 := &velerov1mocks.VeleroV1Interface{} - client := &versionedmocks.Interface{} - bk := &velerov1api.Backup{} - kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() - - backups.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil) - veleroV1.On("Backups", mock.Anything).Return(backups, nil) - client.On("VeleroV1").Return(veleroV1, nil) - f.On("Client").Return(client, nil) + + kbclient := velerotest.NewFakeControllerRuntimeClient(t) + f.On("Namespace").Return(mock.Anything) f.On("KubebuilderClient").Return(kbclient, nil) diff --git a/pkg/cmd/cli/restore/create.go b/pkg/cmd/cli/restore/create.go index 44db8eb8e1..39219a53ff 100644 --- a/pkg/cmd/cli/restore/create.go +++ b/pkg/cmd/cli/restore/create.go @@ -26,16 +26,17 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/tools/cache" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" "github.com/vmware-tanzu/velero/pkg/cmd" "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" "github.com/vmware-tanzu/velero/pkg/cmd/util/output" - veleroclient "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned" - v1 "github.com/vmware-tanzu/velero/pkg/generated/informers/externalversions/velero/v1" "github.com/vmware-tanzu/velero/pkg/util/boolptr" + "github.com/vmware-tanzu/velero/pkg/util/kube" ) func NewCreateCommand(f client.Factory, use string) *cobra.Command { @@ -93,8 +94,7 @@ type CreateOptions struct { Wait bool AllowPartiallyFailed flag.OptionalBool ItemOperationTimeout time.Duration - - client veleroclient.Interface + client kbclient.WithWatch } func NewCreateOptions() *CreateOptions { @@ -153,7 +153,7 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { o.RestoreName = fmt.Sprintf("%s-%s", sourceName, time.Now().Format("20060102150405")) } - client, err := f.Client() + client, err := f.KubebuilderWatchClient() if err != nil { return err } @@ -186,15 +186,20 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto switch { case o.BackupName != "": - if _, err := o.client.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), o.BackupName, metav1.GetOptions{}); err != nil { + backup := new(api.Backup) + if err := o.client.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: o.BackupName}, backup); err != nil { return err } case o.ScheduleName != "": - backupItems, err := o.client.VeleroV1().Backups(f.Namespace()).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", api.ScheduleNameLabel, o.ScheduleName)}) + backupList := new(api.BackupList) + err := o.client.List(context.TODO(), backupList, &kbclient.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{api.ScheduleNameLabel: o.ScheduleName}), + Namespace: f.Namespace(), + }) if err != nil { return err } - if len(backupItems.Items) == 0 { + if len(backupList.Items) == 0 { return errors.Errorf("No backups found for the schedule %s", o.ScheduleName) } } @@ -248,14 +253,18 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { // PartiallyFailed backup for the provided schedule, and use that specific backup // to restore from. if o.ScheduleName != "" && boolptr.IsSetToTrue(o.AllowPartiallyFailed.Value) { - backups, err := o.client.VeleroV1().Backups(f.Namespace()).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", api.ScheduleNameLabel, o.ScheduleName)}) + backupList := new(api.BackupList) + err := o.client.List(context.TODO(), backupList, &kbclient.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{api.ScheduleNameLabel: o.ScheduleName}), + Namespace: f.Namespace(), + }) if err != nil { return err } // if we find a Completed or PartiallyFailed backup for the schedule, restore specifically from that backup. If we don't // find one, proceed as-is -- the Velero server will handle validation. - if backup := mostRecentBackup(backups.Items, api.BackupPhaseCompleted, api.BackupPhasePartiallyFailed); backup != nil { + if backup := mostRecentBackup(backupList.Items, api.BackupPhaseCompleted, api.BackupPhasePartiallyFailed); backup != nil { // TODO(sk): this is kind of a hack -- we should revisit this and probably // move this logic to the server side or otherwise solve this problem. o.BackupName = backup.Name @@ -299,7 +308,6 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { return err } - var restoreInformer cache.SharedIndexInformer var updates chan *api.Restore if o.Wait { stop := make(chan struct{}) @@ -307,7 +315,12 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { updates = make(chan *api.Restore) - restoreInformer = v1.NewRestoreInformer(o.client, f.Namespace(), 0, nil) + lw := kube.InternalLW{ + Client: o.client, + Namespace: f.Namespace(), + ObjectList: new(api.RestoreList), + } + restoreInformer := cache.NewSharedInformer(&lw, &api.Restore{}, time.Second) restoreInformer.AddEventHandler( cache.FilteringResourceEventHandler{ @@ -339,7 +352,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { go restoreInformer.Run(stop) } - restore, err := o.client.VeleroV1().Restores(restore.Namespace).Create(context.TODO(), restore, metav1.CreateOptions{}) + err := o.client.Create(context.TODO(), restore, &kbclient.CreateOptions{}) if err != nil { return err } diff --git a/pkg/cmd/cli/restore/create_test.go b/pkg/cmd/cli/restore/create_test.go new file mode 100644 index 0000000000..e62d3d613d --- /dev/null +++ b/pkg/cmd/cli/restore/create_test.go @@ -0,0 +1,174 @@ +/* +Copyright 2020 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 restore + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func TestIsResourcePolicyValid(t *testing.T) { + require.True(t, isResourcePolicyValid(string(velerov1api.PolicyTypeNone))) + require.True(t, isResourcePolicyValid(string(velerov1api.PolicyTypeUpdate))) + require.False(t, isResourcePolicyValid("")) +} + +func TestMostRecentBackup(t *testing.T) { + backups := []velerov1api.Backup{ + *builder.ForBackup(cmdtest.VeleroNameSpace, "backup0").StartTimestamp(time.Now().Add(3 * time.Second)).Phase(velerov1api.BackupPhaseDeleting).Result(), + *builder.ForBackup(cmdtest.VeleroNameSpace, "backup1").StartTimestamp(time.Now().Add(time.Second)).Phase(velerov1api.BackupPhaseCompleted).Result(), + *builder.ForBackup(cmdtest.VeleroNameSpace, "backup2").StartTimestamp(time.Now().Add(2 * time.Second)).Phase(velerov1api.BackupPhasePartiallyFailed).Result(), + } + + expectedBackup := builder.ForBackup(cmdtest.VeleroNameSpace, "backup2").StartTimestamp(time.Now().Add(2 * time.Second)).Phase(velerov1api.BackupPhasePartiallyFailed).Result() + + resultBackup := mostRecentBackup(backups, velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed) + + require.Equal(t, expectedBackup.Name, resultBackup.Name) +} + +func TestCreateCommand(t *testing.T) { + name := "nameToBeCreated" + args := []string{name} + + t.Run("create a backup create command with full options except fromSchedule and wait, then run by create option", func(t *testing.T) { + + // create a factory + f := &factorymocks.Factory{} + + // create command + cmd := NewCreateCommand(f, "") + require.Equal(t, "Create a restore", cmd.Short) + + backupName := "backup1" + scheduleName := "schedule1" + restoreVolumes := "true" + preserveNodePorts := "true" + labels := "c=foo,b=woo" + includeNamespaces := "app1,app2" + excludeNamespaces := "pod1,pod2,pod3" + existingResourcePolicy := "none" + includeResources := "sc,sts" + excludeResources := "job" + statusIncludeResources := "sc,sts" + statusExcludeResources := "job" + namespaceMappings := "a:b" + selector := "foo=bar" + includeClusterResources := "true" + allowPartiallyFailed := "true" + itemOperationTimeout := "10m0s" + + flags := new(pflag.FlagSet) + o := NewCreateOptions() + o.BindFlags(flags) + + flags.Parse([]string{"--from-backup", backupName}) + flags.Parse([]string{"--from-schedule", scheduleName}) + flags.Parse([]string{"--restore-volumes", restoreVolumes}) + flags.Parse([]string{"--preserve-nodeports", preserveNodePorts}) + flags.Parse([]string{"--labels", labels}) + flags.Parse([]string{"--existing-resource-policy", existingResourcePolicy}) + flags.Parse([]string{"--include-namespaces", includeNamespaces}) + flags.Parse([]string{"--exclude-namespaces", excludeNamespaces}) + flags.Parse([]string{"--include-resources", includeResources}) + flags.Parse([]string{"--exclude-resources", excludeResources}) + flags.Parse([]string{"--status-include-resources", statusIncludeResources}) + flags.Parse([]string{"--status-exclude-resources", statusExcludeResources}) + flags.Parse([]string{"--namespace-mappings", namespaceMappings}) + flags.Parse([]string{"--selector", selector}) + flags.Parse([]string{"--include-cluster-resources", includeClusterResources}) + flags.Parse([]string{"--allow-partially-failed", allowPartiallyFailed}) + flags.Parse([]string{"--item-operation-timeout", itemOperationTimeout}) + + client := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + + f.On("Namespace").Return(mock.Anything) + f.On("KubebuilderWatchClient").Return(client, nil) + + //Complete + e := o.Complete(args, f) + require.NoError(t, e) + + //Validate + e = o.Validate(cmd, args, f) + require.Contains(t, e.Error(), "either a backup or schedule must be specified, but not both") + + //cmd + e = o.Run(cmd, f) + require.NoError(t, e) + + require.Equal(t, backupName, o.BackupName) + require.Equal(t, scheduleName, o.ScheduleName) + require.Equal(t, restoreVolumes, o.RestoreVolumes.String()) + require.Equal(t, preserveNodePorts, o.PreserveNodePorts.String()) + require.Equal(t, labels, o.Labels.String()) + require.Equal(t, includeNamespaces, o.IncludeNamespaces.String()) + require.Equal(t, excludeNamespaces, o.ExcludeNamespaces.String()) + require.Equal(t, existingResourcePolicy, o.ExistingResourcePolicy) + require.Equal(t, includeResources, o.IncludeResources.String()) + require.Equal(t, excludeResources, o.ExcludeResources.String()) + + require.Equal(t, statusIncludeResources, o.StatusIncludeResources.String()) + require.Equal(t, statusExcludeResources, o.StatusExcludeResources.String()) + require.Equal(t, namespaceMappings, o.NamespaceMappings.String()) + require.Equal(t, selector, o.Selector.String()) + require.Equal(t, includeClusterResources, o.IncludeClusterResources.String()) + require.Equal(t, allowPartiallyFailed, o.AllowPartiallyFailed.String()) + require.Equal(t, itemOperationTimeout, o.ItemOperationTimeout.String()) + + }) + + t.Run("create a restore create from schedule", func(t *testing.T) { + f := &factorymocks.Factory{} + c := NewCreateCommand(f, "") + require.Equal(t, "Create a restore", c.Short) + flags := new(pflag.FlagSet) + o := NewCreateOptions() + o.BindFlags(flags) + + fromSchedule := "schedule-name-1" + flags.Parse([]string{"--from-schedule", fromSchedule}) + + fmt.Printf("debug, restore options: %+v\n", o) + + kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + + schedule := builder.ForSchedule(cmdtest.VeleroNameSpace, fromSchedule).Result() + kbclient.Create(context.Background(), schedule, &controllerclient.CreateOptions{}) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderWatchClient").Return(kbclient, nil) + + require.NoError(t, o.Complete(args, f)) + + require.NoError(t, o.Run(c, f)) + }) +} diff --git a/pkg/cmd/cli/restore/restore_test.go b/pkg/cmd/cli/restore/restore_test.go new file mode 100644 index 0000000000..85f4b662a5 --- /dev/null +++ b/pkg/cmd/cli/restore/restore_test.go @@ -0,0 +1,34 @@ +/* +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 restore + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" +) + +func TestNewRestoreCommand(t *testing.T) { + // create a factory + f := &factorymocks.Factory{} + + // create command + cmd := NewCommand(f) + assert.Equal(t, "Work with restores", cmd.Short) +} diff --git a/pkg/util/kube/list_watch_test.go b/pkg/util/kube/list_watch_test.go new file mode 100644 index 0000000000..cf7056f5fc --- /dev/null +++ b/pkg/util/kube/list_watch_test.go @@ -0,0 +1,54 @@ +/* +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 kube + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + "k8s.io/client-go/tools/cache" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func TestInternalLW(t *testing.T) { + stop := make(chan struct{}) + client := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + lw := InternalLW{ + Client: client, + Namespace: cmdtest.VeleroNameSpace, + ObjectList: new(velerov1api.BackupList), + } + + restoreInformer := cache.NewSharedInformer(&lw, &velerov1api.BackupList{}, time.Second) + go restoreInformer.Run(stop) + + time.Sleep(1 * time.Second) + close(stop) + + backupList := new(velerov1api.BackupList) + err := client.List(context.Background(), backupList) + require.NoError(t, err) + + _, err = client.Watch(context.Background(), backupList) + require.NoError(t, err) +} From e51a9d4e1edd909f3ae040b8adcb50ecf46d7c32 Mon Sep 17 00:00:00 2001 From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> Date: Tue, 18 Jul 2023 08:05:57 +0800 Subject: [PATCH 13/14] Integrate pushing to docker hub and gcr.io in one docker build and push command. (#6199) Signed-off-by: Xun Jiang Signed-off-by: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com> Co-authored-by: Xun Jiang --- .github/workflows/push.yml | 23 +++++++---------------- Makefile | 5 +++++ 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index e04c325494..b1af90a379 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -60,6 +60,13 @@ jobs: files: coverage.out verbose: true + # Use the JSON key in secret to login gcr.io + - uses: 'docker/login-action@v2' + with: + registry: 'gcr.io' # or REGION.docker.pkg.dev + username: '_json_key' + password: '${{ secrets.GCR_SA_KEY }}' + # Only try to publish the container image from the root repo; forks don't have permission to do so and will always get failures. - name: Publish container image if: github.repository == 'vmware-tanzu/velero' @@ -91,19 +98,3 @@ jobs: uploader ${VELERO_RESTORE_HELPER_IMAGE_FILE} ${GCS_BUCKET} uploader ${VELERO_IMAGE_BACKUP_FILE} ${GCS_BUCKET} uploader ${VELERO_RESTORE_HELPER_IMAGE_BACKUP_FILE} ${GCS_BUCKET} - - # Use the JSON key in secret to login gcr.io - - uses: 'docker/login-action@v1' - with: - registry: 'gcr.io' # or REGION.docker.pkg.dev - username: '_json_key' - password: '${{ secrets.GCR_SA_KEY }}' - - # Push image to GCR to facilitate some environments that have rate limitation to docker hub, e.g. vSphere. - - name: Publish container image to GCR - if: github.repository == 'vmware-tanzu/velero' - run: | - sudo swapoff -a - sudo rm -f /mnt/swapfile - docker image prune -a --force - REGISTRY=gcr.io/velero-gcp ./hack/docker-push.sh diff --git a/Makefile b/Makefile index 82b5241065..0bdf6e8d78 100644 --- a/Makefile +++ b/Makefile @@ -22,9 +22,11 @@ PKG := github.com/vmware-tanzu/velero # Where to push the docker image. REGISTRY ?= velero +GCR_REGISTRY ?= gcr.io/velero-gcp # Image name IMAGE ?= $(REGISTRY)/$(BIN) +GCR_IMAGE ?= $(GCR_REGISTRY)/$(BIN) # We allow the Dockerfile to be configurable to enable the use of custom Dockerfiles # that pull base images from different registries. @@ -66,8 +68,10 @@ TAG_LATEST ?= false ifeq ($(TAG_LATEST), true) IMAGE_TAGS ?= $(IMAGE):$(VERSION) $(IMAGE):latest + GCR_IMAGE_TAGS ?= $(GCR_IMAGE):$(VERSION) $(GCR_IMAGE):latest else IMAGE_TAGS ?= $(IMAGE):$(VERSION) + GCR_IMAGE_TAGS ?= $(GCR_IMAGE):$(VERSION) endif ifeq ($(shell docker buildx inspect 2>/dev/null | awk '/Status/ { print $$2 }'), running) @@ -183,6 +187,7 @@ endif --output=type=$(BUILDX_OUTPUT_TYPE) \ --platform $(BUILDX_PLATFORMS) \ $(addprefix -t , $(IMAGE_TAGS)) \ + $(addprefix -t , $(GCR_IMAGE_TAGS)) \ --build-arg=GOPROXY=$(GOPROXY) \ --build-arg=PKG=$(PKG) \ --build-arg=BIN=$(BIN) \ From 4a222b76c6ae697253d5606bf56101953cd1ace5 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Mon, 17 Jul 2023 20:39:35 +0800 Subject: [PATCH 14/14] Improve backup log command UT coverage. Signed-off-by: Xun Jiang --- pkg/cmd/cli/backup/create_test.go | 2 +- pkg/cmd/cli/backup/logs_test.go | 22 +++++++ pkg/cmd/cli/restore/create_test.go | 36 +++++++++--- pkg/cmd/cli/restore/delete_test.go | 85 ++++++++++++++++++++++++++++ pkg/cmd/cli/restore/describe.go | 37 ++++++------ pkg/cmd/cli/restore/describe_test.go | 75 ++++++++++++++++++++++++ pkg/cmd/cli/restore/get.go | 15 +++-- pkg/cmd/cli/restore/get_test.go | 81 ++++++++++++++++++++++++++ pkg/cmd/cli/restore/logs_test.go | 55 ++++++++++++++++++ 9 files changed, 374 insertions(+), 34 deletions(-) create mode 100644 pkg/cmd/cli/restore/delete_test.go create mode 100644 pkg/cmd/cli/restore/describe_test.go create mode 100644 pkg/cmd/cli/restore/get_test.go create mode 100644 pkg/cmd/cli/restore/logs_test.go diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 8d20cda18d..34652c9596 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -162,7 +162,7 @@ func TestCreateCommand(t *testing.T) { excludeClusterScopedResources := "MutatingWebhookConfiguration,APIService" includeNamespaceScopedResources := "Endpoints,Event,PodTemplate" excludeNamespaceScopedResources := "Secret,MultiClusterIngress" - labels := "c=foo,b=woo" + labels := "c=foo" storageLocation := "bsl-name-1" snapshotLocations := "region=minio" selector := "a=pod" diff --git a/pkg/cmd/cli/backup/logs_test.go b/pkg/cmd/cli/backup/logs_test.go index 9bb95a7a8d..973c1e4633 100644 --- a/pkg/cmd/cli/backup/logs_test.go +++ b/pkg/cmd/cli/backup/logs_test.go @@ -105,6 +105,8 @@ func TestNewLogsCommand(t *testing.T) { require.Error(t, err) require.Equal(t, fmt.Sprintf("backup \"%s\" does not exist", backupName), err.Error()) + + c.Execute() }) t.Run("Normal backup log test", func(t *testing.T) { @@ -143,4 +145,24 @@ func TestNewLogsCommand(t *testing.T) { case <-done: } }) + + t.Run("Invalid client test", func(t *testing.T) { + // create a factory + f := &factorymocks.Factory{} + + kbClient := velerotest.NewFakeControllerRuntimeClient(t) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + + c := NewLogsCommand(f) + assert.Equal(t, "Get backup logs", c.Short) + + l := NewLogsOptions() + flags := new(flag.FlagSet) + l.BindFlags(flags) + + f.On("KubebuilderClient").Return(kbClient, fmt.Errorf("test error")) + err := l.Complete([]string{""}, f) + require.Equal(t, "test error", err.Error()) + }) } diff --git a/pkg/cmd/cli/restore/create_test.go b/pkg/cmd/cli/restore/create_test.go index e62d3d613d..27f78416d1 100644 --- a/pkg/cmd/cli/restore/create_test.go +++ b/pkg/cmd/cli/restore/create_test.go @@ -18,7 +18,6 @@ package restore import ( "context" - "fmt" "testing" "time" @@ -72,7 +71,7 @@ func TestCreateCommand(t *testing.T) { scheduleName := "schedule1" restoreVolumes := "true" preserveNodePorts := "true" - labels := "c=foo,b=woo" + labels := "c=foo" includeNamespaces := "app1,app2" excludeNamespaces := "pod1,pod2,pod3" existingResourcePolicy := "none" @@ -146,7 +145,7 @@ func TestCreateCommand(t *testing.T) { }) - t.Run("create a restore create from schedule", func(t *testing.T) { + t.Run("create a restore from schedule", func(t *testing.T) { f := &factorymocks.Factory{} c := NewCreateCommand(f, "") require.Equal(t, "Create a restore", c.Short) @@ -157,18 +156,39 @@ func TestCreateCommand(t *testing.T) { fromSchedule := "schedule-name-1" flags.Parse([]string{"--from-schedule", fromSchedule}) - fmt.Printf("debug, restore options: %+v\n", o) - kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) - schedule := builder.ForSchedule(cmdtest.VeleroNameSpace, fromSchedule).Result() - kbclient.Create(context.Background(), schedule, &controllerclient.CreateOptions{}) + require.NoError(t, kbclient.Create(context.Background(), schedule, &controllerclient.CreateOptions{})) + backup := builder.ForBackup(cmdtest.VeleroNameSpace, "test-backup").FromSchedule(schedule).Phase(velerov1api.BackupPhaseCompleted).Result() + require.NoError(t, kbclient.Create(context.Background(), backup, &controllerclient.CreateOptions{})) f.On("Namespace").Return(cmdtest.VeleroNameSpace) f.On("KubebuilderWatchClient").Return(kbclient, nil) require.NoError(t, o.Complete(args, f)) - + require.NoError(t, o.Validate(c, []string{}, f)) require.NoError(t, o.Run(c, f)) }) + + t.Run("create a restore from not-existed backup", func(t *testing.T) { + f := &factorymocks.Factory{} + c := NewCreateCommand(f, "") + require.Equal(t, "Create a restore", c.Short) + flags := new(pflag.FlagSet) + o := NewCreateOptions() + o.BindFlags(flags) + nonExistedBackupName := "not-exist" + + flags.Parse([]string{"--wait", "true"}) + flags.Parse([]string{"--from-backup", nonExistedBackupName}) + + kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch) + + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderWatchClient").Return(kbclient, nil) + + require.NoError(t, o.Complete(nil, f)) + err := o.Validate(c, []string{}, f) + require.Equal(t, "backups.velero.io \"not-exist\" not found", err.Error()) + }) } diff --git a/pkg/cmd/cli/restore/delete_test.go b/pkg/cmd/cli/restore/delete_test.go new file mode 100644 index 0000000000..b504e2c005 --- /dev/null +++ b/pkg/cmd/cli/restore/delete_test.go @@ -0,0 +1,85 @@ +/* +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 restore + +import ( + "context" + "fmt" + "os" + "os/exec" + "testing" + + flag "github.com/spf13/pflag" + "github.com/stretchr/testify/require" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + "github.com/vmware-tanzu/velero/pkg/cmd/cli" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" + veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" +) + +func TestDeleteCommand(t *testing.T) { + restore1 := "restore-name-1" + restore2 := "restore-name-2" + + // create a factory + f := &factorymocks.Factory{} + + client := velerotest.NewFakeControllerRuntimeClient(t) + client.Create(context.Background(), builder.ForRestore(cmdtest.VeleroNameSpace, restore1).Result(), &controllerclient.CreateOptions{}) + client.Create(context.Background(), builder.ForRestore("default", restore2).Result(), &controllerclient.CreateOptions{}) + + f.On("KubebuilderClient").Return(client, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + + // create command + c := NewDeleteCommand(f, "velero restore delete") + c.SetArgs([]string{restore1, restore2}) + require.Equal(t, "Delete restores", c.Short) + + o := cli.NewDeleteOptions("restore") + flags := new(flag.FlagSet) + o.BindFlags(flags) + flags.Parse([]string{"--confirm"}) + + args := []string{restore1, restore2} + + e := o.Complete(f, args) + require.Equal(t, nil, e) + + e = o.Validate(c, f, args) + require.Equal(t, nil, e) + + Run(o) + + e = c.Execute() + require.Equal(t, nil, e) + + if os.Getenv(cmdtest.CaptureFlag) == "1" { + return + } + + cmd := exec.Command(os.Args[0], []string{"-test.run=TestDeleteCommand"}...) + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) + stdout, _, err := veleroexec.RunCommand(cmd) + if err != nil { + require.Contains(t, stdout, fmt.Sprintf("restores.velero.io \"%s\" not found.", restore2)) + } +} diff --git a/pkg/cmd/cli/restore/describe.go b/pkg/cmd/cli/restore/describe.go index 10aca7e025..4bb04b521c 100644 --- a/pkg/cmd/cli/restore/describe.go +++ b/pkg/cmd/cli/restore/describe.go @@ -23,6 +23,8 @@ import ( "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -48,34 +50,37 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { Use: use + " [NAME1] [NAME2] [NAME...]", Short: "Describe restores", Run: func(c *cobra.Command, args []string) { - veleroClient, err := f.Client() - cmd.CheckError(err) - kbClient, err := f.KubebuilderClient() cmd.CheckError(err) - var restores *velerov1api.RestoreList + restoreList := new(velerov1api.RestoreList) if len(args) > 0 { - restores = new(velerov1api.RestoreList) for _, name := range args { - restore, err := veleroClient.VeleroV1().Restores(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{}) + restore := new(velerov1api.Restore) + err := kbClient.Get(context.TODO(), controllerclient.ObjectKey{Namespace: f.Namespace(), Name: name}, restore) cmd.CheckError(err) - restores.Items = append(restores.Items, *restore) + restoreList.Items = append(restoreList.Items, *restore) } } else { - restores, err = veleroClient.VeleroV1().Restores(f.Namespace()).List(context.TODO(), listOptions) + parsedSelector, err := labels.Parse(listOptions.LabelSelector) + cmd.CheckError(err) + + err = kbClient.List(context.TODO(), restoreList, &controllerclient.ListOptions{LabelSelector: parsedSelector, Namespace: f.Namespace()}) cmd.CheckError(err) } first := true - for i, restore := range restores.Items { - opts := newPodVolumeRestoreListOptions(restore.Name) - podvolumeRestoreList, err := veleroClient.VeleroV1().PodVolumeRestores(f.Namespace()).List(context.TODO(), opts) + for i, restore := range restoreList.Items { + podVolumeRestoreList := new(velerov1api.PodVolumeRestoreList) + err = kbClient.List(context.TODO(), podVolumeRestoreList, &controllerclient.ListOptions{ + Namespace: f.Namespace(), + LabelSelector: labels.SelectorFromSet(map[string]string{velerov1api.BackupNameLabel: label.GetValidName(restore.Name)}), + }) if err != nil { fmt.Fprintf(os.Stderr, "error getting PodVolumeRestores for restore %s: %v\n", restore.Name, err) } - s := output.DescribeRestore(context.Background(), kbClient, &restores.Items[i], podvolumeRestoreList.Items, details, insecureSkipTLSVerify, caCertFile) + s := output.DescribeRestore(context.Background(), kbClient, &restoreList.Items[i], podVolumeRestoreList.Items, details, insecureSkipTLSVerify, caCertFile) if first { first = false fmt.Print(s) @@ -94,11 +99,3 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { return c } - -// newPodVolumeRestoreListOptions creates a ListOptions with a label selector configured to -// find PodVolumeRestores for the restore identified by name. -func newPodVolumeRestoreListOptions(name string) metav1.ListOptions { - return metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", velerov1api.RestoreNameLabel, label.GetValidName(name)), - } -} diff --git a/pkg/cmd/cli/restore/describe_test.go b/pkg/cmd/cli/restore/describe_test.go new file mode 100644 index 0000000000..f0e83cce57 --- /dev/null +++ b/pkg/cmd/cli/restore/describe_test.go @@ -0,0 +1,75 @@ +/* +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 restore + +import ( + "context" + "fmt" + "os" + "os/exec" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/client-go/rest" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + "github.com/vmware-tanzu/velero/pkg/features" + "github.com/vmware-tanzu/velero/pkg/test" + veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" +) + +func TestNewDescribeCommand(t *testing.T) { + // create a factory + f := &factorymocks.Factory{} + restoreName := "restore-describe-1" + testRestore := builder.ForRestore(cmdtest.VeleroNameSpace, restoreName).Result() + + clientConfig := rest.Config{} + kbClient := test.NewFakeControllerRuntimeClient(t) + kbClient.Create(context.Background(), testRestore, &controllerclient.CreateOptions{}) + + f.On("ClientConfig").Return(&clientConfig, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(kbClient, nil) + + // create command + c := NewDescribeCommand(f, "velero restore describe") + assert.Equal(t, "Describe restores", c.Short) + + features.NewFeatureFlagSet("EnableCSI") + defer features.NewFeatureFlagSet() + + c.SetArgs([]string{restoreName}) + e := c.Execute() + assert.NoError(t, e) + + if os.Getenv(cmdtest.CaptureFlag) == "1" { + return + } + cmd := exec.Command(os.Args[0], []string{"-test.run=TestNewDescribeCommand"}...) + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) + stdout, _, err := veleroexec.RunCommand(cmd) + + if err == nil { + assert.Contains(t, stdout, fmt.Sprintf("Name: %s", restoreName)) + return + } + t.Fatalf("process ran with err %v, want backups by get()", err) +} diff --git a/pkg/cmd/cli/restore/get.go b/pkg/cmd/cli/restore/get.go index a6370b9a18..74ac42ad3f 100644 --- a/pkg/cmd/cli/restore/get.go +++ b/pkg/cmd/cli/restore/get.go @@ -21,6 +21,8 @@ import ( "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + controllerclient "sigs.k8s.io/controller-runtime/pkg/client" api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/client" @@ -38,19 +40,22 @@ func NewGetCommand(f client.Factory, use string) *cobra.Command { err := output.ValidateFlags(c) cmd.CheckError(err) - veleroClient, err := f.Client() + kbClient, err := f.KubebuilderClient() cmd.CheckError(err) - var restores *api.RestoreList + restores := new(api.RestoreList) if len(args) > 0 { - restores = new(api.RestoreList) for _, name := range args { - restore, err := veleroClient.VeleroV1().Restores(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{}) + restore := new(api.Restore) + err := kbClient.Get(context.TODO(), controllerclient.ObjectKey{Namespace: f.Namespace(), Name: name}, restore) cmd.CheckError(err) restores.Items = append(restores.Items, *restore) } } else { - restores, err = veleroClient.VeleroV1().Restores(f.Namespace()).List(context.TODO(), listOptions) + parsedSelector, err := labels.Parse(listOptions.LabelSelector) + cmd.CheckError(err) + + err = kbClient.List(context.TODO(), restores, &controllerclient.ListOptions{LabelSelector: parsedSelector, Namespace: f.Namespace()}) cmd.CheckError(err) } diff --git a/pkg/cmd/cli/restore/get_test.go b/pkg/cmd/cli/restore/get_test.go new file mode 100644 index 0000000000..211550087b --- /dev/null +++ b/pkg/cmd/cli/restore/get_test.go @@ -0,0 +1,81 @@ +/* +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 restore + +import ( + "context" + "fmt" + "os" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/require" + kbclient "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" + veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec" +) + +func TestNewGetCommand(t *testing.T) { + args := []string{"b1", "b2", "b3"} + + // create a factory + f := &factorymocks.Factory{} + + client := velerotest.NewFakeControllerRuntimeClient(t) + + for _, restoreName := range args { + restore := builder.ForRestore(cmdtest.VeleroNameSpace, restoreName).ObjectMeta(builder.WithLabels("abc", "abc")).Result() + err := client.Create(context.Background(), restore, &kbclient.CreateOptions{}) + require.NoError(t, err) + } + + f.On("KubebuilderClient").Return(client, nil) + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + + // create command + c := NewGetCommand(f, "velero restore get") + require.Equal(t, "Get restores", c.Short) + + c.SetArgs(args) + e := c.Execute() + require.NoError(t, e) + + if os.Getenv(cmdtest.CaptureFlag) == "1" { + return + } + + cmd := exec.Command(os.Args[0], []string{"-test.run=TestNewGetCommand"}...) + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag)) + stdout, _, err := veleroexec.RunCommand(cmd) + require.NoError(t, err) + + if err == nil { + output := strings.Split(stdout, "\n") + i := 0 + for _, line := range output { + if strings.Contains(line, "New") { + i++ + } + } + require.Equal(t, len(args), i) + } +} diff --git a/pkg/cmd/cli/restore/logs_test.go b/pkg/cmd/cli/restore/logs_test.go new file mode 100644 index 0000000000..007da70440 --- /dev/null +++ b/pkg/cmd/cli/restore/logs_test.go @@ -0,0 +1,55 @@ +/* +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 restore + +import ( + "os" + "testing" + + flag "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" +) + +func TestNewLogsCommand(t *testing.T) { + t.Run("Flag test", func(t *testing.T) { + // create a factory + f := &factorymocks.Factory{} + + c := NewLogsCommand(f) + require.Equal(t, "Get restore logs", c.Short) + flags := new(flag.FlagSet) + + timeout := "1m0s" + insecureSkipTLSVerify := "true" + caCertFile := "testing" + + flags.Parse([]string{"--timeout", timeout}) + flags.Parse([]string{"--insecure-skip-tls-verify", insecureSkipTLSVerify}) + flags.Parse([]string{"--cacert", caCertFile}) + + if os.Getenv(cmdtest.CaptureFlag) == "1" { + c.SetArgs([]string{"test"}) + e := c.Execute() + assert.NoError(t, e) + return + } + }) +}