[UPDATED] Add kick leafnode client functionality

[n-holmstedt]

I was looking for a way to rebalance LeafNodes client connections (k8s loadbalancing) and found the original #1556. I was informed at the slack that the KICK functionality in the API lacked the ability to kick leaf node connections.

• Changed the KickClientReq struct to include client kind as per suggestion from slack.
• Defaulted kind to "Client" to keep compability.
• Added kind as an argument to server DisconnectClientByID
• Fetched client based on kind before closing connection.

Tested functionality locally & ran tests without issues. Didn't get travis to work tho.

Signed-off-by: Niklas Holmstedt n.holmstedt@gmail.com

[derekcollison]

I wonder if we should just keep the external API the same and if we fail looking up via the client list just check the leafnode list?

[derekcollison]

I also think it should work on leafnodes.

Want some input from @ripienaar and @jnmoyne

[n-holmstedt]

I wonder if we should just keep the external API the same and if we fail looking up via the client list just check the leafnode list?

This was my initial thought as well, i might have misunderstood @ripienaar suggestions tho.

[ripienaar]

There is a risk since IDs are not somehow unique

CID 10 could be there. I aim for it but it disconnects before I kick it and then I disconnect LID 10

[derekcollison]

They are all unique..

[ripienaar]

They are all unique..

Hmm, ok so if one server cannot have cid:10 and lid:10 at the same time then that's fine then, was not aware

[derekcollison]

All ids are monotonic and come from same server variable under lock, so yes you can never have an id, whether CID, LID, RID or GID overlap or repeat..

[ripienaar]

Great, then yeah lets do as Derek suggest with trying one after the other

[n-holmstedt]

Updated the PR with the fallback to kicking leafnode client if a client id doesn't exist.

[ripienaar]

LGTM, I suspect the team might want a test or the existing one expanded.

[derekcollison]

Let's add a test and also update the PR description which is no longer correct.

Thanks.

[derekcollison]

Any updates here?

[derekcollison]

Closing in favor of #5587