Support helm chart for Logstash Elastic Stack resource #7143

azro352 · 2023-08-31T20:52:54Z

Hi

My contribution to the eck project : the logstash helm chart

Anyone feel free to review and comment, the goal is to put as examples the files of https://github.com/elastic/cloud-on-k8s/tree/main/config/recipes/logstash

In these examples there is not only a logstash, but also some elastic, beats, ... can you confirm me that I only need to put the Logstash part in the corresponding example file for the helm chart ?

Resolves #7128

Thanks

azro352 · 2023-09-01T17:58:12Z

I've write the chart for each section of spec, I've write the values for the tests

Tell me if you want some things more :)

azro352 · 2023-09-11T20:47:42Z

Hi @thbkrkr , Do I have something to do for my PR to be reviewed ? Or just you don't have time for now ?

thbkrkr · 2023-09-12T11:57:41Z

Sorry, it's a question of time and priority. We will review it as soon as possible.

Update eck-stack values to include logstash chart, disabled by default. Include logstash tests. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

naemono

Looks really good initially. Thanks much for the contribution. There are a couple of things I've initially noticed, and I've pushed a commit doing the following

Adding example logstash tests (run them with make helm-test)
Adding logstash to the eck-stack chart values, and Chart.yaml.

deploy/eck-stack/charts/eck-logstash/values.yaml

Co-authored-by: Michael Montgomery <mmontg1@gmail.com>

…ineRef, secureSettings into the values, chart and tests

…stash-eck

azro352 · 2023-09-19T19:17:05Z

Test passed

Ensuring dependencies are updated for eck-logstash chart.
Running 'helm lint' on eck-logstash chart.
==> Linting .

1 chart(s) linted, 0 chart(s) failed

### Chart [ eck-logstash ] .

 PASS  test logstash    templates/tests/logstash_test.yaml

Charts:      1 passed, 1 total
Test Suites: 1 passed, 1 total
Tests:       11 passed, 11 total
Snapshot:    0 passed, 0 total
Time:        4.26977523s

azro352 · 2023-09-19T19:18:49Z

Is that a feature you want to have github actions that tests for example the helm charts ?

naemono · 2023-09-20T18:45:20Z

Is that a feature you want to have github actions that tests for example the helm charts ?

No thank you. There are some buildkite steps that do this that I'll trigger soon after the review. Thanks again for the contributions.

naemono · 2023-10-03T20:06:12Z

I've been testing this a bit more, and found these issue:

diff --git a/deploy/eck-stack/charts/eck-logstash/templates/logstash.yaml b/deploy/eck-stack/charts/eck-logstash/templates/logstash.yaml
index ce720911a..eeb0d6a25 100644
--- a/deploy/eck-stack/charts/eck-logstash/templates/logstash.yaml
+++ b/deploy/eck-stack/charts/eck-logstash/templates/logstash.yaml
@@ -1,5 +1,5 @@
 ---
-apiVersion: logstash.k8s.elastic.co/v1
+apiVersion: logstash.k8s.elastic.co/v1alpha1
 kind: Logstash
 metadata:
   name: {{ include "logstash.fullname" . }}
@@ -44,13 +44,8 @@ spec:
     {{- toYaml . | nindent 4 }}
   {{- end }}

-  pipelines:
-    {{ toYaml .Values.pipelines | nindent 4 }}
-  volumeClaimTemplates:
-    {{ toYaml .Values.volumeClaimTemplates | nindent 4 }}
-  elasticsearchRefs:
-    {{ toYaml .Values.elasticsearchRefs | nindent 4 }}
-  services:
-    {{ toYaml .Values.services | nindent 4 }}
-  secureSettings:
-    {{ toYaml .Values.secureSettings | nindent 4 }}
+  pipelines: {{ toYaml .Values.pipelines | nindent 4 }}
+  volumeClaimTemplates: {{ toYaml .Values.volumeClaimTemplates | nindent 4 }}
+  elasticsearchRefs: {{ toYaml .Values.elasticsearchRefs | nindent 4 }}
+  services: {{ toYaml .Values.services | nindent 4 }}
+  secureSettings: {{ toYaml .Values.secureSettings | nindent 4 }}

The moving of the toYaml and adding a - is cosmetic, but the v1 => v1alpha1 is not.

azro352 · 2023-10-04T05:35:56Z

I'm not sure to understand what are the 2 contents being compared ?

RoseSecurity · 2023-10-04T18:39:07Z

@naemono anyway we can have integrated soon? Looking to utilize this in our infrastructure soon

naemono · 2023-10-04T22:08:05Z

@azro352 I'll update the PR. What I was trying to indicate was that Logstash has no "v1" version, it's only in alpha so it's version is "v1alpha1".

@RoseSecurity We're intending to have this merged and contained within our next release, which is in about a month.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Update Helm stack readme to include logstash. Add logstash example. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Add some notes to the example. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

deploy/eck-stack/charts/eck-logstash/templates/logstash.yaml

pebrc

Looks good. I leave the final 👍 to @naemono. The pipelinesRef is the only major thing to fix.

deploy/eck-stack/charts/eck-logstash/values.yaml

docs/orchestrating-elastic-stack-applications/stack-helm-chart.asciidoc

deploy/eck-stack/charts/eck-logstash/values.yaml

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

naemono

This LGTM. Everything is functional from my testing, and the docs are in place for this feature. Will merge after CI passes.

naemono · 2023-10-16T14:09:56Z

@elasticmachine run elasticsearch-ci/docs

naemono · 2023-10-16T15:04:59Z

💚 All backports created successfully

Status	Branch	Result
✅	2.10

Questions ?

Please refer to the Backport tool documentation

* Add logstash helm chart with examples and documentation * Update eck-stack chart to include logstash chart. * Update eck-stack values to include logstash chart, disabled by default. * Include logstash tests. --------- Signed-off-by: Michael Montgomery <mmontg1@gmail.com> Co-authored-by: Michael Montgomery <mmontg1@gmail.com> Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com> (cherry picked from commit 4fca9b8)

* Add logstash helm chart with examples and documentation * Update eck-stack chart to include logstash chart. * Update eck-stack values to include logstash chart, disabled by default. * Include logstash tests. --------- Signed-off-by: Michael Montgomery <mmontg1@gmail.com> Co-authored-by: Michael Montgomery <mmontg1@gmail.com> Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com> (cherry picked from commit 4fca9b8) Co-authored-by: azro352 <35503478+azro352@users.noreply.github.com>

botelastic bot added the triage label Aug 31, 2023

Add logstash helm chart skeleton

7dc34ef

azro352 force-pushed the helm-logstash-eck branch from 4e745cc to 7dc34ef Compare August 31, 2023 21:17

azro352 added 2 commits August 31, 2023 23:24

Add examples

61d70b2

Add examples and links to matching recipes

970e887

thbkrkr added >enhancement Enhancement of existing functionality :helm-charts labels Sep 1, 2023

botelastic bot removed the triage label Sep 1, 2023

azro352 marked this pull request as ready for review September 1, 2023 17:51

thbkrkr added v2.10.0 :logstash labels Sep 14, 2023

naemono self-requested a review September 14, 2023 14:29

Update eck-stack chart to include logstash chart.

ff374fd

Update eck-stack values to include logstash chart, disabled by default. Include logstash tests. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

naemono requested changes Sep 18, 2023

View reviewed changes

deploy/eck-stack/charts/eck-logstash/values.yaml Show resolved Hide resolved

deploy/eck-stack/charts/eck-logstash/values.yaml Show resolved Hide resolved

deploy/eck-stack/charts/eck-logstash/values.yaml Show resolved Hide resolved

azro352 and others added 3 commits September 19, 2023 20:14

Update deploy/eck-stack/charts/eck-logstash/values.yaml

c81acd2

Co-authored-by: Michael Montgomery <mmontg1@gmail.com>

Add image, serviceAccountName, revisionHistoryLimit, configRef, pipel…

1818904

…ineRef, secureSettings into the values, chart and tests

Merge remote-tracking branch 'origin/helm-logstash-eck' into helm-log…

b84dbe2

…stash-eck

azro352 requested a review from naemono September 19, 2023 19:17

Update apiversion to v1alpha1

0f127b7

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

naemono mentioned this pull request Oct 12, 2023

Update ECK-Beats default values to not include ElasticsearchRef. #7228

Merged

naemono added 6 commits October 12, 2023 08:46

Merge branch 'main' into helm-logstash-eck

dcd6c5b

Add logstash to docs.

040f6b1

Update Helm stack readme to include logstash. Add logstash example. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Add kibana to example to match docs.

f26037e

Add some notes to the example. Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Update logstash example versions.

7678c1f

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Adjusting values file with more links/docs.

dca93d7

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Add the standard note for allow_mmap

5775d5c

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

RoseSecurity approved these changes Oct 12, 2023

View reviewed changes

deploy/eck-stack/charts/eck-logstash/templates/logstash.yaml Outdated Show resolved Hide resolved

naemono requested review from pebrc, thbkrkr and barkbay October 13, 2023 12:55

pebrc reviewed Oct 13, 2023

View reviewed changes

deploy/eck-stack/charts/eck-logstash/values.yaml Outdated Show resolved Hide resolved

docs/orchestrating-elastic-stack-applications/stack-helm-chart.asciidoc Outdated Show resolved Hide resolved

deploy/eck-stack/charts/eck-logstash/values.yaml Outdated Show resolved Hide resolved

azro352 and others added 4 commits October 15, 2023 12:20

Fix pipelineRef to pipelinesRef

84a4c3d

Update deploy/eck-stack/charts/eck-logstash/values.yaml

5d7b848

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

Fix wording in docs.

d7c4a6b

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

Update wording in docs.

a375f89

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>

naemono approved these changes Oct 16, 2023

View reviewed changes

naemono enabled auto-merge (squash) October 16, 2023 13:11

RoseSecurity approved these changes Oct 16, 2023

View reviewed changes

naemono merged commit 4fca9b8 into elastic:main Oct 16, 2023

naemono mentioned this pull request Oct 16, 2023

[2.10] Add logstash helm chart (#7143) #7242

Merged

rhr323 changed the title ~~Add logstash helm chart~~ Support for Logstash Elastic stack resource via Helm Oct 25, 2023

thbkrkr changed the title ~~Support for Logstash Elastic stack resource via Helm~~ Support helm chart for Logstash Elastic Stack resource Oct 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support helm chart for Logstash Elastic Stack resource #7143

Support helm chart for Logstash Elastic Stack resource #7143

azro352 commented Aug 31, 2023 •

edited

Loading

azro352 commented Sep 1, 2023

azro352 commented Sep 11, 2023

thbkrkr commented Sep 12, 2023

naemono left a comment

azro352 commented Sep 19, 2023

azro352 commented Sep 19, 2023

naemono commented Sep 20, 2023

naemono commented Oct 3, 2023

azro352 commented Oct 4, 2023

RoseSecurity commented Oct 4, 2023

naemono commented Oct 4, 2023

pebrc left a comment

naemono left a comment

naemono commented Oct 16, 2023

naemono commented Oct 16, 2023

Support helm chart for Logstash Elastic Stack resource #7143

Support helm chart for Logstash Elastic Stack resource #7143

Conversation

azro352 commented Aug 31, 2023 • edited Loading

azro352 commented Sep 1, 2023

azro352 commented Sep 11, 2023

thbkrkr commented Sep 12, 2023

naemono left a comment

Choose a reason for hiding this comment

azro352 commented Sep 19, 2023

azro352 commented Sep 19, 2023

naemono commented Sep 20, 2023

naemono commented Oct 3, 2023

azro352 commented Oct 4, 2023

RoseSecurity commented Oct 4, 2023

naemono commented Oct 4, 2023

pebrc left a comment

Choose a reason for hiding this comment

naemono left a comment

Choose a reason for hiding this comment

naemono commented Oct 16, 2023

naemono commented Oct 16, 2023

💚 All backports created successfully

Questions ?

azro352 commented Aug 31, 2023 •

edited

Loading