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

helm/loki-stack: render loki datasource only if grafana is enabled #2086

Merged
merged 1 commit into from
May 29, 2020

Conversation

osela
Copy link
Contributor

@osela osela commented May 17, 2020

What this PR does / why we need it:
Currently, the grafana section in the default values.yaml of the loki-stack chart contains

grafana:
  enabled: false
  sidecar:
    datasources:
      enabled: true

which renders the loki datasource although grafana itself is disabled.
This PR changes this behaviour so the datasource is rendered only if grafana is enabled AND the datasources sidecar is enabled.

@codecov-io
Copy link

Codecov Report

Merging #2086 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2086      +/-   ##
==========================================
- Coverage   61.24%   61.22%   -0.02%     
==========================================
  Files         146      146              
  Lines       11187    11187              
==========================================
- Hits         6851     6849       -2     
- Misses       3788     3789       +1     
- Partials      548      549       +1     
Impacted Files Coverage Δ
pkg/logql/evaluator.go 90.69% <0.00%> (-0.59%) ⬇️

@cyriltovena
Copy link
Contributor

Nice can you bump the patch version of the helm chart ?

@osela osela force-pushed the grafana-datasource-condition branch from 9b27e7d to bdcea18 Compare May 28, 2020 14:04
@osela
Copy link
Contributor Author

osela commented May 28, 2020

@cyriltovena done

@codecov-commenter
Copy link

Codecov Report

Merging #2086 into master will increase coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2086      +/-   ##
==========================================
+ Coverage   61.43%   61.52%   +0.09%     
==========================================
  Files         149      149              
  Lines       11935    11935              
==========================================
+ Hits         7332     7343      +11     
+ Misses       4017     4010       -7     
+ Partials      586      582       -4     
Impacted Files Coverage Δ
pkg/logql/evaluator.go 91.46% <0.00%> (+0.55%) ⬆️
pkg/promtail/targets/filetarget.go 70.48% <0.00%> (+1.80%) ⬆️
pkg/querier/queryrange/downstreamer.go 97.93% <0.00%> (+2.06%) ⬆️
pkg/promtail/targets/tailer.go 78.40% <0.00%> (+4.54%) ⬆️

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

Lgtm

@cyriltovena cyriltovena merged commit 8f9e456 into grafana:master May 29, 2020
@gclawes
Copy link

gclawes commented Jun 4, 2020

This breaks usage of loki-stack when installing grafana using prometheus-operator.

Edit: opened issue #2186

cyriltovena added a commit to cyriltovena/loki that referenced this pull request Jun 8, 2020
I didn't realize but this was a perfect ok use case to generate datasource when using a different grafana deployment.

Fixes grafana#2186
Fixes grafana#2195

Now regarding grafana#2086 it seems pretty easy to use `.Values.grafana.sidecar.datasources.enabled` if you don't want any datasources.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
cyriltovena added a commit that referenced this pull request Jun 22, 2020
I didn't realize but this was a perfect ok use case to generate datasource when using a different grafana deployment.

Fixes #2186
Fixes #2195

Now regarding #2086 it seems pretty easy to use `.Values.grafana.sidecar.datasources.enabled` if you don't want any datasources.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
torstenwalter pushed a commit to torstenwalter/grafana-helm-charts that referenced this pull request Oct 3, 2020
I didn't realize but this was a perfect ok use case to generate datasource when using a different grafana deployment.

Fixes grafana#2186
Fixes grafana#2195

Now regarding grafana/loki#2086 it seems pretty easy to use `.Values.grafana.sidecar.datasources.enabled` if you don't want any datasources.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants