fix: initialize retry count at 1 and validate MaxRetryAttempts

Address CodeRabbit findings:
- Initialize RetryCount to 1 on first attempt (was 0)
- Update test expectations to match 1,2,3 sequence
- Add validation to reject negative MaxRetryAttempts
- Add tests for MaxRetryAttempts validation

The retry count now accurately reflects the number of remediation
attempts (first attempt = 1), and the config validation prevents
invalid negative values while preserving 0 as unlimited.

Signed-off-by: Billard <82095453+iacker@users.noreply.github.com>

Author: iacker

fault-remediation/pkg/annotation/annotation.go

@@ -106,7 +106,7 @@ func (m *NodeAnnotationManager) UpdateRemediationState(ctx context.Context, node
 
 		// Increment retry count if this group already exists
 		existingGroup, exists := state.EquivalenceGroups[group]
-		retryCount := 0
+		retryCount := 1
 		if exists {
 			retryCount = existingGroup.RetryCount + 1
 		}

fault-remediation/pkg/annotation/retry_count_test.go

@@ -40,29 +40,29 @@ func TestRetryCountIncrementsOnUpdate(t *testing.T) {
 	client := fake.NewClientBuilder().WithObjects(node).Build()
 	annotationManager := NodeAnnotationManager{client: client}
 
-	// First update - retry count should be 0
+	// First update - retry count should be 1
 	err := annotationManager.UpdateRemediationState(ctx, nodeName, groupName, "cr-1", "RESTART_BM")
 	require.NoError(t, err)
 
 	state, _, err := annotationManager.GetRemediationState(ctx, nodeName)
 	require.NoError(t, err)
-	assert.Equal(t, 0, state.EquivalenceGroups[groupName].RetryCount, "First attempt should have retry count 0")
+	assert.Equal(t, 1, state.EquivalenceGroups[groupName].RetryCount, "First attempt should have retry count 1")
 
-	// Second update - retry count should be 1
+	// Second update - retry count should be 2
 	err = annotationManager.UpdateRemediationState(ctx, nodeName, groupName, "cr-2", "RESTART_BM")
 	require.NoError(t, err)
 
 	state, _, err = annotationManager.GetRemediationState(ctx, nodeName)
 	require.NoError(t, err)
-	assert.Equal(t, 1, state.EquivalenceGroups[groupName].RetryCount, "Second attempt should have retry count 1")
+	assert.Equal(t, 2, state.EquivalenceGroups[groupName].RetryCount, "Second attempt should have retry count 2")
 
-	// Third update - retry count should be 2
+	// Third update - retry count should be 3
 	err = annotationManager.UpdateRemediationState(ctx, nodeName, groupName, "cr-3", "RESTART_BM")
 	require.NoError(t, err)
 
 	state, _, err = annotationManager.GetRemediationState(ctx, nodeName)
 	require.NoError(t, err)
-	assert.Equal(t, 2, state.EquivalenceGroups[groupName].RetryCount, "Third attempt should have retry count 2")
+	assert.Equal(t, 3, state.EquivalenceGroups[groupName].RetryCount, "Third attempt should have retry count 3")
 }
 
 func TestRetryCountIndependentPerGroup(t *testing.T) {
@@ -92,8 +92,8 @@ func TestRetryCountIndependentPerGroup(t *testing.T) {
 	state, _, err := annotationManager.GetRemediationState(ctx, nodeName)
 	require.NoError(t, err)
 
-	assert.Equal(t, 1, state.EquivalenceGroups["group-1"].RetryCount, "group-1 should have retry count 1")
-	assert.Equal(t, 0, state.EquivalenceGroups["group-2"].RetryCount, "group-2 should have retry count 0")
+	assert.Equal(t, 2, state.EquivalenceGroups["group-1"].RetryCount, "group-1 should have retry count 2")
+	assert.Equal(t, 1, state.EquivalenceGroups["group-2"].RetryCount, "group-2 should have retry count 1")
 }
 
 func TestRetryCountPersistsAcrossPodRestarts(t *testing.T) {
@@ -123,6 +123,6 @@ func TestRetryCountPersistsAcrossPodRestarts(t *testing.T) {
 	// Verify retry count persisted
 	state, _, err := manager2.GetRemediationState(ctx, nodeName)
 	require.NoError(t, err)
-	assert.Equal(t, 1, state.EquivalenceGroups[groupName].RetryCount,
+	assert.Equal(t, 2, state.EquivalenceGroups[groupName].RetryCount,
 		"Retry count should persist across pod restarts")
 }

fault-remediation/pkg/config/config.go

@@ -105,6 +105,10 @@ type TomlConfig struct {
 // surface their own error before any cross-reference error they may also
 // participate in.
 func (c *TomlConfig) Validate() error {
+	if c.MaxRetryAttempts < 0 {
+		return fmt.Errorf("maxRetryAttempts must be non-negative (got %d); use 0 for unlimited retries", c.MaxRetryAttempts)
+	}
+
 	if err := c.validateTemplate(); err != nil {
 		return err
 	}

fault-remediation/pkg/config/config_test.go

@@ -47,12 +47,37 @@ func TestTomlConfig_Validate(t *testing.T) {
 		{
 			name: "empty template mountPath should be rejected",
 			config: TomlConfig{
-				Template:          Template{MountPath: ""},
+				Template:           Template{MountPath: ""},
 				RemediationActions: map[string]MaintenanceResource{},
 			},
 			expectError: true,
 			errorSubstr: "template mountPath must be non-empty",
 		},
+		{
+			name: "negative maxRetryAttempts should be rejected",
+			config: TomlConfig{
+				Template:           Template{MountPath: tempDir},
+				MaxRetryAttempts:   -1,
+				RemediationActions: map[string]MaintenanceResource{},
+			},
+			expectError: true,
+			errorSubstr: "maxRetryAttempts must be non-negative",
+		},
+		{
+			name: "zero maxRetryAttempts should be accepted (unlimited)",
+			config: TomlConfig{
+				Template:         Template{MountPath: tempDir},
+				MaxRetryAttempts: 0,
+				RemediationActions: map[string]MaintenanceResource{
+					"ACTION_A": {
+						TemplateFileName: "template-a.yaml",
+						Scope:            "Cluster",
+						EquivalenceGroup: "restart",
+					},
+				},
+			},
+			expectError: false,
+		},
 		{
 			name: "valid config with matching templates",
 			config: TomlConfig{