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.

Sign up for GitHub

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

Jump to bottom

Increase the default PVC size for logstash #7540

Merged
merged 9 commits into from
Feb 9, 2024
Merged

Increase the default PVC size for logstash #7540

merged 9 commits into from
Feb 9, 2024

Conversation

kaisecheng
Copy link
Contributor

@kaisecheng kaisecheng commented Feb 8, 2024 •
edited
Loading

This commit increases the default PVC size to 1.5G for Logstash.
Persistent queue is a popular feature in Logstash. When PQ is enabled, the original PVC size 1G is too small for Logstash to start.

This Logstash resource should start without error

apiVersion: logstash.k8s.elastic.co/v1alpha1
kind: Logstash
metadata:
  name: logstash-sample
spec:
  count: 1
  version: 8.12.0
  config:
    queue.type: persisted
  pipelines:
    - pipeline.id: main
      config.string: input { exec { command => 'uptime' interval => 10 } } output { stdout{} }

relates: #7538

@botelastic botelastic bot added the triage label Feb 8, 2024
@kaisecheng kaisecheng requested a review from robbavey February 8, 2024 15:54
robbavey
robbavey reviewed Feb 8, 2024
View reviewed changes
docs/orchestrating-elastic-stack-applications/logstash.asciidoc
@@ -301,7 +301,7 @@ WARNING: Volume support for Logstash is a breaking change to earlier versions of
[discrete]
== Specifying the volume claim settings

By default, a PersistentVolume called `logstash-data` is created, that maps to `/usr/share/logstash/data` for persistent storage, typically used for storage from plugins. The `logstash-data` volume claim is, by default, a small (1Gi) volume, using the standard StorageClass of your Kubernetes cluster, but can be overridden by adding a `spec.volumeClaimTemplate` section named `logstash-data`.
By default, a PersistentVolume called `logstash-data` is created, that maps to `/usr/share/logstash/data` for persistent storage, typically used for storage from plugins. The `logstash-data` volume claim is, by default, a 1.5Gi volume, using the standard StorageClass of your Kubernetes cluster, but can be overridden by adding a `spec.volumeClaimTemplate` section named `logstash-data`.

For production workloads, you should define your own volume claim template with the desired storage capacity and (optionally) the Kubernetes link:https://kubernetes.io/docs/concepts/storage/storage-classes/[storage class] to associate with the persistent volume. To override this volume claim for `data` usages, the name of this volume claim must be `logstash-data`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be a good idea to add extra information on using persistent volumes, something like:

In particular, when enabling the persistent queue feature, using dead letter queues, or certain plugins that make heavy use of temporary storage, the default volume size will likely be insufficient for production workloads and you should increase the storage capacity, or consider creating separate volumes for these use cases.

Maybe just before the Separate storage section?

@kaisecheng @robbavey
update doc
23f25aa 
Co-authored-by: Rob Bavey <rob.bavey@elastic.co>
robbavey
robbavey approved these changes Feb 8, 2024
View reviewed changes
Copy link
Member

@robbavey robbavey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - maybe @karenzone should verify the amended text?

karenzone
karenzone reviewed Feb 8, 2024
View reviewed changes
Copy link
Contributor

@karenzone karenzone left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left suggestions inline for your consideration, mostly edits for easier parsing and to more closely align with our doc standards. I'm always happy to discuss if you'd like.

docs/orchestrating-elastic-stack-applications/logstash.asciidoc Outdated Show resolved Hide resolved
docs/orchestrating-elastic-stack-applications/logstash.asciidoc Outdated Show resolved Hide resolved
docs/orchestrating-elastic-stack-applications/logstash.asciidoc Outdated Show resolved Hide resolved
kaisecheng and others added 3 commits February 8, 2024 18:52
@kaisecheng @karenzone
Update docs/orchestrating-elastic-stack-applications/logstash.asciidoc
9bbf3fa 
Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>
@kaisecheng @karenzone
Update docs/orchestrating-elastic-stack-applications/logstash.asciidoc
b1e09c9 
Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>
@kaisecheng @karenzone
Update docs/orchestrating-elastic-stack-applications/logstash.asciidoc
9caa231 
Co-authored-by: Karen Metts <35154725+karenzone@users.noreply.github.com>
@kaisecheng
Copy link
Contributor Author

@karenzone your suggestions look fantastic :) thanks

@kaisecheng kaisecheng requested review from barkbay and pebrc February 8, 2024 18:55
@kaisecheng
Copy link
Contributor Author

@pebrc @barkbay this is ready for ECK team to review

barkbay
barkbay reviewed Feb 9, 2024
View reviewed changes
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found a bug in the controller, not sure if it's related to that PR, investigating...

docs/orchestrating-elastic-stack-applications/logstash.asciidoc Outdated Show resolved Hide resolved
docs/orchestrating-elastic-stack-applications/logstash.asciidoc Outdated Show resolved Hide resolved
pkg/controller/logstash/volume/defaults.go Outdated Show resolved Hide resolved
barkbay
barkbay approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 issues actually:

But this PR LGTM

kaisecheng and others added 3 commits February 9, 2024 10:29
@kaisecheng @barkbay
Update docs/orchestrating-elastic-stack-applications/logstash.asciidoc
3536e39 
Co-authored-by: Michael Morello <michael.morello@gmail.com>
@kaisecheng @barkbay
Update docs/orchestrating-elastic-stack-applications/logstash.asciidoc
b8671c0 
Co-authored-by: Michael Morello <michael.morello@gmail.com>
@kaisecheng @barkbay
Update pkg/controller/logstash/volume/defaults.go
43cd3e0 
Co-authored-by: Michael Morello <michael.morello@gmail.com>
@kaisecheng
Copy link
Contributor Author

@barkbay thanks for the suggestions. I think this PR is ready to merge. I will follow up on the new issues you created

@barkbay barkbay added v2.12.0 >enhancement Enhancement of existing functionality labels Feb 9, 2024
@botelastic botelastic bot removed the triage label Feb 9, 2024
@barkbay barkbay merged commit b13e71f into elastic:main Feb 9, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karenzone karenzone karenzone left review comments

@robbavey robbavey robbavey approved these changes

@barkbay barkbay barkbay approved these changes

@pebrc pebrc Awaiting requested review from pebrc

Assignees

@karenzone karenzone

@kaisecheng kaisecheng

Labels
>enhancement Enhancement of existing functionality :logstash v2.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kaisecheng @barkbay @robbavey @karenzone