Skip to content

fix: Add value to helm chart to configure single binary service type - #13028

Closed
mkunkel wants to merge 1 commit into
grafana:mainfrom
mkunkel:singlebinary-service-type
Closed

fix: Add value to helm chart to configure single binary service type#13028
mkunkel wants to merge 1 commit into
grafana:mainfrom
mkunkel:singlebinary-service-type

Conversation

@mkunkel

@mkunkel mkunkel commented May 23, 2024 •
edited
Loading

Copy link
Copy Markdown

What this PR does / why we need it:
Allows the single binary Service type to be configured via values.yaml. In the case of an Ingress that relies on NodePort Services, the Ingress will not be able to reconcile, which can cause problems for other workloads sharing the Ingress group.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
N/A

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
  • 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
  • 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
@mkunkel
mkunkel requested a review from a team as a code owner May 23, 2024 20:23
@CLAassistant

CLAassistant commented May 23, 2024 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.
@github-actions github-actions Bot added area/helm type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories labels May 23, 2024
@mkunkel
mkunkel force-pushed the singlebinary-service-type branch 2 times, most recently from e5cfcfb to 15f34b7 Compare May 23, 2024 20:36
@mkunkel
mkunkel force-pushed the singlebinary-service-type branch from 15f34b7 to 6c8197e Compare May 24, 2024 14:14
@mkunkel

mkunkel commented May 29, 2024

Copy link
Copy Markdown
Author

I'm unsure of how to proceed here. I'm happy to resolve conflicts as needed. This is something that is blocking our implementation, so it would be good to know if this can be merged soon or if we need to fork until this is resolved.
jkroepke
jkroepke reviewed Jul 17, 2025
View reviewed changes

@jkroepke jkroepke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi, thanks for your PR.

please take not of my comments, change the PR title to fix(helm) and update the PR.
Comment thread pkg/bloombuild/planner/task.go
)

// TODO: Extract this definiton to a proto file at pkg/bloombuild/protos/protos.proto
// TODO: Extract this definition to a proto file at pkg/bloombuild/protos/protos.proto

@jkroepke jkroepke Jul 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please revert this
Comment thread production/helm/loki/CHANGELOG.md
## 6.5.2

- [BUGFIX] Fixed Ingress routing for all deployment modes.
- [BUGFIX] Fixed Ingress routing for all deployment modes.

@jkroepke jkroepke Jul 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [BUGFIX] Fixed Ingress routing for all deployment modes.
- [BUGFIX] Fixed Ingress routing for all deployment modes.

@mkunkel mkunkel Aug 27, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I apologize for missing the notification on this, and thank you for taking it across the line. I'm curious about this particular request, though. This would have been due to my editor removing trailing whitespace on save. Why would it be desirable to keep this trailing whitespace?
Comment thread production/helm/loki/CHANGELOG.md
## 6.6.1

- [BUGFIX] Fix query scheduler http-metrics targetPort
- [BUGFIX] Added helm chart value `singleBinary.service.type` to allow configuration of the single binary Service type

@jkroepke jkroepke Jul 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please add this on the top of the list
@jkroepke jkroepke mentioned this pull request Aug 3, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/helm size/S type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories

3 participants

@mkunkel @CLAassistant @jkroepke