[logstash] first version of logstash helm chart #333

jmlrt · 2019-10-15T20:35:54Z

This is the first version of logstash helm chart.

Related to #69

TODO

Need to add ~~templates/tests~~ & NOTES.txt
Need to add python tests
Need to add examples with goss tests
Need to add CI jobs
Need to optimize resources usage for logstash (persistent disk size, resources requests/limits, java heap)
Need to test beats input
Need to add service ~~and ingress for logstash inputs~~

jordansissel

Thanks for working on this! I added some comments on a brief review. I'll also compare this with what we have deployed in Infosec for this later.

logstash/templates/service.yaml

logstash/values.yaml

jordansissel · 2019-10-16T05:46:26Z

logstash/values.yaml

+podManagementPolicy: "Parallel"
+
+protocol: http
+httpPort: 9600


I don't recommend exposing Logstash's http API as it is, to my knowledge, unauthenticated and has an API for mutation of things like log level.

Service and Ingress for Logstash HTTP API where removed in 1768d4b.

Do you think we should also remove port exposition at Pod level?

jordansissel · 2019-10-16T05:48:00Z

logstash/values.yaml

+readinessProbe:
+  httpGet:
+    path: /
+    port: http


readiness is complicated. A successful HTTP call to port 9600 doesn't necessarily mean Logstash is capable of serving business needs.

I don't have any alternative solutions in mind, but I'll try to think of something. In the meantime, this comment is not a blocker ;)

Yes I guess I'll start like that and we'll see how we can set a better probe in a second time.

logstash/values.yaml

logstash/examples/default/values.yaml

jmlrt · 2019-10-17T11:00:42Z

Chart Tests are not relevant for Logstash as we don't use service while chart tests usually imply running another pod which request some service resource.

Crazybus

This is looking great! I didn't try to run it or play around with it yet but it is shaping up very nicely!

The main thing missing functionality wise right now is how to handle services and ingress. This is a bit trickier for logstash since it is normal for a logstash instance to have multiple or none service endpoints.

logstash/Chart.yaml

logstash/README.md

logstash/examples/default/values.yaml

logstash/templates/configmap-config.yaml

logstash/templates/statefulset.yaml

Crazybus · 2019-10-17T13:33:33Z

logstash/values.yaml

+
+# A list of secrets and their paths to mount inside the pod
+secretMounts: []
+


I think leaving in the commented example like the Elasticsearch chart would make it easier for users to know the formatting.

Yeah for this one I'd like to add a commented example. Do you have any good use case needing a secret mount in Logstash so we can have a relevant example.

Best example would be for mounting certificates to be able to talk to Elasticsearch with security enabled.

Crazybus · 2019-10-17T13:34:13Z

logstash/values.yaml

+
+logstashJavaOpts: "-Xmx1g -Xms1g"
+
+resources:


I think these are just copied for Elasticsearch for now. But we should chat with the Logstash team (or check docs) to see what the recommended defaults would be.

jmlrt · 2019-10-21T20:30:56Z

I guess this is ready for a second review and hope to be able to merge it and add the missing parts in following PR.

Status of what's not fully completed in this PR:

Need to optimize resources usage for logstash (persistent disk size, resources requests/limits, java heap)

I didn't find any recommendation for default values in Logstash documentation. I guess we can add them in a following PR.
Need to test Beats input

Beats input has been tested using services provided in 68a85c0 and the Filebeat chart locally.

We have 3 solution to add an automated test:
- Reuse some Beats coming from Filebeat examples in the same way we are reusing Elasticsearch coming from Elasticsearch examples in other chart tests.
  In this case should Logstash and Filebeat test run in // or do we need to start one before the other?
- Do not reuse Beats from another test but a new one (using helm install elastic/filebeat) in the make install target of the new logstash/examples/beats test
- Adding a new "full stack test" outside of the examples of each charts which would deploy and tests all the chart together (filebeat + metricbeat + elasticsearch + logstash + kibana) so we don't need to add a new test for all the combinations of products integration we could have inside specific charts examples. This way also ensure that all chart are running in the same time, while we can't be 100% sure of the order in different sub jobs of a matrix job.
Need to add service ~~and ingress for logstash inputs~~

Service has been added in 68a85c0, I'm not sure if we really want to manage Ingress for the "listener inputs". I guess if we need that we can add it in a following PR.

cc @Crazybus @jordansissel

Logstash HTTP API shouldn't be exposed externally as Logstash doesn't manage any authentication on this API. Also as Logstash doesn't support clustering, being able to access this API via a service doesn't make sense.

`release` label need to be added StatefulSet `spec.selector.matchLabels` to be included in pvc so we can select only pvc matching release

…he hostname

…ipeline configmap has changed

Crazybus

LGTM! This is amazing work :) 🌟

I didn't find any recommendation for default values in Logstash documentation. I guess we can add them in a following PR.

Yup, I do think it would be a good idea to get these set properly before the first release though so that we can still change them.

Need to test Beats input

I don't think we really need to do this. For something like Kibana it makes sense to check that it can connect to Elasticsearch because it is required. I think that as long as we make sure that Logstash has the capability to talk to an output with certificates/passwords that it should be enough. So just making sure that secrets can be mounted, passwords can be passed into the config via the environment variable should mean that all outputs will be supported. Testing all of the output plugins would be a bit unreasonable and is something best left to the Logstash team.

Need to add service and ingress for logstash inputs
Service has been added in 68a85c0, I'm not sure if we really want to manage Ingress for the "listener inputs". I guess if we need that we can add it in a following PR.

+1 On adding ingress, with the ability to create an ingress per service input in Logstash. But yeah that can totally wait for another PR.

Crazybus · 2019-10-24T11:59:19Z

.ci/jobs/elastic+helm-charts+master+integration-logstash.yml

@@ -0,0 +1,39 @@
+---


If you want you could split these jobs off into a separate PR. That way we can have Jenkins running CI tests for the changes in here.

Crazybus · 2019-10-24T12:09:16Z

logstash/values.yaml

+
+# A list of secrets and their paths to mount inside the pod
+secretMounts: []
+


Best example would be for mounting certificates to be able to talk to Elasticsearch with security enabled.

Crazybus · 2019-10-24T12:11:19Z

logstash/values.yaml

+#  annotations: {}
+#  type: ClusterIP
+#  ports:
+#    - name: beats


Nice! It's really cool that you can just support multiple ports and types this way.

masterkain · 2019-10-31T22:18:25Z

can't see this on the repo yet

NAME                 	CHART VERSION	APP VERSION	DESCRIPTION
elastic/elasticsearch	7.4.1        	7.4.1      	Official Elastic helm chart for Elasticsearch
elastic/filebeat     	7.4.1        	7.4.1      	Official Elastic helm chart for Filebeat
elastic/kibana       	7.4.1        	7.4.1      	Official Elastic helm chart for Kibana
elastic/metricbeat   	7.4.1        	7.4.1      	Official Elastic helm chart for Metricbeat

wanted to give it a try, thanks!

jmlrt · 2019-11-04T12:37:54Z

Hi @masterkain,

Logtsash chart hasn't been uploaded to Elastic Helm repository yet. It should be released soon on Helm repos with 7.5.0 release.

In the meantime, you need to install it from GitHub repo:

git clone https://github.com/elastic/helm-charts.git
cd helm-chart/logstash && helm install --name logstash .

jmlrt added the feature label Oct 15, 2019

jmlrt requested a review from Crazybus October 15, 2019 20:35

jmlrt self-assigned this Oct 15, 2019

jmlrt mentioned this pull request Oct 15, 2019

Official Logstash Helm chart feature request #69

Closed

jmlrt force-pushed the logstash-helm-chart branch from 59228fc to fa28783 Compare October 15, 2019 20:41

jordansissel suggested changes Oct 16, 2019

View reviewed changes

jmlrt force-pushed the logstash-helm-chart branch 2 times, most recently from 58ff841 to 39f9723 Compare October 16, 2019 13:55

jmlrt commented Oct 16, 2019

View reviewed changes

logstash/examples/default/values.yaml Outdated Show resolved Hide resolved

Crazybus reviewed Oct 17, 2019

View reviewed changes

jmlrt mentioned this pull request Oct 18, 2019

[elasticsearch] remove unused default value #337

Merged

jmlrt force-pushed the logstash-helm-chart branch 4 times, most recently from 68a85c0 to 92130a8 Compare October 21, 2019 20:30

jmlrt marked this pull request as ready for review October 21, 2019 20:31

jmlrt requested review from Crazybus and jordansissel October 21, 2019 20:32

jmlrt added 10 commits October 24, 2019 08:45

[logstash] first version of logstash helm chart

bbcee73

[logstash] remove service and ingress resources

bb527a8

Logstash HTTP API shouldn't be exposed externally as Logstash doesn't manage any authentication on this API. Also as Logstash doesn't support clustering, being able to access this API via a service doesn't make sense.

[logstash] add first default example

50d0281

[logstash] allow using either configmap or env variable for logstash.yml

0abec32

[logstash] add notes

c26c337

[logstash] add logstash to chart list

9bd9ac9

[logstash] add first goss test

b7fde8b

[logstash] use elasticsearch in logstash example

8cccd76

[logstash] fix deprecated note about default storage class

bd82d68

[logstash] cleanup remaining pv(c) after goss test

1abc224

jmlrt added 17 commits October 24, 2019 08:45

[logstash] add oss and default examples

dfe785b

[logstash] improve pvc purge

50defe8

`release` label need to be added StatefulSet `spec.selector.matchLabels` to be included in pvc so we can select only pvc matching release

[logstash] remove another deprecated note about storage class

1436884

[logstash] stop enforcing node.name as the default value is already t…

68b9b69

…he hostname

[logstash] remove unused default value

2d17d30

[logstash] use a different annotation to handle logstash restart if p…

d1aef83

…ipeline configmap has changed

[logstash] add missing lifecycle

f8acf30

[logstash] fix goss test

a989d81

[logstash] add first python tests

42f5673

[logstash] add ci jobs

af5dc4c

[logstash] cleanup/update values in README and values.yaml

e222ca6

[logstash] disable persistence by default

0acbc1e

[logstash] increase probes initial delay

2c1bf4f

[logstash] manage extra containers

bac794e

[logstash] manage services for inputs implementing a listener

fd3f49f

[logstash] use helm 2.15.1

8c334da

[logstash] add 6.x example

85c657b

jmlrt force-pushed the logstash-helm-chart branch from 92130a8 to 85c657b Compare October 24, 2019 07:53

Crazybus approved these changes Oct 24, 2019

View reviewed changes

jmlrt merged commit b2d5d86 into elastic:master Oct 29, 2019

jmlrt deleted the logstash-helm-chart branch October 29, 2019 10:36

This was referenced Nov 6, 2019

Cannot pull logstash chart #353

Closed

[logstash] upload chart to repository so it can be used #365

Closed

jmlrt mentioned this pull request Nov 13, 2019

Status of the logstash chart? #370

Closed

jmlrt mentioned this pull request Apr 21, 2020

Logstash Helm Chart review elastic/logstash#11681

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[logstash] first version of logstash helm chart #333

[logstash] first version of logstash helm chart #333

jmlrt commented Oct 15, 2019 •

edited

Loading

jordansissel left a comment

jordansissel Oct 16, 2019

jmlrt Oct 16, 2019

jordansissel Oct 16, 2019

jmlrt Oct 16, 2019

jmlrt commented Oct 17, 2019

Crazybus left a comment

Crazybus Oct 17, 2019

jmlrt Oct 17, 2019

Crazybus Oct 24, 2019

Crazybus Oct 17, 2019

jmlrt commented Oct 21, 2019

Crazybus left a comment

Crazybus Oct 24, 2019

Crazybus Oct 24, 2019

Crazybus Oct 24, 2019

masterkain commented Oct 31, 2019

jmlrt commented Nov 4, 2019


		# A list of secrets and their paths to mount inside the pod
		secretMounts: []


		logstashJavaOpts: "-Xmx1g -Xms1g"

		resources:

[logstash] first version of logstash helm chart #333

[logstash] first version of logstash helm chart #333

Conversation

jmlrt commented Oct 15, 2019 • edited Loading

TODO

jordansissel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmlrt commented Oct 17, 2019

Crazybus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmlrt commented Oct 21, 2019

Crazybus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masterkain commented Oct 31, 2019

jmlrt commented Nov 4, 2019

jmlrt commented Oct 15, 2019 •

edited

Loading