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

fix(fluentd): Use buffer volume mount name from spec instead of default #1443

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

cbuto
Copy link
Contributor

@cbuto cbuto commented Aug 23, 2023

Fixes #1368

This PR updates the fluentd buffer metrics sidecar to use the configured buffer volume name instead of the default. Prior to this change, updating the bufferStorageVolume.pvc.source.claimName would result in the buffer metrics sidecar attempting to mount in the wrong volume name because of the hardcoded const value.

tested with:

apiVersion: logging.banzaicloud.io/v1beta1
kind: Logging
metadata:
  name: all-to-file
spec:
  fluentd:
    bufferStorageVolume:
      pvc:
        source:
          claimName: buf
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: 10Gi
          volumeMode: Filesystem
    bufferVolumeMetrics:
      port: 9200
      path: /metrics
      prometheusAnnotations: true
  controlNamespace: default
  fluentbit: {}

@cbuto cbuto force-pushed the feat/buffer-sidecar-mount-name branch from 940d073 to 5a8eeb7 Compare August 23, 2023 19:44
@pepov
Copy link
Member

pepov commented Aug 29, 2023

@cbuto Nice, but please add a check to see if we are dealing with a pvc or not (not flexible but should still be supported): 54fdce9

@cbuto
Copy link
Contributor Author

cbuto commented Aug 29, 2023

@cbuto Nice, but please add a check to see if we are dealing with a pvc or not (not flexible but should still be supported): 54fdce9

@pepov thanks, good call. I didn't know that was an option 😄

@pepov pepov requested a review from OverOrion August 30, 2023 09:18
Copy link
Contributor

@OverOrion OverOrion left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for contributing!

@OverOrion OverOrion merged commit aa83b92 into kube-logging:master Aug 30, 2023
15 checks passed
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

Successfully merging this pull request may close these issues.

Renaming the default Fluentd buffer PVC
3 participants