implement server agent draining

[ipochi]

Currently, the server only logs if it receives a drain request from the agent. Ideally, in that scenario, the server should mark it and not select it for newer requests. This PR implements that.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ipochi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ipochi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jkh52]

I'm happy to see interest in completing this feature! When I had been looking into this, here's the questions I ran into:

1. Whether the server should keep agent draining state by raw connection, or by agent identifiers. The current BackendStorage makes the latter much more complicated, I agree with the former (like this PR does). But note that currently, the agent sync loop will continue to issue new syncs, and a server could get into conflicting drain state for a given agent. So without an accompanying agent change (to stop syncing new connections after entering draining state), it is possible for the server to continue picking these agents.

That potential agent change is probably an improvement but needs careful risk analysis.

2. Fallback logic in case no non-draining agent is found, continue using one that is draining. I think the argument for this is strong, because it's "no worse" than the current server behavior, and may avoid degrading some clusters (those that are limping along with very few agents, e.g. 1).

[ipochi]

I'm happy to see interest in completing this feature! When I had been looking into this, here's the questions I ran into:

1. Whether the server should keep agent draining state by raw connection, or by agent identifiers. The current BackendStorage makes the latter much more complicated, I agree with the former (like this PR does). But note that currently, the agent sync loop will continue to issue new syncs, and a server could get into conflicting drain state for a given agent. So without an accompanying agent change (to stop syncing new connections after entering draining state), it is possible for the server to continue picking these agents.

That potential agent change is probably an improvement but needs careful risk analysis.

2. Fallback logic in case no non-draining agent is found, continue using one that is draining. I think the argument for this is strong, because it's "no worse" than the current server behavior, and may avoid degrading some clusters (those that are limping along with very few agents, e.g. 1).

Thank you for the feedback @jkh52

Your point on 1, hadn't crossed my mind. It seems logical thing to do to stop the agent sync loop if the agent is draining. I'll give more thought on this whether the server should do more so that it doesn't get into a conflicted state or the agent should stop teh sync loops.

On Point 2, I think its valid point that continue to use the draining agent if there are no non-draining agents. Its akin falling back on current behaviour.