Default and validate VolumeSnapshotLocations
Signed-off-by: Carlisia <carlisia@grokkingtech.io>pull/948/head
parent
bbf769850e
commit
1aa712d236
|
@ -636,6 +636,8 @@ func (s *server) runControllers(config *api.Config) error {
|
|||
backupTracker,
|
||||
s.sharedInformerFactory.Ark().V1().BackupStorageLocations(),
|
||||
s.config.defaultBackupLocation,
|
||||
s.sharedInformerFactory.Ark().V1().VolumeSnapshotLocations(),
|
||||
nil,
|
||||
s.metrics,
|
||||
)
|
||||
wg.Add(1)
|
||||
|
|
|
@ -55,18 +55,20 @@ const backupVersion = 1
|
|||
type backupController struct {
|
||||
*genericController
|
||||
|
||||
backupper backup.Backupper
|
||||
pvProviderExists bool
|
||||
lister listers.BackupLister
|
||||
client arkv1client.BackupsGetter
|
||||
clock clock.Clock
|
||||
backupLogLevel logrus.Level
|
||||
newPluginManager func(logrus.FieldLogger) plugin.Manager
|
||||
backupTracker BackupTracker
|
||||
backupLocationLister listers.BackupStorageLocationLister
|
||||
defaultBackupLocation string
|
||||
metrics *metrics.ServerMetrics
|
||||
newBackupStore func(*api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
|
||||
backupper backup.Backupper
|
||||
pvProviderExists bool
|
||||
lister listers.BackupLister
|
||||
client arkv1client.BackupsGetter
|
||||
clock clock.Clock
|
||||
backupLogLevel logrus.Level
|
||||
newPluginManager func(logrus.FieldLogger) plugin.Manager
|
||||
backupTracker BackupTracker
|
||||
backupLocationLister listers.BackupStorageLocationLister
|
||||
defaultBackupLocation string
|
||||
snapshotLocationLister listers.VolumeSnapshotLocationLister
|
||||
defaultSnapshotLocations map[string]string
|
||||
metrics *metrics.ServerMetrics
|
||||
newBackupStore func(*api.BackupStorageLocation, persistence.ObjectStoreGetter, logrus.FieldLogger) (persistence.BackupStore, error)
|
||||
}
|
||||
|
||||
func NewBackupController(
|
||||
|
@ -80,21 +82,25 @@ func NewBackupController(
|
|||
backupTracker BackupTracker,
|
||||
backupLocationInformer informers.BackupStorageLocationInformer,
|
||||
defaultBackupLocation string,
|
||||
volumeSnapshotLocationInformer informers.VolumeSnapshotLocationInformer,
|
||||
defaultSnapshotLocations map[string]string,
|
||||
metrics *metrics.ServerMetrics,
|
||||
) Interface {
|
||||
c := &backupController{
|
||||
genericController: newGenericController("backup", logger),
|
||||
backupper: backupper,
|
||||
pvProviderExists: pvProviderExists,
|
||||
lister: backupInformer.Lister(),
|
||||
client: client,
|
||||
clock: &clock.RealClock{},
|
||||
backupLogLevel: backupLogLevel,
|
||||
newPluginManager: newPluginManager,
|
||||
backupTracker: backupTracker,
|
||||
backupLocationLister: backupLocationInformer.Lister(),
|
||||
defaultBackupLocation: defaultBackupLocation,
|
||||
metrics: metrics,
|
||||
genericController: newGenericController("backup", logger),
|
||||
backupper: backupper,
|
||||
pvProviderExists: pvProviderExists,
|
||||
lister: backupInformer.Lister(),
|
||||
client: client,
|
||||
clock: &clock.RealClock{},
|
||||
backupLogLevel: backupLogLevel,
|
||||
newPluginManager: newPluginManager,
|
||||
backupTracker: backupTracker,
|
||||
backupLocationLister: backupLocationInformer.Lister(),
|
||||
defaultBackupLocation: defaultBackupLocation,
|
||||
snapshotLocationLister: volumeSnapshotLocationInformer.Lister(),
|
||||
defaultSnapshotLocations: defaultSnapshotLocations,
|
||||
metrics: metrics,
|
||||
|
||||
newBackupStore: persistence.NewObjectBackupStore,
|
||||
}
|
||||
|
@ -103,6 +109,7 @@ func NewBackupController(
|
|||
c.cacheSyncWaiters = append(c.cacheSyncWaiters,
|
||||
backupInformer.Informer().HasSynced,
|
||||
backupLocationInformer.Informer().HasSynced,
|
||||
volumeSnapshotLocationInformer.Informer().HasSynced,
|
||||
)
|
||||
|
||||
backupInformer.Informer().AddEventHandler(
|
||||
|
@ -178,9 +185,11 @@ func (c *backupController) processBackup(key string) error {
|
|||
backup.Status.Expiration = metav1.NewTime(c.clock.Now().Add(backup.Spec.TTL.Duration))
|
||||
}
|
||||
|
||||
var backupLocation *api.BackupStorageLocation
|
||||
// validation
|
||||
if backupLocation, backup.Status.ValidationErrors = c.getLocationAndValidate(backup, c.defaultBackupLocation); len(backup.Status.ValidationErrors) > 0 {
|
||||
backupLocation, errs := c.getLocationAndValidate(backup, c.defaultBackupLocation)
|
||||
errs = append(errs, c.defaultAndValidateSnapshotLocations(backup, c.defaultSnapshotLocations)...)
|
||||
backup.Status.ValidationErrors = append(backup.Status.ValidationErrors, errs...)
|
||||
|
||||
if len(backup.Status.ValidationErrors) > 0 {
|
||||
backup.Status.Phase = api.BackupPhaseFailedValidation
|
||||
} else {
|
||||
backup.Status.Phase = api.BackupPhaseInProgress
|
||||
|
@ -258,10 +267,6 @@ func (c *backupController) getLocationAndValidate(itm *api.Backup, defaultBackup
|
|||
validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err))
|
||||
}
|
||||
|
||||
if !c.pvProviderExists && itm.Spec.SnapshotVolumes != nil && *itm.Spec.SnapshotVolumes {
|
||||
validationErrors = append(validationErrors, "Server is not configured for PV snapshots")
|
||||
}
|
||||
|
||||
if itm.Spec.StorageLocation == "" {
|
||||
itm.Spec.StorageLocation = defaultBackupLocation
|
||||
}
|
||||
|
@ -281,6 +286,53 @@ func (c *backupController) getLocationAndValidate(itm *api.Backup, defaultBackup
|
|||
return backupLocation, validationErrors
|
||||
}
|
||||
|
||||
// defaultAndValidateSnapshotLocations ensures:
|
||||
// - each location name in Spec VolumeSnapshotLocation exists as a location
|
||||
// - exactly 1 location per existing or default provider
|
||||
// - a given default provider's location name is added to the Spec VolumeSnapshotLocation if it does not exist as a VSL
|
||||
func (c *backupController) defaultAndValidateSnapshotLocations(itm *api.Backup, defaultLocations map[string]string) []string {
|
||||
var errors []string
|
||||
perProviderLocationName := make(map[string]string)
|
||||
var finalLocationNameList []string
|
||||
for _, locationName := range itm.Spec.VolumeSnapshotLocations {
|
||||
// validate each locationName exists as a VolumeSnapshotLocation
|
||||
location, err := c.snapshotLocationLister.VolumeSnapshotLocations(itm.Namespace).Get(locationName)
|
||||
if err != nil {
|
||||
errors = append(errors, fmt.Sprintf("error getting volume snapshot location named %s: %v", locationName, err))
|
||||
continue
|
||||
}
|
||||
|
||||
// ensure we end up with exactly 1 locationName *per provider*
|
||||
providerLocationName := perProviderLocationName[location.Spec.Provider]
|
||||
if providerLocationName != "" {
|
||||
// if > 1 location name per provider as in ["aws-us-east-1" | "aws-us-west-1"] (same provider, multiple names)
|
||||
if providerLocationName != locationName {
|
||||
errors = append(errors, fmt.Sprintf("more than one VolumeSnapshotLocation name specified for provider %s: %s; unexpected name was %s", location.Spec.Provider, locationName, providerLocationName))
|
||||
continue
|
||||
}
|
||||
} else {
|
||||
// no dup exists: add locationName to the final list
|
||||
finalLocationNameList = append(finalLocationNameList, locationName)
|
||||
// keep track of all valid existing locations, per provider
|
||||
perProviderLocationName[location.Spec.Provider] = locationName
|
||||
}
|
||||
}
|
||||
|
||||
if len(errors) > 0 {
|
||||
return errors
|
||||
}
|
||||
|
||||
for provider, defaultLocationName := range defaultLocations {
|
||||
// if a location name for a given provider does not already exist, add the provider's default
|
||||
if _, ok := perProviderLocationName[provider]; !ok {
|
||||
finalLocationNameList = append(finalLocationNameList, defaultLocationName)
|
||||
}
|
||||
}
|
||||
itm.Spec.VolumeSnapshotLocations = finalLocationNameList
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *backupController) runBackup(backup *api.Backup, backupLocation *api.BackupStorageLocation) error {
|
||||
log := c.logger.WithField("backup", kubeutil.NamespaceAndName(backup))
|
||||
log.Info("Starting backup")
|
||||
|
|
|
@ -24,6 +24,7 @@ import (
|
|||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
"github.com/sirupsen/logrus"
|
||||
"github.com/stretchr/testify/mock"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
@ -66,6 +67,7 @@ func TestProcessBackup(t *testing.T) {
|
|||
backup *arktest.TestBackup
|
||||
expectBackup bool
|
||||
allowSnapshots bool
|
||||
defaultLocations map[string]string
|
||||
}{
|
||||
{
|
||||
name: "bad key",
|
||||
|
@ -198,6 +200,8 @@ func TestProcessBackup(t *testing.T) {
|
|||
NewBackupTracker(),
|
||||
sharedInformers.Ark().V1().BackupStorageLocations(),
|
||||
"default",
|
||||
sharedInformers.Ark().V1().VolumeSnapshotLocations(),
|
||||
test.defaultLocations,
|
||||
metrics.NewServerMetrics(),
|
||||
).(*backupController)
|
||||
|
||||
|
@ -422,3 +426,121 @@ func TestProcessBackup(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultAndValidateSnapshotLocations(t *testing.T) {
|
||||
defaultLocationsAWS := map[string]string{"aws": "aws-us-east-2"}
|
||||
defaultLocationsFake := map[string]string{"fake-provider": "some-name"}
|
||||
|
||||
multipleLocationNames := []string{"aws-us-west-1", "aws-us-east-1"}
|
||||
|
||||
multipleLocation1 := arktest.LocationInfo{
|
||||
Name: multipleLocationNames[0],
|
||||
Provider: "aws",
|
||||
Config: map[string]string{"region": "us-west-1"},
|
||||
}
|
||||
multipleLocation2 := arktest.LocationInfo{
|
||||
Name: multipleLocationNames[1],
|
||||
Provider: "aws",
|
||||
Config: map[string]string{"region": "us-west-1"},
|
||||
}
|
||||
|
||||
multipleLocationList := []arktest.LocationInfo{multipleLocation1, multipleLocation2}
|
||||
|
||||
dupLocationNames := []string{"aws-us-west-1", "aws-us-west-1"}
|
||||
dupLocation1 := arktest.LocationInfo{
|
||||
Name: dupLocationNames[0],
|
||||
Provider: "aws",
|
||||
Config: map[string]string{"region": "us-west-1"},
|
||||
}
|
||||
dupLocation2 := arktest.LocationInfo{
|
||||
Name: dupLocationNames[0],
|
||||
Provider: "aws",
|
||||
Config: map[string]string{"region": "us-west-1"},
|
||||
}
|
||||
dupLocationList := []arktest.LocationInfo{dupLocation1, dupLocation2}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
backup *arktest.TestBackup
|
||||
locations []*arktest.TestVolumeSnapshotLocation
|
||||
defaultLocations map[string]string
|
||||
expectedVolumeSnapshotLocationNames []string // adding these in the expected order will allow to test with better msgs in case of a test failure
|
||||
expectedErrors string
|
||||
expectedSuccess bool
|
||||
}{
|
||||
{
|
||||
name: "location name does not correspond to any existing location",
|
||||
backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations([]string{"random-name"}),
|
||||
locations: arktest.NewTestVolumeSnapshotLocation().WithName(dupLocationNames[0]).WithProviderConfig(dupLocationList),
|
||||
expectedErrors: "error getting volume snapshot location named random-name: volumesnapshotlocation.ark.heptio.com \"random-name\" not found",
|
||||
expectedSuccess: false,
|
||||
},
|
||||
{
|
||||
name: "duplicate locationName per provider: should filter out dups",
|
||||
backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(dupLocationNames),
|
||||
locations: arktest.NewTestVolumeSnapshotLocation().WithName(dupLocationNames[0]).WithProviderConfig(dupLocationList),
|
||||
expectedVolumeSnapshotLocationNames: []string{dupLocationNames[0]},
|
||||
expectedSuccess: true,
|
||||
},
|
||||
{
|
||||
name: "multiple location names per provider",
|
||||
backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(multipleLocationNames),
|
||||
locations: arktest.NewTestVolumeSnapshotLocation().WithName(multipleLocationNames[0]).WithProviderConfig(multipleLocationList),
|
||||
expectedErrors: "more than one VolumeSnapshotLocation name specified for provider aws: aws-us-east-1; unexpected name was aws-us-west-1",
|
||||
expectedSuccess: false,
|
||||
},
|
||||
{
|
||||
name: "no location name for the provider exists: the provider's default should be added",
|
||||
backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew),
|
||||
defaultLocations: defaultLocationsAWS,
|
||||
expectedVolumeSnapshotLocationNames: []string{defaultLocationsAWS["aws"]},
|
||||
expectedSuccess: true,
|
||||
},
|
||||
{
|
||||
name: "no existing location name and no default location name given",
|
||||
backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew),
|
||||
expectedSuccess: true,
|
||||
},
|
||||
{
|
||||
name: "multiple location names for a provider, default location name for another provider",
|
||||
backup: arktest.NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithVolumeSnapshotLocations(dupLocationNames),
|
||||
locations: arktest.NewTestVolumeSnapshotLocation().WithName(dupLocationNames[0]).WithProviderConfig(dupLocationList),
|
||||
defaultLocations: defaultLocationsFake,
|
||||
expectedVolumeSnapshotLocationNames: []string{dupLocationNames[0], defaultLocationsFake["fake-provider"]},
|
||||
expectedSuccess: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
var (
|
||||
client = fake.NewSimpleClientset()
|
||||
sharedInformers = informers.NewSharedInformerFactory(client, 0)
|
||||
)
|
||||
|
||||
c := &backupController{
|
||||
snapshotLocationLister: sharedInformers.Ark().V1().VolumeSnapshotLocations().Lister(),
|
||||
}
|
||||
|
||||
// set up a Backup object to represent what we expect to be passed to backupper.Backup()
|
||||
backup := test.backup.DeepCopy()
|
||||
backup.Spec.VolumeSnapshotLocations = test.backup.Spec.VolumeSnapshotLocations
|
||||
for _, location := range test.locations {
|
||||
require.NoError(t, sharedInformers.Ark().V1().VolumeSnapshotLocations().Informer().GetStore().Add(location.VolumeSnapshotLocation))
|
||||
}
|
||||
|
||||
errs := c.defaultAndValidateSnapshotLocations(backup, test.defaultLocations)
|
||||
if test.expectedSuccess {
|
||||
for _, err := range errs {
|
||||
require.NoError(t, errors.New(err), "defaultAndValidateSnapshotLocations unexpected error: %v", err)
|
||||
}
|
||||
require.Equal(t, test.expectedVolumeSnapshotLocationNames, backup.Spec.VolumeSnapshotLocations)
|
||||
} else {
|
||||
if len(errs) == 0 {
|
||||
require.Error(t, nil, "defaultAndValidateSnapshotLocations expected error")
|
||||
}
|
||||
require.Contains(t, errs, test.expectedErrors)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
@ -140,3 +140,8 @@ func (b *TestBackup) WithStorageLocation(location string) *TestBackup {
|
|||
b.Spec.StorageLocation = location
|
||||
return b
|
||||
}
|
||||
|
||||
func (b *TestBackup) WithVolumeSnapshotLocations(locations []string) *TestBackup {
|
||||
b.Spec.VolumeSnapshotLocations = locations
|
||||
return b
|
||||
}
|
||||
|
|
|
@ -0,0 +1,72 @@
|
|||
/*
|
||||
Copyright 2018 the Heptio Ark contributors.
|
||||
|
||||
Licensed under the Apache License, Version 2.0 (the "License");
|
||||
you may not use this file except in compliance with the License.
|
||||
You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing, software
|
||||
distributed under the License is distributed on an "AS IS" BASIS,
|
||||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
See the License for the specific language governing permissions and
|
||||
limitations under the License.
|
||||
*/
|
||||
|
||||
package test
|
||||
|
||||
import (
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
|
||||
"github.com/heptio/ark/pkg/apis/ark/v1"
|
||||
)
|
||||
|
||||
type TestVolumeSnapshotLocation struct {
|
||||
*v1.VolumeSnapshotLocation
|
||||
}
|
||||
|
||||
func NewTestVolumeSnapshotLocation() *TestVolumeSnapshotLocation {
|
||||
return &TestVolumeSnapshotLocation{
|
||||
VolumeSnapshotLocation: &v1.VolumeSnapshotLocation{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: v1.DefaultNamespace,
|
||||
},
|
||||
Spec: v1.VolumeSnapshotLocationSpec{
|
||||
Provider: "aws",
|
||||
Config: map[string]string{"region": "us-west-1"},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func (location *TestVolumeSnapshotLocation) WithName(name string) *TestVolumeSnapshotLocation {
|
||||
location.Name = name
|
||||
return location
|
||||
}
|
||||
|
||||
func (location *TestVolumeSnapshotLocation) WithProviderConfig(info []LocationInfo) []*TestVolumeSnapshotLocation {
|
||||
var locations []*TestVolumeSnapshotLocation
|
||||
|
||||
for _, v := range info {
|
||||
location := &TestVolumeSnapshotLocation{
|
||||
VolumeSnapshotLocation: &v1.VolumeSnapshotLocation{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Name: v.Name,
|
||||
Namespace: v1.DefaultNamespace,
|
||||
},
|
||||
Spec: v1.VolumeSnapshotLocationSpec{
|
||||
Provider: v.Provider,
|
||||
Config: v.Config,
|
||||
},
|
||||
},
|
||||
}
|
||||
locations = append(locations, location)
|
||||
}
|
||||
return locations
|
||||
}
|
||||
|
||||
type LocationInfo struct {
|
||||
Name, Provider string
|
||||
Config map[string]string
|
||||
}
|
Loading…
Reference in New Issue