Skip to content

Conversation

@jovial
Copy link
Contributor

@jovial jovial commented Dec 5, 2025

This variable needs to be defined on the prometheus group since that is where prometheus.yml is generated.

This variable needs to be defined on the prometheus-server group since that is where prometheus.yml is generated.
@jovial jovial requested a review from a team as a code owner December 5, 2025 18:24
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The change to move the blackbox exporter endpoint definitions to the prometheus-server group variables is correct, as this is where the Prometheus configuration is generated. This correctly fixes the issue described in the pull request.

However, during the review, I found a significant bug in how dynamic endpoints are generated and processed within this file. The current implementation leads to incorrect Prometheus scrape targets for several services. I've added detailed comments with suggestions to fix this. Addressing these issues will make the blackbox monitoring much more reliable.

@Alex-Welsh
Copy link
Member

@jovial could you re-target this at 2025.1 and add a release note?

@jovial
Copy link
Contributor Author

jovial commented Dec 8, 2025

@jovial could you re-target this at 2025.1 and add a release note?

2025.1 is OK because the code was moved the automatic blackbox generation was moved into kolla-ansible 🥳 . I can add a reno.

@Alex-Welsh
Copy link
Member

@jovial could you re-target this at 2025.1 and add a release note?

2025.1 is OK because the code was moved the automatic blackbox generation was moved into kolla-ansible 🥳 . I can add a reno.

Unfortunately, not everything was moved to KA. We still have the backend endpoints defined in SKC in 2025.1. The file still exists

There was a plan to move backend endpoints as well to KA, but we decided to try and use file-based service discovery in prometheus for it. That didn't work, so we implemented it downstream as a quick fix and no one's had time to upstream it yet.

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.

3 participants