code review

Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
vmware-tanzu · Sep 11, 2017 · e8070a8 · e8070a8
1 parent d401930
commit e8070a8
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 15 deletions.
diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go
@@ -39,9 +39,9 @@ import (
 
 // Backupper performs backups.
 type Backupper interface {
-	// Backup takes a backup using the specification in the api.Backup and writes backup data to the
-	// given writers.
-	Backup(backup *api.Backup, data, log io.Writer) error
+	// Backup takes a backup using the specification in the api.Backup and writes backup and log data
+	// to the given writers.
+	Backup(backup *api.Backup, backupFile, logFile io.Writer) error
 }
 
 // kubernetesBackupper implements Backupper.
@@ -184,15 +184,15 @@ func (ctx *backupContext) log(msg string, args ...interface{}) {
 }
 
 // Backup backs up the items specified in the Backup, placing them in a gzip-compressed tar file
-// written to data. The finalized api.Backup is written to metadata.
-func (kb *kubernetesBackupper) Backup(backup *api.Backup, data, log io.Writer) error {
-	gzippedData := gzip.NewWriter(data)
+// written to backupFile. The finalized api.Backup is written to metadata.
+func (kb *kubernetesBackupper) Backup(backup *api.Backup, backupFile, logFile io.Writer) error {
+	gzippedData := gzip.NewWriter(backupFile)
 	defer gzippedData.Close()
 
 	tw := tar.NewWriter(gzippedData)
 	defer tw.Close()
 
-	gzippedLog := gzip.NewWriter(log)
+	gzippedLog := gzip.NewWriter(logFile)
 	defer gzippedLog.Close()
 
 	var errs []error

diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go
@@ -23,6 +23,7 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
+	"io/ioutil"
 	"reflect"
 	"sort"
 	"testing"
@@ -444,8 +445,7 @@ func TestBackupMethod(t *testing.T) {
 	require.NoError(t, err)
 
 	output := new(bytes.Buffer)
-	log := new(bytes.Buffer)
-	err = backupper.Backup(backup, output, log)
+	err = backupper.Backup(backup, output, ioutil.Discard)
 	require.NoError(t, err)
 
 	expectedFiles := sets.NewString(

diff --git a/pkg/cloudprovider/backup_service.go b/pkg/cloudprovider/backup_service.go
@@ -64,8 +64,8 @@ type BackupGetter interface {
 
 const (
 	metadataFileFormatString = "%s/ark-backup.json"
-	backupFileFormatString   = "%s/%s.tar.gz"
-	logFileFormatString      = "%s/%s.log.gz"
+	backupFileFormatString   = "%s/%s-data.tar.gz"
+	logFileFormatString      = "%s/%s-logs.gz"
 )
 
 func getMetadataKey(backup string) string {
@@ -186,7 +186,6 @@ func (br *backupService) DeleteBackupDir(bucket, backupName string) error {
 	var errs []error
 	for _, key := range objects {
 		glog.V(4).Infof("Trying to delete bucket=%s, key=%s", bucket, key)
-		fmt.Printf("Trying to delete bucket=%s, key=%s\n", bucket, key)
 		if err := br.objectStorage.DeleteObject(bucket, key); err != nil {
 			errs = append(errs, err)
 		}

diff --git a/pkg/cloudprovider/backup_service_test.go b/pkg/cloudprovider/backup_service_test.go
@@ -88,10 +88,10 @@ func TestUploadBackup(t *testing.T) {
 				objStore.On("PutObject", bucket, backupName+"/ark-backup.json", test.metadata).Return(test.metadataError)
 			}
 			if test.backup != nil {
-				objStore.On("PutObject", bucket, backupName+"/"+backupName+".tar.gz", test.backup).Return(test.backupError)
+				objStore.On("PutObject", bucket, backupName+"/"+backupName+"-data.tar.gz", test.backup).Return(test.backupError)
 			}
 			if test.log != nil {
-				objStore.On("PutObject", bucket, backupName+"/"+backupName+".log.gz", test.log).Return(test.logError)
+				objStore.On("PutObject", bucket, backupName+"/"+backupName+"-logs.gz", test.log).Return(test.logError)
 			}
 			if test.expectMetadataDelete {
 				objStore.On("DeleteObject", bucket, backupName+"/ark-backup.json").Return(nil)
@@ -116,7 +116,7 @@ func TestDownloadBackup(t *testing.T) {
 	o := &testutil.ObjectStorageAdapter{}
 	bucket := "b"
 	backup := "bak"
-	o.On("GetObject", bucket, backup+"/"+backup+".tar.gz").Return(ioutil.NopCloser(strings.NewReader("foo")), nil)
+	o.On("GetObject", bucket, backup+"/"+backup+"-data.tar.gz").Return(ioutil.NopCloser(strings.NewReader("foo")), nil)
 	s := NewBackupService(o)
 	rc, err := s.DownloadBackup(bucket, backup)
 	require.NoError(t, err)