agent: avoid reverting any check updates that occur while a service is being added or the config is reloaded

[rboyer]

An Agent has two confusingly named locks:

• the outer lock Agent.stateLock
• and the inner Agent.State embedded lock

Operations like Agent.addServiceInternal will grab the outer lock and then briefly acquire the inner lock to clone the current state of all of the checks registered on the agent. This is so that when a service is re-registered any checks re-added can have their statuses carried over.

Unfortunately there is a logical data race (rather than a -race race) whereby the individual check goroutines (like for an alias check) are independently sending updated statuses into the Agent.State and only acquiring the inner lock to do so.

Example situation:

1. (goroutine 1) User registers service "foo".
2. (goroutine 1) addServiceInternal locks the outer lock
3. (goroutine 1) snapshotCheckState locks the inner lock, copies the check states, and unlocks the inner lock
4. (goroutine 2) A checker determines the health of a check "bar" has changed and calls State.UpdateCheck to change from critical -> passing.
5. (goroutine 2) The inner lock is locked, the status for "bar" is flipped to passing, and the inner lock is unlocked.
6. (goroutine 1) The "foo" service finishes being added and the defer restoreCheckState call walks the captured check snapshot from (3).
7. (goroutine 1) The previous value of the "bar" check is reverted back to critical as it was in step (3).

The fix here is to only use the snapshot to seed the value of the check initially inserted/updated, rather than letting the check be inserted/updated at the default unmeasured state of critical.

[banks]

Yay! Great job tracking this down.

[fumikojandr3]

Service A register with status=critical,then re-register with status=passing.This commit makes the "re-register" status still being critical. @rboyer