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

fix(helm): fix query-frontend and ruler targetPort 'http-metrics' in Service template #13024

Merged

Conversation

archimeid
Copy link
Contributor

@archimeid archimeid commented May 23, 2024

What this PR does / why we need it:

This PR is a fix for Loki Helm Chart.

The templates for query-frontend-headless Service and ruler Service have an issue on "targetPort".

This issue is the same as the one detected for query-scheduler in version 6.6.1.

- name: http-metrics
   port: 3100
   targetPort: http
   protocol: TCP

targetPort is currently http but the port name in Deployment (or StatefulSet) template is http-metrics.

Template for query-frontend deployment

Template for ruler stateful-set

Thus, those Services have no endpoint available for the http-metrics port because of the mismatch.

Setting targetPort to http-metrics has solved the issue.

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

@archimeid archimeid requested a review from a team as a code owner May 23, 2024 13:58
@CLAassistant
Copy link

CLAassistant commented May 23, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@DylanGuedes DylanGuedes left a comment

Choose a reason for hiding this comment

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

could you also fix the ruler and QF?

@archimeid archimeid force-pushed the helm-fix-targetport-name-query-scheduler branch from f349444 to f4f2f31 Compare May 23, 2024 14:50
@archimeid
Copy link
Contributor Author

could you also fix the ruler and QF?

Done

@archimeid archimeid force-pushed the helm-fix-targetport-name-query-scheduler branch from f4f2f31 to e693d59 Compare May 23, 2024 14:52
@archimeid archimeid changed the title helm(query-scheduler): fix targetPort 'http-metrics' in Service template helm(query-scheduler,ruler,query-frontend): fix targetPort 'http-metrics' in Service template May 23, 2024
@DylanGuedes
Copy link
Contributor

thanks, this is fantastic ❤️

  • I have changed the PR title to fix the CI
  • could you also approve the CLA?

@DylanGuedes DylanGuedes changed the title helm(query-scheduler,ruler,query-frontend): fix targetPort 'http-metrics' in Service template fix: helm(query-scheduler,ruler,query-frontend): fix targetPort 'http-metrics' in Service template May 23, 2024
@archimeid
Copy link
Contributor Author

archimeid commented May 23, 2024

thanks, this is fantastic ❤️

* I have changed the PR title to fix the CI

* could you also approve the CLA?

Happy if I could help! 👍

I've already approved it several times but it didn't refresh in here.

image

@DylanGuedes
Copy link
Contributor

could you trigger a recheck by visiting https://cla-assistant.io/check/grafana/loki?pullRequest=13024?

@archimeid
Copy link
Contributor Author

I did many times but it does not work.

@archimeid
Copy link
Contributor Author

A fix seems to be merged for the same issue now apparently when trying to resolve conflicts with main branch.

@DylanGuedes
Copy link
Contributor

@archimeid a colleague just pointed out the fact that there are commits from this other account @vpo-cdc, it needs to have a CLA approval from him too

@archimeid archimeid force-pushed the helm-fix-targetport-name-query-scheduler branch from e693d59 to 7716d84 Compare May 27, 2024 07:34
@archimeid archimeid changed the title fix: helm(query-scheduler,ruler,query-frontend): fix targetPort 'http-metrics' in Service template fix(helm): fix query-frontend and ruler targetPort 'http-metrics' in Service template May 27, 2024
@archimeid
Copy link
Contributor Author

OMG, you're totally right. I have edited my commit. It was another user I am using at work.
As the issue was resolved for query-scheduler in 6.6.1 but not for query-frontend and ruler, I edited my commit to fix both and bump to 6.6.2.

@DylanGuedes DylanGuedes merged commit 1ab9d27 into grafana:main May 28, 2024
59 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants