[JUJU-672] Retry more classes of Raft leader errors

[manadart]

For API-based leases, the behaviour has been to pre-emptively check the Raft state prior to applying each operation, and if we are not running on the leader, return a NotLeaderError, which is interpreted downstream as being retry-able.

However it is possible after checking state and verifying leadership, that we can get an error such as raft.ErrLeadershipLost while applying the log, which flows up and is not recognised as retry-able, causing the lease manager to bounce.

Here we remove the pre-emptive check along with its local dedicated metric and time tracking. Instead we check the Raft state after getting leadership-based errors, with the following benefits:

• The increased efficiency of not checking Raft state for every log application.
• We represent more of the leadership error types as NotLeaderError, meaning retry instead of lease manager crash and restart.

QA steps

• Bootstrap with --config features="[raft-api-leases]".
• juju model-config -m controller logging-config="<root>=INFO;juju.worker.lease.raft=TRACE;juju.core.raftlease=TRACE;juju.worker.globalclockupdater.raft=TRACE;juju.worker.raft=TRACE;juju.worker.raft.raftforwarder=TRACE;juju.worker.raft.rafttransport=TRACE".
• Watch debug-log -m controller.
• juju enable-ha.
• juju deploy ubuntu -n 3.
• Normal operation will see logging with sections like this:

machine-0: 10:32:46 TRACE juju.worker.globalclockupdater.raft incremented lease clock by 1s
machine-0: 10:32:47 TRACE juju.worker.globalclockupdater.raft incremented lease clock by 1s
machine-0: 10:32:48 TRACE juju.worker.globalclockupdater.raft incremented lease clock by 1s
machine-0: 10:32:49 TRACE juju.worker.globalclockupdater.raft incremented lease clock by 1s
machine-0: 10:32:50 TRACE juju.worker.lease.raft [df51a1] machine-0 extending lease df51a145-6b04-434e-8abb-663951fcc757 for 1m0s
machine-0: 10:32:50 TRACE juju.worker.raft Dequeued 1 commands to be applied on the raft log
machine-0: 10:32:50 TRACE juju.worker.raft Applying command 0 version: 1
operation: extend
namespace: singular-controller
model-uuid: 847b5f2d-0dc6-440a-837e-86d7b0bc8ce8
lease: df51a145-6b04-434e-8abb-663951fcc757
holder: machine-0
duration: 1m0s

• Note that the lease clock updater runs on the Raft leader, in the case above, machine-0.
• Restart the leader node: juju ssh -m controller 0 -- sudo reboot now. You will probably have to reconnect your debug-log session.
• Observe a period of churn where you may see lines like this:

machine-2: 10:33:49 ERROR juju.worker.raft Raft future error: node is not the leader
machine-2: 10:33:49 INFO juju.worker.raft Attempt to apply the lease failed, we're not the leader. State: Follower, Leader: 10.161.87.110:17070

• Control plane should quiesce quite quickly, and normal lease operation will resume.
• (BONUS) Step down the Mongo primary and observe relatively quick resumption of quiesced state.

Documentation changes

None.

Bug reference

N/A

[SimonRichardson]

I'm not sure I agree with the removal of this for two reasons:

1. By not measuring failures, you don't show the real perception of the state of the world. Only showing good things leads to bad outcomes.
2. By not measuring the actual timing of failures we don't know if we've actually improved anything here.

[manadart]

We still log failures as failures, with timing. This was only specific to the pre-emptive check, which is gone.

The failures can still be correlated with log errors, which will indicate cause.

[manadart]

$$merge$$