client: hclog-ify most of the client

Leaving fingerprinters in case that interface changes with plugins.
hashicorp · Oct 16, 2018 · 9da25ad · 9da25ad
1 parent c95155d
commit 9da25ad
Show file tree

Hide file tree

Showing 17 changed files with 260 additions and 215 deletions.
diff --git a/client/acl.go b/client/acl.go
@@ -134,7 +134,7 @@ func (c *Client) resolveTokenValue(secretID string) (*structs.ACLToken, error) {
 	if err := c.RPC("ACL.ResolveToken", &req, &resp); err != nil {
 		// If we encounter an error but have a cached value, mask the error and extend the cache
 		if ok {
-			c.logger.Printf("[WARN] client: failed to resolve token, using expired cached value: %v", err)
+			c.logger.Warn("failed to resolve token, using expired cached value", "error", err)
 			cached := raw.(*cachedACLValue)
 			return cached.Token, nil
 		}
@@ -198,7 +198,7 @@ func (c *Client) resolvePolicies(secretID string, policies []string) ([]*structs
 	if err := c.RPC("ACL.GetPolicies", &req, &resp); err != nil {
 		// If we encounter an error but have cached policies, mask the error and extend the cache
 		if len(missing) == 0 {
-			c.logger.Printf("[WARN] client: failed to resolve policies, using expired cached value: %v", err)
+			c.logger.Warn("failed to resolve policies, using expired cached value", "error", err)
 			out = append(out, expired...)
 			return out, nil
 		}

diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go
@@ -6,12 +6,12 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"log"
 	"os"
 	"path/filepath"
 	"sync"
 	"time"
 
+	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-multierror"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -78,7 +78,7 @@ type AllocDir struct {
 
 	mu sync.RWMutex
 
-	logger *log.Logger
+	logger hclog.Logger
 }
 
 // AllocDirFS exposes file operations on the alloc dir
@@ -93,7 +93,8 @@ type AllocDirFS interface {
 
 // NewAllocDir initializes the AllocDir struct with allocDir as base path for
 // the allocation directory.
-func NewAllocDir(logger *log.Logger, allocDir string) *AllocDir {
+func NewAllocDir(logger hclog.Logger, allocDir string) *AllocDir {
+	logger = logger.Named("alloc_dir")
 	return &AllocDir{
 		AllocDir:  allocDir,
 		SharedDir: filepath.Join(allocDir, SharedAllocName),
@@ -209,7 +210,7 @@ func (d *AllocDir) Snapshot(w io.Writer) error {
 				// the snapshotting side closed the connect
 				// prematurely and won't try to use the tar
 				// anyway.
-				d.logger.Printf("[WARN] client: snapshotting failed and unable to write error marker: %v", writeErr)
+				d.logger.Warn("snapshotting failed and unable to write error marker", "error", writeErr)
 			}
 			return fmt.Errorf("failed to snapshot %s: %v", path, err)
 		}

diff --git a/client/allocdir/alloc_dir_test.go b/client/allocdir/alloc_dir_test.go
@@ -54,7 +54,7 @@ func TestAllocDir_BuildAlloc(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	d.NewTaskDir(t1.Name)
 	d.NewTaskDir(t2.Name)
@@ -91,7 +91,7 @@ func TestAllocDir_MountSharedAlloc(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	if err := d.Build(); err != nil {
 		t.Fatalf("Build() failed: %v", err)
@@ -136,7 +136,7 @@ func TestAllocDir_Snapshot(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	if err := d.Build(); err != nil {
 		t.Fatalf("Build() failed: %v", err)
@@ -223,13 +223,13 @@ func TestAllocDir_Move(t *testing.T) {
 	defer os.RemoveAll(tmp2)
 
 	// Create two alloc dirs
-	d1 := NewAllocDir(testlog.Logger(t), tmp1)
+	d1 := NewAllocDir(testlog.HCLogger(t), tmp1)
 	if err := d1.Build(); err != nil {
 		t.Fatalf("Build() failed: %v", err)
 	}
 	defer d1.Destroy()
 
-	d2 := NewAllocDir(testlog.Logger(t), tmp2)
+	d2 := NewAllocDir(testlog.HCLogger(t), tmp2)
 	if err := d2.Build(); err != nil {
 		t.Fatalf("Build() failed: %v", err)
 	}
@@ -284,7 +284,7 @@ func TestAllocDir_EscapeChecking(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	if err := d.Build(); err != nil {
 		t.Fatalf("Build() failed: %v", err)
 	}
@@ -325,7 +325,7 @@ func TestAllocDir_ReadAt_SecretDir(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	if err := d.Build(); err != nil {
 		t.Fatalf("Build() failed: %v", err)
 	}
@@ -410,7 +410,7 @@ func TestAllocDir_CreateDir(t *testing.T) {
 // TestAllocDir_Copy asserts that AllocDir.Copy does a deep copy of itself and
 // all TaskDirs.
 func TestAllocDir_Copy(t *testing.T) {
-	a := NewAllocDir(testlog.Logger(t), "foo")
+	a := NewAllocDir(testlog.HCLogger(t), "foo")
 	a.NewTaskDir("bar")
 	a.NewTaskDir("baz")
 

diff --git a/client/allocdir/task_dir.go b/client/allocdir/task_dir.go
@@ -3,10 +3,10 @@ package allocdir
 import (
 	"fmt"
 	"io/ioutil"
-	"log"
 	"os"
 	"path/filepath"
 
+	hclog "github.com/hashicorp/go-hclog"
 	cstructs "github.com/hashicorp/nomad/client/structs"
 )
 
@@ -37,15 +37,18 @@ type TaskDir struct {
 	// <task_dir>/secrets/
 	SecretsDir string
 
-	logger *log.Logger
+	logger hclog.Logger
 }
 
 // newTaskDir creates a TaskDir struct with paths set. Call Build() to
 // create paths on disk.
 //
 // Call AllocDir.NewTaskDir to create new TaskDirs
-func newTaskDir(logger *log.Logger, allocDir, taskName string) *TaskDir {
+func newTaskDir(logger hclog.Logger, allocDir, taskName string) *TaskDir {
 	taskDir := filepath.Join(allocDir, taskName)
+
+	logger = logger.Named("task_dir").With("task_name", taskName)
+
 	return &TaskDir{
 		Dir:            taskDir,
 		SharedAllocDir: filepath.Join(allocDir, SharedAllocName),

diff --git a/client/allocdir/task_dir_linux_test.go b/client/allocdir/task_dir_linux_test.go
@@ -22,7 +22,7 @@ func TestLinuxSpecialDirs(t *testing.T) {
 	}
 	defer os.RemoveAll(allocDir)
 
-	td := newTaskDir(testlog.Logger(t), allocDir, "test")
+	td := newTaskDir(testlog.HCLogger(t), allocDir, "test")
 
 	// Despite the task dir not existing, unmountSpecialDirs should *not*
 	// return an error

diff --git a/client/allocdir/task_dir_test.go b/client/allocdir/task_dir_test.go
@@ -18,7 +18,7 @@ func TestTaskDir_EmbedNonexistent(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	td := d.NewTaskDir(t1.Name)
 	if err := d.Build(); err != nil {
@@ -40,7 +40,7 @@ func TestTaskDir_EmbedDirs(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	td := d.NewTaskDir(t1.Name)
 	if err := d.Build(); err != nil {
@@ -97,7 +97,7 @@ func TestTaskDir_NonRoot_Image(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	td := d.NewTaskDir(t1.Name)
 	if err := d.Build(); err != nil {
@@ -120,7 +120,7 @@ func TestTaskDir_NonRoot(t *testing.T) {
 	}
 	defer os.RemoveAll(tmp)
 
-	d := NewAllocDir(testlog.Logger(t), tmp)
+	d := NewAllocDir(testlog.HCLogger(t), tmp)
 	defer d.Destroy()
 	td := d.NewTaskDir(t1.Name)
 	if err := d.Build(); err != nil {

diff --git a/client/allocrunner/alloc_runner.go b/client/allocrunner/alloc_runner.go
@@ -144,9 +144,9 @@ func NewAllocRunner(logger *log.Logger, config *config.Config, stateDB *bolt.DB,
 		alloc:   alloc,
 		allocID: alloc.ID,
 		//allocBroadcast: cstructs.NewAllocBroadcaster(8),
-		prevAlloc:    prevAlloc,
-		dirtyCh:      make(chan struct{}, 1),
-		allocDir:     allocdir.NewAllocDir(logger, filepath.Join(config.AllocDir, alloc.ID)),
+		prevAlloc: prevAlloc,
+		dirtyCh:   make(chan struct{}, 1),
+		//allocDir:     allocdir.NewAllocDir(logger, filepath.Join(config.AllocDir, alloc.ID)),
 		tasks:        make(map[string]*taskrunner.TaskRunner),
 		taskStates:   copyTaskStates(alloc.TaskStates),
 		restored:     make(map[string]struct{}),

diff --git a/client/allocrunner/testing.go b/client/allocrunner/testing.go
@@ -52,7 +52,7 @@ func TestAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation, restarts
 		alloc.Job.Type = structs.JobTypeBatch
 	}
 	vclient := vaultclient.NewMockVaultClient()
-	ar := NewAllocRunner(testlog.Logger(t), conf, db, upd.Update, alloc, vclient, consulApi.NewMockConsulServiceClient(t), allocwatcher.NoopPrevAlloc{})
+	ar := NewAllocRunner(testlog.Logger(t), conf, db, upd.Update, alloc, vclient, consulApi.NewMockConsulServiceClient(t, testlog.HCLogger(t)), allocwatcher.NoopPrevAlloc{})
 	return upd, ar
 }
 

diff --git a/client/allocrunnerv2/alloc_runner.go b/client/allocrunnerv2/alloc_runner.go
@@ -119,13 +119,12 @@ func NewAllocRunner(config *Config) (*allocRunner, error) {
 		prevAllocWatcher: config.PrevAllocWatcher,
 	}
 
-	// Create alloc dir
-	//XXX update AllocDir to hc log
-	ar.allocDir = allocdir.NewAllocDir(nil, filepath.Join(config.ClientConfig.AllocDir, alloc.ID))
-
 	// Create the logger based on the allocation ID
 	ar.logger = config.Logger.With("alloc_id", alloc.ID)
 
+	// Create alloc dir
+	ar.allocDir = allocdir.NewAllocDir(ar.logger, filepath.Join(config.ClientConfig.AllocDir, alloc.ID))
+
 	// Initialize the runners hooks.
 	ar.initRunnerHooks()