Skip to content

Commit

Permalink
Properly truncate TTL checks
Browse files Browse the repository at this point in the history
Check that OutputMaxSize is properly taken into account.

BUGFIX: properly update local state on update of TTL
  • Loading branch information
pierresouchay committed Jun 25, 2019
1 parent 2b3df47 commit a68ecd0
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 11 deletions.
21 changes: 14 additions & 7 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -2259,10 +2259,11 @@ func (a *Agent) addCheck(check *structs.HealthCheck, chkType *structs.CheckType,
}

ttl := &checks.CheckTTL{
Notify: a.State,
CheckID: check.CheckID,
TTL: chkType.TTL,
Logger: a.logger,
Notify: a.State,
CheckID: check.CheckID,
TTL: chkType.TTL,
Logger: a.logger,
OutputMaxSize: maxOutputSize,
}

// Restore persisted state, if any
Expand Down Expand Up @@ -2878,16 +2879,22 @@ func (a *Agent) updateTTLCheck(checkID types.CheckID, status, output string) err
}

// Set the status through CheckTTL to reset the TTL.
check.SetStatus(status, output)

outputTruncated := check.SetStatus(status, output)
stateCheck := a.State.Checks()[checkID]
if stateCheck != nil {
stateCheck.Output = outputTruncated
stateCheck.Status = status
} else {
a.logger.Printf("[WARN] agent: Unexpected missing state for check %v", checkID)
}
// We don't write any files in dev mode so bail here.
if a.config.DataDir == "" {
return nil
}

// Persist the state so the TTL check can come up in a good state after
// an agent restart, especially with long TTL values.
if err := a.persistCheckState(check, status, output); err != nil {
if err := a.persistCheckState(check, status, outputTruncated); err != nil {
return fmt.Errorf("failed persisting state for check %q: %s", checkID, err)
}

Expand Down
2 changes: 1 addition & 1 deletion agent/agent_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2360,7 +2360,7 @@ func TestAgent_UpdateCheck(t *testing.T) {
// isn't super brittle.
state := a.State.Checks()["test"]
if state.Status != api.HealthPassing || len(state.Output) > 2*checks.DefaultBufSize {
t.Fatalf("bad: %v", state)
t.Fatalf("bad: %v, (len:=%d)", state, len(state.Output))
}
})

Expand Down
18 changes: 16 additions & 2 deletions agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1405,15 +1405,16 @@ func TestAgent_updateTTLCheck(t *testing.T) {
t.Parallel()
a := NewTestAgent(t, t.Name(), "")
defer a.Shutdown()

checkBufSize := 100
health := &structs.HealthCheck{
Node: "foo",
CheckID: "mem",
Name: "memory util",
Status: api.HealthCritical,
}
chk := &structs.CheckType{
TTL: 15 * time.Second,
TTL: 15 * time.Second,
OutputMaxSize: checkBufSize,
}

// Add check and update it.
Expand All @@ -1433,6 +1434,19 @@ func TestAgent_updateTTLCheck(t *testing.T) {
if status.Output != "foo" {
t.Fatalf("bad: %v", status)
}

if err := a.updateTTLCheck("mem", api.HealthCritical, strings.Repeat("--bad-- ", 5*checkBufSize)); err != nil {
t.Fatalf("err: %v", err)
}

// Ensure we have a check mapping.
status = a.State.Checks()["mem"]
if status.Status != api.HealthCritical {
t.Fatalf("bad: %v", status)
}
if len(status.Output) > checkBufSize*2 {
t.Fatalf("bad: %v", len(status.Output))
}
}

func TestAgent_PersistService(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion agent/checks/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,8 @@ func (c *CheckTTL) getExpiredOutput() string {

// SetStatus is used to update the status of the check,
// and to renew the TTL. If expired, TTL is restarted.
func (c *CheckTTL) SetStatus(status, output string) {
// output is returned (might be truncated)
func (c *CheckTTL) SetStatus(status, output string) string {
c.Logger.Printf("[DEBUG] agent: Check %q status is now %s", c.CheckID, status)
c.Notify.UpdateCheck(c.CheckID, status, output)
total := len(output)
Expand All @@ -295,6 +296,7 @@ func (c *CheckTTL) SetStatus(status, output string) {
c.lastOutputLock.Unlock()

c.timer.Reset(c.TTL)
return output
}

// CheckHTTP is used to periodically make an HTTP request to
Expand Down

0 comments on commit a68ecd0

Please sign in to comment.