Add backup deletion e2e test #4401

danfengliu · 2021-11-26T10:30:33Z

Fixed issue : #4348

Signed-off-by: danfengl danfengl@vmware.com

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

Accepted the DCO. Commits without the DCO will delay acceptance.
Created a changelog file or added /kind changelog-not-required.
Updated the corresponding documentation in site/content/docs/main.

qiuming-best

Some detail maybe should be modified.

test/e2e/backup-resource/deletion.go

test/e2e/e2e_suite_test.go

test/e2e/util/providers/azure_utils.go

qiuming-best · 2021-12-01T07:14:20Z

test/e2e/util/providers/azure_utils.go

+		if serr, ok := err.(azblob.StorageError); ok { // This error is a Service-specific
+			switch serr.ServiceCode() { // Compare serviceCode to ServiceCodeXxx constants
+			case azblob.ServiceCodeContainerAlreadyExists:
+				//fmt.Println("Received 409. Container already exists")


Should be removed or print it

ywk253100 · 2021-12-03T03:39:55Z

test/e2e/backup-resource/deletion.go

+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package backupresource


There is already a package called backup here, so what's different between the backup and backupresource? It's confusing.

ywk253100 · 2021-12-03T03:44:56Z

test/e2e/backup-resource/deletion.go

+		return err
+	}
+	backupName = "backup-1-" + UUIDgen.String()
+	if err := VeleroBackupNamespace(oneHourTimeout, veleroCLI, veleroNamespace, backupName, deletionTest, backupLocation, useVolumeSnapshots); err != nil {


Why do we need to backup again?

Because the 1st backup was deleted locally for verification that backup can be deleted from object store right after local deletion.

ywk253100 · 2021-12-03T03:46:52Z

test/e2e/backup-resource/deletion.go

+func Backup_deletion_with_restic() {
+	backup_deletion_test(false)
+}
+func backup_deletion_test(useVolumeSnapshots bool) {


Does this case check the snapshots or restic data? If it doesn't, why we need to back up the volumes?

Snapshot verification will be provide in a new PR. Way of verifying Restic repo changes is under investigation.

ywk253100 · 2021-12-03T03:51:19Z

test/e2e/backup-resource/deletion.go

+	})
+
+	When("kibishii is the sample workload", func() {
+		It("should be successfully backed up and restored to the default BackupStorageLocation", func() {


Correct the case description?

ywk253100 · 2021-12-03T03:52:19Z

test/e2e/e2e_suite_test.go

@@ -76,6 +77,8 @@ var _ = Describe("[Upgrade][Restic] Velero upgrade tests on cluster using the pl

 var _ = Describe("[Upgrade][Snapshot] Velero upgrade tests on cluster using the plugin provider for object storage and snapshots for volume backups", BackupUpgradeRestoreWithSnapshots)

+var _ = Describe("[backup-resource][deletion][restic] Velero tests on cluster using the plugin provider for object storage and Restic for volume backups", Backup_deletion_with_restic)


Correct the case description

Changing [backup-resource][deletion] to [backup-deletion] is more reasonable?

There will be more tests under backup-resource, the resource does not mean as k8s resources, it is the resource caused by backup action, it's the object of velero.

Should we use unified naming conventions like [BackupResource][Deletion][Restic]?

Test case description is "Deleted backups are deleted from object storage and backups deleted from object storage can be deleted locally", in this test, only resource backup objects are target for verifition, restic repo verification is not included in this PR, and snapshot verification will be in later PR Signed-off-by: danfengl <danfengl@vmware.com>

github-actions bot assigned danfengliu Nov 26, 2021

github-actions bot requested review from dsu-igeek and zubron November 26, 2021 10:30

github-actions bot added Dependencies Pull requests that update a dependency file has-e2e-tests labels Nov 26, 2021

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 2 times, most recently from 015e721 to f8be092 Compare November 29, 2021 11:13

github-actions bot added the has-changelog label Nov 29, 2021

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 4 times, most recently from 01a0f15 to 62ad0ba Compare November 30, 2021 00:36

danfengliu changed the title ~~Add backup deletion e2e test~~ Add backup deletion e2e test - backup with restic Nov 30, 2021

danfengliu assigned qiuming-best and ywk253100 Dec 1, 2021

danfengliu requested a review from ywk253100 December 1, 2021 02:41

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 2 times, most recently from 6bada56 to f465ae1 Compare December 1, 2021 06:56

qiuming-best reviewed Dec 1, 2021

View reviewed changes

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 2 times, most recently from b30eaaa to d07c689 Compare December 1, 2021 07:48

qiuming-best previously approved these changes Dec 1, 2021

View reviewed changes

ywk253100 reviewed Dec 3, 2021

View reviewed changes

danfengliu dismissed qiuming-best’s stale review via e0c0105 December 12, 2021 07:55

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 2 times, most recently from e0c0105 to 4e2df0a Compare December 14, 2021 07:15

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 8 times, most recently from c409c81 to 6a24e03 Compare December 17, 2021 08:03

danfengliu changed the title ~~Add backup deletion e2e test - backup with restic~~ Add backup deletion e2e test Dec 17, 2021

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 4 times, most recently from 2447128 to 7561e17 Compare December 20, 2021 07:48

danfengliu linked an issue Dec 20, 2021 that may be closed by this pull request

Add E2E test of backups deletion #4348

Closed

qiuming-best previously approved these changes Dec 21, 2021

View reviewed changes

danfengliu dismissed qiuming-best’s stale review via 7ebf865 December 23, 2021 09:16

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch 6 times, most recently from efe809c to 9f898df Compare December 23, 2021 12:29

danfengliu force-pushed the add-backup-deletion-e2e-test-to-main branch from 9f898df to 29b2cd1 Compare December 23, 2021 12:45

qiuming-best approved these changes Dec 24, 2021

View reviewed changes

ywk253100 approved these changes Dec 24, 2021

View reviewed changes

ywk253100 merged commit 3feb259 into vmware-tanzu:main Dec 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add backup deletion e2e test #4401

Add backup deletion e2e test #4401

danfengliu commented Nov 26, 2021 •

edited

qiuming-best left a comment

qiuming-best Dec 1, 2021

ywk253100 Dec 3, 2021

ywk253100 Dec 3, 2021

danfengliu Dec 15, 2021

ywk253100 Dec 3, 2021

danfengliu Dec 15, 2021 •

edited

ywk253100 Dec 3, 2021

ywk253100 Dec 3, 2021

ywk253100 Dec 3, 2021

danfengliu Dec 15, 2021

qiuming-best Dec 15, 2021

		@@ -76,6 +77,8 @@ var _ = Describe("[Upgrade][Restic] Velero upgrade tests on cluster using the pl

		var _ = Describe("[Upgrade][Snapshot] Velero upgrade tests on cluster using the plugin provider for object storage and snapshots for volume backups", BackupUpgradeRestoreWithSnapshots)

		var _ = Describe("[backup-resource][deletion][restic] Velero tests on cluster using the plugin provider for object storage and Restic for volume backups", Backup_deletion_with_restic)

Add backup deletion e2e test #4401

Add backup deletion e2e test #4401

Conversation

danfengliu commented Nov 26, 2021 • edited

Please add a summary of your change

Does your change fix a particular issue?

Please indicate you've done the following:

qiuming-best left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danfengliu Dec 15, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danfengliu commented Nov 26, 2021 •

edited

danfengliu Dec 15, 2021 •

edited