From 725f36a7ca638c0196eb9e592be5810adb7a75ba Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Apr 2018 11:26:00 -0700 Subject: [PATCH 1/5] Non-verbose driver formatting and don't display non-detected --- command/node_status.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/command/node_status.go b/command/node_status.go index 9f484203f..26e201dcc 100644 --- a/command/node_status.go +++ b/command/node_status.go @@ -396,13 +396,17 @@ func (c *NodeStatusCommand) outputTruncatedNodeDriverInfo(node *api.Node) string drivers := make([]string, 0, len(node.Drivers)) for driverName, driverInfo := range node.Drivers { + if !driverInfo.Detected { + continue + } + if !driverInfo.Healthy { drivers = append(drivers, fmt.Sprintf("%s (unhealthy)", driverName)) } else { drivers = append(drivers, driverName) } } - return strings.Trim(strings.Join(drivers, ", "), ", ") + return strings.Trim(strings.Join(drivers, ","), ", ") } func (c *NodeStatusCommand) outputNodeDriverInfo(node *api.Node) { From f45b51a13884be162397e8ad203b11debe5baa40 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Apr 2018 12:46:40 -0700 Subject: [PATCH 2/5] Driver health detection cleanups This PR does: 1. Health message based on detection has format "Driver XXX detected" and "Driver XXX not detected" 2. Set initial health description based on detection status and don't wait for the first health check. 3. Combine updating attributes on the node, fingerprint and health checking update for drivers into a single call back. 4. Condensed driver info in `node status` only shows detected drivers and make the output less wide by removing spaces. --- client/client.go | 14 +++++-- client/driver/rkt.go | 2 +- client/fingerprint_manager.go | 73 ++++++++++++++++------------------- 3 files changed, 46 insertions(+), 43 deletions(-) diff --git a/client/client.go b/client/client.go index 33885586e..914e1d2ac 100644 --- a/client/client.go +++ b/client/client.go @@ -1023,11 +1023,15 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs. var hasChanged bool + hadDriver := c.config.Node.Drivers[name] != nil if fingerprint != nil { - if c.config.Node.Drivers[name] == nil { + if !hadDriver { // If the driver info has not yet been set, do that here hasChanged = true c.config.Node.Drivers[name] = fingerprint + for attrName, newVal := range fingerprint.Attributes { + c.config.Node.Attributes[attrName] = newVal + } } else { // The driver info has already been set, fix it up if c.config.Node.Drivers[name].Detected != fingerprint.Detected { @@ -1052,9 +1056,13 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs. } if health != nil { - if c.config.Node.Drivers[name] == nil { + if !hadDriver { hasChanged = true - c.config.Node.Drivers[name] = health + if info, ok := c.config.Node.Drivers[name]; !ok { + c.config.Node.Drivers[name] = health + } else { + info.MergeHealthCheck(health) + } } else { oldVal := c.config.Node.Drivers[name] if health.HealthCheckEquals(oldVal) { diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 4554fb88e..0a990dc83 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -370,7 +370,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs } // Output version information when the fingerprinter first sees rkt - if req.Node.Attributes[rktDriverAttr] != "1" { + if info, ok := req.Node.Drivers["rkt"]; ok && info != nil && !info.Detected { d.logger.Printf("[DEBUG] driver.rkt: detect version: %s", strings.Replace(out, "\n", " ", -1)) } resp.AddAttribute(rktDriverAttr, "1") diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index 37d77ecc3..f3f64b873 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -187,17 +187,6 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error { return err } - // Set the initial health check status to be the driver detected status. - // Later, the periodic health checker will update this value for drivers - // where health checks are enabled. - healthInfo := &structs.DriverInfo{ - Healthy: detected, - UpdateTime: time.Now(), - } - if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil { - fm.setNode(node) - } - // Start a periodic watcher to detect changes to a drivers health and // attributes. go fm.watchDriver(d, name) @@ -336,6 +325,18 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge var response cstructs.FingerprintResponse fm.nodeLock.Lock() + + // Determine if the driver has been detected before. + originalNode, haveDriver := fm.node.Drivers[name] + firstDetection := !haveDriver + + // Determine if the driver is healthy + var driverIsHealthy bool + if haveDriver && originalNode.Healthy { + driverIsHealthy = true + } + + // Fingerprint the driver. request := &cstructs.FingerprintRequest{Config: fm.getConfig(), Node: fm.node} err := f.Fingerprint(request, &response) fm.nodeLock.Unlock() @@ -344,45 +345,39 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge return false, err } - if node := fm.updateNodeAttributes(&response); node != nil { - fm.setNode(node) - } - - di := &structs.DriverInfo{ + fingerprintInfo := &structs.DriverInfo{ Attributes: response.Attributes, Detected: response.Detected, } // Remove the attribute indicating the status of the driver, as the overall // driver info object should indicate this. - driverKey := fmt.Sprintf("driver.%s", name) - delete(di.Attributes, driverKey) + delete(fingerprintInfo.Attributes, fmt.Sprintf("driver.%s", name)) - if node := fm.updateNodeFromDriver(name, di, nil); node != nil { - fm.setNode(node) - } + // We set the health status based on the detection state of the driver if: + // * It is the first time we are fingerprinting the driver. This gives all + // drivers an initial health. + // * If the driver becomes undetected. This gives us an immediate unhealthy + // state and description when it transistions from detected and healthy to + // undetected. + // * If the driver does not have its own health checks. Then we always + // couple the states. + var healthInfo *structs.DriverInfo + if firstDetection || !hasPeriodicHealthCheck || !response.Detected && driverIsHealthy { + state := " " + if !response.Detected { + state = " not " + } - // Get a copy of the current node - var driverExists, driverIsHealthy bool - fm.nodeLock.Lock() - driverInfo, driverExists := fm.node.Drivers[name] - if driverExists { - driverIsHealthy = driverInfo.Healthy - } - fm.nodeLock.Unlock() - - // If either 1) the driver is undetected or 2) if the driver does not have - // periodic health checks enabled, set the health status to the match that - // of the fingerprinter - if !hasPeriodicHealthCheck || !response.Detected && driverExists && driverIsHealthy { - healthInfo := &structs.DriverInfo{ + healthInfo = &structs.DriverInfo{ Healthy: response.Detected, - HealthDescription: fmt.Sprintf("Driver %s is detected: %t", name, response.Detected), + HealthDescription: fmt.Sprintf("Driver %s is%sdetected", name, state), UpdateTime: time.Now(), } - if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil { - fm.setNode(node) - } + } + + if node := fm.updateNodeFromDriver(name, fingerprintInfo, healthInfo); node != nil { + fm.setNode(node) } return response.Detected, nil From d0605c5229fc52ff2826b29925b231b75e537702 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Apr 2018 14:29:30 -0700 Subject: [PATCH 3/5] Fix tests --- client/client_test.go | 35 +++--- client/fingerprint_manager_test.go | 179 ++++++----------------------- 2 files changed, 51 insertions(+), 163 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 3ce47872c..78d5d376d 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -20,7 +20,6 @@ import ( nconfig "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ctestutil "github.com/hashicorp/nomad/client/testutil" ) @@ -139,18 +138,22 @@ func TestClient_RPC_Passthrough(t *testing.T) { func TestClient_Fingerprint(t *testing.T) { t.Parallel() - require := require.New(t) - - driver.CheckForMockDriver(t) - c := TestClient(t, nil) defer c.Shutdown() - // Ensure default values are present - node := c.Node() - require.NotEqual("", node.Attributes["kernel.name"]) - require.NotEqual("", node.Attributes["cpu.arch"]) - require.NotEqual("", node.Attributes["driver.mock_driver"]) + // Ensure we are fingerprinting + testutil.WaitForResult(func() (bool, error) { + node := c.Node() + if _, ok := node.Attributes["kernel.name"]; !ok { + return false, fmt.Errorf("Expected value for kernel.name") + } + if _, ok := node.Attributes["cpu.arch"]; !ok { + return false, fmt.Errorf("Expected value for cpu.arch") + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) } func TestClient_Fingerprint_Periodic(t *testing.T) { @@ -162,24 +165,18 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { c1 := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ driver.ShutdownPeriodicAfter: "true", - driver.ShutdownPeriodicDuration: "3", + driver.ShutdownPeriodicDuration: "1", } }) defer c1.Shutdown() node := c1.config.Node - mockDriverName := "driver.mock_driver" - { // Ensure the mock driver is registered on the client testutil.WaitForResult(func() (bool, error) { c1.configLock.Lock() defer c1.configLock.Unlock() - mockDriverStatus := node.Attributes[mockDriverName] mockDriverInfo := node.Drivers["mock_driver"] - if mockDriverStatus == "" { - return false, fmt.Errorf("mock driver attribute should be set on the client") - } // assert that the Driver information for the node is also set correctly if mockDriverInfo == nil { @@ -204,11 +201,7 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { testutil.WaitForResult(func() (bool, error) { c1.configLock.Lock() defer c1.configLock.Unlock() - mockDriverStatus := node.Attributes[mockDriverName] mockDriverInfo := node.Drivers["mock_driver"] - if mockDriverStatus != "" { - return false, fmt.Errorf("mock driver attribute should not be set on the client") - } // assert that the Driver information for the node is also set correctly if mockDriverInfo == nil { return false, fmt.Errorf("mock driver is nil when it should be set on node Drivers") diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index 10f572d37..8acacbaab 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -16,19 +16,10 @@ func TestFingerprintManager_Run_MockDriver(t *testing.T) { driver.CheckForMockDriver(t) t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} - }) + testClient := TestClient(t, nil) testClient.logger = log.New(os.Stderr, "", log.LstdFlags) defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -44,26 +35,19 @@ func TestFingerprintManager_Run_MockDriver(t *testing.T) { node := testClient.config.Node - require.NotEqual("", node.Attributes["driver.mock_driver"]) + require.NotNil(node.Drivers["mock_driver"]) + require.True(node.Drivers["mock_driver"].Detected) + require.True(node.Drivers["mock_driver"].Healthy) } func TestFingerprintManager_Run_ResourcesFingerprint(t *testing.T) { - driver.CheckForMockDriver(t) t.Parallel() + driver.CheckForMockDriver(t) require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} - }) + testClient := TestClient(t, nil) testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -87,19 +71,10 @@ func TestFingerprintManager_Run_ResourcesFingerprint(t *testing.T) { func TestFingerprintManager_Fingerprint_Run(t *testing.T) { t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} - }) + testClient := TestClient(t, nil) testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -115,7 +90,7 @@ func TestFingerprintManager_Fingerprint_Run(t *testing.T) { node := testClient.config.Node - require.NotEqual("", node.Attributes["driver.raw_exec"]) + require.NotNil(node.Drivers["raw_exec"]) require.True(node.Drivers["raw_exec"].Detected) require.True(node.Drivers["raw_exec"].Healthy) } @@ -123,16 +98,9 @@ func TestFingerprintManager_Fingerprint_Run(t *testing.T) { func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - + driver.CheckForMockDriver(t) testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ - "driver.raw_exec.enable": "1", "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } @@ -140,7 +108,6 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -158,13 +125,13 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { // Ensure the mock driver is registered and healthy on the client testutil.WaitForResult(func() (bool, error) { fm.nodeLock.Lock() - node := fm.node defer fm.nodeLock.Unlock() - - mockDriverStatus := node.Attributes["driver.mock_driver"] - if mockDriverStatus == "" { - return false, fmt.Errorf("mock driver attribute should be set on the client") + node := fm.node + dinfo, ok := node.Drivers["mock_driver"] + if !ok || !dinfo.Detected || !dinfo.Healthy { + return false, fmt.Errorf("mock driver should be detected and healthy: %+v", dinfo) } + return true, nil }, func(err error) { t.Fatalf("err: %v", err) @@ -175,12 +142,11 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { { testutil.WaitForResult(func() (bool, error) { fm.nodeLock.Lock() - node := fm.node defer fm.nodeLock.Unlock() - - mockDriverStatus := node.Attributes["driver.mock_driver"] - if mockDriverStatus != "" { - return false, fmt.Errorf("mock driver attribute should not be set on the client") + node := fm.node + dinfo, ok := node.Drivers["mock_driver"] + if !ok || dinfo.Detected || dinfo.Healthy { + return false, fmt.Errorf("mock driver should not be detected and healthy") } return true, nil }, func(err error) { @@ -194,14 +160,7 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "driver.raw_exec.enable": "1", "test.shutdown_periodic_after": "true", @@ -211,7 +170,6 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -232,8 +190,8 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { defer fm.nodeLock.Unlock() mockDriverAttribute := node.Attributes["driver.mock_driver"] - if mockDriverAttribute == "" { - return false, fmt.Errorf("mock driver info should be set on the client attributes") + if mockDriverAttribute != "" { + return false, fmt.Errorf("mock driver info should not be set on the client attributes") } mockDriverInfo := node.Drivers["mock_driver"] if mockDriverInfo == nil { @@ -254,8 +212,8 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { defer fm.nodeLock.Unlock() rawExecAttribute := node.Attributes["driver.raw_exec"] - if rawExecAttribute == "" { - return false, fmt.Errorf("raw exec info should be set on the client attributes") + if rawExecAttribute != "" { + return false, fmt.Errorf("raw exec info should not be set on the client attributes") } rawExecInfo := node.Drivers["raw_exec"] if rawExecInfo == nil { @@ -276,8 +234,8 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { defer fm.nodeLock.Unlock() mockDriverAttribute := node.Attributes["driver.mock_driver"] - if mockDriverAttribute == "" { - return false, fmt.Errorf("mock driver info should be set on the client attributes") + if mockDriverAttribute != "" { + return false, fmt.Errorf("mock driver info should not be set on the client attributes") } mockDriverInfo := node.Drivers["mock_driver"] if mockDriverInfo == nil { @@ -297,22 +255,14 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { node := fm.node fm.nodeLock.Unlock() mockDriverAttributes := node.Drivers["mock_driver"].Attributes - require.Equal(mockDriverAttributes["driver.mock_driver"], "") + require.NotContains(mockDriverAttributes, "driver.mock_driver") } func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) { t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ - "driver.raw_exec.enable": "1", "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } @@ -320,7 +270,6 @@ func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -408,15 +357,8 @@ func TestFimgerprintManager_Run_InWhitelist(t *testing.T) { t.Parallel() require := require.New(t) - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ - "driver.raw_exec.enable": "1", "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } @@ -424,7 +366,6 @@ func TestFimgerprintManager_Run_InWhitelist(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -446,14 +387,7 @@ func TestFimgerprintManager_Run_InWhitelist(t *testing.T) { func TestFingerprintManager_Run_InBlacklist(t *testing.T) { t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "fingerprint.whitelist": " arch,memory,foo,bar ", "fingerprint.blacklist": " cpu ", @@ -462,7 +396,6 @@ func TestFingerprintManager_Run_InBlacklist(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -478,7 +411,7 @@ func TestFingerprintManager_Run_InBlacklist(t *testing.T) { node := testClient.config.Node - require.Equal(node.Attributes["cpu.frequency"], "") + require.NotContains(node.Attributes, "cpu.frequency") require.NotEqual(node.Attributes["memory.totalbytes"], "") } @@ -486,13 +419,7 @@ func TestFingerprintManager_Run_Combination(t *testing.T) { t.Parallel() require := require.New(t) - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "fingerprint.whitelist": " arch,cpu,memory,foo,bar ", "fingerprint.blacklist": " memory,nomad ", @@ -501,7 +428,6 @@ func TestFingerprintManager_Run_Combination(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -519,21 +445,14 @@ func TestFingerprintManager_Run_Combination(t *testing.T) { require.NotEqual(node.Attributes["cpu.frequency"], "") require.NotEqual(node.Attributes["cpu.arch"], "") - require.Equal(node.Attributes["memory.totalbytes"], "") - require.Equal(node.Attributes["nomad.version"], "") + require.NotContains(node.Attributes, "memory.totalbytes") + require.NotContains(node.Attributes, "nomad.version") } func TestFingerprintManager_Run_WhitelistDrivers(t *testing.T) { t.Parallel() require := require.New(t) - - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " raw_exec , foo ", @@ -542,7 +461,6 @@ func TestFingerprintManager_Run_WhitelistDrivers(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -557,20 +475,15 @@ func TestFingerprintManager_Run_WhitelistDrivers(t *testing.T) { require.Nil(err) node := testClient.config.Node - require.NotEqual(node.Attributes["driver.raw_exec"], "") + require.NotNil(node.Drivers["raw_exec"]) + require.NotContains(node.Drivers, "java") } func TestFingerprintManager_Run_AllDriversBlacklisted(t *testing.T) { t.Parallel() require := require.New(t) - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "driver.whitelist": " foo,bar,baz ", } @@ -578,7 +491,6 @@ func TestFingerprintManager_Run_AllDriversBlacklisted(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -594,22 +506,16 @@ func TestFingerprintManager_Run_AllDriversBlacklisted(t *testing.T) { node := testClient.config.Node - require.Equal(node.Attributes["driver.raw_exec"], "") - require.Equal(node.Attributes["driver.exec"], "") - require.Equal(node.Attributes["driver.docker"], "") + require.NotContains(node.Attributes, "raw_exec") + require.NotContains(node.Attributes, "exec") + require.NotContains(node.Attributes, "docker") } func TestFingerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing.T) { t.Parallel() require := require.New(t) - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " raw_exec,exec,foo,bar,baz ", @@ -619,7 +525,6 @@ func TestFingerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing. testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -635,24 +540,15 @@ func TestFingerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing. node := testClient.config.Node - require.NotEqual(node.Attributes["driver.raw_exec"], "") - require.Equal(node.Attributes["driver.exec"], "") - require.Equal(node.Attributes["foo"], "") - require.Equal(node.Attributes["bar"], "") - require.Equal(node.Attributes["baz"], "") + require.NotNil(node.Drivers["raw_exec"]) + require.NotContains(node.Drivers, "exec") } func TestFingerprintManager_Run_DriversInBlacklist(t *testing.T) { t.Parallel() require := require.New(t) - s1, serverAddr := testServer(t, nil) - defer s1.Shutdown() - testutil.WaitForLeader(t, s1.RPC) - testClient := TestClient(t, func(c *config.Config) { - c.RPCHandler = s1 - c.Servers = []string{serverAddr} c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " raw_exec,foo,bar,baz ", @@ -662,7 +558,6 @@ func TestFingerprintManager_Run_DriversInBlacklist(t *testing.T) { testClient.logger = testLogger() defer testClient.Shutdown() - waitTilNodeReady(testClient, t) fm := NewFingerprintManager( testClient.GetConfig, @@ -678,6 +573,6 @@ func TestFingerprintManager_Run_DriversInBlacklist(t *testing.T) { node := testClient.config.Node - require.NotEqual(node.Attributes["driver.raw_exec"], "") - require.Equal(node.Attributes["driver.exec"], "") + require.NotNil(node.Drivers["raw_exec"]) + require.NotContains(node.Drivers, "exec") } From 228a2319c2c45612f5e8194d402bf55e53177d6e Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 12 Apr 2018 17:52:50 -0400 Subject: [PATCH 4/5] delete driver name from only health check attributes --- client/client_test.go | 7 +++++++ client/fingerprint_manager.go | 12 ++++++++---- client/fingerprint_manager_test.go | 20 ++++++++++---------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index 78d5d376d..828dfdab7 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -176,6 +176,13 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { testutil.WaitForResult(func() (bool, error) { c1.configLock.Lock() defer c1.configLock.Unlock() + + // assert that the driver is set on the node attributes + mockDriverInfoAttr := node.Attributes["driver.mock_driver"] + if mockDriverInfoAttr == "" { + return false, fmt.Errorf("mock driver is empty when it should be set on the node attributes") + } + mockDriverInfo := node.Drivers["mock_driver"] // assert that the Driver information for the node is also set correctly diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index f3f64b873..abe2f6ed6 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -345,15 +345,19 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge return false, err } + if node := fm.updateNodeAttributes(&response); node != nil { + fm.setNode(node) + } + + // Remove the health check attribute indicating the status of the driver, + // as the overall driver info object should indicate this. + delete(response.Attributes, fmt.Sprintf("driver.%s", name)) + fingerprintInfo := &structs.DriverInfo{ Attributes: response.Attributes, Detected: response.Detected, } - // Remove the attribute indicating the status of the driver, as the overall - // driver info object should indicate this. - delete(fingerprintInfo.Attributes, fmt.Sprintf("driver.%s", name)) - // We set the health status based on the detection state of the driver if: // * It is the first time we are fingerprinting the driver. This gives all // drivers an initial health. diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index 8acacbaab..b2cca2bc9 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -190,8 +190,8 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { defer fm.nodeLock.Unlock() mockDriverAttribute := node.Attributes["driver.mock_driver"] - if mockDriverAttribute != "" { - return false, fmt.Errorf("mock driver info should not be set on the client attributes") + if mockDriverAttribute == "" { + return false, fmt.Errorf("mock driver info should be set on the client attributes") } mockDriverInfo := node.Drivers["mock_driver"] if mockDriverInfo == nil { @@ -212,8 +212,8 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { defer fm.nodeLock.Unlock() rawExecAttribute := node.Attributes["driver.raw_exec"] - if rawExecAttribute != "" { - return false, fmt.Errorf("raw exec info should not be set on the client attributes") + if rawExecAttribute == "" { + return false, fmt.Errorf("raw exec info should be set on the client attributes") } rawExecInfo := node.Drivers["raw_exec"] if rawExecInfo == nil { @@ -227,15 +227,15 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { t.Fatalf("err: %v", err) }) - // Ensure the mock driver is de-registered when it becomes unhealthy + // Ensure the mock driver is registered testutil.WaitForResult(func() (bool, error) { fm.nodeLock.Lock() node := fm.node defer fm.nodeLock.Unlock() mockDriverAttribute := node.Attributes["driver.mock_driver"] - if mockDriverAttribute != "" { - return false, fmt.Errorf("mock driver info should not be set on the client attributes") + if mockDriverAttribute == "" { + return false, fmt.Errorf("mock driver info should set on the client attributes") } mockDriverInfo := node.Drivers["mock_driver"] if mockDriverInfo == nil { @@ -506,9 +506,9 @@ func TestFingerprintManager_Run_AllDriversBlacklisted(t *testing.T) { node := testClient.config.Node - require.NotContains(node.Attributes, "raw_exec") - require.NotContains(node.Attributes, "exec") - require.NotContains(node.Attributes, "docker") + require.NotContains(node.Attributes, "driver.raw_exec") + require.NotContains(node.Attributes, "driver.exec") + require.NotContains(node.Attributes, "driver.docker") } func TestFingerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing.T) { From e0171acbdd9ed404bf356f486a15542693a7bd9c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 12 Apr 2018 15:50:25 -0700 Subject: [PATCH 5/5] Move where attribute for driver detection is set --- client/client.go | 10 ++++++++++ client/fingerprint_manager.go | 4 ---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/client/client.go b/client/client.go index 914e1d2ac..46d8bd1d7 100644 --- a/client/client.go +++ b/client/client.go @@ -1053,6 +1053,16 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs. } } } + + // COMPAT Remove in Nomad 0.10 + // We maintain the driver enabled attribute until all drivers expose + // their attributes as DriverInfo + driverName := fmt.Sprintf("driver.%s", name) + if fingerprint.Detected { + c.config.Node.Attributes[driverName] = "1" + } else { + delete(c.config.Node.Attributes, driverName) + } } if health != nil { diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index abe2f6ed6..499cc16d7 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -345,10 +345,6 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge return false, err } - if node := fm.updateNodeAttributes(&response); node != nil { - fm.setNode(node) - } - // Remove the health check attribute indicating the status of the driver, // as the overall driver info object should indicate this. delete(response.Attributes, fmt.Sprintf("driver.%s", name))