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

[bitnami/fluentd] Remove template fragments from example ConfigMap in README #2400

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

stv0g
Copy link
Contributor

@stv0g stv0g commented Apr 23, 2020

Description of the change

This PR closes #2389

Benefits

Users dont get errors when using the example ConfigMap for their setups.

Possible drawbacks

None.

Checklist

  • Chart version bumped in Chart.yaml according to semver.
  • Title of the PR starts with chart name (e.g. [bitnami/chart])

@stv0g
Copy link
Contributor Author

stv0g commented Apr 23, 2020

Please merge #2404 first. Afterwards I will rebase and bump the chart version

@andresbono
Copy link
Member

andresbono commented Apr 24, 2020

Hi @stv0g, thanks for opening this PR.

In the meantime the PR gets merged, we both agree that the {{- end }} line should be removed. However I don't see why the other changes are required. This is how I am testing it:

  1. Create the configmap.yaml file in bitnami/fluentd/templates/configmap.yaml
  2. Execute helm template bitnami/fluentd
  3. Look for the configmap k8s manifest:
  fluentd.conf: |
    # Prometheus Exporter Plugin
    # input plugin that exports metrics
    <source>
      @type prometheus
      port 24231
    </source>
    # TCP input to receive logs from the forwarders
    <source>
      @type forward
      bind 0.0.0.0
      port 24224
    </source>

Both templates ({{ .Values.metrics.service.port }} and {{ .Values.aggregator.port }}) are correctly substituted. Is it different for you?

@stv0g
Copy link
Contributor Author

stv0g commented Apr 24, 2020

Hi @andresbono,

Ah okay I see.. I usually dont checkout the Helm charts locally.
I just copied the example ConfigMap and applied it alongside Helm with kubectl.
I think its the more common scenario, or isn't it?

@andresbono
Copy link
Member

That's true, @stv0g. Let's hard-code the ports as this is an example.

Copy link
Member

@andresbono andresbono left a comment

Choose a reason for hiding this comment

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

LGTM

@andresbono andresbono merged commit c412525 into bitnami:master Apr 27, 2020
@andresbono
Copy link
Member

@stv0g, I merged the PR before we bumped the chart version, sorry for that. This is the commit 92a2215 that increases the chart version of bitnami/fluentd to 1.0.4.

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.

[bitnami/fluentd] Remaining Helm / GoLang template fragments in README
2 participants