From e897f462d8b66f405776b3c8d079ee67e1737d9d Mon Sep 17 00:00:00 2001 From: fracappa Date: Fri, 8 May 2026 17:48:58 +0200 Subject: [PATCH] fix: udpate healthcheck previous state only after successful operator status update Previously, getPacemakerStatus updated c.previous before updateOperatorStatus was called. If the status update failed, subsequent syncs would skip processing because c.previous already reflected the new CR timestamp, leaving PacemakerHealthCheckDegraded stuck a True indefinitely. --- pkg/tnf/pkg/pacemaker/healthcheck.go | 14 +++++++------- pkg/tnf/pkg/pacemaker/healthcheck_test.go | 3 +++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/tnf/pkg/pacemaker/healthcheck.go b/pkg/tnf/pkg/pacemaker/healthcheck.go index 684f9ee540..842b7bbb96 100644 --- a/pkg/tnf/pkg/pacemaker/healthcheck.go +++ b/pkg/tnf/pkg/pacemaker/healthcheck.go @@ -245,6 +245,13 @@ func (c *HealthCheck) sync(ctx context.Context, syncCtx factory.SyncContext) err c.recordHealthCheckEvents(currentStatus, previousStatus) + // Only mark as processed after status was successfully updated + if currentStatus.OverallStatus != statusUnknown { + c.previousMu.Lock() + c.previous = currentStatus + c.previousMu.Unlock() + } + return nil } @@ -324,13 +331,6 @@ func (c *HealthCheck) getPacemakerStatus(ctx context.Context) (*HealthStatus, *H currentStatus := c.buildHealthStatusFromCR(pacemakerCR) currentStatus.CRLastUpdated = crLastUpdated - // Only update previous for non-Unknown status (preserves last valid for grace period) - if currentStatus.OverallStatus != statusUnknown { - c.previousMu.Lock() - c.previous = currentStatus - c.previousMu.Unlock() - } - return currentStatus, previous, nil } diff --git a/pkg/tnf/pkg/pacemaker/healthcheck_test.go b/pkg/tnf/pkg/pacemaker/healthcheck_test.go index 3d87e7183a..e598aa3c4e 100644 --- a/pkg/tnf/pkg/pacemaker/healthcheck_test.go +++ b/pkg/tnf/pkg/pacemaker/healthcheck_test.go @@ -347,6 +347,9 @@ func TestHealthCheck_getPacemakerStatus_UnchangedTimestamp(t *testing.T) { require.NotNil(t, current1, "First call should return a status") require.Equal(t, statusHealthy, current1.OverallStatus, "First call should return healthy status") + // Simulate what sync() does after successful updateOperatorStatus + controller.previous = current1 + // Second call with same timestamp should return nil (no change) current2, _, err2 := controller.getPacemakerStatus(ctx) require.NoError(t, err2, "Second getPacemakerStatus should not return an error")