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

Logstash Helm Chart review #11681

Closed
1 task
jmlrt opened this issue Mar 12, 2020 · 14 comments
Closed
1 task

Logstash Helm Chart review #11681

jmlrt opened this issue Mar 12, 2020 · 14 comments
Assignees

Comments

@jmlrt
Copy link
Member

jmlrt commented Mar 12, 2020

Product teams tasks

We’d like to have chart reviews from product teams to validate that they are following product recommended configuration for Kubernetes:

Logstash

  • Review Logstash Helm Chart @jsvd
@jmlrt
Copy link
Member Author

jmlrt commented Mar 12, 2020

Hi @jsvd
Thank you for working on this issue.

Logstash

Logstash chart code is in helm-charts/logstash.

This chart will create the following K8S resources:

What we would like for this review

  • Validate that a chart deployment with default values provides a well-configured Logstash
  • Validate that we don't need additional K8S resources for specific Logstash use cases (examples additional initContainer, additional volumes, Ingress...)
  • Validate the default resources requests / limits and LS_JAVA_OPTS
  • Help us define better probes to check that Logstash is up and ready (quoting @Crazybus livenessProbe = I'm dead, restart me / readinessProbe = I'm not ready to receive traffic, wait for me)
  • Validate that we can use RollingUpdate strategy for StatefulSet during updates
  • Any other thing that could be relevant

Please ping me if you have any questions

@jsvd
Copy link
Member

jsvd commented Apr 8, 2020

Things I've checked so far:

  1. Using a values.yml to change the configMaps of logstashConfig/logstashPipeline/logstashJavaOpts produces the expected results of configuring logstash settings and the pipelines without any surprises other than ensuring one overwrites the default logstash.conf
  2. Default values are sound.
  3. Probes need to reach Logstash's API which binds to localhost:9600 by default, so we may need to have http.port: 0.0.0.0 by default, or run a script in the container that does the curl request.
  4. The RollingUpdate strategy seems to work well after some tests. One thing to consider is that logstash is often used as a persistent process with configuration reloading, where chances to the local configuration files are picked up and pipelines are restarted as needed. From what I understand this isn't a normal way to interact and upgrade pods, which are killed and recreated.

@jsvd
Copy link
Member

jsvd commented Apr 9, 2020

Another comment on the probes:

Logstash doesn't have a good way to signal the readiness of its pipelines, only that it is up.
We can advise users to build custom readiness probes depending on the specific inputs of the pipeline: with an http input you can set the readinessProbe to ping that endpoint, with a tcp input we can check that the port is open, etc. Many inputs, like those that reach out like the jdbc input won't have a suitable readiness probe.

@jmlrt
Copy link
Member Author

jmlrt commented Apr 9, 2020

Probes need to reach Logstash's API which binds to localhost:9600 by default, so we may need to have http.port: 0.0.0.0 by default, or run a script in the container that does the curl request.

With default chart values, this is using logstash.yml from docker image which is already setting http.host: 0.0.0.0

Is it enough for probes?

@jmlrt
Copy link
Member Author

jmlrt commented Apr 9, 2020

One thing to consider is that logstash is often used as a persistent process with configuration reloading, where chances to the local configuration files are picked up and pipelines are restarted as needed. From what I understand this isn't a normal way to interact and upgrade pods, which are killed and recreated.

Yeah this chart is configured to automatically recreate pods if config files are changing using this block.

We already have requests to add config hot-reloading in elastic/helm-charts#474 and it is planned to add it later.

@jsvd
Copy link
Member

jsvd commented Apr 9, 2020

Is it enough for probes?

Indeed it is, I must have messed up something in my tests where I had to add that explicitly, but I can confirm the default is enough to make the probe work.

@jmlrt
Copy link
Member Author

jmlrt commented Apr 9, 2020

Did you use default config or override it?

When you are using default values, logstashConfig is empty so it is reusing the logstash.yml from Docker image, but if you are overriding logstashConfig, it will replace the default logstash.yml so http.host: 0.0.0.0 should always be included when overriding logstashConfig.

@jmlrt
Copy link
Member Author

jmlrt commented Apr 9, 2020

Is there a way to configure http.host: 0.0.0.0 via environment variables using env2yaml?

If we can setup some environment variables, we could be sure it is always setup, even if people override logstashConfig without setting it.

@jsvd
Copy link
Member

jsvd commented Apr 9, 2020

When you are using default values, logstashConfig is empty so it is reusing the logstash.yml from Docker image, but if you are overriding logstashConfig, it will replace the default logstash.yml so http.host: 0.0.0.0 should always be included when overriding logstashConfig.

That is a good idea, that's likely what happened to me. Is there any way to execute code in the pod for this probe? we could do a local curl localhost:9200 instead of doing that request from the outside

Is there a way to configure http.host: 0.0.0.0 via environment variables using env2yaml?

docker run -e HTTP_HOST=127.0.0.1 -it docker.elastic.co/logstash/logstash:7.6.1
2020/04/09 16:54:47 Setting 'http.host' from environment.

@jmlrt
Copy link
Member Author

jmlrt commented Apr 21, 2020

Is there a way to configure http.host: 0.0.0.0 via environment variables using env2yaml?

docker run -e HTTP_HOST=127.0.0.1 -it docker.elastic.co/logstash/logstash:7.6.1
2020/04/09 16:54:47 Setting 'http.host' from environment.

So I was planning to add HTTP_HOST=127.0.0.1 variable to Logstash container env, but it failed due to an issue related to env2yaml which prevent us to mix environment variables and logstash.yml (see elastic/helm-charts#333 (comment) for more details).

Is there any way to execute code in the pod for this probe? we could do a local curl localhost:9200 instead of doing that request from the outside

We could create an exec probe to do the curl from inside the pod (some commented example here).

Some version of k8s have a bug which result in very high CPU usage with exec probe (see elastic/helm-charts#485 (comment) for more details) so we prefer using httpGet as default probes when possible.

I created elastic/helm-charts#591 to include that in documentation.

@jmlrt
Copy link
Member Author

jmlrt commented Apr 21, 2020

  • Validate that we don't need additional K8S resources for specific Logstash use cases (examples additional initContainer, additional volumes, Ingress...)

FYI, we currently have elastic/helm-charts#453 request to add Ingress support so HTTP plugin port for example can be called from outside the K8S cluster.

Any other thing that could be relevant

FYI we currently have elastic/helm-charts#587 issue when mounting a certificate to reach a TLS elasticsearch.

@jsvd, thank you for your work.
Do you see any other thing to add to this review or shall we close it?

@jsvd
Copy link
Member

jsvd commented Apr 21, 2020

I think we're good here @jmlrt, can you only update the links to the issues you mention to target the elastic/helm-chars repo instead?

Thanks for all your work on this, k8s world is quite interesting :)

@jmlrt
Copy link
Member Author

jmlrt commented Apr 21, 2020

can you only update the links to the issues you mention to target the elastic/helm-chars repo instead?

done 🤦

@jmlrt
Copy link
Member Author

jmlrt commented Apr 21, 2020

I think we're good here @jmlrt, can you only update the links to the issues you mention to target the elastic/helm-chars repo instead?

Thanks for all your work on this, k8s world is quite interesting :)

I'm closing this issue so. I may ping you again in the future when we'll work on the mentionned tickets especially.
Don't hesitate to ping me also if you have some questions on k8s or have changes on Logstash side that may impact this chart.
Thanks for your review 👍

@jmlrt jmlrt closed this as completed Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants