Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add E2E test for multiple credentials #3559

Merged
merged 2 commits into from
Mar 10, 2021

Conversation

zubron
Copy link
Contributor

@zubron zubron commented Mar 10, 2021

Please add a summary of your change

This change adds an E2E test which exercises the mulitple credentials
feature added in #3489. The test creates a secret from the given
credentials and creates a BackupStorageLocation which uses those
credentials. A backup and restore is then performed to the default
BSL and to the newly created BSL.

This change adds new flags to the E2E test suite to configure the BSL
created and used in the test.

Signed-off-by: Bridget McErlean [email protected]

Does your change fix a particular issue?

Part of #3304

Please indicate you've done the following:

@zubron
Copy link
Contributor Author

zubron commented Mar 10, 2021

/kind changelog-not-required

@github-actions github-actions bot added Dependencies Pull requests that update a dependency file has-e2e-tests labels Mar 10, 2021
@zubron zubron added P1 - Important kind/release-blocker Must fix issues for the coming release (milestone) labels Mar 10, 2021
@github-actions github-actions bot added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Mar 10, 2021
@zubron zubron force-pushed the add-e2e-test-for-multi-creds branch from 2b8bc49 to 3b969b0 Compare March 10, 2021 21:30
@github-actions github-actions bot requested a review from dsu-igeek March 10, 2021 21:31
@zubron zubron force-pushed the add-e2e-test-for-multi-creds branch from 3b969b0 to 3e1b2a5 Compare March 10, 2021 21:33
This change adds an E2E test which exercises the mulitple credentials
feature added in vmware-tanzu#3489. The test creates a secret from the given
credentials and creates a BackupStorageLocation which uses those
credentials. A backup and restore is then performed to the default
BSL and to the newly created BSL.

This change adds new flags to the E2E test suite to configure the BSL
created and used in the test.

Signed-off-by: Bridget McErlean <[email protected]>
@zubron zubron force-pushed the add-e2e-test-for-multi-creds branch from 3e1b2a5 to 9ad1e89 Compare March 10, 2021 21:43
@github-actions github-actions bot requested review from nrb and carlisia March 10, 2021 21:45
Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor suggestions.

It is so good to have these e2e tests! This looks great.

"Failed to successfully backup and restore Kibishii namespace")
})

It("should successfully back up and restore to multiple BackupStorageLocations with unique credentials", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test name could be more precise and helpful. By itself it is not operating with multiple BSLs, only an additional one besides the existing default. So maybe if it's named "should successfully back up and restore to an additional BackupStorageLocation with unique credentials" it'd avoid people looking for more than one BSL being used in this test (I have no idea who'd do that... maybe me :) ).

These are such nits:
s/additionalBsl/additionalBSL
s/additionalBslCredentials/additionalBSLCredentials
s/additionalBslBucket/additionalBSLBucket
etc

Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

Signed-off-by: Bridget McErlean <[email protected]>
Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👍

Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'd like for this to handle more than one credential, but it's OK for this release. Would you add an issue please to be able to test with N credentials. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file has-e2e-tests kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes kind/release-blocker Must fix issues for the coming release (milestone)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add E2E backup/restore test for multiple object store credentials
3 participants