Merge pull request #352 from aneeshkp/holdover-locked

OCPBUGS-41636: Maintain HOLDOVER for BC/OC when faulty interface returns until Clock is locked

plugins/ptp_operator/metrics/manager.go

@@ -392,7 +392,17 @@ func (p *PTPEventManager) GenPTPEvent(ptpProfileName string, oStats *stats.Stats
 				oStats.SetLastOffset(ptpOffset)
 			}
 		case ptp.HOLDOVER:
-			// do nothing, the timeout will switch holdover to FREE-RUN
+			// previous state was HOLDOVER, now it is in LOCKED state, cancel any HOLDOVER
+			if isOffsetInRange(ptpOffset, threshold.MaxOffsetThreshold, threshold.MinOffsetThreshold) {
+				log.Infof("interface %s is in LOCKED state, cancel any holdover states", eventResourceName)
+				threshold.SafeClose()
+				log.Infof(" publishing event for ( profile %s) %s with last state %s and current clock state %s and offset %d for ( Max/Min Threshold %d/%d )",
+					ptpProfileName, eventResourceName, lastClockState, clockState, ptpOffset, threshold.MaxOffsetThreshold, threshold.MinOffsetThreshold)
+				p.PublishEvent(clockState, ptpOffset, eventResourceName, eventType) // change to locked
+				oStats.SetLastSyncState(clockState)
+				oStats.SetLastOffset(ptpOffset)
+				oStats.AddValue(ptpOffset) // update off set when its in locked state and hold over only/ update off set when its in locked state and hold over only
+			} // else continue to stay in HOLDOVER
 		default: // not yet used states
 			log.Warnf("%s sync state %s, last ptp state is unknown: %s", eventResourceName, clockState, lastClockState)
 			if !isOffsetInRange(ptpOffset, threshold.MaxOffsetThreshold, threshold.MinOffsetThreshold) {
@@ -405,7 +415,8 @@ func (p *PTPEventManager) GenPTPEvent(ptpProfileName string, oStats *stats.Stats
 			oStats.SetLastOffset(ptpOffset)
 		}
 	case ptp.FREERUN:
-		if lastClockState != ptp.FREERUN { // within range
+		if lastClockState != ptp.HOLDOVER {
+			// within range
 			log.Infof(" publishing event for (profile %s) %s with last state %s and current clock state %s and offset %d for ( Max/Min Threshold %d/%d )",
 				ptpProfileName, eventResourceName, oStats.LastSyncState(), clockState, ptpOffset, threshold.MaxOffsetThreshold, threshold.MinOffsetThreshold)
 			p.PublishEvent(clockState, ptpOffset, eventResourceName, eventType) // change to locked

plugins/ptp_operator/metrics/manager_test.go

@@ -0,0 +1,107 @@
+package metrics_test
+
+import (
+	"testing"
+
+	ptpConfig "github.com/redhat-cne/cloud-event-proxy/plugins/ptp_operator/config"
+	"github.com/redhat-cne/cloud-event-proxy/plugins/ptp_operator/metrics"
+	"github.com/redhat-cne/cloud-event-proxy/plugins/ptp_operator/stats"
+	"github.com/redhat-cne/sdk-go/pkg/event/ptp"
+)
+
+func TestPTPEventManager_GenPTPEvent(t *testing.T) {
+	tests := []struct {
+		name              string
+		ptpProfileName    string
+		oStats            *stats.Stats
+		eventResourceName string
+		ptpOffset         int64
+		clockState        ptp.SyncState
+		lastClockState    ptp.SyncState
+		eventType         ptp.EventType
+		mock              bool
+		wantLastSyncState ptp.SyncState
+	}{
+		{
+			name:              "locked state within threshold",
+			ptpProfileName:    "profile1",
+			oStats:            stats.NewStats("profile1"),
+			lastClockState:    ptp.LOCKED,
+			eventResourceName: "resource1",
+			ptpOffset:         100,
+			clockState:        ptp.LOCKED,
+			eventType:         ptp.PtpStateChange,
+			mock:              true,
+			wantLastSyncState: ptp.LOCKED,
+		},
+		{
+			name:              "freerun state outside threshold",
+			ptpProfileName:    "profile2",
+			oStats:            stats.NewStats("profile2"),
+			eventResourceName: "resource2",
+			ptpOffset:         1000,
+			clockState:        ptp.FREERUN,
+			lastClockState:    ptp.LOCKED,
+			eventType:         ptp.PtpStateChange,
+			mock:              true,
+			wantLastSyncState: ptp.FREERUN,
+		},
+		{
+			name:              "freerun to Locked state",
+			ptpProfileName:    "profile1",
+			oStats:            stats.NewStats("profile1"),
+			lastClockState:    ptp.FREERUN,
+			eventResourceName: "resource1",
+			ptpOffset:         100,
+			clockState:        ptp.LOCKED,
+			eventType:         ptp.PtpStateChange,
+			mock:              true,
+			wantLastSyncState: ptp.LOCKED,
+		},
+		{
+			name:              "holdover to holdover state",
+			ptpProfileName:    "profile3",
+			oStats:            stats.NewStats("profile3"),
+			eventResourceName: "resource3",
+			ptpOffset:         500,
+			lastClockState:    ptp.HOLDOVER,
+			clockState:        ptp.FREERUN,
+			eventType:         ptp.PtpStateChange,
+			mock:              true,
+			wantLastSyncState: ptp.HOLDOVER,
+		},
+		{
+			name:              "holdover to locked state",
+			ptpProfileName:    "profile3",
+			oStats:            stats.NewStats("profile3"),
+			eventResourceName: "resource3",
+			ptpOffset:         50,
+			lastClockState:    ptp.HOLDOVER,
+			clockState:        ptp.LOCKED,
+			eventType:         ptp.PtpStateChange,
+			mock:              true,
+			wantLastSyncState: ptp.LOCKED,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.oStats.SetLastSyncState(tt.lastClockState)
+			p := &metrics.PTPEventManager{
+				PtpConfigMapUpdates: &ptpConfig.LinuxPTPConfigMapUpdate{
+					EventThreshold: map[string]*ptpConfig.PtpClockThreshold{
+						tt.ptpProfileName: {
+							MaxOffsetThreshold: 500,
+							MinOffsetThreshold: 10,
+						},
+					},
+				},
+			}
+			p.MockTest(tt.mock)
+			p.GenPTPEvent(tt.ptpProfileName, tt.oStats, tt.eventResourceName, tt.ptpOffset, tt.clockState, tt.eventType)
+			if got := tt.oStats.LastSyncState(); got != tt.wantLastSyncState {
+				t.Errorf("GenPTPEvent() = %v, want %v", got, tt.wantLastSyncState)
+			}
+		})
+	}
+}

plugins/ptp_operator/metrics/metrics_test.go

@@ -280,6 +280,7 @@ var testCases = []TestCase{
 		expectedNmeaStatus:             SKIP,
 		expectedPpsStatus:              SKIP,
 		expectedClockClassMetrics:      SKIP,
+		expectedEvent:                  ptp.PtpStateChange,
 	},
 	{
 		log:                            "gnss[1000000500]:[ts2phc.0.config] ens2f1 gnss_status 3 offset 5 s2",
@@ -341,6 +342,7 @@ var testCases = []TestCase{
 		expectedNmeaStatus:             SKIP,
 		expectedPpsStatus:              SKIP,
 		expectedClockClassMetrics:      SKIP,
+		expectedEvent:                  ptp.OsClockSyncStateChange,
 	},
 	{
 		log:                            "phc2sys[1000000710]: [ptp4l.0.config] CLOCK_REALTIME phc offset       -62 s0 freq  -78368 delay   1100",

plugins/ptp_operator/metrics/ptp4lParse.go

@@ -82,26 +82,12 @@ func (p *PTPEventManager) ParsePTP4l(processName, configName, profileName, outpu
 			// based on its last role  as slave port  we should make sure we report HOLDOVER event
 			if lastRole == types.FAULTY { // recovery
 				if role == types.SLAVE { // cancel any HOLDOVER timeout, if new role is slave
-					if masterOffsetSource == ptp4lProcessName {
-						if t, ok := p.PtpConfigMapUpdates.EventThreshold[profileName]; ok { // only if offset was reported by ptp4l process
-							log.Infof("interface %s is not anymore faulty, cancel any holdover states", ptpIFace)
-							t.SafeClose() // close any holdover go routines
-						}
-						alias := ptpStats[master].Alias()
-						if alias == "" {
-							alias = ptp4lCfg.GetAliasByInterface(ptpInterface) // this is default to any interface
-						}
-						masterResource := fmt.Sprintf("%s/%s", alias, MasterClockType)
-						p.GenPTPEvent(profileName, ptpStats[master], masterResource, FreeRunOffsetValue, ptp.FREERUN, ptp.PtpStateChange)
-						UpdateSyncStateMetrics(ptpStats[master].ProcessName(), alias, ptp.FREERUN)
-						// Send os clock sync state only if os clock is synced via phc
-						if t, ok := p.PtpConfigMapUpdates.PtpProcessOpts[profileName]; ok && t.Phc2SysEnabled() {
-							p.GenPTPEvent(profileName, ptpStats[ClockRealTime], ClockRealTime, FreeRunOffsetValue, ptp.FREERUN, ptp.OsClockSyncStateChange)
-							UpdateSyncStateMetrics(phc2sysProcessName, ClockRealTime, ptp.FREERUN)
-						} else {
-							log.Infof("phc2sys is not enabled for profile %s, skiping os clock syn state ", profileName)
-						}
+					// Do not cancel any HOLDOVER until holodover times out or PTP sync state is back in locked state
+					if masterOffsetSource == ptp4lProcessName && ptpStats[master].LastSyncState() == ptp.HOLDOVER {
+						log.Infof("Interface %s is no longer faulty. The holdover state will remain active until the PTP sync state is detected as LOCKED or the holdover times out.", ptpIFace)
+						// No events or metrics will  be generated instead will remain in Holdover state, when net iteration finds it in locked state
 					}
+
 					ptpStats[master].SetRole(role)
 				}
 			}
@@ -165,7 +151,7 @@ func handleHoldOverState(ptpManager *PTPEventManager,
 		log.Infof("time expired for interface %s", ptpIFace)
 		ptpStats := ptpManager.GetStats(types.ConfigName(configName))
 		if mStats, found := ptpStats[master]; found {
-			if mStats.LastSyncState() == ptp.HOLDOVER {
+			if mStats.LastSyncState() == ptp.HOLDOVER { // if it was still in holdover while timing out then switch to FREERUN
 				log.Infof("HOLDOVER timeout after %d secs,setting clock state to FREERUN from HOLDOVER state for %s",
 					holdoverTimeout, master)
 				masterResource := fmt.Sprintf("%s/%s", mStats.Alias(), MasterClockType)