From cc1213a8bbdf0e326f1adc22a0ca099470b5f848 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 24 Jan 2018 08:01:37 -0500 Subject: [PATCH 1/4] add fingerprint manager --- client/client.go | 112 ++++++----------------- client/fingerprint_manager.go | 122 +++++++++++++++++++++++++ client/fingerprint_manager_test.go | 139 +++++++++++++++++++++++++++++ 3 files changed, 287 insertions(+), 86 deletions(-) create mode 100644 client/fingerprint_manager.go create mode 100644 client/fingerprint_manager_test.go diff --git a/client/client.go b/client/client.go index 4054fca6b10..9cc72a5af11 100644 --- a/client/client.go +++ b/client/client.go @@ -230,13 +230,24 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic return nil, fmt.Errorf("node setup failed: %v", err) } + fingerprintManager := &FingerprintManager{ + getConfig: func() *config.Config { + c.configLock.Lock() + defer c.configLock.Unlock() + return c.config + }, + node: c.config.Node, + client: c, + logger: c.logger, + } + // Fingerprint the node - if err := c.fingerprint(); err != nil { + if err := c.fingerprint(fingerprintManager); err != nil { return nil, fmt.Errorf("fingerprinting failed: %v", err) } // Scan for drivers - if err := c.setupDrivers(); err != nil { + if err := c.setupDrivers(fingerprintManager); err != nil { return nil, fmt.Errorf("driver setup failed: %v", err) } @@ -925,14 +936,14 @@ func (c *Client) reservePorts() { } // fingerprint is used to fingerprint the client and setup the node -func (c *Client) fingerprint() error { +func (c *Client) fingerprint(fingerprintManager *FingerprintManager) error { whitelist := c.config.ReadStringListToMap("fingerprint.whitelist") whitelistEnabled := len(whitelist) > 0 blacklist := c.config.ReadStringListToMap("fingerprint.blacklist") c.logger.Printf("[DEBUG] client: built-in fingerprints: %v", fingerprint.BuiltinFingerprints()) - var detectedFingerprints []string + var availableFingerprints []string var skippedFingerprints []string for _, name := range fingerprint.BuiltinFingerprints() { // Skip modules that are not in the whitelist if it is enabled. @@ -946,78 +957,29 @@ func (c *Client) fingerprint() error { continue } - f, err := fingerprint.NewFingerprint(name, c.logger) - if err != nil { - return err - } - - c.configLock.Lock() - request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} - var response cstructs.FingerprintResponse - err = f.Fingerprint(request, &response) - c.configLock.Unlock() - if err != nil { - return err - } - - // log the fingerprinters which have been applied - if response.Detected { - detectedFingerprints = append(detectedFingerprints, name) - } - - // add the diff found from each fingerprinter - c.updateNodeFromFingerprint(&response) + availableFingerprints = append(availableFingerprints, name) + } - p, period := f.Periodic() - if p { - // TODO: If more periodic fingerprinters are added, then - // fingerprintPeriodic should be used to handle all the periodic - // fingerprinters by using a priority queue. - go c.fingerprintPeriodic(name, f, period) - } + if err := fingerprintManager.SetupFingerprints(availableFingerprints); err != nil { + return err } - c.logger.Printf("[DEBUG] client: detected fingerprints %v", detectedFingerprints) if len(skippedFingerprints) != 0 { c.logger.Printf("[DEBUG] client: fingerprint modules skipped due to white/blacklist: %v", skippedFingerprints) } return nil } -// fingerprintPeriodic runs a fingerprinter at the specified duration. -func (c *Client) fingerprintPeriodic(name string, f fingerprint.Fingerprint, d time.Duration) { - c.logger.Printf("[DEBUG] client: fingerprinting %v every %v", name, d) - for { - select { - case <-time.After(d): - c.configLock.Lock() - request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} - var response cstructs.FingerprintResponse - err := f.Fingerprint(request, &response) - c.configLock.Unlock() - - if err != nil { - c.logger.Printf("[DEBUG] client: periodic fingerprinting for %v failed: %v", name, err) - } else { - c.updateNodeFromFingerprint(&response) - } - - case <-c.shutdownCh: - return - } - } -} - // setupDrivers is used to find the available drivers -func (c *Client) setupDrivers() error { +func (c *Client) setupDrivers(fingerprintManager *FingerprintManager) error { // Build the white/blacklists of drivers. whitelist := c.config.ReadStringListToMap("driver.whitelist") whitelistEnabled := len(whitelist) > 0 blacklist := c.config.ReadStringListToMap("driver.blacklist") - var detectedDrivers []string + var availDrivers []string var skippedDrivers []string - driverCtx := driver.NewDriverContext("", "", c.config, c.config.Node, c.logger, nil) + for name := range driver.BuiltinDrivers { // Skip fingerprinting drivers that are not in the whitelist if it is // enabled. @@ -1031,35 +993,13 @@ func (c *Client) setupDrivers() error { continue } - d, err := driver.NewDriver(name, driverCtx) - if err != nil { - return err - } - - c.configLock.Lock() - request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} - var response cstructs.FingerprintResponse - err = d.Fingerprint(request, &response) - c.configLock.Unlock() - if err != nil { - return err - } - - // log the fingerprinters which have been applied - if response.Detected { - detectedDrivers = append(detectedDrivers, name) - } - - c.updateNodeFromFingerprint(&response) - - p, period := d.Periodic() - if p { - go c.fingerprintPeriodic(name, d, period) - } + availDrivers = append(availDrivers, name) + } + if err := fingerprintManager.SetupDrivers(availDrivers); err != nil { + return err } - c.logger.Printf("[DEBUG] client: detected drivers %v", detectedDrivers) if len(skippedDrivers) > 0 { c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skippedDrivers) } diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go new file mode 100644 index 00000000000..c0c438827a1 --- /dev/null +++ b/client/fingerprint_manager.go @@ -0,0 +1,122 @@ +package client + +import ( + "log" + "time" + + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/driver" + "github.com/hashicorp/nomad/client/fingerprint" + cstructs "github.com/hashicorp/nomad/client/structs" + "github.com/hashicorp/nomad/nomad/structs" +) + +// FingerprintManager runs a client fingerprinters on a continuous basis, and +// updates the client when the node has changed +type FingerprintManager struct { + getConfig func() *config.Config + node *structs.Node + client *Client + logger *log.Logger +} + +// run runs each fingerprinter individually on an ongoing basis +func (fm *FingerprintManager) run(f fingerprint.Fingerprint, period time.Duration, name string) { + fm.logger.Printf("[DEBUG] fingerprint_manager: fingerprinting %s every %v", name, period) + + for { + select { + case <-time.After(period): + _, err := fm.fingerprint(name, f) + if err != nil { + fm.logger.Printf("[DEBUG] fingerprint_manager: periodic fingerprinting for %v failed: %+v", name, err) + continue + } + + case <-fm.client.shutdownCh: + return + } + } +} + +// setupDrivers is used to fingerprint the node to see if these drivers are +// supported +func (fm *FingerprintManager) SetupDrivers(drivers []string) error { + var availDrivers []string + driverCtx := driver.NewDriverContext("", "", fm.getConfig(), fm.node, fm.logger, nil) + for _, name := range drivers { + + d, err := driver.NewDriver(name, driverCtx) + if err != nil { + return err + } + + detected, err := fm.fingerprint(name, d) + if err != nil { + fm.logger.Printf("[DEBUG] fingerprint_manager: fingerprinting for %v failed: %+v", name, err) + return err + } + + // log the fingerprinters which have been applied + if detected { + availDrivers = append(availDrivers, name) + } + + p, period := d.Periodic() + if p { + go fm.run(d, period, name) + } + } + + fm.logger.Printf("[DEBUG] fingerprint_manager: available drivers %v", availDrivers) + + return nil +} + +// fingerprint does an initial fingerprint of the client. If the fingerprinter +// is meant to be run continuously, a process is launched ro perform this +// fingerprint on an ongoing basis in the background. +func (fm *FingerprintManager) fingerprint(name string, f fingerprint.Fingerprint) (bool, error) { + request := &cstructs.FingerprintRequest{Config: fm.getConfig(), Node: fm.node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) + if err != nil { + return false, err + } + + fm.client.updateNodeFromFingerprint(&response) + return response.Detected, nil +} + +// setupDrivers is used to fingerprint the node to see if these attributes are +// supported +func (fm *FingerprintManager) SetupFingerprints(fingerprints []string) error { + var appliedFingerprints []string + + for _, name := range fingerprints { + f, err := fingerprint.NewFingerprint(name, fm.logger) + + if err != nil { + fm.logger.Printf("[DEBUG] fingerprint_manager: fingerprinting for %v failed: %+v", name, err) + return err + } + + detected, err := fm.fingerprint(name, f) + if err != nil { + return err + } + + // log the fingerprinters which have been applied + if detected { + appliedFingerprints = append(appliedFingerprints, name) + } + + p, period := f.Periodic() + if p { + go fm.run(f, period, name) + } + } + + fm.logger.Printf("[DEBUG] fingerprint_manager: applied fingerprints %v", appliedFingerprints) + return nil +} diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go new file mode 100644 index 00000000000..a0f0c600d1d --- /dev/null +++ b/client/fingerprint_manager_test.go @@ -0,0 +1,139 @@ +package client + +import ( + "fmt" + "testing" + + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/driver" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/require" +) + +// test that the driver manager updates a node when its attributes change +func TestFingerprintManager_Fingerprint_MockDriver(t *testing.T) { + if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { + t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) + } + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + mockConfig := &config.Config{ + Node: node, + } + c := &Client{ + config: mockConfig, + } + getConfig := func() *config.Config { + return mockConfig + } + + fm := FingerprintManager{ + getConfig: getConfig, + node: node, + client: c, + logger: testLogger(), + } + + // test setting up a mock driver + drivers := []string{"mock_driver"} + err := fm.SetupDrivers(drivers) + require.Nil(err) + + require.NotEqual("", node.Attributes["driver.mock_driver"]) +} + +func TestFingerprintManager_Fingerprint_RawExec(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + mockConfig := &config.Config{ + Node: node, + Options: map[string]string{ + "driver.raw_exec.enable": "true", + }, + } + c := &Client{ + config: mockConfig, + } + getConfig := func() *config.Config { + return mockConfig + } + + fm := FingerprintManager{ + getConfig: getConfig, + node: node, + client: c, + logger: testLogger(), + } + + // test setting up a mock driver + drivers := []string{"raw_exec"} + err := fm.SetupDrivers(drivers) + require.Nil(err) + + require.NotEqual("", node.Attributes["driver.raw_exec"]) +} + +func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + mockConfig := &config.Config{ + Node: node, + Options: map[string]string{ + "test.shutdown_periodic_after": "true", + "test.shutdown_periodic_duration": "3", + }, + } + c := &Client{ + config: mockConfig, + } + getConfig := func() *config.Config { + return mockConfig + } + + fm := FingerprintManager{ + getConfig: getConfig, + node: node, + client: c, + logger: testLogger(), + } + + // test setting up a mock driver + drivers := []string{"mock_driver"} + err := fm.SetupDrivers(drivers) + require.Nil(err) + + // Ensure the mock driver is registered on the client + testutil.WaitForResult(func() (bool, error) { + mockDriverStatus := node.Attributes["driver.mock_driver"] + if mockDriverStatus == "" { + return false, fmt.Errorf("mock driver attribute should be set on the client") + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) + + // Ensure that the client fingerprinter eventually removes this attribute + testutil.WaitForResult(func() (bool, error) { + mockDriverStatus := node.Attributes["driver.mock_driver"] + if mockDriverStatus != "" { + return false, fmt.Errorf("mock driver attribute should not be set on the client") + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) +} From ca3b28c5c61f45f397c7eca6b96cff525168b6d1 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 5 Feb 2018 12:20:07 -0500 Subject: [PATCH 2/4] remove dependency on client for fingerprint manager --- client/client.go | 9 +++-- client/fingerprint_manager.go | 22 ++++++----- client/fingerprint_manager_test.go | 60 ++++++++++++++++++------------ 3 files changed, 55 insertions(+), 36 deletions(-) diff --git a/client/client.go b/client/client.go index 9cc72a5af11..35a6b4f3221 100644 --- a/client/client.go +++ b/client/client.go @@ -236,9 +236,10 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic defer c.configLock.Unlock() return c.config }, - node: c.config.Node, - client: c, - logger: c.logger, + node: c.config.Node, + updateNode: c.updateNodeFromFingerprint, + shutdownCh: c.shutdownCh, + logger: c.logger, } // Fingerprint the node @@ -960,7 +961,7 @@ func (c *Client) fingerprint(fingerprintManager *FingerprintManager) error { availableFingerprints = append(availableFingerprints, name) } - if err := fingerprintManager.SetupFingerprints(availableFingerprints); err != nil { + if err := fingerprintManager.SetupFingerprinters(availableFingerprints); err != nil { return err } diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index c0c438827a1..91ca5ac8e87 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -14,10 +14,14 @@ import ( // FingerprintManager runs a client fingerprinters on a continuous basis, and // updates the client when the node has changed type FingerprintManager struct { - getConfig func() *config.Config - node *structs.Node - client *Client - logger *log.Logger + getConfig func() *config.Config + node *structs.Node + shutdownCh chan struct{} + + // updateNode is a callback to the client to update the state of its + // associated node + updateNode func(*cstructs.FingerprintResponse) + logger *log.Logger } // run runs each fingerprinter individually on an ongoing basis @@ -33,7 +37,7 @@ func (fm *FingerprintManager) run(f fingerprint.Fingerprint, period time.Duratio continue } - case <-fm.client.shutdownCh: + case <-fm.shutdownCh: return } } @@ -74,7 +78,7 @@ func (fm *FingerprintManager) SetupDrivers(drivers []string) error { } // fingerprint does an initial fingerprint of the client. If the fingerprinter -// is meant to be run continuously, a process is launched ro perform this +// is meant to be run continuously, a process is launched to perform this // fingerprint on an ongoing basis in the background. func (fm *FingerprintManager) fingerprint(name string, f fingerprint.Fingerprint) (bool, error) { request := &cstructs.FingerprintRequest{Config: fm.getConfig(), Node: fm.node} @@ -84,13 +88,13 @@ func (fm *FingerprintManager) fingerprint(name string, f fingerprint.Fingerprint return false, err } - fm.client.updateNodeFromFingerprint(&response) + fm.updateNode(&response) return response.Detected, nil } -// setupDrivers is used to fingerprint the node to see if these attributes are +// setupFingerprints is used to fingerprint the node to see if these attributes are // supported -func (fm *FingerprintManager) SetupFingerprints(fingerprints []string) error { +func (fm *FingerprintManager) SetupFingerprinters(fingerprints []string) error { var appliedFingerprints []string for _, name := range fingerprints { diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index a0f0c600d1d..fa5894fd483 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" @@ -25,18 +26,21 @@ func TestFingerprintManager_Fingerprint_MockDriver(t *testing.T) { mockConfig := &config.Config{ Node: node, } - c := &Client{ - config: mockConfig, + + var resp *cstructs.FingerprintResponse + updateNode := func(r *cstructs.FingerprintResponse) { + resp = r } + getConfig := func() *config.Config { return mockConfig } fm := FingerprintManager{ - getConfig: getConfig, - node: node, - client: c, - logger: testLogger(), + getConfig: getConfig, + node: node, + updateNode: updateNode, + logger: testLogger(), } // test setting up a mock driver @@ -44,7 +48,7 @@ func TestFingerprintManager_Fingerprint_MockDriver(t *testing.T) { err := fm.SetupDrivers(drivers) require.Nil(err) - require.NotEqual("", node.Attributes["driver.mock_driver"]) + require.NotEqual("", resp.Attributes["driver.mock_driver"]) } func TestFingerprintManager_Fingerprint_RawExec(t *testing.T) { @@ -60,18 +64,20 @@ func TestFingerprintManager_Fingerprint_RawExec(t *testing.T) { "driver.raw_exec.enable": "true", }, } - c := &Client{ - config: mockConfig, + var resp *cstructs.FingerprintResponse + updateNode := func(r *cstructs.FingerprintResponse) { + resp = r } + getConfig := func() *config.Config { return mockConfig } fm := FingerprintManager{ - getConfig: getConfig, - node: node, - client: c, - logger: testLogger(), + getConfig: getConfig, + node: node, + updateNode: updateNode, + logger: testLogger(), } // test setting up a mock driver @@ -79,7 +85,7 @@ func TestFingerprintManager_Fingerprint_RawExec(t *testing.T) { err := fm.SetupDrivers(drivers) require.Nil(err) - require.NotEqual("", node.Attributes["driver.raw_exec"]) + require.NotEqual("", resp.Attributes["driver.raw_exec"]) } func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { @@ -89,6 +95,10 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string, 0), } + var resp *cstructs.FingerprintResponse + updateNode := func(r *cstructs.FingerprintResponse) { + resp = r + } mockConfig := &config.Config{ Node: node, Options: map[string]string{ @@ -96,18 +106,22 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { "test.shutdown_periodic_duration": "3", }, } - c := &Client{ - config: mockConfig, - } + getConfig := func() *config.Config { return mockConfig } + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + fm := FingerprintManager{ - getConfig: getConfig, - node: node, - client: c, - logger: testLogger(), + getConfig: getConfig, + node: node, + updateNode: updateNode, + shutdownCh: shutdownCh, + logger: testLogger(), } // test setting up a mock driver @@ -117,7 +131,7 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { // Ensure the mock driver is registered on the client testutil.WaitForResult(func() (bool, error) { - mockDriverStatus := node.Attributes["driver.mock_driver"] + mockDriverStatus := resp.Attributes["driver.mock_driver"] if mockDriverStatus == "" { return false, fmt.Errorf("mock driver attribute should be set on the client") } @@ -128,7 +142,7 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { // Ensure that the client fingerprinter eventually removes this attribute testutil.WaitForResult(func() (bool, error) { - mockDriverStatus := node.Attributes["driver.mock_driver"] + mockDriverStatus := resp.Attributes["driver.mock_driver"] if mockDriverStatus != "" { return false, fmt.Errorf("mock driver attribute should not be set on the client") } From 1a26950114aa1163b12b7ce86e86ed0eec22ae37 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 5 Feb 2018 18:02:52 -0500 Subject: [PATCH 3/4] code review feedback --- client/client.go | 103 +------- client/client_test.go | 179 +------------ client/fingerprint_manager.go | 112 +++++++- client/fingerprint_manager_test.go | 406 ++++++++++++++++++++++++----- 4 files changed, 459 insertions(+), 341 deletions(-) diff --git a/client/client.go b/client/client.go index 35a6b4f3221..e27d2d62caf 100644 --- a/client/client.go +++ b/client/client.go @@ -20,8 +20,6 @@ import ( multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver" - "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/client/stats" cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/client/vaultclient" @@ -230,28 +228,14 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic return nil, fmt.Errorf("node setup failed: %v", err) } - fingerprintManager := &FingerprintManager{ - getConfig: func() *config.Config { - c.configLock.Lock() - defer c.configLock.Unlock() - return c.config - }, - node: c.config.Node, - updateNode: c.updateNodeFromFingerprint, - shutdownCh: c.shutdownCh, - logger: c.logger, - } + fingerprintManager := NewFingerprintManager(c.GetConfig, c.config.Node, + c.shutdownCh, c.updateNodeFromFingerprint, c.logger) - // Fingerprint the node - if err := c.fingerprint(fingerprintManager); err != nil { + // Fingerprint the node and scan for drivers + if err := fingerprintManager.Run(); err != nil { return nil, fmt.Errorf("fingerprinting failed: %v", err) } - // Scan for drivers - if err := c.setupDrivers(fingerprintManager); err != nil { - return nil, fmt.Errorf("driver setup failed: %v", err) - } - // Setup the reserved resources c.reservePorts() @@ -412,8 +396,10 @@ func (c *Client) Leave() error { return nil } -// GetConfig returns the config of the client for testing purposes only +// GetConfig returns the config of the client func (c *Client) GetConfig() *config.Config { + c.configLock.Lock() + defer c.configLock.Unlock() return c.config } @@ -936,81 +922,9 @@ func (c *Client) reservePorts() { } } -// fingerprint is used to fingerprint the client and setup the node -func (c *Client) fingerprint(fingerprintManager *FingerprintManager) error { - whitelist := c.config.ReadStringListToMap("fingerprint.whitelist") - whitelistEnabled := len(whitelist) > 0 - blacklist := c.config.ReadStringListToMap("fingerprint.blacklist") - - c.logger.Printf("[DEBUG] client: built-in fingerprints: %v", fingerprint.BuiltinFingerprints()) - - var availableFingerprints []string - var skippedFingerprints []string - for _, name := range fingerprint.BuiltinFingerprints() { - // Skip modules that are not in the whitelist if it is enabled. - if _, ok := whitelist[name]; whitelistEnabled && !ok { - skippedFingerprints = append(skippedFingerprints, name) - continue - } - // Skip modules that are in the blacklist - if _, ok := blacklist[name]; ok { - skippedFingerprints = append(skippedFingerprints, name) - continue - } - - availableFingerprints = append(availableFingerprints, name) - } - - if err := fingerprintManager.SetupFingerprinters(availableFingerprints); err != nil { - return err - } - - if len(skippedFingerprints) != 0 { - c.logger.Printf("[DEBUG] client: fingerprint modules skipped due to white/blacklist: %v", skippedFingerprints) - } - return nil -} - -// setupDrivers is used to find the available drivers -func (c *Client) setupDrivers(fingerprintManager *FingerprintManager) error { - // Build the white/blacklists of drivers. - whitelist := c.config.ReadStringListToMap("driver.whitelist") - whitelistEnabled := len(whitelist) > 0 - blacklist := c.config.ReadStringListToMap("driver.blacklist") - - var availDrivers []string - var skippedDrivers []string - - for name := range driver.BuiltinDrivers { - // Skip fingerprinting drivers that are not in the whitelist if it is - // enabled. - if _, ok := whitelist[name]; whitelistEnabled && !ok { - skippedDrivers = append(skippedDrivers, name) - continue - } - // Skip fingerprinting drivers that are in the blacklist - if _, ok := blacklist[name]; ok { - skippedDrivers = append(skippedDrivers, name) - continue - } - - availDrivers = append(availDrivers, name) - } - - if err := fingerprintManager.SetupDrivers(availDrivers); err != nil { - return err - } - - if len(skippedDrivers) > 0 { - c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skippedDrivers) - } - - return nil -} - // updateNodeFromFingerprint updates the node with the result of // fingerprinting the node from the diff that was created -func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) { +func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) *structs.Node { c.configLock.Lock() defer c.configLock.Unlock() for name, val := range response.Attributes { @@ -1034,6 +948,7 @@ func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintRespons if response.Resources != nil { c.config.Node.Resources.Merge(response.Resources) } + return c.config.Node } // retryIntv calculates a retry interval value given the base diff --git a/client/client_test.go b/client/client_test.go index 9b16f93e7a1..f653b3eb903 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -26,6 +26,7 @@ import ( "github.com/hashicorp/nomad/testutil" "github.com/mitchellh/hashstructure" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ctestutil "github.com/hashicorp/nomad/client/testutil" ) @@ -208,17 +209,19 @@ func TestClient_RPC_Passthrough(t *testing.T) { func TestClient_Fingerprint(t *testing.T) { t.Parallel() + require := require.New(t) + if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { + t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) + } + c := testClient(t, nil) defer c.Shutdown() - // Ensure kernel and arch are always present + // Ensure default values are present node := c.Node() - if node.Attributes["kernel.name"] == "" { - t.Fatalf("missing kernel.name") - } - if node.Attributes["cpu.arch"] == "" { - t.Fatalf("missing cpu arch") - } + require.NotEqual("", node.Attributes["kernel.name"]) + require.NotEqual("", node.Attributes["cpu.arch"]) + require.NotEqual("", node.Attributes["driver.mock_driver"]) } func TestClient_HasNodeChanged(t *testing.T) { @@ -295,168 +298,6 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { }) } -func TestClient_Fingerprint_InWhitelist(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - // Weird spacing to test trimming. Whitelist all modules expect cpu. - c.Options["fingerprint.whitelist"] = " arch, consul,cpu,env_aws,env_gce,host,memory,network,storage,foo,bar " - }) - defer c.Shutdown() - - node := c.Node() - if node.Attributes["cpu.frequency"] == "" { - t.Fatalf("missing cpu fingerprint module") - } -} - -func TestClient_Fingerprint_InBlacklist(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - // Weird spacing to test trimming. Blacklist cpu. - c.Options["fingerprint.blacklist"] = " cpu " - }) - defer c.Shutdown() - - node := c.Node() - if node.Attributes["cpu.frequency"] != "" { - t.Fatalf("cpu fingerprint module loaded despite blacklisting") - } -} - -func TestClient_Fingerprint_OutOfWhitelist(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - c.Options["fingerprint.whitelist"] = "arch,consul,env_aws,env_gce,host,memory,network,storage,foo,bar" - }) - defer c.Shutdown() - - node := c.Node() - if node.Attributes["cpu.frequency"] != "" { - t.Fatalf("found cpu fingerprint module") - } -} - -func TestClient_Fingerprint_WhitelistBlacklistCombination(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - // With both white- and blacklist, should return the set difference of modules (arch, cpu) - c.Options["fingerprint.whitelist"] = "arch,memory,cpu" - c.Options["fingerprint.blacklist"] = "memory,nomad" - }) - defer c.Shutdown() - - node := c.Node() - // Check expected modules are present - if node.Attributes["cpu.frequency"] == "" { - t.Fatalf("missing cpu fingerprint module") - } - if node.Attributes["cpu.arch"] == "" { - t.Fatalf("missing arch fingerprint module") - } - // Check remainder _not_ present - if node.Attributes["memory.totalbytes"] != "" { - t.Fatalf("found memory fingerprint module") - } - if node.Attributes["nomad.version"] != "" { - t.Fatalf("found nomad fingerprint module") - } -} - -func TestClient_Drivers_InWhitelist(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - // Weird spacing to test trimming - c.Options["driver.raw_exec.enable"] = "1" - c.Options["driver.whitelist"] = " raw_exec , foo " - }) - defer c.Shutdown() - - node := c.Node() - if node.Attributes["driver.raw_exec"] == "" { - t.Fatalf("missing raw_exec driver") - } -} - -func TestClient_Drivers_InBlacklist(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - // Weird spacing to test trimming - c.Options["driver.raw_exec.enable"] = "1" - c.Options["driver.blacklist"] = " raw_exec , foo " - }) - defer c.Shutdown() - - node := c.Node() - if node.Attributes["driver.raw_exec"] != "" { - t.Fatalf("raw_exec driver loaded despite blacklist") - } -} - -func TestClient_Drivers_OutOfWhitelist(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - c.Options["driver.whitelist"] = "foo,bar,baz" - }) - defer c.Shutdown() - - node := c.Node() - if node.Attributes["driver.exec"] != "" { - t.Fatalf("found exec driver") - } -} - -func TestClient_Drivers_WhitelistBlacklistCombination(t *testing.T) { - t.Parallel() - c := testClient(t, func(c *config.Config) { - if c.Options == nil { - c.Options = make(map[string]string) - } - - // Expected output is set difference (raw_exec) - c.Options["driver.whitelist"] = "raw_exec,exec" - c.Options["driver.blacklist"] = "exec" - }) - defer c.Shutdown() - - node := c.Node() - // Check expected present - if node.Attributes["driver.raw_exec"] == "" { - t.Fatalf("missing raw_exec driver") - } - // Check expected absent - if node.Attributes["driver.exec"] != "" { - t.Fatalf("exec driver loaded despite blacklist") - } -} - // TestClient_MixedTLS asserts that when a server is running with TLS enabled // it will reject any RPC connections from clients that lack TLS. See #2525 func TestClient_MixedTLS(t *testing.T) { diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index 91ca5ac8e87..5e22396ca90 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -20,20 +20,36 @@ type FingerprintManager struct { // updateNode is a callback to the client to update the state of its // associated node - updateNode func(*cstructs.FingerprintResponse) + updateNode func(*cstructs.FingerprintResponse) *structs.Node logger *log.Logger } +// NewFingerprintManager is a constructor that creates and returns an instance +// of FingerprintManager +func NewFingerprintManager(getConfig func() *config.Config, + node *structs.Node, + shutdownCh chan struct{}, + updateNode func(*cstructs.FingerprintResponse) *structs.Node, + logger *log.Logger) *FingerprintManager { + return &FingerprintManager{ + getConfig: getConfig, + updateNode: updateNode, + node: node, + shutdownCh: shutdownCh, + logger: logger, + } +} + // run runs each fingerprinter individually on an ongoing basis func (fm *FingerprintManager) run(f fingerprint.Fingerprint, period time.Duration, name string) { - fm.logger.Printf("[DEBUG] fingerprint_manager: fingerprinting %s every %v", name, period) + fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting %s every %v", name, period) for { select { case <-time.After(period): _, err := fm.fingerprint(name, f) if err != nil { - fm.logger.Printf("[DEBUG] fingerprint_manager: periodic fingerprinting for %v failed: %+v", name, err) + fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for %v failed: %+v", name, err) continue } @@ -45,7 +61,7 @@ func (fm *FingerprintManager) run(f fingerprint.Fingerprint, period time.Duratio // setupDrivers is used to fingerprint the node to see if these drivers are // supported -func (fm *FingerprintManager) SetupDrivers(drivers []string) error { +func (fm *FingerprintManager) setupDrivers(drivers []string) error { var availDrivers []string driverCtx := driver.NewDriverContext("", "", fm.getConfig(), fm.node, fm.logger, nil) for _, name := range drivers { @@ -57,7 +73,7 @@ func (fm *FingerprintManager) SetupDrivers(drivers []string) error { detected, err := fm.fingerprint(name, d) if err != nil { - fm.logger.Printf("[DEBUG] fingerprint_manager: fingerprinting for %v failed: %+v", name, err) + fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting for %v failed: %+v", name, err) return err } @@ -72,8 +88,7 @@ func (fm *FingerprintManager) SetupDrivers(drivers []string) error { } } - fm.logger.Printf("[DEBUG] fingerprint_manager: available drivers %v", availDrivers) - + fm.logger.Printf("[DEBUG] client.fingerprint_manager: detected drivers %v", availDrivers) return nil } @@ -83,25 +98,27 @@ func (fm *FingerprintManager) SetupDrivers(drivers []string) error { func (fm *FingerprintManager) fingerprint(name string, f fingerprint.Fingerprint) (bool, error) { request := &cstructs.FingerprintRequest{Config: fm.getConfig(), Node: fm.node} var response cstructs.FingerprintResponse - err := f.Fingerprint(request, &response) - if err != nil { + if err := f.Fingerprint(request, &response); err != nil { return false, err } - fm.updateNode(&response) + if node := fm.updateNode(&response); node != nil { + fm.node = node + } + return response.Detected, nil } // setupFingerprints is used to fingerprint the node to see if these attributes are // supported -func (fm *FingerprintManager) SetupFingerprinters(fingerprints []string) error { +func (fm *FingerprintManager) setupFingerprinters(fingerprints []string) error { var appliedFingerprints []string for _, name := range fingerprints { f, err := fingerprint.NewFingerprint(name, fm.logger) if err != nil { - fm.logger.Printf("[DEBUG] fingerprint_manager: fingerprinting for %v failed: %+v", name, err) + fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting for %v failed: %+v", name, err) return err } @@ -121,6 +138,75 @@ func (fm *FingerprintManager) SetupFingerprinters(fingerprints []string) error { } } - fm.logger.Printf("[DEBUG] fingerprint_manager: applied fingerprints %v", appliedFingerprints) + fm.logger.Printf("[DEBUG] client.fingerprint_manager: detected fingerprints %v", appliedFingerprints) + return nil +} + +func (fp *FingerprintManager) Run() error { + // first, set up all fingerprints + cfg := fp.getConfig() + whitelistFingerprints := cfg.ReadStringListToMap("fingerprint.whitelist") + whitelistFingerprintsEnabled := len(whitelistFingerprints) > 0 + blacklistFingerprints := cfg.ReadStringListToMap("fingerprint.blacklist") + + fp.logger.Printf("[DEBUG] client.fingerprint_manager: built-in fingerprints: %v", fingerprint.BuiltinFingerprints()) + + var availableFingerprints []string + var skippedFingerprints []string + for _, name := range fingerprint.BuiltinFingerprints() { + // Skip modules that are not in the whitelist if it is enabled. + if _, ok := whitelistFingerprints[name]; whitelistFingerprintsEnabled && !ok { + skippedFingerprints = append(skippedFingerprints, name) + continue + } + // Skip modules that are in the blacklist + if _, ok := blacklistFingerprints[name]; ok { + skippedFingerprints = append(skippedFingerprints, name) + continue + } + + availableFingerprints = append(availableFingerprints, name) + } + + if err := fp.setupFingerprinters(availableFingerprints); err != nil { + return err + } + + if len(skippedFingerprints) != 0 { + fp.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprint modules skipped due to white/blacklist: %v", skippedFingerprints) + } + + // next, set up drivers + // Build the white/blacklists of drivers. + whitelistDrivers := cfg.ReadStringListToMap("driver.whitelist") + whitelistDriversEnabled := len(whitelistDrivers) > 0 + blacklistDrivers := cfg.ReadStringListToMap("driver.blacklist") + + var availDrivers []string + var skippedDrivers []string + + for name := range driver.BuiltinDrivers { + // Skip fingerprinting drivers that are not in the whitelist if it is + // enabled. + if _, ok := whitelistDrivers[name]; whitelistDriversEnabled && !ok { + skippedDrivers = append(skippedDrivers, name) + continue + } + // Skip fingerprinting drivers that are in the blacklist + if _, ok := blacklistDrivers[name]; ok { + skippedDrivers = append(skippedDrivers, name) + continue + } + + availDrivers = append(availDrivers, name) + } + + if err := fp.setupDrivers(availDrivers); err != nil { + return err + } + + if len(skippedDrivers) > 0 { + fp.logger.Printf("[DEBUG] client.fingerprint_manager: drivers skipped due to white/blacklist: %v", skippedDrivers) + } return nil } diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index fa5894fd483..b8091080816 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -12,8 +12,7 @@ import ( "github.com/stretchr/testify/require" ) -// test that the driver manager updates a node when its attributes change -func TestFingerprintManager_Fingerprint_MockDriver(t *testing.T) { +func TestFingerprintManager_Run_MockDriver(t *testing.T) { if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) } @@ -23,69 +22,64 @@ func TestFingerprintManager_Fingerprint_MockDriver(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string, 0), } - mockConfig := &config.Config{ - Node: node, - } - var resp *cstructs.FingerprintResponse - updateNode := func(r *cstructs.FingerprintResponse) { - resp = r + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node } - + conf := config.DefaultConfig() getConfig := func() *config.Config { - return mockConfig + return conf } - fm := FingerprintManager{ - getConfig: getConfig, - node: node, - updateNode: updateNode, - logger: testLogger(), - } + fm := NewFingerprintManager( + getConfig, + node, + make(chan struct{}), + updateNode, + testLogger(), + ) - // test setting up a mock driver - drivers := []string{"mock_driver"} - err := fm.SetupDrivers(drivers) + err := fm.Run() require.Nil(err) - - require.NotEqual("", resp.Attributes["driver.mock_driver"]) + require.NotEqual("", node.Attributes["driver.mock_driver"]) } -func TestFingerprintManager_Fingerprint_RawExec(t *testing.T) { +func TestFingerprintManager_Fingerprint_Run(t *testing.T) { t.Parallel() require := require.New(t) node := &structs.Node{ Attributes: make(map[string]string, 0), } - mockConfig := &config.Config{ - Node: node, - Options: map[string]string{ - "driver.raw_exec.enable": "true", - }, - } - var resp *cstructs.FingerprintResponse - updateNode := func(r *cstructs.FingerprintResponse) { - resp = r - } + conf := config.DefaultConfig() + conf.Options = map[string]string{"driver.raw_exec.enable": "true"} getConfig := func() *config.Config { - return mockConfig + return conf } - fm := FingerprintManager{ - getConfig: getConfig, - node: node, - updateNode: updateNode, - logger: testLogger(), + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node } - // test setting up a mock driver - drivers := []string{"raw_exec"} - err := fm.SetupDrivers(drivers) + fm := NewFingerprintManager( + getConfig, + node, + make(chan struct{}), + updateNode, + testLogger(), + ) + + err := fm.Run() require.Nil(err) - require.NotEqual("", resp.Attributes["driver.raw_exec"]) + require.NotEqual("", node.Attributes["driver.raw_exec"]) } func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { @@ -95,20 +89,19 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string, 0), } - var resp *cstructs.FingerprintResponse - updateNode := func(r *cstructs.FingerprintResponse) { - resp = r + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node } - mockConfig := &config.Config{ - Node: node, - Options: map[string]string{ - "test.shutdown_periodic_after": "true", - "test.shutdown_periodic_duration": "3", - }, + conf := config.DefaultConfig() + conf.Options = map[string]string{ + "test.shutdown_periodic_after": "true", + "test.shutdown_periodic_duration": "3", } - getConfig := func() *config.Config { - return mockConfig + return conf } shutdownCh := make(chan struct{}) @@ -116,22 +109,20 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { close(shutdownCh) })() - fm := FingerprintManager{ - getConfig: getConfig, - node: node, - updateNode: updateNode, - shutdownCh: shutdownCh, - logger: testLogger(), - } + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) - // test setting up a mock driver - drivers := []string{"mock_driver"} - err := fm.SetupDrivers(drivers) + err := fm.Run() require.Nil(err) // Ensure the mock driver is registered on the client testutil.WaitForResult(func() (bool, error) { - mockDriverStatus := resp.Attributes["driver.mock_driver"] + mockDriverStatus := node.Attributes["driver.mock_driver"] if mockDriverStatus == "" { return false, fmt.Errorf("mock driver attribute should be set on the client") } @@ -142,7 +133,7 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { // Ensure that the client fingerprinter eventually removes this attribute testutil.WaitForResult(func() (bool, error) { - mockDriverStatus := resp.Attributes["driver.mock_driver"] + mockDriverStatus := node.Attributes["driver.mock_driver"] if mockDriverStatus != "" { return false, fmt.Errorf("mock driver attribute should not be set on the client") } @@ -151,3 +142,288 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { t.Fatalf("err: %v", err) }) } + +func TestFimgerprintManager_Run_InWhitelist(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{"fingerprint.whitelist": " arch,cpu,memory,network,storage,foo,bar "} + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + require.NotEqual(node.Attributes["cpu.frequency"], "") +} + +func TestFimgerprintManager_Run_InBlacklist(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{"fingerprint.whitelist": " arch,memory,foo,bar "} + conf.Options = map[string]string{"fingerprint.blacklist": " cpu "} + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + require.Equal(node.Attributes["cpu.frequency"], "") + require.NotEqual(node.Attributes["memory.totalbytes"], "") +} + +func TestFimgerprintManager_Run_Combination(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{"fingerprint.whitelist": " arch,cpu,memory,foo,bar "} + conf.Options = map[string]string{"fingerprint.blacklist": " memory,nomad "} + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + require.NotEqual(node.Attributes["cpu.frequency"], "") + require.NotEqual(node.Attributes["cpu.arch"], "") + require.Equal(node.Attributes["memory.totalbytes"], "") + require.Equal(node.Attributes["nomad.version"], "") +} + +func TestFimgerprintManager_Run_WhitelistDrivers(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{ + "driver.raw_exec.enable": "1", + "driver.whitelist": " raw_exec , foo ", + } + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + require.NotEqual(node.Attributes["driver.raw_exec"], "") +} + +func TestFimgerprintManager_Run_AllDriversBlacklisted(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{ + "driver.whitelist": " foo,bar,baz ", + } + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + require.Equal(node.Attributes["driver.raw_exec"], "") + require.Equal(node.Attributes["driver.exec"], "") + require.Equal(node.Attributes["driver.docker"], "") +} + +func TestFimgerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{ + "driver.raw_exec.enable": "1", + "driver.whitelist": " raw_exec,exec,foo,bar,baz ", + "driver.blacklist": " exec,foo,bar,baz ", + } + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + 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"], "") +} + +func TestFimgerprintManager_Run_DriversInBlacklist(t *testing.T) { + t.Parallel() + require := require.New(t) + + node := &structs.Node{ + Attributes: make(map[string]string, 0), + } + updateNode := func(r *cstructs.FingerprintResponse) *structs.Node { + for k, v := range r.Attributes { + node.Attributes[k] = v + } + return node + } + conf := config.DefaultConfig() + conf.Options = map[string]string{ + "driver.raw_exec.enable": "1", + "driver.whitelist": " raw_exec,foo,bar,baz ", + "driver.blacklist": " exec,foo,bar,baz ", + } + getConfig := func() *config.Config { + return conf + } + + shutdownCh := make(chan struct{}) + defer (func() { + close(shutdownCh) + })() + + fm := NewFingerprintManager( + getConfig, + node, + shutdownCh, + updateNode, + testLogger(), + ) + + err := fm.Run() + require.Nil(err) + require.NotEqual(node.Attributes["driver.raw_exec"], "") + require.Equal(node.Attributes["driver.exec"], "") +} From 12ee25d852c4c55935b041b805631775237b94d5 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 12 Feb 2018 18:15:15 -0500 Subject: [PATCH 4/4] extract test helper lock concurrent accesses to node comment exported method --- client/client_test.go | 9 +++------ client/driver/testing.go | 13 +++++++++++++ client/fingerprint_manager.go | 8 ++++++++ client/fingerprint_manager_test.go | 4 +--- 4 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 client/driver/testing.go diff --git a/client/client_test.go b/client/client_test.go index f653b3eb903..da6a14fb7b4 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -210,9 +210,8 @@ func TestClient_RPC_Passthrough(t *testing.T) { func TestClient_Fingerprint(t *testing.T) { t.Parallel() require := require.New(t) - if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { - t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) - } + + driver.CheckForMockDriver(t) c := testClient(t, nil) defer c.Shutdown() @@ -257,9 +256,7 @@ func TestClient_HasNodeChanged(t *testing.T) { } func TestClient_Fingerprint_Periodic(t *testing.T) { - if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { - t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) - } + driver.CheckForMockDriver(t) t.Parallel() // these constants are only defined when nomad_test is enabled, so these fail diff --git a/client/driver/testing.go b/client/driver/testing.go new file mode 100644 index 00000000000..11ea3957b06 --- /dev/null +++ b/client/driver/testing.go @@ -0,0 +1,13 @@ +package driver + +import ( + "testing" +) + +// CheckForMockDriver is a test helper that ensures the mock driver is enabled. +// If not, it skips the current test. +func CheckForMockDriver(t *testing.T) { + if _, ok := BuiltinDrivers["mock_driver"]; !ok { + t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) + } +} diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index 5e22396ca90..a191555aaad 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -2,6 +2,7 @@ package client import ( "log" + "sync" "time" "github.com/hashicorp/nomad/client/config" @@ -16,6 +17,7 @@ import ( type FingerprintManager struct { getConfig func() *config.Config node *structs.Node + nodeLock sync.Mutex shutdownCh chan struct{} // updateNode is a callback to the client to update the state of its @@ -102,9 +104,11 @@ func (fm *FingerprintManager) fingerprint(name string, f fingerprint.Fingerprint return false, err } + fm.nodeLock.Lock() if node := fm.updateNode(&response); node != nil { fm.node = node } + fm.nodeLock.Unlock() return response.Detected, nil } @@ -142,6 +146,10 @@ func (fm *FingerprintManager) setupFingerprinters(fingerprints []string) error { return nil } +// Run starts the process of fingerprinting the node. It does an initial pass, +// identifying whitelisted and blacklisted fingerprints/drivers. Then, for +// those which require periotic checking, it starts a periodic process for +// each. func (fp *FingerprintManager) Run() error { // first, set up all fingerprints cfg := fp.getConfig() diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index b8091080816..fe37966e3e3 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -13,9 +13,7 @@ import ( ) func TestFingerprintManager_Run_MockDriver(t *testing.T) { - if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { - t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) - } + driver.CheckForMockDriver(t) t.Parallel() require := require.New(t)