VPA updater: export admission controller status metric - #10182
Conversation
Signed-off-by: Luke Addison <lukeaddison785@gmail.com>
|
This issue is currently awaiting triage. If SIG Autoscaling contributors determines this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe updater now records invalid Admission Controller status events with separate reasons for validation errors and invalid status results. Tests cover metric recording and status-check error handling. ChangesAdmission Controller status metrics
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This PR adds a localized metric for monitoring VPA updater admission-controller status; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hi @dippynark. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dippynark 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 |
|
Is it possible to add a test for this? |
Signed-off-by: Luke Addison <lukeaddison785@gmail.com>
|
@adrianmoisey have added a couple covering the changed paths |
|
/ok-to-test |
|
Seems reasonable to me. /lgtm $ kubectl -n kube-system delete lease vpa-admission-controller
lease.coordination.k8s.io "vpa-admission-controller" deleted from kube-system namespace
$ kubectl -n kube-system port-forward deploy/vpa-updater 8943:8943
...
...
$ curl -s localhost:8943/metrics | grep admission_controller_status
# HELP vpa_updater_admission_controller_status_invalid_total Number of times Updater skipped its main loop because the Admission Controller status Lease was missing, stale or otherwise invalid.
# TYPE vpa_updater_admission_controller_status_invalid_total counter
vpa_updater_admission_controller_status_invalid_total{reason="error"} 1 |
|
/lgtm Thanks! |
What type of PR is this?
/kind feature
What this PR does / why we need it:
If VPA updater is stuck taking action due to the admission controller status check, there is currently no way of detecting and alerting on this.
This PR fixes this by adding a corresponding metric so that we can alert on the following after 10 minutes:
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Summary by CodeRabbit