Enhance max capacity metrics - #308
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
a971495 to
8392a40
Compare
Kostov6
left a comment
There was a problem hiding this comment.
Initial review + a question:
Should we adapt the e2e test to check for those metrics if they are correctly calculated?
Maybe if it requires a lot of complexity this check is not worth it
| github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 // indirect | ||
| github.com/google/uuid v1.6.0 // indirect | ||
| github.com/json-iterator/go v1.1.12 // indirect | ||
| github.com/kylelemons/godebug v1.1.0 // indirect |
There was a problem hiding this comment.
Where does this come from?
There was a problem hiding this comment.
From my understanding the changes in metrics.go that use testutils helpers directly from github.com/prometheus/client_golang/prometheus/testutil (which requires godebug) trigger this change. Since the package transitively needs godebug and became part of the build graph, go mod tidy started recording it explicitly in go.mod with the // indirect marker, even though we already have the direct dependency.
You're right, it would be too complex - the e2e scripts spin up kind and check actual PVC resize behavior. Asserting metric values there would mean scraping the metrics endpoint from inside the cluster, adding polling/timing flakiness for a value the existing unit tests already cover. Not worth it to add the complexity (and slowing down of the e2e tests) imo. |
How to categorize this PR?
/area monitoring
/kind enhancement
What this PR does / why we need it:
Improves the max-capacity metrics and consolidates the "max capacity reached" logic:
max_capacity_reachedGauge that reports how many currently targeted PVCs sit at their configured max capacity. Unlike the existingmax_capacity_reached_totalcounter, this is a live snapshot that rises and falls as PVCs enter and leave the max-capacity state. It is reset to0when there are no PVCAs to reconcile.shouldResizePVC, so a PVC at max is detected up front and skipped cleanly (with theMaxCapacityReachedwarning event and resizing condition) instead of being handled insideresizePVC, where only actually resizable PVCs should be able to enter.max_capacity_reached_totalcounter so it increments once when a resize of a PVC leads to it reaching max capacity, rather than on every reconcile while the PVC continues to sit at max. Now it only increases if the max capacity is reached or if the max capacity was augmented and was reached again.Which issue(s) this PR fixes:
Part of #88
Special notes for your reviewer:
go.modpicks upgithub.com/kylelemons/godebugas an indirect dependency./cc @Kostov6 @plkokanov
Release note: