From a340baddf14f2e90912fd2210f85673d1bbf7438 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 22 Feb 2018 17:24:34 -0500 Subject: [PATCH] allow nomad to schedule based on the status of a client driver health check Slight updates for go style --- client/client.go | 24 +++++++-------- nomad/mock/mock.go | 1 + scheduler/feasible.go | 42 +++++++++++++++++--------- scheduler/feasible_test.go | 60 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 25 deletions(-) diff --git a/client/client.go b/client/client.go index 909fd368b..eb801f0d6 100644 --- a/client/client.go +++ b/client/client.go @@ -1006,15 +1006,15 @@ func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintRespons c.config.Node.Resources.Merge(response.Resources) } - for name, new_val := range response.Drivers { - old_val := c.config.Node.Drivers[name] - if new_val.Equals(old_val) { + for name, newVal := range response.Drivers { + oldVal := c.config.Node.Drivers[name] + if newVal.Equals(oldVal) { continue } - if old_val == nil { - c.config.Node.Drivers[name] = new_val + if oldVal == nil { + c.config.Node.Drivers[name] = newVal } else { - c.config.Node.Drivers[name].MergeFingerprintInfo(new_val) + c.config.Node.Drivers[name].MergeFingerprintInfo(newVal) } } if nodeHasChanged { @@ -1031,16 +1031,16 @@ func (c *Client) updateNodeFromHealthCheck(response *cstructs.HealthCheckRespons nodeHasChanged := false // update the node with the latest driver health information - for name, new_val := range response.Drivers { - old_val := c.config.Node.Drivers[name] - if new_val.Equals(old_val) { + for name, newVal := range response.Drivers { + oldVal := c.config.Node.Drivers[name] + if newVal.Equals(oldVal) { continue } nodeHasChanged = true - if old_val == nil { - c.config.Node.Drivers[name] = new_val + if oldVal == nil { + c.config.Node.Drivers[name] = newVal } else { - c.config.Node.Drivers[name].MergeHealthCheck(new_val) + c.config.Node.Drivers[name].MergeHealthCheck(newVal) } } diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 3a8588b9c..2c2338dcf 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -46,6 +46,7 @@ func Node() *structs.Node { }, }, }, + Drivers: make(map[string]*structs.DriverInfo), Links: map[string]string{ "consul": "foobar.dc1", }, diff --git a/scheduler/feasible.go b/scheduler/feasible.go index 9ba83195c..76d35138b 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -129,21 +129,37 @@ func (c *DriverChecker) Feasible(option *structs.Node) bool { func (c *DriverChecker) hasDrivers(option *structs.Node) bool { for driver := range c.drivers { driverStr := fmt.Sprintf("driver.%s", driver) - value, ok := option.Attributes[driverStr] - if !ok { - return false - } - enabled, err := strconv.ParseBool(value) - if err != nil { - c.ctx.Logger(). - Printf("[WARN] scheduler.DriverChecker: node %v has invalid driver setting %v: %v", - option.ID, driverStr, value) - return false - } + // TODO this is a compatibility mechanism- as of Nomad 0.8, nodes have a + // DriverInfo that corresponds with every driver. As a Nomad server might + // be on a later version than a Nomad client, we need to check for + // compatibility here to verify the client supports this. + if option.Drivers != nil { + driverInfo := option.Drivers[driverStr] + if driverInfo == nil { + c.ctx.Logger(). + Printf("[WARN] scheduler.DriverChecker: node %v has no driver info set for %v", + option.ID, driverStr) + return false + } + return driverInfo.Detected && driverInfo.Healthy + } else { + value, ok := option.Attributes[driverStr] + if !ok { + return false + } - if !enabled { - return false + enabled, err := strconv.ParseBool(value) + if err != nil { + c.ctx.Logger(). + Printf("[WARN] scheduler.DriverChecker: node %v has invalid driver setting %v: %v", + option.ID, driverStr, value) + return false + } + + if !enabled { + return false + } } } return true diff --git a/scheduler/feasible_test.go b/scheduler/feasible_test.go index a22f66e1e..9ee70a570 100644 --- a/scheduler/feasible_test.go +++ b/scheduler/feasible_test.go @@ -4,6 +4,7 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" @@ -125,6 +126,65 @@ func TestDriverChecker(t *testing.T) { } } +func TestDriverChecker_HealthChecks(t *testing.T) { + _, ctx := testContext(t) + nodes := []*structs.Node{ + mock.Node(), + mock.Node(), + mock.Node(), + } + nodes[0].Attributes["driver.foo"] = "1" + nodes[0].Drivers["driver.foo"] = &structs.DriverInfo{ + Detected: true, + Healthy: true, + HealthDescription: "running", + UpdateTime: time.Now(), + } + nodes[1].Attributes["driver.bar"] = "1" + nodes[1].Drivers["driver.bar"] = &structs.DriverInfo{ + Detected: true, + Healthy: false, + HealthDescription: "not running", + UpdateTime: time.Now(), + } + nodes[2].Attributes["driver.baz"] = "0" + nodes[2].Drivers["driver.baz"] = &structs.DriverInfo{ + Detected: false, + Healthy: false, + HealthDescription: "not running", + UpdateTime: time.Now(), + } + + testDrivers := []string{"foo", "bar", "baz"} + cases := []struct { + Node *structs.Node + Result bool + }{ + { + Node: nodes[0], + Result: true, + }, + { + Node: nodes[1], + Result: false, + }, + { + Node: nodes[2], + Result: false, + }, + } + + for i, c := range cases { + drivers := map[string]struct{}{ + testDrivers[i]: {}, + } + checker := NewDriverChecker(ctx, drivers) + if act := checker.Feasible(c.Node); act != c.Result { + t.Fatalf("case(%d) failed: got %v; want %v", i, act, c.Result) + } + } +} + func TestConstraintChecker(t *testing.T) { _, ctx := testContext(t) nodes := []*structs.Node{