remove SnapshotService, replace with direct BlockStore usage

Signed-off-by: Steve Kriss <steve@heptio.com>
This commit is contained in:
Steve Kriss
2018-07-27 18:50:35 -07:00
parent 430ec2451a
commit 1c26fbde32
26 changed files with 133 additions and 443 deletions

View File

@@ -55,7 +55,7 @@ type kubernetesBackupper struct {
discoveryHelper discovery.Helper
podCommandExecutor podexec.PodCommandExecutor
groupBackupperFactory groupBackupperFactory
snapshotService cloudprovider.SnapshotService
blockStore cloudprovider.BlockStore
resticBackupperFactory restic.BackupperFactory
resticTimeout time.Duration
}
@@ -93,7 +93,7 @@ func NewKubernetesBackupper(
discoveryHelper discovery.Helper,
dynamicFactory client.DynamicFactory,
podCommandExecutor podexec.PodCommandExecutor,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupperFactory restic.BackupperFactory,
resticTimeout time.Duration,
) (Backupper, error) {
@@ -102,7 +102,7 @@ func NewKubernetesBackupper(
dynamicFactory: dynamicFactory,
podCommandExecutor: podCommandExecutor,
groupBackupperFactory: &defaultGroupBackupperFactory{},
snapshotService: snapshotService,
blockStore: blockStore,
resticBackupperFactory: resticBackupperFactory,
resticTimeout: resticTimeout,
}, nil
@@ -276,7 +276,7 @@ func (kb *kubernetesBackupper) Backup(logger logrus.FieldLogger, backup *api.Bac
kb.podCommandExecutor,
tw,
resourceHooks,
kb.snapshotService,
kb.blockStore,
resticBackupper,
newPVCSnapshotTracker(),
)

View File

@@ -652,7 +652,7 @@ func (f *mockGroupBackupperFactory) newGroupBackupper(
podCommandExecutor podexec.PodCommandExecutor,
tarWriter tarWriter,
resourceHooks []resourceHook,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) groupBackupper {
@@ -669,7 +669,7 @@ func (f *mockGroupBackupperFactory) newGroupBackupper(
podCommandExecutor,
tarWriter,
resourceHooks,
snapshotService,
blockStore,
resticBackupper,
resticSnapshotTracker,
)

View File

@@ -49,7 +49,7 @@ type groupBackupperFactory interface {
podCommandExecutor podexec.PodCommandExecutor,
tarWriter tarWriter,
resourceHooks []resourceHook,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) groupBackupper
@@ -69,7 +69,7 @@ func (f *defaultGroupBackupperFactory) newGroupBackupper(
podCommandExecutor podexec.PodCommandExecutor,
tarWriter tarWriter,
resourceHooks []resourceHook,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) groupBackupper {
@@ -86,7 +86,7 @@ func (f *defaultGroupBackupperFactory) newGroupBackupper(
podCommandExecutor: podCommandExecutor,
tarWriter: tarWriter,
resourceHooks: resourceHooks,
snapshotService: snapshotService,
blockStore: blockStore,
resticBackupper: resticBackupper,
resticSnapshotTracker: resticSnapshotTracker,
resourceBackupperFactory: &defaultResourceBackupperFactory{},
@@ -109,7 +109,7 @@ type defaultGroupBackupper struct {
podCommandExecutor podexec.PodCommandExecutor
tarWriter tarWriter
resourceHooks []resourceHook
snapshotService cloudprovider.SnapshotService
blockStore cloudprovider.BlockStore
resticBackupper restic.Backupper
resticSnapshotTracker *pvcSnapshotTracker
resourceBackupperFactory resourceBackupperFactory
@@ -133,7 +133,7 @@ func (gb *defaultGroupBackupper) backupGroup(group *metav1.APIResourceList) erro
gb.podCommandExecutor,
gb.tarWriter,
gb.resourceHooks,
gb.snapshotService,
gb.blockStore,
gb.resticBackupper,
gb.resticSnapshotTracker,
)

View File

@@ -161,7 +161,7 @@ func (rbf *mockResourceBackupperFactory) newResourceBackupper(
podCommandExecutor podexec.PodCommandExecutor,
tarWriter tarWriter,
resourceHooks []resourceHook,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) resourceBackupper {
@@ -178,7 +178,7 @@ func (rbf *mockResourceBackupperFactory) newResourceBackupper(
podCommandExecutor,
tarWriter,
resourceHooks,
snapshotService,
blockStore,
resticBackupper,
resticSnapshotTracker,
)

View File

@@ -54,7 +54,7 @@ type itemBackupperFactory interface {
resourceHooks []resourceHook,
dynamicFactory client.DynamicFactory,
discoveryHelper discovery.Helper,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) ItemBackupper
@@ -72,7 +72,7 @@ func (f *defaultItemBackupperFactory) newItemBackupper(
resourceHooks []resourceHook,
dynamicFactory client.DynamicFactory,
discoveryHelper discovery.Helper,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) ItemBackupper {
@@ -86,7 +86,7 @@ func (f *defaultItemBackupperFactory) newItemBackupper(
resourceHooks: resourceHooks,
dynamicFactory: dynamicFactory,
discoveryHelper: discoveryHelper,
snapshotService: snapshotService,
blockStore: blockStore,
itemHookHandler: &defaultItemHookHandler{
podCommandExecutor: podCommandExecutor,
},
@@ -114,7 +114,7 @@ type defaultItemBackupper struct {
resourceHooks []resourceHook
dynamicFactory client.DynamicFactory
discoveryHelper discovery.Helper
snapshotService cloudprovider.SnapshotService
blockStore cloudprovider.BlockStore
resticBackupper restic.Backupper
resticSnapshotTracker *pvcSnapshotTracker
@@ -219,7 +219,7 @@ func (ib *defaultItemBackupper) backupItem(logger logrus.FieldLogger, obj runtim
obj = updatedObj
if groupResource == kuberesource.PersistentVolumes {
if ib.snapshotService == nil {
if ib.blockStore == nil {
log.Debug("Skipping Persistent Volume snapshot because they're not enabled.")
} else if err := ib.takePVSnapshot(obj, ib.backup, log); err != nil {
backupErrs = append(backupErrs, err)
@@ -399,7 +399,7 @@ func (ib *defaultItemBackupper) takePVSnapshot(obj runtime.Unstructured, backup
log.Infof("label %q is not present on PersistentVolume", zoneLabel)
}
volumeID, err := ib.snapshotService.GetVolumeID(obj)
volumeID, err := ib.blockStore.GetVolumeID(obj)
if err != nil {
return errors.Wrapf(err, "error getting volume ID for PersistentVolume")
}
@@ -416,14 +416,14 @@ func (ib *defaultItemBackupper) takePVSnapshot(obj runtime.Unstructured, backup
}
log.Info("Snapshotting PersistentVolume")
snapshotID, err := ib.snapshotService.CreateSnapshot(volumeID, pvFailureDomainZone, tags)
snapshotID, err := ib.blockStore.CreateSnapshot(volumeID, pvFailureDomainZone, tags)
if err != nil {
// log+error on purpose - log goes to the per-backup log file, error goes to the backup
log.WithError(err).Error("error creating snapshot")
return errors.WithMessage(err, "error creating snapshot")
}
volumeType, iops, err := ib.snapshotService.GetVolumeInfo(volumeID, pvFailureDomainZone)
volumeType, iops, err := ib.blockStore.GetVolumeInfo(volumeID, pvFailureDomainZone)
if err != nil {
log.WithError(err).Error("error getting volume info")
return errors.WithMessage(err, "error getting volume info")

View File

@@ -277,7 +277,7 @@ func TestBackupItemNoSkips(t *testing.T) {
additionalItemError: errors.New("foo"),
},
{
name: "takePVSnapshot is not invoked for PVs when snapshotService == nil",
name: "takePVSnapshot is not invoked for PVs when blockStore == nil",
namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"),
item: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/zone": "us-east-1c"}}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-east-1c/vol-abc123"}}}`,
expectError: false,
@@ -286,7 +286,7 @@ func TestBackupItemNoSkips(t *testing.T) {
groupResource: "persistentvolumes",
},
{
name: "takePVSnapshot is invoked for PVs when snapshotService != nil",
name: "takePVSnapshot is invoked for PVs when blockStore != nil",
namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"),
item: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv", "labels": {"failure-domain.beta.kubernetes.io/zone": "us-east-1c"}}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-east-1c/vol-abc123"}}}`,
expectError: false,
@@ -305,7 +305,7 @@ func TestBackupItemNoSkips(t *testing.T) {
expectExcluded: false,
expectedTarHeaderName: "resources/persistentvolumes/cluster/mypv.json",
groupResource: "persistentvolumes",
// empty snapshottableVolumes causes a snapshotService to be created, but no
// empty snapshottableVolumes causes a blockStore to be created, but no
// snapshots are expected to be taken.
snapshottableVolumes: map[string]api.VolumeBackupInfo{},
trackedPVCs: sets.NewString(key("pvc-ns", "pvc"), key("another-pvc-ns", "another-pvc")),
@@ -419,14 +419,14 @@ func TestBackupItemNoSkips(t *testing.T) {
newPVCSnapshotTracker(),
).(*defaultItemBackupper)
var snapshotService *arktest.FakeSnapshotService
var blockStore *arktest.FakeBlockStore
if test.snapshottableVolumes != nil {
snapshotService = &arktest.FakeSnapshotService{
blockStore = &arktest.FakeBlockStore{
SnapshottableVolumes: test.snapshottableVolumes,
VolumeID: "vol-abc123",
Error: test.snapshotError,
}
b.snapshotService = snapshotService
b.blockStore = blockStore
}
if test.trackedPVCs != nil {
@@ -514,7 +514,7 @@ func TestBackupItemNoSkips(t *testing.T) {
}
if test.snapshottableVolumes != nil {
require.Equal(t, len(test.snapshottableVolumes), len(snapshotService.SnapshotsTaken))
require.Equal(t, len(test.snapshottableVolumes), len(blockStore.SnapshotsTaken))
var expectedBackups []api.VolumeBackupInfo
for _, vbi := range test.snapshottableVolumes {
@@ -719,12 +719,12 @@ func TestTakePVSnapshot(t *testing.T) {
},
}
snapshotService := &arktest.FakeSnapshotService{
blockStore := &arktest.FakeBlockStore{
SnapshottableVolumes: test.volumeInfo,
VolumeID: test.expectedVolumeID,
}
ib := &defaultItemBackupper{snapshotService: snapshotService}
ib := &defaultItemBackupper{blockStore: blockStore}
pv, err := arktest.GetAsMap(test.pv)
if err != nil {
@@ -754,12 +754,12 @@ func TestTakePVSnapshot(t *testing.T) {
}
// we should have one snapshot taken exactly
require.Equal(t, test.expectedSnapshotsTaken, snapshotService.SnapshotsTaken.Len())
require.Equal(t, test.expectedSnapshotsTaken, blockStore.SnapshotsTaken.Len())
if test.expectedSnapshotsTaken > 0 {
// the snapshotID should be the one in the entry in snapshotService.SnapshottableVolumes
// the snapshotID should be the one in the entry in blockStore.SnapshottableVolumes
// for the volume we ran the test for
snapshotID, _ := snapshotService.SnapshotsTaken.PopAny()
snapshotID, _ := blockStore.SnapshotsTaken.PopAny()
expectedVolumeBackups["mypv"] = &v1.VolumeBackupInfo{
SnapshotID: snapshotID,

View File

@@ -51,7 +51,7 @@ type resourceBackupperFactory interface {
podCommandExecutor podexec.PodCommandExecutor,
tarWriter tarWriter,
resourceHooks []resourceHook,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) resourceBackupper
@@ -72,7 +72,7 @@ func (f *defaultResourceBackupperFactory) newResourceBackupper(
podCommandExecutor podexec.PodCommandExecutor,
tarWriter tarWriter,
resourceHooks []resourceHook,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) resourceBackupper {
@@ -89,7 +89,7 @@ func (f *defaultResourceBackupperFactory) newResourceBackupper(
podCommandExecutor: podCommandExecutor,
tarWriter: tarWriter,
resourceHooks: resourceHooks,
snapshotService: snapshotService,
blockStore: blockStore,
resticBackupper: resticBackupper,
resticSnapshotTracker: resticSnapshotTracker,
itemBackupperFactory: &defaultItemBackupperFactory{},
@@ -113,7 +113,7 @@ type defaultResourceBackupper struct {
podCommandExecutor podexec.PodCommandExecutor
tarWriter tarWriter
resourceHooks []resourceHook
snapshotService cloudprovider.SnapshotService
blockStore cloudprovider.BlockStore
resticBackupper restic.Backupper
resticSnapshotTracker *pvcSnapshotTracker
itemBackupperFactory itemBackupperFactory
@@ -189,7 +189,7 @@ func (rb *defaultResourceBackupper) backupResource(
rb.resourceHooks,
rb.dynamicFactory,
rb.discoveryHelper,
rb.snapshotService,
rb.blockStore,
rb.resticBackupper,
rb.resticSnapshotTracker,
)

View File

@@ -544,7 +544,6 @@ func TestBackupResourceOnlyIncludesSpecifiedNamespaces(t *testing.T) {
dynamicFactory: dynamicFactory,
discoveryHelper: discoveryHelper,
itemHookHandler: itemHookHandler,
snapshotService: nil,
}
itemBackupperFactory.On("newItemBackupper",
@@ -690,7 +689,7 @@ func (ibf *mockItemBackupperFactory) newItemBackupper(
resourceHooks []resourceHook,
dynamicFactory client.DynamicFactory,
discoveryHelper discovery.Helper,
snapshotService cloudprovider.SnapshotService,
blockStore cloudprovider.BlockStore,
resticBackupper restic.Backupper,
resticSnapshotTracker *pvcSnapshotTracker,
) ItemBackupper {
@@ -705,7 +704,7 @@ func (ibf *mockItemBackupperFactory) newItemBackupper(
resourceHooks,
dynamicFactory,
discoveryHelper,
snapshotService,
blockStore,
resticBackupper,
resticSnapshotTracker,
)