diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index 3f41ae139..63c46d5bf 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -54,11 +54,20 @@ type kubernetesBackupper struct { var _ Backupper = &kubernetesBackupper{} +// ActionContext contains contextual information for actions. +type ActionContext struct { + logger *logger +} + +func (ac ActionContext) log(msg string, args ...interface{}) { + ac.logger.log(msg, args...) +} + // Action is an actor that performs an operation on an individual item being backed up. type Action interface { // Execute is invoked on an item being backed up. If an error is returned, the Backup is marked as // failed. - Execute(item map[string]interface{}, backup *api.Backup) error + Execute(ctx ActionContext, item map[string]interface{}, backup *api.Backup) error } // NewKubernetesBackupper creates a new kubernetesBackupper. @@ -144,10 +153,20 @@ func getNamespaceIncludesExcludes(backup *api.Backup) *collections.IncludesExclu return collections.NewIncludesExcludes().Includes(backup.Spec.IncludedNamespaces...).Excludes(backup.Spec.ExcludedNamespaces...) } +type logger struct { + w io.Writer +} + +func (l *logger) log(msg string, args ...interface{}) { + // TODO use a real logger that supports writing to files + now := time.Now().Format(time.RFC3339) + fmt.Fprintf(l.w, now+" "+msg+"\n", args...) +} + type backupContext struct { backup *api.Backup w tarWriter - logger io.Writer + logger *logger namespaceIncludesExcludes *collections.IncludesExcludes resourceIncludesExcludes *collections.IncludesExcludes // deploymentsBackedUp marks whether we've seen and are backing up the deployments resource, from @@ -161,9 +180,7 @@ type backupContext struct { } func (ctx *backupContext) log(msg string, args ...interface{}) { - // TODO use a real logger that supports writing to files - now := time.Now().Format(time.RFC3339) - fmt.Fprintf(ctx.logger, now+" "+msg+"\n", args...) + ctx.logger.log(msg, args...) } // Backup backs up the items specified in the Backup, placing them in a gzip-compressed tar file @@ -183,7 +200,7 @@ func (kb *kubernetesBackupper) Backup(backup *api.Backup, data, log io.Writer) e ctx := &backupContext{ backup: backup, w: tw, - logger: gzippedLog, + logger: &logger{w: gzippedLog}, namespaceIncludesExcludes: getNamespaceIncludesExcludes(backup), } @@ -389,7 +406,8 @@ func (*realItemBackupper) backupItem(ctx *backupContext, item map[string]interfa if action != nil { ctx.log("Executing action on %s, ns=%s, name=%s", groupResource, namespace, name) - if err := action.Execute(item, ctx.backup); err != nil { + actionCtx := ActionContext{logger: ctx.logger} + if err := action.Execute(actionCtx, item, ctx.backup); err != nil { return err } } diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 5c2ee25a5..8f06b4901 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -50,7 +50,7 @@ type fakeAction struct { var _ Action = &fakeAction{} -func (a *fakeAction) Execute(item map[string]interface{}, backup *v1.Backup) error { +func (a *fakeAction) Execute(ctx ActionContext, item map[string]interface{}, backup *v1.Backup) error { metadata, err := collections.GetMap(item, "metadata") if err != nil { return err @@ -186,7 +186,7 @@ func TestGetResourceIncludesExcludes(t *testing.T) { b := new(bytes.Buffer) ctx := &backupContext{ - logger: b, + logger: &logger{w: b}, } actual := ctx.getResourceIncludesExcludes(mapper, test.includes, test.excludes) @@ -781,7 +781,7 @@ func TestBackupResource(t *testing.T) { namespaceIncludesExcludes: test.namespaceIncludesExcludes, deploymentsBackedUp: test.deploymentsBackedUp, networkPoliciesBackedUp: test.networkPoliciesBackedUp, - logger: new(bytes.Buffer), + logger: &logger{w: new(bytes.Buffer)}, } group := &metav1.APIResourceList{ @@ -1009,7 +1009,7 @@ func TestBackupItem(t *testing.T) { backup: backup, namespaceIncludesExcludes: namespaces, w: w, - logger: new(bytes.Buffer), + logger: &logger{w: new(bytes.Buffer)}, } b := &realItemBackupper{} err = b.backupItem(ctx, item, "resource.group", actionParam) diff --git a/pkg/backup/volume_snapshot_action.go b/pkg/backup/volume_snapshot_action.go index d42cd32cc..2aebd4bf2 100644 --- a/pkg/backup/volume_snapshot_action.go +++ b/pkg/backup/volume_snapshot_action.go @@ -20,8 +20,6 @@ import ( "errors" "fmt" - "github.com/golang/glog" - "k8s.io/apimachinery/pkg/util/clock" api "github.com/heptio/ark/pkg/apis/ark/v1" @@ -52,10 +50,10 @@ func NewVolumeSnapshotAction(snapshotService cloudprovider.SnapshotService) (Act // Execute triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided // backup has volume snapshots enabled and the PV is of a compatible type. Also records cloud // disk type and IOPS (if applicable) to be able to restore to current state later. -func (a *volumeSnapshotAction) Execute(volume map[string]interface{}, backup *api.Backup) error { +func (a *volumeSnapshotAction) Execute(ctx ActionContext, volume map[string]interface{}, backup *api.Backup) error { backupName := fmt.Sprintf("%s/%s", backup.Namespace, backup.Name) if backup.Spec.SnapshotVolumes != nil && !*backup.Spec.SnapshotVolumes { - glog.V(2).Infof("Backup %q has volume snapshots disabled; skipping volume snapshot action.", backupName) + ctx.log("Backup %q has volume snapshots disabled; skipping volume snapshot action.", backupName) return nil } @@ -69,23 +67,23 @@ func (a *volumeSnapshotAction) Execute(volume map[string]interface{}, backup *ap } // no volumeID / nil error means unsupported PV source if volumeID == "" { - glog.V(2).Infof("Backup %q: PersistentVolume %q is not a supported volume type for snapshots, skipping.", backupName, name) + ctx.log("Backup %q: PersistentVolume %q is not a supported volume type for snapshots, skipping.", backupName, name) return nil } expiration := a.clock.Now().Add(backup.Spec.TTL.Duration) - glog.Infof("Backup %q: snapshotting PersistentVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration) + ctx.log("Backup %q: snapshotting PersistentVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration) snapshotID, err := a.snapshotService.CreateSnapshot(volumeID) if err != nil { - glog.V(4).Infof("error creating snapshot for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err) + ctx.log("error creating snapshot for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err) return err } volumeType, iops, err := a.snapshotService.GetVolumeInfo(volumeID) if err != nil { - glog.V(4).Infof("error getting volume info for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err) + ctx.log("error getting volume info for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err) return err } diff --git a/pkg/backup/volume_snapshot_action_test.go b/pkg/backup/volume_snapshot_action_test.go index 3c1c4f05f..e524a5955 100644 --- a/pkg/backup/volume_snapshot_action_test.go +++ b/pkg/backup/volume_snapshot_action_test.go @@ -17,6 +17,7 @@ limitations under the License. package backup import ( + "bytes" "reflect" "testing" "time" @@ -185,7 +186,8 @@ func TestVolumeSnapshotAction(t *testing.T) { t.Fatal(err) } - err = action.Execute(pv, backup) + actionCtx := ActionContext{logger: &logger{w: new(bytes.Buffer)}} + err = action.Execute(actionCtx, pv, backup) gotErr := err != nil if e, a := test.expectError, gotErr; e != a {