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 contour prom query for when service name is overwritten #1204

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

BrandonCate
Copy link
Contributor

Problem:
When the Canary.Spec.Service.Name field is used to override default generated name the corresponding prom queries fail to return anything during the canary analysis phase due to an incorrect envoy_cluster_name

Fix:
The envoy_cluster_name is based off of the service name and so the service name is used instead of the target name (targetRef.Name)

Note:
Unsure if this is a similar problem for all envoy based controllers

Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 🙇
Just have one small nitpick.

pkg/metrics/observers/contour_test.go Outdated Show resolved Hide resolved
pkg/metrics/observers/contour_test.go Outdated Show resolved Hide resolved
@aryan9600
Copy link
Member

I'd imagine this problem exists with all other providers that use Envoy, like Kuma, Gloo, etc. If you'd be willing to modify this PR to fix and test those as well, that'd be great 😄

@BrandonCate
Copy link
Contributor Author

@aryan9600 updated with suggestions
And can we open up a separate issue for the other providers that use envoy and work it from there? I don't mind doing it when I have spare time, but it will take me a little longer just because I'm less familiar with the other providers

Copy link
Member

@aryan9600 aryan9600 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 please squash all three commits into one with the message fix contour prom query when service name is specified. Thanks!

@BrandonCate
Copy link
Contributor Author

squashed

@aryan9600 aryan9600 merged commit 358391b into fluxcd:main Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants