-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Make the E2E supporting VKS data mover environment. #8371
base: main
Are you sure you want to change the base?
Make the E2E supporting VKS data mover environment. #8371
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8371 +/- ##
==========================================
- Coverage 58.95% 58.93% -0.03%
==========================================
Files 367 367
Lines 38876 38902 +26
==========================================
+ Hits 22920 22925 +5
- Misses 14493 14514 +21
Partials 1463 1463 ☔ View full report in Codecov by Sentry. |
f2d92f0
to
af1b86f
Compare
test/types.go
Outdated
@@ -118,6 +118,7 @@ type VeleroConfig struct { | |||
ServiceAccountNameToInstall string | |||
EKSPolicyARN string | |||
FailFast bool | |||
DisableVspherePlugin bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change it to "HasVSpherePlugin"? And following "if" will only need to check this single flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that using the positive wording is more intuitive, but we cannot replace all the provider checking with this flag, for example:
// If the CloudProvider is vSphere and don't need vSphere plugin,
// Only install the AWS plugin.
// If do nothing here, the default behavior is
// installing both AWS and vSphere plugins.
if OriginVeleroCfg.CloudProvider == Vsphere &&
OriginVeleroCfg.DisableVspherePlugin {
OriginVeleroCfg.Plugins = migrationNeedPlugins[AWS][0]
}
It should be changed to something like the following.
The HasVspherePlugin flag is defaulted to false, so we need to check the provider to make sure it's for vSphere environment.
// If HasVspherePlugin is false, only install the AWS plugin.
// If do nothing here, the default behavior is
// installing both AWS and vSphere plugins.
if OriginVeleroCfg.CloudProvider == Vsphere &&
!OriginVeleroCfg.HasVspherePlugin {
OriginVeleroCfg.Plugins = migrationNeedPlugins[AWS][0]
}
e62b50b
to
8533394
Compare
* Add new flag HAS_VSPHERE_PLUGIN for E2E test. * Modify the E2E README for the new parameter. * Add the VolumeSnapshotClass for VKS. * Modify the plugin install logic. * Modify the cases to support data mover case in VKS. Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
3bdbc0d
to
dd27aa4
Compare
Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
dd27aa4
to
f9d421e
Compare
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:
make new-changelog
) or comment/kind changelog-not-required
on this PR.site/content/docs/main
.