Add ActionContext so backup actions can log

Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>

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
 		}
 	}

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)

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
 	}
 

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 {