[release-4.22] OCPBUGS-109638: Fix clock_state metric: HOLDOVER mapped to 0, stale when CEP enabled - #636
Conversation
|
@jzding: This pull request references Jira Issue OCPBUGS-95587, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jzding The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@jzding: This pull request references Jira Issue OCPBUGS-109638, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
Two issues with the openshift_ptp_clock_state Prometheus metric:
1. updateClockStateMetrics in metrics.go mapped HOLDOVER to 0 (same as
FREERUN), making the two states indistinguishable. The help text
declares "0 = FREERUN, 1 = LOCKED, 2 = HOLDOVER". Replace the
if/else with a switch using typed constants (ClockStateFreerun,
ClockStateLocked, ClockStateHoldover) defined in pkg/event and
shared by both metrics.go and event.go, matching the pattern in
cloud-event-proxy.
2. EventHandler.UpdateClockStateMetrics in event.go was guarded by
`if e.stdoutToSocket { return }`, skipping Prometheus updates when
cloud-event-proxy was enabled. Since CEP does not register its own
Prometheus metrics, this left the gauge stale. Remove the guard so
the metric is always updated regardless of CEP mode.
Fixes: OCPBUGS-95587
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jack Ding <jackding@gmail.com>
076a951 to
265aa58
Compare
|
/jira refresh |
|
@jzding: This pull request references Jira Issue OCPBUGS-109638, which is invalid:
Comment DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@jzding: This pull request references Jira Issue OCPBUGS-109638, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@openshift-ci-robot: GitHub didn't allow me to request PR reviews from the following users: dpopsuev. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions 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. |
|
@jzding: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Summary
Backport of upstream PR k8snetworkplumbingwg/linuxptp-daemon#250 to release-4.22.
Two fixes for the
openshift_ptp_clock_statePrometheus metric:HOLDOVER mapped to 0 instead of 2 —
updateClockStateMetricsonly distinguished LOCKED (1) from everything else (0). HOLDOVER was indistinguishable from FREERUN despite the help text declaring0 = FREERUN, 1 = LOCKED, 2 = HOLDOVER. Replaced the if/else with a switch using typed constants shared betweenmetrics.goandevent.go.Metric stale when CEP enabled —
EventHandler.UpdateClockStateMetricswas guarded byif e.stdoutToSocket { return }, skipping Prometheus updates when cloud-event-proxy was enabled. Since CEP does not register its ownopenshift_ptp_clock_stategauge, this left the metric stale. Removed the guard so the metric is always updated.Test plan
TestUpdateClockStateMetrics— verifies FREERUN→0, LOCKED→1, HOLDOVER→2go build ./...passes