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

Skipping validation for volumesnapshotlocation for backup if snapshotvolume set to false #2450

Merged
merged 3 commits into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/2450-mynktl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support to skip VSL validation for the backup having SnapshotVolumes set to false or created with `--snapshot-volumes=false`
6 changes: 6 additions & 0 deletions pkg/controller/backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,10 +397,16 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg
// - a given provider's default location name is added to .spec.volumeSnapshotLocations if one
// is not explicitly specified for the provider (if there's only one location for the provider,
// it will automatically be used)
// if backup has snapshotVolume disabled then it returns empty VSL
func (c *backupController) validateAndGetSnapshotLocations(backup *velerov1api.Backup) (map[string]*velerov1api.VolumeSnapshotLocation, []string) {
errors := []string{}
providerLocations := make(map[string]*velerov1api.VolumeSnapshotLocation)

// if snapshotVolume is set to false then we don't need to validate volumesnapshotlocation
if backup.Spec.SnapshotVolumes != nil && !*backup.Spec.SnapshotVolumes {
skriss marked this conversation as resolved.
Show resolved Hide resolved
return nil, nil
}

for _, locationName := range backup.Spec.VolumeSnapshotLocations {
// validate each locationName exists as a VolumeSnapshotLocation
location, err := c.snapshotLocationLister.VolumeSnapshotLocations(backup.Namespace).Get(locationName)
Expand Down
40 changes: 40 additions & 0 deletions pkg/controller/backup_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,46 @@ func TestValidateAndGetSnapshotLocations(t *testing.T) {
expectedVolumeSnapshotLocationNames: []string{"aws-us-west-1", "some-name"},
expectedSuccess: true,
},
{
name: "location name does not correspond to any existing location and snapshotvolume disabled; should return empty VSL and no error",
backup: defaultBackup().Phase(velerov1api.BackupPhaseNew).VolumeSnapshotLocations("random-name").SnapshotVolumes(false).Result(),
locations: []*velerov1api.VolumeSnapshotLocation{
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-east-1").Provider("aws").Result(),
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-west-1").Provider("aws").Result(),
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "some-name").Provider("fake-provider").Result(),
},
expectedVolumeSnapshotLocationNames: nil,
expectedSuccess: true,
},
{
name: "duplicate locationName per provider and snapshotvolume disabled; should return empty VSL and no error",
backup: defaultBackup().Phase(velerov1api.BackupPhaseNew).VolumeSnapshotLocations("aws-us-west-1", "aws-us-west-1").SnapshotVolumes(false).Result(),
locations: []*velerov1api.VolumeSnapshotLocation{
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-east-1").Provider("aws").Result(),
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-west-1").Provider("aws").Result(),
},
expectedVolumeSnapshotLocationNames: nil,
expectedSuccess: true,
},
{
name: "no location name for the provider exists, only one VSL created and snapshotvolume disabled; should return empty VSL and no error",
backup: defaultBackup().Phase(velerov1api.BackupPhaseNew).SnapshotVolumes(false).Result(),
locations: []*velerov1api.VolumeSnapshotLocation{
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-east-1").Provider("aws").Result(),
},
expectedVolumeSnapshotLocationNames: nil,
expectedSuccess: true,
},
{
name: "multiple location names for a provider, no default location and backup has no location defined, but snapshotvolume disabled, should return empty VSL and no error",
backup: defaultBackup().Phase(velerov1api.BackupPhaseNew).SnapshotVolumes(false).Result(),
locations: []*velerov1api.VolumeSnapshotLocation{
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-west-1").Provider("aws").Result(),
builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "aws-us-east-1").Provider("aws").Result(),
},
expectedVolumeSnapshotLocationNames: nil,
expectedSuccess: true,
},
}

for _, test := range tests {
Expand Down