diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 9f0788b09b..2e4d5010ef 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -204,10 +204,20 @@ func (ib *defaultItemBackupper) backupItem(logger logrus.FieldLogger, obj runtim } } - if err := ib.executeActions(log, obj, groupResource, name, namespace, metadata); err != nil { + updatedObj, err := ib.executeActions(log, obj, groupResource, name, namespace, metadata) + if err != nil { log.WithError(err).Error("Error executing item actions") backupErrs = append(backupErrs, err) + + // if there was an error running actions, execute post hooks and return + log.Debug("Executing post hooks") + if err := ib.itemHookHandler.handleHooks(log, groupResource, obj, ib.resourceHooks, hookPhasePost); err != nil { + backupErrs = append(backupErrs, err) + } + + return kubeerrs.NewAggregate(backupErrs) } + obj = updatedObj if groupResource == kuberesource.PersistentVolumes { if ib.snapshotService == nil { @@ -285,7 +295,13 @@ func (ib *defaultItemBackupper) backupPodVolumes(log logrus.FieldLogger, pod *co return ib.resticBackupper.BackupPodVolumes(ib.backup, pod, log) } -func (ib *defaultItemBackupper) executeActions(log logrus.FieldLogger, obj runtime.Unstructured, groupResource schema.GroupResource, name, namespace string, metadata metav1.Object) error { +func (ib *defaultItemBackupper) executeActions( + log logrus.FieldLogger, + obj runtime.Unstructured, + groupResource schema.GroupResource, + name, namespace string, + metadata metav1.Object, +) (runtime.Unstructured, error) { for _, action := range ib.actions { if !action.resourceIncludesExcludes.ShouldInclude(groupResource.String()) { log.Debug("Skipping action because it does not apply to this resource") @@ -308,40 +324,40 @@ func (ib *defaultItemBackupper) executeActions(log logrus.FieldLogger, obj runti logSetter.SetLog(log) } - if updatedItem, additionalItemIdentifiers, err := action.Execute(obj, ib.backup); err == nil { - obj = updatedItem + updatedItem, additionalItemIdentifiers, err := action.Execute(obj, ib.backup) + if err != nil { + // We want this to show up in the log file at the place where the error occurs. When we return + // the error, it get aggregated with all the other ones at the end of the backup, making it + // harder to tell when it happened. + log.WithError(err).Error("error executing custom action") - for _, additionalItem := range additionalItemIdentifiers { - gvr, resource, err := ib.discoveryHelper.ResourceFor(additionalItem.GroupResource.WithVersion("")) - if err != nil { - return err - } + return nil, errors.Wrapf(err, "error executing custom action (groupResource=%s, namespace=%s, name=%s)", groupResource.String(), namespace, name) + } + obj = updatedItem - client, err := ib.dynamicFactory.ClientForGroupVersionResource(gvr.GroupVersion(), resource, additionalItem.Namespace) - if err != nil { - return err - } + for _, additionalItem := range additionalItemIdentifiers { + gvr, resource, err := ib.discoveryHelper.ResourceFor(additionalItem.GroupResource.WithVersion("")) + if err != nil { + return nil, err + } - additionalItem, err := client.Get(additionalItem.Name, metav1.GetOptions{}) - if err != nil { - return err - } + client, err := ib.dynamicFactory.ClientForGroupVersionResource(gvr.GroupVersion(), resource, additionalItem.Namespace) + if err != nil { + return nil, err + } - if err = ib.additionalItemBackupper.backupItem(log, additionalItem, gvr.GroupResource()); err != nil { - return err - } + additionalItem, err := client.Get(additionalItem.Name, metav1.GetOptions{}) + if err != nil { + return nil, err } - } else { - // We want this to show up in the log file at the place where the error occurs. When we return - // the error, it get aggregated with all the other ones at the end of the backup, making it - // harder to tell when it happened. - log.WithError(err).Error("error executing custom action") - return errors.Wrapf(err, "error executing custom action (groupResource=%s, namespace=%s, name=%s)", groupResource.String(), namespace, name) + if err = ib.additionalItemBackupper.backupItem(log, additionalItem, gvr.GroupResource()); err != nil { + return nil, err + } } } - return nil + return obj, nil } // zoneLabel is the label that stores availability-zone info diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 2d17e5209b..9be8b7bf86 100644 --- a/pkg/backup/item_backupper_test.go +++ b/pkg/backup/item_backupper_test.go @@ -34,6 +34,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -539,6 +540,84 @@ func TestBackupItemNoSkips(t *testing.T) { } } +type addAnnotationAction struct{} + +func (a *addAnnotationAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []ResourceIdentifier, error) { + // since item actions run out-of-proc, do a deep-copy here to simulate passing data + // across a process boundary. + copy := item.(*unstructured.Unstructured).DeepCopy() + + metadata, err := meta.Accessor(copy) + if err != nil { + return copy, nil, nil + } + + annotations := metadata.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + annotations["foo"] = "bar" + metadata.SetAnnotations(annotations) + + return copy, nil, nil +} + +func (a *addAnnotationAction) AppliesTo() (ResourceSelector, error) { + panic("not implemented") +} + +func TestItemActionModificationsToItemPersist(t *testing.T) { + var ( + w = &fakeTarWriter{} + obj = &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "namespace": "myns", + "name": "bar", + }, + }, + } + actions = []resolvedAction{ + { + ItemAction: &addAnnotationAction{}, + namespaceIncludesExcludes: collections.NewIncludesExcludes(), + resourceIncludesExcludes: collections.NewIncludesExcludes(), + selector: labels.Everything(), + }, + } + b = (&defaultItemBackupperFactory{}).newItemBackupper( + &v1.Backup{}, + collections.NewIncludesExcludes(), + collections.NewIncludesExcludes(), + make(map[itemKey]struct{}), + actions, + nil, + w, + nil, + &arktest.FakeDynamicFactory{}, + arktest.NewFakeDiscoveryHelper(true, nil), + nil, + nil, + newPVCSnapshotTracker(), + ).(*defaultItemBackupper) + ) + + // our expected backed-up object is the passed-in object plus the annotation + // that the backup item action adds. + expected := obj.DeepCopy() + expected.SetAnnotations(map[string]string{"foo": "bar"}) + + // method under test + require.NoError(t, b.backupItem(arktest.NewLogger(), obj, schema.ParseGroupResource("resource.group"))) + + // get the actual backed-up item + require.Len(t, w.data, 1) + actual, err := arktest.GetAsMap(string(w.data[0])) + require.NoError(t, err) + + assert.EqualValues(t, expected.Object, actual) +} + func TestTakePVSnapshot(t *testing.T) { iops := int64(1000)