From 265aa581648b917452071fb6c7675768832592ec Mon Sep 17 00:00:00 2001 From: Jack Ding Date: Mon, 24 Aug 2026 14:05:20 -0400 Subject: [PATCH] Fix clock_state metric: HOLDOVER mapped to 0, stale when CEP enabled 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) Signed-off-by: Jack Ding --- pkg/daemon/daemon_internal_test.go | 37 ++++++++++++++++++++++++++++++ pkg/daemon/metrics.go | 23 ++++++++++--------- pkg/event/event.go | 25 +++++++++++--------- 3 files changed, 63 insertions(+), 22 deletions(-) diff --git a/pkg/daemon/daemon_internal_test.go b/pkg/daemon/daemon_internal_test.go index 6f23ed77..92fe4d99 100644 --- a/pkg/daemon/daemon_internal_test.go +++ b/pkg/daemon/daemon_internal_test.go @@ -25,6 +25,8 @@ import ( "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/leap" ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1" ptpv2alpha1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v2alpha1" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" @@ -34,6 +36,10 @@ import ( const ( testPtp4lOffsetLine = "ptp4l[1.000]: [ptp4l.0.config] master offset 5 s2 freq -1000 path delay 100" testSkipStartupReason = "delayed" + testDUTLeadingIface = "ens2f0" + labelIface = "iface" + labelNode = "node" + labelProcess = "process" ) // vendor defaults are embedded; no filesystem setup needed @@ -3546,3 +3552,34 @@ func TestFindProcessesByName(t *testing.T) { procs = pm.findProcessesByName("nonexistent") assert.Equal(t, 0, len(procs)) } + +func TestUpdateClockStateMetrics(t *testing.T) { + origNodeName := NodeName + defer func() { NodeName = origNodeName }() + + NodeName = "test-node" + RegisterMetrics(NodeName) + ClockState.Reset() + + tests := []struct { + state string + expected float64 + }{ + {FREERUN, event.ClockStateFreerun}, + {LOCKED, event.ClockStateLocked}, + {HOLDOVER, event.ClockStateHoldover}, + } + + for _, tt := range tests { + t.Run(tt.state, func(t *testing.T) { + updateClockStateMetrics(ptp4lProcessName, testDUTLeadingIface, tt.state) + gauge, err := ClockState.GetMetricWith(prometheus.Labels{ + labelProcess: ptp4lProcessName, labelNode: NodeName, labelIface: testDUTLeadingIface, + }) + assert.NoError(t, err) + actual := testutil.ToFloat64(gauge) + assert.Equal(t, tt.expected, actual, + "clock_state for %s should be %v", tt.state, tt.expected) + }) + } +} diff --git a/pkg/daemon/metrics.go b/pkg/daemon/metrics.go index 53176989..1dc0cb93 100644 --- a/pkg/daemon/metrics.go +++ b/pkg/daemon/metrics.go @@ -11,6 +11,7 @@ import ( "time" "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/alias" + "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/event" "github.com/k8snetworkplumbingwg/linuxptp-daemon/pkg/synce" "github.com/golang/glog" @@ -48,12 +49,10 @@ const ( sys = "sys" ) +// Clock state string values used in log parsing and state transitions. const ( - //LOCKED ... - LOCKED string = "LOCKED" - //FREERUN ... - FREERUN = "FREERUN" - // HOLDOVER + LOCKED = "LOCKED" + FREERUN = "FREERUN" HOLDOVER = "HOLDOVER" ) @@ -529,12 +528,14 @@ func updateClockStateMetrics(process, iface string, state string) { return } glog.V(14).Infof("updateClockStateMetrics: process=%s iface=%s state=%s", process, iface, state) - if state == LOCKED { - ClockState.With(prometheus.Labels{ - "process": process, "node": NodeName, "iface": iface}).Set(1) - } else { - ClockState.With(prometheus.Labels{ - "process": process, "node": NodeName, "iface": iface}).Set(0) + labels := prometheus.Labels{"process": process, "node": NodeName, "iface": iface} + switch state { + case LOCKED: + ClockState.With(labels).Set(event.ClockStateLocked) + case HOLDOVER: + ClockState.With(labels).Set(event.ClockStateHoldover) + default: + ClockState.With(labels).Set(event.ClockStateFreerun) } } diff --git a/pkg/event/event.go b/pkg/event/event.go index 6ab7c475..dce91d80 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -150,6 +150,13 @@ const ( PTP_NOTSET PTPState = "-2" ) +// Clock state metric values for openshift_ptp_clock_state gauge. +const ( + ClockStateFreerun float64 = 0 + ClockStateLocked float64 = 1 + ClockStateHoldover float64 = 2 +) + const ( // socketDialTimeout is the maximum time to wait for a single dial attempt to the event socket. socketDialTimeout = 5 * time.Second @@ -1129,19 +1136,15 @@ func (e *EventHandler) UpdateClockStateMetrics(state PTPState, process, iFace st if !utils.CheckMetricSanity("ClockState", process, iFace) { return } - if e.stdoutToSocket { - return - } labels := prometheus.Labels{ "process": process, "node": e.nodeName, "iface": iFace} - if state == PTP_LOCKED { - e.clockMetric.With(labels).Set(1) - } else if state == PTP_FREERUN { - e.clockMetric.With(labels).Set(0) - } else if state == PTP_HOLDOVER { - e.clockMetric.With(labels).Set(2) - } else { - e.clockMetric.With(labels).Set(3) + switch state { + case PTP_LOCKED: + e.clockMetric.With(labels).Set(ClockStateLocked) + case PTP_HOLDOVER: + e.clockMetric.With(labels).Set(ClockStateHoldover) + default: + e.clockMetric.With(labels).Set(ClockStateFreerun) } }