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 openvsx-proxy metrics #11699

Merged
merged 1 commit into from
Jul 28, 2022
Merged

Fix openvsx-proxy metrics #11699

merged 1 commit into from
Jul 28, 2022

Conversation

vulkoingim
Copy link
Contributor

@vulkoingim vulkoingim commented Jul 27, 2022

Description

This adds the kube-rbac-proxy to the openvxs-proxy statefulset, exposing the metrics endpoint through it, so prometheus is able to scrape it correctly.

How to test

Before and after updating the address

Release Notes

NONE

Werft options:

  • /werft with-preview

@vulkoingim vulkoingim requested review from a team July 27, 2022 15:20
@github-actions github-actions bot added team: IDE team: delivery Issue belongs to the self-hosted team labels Jul 27, 2022
@vulkoingim vulkoingim requested a review from aledbf July 27, 2022 15:24
@aledbf
Copy link
Member

aledbf commented Jul 27, 2022

@vulkoingim Instead of this change, I would suggest to add the kube-rbac-proxy to the statefulset *common.KubeRBACProxyContainer(ctx)

Edit: the role bindings are already there

@vulkoingim vulkoingim force-pushed the aa/fix-openvsx-proxy-metrics branch from 2706efa to 15b557d Compare July 27, 2022 15:43
@@ -132,7 +132,7 @@ func statefulset(ctx *common.RenderContext) ([]runtime.Object, error) {
Name: "redis-data",
MountPath: "/data",
}},
},
}, *common.KubeRBACProxyContainer(ctx),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to understand how this works, we didn't have this param specified here before. How did this impact? Prometheus wasn't able to scrape the metrics at all? and why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heya, there was a bit of a longer PR description (guess you can still see it in the history), but trimmed it, since I changed the solution on advise of Alejandro.

Long-story short is that there was a breaking change introduced in: 14095f4 It changed the prometheus listen address from any (0.0.0.0:9500) to 127.0.0.1:9500. Listening on 127.0.0.1 doesn't work because it is not accessible from outside of the container.

The rest of the services in the cluster use kube-rbac-proxy to proxy the metrics endpoint, which this PR adds. The *common.KubeRBACProxyContainer(ctx) is just a function that generates the container spec and we add it to the list of containers of the StatefulSet.

There's a longer discussion on Slack about that, if you are interested: https://gitpod.slack.com/archives/C01KGM9EBD4/p1658907591252089

@roboquat roboquat merged commit f235d61 into main Jul 28, 2022
@roboquat roboquat deleted the aa/fix-openvsx-proxy-metrics branch July 28, 2022 09:00
@roboquat roboquat added the deployed: IDE IDE change is running in production label Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production release-note-none size/XS team: delivery Issue belongs to the self-hosted team team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants