From 80c00fc26886bdf32595b07037578baea5b485dc Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 14 Jan 2019 15:33:42 -0600 Subject: [PATCH] Refactor logging in drivers to use a tri-state boolean Changes logging warnings/errors only if the state changes from healthy to unhealthy --- drivers/docker/driver.go | 7 ++++--- drivers/docker/fingerprint.go | 29 ++++++++++++++-------------- drivers/exec/driver.go | 30 ++++++++++++++++++++++++++++- drivers/rkt/driver.go | 36 ++++++++++++++++++----------------- 4 files changed, 67 insertions(+), 35 deletions(-) diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index 068032481..0de93aba7 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -89,9 +89,10 @@ type Driver struct { // gpuRuntime indicates nvidia-docker runtime availability gpuRuntime bool - // hasFingerprinted is used to store whether we have fingerprinted before - hasFingerprinted bool - fingerprintLock sync.Mutex + // A tri-state boolean to know if the fingerprinting has happened and + // whether it has been successful + fingerprintSuccess *bool + fingerprintLock sync.Mutex } // NewDockerDriver returns a docker implementation of a driver plugin diff --git a/drivers/docker/fingerprint.go b/drivers/docker/fingerprint.go index 6bb14c518..50d64374c 100644 --- a/drivers/docker/fingerprint.go +++ b/drivers/docker/fingerprint.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/plugins/drivers" pstructs "github.com/hashicorp/nomad/plugins/shared/structs" ) @@ -16,17 +17,17 @@ func (d *Driver) Fingerprint(ctx context.Context) (<-chan *drivers.Fingerprint, return ch, nil } -// fingerprinted returns whether the driver has fingerprinted before -func (d *Driver) fingerprinted() bool { +// setFingerprintSuccess marks the driver as having fingerprinted successfully +func (d *Driver) setFingerprintSuccess() { d.fingerprintLock.Lock() - defer d.fingerprintLock.Unlock() - return d.hasFingerprinted + d.fingerprintSuccess = helper.BoolToPtr(true) + d.fingerprintLock.Unlock() } -// setFingerprinted marks the driver as having fingerprinted once before -func (d *Driver) setFingerprinted() { +// setFingerprintFailure marks the driver as having failed fingerprinting +func (d *Driver) setFingerprintFailure() { d.fingerprintLock.Lock() - d.hasFingerprinted = true + d.fingerprintSuccess = helper.BoolToPtr(false) d.fingerprintLock.Unlock() } @@ -47,10 +48,6 @@ func (d *Driver) handleFingerprint(ctx context.Context, ch chan *drivers.Fingerp } func (d *Driver) buildFingerprint() *drivers.Fingerprint { - defer func() { - d.setFingerprinted() - }() - fp := &drivers.Fingerprint{ Attributes: map[string]*pstructs.Attribute{}, Health: drivers.HealthStateHealthy, @@ -58,9 +55,10 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { } client, _, err := d.dockerClients() if err != nil { - if !d.fingerprinted() { + if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Info("failed to initialize client", "error", err) } + d.setFingerprintFailure() return &drivers.Fingerprint{ Health: drivers.HealthStateUndetected, HealthDescription: "Failed to initialize docker client", @@ -69,9 +67,10 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { env, err := client.Version() if err != nil { - if !d.fingerprinted() { + if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Debug("could not connect to docker daemon", "endpoint", client.Endpoint(), "error", err) } + d.setFingerprintFailure() return &drivers.Fingerprint{ Health: drivers.HealthStateUnhealthy, HealthDescription: "Failed to connect to docker daemon", @@ -106,7 +105,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { } else { // Docker 17.09.0-ce dropped the Gateway IP from the bridge network // See https://github.com/moby/moby/issues/32648 - if !d.fingerprinted() { + if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Debug("bridge_ip could not be discovered") } } @@ -132,5 +131,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { strings.Join(runtimeNames, ",")) } + d.setFingerprintSuccess() + return fp } diff --git a/drivers/exec/driver.go b/drivers/exec/driver.go index 3a085345b..8aab879ed 100644 --- a/drivers/exec/driver.go +++ b/drivers/exec/driver.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/drivers/shared/eventer" "github.com/hashicorp/nomad/drivers/shared/executor" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/plugins/base" "github.com/hashicorp/nomad/plugins/drivers" "github.com/hashicorp/nomad/plugins/drivers/utils" @@ -20,6 +21,7 @@ import ( "github.com/hashicorp/nomad/plugins/shared/hclspec" "github.com/hashicorp/nomad/plugins/shared/loader" pstructs "github.com/hashicorp/nomad/plugins/shared/structs" + "sync" ) const ( @@ -95,6 +97,11 @@ type Driver struct { // logger will log to the Nomad agent logger hclog.Logger + + // A tri-state boolean to know if the fingerprinting has happened and + // whether it has been successful + fingerprintSuccess *bool + fingerprintLock sync.Mutex } // TaskConfig is the driver configuration of a task within a job @@ -126,6 +133,20 @@ func NewExecDriver(logger hclog.Logger) drivers.DriverPlugin { } } +// setFingerprintSuccess marks the driver as having fingerprinted successfully +func (d *Driver) setFingerprintSuccess() { + d.fingerprintLock.Lock() + d.fingerprintSuccess = helper.BoolToPtr(true) + d.fingerprintLock.Unlock() +} + +// setFingerprintFailure marks the driver as having failed fingerprinting +func (d *Driver) setFingerprintFailure() { + d.fingerprintLock.Lock() + d.fingerprintSuccess = helper.BoolToPtr(false) + d.fingerprintLock.Unlock() +} + func (d *Driver) PluginInfo() (*base.PluginInfoResponse, error) { return pluginInfo, nil } @@ -177,6 +198,7 @@ func (d *Driver) handleFingerprint(ctx context.Context, ch chan<- *drivers.Finge func (d *Driver) buildFingerprint() *drivers.Fingerprint { if runtime.GOOS != "linux" { + d.setFingerprintFailure() return &drivers.Fingerprint{ Health: drivers.HealthStateUndetected, HealthDescription: "exec driver unsupported on client OS", @@ -192,6 +214,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { if !utils.IsUnixRoot() { fp.Health = drivers.HealthStateUndetected fp.HealthDescription = drivers.DriverRequiresRootMessage + d.setFingerprintFailure() return fp } @@ -199,17 +222,22 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { if err != nil { fp.Health = drivers.HealthStateUnhealthy fp.HealthDescription = drivers.NoCgroupMountMessage - d.logger.Warn(fp.HealthDescription, "error", err) + if d.fingerprintSuccess == nil || *d.fingerprintSuccess { + d.logger.Warn(fp.HealthDescription, "error", err) + } + d.setFingerprintFailure() return fp } if mount == "" { fp.Health = drivers.HealthStateUnhealthy fp.HealthDescription = drivers.CgroupMountEmpty + d.setFingerprintFailure() return fp } fp.Attributes["driver.exec"] = pstructs.NewBoolAttribute(true) + d.setFingerprintSuccess() return fp } diff --git a/drivers/rkt/driver.go b/drivers/rkt/driver.go index 067e78ef6..2035327d0 100644 --- a/drivers/rkt/driver.go +++ b/drivers/rkt/driver.go @@ -28,6 +28,7 @@ import ( "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/drivers/shared/eventer" "github.com/hashicorp/nomad/drivers/shared/executor" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/plugins/base" "github.com/hashicorp/nomad/plugins/drivers" "github.com/hashicorp/nomad/plugins/shared" @@ -193,9 +194,10 @@ type Driver struct { // logger will log to the Nomad agent logger hclog.Logger - // hasFingerprinted is used to store whether we have fingerprinted before - hasFingerprinted bool - fingerprintLock sync.Mutex + // A tri-state boolean to know if the fingerprinting has happened and + // whether it has been successful + fingerprintSuccess *bool + fingerprintLock sync.Mutex } func NewRktDriver(logger hclog.Logger) drivers.DriverPlugin { @@ -264,25 +266,21 @@ func (d *Driver) handleFingerprint(ctx context.Context, ch chan *drivers.Fingerp } } -// setFingerprinted marks the driver as having fingerprinted once before -func (d *Driver) setFingerprinted() { +// setFingerprintSuccess marks the driver as having fingerprinted successfully +func (d *Driver) setFingerprintSuccess() { d.fingerprintLock.Lock() - d.hasFingerprinted = true + d.fingerprintSuccess = helper.BoolToPtr(true) d.fingerprintLock.Unlock() } -// fingerprinted returns whether the driver has fingerprinted before -func (d *Driver) fingerprinted() bool { +// setFingerprintFailure marks the driver as having failed fingerprinting +func (d *Driver) setFingerprintFailure() { d.fingerprintLock.Lock() - defer d.fingerprintLock.Unlock() - return d.hasFingerprinted + d.fingerprintSuccess = helper.BoolToPtr(false) + d.fingerprintLock.Unlock() } func (d *Driver) buildFingerprint() *drivers.Fingerprint { - defer func() { - d.setFingerprinted() - }() - fingerprint := &drivers.Fingerprint{ Attributes: map[string]*pstructs.Attribute{}, Health: drivers.HealthStateHealthy, @@ -291,9 +289,10 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { // Only enable if we are root if syscall.Geteuid() != 0 { - if !d.fingerprinted() { + if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Debug("must run as root user, disabling") } + d.setFingerprintFailure() fingerprint.Health = drivers.HealthStateUndetected fingerprint.HealthDescription = drivers.DriverRequiresRootMessage return fingerprint @@ -303,6 +302,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { if err != nil { fingerprint.Health = drivers.HealthStateUndetected fingerprint.HealthDescription = fmt.Sprintf("Failed to execute %s version: %v", rktCmd, err) + d.setFingerprintFailure() return fingerprint } out := strings.TrimSpace(string(outBytes)) @@ -312,6 +312,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { if len(rktMatches) != 2 || len(appcMatches) != 2 { fingerprint.Health = drivers.HealthStateUndetected fingerprint.HealthDescription = "Unable to parse rkt version string" + d.setFingerprintFailure() return fingerprint } @@ -321,10 +322,11 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { // Do not allow ancient rkt versions fingerprint.Health = drivers.HealthStateUndetected fingerprint.HealthDescription = fmt.Sprintf("Unsuported rkt version %s", currentVersion) - if !d.fingerprinted() { + if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Warn("unsupported rkt version please upgrade to >= "+minVersion.String(), "rkt_version", currentVersion) } + d.setFingerprintFailure() return fingerprint } @@ -334,7 +336,7 @@ func (d *Driver) buildFingerprint() *drivers.Fingerprint { if d.config.VolumesEnabled { fingerprint.Attributes["driver.rkt.volumes.enabled"] = pstructs.NewBoolAttribute(true) } - + d.setFingerprintSuccess() return fingerprint }