From 44b6951dda897c08b28c2fc6c0219ca669278c4f Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 7 Mar 2018 11:58:14 -0500 Subject: [PATCH] improve tests --- client/client.go | 4 ++++ client/client_test.go | 16 +++++++++++++++- client/fingerprint_manager.go | 26 ++++++++++++++++++++------ client/fingerprint_manager_test.go | 19 +++++++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/client/client.go b/client/client.go index 3662cf6b2..4019941ae 100644 --- a/client/client.go +++ b/client/client.go @@ -1036,6 +1036,10 @@ func (c *Client) updateNodeFromHealthCheck(response *cstructs.HealthCheckRespons } oldVal := c.config.Node.Drivers[name] if newVal.HealthCheckEquals(oldVal) { + + // make sure we accurately reflect the last time a health check has been + // performed for the driver. + oldVal.UpdateTime = newVal.UpdateTime continue } nodeHasChanged = true diff --git a/client/client_test.go b/client/client_test.go index 2ec869320..c8a145c69 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -158,7 +158,7 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { return false, fmt.Errorf("mock driver is nil when it should be set on node Drivers") } if !mockDriverInfo.Detected { - return false, fmt.Errorf("mock driver should be set as healthy") + return false, fmt.Errorf("mock driver should be set as detected") } if !mockDriverInfo.Healthy { return false, fmt.Errorf("mock driver should be set as healthy") @@ -176,10 +176,24 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { testutil.WaitForResult(func() (bool, error) { c1.configLock.Lock() mockDriverStatus := node.Attributes[mockDriverName] + mockDriverInfo := node.Drivers["mock_driver"] c1.configLock.Unlock() 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") + } + if !mockDriverInfo.Detected { + return false, fmt.Errorf("mock driver should be set as detected") + } + if mockDriverInfo.Healthy { + return false, fmt.Errorf("mock driver should be set as healthy") + } + if mockDriverInfo.HealthDescription == "" { + return false, fmt.Errorf("mock driver description should not be empty") + } return true, nil }, func(err error) { t.Fatalf("err: %v", err) diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index c2541d435..817a86bd1 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -99,6 +99,10 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error { return err } + if hc, ok := d.(fingerprint.HealthCheck); ok { + fm.healthCheck(name, hc) + } + detected, err := fm.fingerprintDriver(name, d) if err != nil { fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting for %v failed: %+v", name, err) @@ -171,7 +175,9 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge return false, err } - // TODO This is a temporary measure, as eventually all drivers will need to + // COMPAT: Remove in 0.9: As of Nomad 0.8 there is a temporary measure to + // update all driver attributes to its corresponding driver info object, + // as eventually all drivers will need to // support this. Doing this so that we can enable this iteratively and also // in a backwards compatible way, where node attributes for drivers will // eventually be phased out. @@ -193,13 +199,21 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge fm.nodeLock.Unlock() } - if hc, ok := f.(fingerprint.HealthCheck); ok { - fm.healthCheck(name, hc) - } else { - + // COMPAT: Remove in 0.9: As of Nomad 0.8, there is a driver info for every + // driver. As a compatibility mechanism, we proactively set this driver info + // for drivers which do not yet support health checks. + if _, ok := f.(fingerprint.HealthCheck); !ok { resp := &cstructs.HealthCheckResponse{ Drivers: map[string]*structs.DriverInfo{ - name: di, + name: &structs.DriverInfo{ + // This achieves backwards compatibility- before, we only relied on the + // status of the fingerprint method to determine whether a driver was + // enabled. For drivers without health checks yet enabled, we should always + // set this to true, and then once we enable health checks, this can + // dynamically update this value. + Healthy: true, + UpdateTime: time.Now(), + }, }, } diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index 7a0615f6a..b37272871 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -113,6 +113,7 @@ func TestFingerprintManager_Fingerprint_Run(t *testing.T) { require.NotEqual("", node.Attributes["driver.raw_exec"]) require.True(node.Drivers["raw_exec"].Detected) + require.True(node.Drivers["raw_exec"].Healthy) } func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { @@ -273,6 +274,24 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { }, func(err error) { t.Fatalf("err: %v", err) }) + + // Ensure the mock driver is de-registered when it becomes unhealthy + testutil.WaitForResult(func() (bool, error) { + mockDriverAttribute := node.Attributes["driver.mock_driver"] + if mockDriverAttribute == "" { + return false, fmt.Errorf("mock driver info should be set on the client attributes") + } + mockDriverInfo := node.Drivers["mock_driver"] + if mockDriverInfo == nil { + return false, fmt.Errorf("mock driver info should be set on the client") + } + if !mockDriverInfo.Healthy { + return false, fmt.Errorf("mock driver info should not be healthy") + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) } func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) {