From a19ac99b87946862b5fc64402a23046c4c200b3f Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 26 Jan 2018 14:31:37 -0500 Subject: [PATCH] remove attributes from periodic fingerprints when state changes write test for client periodic fingerprinters --- client/client.go | 14 ++++++++++ client/client_test.go | 44 ++++++++++++++++++++++++++++++ client/driver/docker.go | 2 ++ client/driver/exec_linux.go | 2 ++ client/driver/java.go | 3 ++ client/driver/mock_driver.go | 18 ++++++++++-- client/driver/qemu.go | 4 ++- client/driver/raw_exec.go | 1 + client/driver/rkt.go | 3 ++ client/fingerprint/cgroup.go | 8 ++++++ client/fingerprint/cgroup_linux.go | 4 +++ client/fingerprint/consul.go | 13 ++++++++- client/fingerprint/vault.go | 8 ++++++ client/structs/structs.go | 22 +++++++++++++++ 14 files changed, 141 insertions(+), 5 deletions(-) diff --git a/client/client.go b/client/client.go index 9542f734f7a..95f822a9c8b 100644 --- a/client/client.go +++ b/client/client.go @@ -100,6 +100,10 @@ type Client struct { config *config.Config start time.Time + // fingerprinters is a list of a client's periodic fingerprinters. + // TODO this will later be moved to a driver manager + fingerprinters map[string]fingerprint.Fingerprint + // stateDB is used to efficiently store client state. stateDB *bolt.DB @@ -197,6 +201,7 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic servers: newServerList(), triggerDiscoveryCh: make(chan struct{}), serversDiscoveredCh: make(chan struct{}), + fingerprinters: make(map[string]fingerprint.Fingerprint), } // Initialize the client @@ -969,6 +974,7 @@ func (c *Client) fingerprint() error { // TODO: If more periodic fingerprinters are added, then // fingerprintPeriodic should be used to handle all the periodic // fingerprinters by using a priority queue. + c.fingerprinters[name] = f go c.fingerprintPeriodic(name, f, period) } } @@ -1044,6 +1050,7 @@ func (c *Client) setupDrivers() error { p, period := d.Periodic() if p { + c.fingerprinters[name] = d go c.fingerprintPeriodic(name, d, period) } @@ -1062,11 +1069,18 @@ func (c *Client) setupDrivers() error { func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) { c.configLock.Lock() defer c.configLock.Unlock() + c.logger.Printf("RESPONSE ATTRIBUTES %+v", response.GetAttributes()) for name, val := range response.GetAttributes() { if val == "" { + if name == "driver.mock_driver" { + c.logger.Printf("REMOVING MOCK ADDR ATTRIBUTE") + } delete(c.config.Node.Attributes, name) } else { c.config.Node.Attributes[name] = val + if name == "driver.mock_driver" { + c.logger.Printf("ADDING MOCK ADDR ATTRIBUTE") + } } } diff --git a/client/client_test.go b/client/client_test.go index 95ff480d357..5313fd04b34 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/consul/lib/freeport" memdb "github.com/hashicorp/go-memdb" "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/driver" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" @@ -252,6 +253,49 @@ 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"`) + } + t.Parallel() + + c1 := testClient(t, nil) + 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) { + mockDriverStatus := node.Attributes[mockDriverName] + 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) + }) + + // set IsShutdown to true on the mock driver + md := c1.fingerprinters["mock_driver"] + dd, ok := md.(*driver.MockDriver) + if !ok { + t.Fatalf("should be ok") + } + dd.IsShutdown = true + + // Ensure that the client fingerprinter removes this attribute + testutil.WaitForResult(func() (bool, error) { + mockDriverStatus := node.Attributes[mockDriverName] + 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) + }) +} + func TestClient_Fingerprint_InWhitelist(t *testing.T) { t.Parallel() c := testClient(t, func(c *config.Config) { diff --git a/client/driver/docker.go b/client/driver/docker.go index be4bfd1b5b4..7fa709d2e65 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -482,6 +482,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru d.logger.Printf("[INFO] driver.docker: failed to initialize client: %s", err) } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(dockerDriverAttr) return nil } @@ -494,6 +495,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru d.logger.Printf("[DEBUG] driver.docker: could not connect to docker daemon at %s: %s", client.Endpoint(), err) } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(dockerDriverAttr) return nil } diff --git a/client/driver/exec_linux.go b/client/driver/exec_linux.go index 258b8a1d354..e35c8168372 100644 --- a/client/driver/exec_linux.go +++ b/client/driver/exec_linux.go @@ -19,12 +19,14 @@ func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct d.logger.Printf("[DEBUG] driver.exec: cgroups unavailable, disabling") } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(execDriverAttr) return nil } else if unix.Geteuid() != 0 { if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.exec: must run as root user, disabling") } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(execDriverAttr) return nil } diff --git a/client/driver/java.go b/client/driver/java.go index bf125fd3c03..d3ee3df1725 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -118,6 +118,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct d.logger.Printf("[DEBUG] driver.java: root privileges and mounted cgroups required on linux, disabling") } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(javaDriverAttr) return nil } @@ -131,6 +132,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct if err != nil { // assume Java wasn't found d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(javaDriverAttr) return nil } @@ -150,6 +152,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct d.logger.Println("[WARN] driver.java: error parsing Java version information, aborting") } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(javaDriverAttr) return nil } diff --git a/client/driver/mock_driver.go b/client/driver/mock_driver.go index ad9ce1dedff..8964a4faa8d 100644 --- a/client/driver/mock_driver.go +++ b/client/driver/mock_driver.go @@ -16,7 +16,6 @@ import ( "github.com/mitchellh/mapstructure" dstructs "github.com/hashicorp/nomad/client/driver/structs" - "github.com/hashicorp/nomad/client/fingerprint" cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -77,7 +76,10 @@ type MockDriverConfig struct { // MockDriver is a driver which is used for testing purposes type MockDriver struct { DriverContext - fingerprint.StaticFingerprinter + + // IsShutdown is Used purely for testing purposes in order to validate + // periodic fingerprint changes are picked up by the client and set on the node + IsShutdown bool cleanupFailNum int } @@ -194,7 +196,12 @@ func (m *MockDriver) Validate(map[string]interface{}) error { // Fingerprint fingerprints a node and returns if MockDriver is enabled func (m *MockDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { - resp.AddAttribute("driver.mock_driver", "1") + switch { + case m.IsShutdown: + resp.RemoveAttribute("driver.mock_driver") + default: + resp.AddAttribute("driver.mock_driver", "1") + } return nil } @@ -338,3 +345,8 @@ func (h *mockDriverHandle) run() { } } } + +// When testing, poll for updates +func (m *MockDriver) Periodic() (bool, time.Duration) { + return true, 1 * time.Second +} diff --git a/client/driver/qemu.go b/client/driver/qemu.go index a4d740ef9e9..98b728172b5 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -163,12 +163,14 @@ func (d *QemuDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct } outBytes, err := exec.Command(bin, "--version").Output() if err != nil { - return nil + resp.RemoveAttribute(qemuDriverAttr) + return err } out := strings.TrimSpace(string(outBytes)) matches := reQemuVersion.FindStringSubmatch(out) if len(matches) != 2 { + resp.RemoveAttribute(qemuDriverAttr) return fmt.Errorf("Unable to parse Qemu version string: %#v", matches) } currentQemuVersion := matches[1] diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index 9d7ba400a60..bb11387e163 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -101,6 +101,7 @@ func (d *RawExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstr return nil } + resp.RemoveAttribute(rawExecDriverAttr) return nil } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 789a8f6aa26..1ace66bd7d0 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -318,6 +318,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs d.logger.Printf("[DEBUG] driver.rkt: must run as root user, disabling") } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(rktDriverAttr) return nil } @@ -332,6 +333,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs appcMatches := reAppcVersion.FindStringSubmatch(out) if len(rktMatches) != 2 || len(appcMatches) != 2 { d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(rktDriverAttr) return fmt.Errorf("Unable to parse Rkt version string: %#v", rktMatches) } @@ -345,6 +347,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs currentVersion, minVersion) } d.fingerprintSuccess = helper.BoolToPtr(false) + resp.RemoveAttribute(rktDriverAttr) return nil } diff --git a/client/fingerprint/cgroup.go b/client/fingerprint/cgroup.go index 5ee5b26ccc3..2e6c446379a 100644 --- a/client/fingerprint/cgroup.go +++ b/client/fingerprint/cgroup.go @@ -5,6 +5,8 @@ package fingerprint import ( "log" "time" + + cstructs "github.com/hashicorp/nomad/client/structs" ) const ( @@ -45,6 +47,12 @@ func NewCGroupFingerprint(logger *log.Logger) Fingerprint { return f } +// clearCGroupAttributes clears any node attributes related to cgroups that might +// have been set in a previous fingerprint run. +func (f *CGroupFingerprint) clearCGroupAttributes(r *cstructs.FingerprintResponse) { + r.RemoveAttribute("unique.cgroup.mountpoint") +} + // Periodic determines the interval at which the periodic fingerprinter will run. func (f *CGroupFingerprint) Periodic() (bool, time.Duration) { return true, interval * time.Second diff --git a/client/fingerprint/cgroup_linux.go b/client/fingerprint/cgroup_linux.go index 47beecc49c2..a13e17d3b51 100644 --- a/client/fingerprint/cgroup_linux.go +++ b/client/fingerprint/cgroup_linux.go @@ -30,11 +30,15 @@ func FindCgroupMountpointDir() (string, error) { func (f *CGroupFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { mount, err := f.mountPointDetector.MountPoint() if err != nil { + f.clearCGroupAttributes(resp) return fmt.Errorf("Failed to discover cgroup mount point: %s", err) } // Check if a cgroup mount point was found if mount == "" { + + f.clearCGroupAttributes(resp) + if f.lastState == cgroupAvailable { f.logger.Printf("[INFO] fingerprint.cgroups: cgroups are unavailable") } diff --git a/client/fingerprint/consul.go b/client/fingerprint/consul.go index 79ed813e526..e640c3d6f3b 100644 --- a/client/fingerprint/consul.go +++ b/client/fingerprint/consul.go @@ -47,7 +47,7 @@ func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp * // If we can't hit this URL consul is probably not running on this machine. info, err := f.client.Agent().Self() if err != nil { - // TODO this should set consul in the response if the error is not nil + f.clearConsulAttributes(resp) // Print a message indicating that the Consul Agent is not available // anymore @@ -105,3 +105,14 @@ func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp * func (f *ConsulFingerprint) Periodic() (bool, time.Duration) { return true, 15 * time.Second } + +// clearConsulAttributes removes consul attributes and links from the passed +// Node. +func (f *ConsulFingerprint) clearConsulAttributes(r *cstructs.FingerprintResponse) { + r.RemoveAttribute("consul.server") + r.RemoveAttribute("consul.version") + r.RemoveAttribute("consul.revision") + r.RemoveAttribute("unique.consul.name") + r.RemoveAttribute("consul.datacenter") + r.RemoveLink("consul") +} diff --git a/client/fingerprint/vault.go b/client/fingerprint/vault.go index 612302365cc..04d8e1eba97 100644 --- a/client/fingerprint/vault.go +++ b/client/fingerprint/vault.go @@ -52,6 +52,7 @@ func (f *VaultFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *c // Connect to vault and parse its information status, err := f.client.Sys().SealStatus() if err != nil { + f.clearVaultAttributes(resp) // Print a message indicating that Vault is not available anymore if f.lastState == vaultAvailable { f.logger.Printf("[INFO] fingerprint.vault: Vault is unavailable") @@ -79,3 +80,10 @@ func (f *VaultFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *c func (f *VaultFingerprint) Periodic() (bool, time.Duration) { return true, 15 * time.Second } + +func (f *VaultFingerprint) clearVaultAttributes(r *cstructs.FingerprintResponse) { + r.RemoveAttribute("vault.accessible") + r.RemoveAttribute("vault.version") + r.RemoveAttribute("vault.cluster_id") + r.RemoveAttribute("vault.cluster_name") +} diff --git a/client/structs/structs.go b/client/structs/structs.go index 7c9ea147987..9d21b436260 100644 --- a/client/structs/structs.go +++ b/client/structs/structs.go @@ -210,6 +210,17 @@ func (f *FingerprintResponse) AddAttribute(name, value string) { f.attributes[name] = value } +// RemoveAttribute sets the given attribute to empty, which will later remove +// it entirely from the node +func (f *FingerprintResponse) RemoveAttribute(name string) { + // initialize attributes if it has not been already + if f.attributes == nil { + f.attributes = make(map[string]string, 0) + } + + f.attributes[name] = "" +} + // GetAttributes fetches the attributes for the fingerprint response func (f *FingerprintResponse) GetAttributes() map[string]string { // initialize attributes if it has not been already @@ -230,6 +241,17 @@ func (f *FingerprintResponse) AddLink(name, value string) { f.links[name] = value } +// RemoveLink removes a link entry from the fingerprint response. This will +// later remove it entirely from the node +func (f *FingerprintResponse) RemoveLink(name string) { + // initialize links if it has not been already + if f.links == nil { + f.links = make(map[string]string, 0) + } + + f.links[name] = "" +} + // GetLinks returns the links for the fingerprint response func (f *FingerprintResponse) GetLinks() map[string]string { // initialize links if it has not been already