From 06a306e460042e30d6d001d545a7a6d44367efaf Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Mon, 19 Mar 2018 08:06:09 -0400 Subject: [PATCH] improve comments; update watchDriver --- client/client.go | 8 +-- client/driver/docker.go | 2 +- client/fingerprint_manager.go | 116 ++++++++++++++++++++-------------- scheduler/feasible.go | 2 +- 4 files changed, 73 insertions(+), 55 deletions(-) diff --git a/client/client.go b/client/client.go index b2189ed62..00bed1fd6 100644 --- a/client/client.go +++ b/client/client.go @@ -1024,11 +1024,11 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs. if fingerprint != nil { if c.config.Node.Drivers[name] == nil { - // if the driver info has not yet been set, do that here + // If the driver info has not yet been set, do that here hasChanged = true c.config.Node.Drivers[name] = fingerprint } else { - // the driver info has already been set, fix it up + // The driver info has already been set, fix it up if c.config.Node.Drivers[name].Detected != fingerprint.Detected { hasChanged = true c.config.Node.Drivers[name].Detected = fingerprint.Detected @@ -1057,7 +1057,7 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs. } else { oldVal := c.config.Node.Drivers[name] if health.HealthCheckEquals(oldVal) { - // make sure we accurately reflect the last time a health check has been + // Make sure we accurately reflect the last time a health check has been // performed for the driver. oldVal.UpdateTime = health.UpdateTime } else { @@ -1067,7 +1067,7 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs. events := []*structs.NodeEvent{ { Subsystem: "Driver", - Message: fmt.Sprintf("Driver status changed: %s", health.HealthDescription), + Message: health.HealthDescription, Timestamp: time.Now().Unix(), }, } diff --git a/client/driver/docker.go b/client/driver/docker.go index c2fc516ce..c70c00df6 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -569,7 +569,7 @@ func (d *DockerDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstru _, err = client.ListContainers(docker.ListContainersOptions{All: false}) if err != nil { - d.logger.Printf("[WARN] driver.docker: failed to list Docker containers in the process of a docker health check: %v", err) + d.logger.Printf("[WARN] driver.docker: failed to list Docker containers in the process of a Docker health check: %v", err) resp.AddDriverInfo("docker", unhealthy) return err } diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go index d3fccb265..2ae904742 100644 --- a/client/fingerprint_manager.go +++ b/client/fingerprint_manager.go @@ -25,6 +25,8 @@ type FingerprintManager struct { // associated node updateNodeAttributes func(*cstructs.FingerprintResponse) *structs.Node + // updateNodeFromDriver is a callback to the client to update the state of a + // specific driver for the node updateNodeFromDriver func(string, *structs.DriverInfo, *structs.DriverInfo) *structs.Node logger *log.Logger } @@ -85,29 +87,26 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error { detected, err := fm.fingerprintDriver(name, d) if err != nil { - fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting for %v failed: %+v", name, err) + fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting driver %v failed: %+v", name, err) return err } - if hc, ok := d.(fingerprint.HealthCheck); ok { - fm.runHealthCheck(name, hc) - } else { - // for drivers which are not of type health check, update them to have their - // health status match the status of whether they are detected or not. - healthInfo := &structs.DriverInfo{ - Healthy: detected, - UpdateTime: time.Now(), - } - if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil { - fm.nodeLock.Lock() - fm.node = node - fm.nodeLock.Unlock() - } + // For all drivers upon initialization, create a driver info which matches + // its fingerprint status. Later, for drivers that have the health check + // interface implemented, a periodic health check will be run + healthInfo := &structs.DriverInfo{ + Healthy: detected, + UpdateTime: time.Now(), + } + if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil { + fm.nodeLock.Lock() + fm.node = node + fm.nodeLock.Unlock() } go fm.watchDriver(d, name) - // log the fingerprinters which have been applied + // Log the fingerprinters which have been applied if detected { availDrivers = append(availDrivers, name) } @@ -120,45 +119,61 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error { // watchDrivers facilitates the different periods between fingerprint and // health checking a driver func (fm *FingerprintManager) watchDriver(d driver.Driver, name string) { - _, fingerprintPeriod := d.Periodic() - fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting driver %s every %v", name, fingerprintPeriod) - - var healthCheckPeriod time.Duration - hc, isHealthCheck := d.(fingerprint.HealthCheck) - if isHealthCheck { - req := &cstructs.HealthCheckIntervalRequest{} - var resp cstructs.HealthCheckIntervalResponse - hc.GetHealthCheckInterval(req, &resp) - if resp.Eligible { - fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking driver %s every %v", name, healthCheckPeriod) - healthCheckPeriod = resp.Period - } + isPeriodic, fingerprintPeriod := d.Periodic() + if !isPeriodic { + return } - t1 := time.NewTimer(fingerprintPeriod) - defer t1.Stop() - t2 := time.NewTimer(healthCheckPeriod) - defer t2.Stop() + fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting driver %s every %v", name, fingerprintPeriod) - for { - select { - case <-fm.shutdownCh: - return - case <-t1.C: - t1.Reset(fingerprintPeriod) - _, err := fm.fingerprintDriver(name, d) - if err != nil { - fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for driver %v failed: %+v", name, err) - } - case <-t2.C: - if isHealthCheck { - t2.Reset(healthCheckPeriod) + fingerprintTimer := time.NewTicker(fingerprintPeriod) + defer fingerprintTimer.Stop() + + hc, isHealthCheck := d.(fingerprint.HealthCheck) + if isHealthCheck { + // For types that implement the health check interface, run both the + // fingerprint and health check periodic functions and update the node + req := &cstructs.HealthCheckIntervalRequest{} + var resp cstructs.HealthCheckIntervalResponse + err := hc.GetHealthCheckInterval(req, &resp) + if err != nil { + fm.logger.Printf("[ERR] client.fingerprint_manager: error when getting health check interval: %v", err) + } + + fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking driver %s every %v", name, resp.Period) + healthCheckTimer := time.NewTicker(resp.Period) + defer healthCheckTimer.Stop() + + for { + select { + case <-fm.shutdownCh: + return + case <-fingerprintTimer.C: + _, err := fm.fingerprintDriver(name, d) + if err != nil { + fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for driver %v failed: %+v", name, err) + } + case <-healthCheckTimer.C: err := fm.runHealthCheck(name, hc) if err != nil { fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking for %v failed: %v", name, err) } } } + } else { + // For types that do not have a health check capacity, run only the + // periodic fingerprint method + for { + select { + case <-fm.shutdownCh: + return + case <-fingerprintTimer.C: + _, err := fm.fingerprintDriver(name, d) + if err != nil { + fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for driver %v failed: %+v", name, err) + } + } + } } } @@ -239,6 +254,9 @@ func (fm *FingerprintManager) runHealthCheck(name string, hc fingerprint.HealthC request := &cstructs.HealthCheckRequest{} var response cstructs.HealthCheckResponse err := hc.HealthCheck(request, &response) + if err != nil { + return err + } if node := fm.updateNodeFromDriver(name, nil, response.Drivers[name]); node != nil { fm.nodeLock.Lock() @@ -246,7 +264,7 @@ func (fm *FingerprintManager) runHealthCheck(name string, hc fingerprint.HealthC fm.nodeLock.Unlock() } - return err + return nil } // setupFingerprints is used to fingerprint the node to see if these attributes are @@ -287,7 +305,7 @@ func (fm *FingerprintManager) setupFingerprinters(fingerprints []string) error { // those which require periotic checking, it starts a periodic process for // each. func (fp *FingerprintManager) Run() error { - // first, set up all fingerprints + // First, set up all fingerprints cfg := fp.getConfig() whitelistFingerprints := cfg.ReadStringListToMap("fingerprint.whitelist") whitelistFingerprintsEnabled := len(whitelistFingerprints) > 0 @@ -320,7 +338,7 @@ func (fp *FingerprintManager) Run() error { fp.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprint modules skipped due to white/blacklist: %v", skippedFingerprints) } - // next, set up drivers + // Next, set up drivers // Build the white/blacklists of drivers. whitelistDrivers := cfg.ReadStringListToMap("driver.whitelist") whitelistDriversEnabled := len(whitelistDrivers) > 0 diff --git a/scheduler/feasible.go b/scheduler/feasible.go index f6d862cb6..b8bfc758b 100644 --- a/scheduler/feasible.go +++ b/scheduler/feasible.go @@ -130,7 +130,7 @@ func (c *DriverChecker) hasDrivers(option *structs.Node) bool { for driver := range c.drivers { driverStr := fmt.Sprintf("driver.%s", driver) - // COMPAT: Remove in 0.X: As of Nomad 0.8, nodes have a DriverInfo that + // COMPAT: Remove in 0.10: As of Nomad 0.8, nodes have a DriverInfo that // corresponds with every driver. As a Nomad server might be on a later // version than a Nomad client, we need to check for compatibility here // to verify the client supports this.