Repository navigation
Add mechanism to be able query metrics from hypershift clusters - #195
Merged
Merged
Conversation
QuanMPhm
requested review from
KelvinLinBU and
marcoagonzales007
and removed request for
KelvinLinBU
October 13, 2026 15:57
Contributor
|
Added @marcoagonzales007 as a reviewer to give him some exposure to this repo. @marcoagonzales007 Feel free to learn the code and leave a review whenever you have time. |
QuanMPhm
reviewed
Oct 13, 2026
Comment on lines
+45
to
+48
| def test_cluster_id_with_special_characters(self): | ||
| cluster_id = "test-cluster-123_xyz" | ||
| query = QueryBuilder.build_cpu_request("hypershift", cluster_id) | ||
| assert f'_id="{cluster_id}"' in query |
Contributor
There was a problem hiding this comment.
Is this test nessecary? I suppose there's no harm but I'm not sure what the benefit is.
Contributor
Author
There was a problem hiding this comment.
not necessary, its just slop so I removed it.
Comment on lines
+11
to
+13
| if cluster_type == "hypershift": | ||
| base_query += f', _id="{cluster_id}"' | ||
| base_query += "}" |
Contributor
There was a problem hiding this comment.
This block is repeated 3 times in this file. Would it make sense to put this in a class function?
Since the script seems to expect to be ran with only a cluster type at a time, would it also make sense to make cluster_type a class argument instead an arguement to each "build_request" function?
naved001
force-pushed
the
hypershift-update
branch
from
October 21, 2026 20:30
b9bc943 to
9c92c70
Compare
With clusters deployed using hosted control plane (hypershift) the metrics kube_pod_resource_request now come from the hosted cluster where the query requires an additional parameter. This PR adds a mechanism to pass cluster_id to our queries. the queries kube_node_label and kube_pod_labels are ignored for now as they are still on the hosted cluster. it's easy to workaround those 2 since we can supply the information of kube_node_labels statically (we have already have a mechanism for that). Support for gathering kube_pod_labels will added in a different PR (we'll either query the other prometheus or wait for multi cluster observibility to have all the data in one place). This also drops support for kube_pod_status_unschedulable because 1. this will need to come from the hosted cluster and more importantly 2. it is just redundant as we already check for node != "" in our kube_pod_resource_requests query.
naved001
force-pushed
the
hypershift-update
branch
from
October 21, 2026 20:32
9c92c70 to
0042d06
Compare
Looking at the tests again I wanted them to use more realistic data so I updated them
QuanMPhm
approved these changes
Oct 22, 2026
knikolla
approved these changes
Oct 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With clusters deployed using hosted control plane (hypershift) the metrics kube_pod_resource_request now come from the hosted cluster where the query requires an additional parameter. This PR adds a mechanism to pass cluster_id to our queries.
the queries kube_node_label and kube_pod_labels are ignored for now as they are still on the hosted cluster. it's easy to workaround those 2 since we can supply the information of kube_node_labels statically (we have already have a mechanism for that). Support for gathering kube_pod_labels will added in a different PR (we'll either query the other prometheus or wait for multi cluster observibility to have all the data in one place).
This also drops support for kube_pod_status_unschedulable because 1. this will need to come from the hosted cluster and more importantly 2. it is just redundant as we already check for node != "" in our kube_pod_resource_requests query.
Disclaimer: I definitely used my brand new claude pro subscription lol