Stop Vault token renew on task exit

[dadgar]

This PR fixes an oversight in which the client would attempt to renew a
token even after the task exits.

Fixes #2475

[dadgar]

For reviewers sake, there are so many things called heap. The c.heap.Remove method has this definition:

func (h *vaultClientHeap) Remove(id string) error {
	if entry, ok := h.heapMap[id]; ok {
		heap.Remove(&h.heap, entry.index)
		delete(h.heapMap, id)
		return nil
	}

	return fmt.Errorf("heap doesn't contain entry for %v", id)
}

Thus the delete(c.heap.heapMap, req.id) is not necessary

[stevenscg]

@dadgar I'm running 0.5.6-rc1 in the same cluster and config as yesterday, but still seeing the worker node trying to call renew-self like before.

I thought there might have been something lingering so I removed the dispatch job, GCd, restarted the nomad agent, added the job back, and dispatched 1 instance of it. Same issue.

It seems like the worker doesn't get notified that the server revoked the token. Or if it does get notified, it isn't registering that fact locally.

Timing is about the same as previous test runs given my shortened vault TTLs:

Mar 29 15:39:17 - http request to dispatch

Mar 29 15:39:17 - client: starting task context for 'scan' (alloc 'b7544935-685a-1b3e-3888-a15a84e54550')

Mar 29 15:42:43 ip-10-101-27-196 nomad[3082]: client: task "scan" for alloc "b7544935-685a-1b3e-3888-a15a84e54550" completed successfully

2017-03-29T15:42:43Z (vault log entry. good revoke via accessor by the nomad leader server)

Mar 29 15:54:47 ip-10-101-27-196 nomad: 2017/03/29 15:54:47.350455 [ERR] client.vault: renewal of token failed: failed to renew the vault token: Error making API request.

[dadgar]

@stevenscg You on linux-64bit? Would you be amenable to me giving you a build with lots of debug logs?

[stevenscg]

@dadgar Yes. linux-64bit.

[stevenscg]

@dadgar And yes, I should be able to test a custom build. Might be late tonight/early tomorrow though.

[dadgar]

@stevenscg I am hitting it now as well which is odd because I ran the same tests for the original PR. Hopefully I can just get to the bottom of this and get you a test build.

[dadgar]

@stevenscg Funny why it didn't work. When I built the PR I had this as the defer:

defer func() {
  token := getToken()
  StopRenewal(token)
  log.Println("Stopping renewal of token", token)
}()

And when I was ready to submit the PR changed to:

defer StopRenewal(getToken())

Which changed when the token's value was captured and broke it! Super subtle. This #2503 fixes it and adds tests. So we should be good to go!

[dadgar]

@stevenscg Let me know how it goes:
nomad.zip

[stevenscg]

@dadgar I've been running that build for hours without an issue. I am going to add some more tasks, but this looks fixed to me.

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.