Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch per-backup/restore logs to use logrus #123

Merged
merged 1 commit into from
Oct 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 23 additions & 30 deletions pkg/backup/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import (
"archive/tar"
"compress/gzip"
"encoding/json"
"fmt"
"io"
"strings"
"time"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -58,11 +58,11 @@ var _ Backupper = &kubernetesBackupper{}

// ActionContext contains contextual information for actions.
type ActionContext struct {
logger *logger
logger *logrus.Logger
}

func (ac ActionContext) log(msg string, args ...interface{}) {
ac.logger.log(msg, args...)
func (ac ActionContext) infof(msg string, args ...interface{}) {
ac.logger.Infof(msg, args...)
}

// Action is an actor that performs an operation on an individual item being backed up.
Expand Down Expand Up @@ -117,7 +117,7 @@ func (ctx *backupContext) getResourceIncludesExcludes(helper discovery.Helper, i
func(item string) string {
gr, err := helper.ResolveGroupResource(item)
if err != nil {
ctx.log("Unable to resolve resource %q: %v", item, err)
ctx.infof("Unable to resolve resource %q: %v", item, err)
return ""
}

Expand All @@ -132,20 +132,10 @@ 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 *logger
logger *logrus.Logger
namespaceIncludesExcludes *collections.IncludesExcludes
resourceIncludesExcludes *collections.IncludesExcludes
// deploymentsBackedUp marks whether we've seen and are backing up the deployments resource, from
Expand All @@ -158,8 +148,8 @@ type backupContext struct {
networkPoliciesBackedUp bool
}

func (ctx *backupContext) log(msg string, args ...interface{}) {
ctx.logger.log(msg, args...)
func (ctx *backupContext) infof(msg string, args ...interface{}) {
ctx.logger.Infof(msg, args...)
}

// Backup backs up the items specified in the Backup, placing them in a gzip-compressed tar file
Expand All @@ -176,29 +166,32 @@ func (kb *kubernetesBackupper) Backup(backup *api.Backup, backupFile, logFile io

var errs []error

log := logrus.New()
log.Out = gzippedLog

ctx := &backupContext{
backup: backup,
w: tw,
logger: &logger{w: gzippedLog},
logger: log,
namespaceIncludesExcludes: getNamespaceIncludesExcludes(backup),
}

ctx.log("Starting backup")
ctx.infof("Starting backup")

ctx.resourceIncludesExcludes = ctx.getResourceIncludesExcludes(kb.discoveryHelper, backup.Spec.IncludedResources, backup.Spec.ExcludedResources)

for _, group := range kb.discoveryHelper.Resources() {
ctx.log("Processing group %s", group.GroupVersion)
ctx.infof("Processing group %s", group.GroupVersion)
if err := kb.backupGroup(ctx, group); err != nil {
errs = append(errs, err)
}
}

err := kuberrs.NewAggregate(errs)
if err == nil {
ctx.log("Backup completed successfully")
ctx.infof("Backup completed successfully")
} else {
ctx.log("Backup completed with errors: %v", err)
ctx.infof("Backup completed with errors: %v", err)
}

return err
Expand All @@ -215,7 +208,7 @@ func (kb *kubernetesBackupper) backupGroup(ctx *backupContext, group *metav1.API
var errs []error

for _, resource := range group.APIResources {
ctx.log("Processing resource %s/%s", group.GroupVersion, resource.Name)
ctx.infof("Processing resource %s/%s", group.GroupVersion, resource.Name)
if err := kb.backupResource(ctx, group, resource); err != nil {
errs = append(errs, err)
}
Expand Down Expand Up @@ -248,7 +241,7 @@ func (kb *kubernetesBackupper) backupResource(
grString := gr.String()

if !ctx.resourceIncludesExcludes.ShouldInclude(grString) {
ctx.log("Resource %s is excluded", grString)
ctx.infof("Resource %s is excluded", grString)
return nil
}

Expand All @@ -260,7 +253,7 @@ func (kb *kubernetesBackupper) backupResource(
} else {
other = appsDeploymentsResource
}
ctx.log("Skipping resource %q because it's a duplicate of %q", grString, other)
ctx.infof("Skipping resource %q because it's a duplicate of %q", grString, other)
return nil
}

Expand All @@ -275,7 +268,7 @@ func (kb *kubernetesBackupper) backupResource(
} else {
other = networkingNetworkPoliciesResource
}
ctx.log("Skipping resource %q because it's a duplicate of %q", grString, other)
ctx.infof("Skipping resource %q because it's a duplicate of %q", grString, other)
return nil
}

Expand Down Expand Up @@ -377,21 +370,21 @@ func (*realItemBackupper) backupItem(ctx *backupContext, item map[string]interfa
namespace, err := collections.GetString(metadata, "namespace")
if err == nil {
if !ctx.namespaceIncludesExcludes.ShouldInclude(namespace) {
ctx.log("Excluding item %s because namespace %s is excluded", name, namespace)
ctx.infof("Excluding item %s because namespace %s is excluded", name, namespace)
return nil
}
}

if action != nil {
ctx.log("Executing action on %s, ns=%s, name=%s", groupResource, namespace, name)
ctx.infof("Executing action on %s, ns=%s, name=%s", groupResource, namespace, name)

actionCtx := ActionContext{logger: ctx.logger}
if err := action.Execute(actionCtx, item, ctx.backup); err != nil {
return err
}
}

ctx.log("Backing up resource=%s, ns=%s, name=%s", groupResource, namespace, name)
ctx.infof("Backing up resource=%s, ns=%s, name=%s", groupResource, namespace, name)

var filePath string
if namespace != "" {
Expand Down
23 changes: 16 additions & 7 deletions pkg/backup/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import (
"testing"
"time"

testlogger "github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -39,9 +44,6 @@ import (
"github.com/heptio/ark/pkg/apis/ark/v1"
"github.com/heptio/ark/pkg/util/collections"
. "github.com/heptio/ark/pkg/util/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

type fakeAction struct {
Expand Down Expand Up @@ -189,9 +191,10 @@ func TestGetResourceIncludesExcludes(t *testing.T) {
},
}

b := new(bytes.Buffer)
log, _ := testlogger.NewNullLogger()

ctx := &backupContext{
logger: &logger{w: b},
logger: log,
}

actual := ctx.getResourceIncludesExcludes(dh, test.includes, test.excludes)
Expand Down Expand Up @@ -758,6 +761,9 @@ func TestBackupResource(t *testing.T) {
require.NoError(t, err)
labelSelector = s
}

log, _ := testlogger.NewNullLogger()

ctx := &backupContext{
backup: &v1.Backup{
Spec: v1.BackupSpec{
Expand All @@ -768,7 +774,7 @@ func TestBackupResource(t *testing.T) {
namespaceIncludesExcludes: test.namespaceIncludesExcludes,
deploymentsBackedUp: test.deploymentsBackedUp,
networkPoliciesBackedUp: test.networkPoliciesBackedUp,
logger: &logger{w: new(bytes.Buffer)},
logger: log,
}

group := &metav1.APIResourceList{
Expand Down Expand Up @@ -992,11 +998,14 @@ func TestBackupItem(t *testing.T) {
actionParam = action
backup = &v1.Backup{}
}

log, _ := testlogger.NewNullLogger()

ctx := &backupContext{
backup: backup,
namespaceIncludesExcludes: namespaces,
w: w,
logger: &logger{w: new(bytes.Buffer)},
logger: log,
}
b := &realItemBackupper{}
err = b.backupItem(ctx, item, "resource.group", actionParam)
Expand Down
14 changes: 7 additions & 7 deletions pkg/backup/volume_snapshot_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func NewVolumeSnapshotAction(snapshotService cloudprovider.SnapshotService) (Act
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 {
ctx.log("Backup %q has volume snapshots disabled; skipping volume snapshot action.", backupName)
ctx.infof("Backup %q has volume snapshots disabled; skipping volume snapshot action.", backupName)
return nil
}

Expand All @@ -68,12 +68,12 @@ func (a *volumeSnapshotAction) Execute(ctx ActionContext, volume map[string]inte
var pvFailureDomainZone string

if labelsMap, err := collections.GetMap(metadata, "labels"); err != nil {
ctx.log("error getting labels on PersistentVolume %q for backup %q: %v", name, backupName, err)
ctx.infof("error getting labels on PersistentVolume %q for backup %q: %v", name, backupName, err)
} else {
if labelsMap[zoneLabel] != nil {
pvFailureDomainZone = labelsMap[zoneLabel].(string)
} else {
ctx.log("label %q is not present on PersistentVolume %q for backup %q.", zoneLabel, name, backupName)
ctx.infof("label %q is not present on PersistentVolume %q for backup %q.", zoneLabel, name, backupName)
}
}

Expand All @@ -84,23 +84,23 @@ func (a *volumeSnapshotAction) Execute(ctx ActionContext, volume map[string]inte
}
// no volumeID / nil error means unsupported PV source
if volumeID == "" {
ctx.log("Backup %q: PersistentVolume %q is not a supported volume type for snapshots, skipping.", backupName, name)
ctx.infof("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)

ctx.log("Backup %q: snapshotting PersistentVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration)
ctx.infof("Backup %q: snapshotting PersistentVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration)

snapshotID, err := a.snapshotService.CreateSnapshot(volumeID, pvFailureDomainZone)
if err != nil {
ctx.log("error creating snapshot for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err)
ctx.infof("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, pvFailureDomainZone)
if err != nil {
ctx.log("error getting volume info for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err)
ctx.infof("error getting volume info for backup %q, volume %q, volume-id %q: %v", backupName, name, volumeID, err)
return err
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/backup/volume_snapshot_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ limitations under the License.
package backup

import (
"bytes"
"reflect"
"testing"
"time"

testlogger "github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/require"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -198,7 +198,9 @@ func TestVolumeSnapshotAction(t *testing.T) {
t.Fatal(err)
}

actionCtx := ActionContext{logger: &logger{w: new(bytes.Buffer)}}
log, _ := testlogger.NewNullLogger()

actionCtx := ActionContext{logger: log}
err = action.Execute(actionCtx, pv, backup)
gotErr := err != nil

Expand Down
Loading