client: updates from pr feedback

.changelog/13715.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+client: Add built-in support for checks on nomad services
+```

client/allochealth/tracker.go

@@ -366,7 +366,7 @@ func (t *Tracker) watchTaskEvents() {
 			// reset task health
 			t.setTaskHealth(false, false)
 
-			// Avoid the timer from firing at the old start time
+			// Prevent the timer from firing at the old start time
 			waiter.disable()
 
 			// Set the timer since all tasks are started
@@ -492,9 +492,6 @@ OUTER:
 		// Store the task registrations
 		t.lock.Lock()
 		for task, reg := range allocReg.Tasks {
-			//TODO(schmichael) for now skip unknown tasks as
-			//they're task group services which don't currently
-			//support checks anyway
 			if v, ok := t.taskHealth[task]; ok {
 				v.taskRegistrations = reg
 			}
@@ -571,7 +568,7 @@ func (t *Tracker) watchNomadEvents() {
 	for {
 		select {
 
-		// we are shutting down
+		// tracker has been canceled, so stop waiting
 		case <-t.ctx.Done():
 			return
 

client/allocrunner/checks_hook.go

@@ -55,7 +55,7 @@ func (o *observer) start() {
 			query := checks.GetCheckQuery(o.check)
 			result := o.checker.Do(o.ctx, o.qc, query)
 
-			// and put the results into the store
+			// and put the results into the store (already logged)
 			_ = o.checkStore.Set(o.allocID, result)
 
 			// setup timer for next interval

client/serviceregistration/checks/checkstore/shim.go

@@ -57,6 +57,8 @@ func (s *shim) restore() {
 	results, err := s.db.GetCheckResults()
 	if err != nil {
 		s.log.Error("failed to restore health check results", "error", err)
+		// may as well continue and let the check observers repopulate - maybe
+		// the persistent storage error was transitory
 		return
 	}
 
@@ -90,7 +92,10 @@ func (s *shim) Set(allocID string, qr *structs.CheckQueryResult) error {
 	// on Client restart restored check results may be outdated but the status
 	// is the same as the most recent result
 	if !exists || previous.Status != qr.Status {
-		return s.db.PutCheckResult(allocID, qr)
+		if err := s.db.PutCheckResult(allocID, qr); err != nil {
+			s.log.Error("failed to set check status", "alloc_id", allocID, "check_id", qr.ID, "error", err)
+			return err
+		}
 	}
 
 	return nil

nomad/structs/checks.go

@@ -15,7 +15,7 @@ const (
 	// will not move forward while the check is failing.
 	Healthiness CheckMode = "healthiness"
 
-	// A Readiness check is useful in the context of ensuring a service is
+	// A Readiness check is useful in the context of ensuring a service
 	// should be performing its duties (regardless of healthiness). This is an
 	// indicator that the check's on_update configuration is set to "ignore",
 	// implying that Deployments will move forward regardless if the check is