Skip to content

Conversation

@rogly
Copy link
Contributor

@rogly rogly commented May 2, 2025

What this PR does / why we need it:

  • Sets ui.enabled to True to Loki ConfigMap when loki.ui.enabled is set to True to enable UI on pods deployed by the chart.

Which issue(s) this PR fixes:
Fixes #17561

Special notes for your reviewer:

Checklist

  • [ ✅] Reviewed the CONTRIBUTING.md guide (required)
  • [✅ ] Documentation added
  • [ ❌] Tests updated
  • [ ✅] Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • [ ❌] Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • [❌ ] If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@rogly rogly requested a review from a team as a code owner May 2, 2025 19:56
@CLAassistant
Copy link

CLAassistant commented May 2, 2025

CLA assistant check
All committers have signed the CLA.

@JStickler
Copy link
Contributor

The Loki UI is very experimental. I'm not sure that we want to update the Helm chart to support it yet.

@rogly
Copy link
Contributor Author

rogly commented May 8, 2025

The Loki UI is very experimental. I'm not sure that we want to update the Helm chart to support it yet.

I guess why include it in the values file as an option then? - my point is that the current chart does not function as expected so either the UI flag should be removed from values, or it should be fixed.

rogly and others added 2 commits May 10, 2025 13:45
sure I suppose that works as well

Co-authored-by: Thomas Decaux <[email protected]>
Signed-off-by: Rogly <[email protected]>
@Jayclifford345
Copy link
Contributor

Fair point @rogly, would you mind resolving the conflict in the change log? I have kicked off the automatic tests. If they pass we can get this merged

@rogly
Copy link
Contributor Author

rogly commented May 29, 2025

Fair point @rogly, would you mind resolving the conflict in the change log? I have kicked off the automatic tests. If they pass we can get this merged

@Jayclifford345 Conflict should be resolved now.

@Jayclifford345 Jayclifford345 enabled auto-merge (squash) June 3, 2025 10:34
@Jayclifford345
Copy link
Contributor

Sorry for the late reply on this one it missed my inbox. I have enabled auto-merge once all the tests pass

@Jayclifford345
Copy link
Contributor

Hey sorry @rogly, please run

make helm-docs

At the top of the loki repo to update the docs

@Jayclifford345 Jayclifford345 merged commit 917a60c into grafana:main Jun 3, 2025
73 of 74 checks passed
@rogly rogly deleted the helm-fix-ui-config branch June 6, 2025 14:30
ianhomer added a commit to adaptivekind/app-of-apps that referenced this pull request Jun 15, 2025
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.

Enabling UI in Helm values does not work

6 participants