Logstash improvements: auto pipeline reload. #1668

mashhurs · 2024-02-08T20:15:51Z

Description

This change includes number of Logstash improvements:

auto pipeline reload: sometimes we need to update the pipeline config and run with some integrations. Todo so, we need to make pipeline config writable (disable read-only set logic) and add --config.reload.automatic when running Logstash to avoid extra container restart.
Separated elastic-package generated config as it will be mounted file system, cannot be writable while docker container is active. Current approach generates and copies to a writable file where final file can be changed and Logstash automatically reloads pipeline.
~~enable SSL between LS and agent: depends on Refactor New cert to use enum #1664, most users use SSL between agent and Logstash and it is the best practise.~~
~~remove doc id in pipeline config to cover generic cases: the integrations with time series do not accept the document with ID set (reference)~~ - this will cause integration tests fail and we believe this will be anyway short term solution, we should rather focus on long-term: changeable config, so I am leaving this AS-IS as well.

internal/stack/_static/logstash.conf.tmpl

bhapas

To have ssl enabled between logstash and agent, a client certificate needs to be added to the fleet-logstash-output created in fleet. Similar to https://github.com/elastic/elastic-package/pull/1646/files#diff-16829f0b151b844f24aebc77c8aea194ae3b84fca32debdd296aa22cba7a99bfR36

These certificates are then authorized by logstash

mashhurs · 2024-02-09T18:15:08Z

To have ssl enabled between logstash and agent, a client certificate needs to be added to the fleet-logstash-output created in fleet. Similar to https://github.com/elastic/elastic-package/pull/1646/files#diff-16829f0b151b844f24aebc77c8aea194ae3b84fca32debdd296aa22cba7a99bfR36

These certificates are then authorized by logstash

Thanks for letting me know. Since I don't have much internal detail knowledge, I will leave this part AS-IS and focus on my current needs.

internal/stack/_static/docker-compose-stack.yml.tmpl

changes provided

internal/stack/_static/logstash_startup.sh

internal/stack/_static/docker-compose-stack.yml.tmpl

internal/stack/resources.go

internal/stack/_static/logstash_startup.sh

jsoriano · 2024-02-14T13:34:50Z

internal/stack/_static/docker-compose-stack.yml.tmpl

      - "../certs/logstash:/usr/share/logstash/config/certs"
      - "../certs/elasticsearch/cert.pem:/usr/share/logstash/config/certs/elasticsearch.pem"


To ensure that we neither try to write here when running locally.

Suggested change

- "../certs/logstash:/usr/share/logstash/config/certs"

- "../certs/elasticsearch/cert.pem:/usr/share/logstash/config/certs/elasticsearch.pem"

- "../certs/logstash:/usr/share/logstash/config/certs:ro"

- "../certs/elasticsearch/cert.pem:/usr/share/logstash/config/certs/elasticsearch.pem:ro"

It seems making config/certs read-only, mounting elasticsearch.pem is failing?! Removing read-only from certs works fine, we need to mount file one by one if it is strictly required.

Error response from daemon: failed to create task for container: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error mounting "/opt/buildkite-agent/.elastic-package/profiles/logstash/certs/elasticsearch/cert.pem" to rootfs at "/usr/share/logstash/config/certs/elasticsearch.pem": open /var/lib/docker/overlay2/c73fe26af0e42262b9972a5dc32a073d3141e9dee2467ca8ed3bc5d07ce6733b/merged/usr/share/logstash/config/certs/elasticsearch.pem: read-only file system: unknown

…d agent, remove doc id in pipeline config to cover generic cases.

…res with ES.

…Docker volumes which will be busy during the Logstash run.

Read-only mounted certificates, set proper permission to a Logstash startup shell script. Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>

elasticmachine · 2024-02-15T00:16:14Z

💚 Build Succeeded

Buildkite Build
Commit: cea200c

History

💚 Build #2399 succeeded a9cac49
💔 Build #2398 failed 0fb0711
💔 Build #2389 failed eaed0b5
💔 Build #2388 failed 91972a2
💔 Build #2387 failed e556554
💔 Build #2371 failed cf8c563

cc @mashhurs

bhapas

LGTM. Please hold for @jsoriano to approve as well

jsoriano · 2024-02-15T11:36:08Z

internal/stack/_static/docker-compose-stack.yml.tmpl

-    # Also logstash-filter-elastic_integration plugin is installed by default to run ingest pipelines in logstash.
-    # elastic-package#1637 made improvements to enable logstash stats through port 9600.
-    command: bash -c 'openssl pkcs8 -inform PEM -in /usr/share/logstash/config/certs/key.pem -topk8 -nocrypt -outform PEM -out /tmp/logstash.pkcs8.key && chmod +x /tmp/logstash.pkcs8.key && if [[ ! $(bin/logstash-plugin list) ==  *"logstash-filter-elastic_integration"* ]]; then echo "Missing plugin  logstash-filter-elastic_integration, installing now" && bin/logstash-plugin install  logstash-filter-elastic_integration; fi && bin/logstash -f /usr/share/logstash/pipeline/logstash.conf'
+    command: bash /usr/share/logstash/startup.sh


👍 thanks!

jsoriano · 2024-02-15T11:37:47Z

internal/stack/_static/logstash_startup.sh

+
+# runs Logstash
+run() {
+  bin/logstash -f "$LOGSTASH_HOME/pipeline/logstash.conf" --config.reload.automatic


Nit. Could we use exec to replace the process?

Suggested change

bin/logstash -f "$LOGSTASH_HOME/pipeline/logstash.conf" --config.reload.automatic

exec bin/logstash -f "$LOGSTASH_HOME/pipeline/logstash.conf" --config.reload.automatic

jsoriano · 2024-02-15T11:38:48Z

internal/stack/_static/logstash_startup.sh

+  openssl pkcs8 -inform PEM -in "$ls_cert_path/key.pem" -topk8 -nocrypt -outform PEM -out "/tmp/logstash.pkcs8.key"
+  chmod 777 "/tmp/logstash.pkcs8.key"


Nit. It doesn't use to be a good practice to write a key in /tmp or with 777. But this is a testing scenario, so as you prefer if we don't have another way to do it now.

Sounds good! Let me follow up Nits with separate PR because this is somehow blocking my another task: elastic/logstash-filter-elastic_integration#122 (comment)

Polish some things in the logstash scenario: * Pre-build logstash image: * It includes the plugin now so it doesn't need to be installed on every run, speeding up startup after first run. * It includes the configuration, so it can be modified with auto-reload, as introduced in #1668. * Use private key without converting it to PKCS8. * Use actual CA instead of elasticsearch certificate as CA in logstash output. * Startup script removed as operations moved to other places or not needed. * Use the same configuration template for serverless and compose providers.

mashhurs requested review from a team and bhapas February 8, 2024 20:15

mashhurs self-assigned this Feb 8, 2024

mashhurs commented Feb 8, 2024

View reviewed changes

internal/stack/_static/logstash.conf.tmpl Show resolved Hide resolved

mashhurs mentioned this pull request Feb 8, 2024

E2E tests for the plugin. elastic/logstash-filter-elastic_integration#122

Merged

1 task

bhapas previously requested changes Feb 9, 2024

View reviewed changes

mashhurs force-pushed the logstash-improvements-reload branch from f8d5bb3 to bd9c877 Compare February 9, 2024 18:13

mashhurs force-pushed the logstash-improvements-reload branch from edfec7c to 6cdd220 Compare February 12, 2024 23:38

mashhurs commented Feb 13, 2024

View reviewed changes

internal/stack/_static/docker-compose-stack.yml.tmpl Show resolved Hide resolved

mashhurs requested a review from bhapas February 13, 2024 00:10

bhapas reviewed Feb 13, 2024

View reviewed changes

internal/stack/_static/docker-compose-stack.yml.tmpl Outdated Show resolved Hide resolved

bhapas mentioned this pull request Feb 13, 2024

Enable ssl between logstash and agent #1669

Merged

mashhurs force-pushed the logstash-improvements-reload branch from 774fbf1 to 75ca508 Compare February 13, 2024 20:43

mashhurs requested review from bhapas and jsoriano February 13, 2024 20:47

bhapas reviewed Feb 13, 2024

View reviewed changes

internal/stack/_static/logstash_startup.sh Outdated Show resolved Hide resolved

internal/stack/_static/logstash_startup.sh Outdated Show resolved Hide resolved

mashhurs requested a review from bhapas February 13, 2024 23:45

mashhurs commented Feb 14, 2024

View reviewed changes

internal/stack/_static/docker-compose-stack.yml.tmpl Outdated Show resolved Hide resolved

jsoriano reviewed Feb 14, 2024

View reviewed changes

mashhurs and others added 9 commits February 14, 2024 07:49

Logstash improvements: auto pipeline reload, enable SSL between LS an…

afd8bf2

…d agent, remove doc id in pipeline config to cover generic cases.

Revert SSL changes between agent and Logstash.

caf78b4

Make Logstash pipeline configs changeable.

cd84490

Let's focus on Logstash integration plugin and avoid connection failu…

dee03da

…res with ES.

Put back config changes and make overwritable config separating from …

75d9e6e

…Docker volumes which will be busy during the Logstash run.

Separate Logstash initialization script and define it as a resource.

1209dda

Revise the comment to make the generic statement.

f8294df

Test if permissions inherited on mounted fs.

0159be7

Apply suggestions from code review

94258b8

Read-only mounted certificates, set proper permission to a Logstash startup shell script. Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>

Refining the logics applied by code review: same apply on serverless.

eaed0b5

mashhurs force-pushed the logstash-improvements-reload branch from 91972a2 to eaed0b5 Compare February 14, 2024 15:54

mashhurs added 3 commits February 14, 2024 11:45

Revert the read-only mounting since it is failing on BK agent.

0fb0711

Enable SSL between LS and agent.

a9cac49

Make mounted files read-only except certs folder.

cea200c

mashhurs requested a review from jsoriano February 15, 2024 00:18

bhapas approved these changes Feb 15, 2024

View reviewed changes

jsoriano approved these changes Feb 15, 2024

View reviewed changes

mashhurs merged commit b2ee264 into elastic:main Feb 15, 2024

mashhurs deleted the logstash-improvements-reload branch February 15, 2024 14:43

jsoriano mentioned this pull request Feb 15, 2024

Pre-build logstash image with plugin and configuration #1687

Merged

		- "../certs/logstash:/usr/share/logstash/config/certs"
		- "../certs/elasticsearch/cert.pem:/usr/share/logstash/config/certs/elasticsearch.pem"

	bin/logstash -f "$LOGSTASH_HOME/pipeline/logstash.conf" --config.reload.automatic
	exec bin/logstash -f "$LOGSTASH_HOME/pipeline/logstash.conf" --config.reload.automatic

		openssl pkcs8 -inform PEM -in "$ls_cert_path/key.pem" -topk8 -nocrypt -outform PEM -out "/tmp/logstash.pkcs8.key"
		chmod 777 "/tmp/logstash.pkcs8.key"

Uh oh!

Logstash improvements: auto pipeline reload. #1668

Logstash improvements: auto pipeline reload. #1668

Uh oh!

Conversation

mashhurs commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Uh oh!

bhapas left a comment

Choose a reason for hiding this comment

Uh oh!

mashhurs commented Feb 9, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsoriano Feb 14, 2024

Choose a reason for hiding this comment

Uh oh!

mashhurs Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 15, 2024

💚 Build Succeeded

History

Uh oh!

bhapas left a comment

Choose a reason for hiding this comment

Uh oh!

jsoriano Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

jsoriano Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

jsoriano Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

mashhurs Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mashhurs commented Feb 8, 2024 •

edited

Loading