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: Grafana Dashboards only loaded when isSimpleScalable #10885

Closed
itay-grudev opened this issue Oct 12, 2023 · 0 comments · Fixed by #10902
Closed

Helm: Grafana Dashboards only loaded when isSimpleScalable #10885

itay-grudev opened this issue Oct 12, 2023 · 0 comments · Fixed by #10902
Labels
type/question

Comments

@itay-grudev
Copy link
Contributor

I was debugging why the default dashboards weren't loaded in a Loki installation and stumbled upon the following condition, which I don't really understand:

loki/production/helm/loki/templates/monitoring/dashboards/configmap-1.yaml

Lines 1 to 4 in a9b2dee

{{- $isSimpleScalable := eq (include "loki.deployment.isScalable" .) "true" -}}
{{- with .Values.monitoring.dashboards }}
{{- if and $isSimpleScalable .enabled }}
---

loki/production/helm/loki/templates/_helpers.tpl

Lines 49 to 54 in a9b2dee

{{/*
Return if deployment mode is simple scalable
*/}}
{{- define "loki.deployment.isScalable" -}}
{{- and (eq (include "loki.isUsingObjectStorage" . ) "true") (eq (int .Values.singleBinary.replicas) 0) }}
{{- end -}}

What is the reason Grafana Dashboard are only loaded when Loki's Backend storage is cloud based? Surely dashboards will work anyway.

If it's a mistake I can save you the trouble and submit a PR with the fix. If not - I'm sorry for wasting your time.
@itay-grudev itay-grudev mentioned this issue Oct 14, 2023
Merged
7 tasks
itay-grudev added a commit to itay-grudev/loki that referenced this issue Oct 16, 2023
@itay-grudev
Fixes: grafana#10885. Removed extra check preventing Grafana dashboar…
f2d59d0 
…ds from getting created.

Signed-off-by: Itay Grudev <itay.grudev@essentim.com>
JStickler added a commit that referenced this issue Oct 17, 2023
@itay-grudev @JStickler @MichelHollands
Helm: Removed extra check preventing Grafana dashboards from getting …
cfc4f0e 
…created (#10902)

**What this PR does / why we need it**:

Fixes a bug in the Loki Helm chart that prevents Grafana dashboards from
getting created when external storage (`s3/gcs/azure`) isn't used or
when in `singleBinary` mode.

With this change dashboard creation is controlled solely by the
`.Values.monitoring.dashboards.enabled` flag.

**Which issue(s) this PR fixes**:
Fixes #10885

**Special notes for your reviewer**:

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [x] Documentation added
- [ ] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [x] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](d10549e)

---------

Signed-off-by: Itay Grudev <itay.grudev@essentim.com>
Co-authored-by: J Stickler <julie.stickler@grafana.com>
Co-authored-by: Michel Hollands <42814411+MichelHollands@users.noreply.github.com>
rhnasc pushed a commit to inloco/loki that referenced this issue Apr 12, 2024
@itay-grudev @JStickler
@MichelHollands @rhnasc
Helm: Removed extra check preventing Grafana dashboards from getting …
b14a58d 
…created (grafana#10902)

**What this PR does / why we need it**:

Fixes a bug in the Loki Helm chart that prevents Grafana dashboards from
getting created when external storage (`s3/gcs/azure`) isn't used or
when in `singleBinary` mode.

With this change dashboard creation is controlled solely by the
`.Values.monitoring.dashboards.enabled` flag.

**Which issue(s) this PR fixes**:
Fixes grafana#10885

**Special notes for your reviewer**:

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [x] Documentation added
- [ ] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [x] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](grafana@d10549e)

---------

Signed-off-by: Itay Grudev <itay.grudev@essentim.com>
Co-authored-by: J Stickler <julie.stickler@grafana.com>
Co-authored-by: Michel Hollands <42814411+MichelHollands@users.noreply.github.com>
mraboosk pushed a commit to mraboosk/loki that referenced this issue Oct 7, 2024
@itay-grudev @JStickler @MichelHollands
Helm: Removed extra check preventing Grafana dashboards from getting …
e4891ea 
…created (grafana#10902)

**What this PR does / why we need it**:

Fixes a bug in the Loki Helm chart that prevents Grafana dashboards from
getting created when external storage (`s3/gcs/azure`) isn't used or
when in `singleBinary` mode.

With this change dashboard creation is controlled solely by the
`.Values.monitoring.dashboards.enabled` flag.

**Which issue(s) this PR fixes**:
Fixes grafana#10885

**Special notes for your reviewer**:

**Checklist**
- [x] Reviewed the
[`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md)
guide (**required**)
- [x] Documentation added
- [ ] Tests updated
- [ ] `CHANGELOG.md` updated
- [ ] If the change is worth mentioning in the release notes, add
`add-to-release-notes` label
- [ ] Changes that require user attention or interaction to upgrade are
documented in `docs/sources/setup/upgrade/_index.md`
- [x] For Helm chart changes bump the Helm chart version in
`production/helm/loki/Chart.yaml` and update
`production/helm/loki/CHANGELOG.md` and
`production/helm/loki/README.md`. [Example
PR](grafana@2cef71e)

---------

Signed-off-by: Itay Grudev <itay.grudev@essentim.com>
Co-authored-by: J Stickler <julie.stickler@grafana.com>
Co-authored-by: Michel Hollands <42814411+MichelHollands@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/question
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Helm: Removed extra check preventing Grafana dashboards from getting created itay-grudev/loki
2 participants
@itay-grudev @JStickler