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

Honor minio fullname #9565

Merged
merged 4 commits into from
Oct 31, 2024
Merged

Honor minio fullname #9565

merged 4 commits into from
Oct 31, 2024

Conversation

elsoa-invitech
Copy link
Contributor

What this PR does

the subchart full name override is not honored is referenced template files. (eg minio service name or configmap name)

Which issue(s) this PR fixes or relates to

Fixes #9564.

@elsoa-invitech elsoa-invitech requested a review from a team as a code owner October 8, 2024 11:47
@CLAassistant
Copy link

CLAassistant commented Oct 8, 2024

CLA assistant check
All committers have signed the CLA.

@elsoa-invitech
Copy link
Contributor Author

Solution from here: https://stackoverflow.com/a/69515809

@56quarters 56quarters added helm bug Something isn't working labels Oct 11, 2024
@armandgrillet armandgrillet requested a review from narqo October 25, 2024 14:29
@narqo
Copy link
Contributor

narqo commented Oct 25, 2024

The fix looks good to me. But I think there is one place in the changeset, that also needs fixing (refer to the in place comment).

Also, could you add a BUGFIX entry to the chart's CHANGELOG in the operations/helm/charts/mimir-distributed/CHANGELOG.md file.

@elsoa-invitech
Copy link
Contributor Author

anyway, what do you think: in values.yaml mimir config endpoint sould use {{ .Values.minio.service.port }} instead of 9000?

@narqo
Copy link
Contributor

narqo commented Oct 29, 2024

in values.yaml mimir config endpoint sould use {{ .Values.minio.service.port }} instead of 9000?

That's a great point, thank you. Fixing it should be a noop for the chart and its tests so if you could fix the values here, that'd be nice.

@elsoa-invitech
Copy link
Contributor Author

Changed the port to minio.service values

Copy link
Contributor

@narqo narqo left a comment

Choose a reason for hiding this comment

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

🔥 Looks good, thank you

@narqo narqo enabled auto-merge (squash) October 31, 2024 06:55
@narqo narqo merged commit a817319 into grafana:main Oct 31, 2024
31 checks passed
@elsoa-invitech elsoa-invitech deleted the fullname-fix branch December 19, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working helm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fullnameOverride mixing in helm subcharts
4 participants