-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor client fingerprinters to return a diff of node attributes #3781
Changes from all commits
5e8151d
a76a404
c21ac46
f5fc20a
ae889b4
e8aaa93
a9447ad
ba2ebbc
e5ccc55
15cb768
3202200
8049aa0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import ( | |
"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" | ||
"github.com/hashicorp/nomad/command/agent/consul" | ||
"github.com/hashicorp/nomad/helper" | ||
|
@@ -931,33 +932,42 @@ func (c *Client) fingerprint() error { | |
|
||
c.logger.Printf("[DEBUG] client: built-in fingerprints: %v", fingerprint.BuiltinFingerprints()) | ||
|
||
var applied []string | ||
var skipped []string | ||
var detectedFingerprints []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 { | ||
skipped = append(skipped, name) | ||
skippedFingerprints = append(skippedFingerprints, name) | ||
continue | ||
} | ||
// Skip modules that are in the blacklist | ||
if _, ok := blacklist[name]; ok { | ||
skipped = append(skipped, name) | ||
skippedFingerprints = append(skippedFingerprints, name) | ||
continue | ||
} | ||
|
||
f, err := fingerprint.NewFingerprint(name, c.logger) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
c.configLock.Lock() | ||
applies, err := f.Fingerprint(c.config, c.config.Node) | ||
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 | ||
} | ||
if applies { | ||
applied = append(applied, name) | ||
|
||
// log the fingerprinters which have been applied | ||
if response.Detected { | ||
detectedFingerprints = append(detectedFingerprints, name) | ||
} | ||
|
||
// add the diff found from each fingerprinter | ||
c.updateNodeFromFingerprint(&response) | ||
|
||
p, period := f.Periodic() | ||
if p { | ||
// TODO: If more periodic fingerprinters are added, then | ||
|
@@ -966,9 +976,10 @@ func (c *Client) fingerprint() error { | |
go c.fingerprintPeriodic(name, f, period) | ||
} | ||
} | ||
c.logger.Printf("[DEBUG] client: applied fingerprints %v", applied) | ||
if len(skipped) != 0 { | ||
c.logger.Printf("[DEBUG] client: fingerprint modules skipped due to white/blacklist: %v", skipped) | ||
|
||
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 | ||
} | ||
|
@@ -980,10 +991,17 @@ func (c *Client) fingerprintPeriodic(name string, f fingerprint.Fingerprint, d t | |
select { | ||
case <-time.After(d): | ||
c.configLock.Lock() | ||
if _, err := f.Fingerprint(c.config, c.config.Node); err != nil { | ||
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) | ||
} | ||
c.configLock.Unlock() | ||
|
||
case <-c.shutdownCh: | ||
return | ||
} | ||
|
@@ -997,52 +1015,86 @@ func (c *Client) setupDrivers() error { | |
whitelistEnabled := len(whitelist) > 0 | ||
blacklist := c.config.ReadStringListToMap("driver.blacklist") | ||
|
||
var avail []string | ||
var skipped []string | ||
var detectedDrivers []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. | ||
if _, ok := whitelist[name]; whitelistEnabled && !ok { | ||
skipped = append(skipped, name) | ||
skippedDrivers = append(skippedDrivers, name) | ||
continue | ||
} | ||
// Skip fingerprinting drivers that are in the blacklist | ||
if _, ok := blacklist[name]; ok { | ||
skipped = append(skipped, name) | ||
skippedDrivers = append(skippedDrivers, name) | ||
continue | ||
} | ||
|
||
d, err := driver.NewDriver(name, driverCtx) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
c.configLock.Lock() | ||
applies, err := d.Fingerprint(c.config, c.config.Node) | ||
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 | ||
} | ||
if applies { | ||
avail = append(avail, name) | ||
|
||
// 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) | ||
} | ||
|
||
} | ||
|
||
c.logger.Printf("[DEBUG] client: available drivers %v", avail) | ||
|
||
if len(skipped) != 0 { | ||
c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skipped) | ||
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) | ||
} | ||
|
||
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) { | ||
c.configLock.Lock() | ||
defer c.configLock.Unlock() | ||
for name, val := range response.Attributes { | ||
if val == "" { | ||
delete(c.config.Node.Attributes, name) | ||
} else { | ||
c.config.Node.Attributes[name] = val | ||
} | ||
} | ||
|
||
// update node links and resources from the diff created from | ||
// fingerprinting | ||
for name, val := range response.Links { | ||
if val == "" { | ||
delete(c.config.Node.Links, name) | ||
} else { | ||
c.config.Node.Links[name] = val | ||
} | ||
} | ||
|
||
if response.Resources != nil { | ||
c.config.Node.Resources.Merge(response.Resources) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should change the node watcher from checking periodically to check via a channel trigger and send it when this function is called: https://github.com/hashicorp/nomad/blob/master/client/client.go#L1588 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be in a follow up PR. |
||
|
||
// retryIntv calculates a retry interval value given the base | ||
func (c *Client) retryIntv(base time.Duration) time.Duration { | ||
if c.config.DevMode { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are you handling removing Attributes/Links?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in fingerprinter_foo adds attribute = {"foo": "bar", "bam": "baz"} and then the next time it is called only returns {"foo": "bar"} because "bam" should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. One way we can handle this is if the attribute is equal to empty in the fingerprinter's response, we can delete this attribute from the node's attributes. That way we don't need to rely on the fingerprinter modifying the node directly.