Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users to suspend the Elasticsearch process for debugging purposes #4546

Closed
pebrc opened this issue Jun 2, 2021 · 5 comments · Fixed by #4946
Closed

Allow users to suspend the Elasticsearch process for debugging purposes #4546

pebrc opened this issue Jun 2, 2021 · 5 comments · Fixed by #4946
Assignees
Labels
>feature Adds or discusses adding a feature to the product

Comments

@pebrc
Copy link
Collaborator

pebrc commented Jun 2, 2021

Currently it is not possible to run debug or run disaster recovery operations like the elasticsearch-node tool on clusters managed by ECK.

We should create a mechanism by which users can specify to suspend the Elasticsearch process to then allow users/admins to exec into the the container to run the necessary operations.

A possible implementation approach:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: elasticsearch-sample
  annotations: 
        eck.elastic.co/suspend: "pod-1,pod-2'
spec:
  version: 7.13.0

This could be implemented by mounting a ConfigMap into the initcontainer of the Elasticsearch Pods with a list of Pods to be suspended and the initcontainer would simply not terminate as long as the Pods name is in that file or file exists with the Pods name.

@pebrc pebrc added discuss We need to figure this out >feature Adds or discusses adding a feature to the product labels Jun 2, 2021
@pebrc pebrc removed the discuss We need to figure this out label Oct 5, 2021
@sebgl
Copy link
Contributor

sebgl commented Oct 5, 2021

I think the approach @pebrc outlined above is probably the best thing we can do; I don't see a better way.

The only other alternative I see is to rely on a different container entrypoint where we can suspend the Elasticsearch process without stopping the container, but this feels very much against the spirit of containers.

Some "implementation details" to consider:

  • configmap granularity: per cluster vs. per statefulset? I'd argue per-cluster is simpler and leads to less extra K8s resources.
  • always run the init container vs. tweak the statefulset spec when required to add the init container only when requested? I think it's simpler to always have this init container in place in the StatefulSet spec, although it adds a small time overhead to startup time, so we can completely decouple the stop/start decision from the StatefulSet spec which doesn't change.
  • I think we want the operator to reconcile the configmap according to the annotation (rather than users manipulating the configmap directly), but also have the operator to delete the corresponding Pods immediately (rather than users having to edit the annotation then delete the Pod). This feature is there for exceptional cases where I think we don't mind much a "brutal" Pod deletion. From a reconciliation loop perspective, we need to handle the case where the configmap has been changed but the Pod has not yet been deleted (or has been already deleted :)). I think the latter can be simplified to: if the ES Pod status reports it has passed the initContainer, then delete the Pod (which will then be stuck in its init container), otherwise do nothing (either we're in the initContainer already, either it has not ran yer).
  • What should the init container do? I guess run a bash infinite for loop where it mostly sleeps, but also checks for the content of the mounted secret file every X seconds (something like 10 seconds seems reasonable, as we'd like this to be reactive-enough but not necessarily super fast)? If the configmap says the Pod should run, then exit the init container process.
  • Annotation name: I like the eck.elastic.co/suspend suggestion.

@pebrc
Copy link
Collaborator Author

pebrc commented Oct 5, 2021

always run the init container vs. tweak the statefulset spec when required to add the init container only when requested? I think it's simpler to always have this init container in place in the StatefulSet spec, although it adds a small time overhead to startup time, so we can completely decouple the stop/start decision from the StatefulSet spec which doesn't change.

Always running this initContainer would mean a rolling restart of all managed workloads on the ECK upgrade that ships that feature. I wonder if that is worth avoiding.

@sebgl
Copy link
Contributor

sebgl commented Oct 5, 2021

Always running this initContainer would mean a rolling restart of all managed workloads on the ECK upgrade that ships that feature. I wonder if that is worth avoiding.

If we tweak the StatefulSet spec on-demand, it means other Pods we're not interested in restarting will be upgraded/restarted with the new spec once to account for the other Pod to stop; then a second time to account for the other Pod to not-stop anymore?

@pebrc pebrc self-assigned this Oct 12, 2021
@pebrc
Copy link
Collaborator Author

pebrc commented Oct 12, 2021

One additional aspect is what should happen with regular reconciliation while a node is suspended. I see two ways:

  1. either we accept that a suspended node will stop almost all regular reconciliation attempt
  2. or we want to have it so that the suspended node is effectively ignored during regular reconciliation

The first option is pretty straightforward and will happened automatically: once a node is suspended our predicate system will register it as "missing" from the cluster and not make progress with most rolling upgrades and anything that is guarded by change budgets.

The second option would be quite intrusive, we would have to excluded the suspended nodes from all calculations: the expectations mechanism (Statefulset reconciliation will never reach expected state), cached Elasticsearch membership state, Pod readiness check during downscales etc.

@sebgl
Copy link
Contributor

sebgl commented Oct 13, 2021

I think we should treat the suspended node exactly as we already treat a regular Pod that is in its init container phase, no difference (I think that's your option 1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature Adds or discusses adding a feature to the product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants