Add /ingester/unregister-on-shutdown HTTP endpoint

[LasseHels]

What this PR does

This pull request adds a HTTP /ingester/unregister-on-shutdown endpoint to ingesters. The endpoint was originally conceived here and has a formal proposal document here.

Which issue(s) this PR fixes or relates to

Fixes #5901.

Checklist

• Tests updated.
• Documentation added.
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
• about-versioning.md updated with experimental features. I'm not sure if this is an experimental feature?

[LasseHels]

The original proposal suggested an /ingester/unregister-on-shutdown name for the endpoint. I've only now realised that this is probably a terrible name, as the term shutdown carries a specific meaning in the context of ingesters.

The name /ingester/unregister-on-shutdown invites operators to erroneously believe that the endpoint controls whether the endpoint unregisters from the ring on its next shutdown, as opposed to its next termination. A shutdown is a termination, but a termination is not necessarily a shutdown, and the distinction is important.

Ingesters already have an /ingester/shutdown endpoint, and I think it's important that the unregister endpoint explicitly conveys that it is not related specifically to shutdowns, but more generally to any kind of graceful termination.

I'll give some thought to a better name. Currently, I am partial to a name like /ingester/unregister-on-termination.

[LasseHels]

After giving it some thought, I think I prefer the original /ingester/unregister-on-shutdown name. While it is true that the term "shutdown" is ambiguous in the context of ingesters, "unregister-on-shutdown" is at least consistent with the name of the -ingester.ring.unregister-on-shutdown configuration option.

Having an endpoint called /ingester/unregister-on-termination that manipulates a -ingester.ring.unregister-on-shutdown configuration option is likely more confusing than re-using the technically incorrect but semantically consistent "shutdown" term.

I expect much of this ambiguity can be clarified in the endpoint's documentation, and I will attempt to do so.

Opinions on the name of the endpoint are welcome.

[dimitarvdimitrov]

my 2c: Mimir has already conflated scaledown with shutdown (e.g. -ingester.ring.unregister-on-shutdown vs POST /ingester/shutdown vs POST /ingester/prepare-shutdown). Your decision to keep /ingester/unregister-on-shutdown makes sense to me

[LasseHels]

We are still in the process of testing out a crude implementation of this endpoint in our cluster. Since that is expected to take some time, I figured the review of this feature could be performed in parallel.

[jdbaldry]

docs lgtm

[LasseHels]

@dimitarvdimitrov It would be great if we could proceed with the review of this change. I'm not sure who to ping (or who can review) so I am reaching out to you as you've been previously involved with the issue.

[LasseHels]

@dimitarvdimitrov Any update on the review status of this pull request? I'd be happy to reach out to someone else from the Mimir team in case you're not the right person to review this.

[pstibrany]

Thank you for your work on this. I'd suggest to simplify the handler code, and fix mismatch in HTTP method names (endpoint is registered with POST but reacts on PUT).

[LasseHels]

@pstibrany Thanks for reviewing! With your suggestions, the diff is now much leaner. I've addressed all comments. Feel free to take another look.

[pstibrany]

Thank you very much for addressing my feedback and very clear explanation of taken actions! Let's fix the changelog entry (my bad that I missed in previous review), and we can merge it.