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: expose metrics port using kubernetes_service_patch lib #332

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

DnPlas
Copy link
Contributor

@DnPlas DnPlas commented Feb 7, 2024

This commit ensures the metrics port is exposed in the Kubernetes Service for the jupyter-controller using the kubernetes_service_patch lib. This makes the metrics endpoint reachable from external prometheus scraper.
This commit also changes the unit tests slightly to adapt to the added service patcher.

Part of canonical/bundle-kubeflow#564

Testing

Please refer to the steps to reproduce in this comment, just deploying this app. After deploying this app and cos-lite, relations and dependencies, and waiting a couple minutes (10 min) none of the alerts should fire (for this app only).

TODO

  • refactor integration test case test_prometheus_grafana_integration
  • create a backport PR for track/ckf-1.8

This commit ensures the metrics port is exposed in the Kubernetes Service for the jupyter-controller
using the kubernetes_service_patch lib. This makes the metrics endpoint reachable from external
prometheus scraper.
This commit also changes the unit tests slightly to adapt to the added service patcher.

Part of canonical/bundle-kubeflow#564
The test_prometheus_grafana_integration test case was doing queries to prometheus
and checking the request returned successfully and that the application name and model
was listed correctly. To make this test case more accurately, we can add an assertion that
also checks that the unit is available, this way we avoid issues like the one described in
canonical/bundle-kubeflow#564.

Part of canonical/bundle-kubeflow#564
Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

LGTM, let me deploy and confirm this is working!

Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

Thank you @DnPlas.

@DnPlas DnPlas merged commit cac3b65 into main Feb 13, 2024
14 checks passed
@DnPlas DnPlas deleted the KF-1647-fix-alerts-firing branch February 13, 2024 12:29
DnPlas added a commit that referenced this pull request Feb 13, 2024
* fix: expose metrics port using kubernetes_service_patch lib

This commit ensures the metrics port is exposed in the Kubernetes Service for the jupyter-controller
using the kubernetes_service_patch lib. This makes the metrics endpoint reachable from external
prometheus scraper.
This commit also changes the unit tests slightly to adapt to the added service patcher.

Part of canonical/bundle-kubeflow#564

* tests: add an assertion for checking unit is available

The test_prometheus_grafana_integration test case was doing queries to prometheus
and checking the request returned successfully and that the application name and model
was listed correctly. To make this test case more accurately, we can add an assertion that
also checks that the unit is available, this way we avoid issues like the one described in
canonical/bundle-kubeflow#564.

Part of canonical/bundle-kubeflow#564
DnPlas added a commit that referenced this pull request Feb 14, 2024
)

* fix: expose metrics port using kubernetes_service_patch lib

This commit ensures the metrics port is exposed in the Kubernetes Service for the jupyter-controller
using the kubernetes_service_patch lib. This makes the metrics endpoint reachable from external
prometheus scraper.
This commit also changes the unit tests slightly to adapt to the added service patcher.

Part of canonical/bundle-kubeflow#564

* tests: add an assertion for checking unit is available

The test_prometheus_grafana_integration test case was doing queries to prometheus
and checking the request returned successfully and that the application name and model
was listed correctly. To make this test case more accurately, we can add an assertion that
also checks that the unit is available, this way we avoid issues like the one described in
canonical/bundle-kubeflow#564.

Part of canonical/bundle-kubeflow#564
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.

2 participants