fix comments

Author: 玖宇

doc/features/failure-handling.md

@@ -58,9 +58,9 @@ spec:
                 image: nginx:latest
 ```
 
-## Excluding Components from Restart Policy Trigger (CustomComponentsPattern)
+## Excluding Components from Restart Policy Trigger
 
-When using `customComponentsPattern`, you can prevent specific components from triggering the role's restart policy by setting an annotation on the component's pod template. This is useful for auxiliary components (e.g., monitoring, logging sidecars) whose failures should not affect the main workload.
+You can prevent specific pods from triggering the role's restart policy by setting an annotation on the pod template. This is useful for auxiliary components (e.g., monitoring, logging sidecars) whose failures should not affect the main workload. The annotation works with any deployment pattern (standalonePattern, leaderWorkerPattern, or customComponentsPattern).
 
 Set the annotation `rbg.workloads.x-k8s.io/restart-trigger-policy: "Ignore"` in the component's `template.metadata.annotations`:
 
@@ -109,9 +109,9 @@ When this annotation is set to "Ignore" on a component's pod template:
 - **RecreateRBGOnPodRestart**: Gateway/router roles that require all downstream services to be healthy.
 - **RecreateRoleInstanceOnPodRestart**: Worker roles that can tolerate individual instance failures.
 - **None**: Monitoring/logging sidecars that don't affect the main workload.
-- **ignoreRestartPolicy**: Auxiliary components within customComponentsPattern that should be isolated from restart policy actions.
+- **restart-trigger-policy: "Ignore"** (annotation): Auxiliary components whose pod failures should not trigger the role's restart policy actions.
 
 ## Examples
 
 - [Restart Policy Examples](../../examples/basic/rbg/restart-policy/restart-policy.yaml)
-- [CustomComponentsPattern with ignoreRestartPolicy](../../examples/basic/rbg/patterns/custom-components-pattern.yaml)
+- [CustomComponentsPattern with restart-trigger-policy annotation](../../examples/basic/rbg/patterns/custom-components-pattern.yaml)

examples/basic/rbg/patterns/custom-components-pattern.yaml

@@ -5,10 +5,11 @@
 #
 # Annotation rbg.workloads.x-k8s.io/restart-trigger-policy:
 # Valid values are "Inherit" (or empty) and "Ignore". When set to "Ignore" on a
-# component's pod template, this component's pod restart/delete events will NOT
-# trigger the role's restart policy (RecreateRoleInstanceOnPodRestart or
-# RecreateRBGOnPodRestart). This is useful for auxiliary components (e.g.,
-# monitoring, logging sidecars) whose failures should not affect the main workload.
+# pod template, pod restart/delete events from that pod will NOT trigger the
+# role's restart policy (RecreateRoleInstanceOnPodRestart or RecreateRBGOnPodRestart).
+# This annotation works with any deployment pattern and is useful for auxiliary
+# components (e.g., monitoring, logging sidecars) whose failures should not
+# affect the main workload.
 apiVersion: workloads.x-k8s.io/v1alpha2
 kind: RoleBasedGroup
 metadata:

internal/controller/workloads/pod_controller_test.go

@@ -632,6 +632,61 @@ func TestPodReconciler_podToRBG_EdgeCases(t *testing.T) {
 				).Obj(),
 			want: []reconcile.Request{},
 		},
+		{
+			name: "Pod with restart-trigger-policy Ignore annotation should not trigger restart",
+			args: args{
+				ctx: context.TODO(),
+				pod: func() *corev1.Pod {
+					pod := wrappers.BuildDeletingPod().WithLabels(
+						map[string]string{
+							constants.GroupNameLabelKey: "test-rbg",
+							constants.RoleNameLabelKey:  "test-role",
+						},
+					).Obj()
+					pod.Annotations = map[string]string{
+						constants.RestartTriggerPolicyAnnotationKey: constants.RestartTriggerPolicyIgnore,
+					}
+					return pod
+				}(),
+			},
+			setupRBG: wrappersv2.BuildBasicRoleBasedGroup("test-rbg", "default").
+				WithRoles(
+					[]workloadsv1alpha2.RoleSpec{
+						wrappersv2.BuildStandaloneRole("test-role").
+							WithRestartPolicy(workloadsv1alpha2.RecreateRBGOnPodRestart).
+							Obj(),
+					},
+				).Obj(),
+			want: []reconcile.Request{},
+		},
+		{
+			name: "Pod without Ignore annotation triggers RecreateRBGOnPodRestart",
+			args: args{
+				ctx: context.TODO(),
+				pod: wrappers.BuildDeletingPod().WithLabels(
+					map[string]string{
+						constants.GroupNameLabelKey: "test-rbg",
+						constants.RoleNameLabelKey:  "test-role",
+					},
+				).Obj(),
+			},
+			setupRBG: wrappersv2.BuildBasicRoleBasedGroup("test-rbg", "default").
+				WithRoles(
+					[]workloadsv1alpha2.RoleSpec{
+						wrappersv2.BuildStandaloneRole("test-role").
+							WithRestartPolicy(workloadsv1alpha2.RecreateRBGOnPodRestart).
+							Obj(),
+					},
+				).Obj(),
+			want: []reconcile.Request{
+				{
+					NamespacedName: types.NamespacedName{
+						Name:      "test-rbg",
+						Namespace: "default",
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {

pkg/reconciler/roleinstance/sync/instance_scale.go

@@ -28,6 +28,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/rbgs/api/workloads/constants"
 	portallocator "sigs.k8s.io/rbgs/pkg/port-allocator"
+	"sigs.k8s.io/rbgs/pkg/utils"
 
 	workloadsv1alpha2 "sigs.k8s.io/rbgs/api/workloads/v1alpha2"
 	podinplace "sigs.k8s.io/rbgs/pkg/inplace/pod"
@@ -213,22 +214,18 @@ func (c *realControl) createOnePod(ctx context.Context, instance *workloadsv1alp
 	return nil
 }
 
-// shouldRecreateInstance checks if the instance should be recreated (all pods deleted then recreated).
-// This applies when:
-// 1. restartPolicy = RecreateRoleInstanceOnPodRestart AND Pod Failed
-//   - Instead of replacement Pod, recreate entire Instance for consistency
+// shouldRecreateInstance checks if the instance should be recreated based on RestartPolicy.
+// When RestartPolicy is RecreateInstanceOnPodRestart, the instance should be recreated if:
+// 1. Any container has restarted (RestartCount > 0)
+// 2. Any pod has been deleted (only when Instance was previously Ready)
 //
-// Note: This function does NOT handle container restart (Dimension 1).
-// Container restart with RecreateRoleInstanceOnPodRestart is handled by LWS controller,
-// RBG controller does nothing in that case.
+// Pods with the restart-trigger-policy annotation set to "Ignore" are excluded from
+// both checks — their restarts and deletions will not trigger instance recreation.
 //
-// For restartPolicy=None or RecreateRBGOnPodRestart, Pod Failed triggers replacement Pod
-// creation through normal reconciliation (GetActiveAndInactivePods → createPods).
-//
-// Per KEP Non-Goals: Succeeded pods are NOT handled here - they represent normal completion
-// and should not trigger Instance recreation.
+// Note: We use Instance's Ready condition to distinguish between initial creation/scaling
+// and pod deletion. This avoids infinite recreation loops.
 func shouldRecreateInstance(instance *workloadsv1alpha2.RoleInstance, pods []*v1.Pod) bool {
-	// Only apply when restartPolicy is RecreateRoleInstanceOnPodRestart
+	// Only check when RestartPolicy is RecreateInstanceOnPodRestart
 	if instance.Spec.RestartPolicy != workloadsv1alpha2.RoleInstanceRestartPolicyRecreateOnPodRestart {
 		return false
 	}
@@ -238,27 +235,63 @@ func shouldRecreateInstance(instance *workloadsv1alpha2.RoleInstance, pods []*v1
 		return false
 	}
 
-	// Check if any Pod has become Failed (Dimension 2)
-	// With RecreateRoleInstanceOnPodRestart policy, Pod Failed triggers Instance recreation
-	// (instead of just replacement Pod)
-	//
-	// Per KEP Non-Goals: Succeeded pods are explicitly excluded - only Failed pods trigger recreation.
-	// Pod being deleted (with DeletionTimestamp) is also excluded as it's handled separately.
+	// Check each pod for container restart, skipping pods with Ignore annotation
+	for _, pod := range pods {
+		if hasTriggerPolicyIgnore(pod) {
+			continue
+		}
+		if utils.ContainerRestarted(pod) {
+			return true
+		}
+	}
+
+	// Check if any pod has been deleted:
+	// Only trigger recreate if Instance was previously Ready (stable state)
+	// and spec is not being changed (Generation == ObservedGeneration).
+	// This avoids triggering recreate during initial creation or scaling up.
 	if wasInstanceReady(instance) && instance.Generation == instance.Status.ObservedGeneration {
-		// Check if any Pod is Failed (excluding Succeeded and pods being deleted)
+		expectedPodCount := getExpectedPodCount(instance)
+		ignoredExpectedCount := getIgnoredExpectedPodCount(instance)
+		// Calculate active pods (exclude those being deleted and those with Ignore annotation).
+		// Pods with DeletionTimestamp != nil are in Terminating state and should not
+		// be counted as active, otherwise the controller may fail to detect pod deletion.
+		activeCount := 0
 		for _, p := range pods {
-			// Only Failed pods trigger Instance recreation
-			// Succeeded pods (normal completion) are explicitly excluded per KEP Non-Goals
-			// Pods being deleted are excluded as they're handled through normal deletion flow
-			if p.Status.Phase == v1.PodFailed && p.DeletionTimestamp == nil {
-				return true
+			if hasTriggerPolicyIgnore(p) {
+				continue
 			}
+			if p.DeletionTimestamp == nil {
+				activeCount++
+			}
+		}
+		if activeCount < expectedPodCount-ignoredExpectedCount {
+			return true
 		}
 	}
 
 	return false
 }
 
+// hasTriggerPolicyIgnore returns true if the pod has the restart-trigger-policy
+// annotation set to "Ignore".
+func hasTriggerPolicyIgnore(pod *v1.Pod) bool {
+	return pod.Annotations[constants.RestartTriggerPolicyAnnotationKey] == constants.RestartTriggerPolicyIgnore
+}
+
+// getIgnoredExpectedPodCount calculates the expected pod count from components
+// whose templates have the restart-trigger-policy annotation set to "Ignore".
+func getIgnoredExpectedPodCount(instance *workloadsv1alpha2.RoleInstance) int {
+	count := 0
+	for _, component := range instance.Spec.Components {
+		if component.Template.Annotations[constants.RestartTriggerPolicyAnnotationKey] == constants.RestartTriggerPolicyIgnore {
+			if component.Size != nil {
+				count += int(*component.Size)
+			}
+		}
+	}
+	return count
+}
+
 // wasInstanceReady checks if the Instance was previously in Ready state
 func wasInstanceReady(instance *workloadsv1alpha2.RoleInstance) bool {
 	for _, cond := range instance.Status.Conditions {