fix issue #1464: Added VSL Backup/Restore Tests #1552
Conversation
|
/retest |
stillalearner
force-pushed
the
add_vsl_backup_restore_test
branch
2 times, most recently
from
f3c4da7
to
48c1618
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Thanks. |
stillalearner
force-pushed
the
add_vsl_backup_restore_test
branch
from
48c1618
to
6ed4f85
Compare
stillalearner
force-pushed
the
add_vsl_backup_restore_test
branch
from
6ed4f85
to
0ceebe0
Compare
stillalearner
force-pushed
the
add_vsl_backup_restore_test
branch
from
0ceebe0
to
d560441
Compare
|
/retest |
stillalearner
force-pushed
the
add_vsl_backup_restore_test
branch
from
d560441
to
11ce9c4
Compare
|
/retest |
|
@stillalearner: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-ci
bot
added
the
approved
| @@ -68,6 +68,8 @@ func prepareBackupAndRestore(brCase BackupRestoreCase, updateLastInstallTime fun | |||
| gomega.Eventually(lib.AreNodeAgentPodsRunning(kubernetesClientForSuiteRun, namespace), time.Minute*3, time.Second*5).Should(gomega.BeTrue()) | |||
| } | |||
|
|
|||
| // Velero does not change status of VSL objects. Users can only confirm if VSLs are correct configured when running a native snapshot backup/restore | |||
There was a problem hiding this comment.
I think it was said in some scrum in 2021/2022 that we don't need VSL tests when implementing e2e due to "CSI is the future, we will not suggests VSL to anyone new onboarding". Just an info.
There was a problem hiding this comment.
your call. I guess keeping tests if functionality is there / not deprecated yet is fine..
There was a problem hiding this comment.
adding tests is fine yes. Pretty sure we had it at some point here
| @@ -112,6 +114,13 @@ func (v *DpaCustomResource) Build(backupRestoreType BackupRestoreType) *oadpv1al | |||
| dpaSpec.Configuration.Velero.DefaultPlugins = append(dpaSpec.Configuration.Velero.DefaultPlugins, oadpv1alpha1.DefaultPluginCSI) | |||
| dpaSpec.Configuration.Velero.FeatureFlags = append(dpaSpec.Configuration.Velero.FeatureFlags, velero.CSIFeatureFlag) | |||
| dpaSpec.SnapshotLocations = nil | |||
| case NativeSnapshots: | |||
| dpaSpec.SnapshotLocations[0].Velero.Credential = &corev1.SecretKeySelector{ | |||
There was a problem hiding this comment.
Shouldn't we just load the VSL spec only for the NativeSnapshots spec ? Here we are only adding the credential reference, IMO we should configure VSL only for this case and not by default.
There was a problem hiding this comment.
It is added to the default aws_settings right now.
The region field in VSL spec is dynamically filled based on the infra. So,i guess it would be better to load it by default from there, anyways it doesnt do any harm, if we choose FSB, or Datamover, that will be done..what say?
There was a problem hiding this comment.
Cool, I am fine with it.
|
/override ci/prow/4.17-e2e-test-kubevirt-aws |
|
@kaovilai: Overrode contexts on behalf of kaovilai: ci/prow/4.17-e2e-test-kubevirt-aws In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, mateusoliveira43, shubham-pampattiwar, stillalearner The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
Why the changes were made
This PR adds VSL backup / restore tests to the e2e tests, for both mysql and mongo apps.
Fixes #1464
How to test the changes made
Use "make test-e2e" and run the new test cases with proper ginkgo variables.