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

Helm: Fix regression in chart name #2406

Merged
merged 1 commit into from
Jul 24, 2020
Merged

Helm: Fix regression in chart name #2406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion production/helm/loki-stack/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: "v1"
name: loki-stack
version: 0.38.2
version: 0.38.3
appVersion: v1.5.0
kubeVersion: "^1.10.0-0"
description: "Loki: like Prometheus, but for logs."
Expand Down
6 changes: 3 additions & 3 deletions production/helm/loki-stack/requirements.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ dependencies:
- name: "loki"
condition: loki.enabled
repository: "file://../loki"
version: "^0.6.0"
version: "^0.30.0"
- name: "promtail"
condition: promtail.enabled
repository: "file://../promtail"
version: "^0.6.0"
version: "^0.23.0"
- name: "fluent-bit"
condition: fluent-bit.enabled
repository: "file://../fluent-bit"
version: "^0.0.1"
version: "^0.1.0"
- name: "grafana"
condition: grafana.enabled
version: "~3.8.15"
Expand Down
4 changes: 0 additions & 4 deletions production/helm/loki-stack/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
loki:
fullnameOverride: loki
enabled: true

promtail:
enabled: true
loki:
serviceName: loki

fluent-bit:
enabled: false
Expand Down Expand Up @@ -41,7 +38,6 @@ filebeat:

logstash:
enabled: false
fullnameOverride: logstash-loki
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is this is required for the default chart to work with logstash.

Unfortunately there is no way to know the service name within the value file. This is used by both the filebeat.yml and the main output of logstash.

Do you have an alternative ?

Copy link
Contributor Author

@steven-sheehy steven-sheehy Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it makes sense to break existing deployments for a disabled by default feature. A better alternative is to enhance the upstream chart to support templating of the necessary values so you can specify templated values in the values.yaml. For example, here is the config from my values.yaml to configure Grafana to use Loki (which by the way is broken currently due to this issue):

  grafana:
    additionalDataSources:
      - name: Loki
        type: loki
        access: proxy
        url: http://{{ .Release.Name }}-loki:3100
        jsonData:
          maxLines: 500

See the templating PR I did for Grafana's ingress annotations to see how it's done. I don't use logstash or know which fields have to be templated to be able to help with this.

Or if that's not feasible then simply documenting how users can adjust the chart for use with Logstash should suffice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gonna need some time to figure this out. May be we can add those override in the docs via —set for now as examples for logstash.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found an other bug with those templates, for example promtail in the umbrella chart will use the stack template but with value from the subchart and the template is wrong.

Subchart and templates is not a fun area.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright I've sent a PR to make this part of the doc instead.

image:
repository: grafana/logstash-output-loki
tag: 1.0.1
Expand Down