diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index b94303b492..28b6fbe7d8 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -206,7 +206,9 @@ func (ib *defaultItemBackupper) backupItem(logger logrus.FieldLogger, obj runtim return err } - ib.additionalItemBackupper.backupItem(log, additionalItem, gvr.GroupResource()) + if err = ib.additionalItemBackupper.backupItem(log, additionalItem, gvr.GroupResource()); err != nil { + return err + } } } else { // We want this to show up in the log file at the place where the error occurs. When we return diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 73917d139c..7d825f5108 100644 --- a/pkg/backup/item_backupper_test.go +++ b/pkg/backup/item_backupper_test.go @@ -139,6 +139,8 @@ func TestBackupItemNoSkips(t *testing.T) { customActionAdditionalItems []runtime.Unstructured groupResource string snapshottableVolumes map[string]api.VolumeBackupInfo + snapshotError error + additionalItemError error }{ { name: "explicit namespace include", @@ -221,6 +223,33 @@ func TestBackupItemNoSkips(t *testing.T) { unstructuredOrDie(`{"apiVersion":"g2/v1","kind":"r1","metadata":{"namespace":"ns2","name":"n2"}}`), }, }, + { + name: "action invoked - additional items - error", + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + item: `{"metadata":{"namespace": "myns", "name":"bar"}}`, + expectError: true, + expectExcluded: false, + expectedTarHeaderName: "resources/resource.group/namespaces/myns/bar.json", + customAction: true, + expectedActionID: "myns/bar", + customActionAdditionalItemIdentifiers: []ResourceIdentifier{ + { + GroupResource: schema.GroupResource{Group: "g1", Resource: "r1"}, + Namespace: "ns1", + Name: "n1", + }, + { + GroupResource: schema.GroupResource{Group: "g2", Resource: "r2"}, + Namespace: "ns2", + Name: "n2", + }, + }, + customActionAdditionalItems: []runtime.Unstructured{ + unstructuredOrDie(`{"apiVersion":"g1/v1","kind":"r1","metadata":{"namespace":"ns1","name":"n1"}}`), + unstructuredOrDie(`{"apiVersion":"g2/v1","kind":"r1","metadata":{"namespace":"ns2","name":"n2"}}`), + }, + additionalItemError: errors.New("foo"), + }, { name: "takePVSnapshot is not invoked for PVs when snapshotService == nil", namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), @@ -242,6 +271,17 @@ func TestBackupItemNoSkips(t *testing.T) { "vol-abc123": {SnapshotID: "snapshot-1", AvailabilityZone: "us-east-1c"}, }, }, + { + name: "backup fails when takePVSnapshot fails", + 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: true, + groupResource: "persistentvolumes", + snapshottableVolumes: map[string]api.VolumeBackupInfo{ + "vol-abc123": {SnapshotID: "snapshot-1", AvailabilityZone: "us-east-1c"}, + }, + snapshotError: fmt.Errorf("failure"), + }, } for _, test := range tests { @@ -320,6 +360,7 @@ func TestBackupItemNoSkips(t *testing.T) { snapshotService = &arktest.FakeSnapshotService{ SnapshottableVolumes: test.snapshottableVolumes, VolumeID: "vol-abc123", + Error: test.snapshotError, } b.snapshotService = snapshotService } @@ -337,9 +378,15 @@ func TestBackupItemNoSkips(t *testing.T) { obj := &unstructured.Unstructured{Object: item} itemHookHandler.On("handleHooks", mock.Anything, groupResource, obj, resourceHooks, hookPhasePre).Return(nil) - itemHookHandler.On("handleHooks", mock.Anything, groupResource, obj, resourceHooks, hookPhasePost).Return(nil) + if test.snapshotError == nil && test.additionalItemError == nil { + // TODO: Remove if-clause when #511 is resolved. + itemHookHandler.On("handleHooks", mock.Anything, groupResource, obj, resourceHooks, hookPhasePost).Return(nil) + } for i, item := range test.customActionAdditionalItemIdentifiers { + if test.additionalItemError != nil && i > 0 { + break + } itemClient := &arktest.FakeDynamicClient{} defer itemClient.AssertExpectations(t) @@ -347,7 +394,7 @@ func TestBackupItemNoSkips(t *testing.T) { itemClient.On("Get", item.Name, metav1.GetOptions{}).Return(test.customActionAdditionalItems[i], nil) - additionalItemBackupper.On("backupItem", mock.AnythingOfType("*logrus.Entry"), test.customActionAdditionalItems[i], item.GroupResource).Return(nil) + additionalItemBackupper.On("backupItem", mock.AnythingOfType("*logrus.Entry"), test.customActionAdditionalItems[i], item.GroupResource).Return(test.additionalItemError) } err = b.backupItem(arktest.NewLogger(), obj, groupResource) diff --git a/pkg/util/test/fake_snapshot_service.go b/pkg/util/test/fake_snapshot_service.go index f5ffaaff75..ade7856681 100644 --- a/pkg/util/test/fake_snapshot_service.go +++ b/pkg/util/test/fake_snapshot_service.go @@ -37,9 +37,15 @@ type FakeSnapshotService struct { VolumeID string VolumeIDSet string + + Error error } func (s *FakeSnapshotService) CreateSnapshot(volumeID, volumeAZ string, tags map[string]string) (string, error) { + if s.Error != nil { + return "", s.Error + } + if _, exists := s.SnapshottableVolumes[volumeID]; !exists { return "", errors.New("snapshottable volume not found") } @@ -53,6 +59,10 @@ func (s *FakeSnapshotService) CreateSnapshot(volumeID, volumeAZ string, tags map } func (s *FakeSnapshotService) CreateVolumeFromSnapshot(snapshotID, volumeType, volumeAZ string, iops *int64) (string, error) { + if s.Error != nil { + return "", s.Error + } + key := api.VolumeBackupInfo{ SnapshotID: snapshotID, Type: volumeType, @@ -64,6 +74,10 @@ func (s *FakeSnapshotService) CreateVolumeFromSnapshot(snapshotID, volumeType, v } func (s *FakeSnapshotService) DeleteSnapshot(snapshotID string) error { + if s.Error != nil { + return s.Error + } + if !s.SnapshotsTaken.Has(snapshotID) { return errors.New("snapshot not found") } @@ -74,6 +88,10 @@ func (s *FakeSnapshotService) DeleteSnapshot(snapshotID string) error { } func (s *FakeSnapshotService) GetVolumeInfo(volumeID, volumeAZ string) (string, *int64, error) { + if s.Error != nil { + return "", nil, s.Error + } + if volumeInfo, exists := s.SnapshottableVolumes[volumeID]; !exists { return "", nil, errors.New("VolumeID not found") } else { @@ -82,10 +100,14 @@ func (s *FakeSnapshotService) GetVolumeInfo(volumeID, volumeAZ string) (string, } func (s *FakeSnapshotService) GetVolumeID(pv runtime.Unstructured) (string, error) { + if s.Error != nil { + return "", s.Error + } + return s.VolumeID, nil } func (s *FakeSnapshotService) SetVolumeID(pv runtime.Unstructured, volumeID string) (runtime.Unstructured, error) { s.VolumeIDSet = volumeID - return pv, nil + return pv, s.Error }