feat(api): correlate failures by container name across replicas

README.md

@@ -70,6 +70,7 @@ Short name: `clp` (`kubectl get clp`).
 | `restartThreshold` | `10` | Number of container restarts before action |
 | `durationThreshold` | `30m` | How long a pod must have been continuously not ready before action. Go duration format, rejected by the API server if malformed |
 | `allReplicasFailing` | `true` | Require all replicas to be failing |
+| `failureCorrelation` | `Pod` | How replicas are compared when `allReplicasFailing` is true. `Container` additionally requires the same container name to be the failing one everywhere |
 | `targets` | `[Deployment, StatefulSet, CronJob]` | Workload types to act on. Only these three values are accepted |
 | `namespaceSelector` | `nil` | Label selector for namespaces to watch (nil = all) |
 | `excludeNamespaces` | `[kube-system, kube-public, kube-node-lease]` | Namespaces to ignore (applied after namespaceSelector) |
@@ -175,8 +176,9 @@ Restrictiveness is compared in this order, and the first difference decides:
 1. Lower `restartThreshold`
 2. Shorter `durationThreshold`
 3. `allReplicasFailing: false` before `true`, since it also acts on partial failure
-4. `dryRun: false` before `true`, so a real action outranks a simulated one
-5. Name in ascending order, purely to break a remaining tie
+4. `failureCorrelation: Pod` before `Container`, since it acts in more situations
+5. `dryRun: false` before `true`, so a real action outranks a simulated one
+6. Name in ascending order, purely to break a remaining tie
 
 Because the order is total, the winner does not depend on the order in which
 policies are evaluated.
@@ -289,6 +291,10 @@ work through the conditions the operator applies, in the order it applies them:
   in a loop accumulates it correctly rather than having the clock reset on
   every restart. A container that recovers and later fails again starts the
   clock over.
+- **`failureCorrelation: Container` and the replicas fail in different
+  containers.** The stricter mode holds off when the failures do not share a
+  container name, on the grounds that they look coincidental rather than
+  systematic. Switch back to `Pod` if you want any broken replica to count.
 - **`allReplicasFailing` is true and some replica is healthy.** This defaults to
   true, so a Deployment with one broken and one running pod is left alone by
   design. Set it to `false` if you want partial failure to count.

api/v1alpha1/crashlooppolicy_types.go

@@ -64,6 +64,21 @@ type CrashLoopPolicySpec struct {
 	// +optional
 	AllReplicasFailing *bool `json:"allReplicasFailing,omitempty"`
 
+	// FailureCorrelation controls how AllReplicasFailing compares replicas.
+	//
+	// Pod, the default, requires every replica to be failing in some way.
+	// Container additionally requires the same container name to be the
+	// failing one in every replica, which distinguishes a systematic problem
+	// such as a bad image or a missing Secret from unrelated failures that
+	// happen to coincide. It is the stricter setting and acts in fewer
+	// situations.
+	//
+	// Has no effect when AllReplicasFailing is false.
+	// +kubebuilder:default=Pod
+	// +kubebuilder:validation:Enum=Pod;Container
+	// +optional
+	FailureCorrelation string `json:"failureCorrelation,omitempty"`
+
 	// Targets lists workload types to act on.
 	// +kubebuilder:default={"Deployment","StatefulSet","CronJob"}
 	// +kubebuilder:validation:items:Enum=Deployment;StatefulSet;CronJob

charts/crashloop-operator/crds/crashloop-operator.lauger.de_crashlooppolicies.yaml

@@ -133,6 +133,23 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
+              failureCorrelation:
+                default: Pod
+                description: |-
+                  FailureCorrelation controls how AllReplicasFailing compares replicas.
+
+                  Pod, the default, requires every replica to be failing in some way.
+                  Container additionally requires the same container name to be the
+                  failing one in every replica, which distinguishes a systematic problem
+                  such as a bad image or a missing Secret from unrelated failures that
+                  happen to coincide. It is the stricter setting and acts in fewer
+                  situations.
+
+                  Has no effect when AllReplicasFailing is false.
+                enum:
+                - Pod
+                - Container
+                type: string
               namespaceSelector:
                 description: |-
                   NamespaceSelector selects namespaces by labels. If set, only pods in

config/crd/bases/crashloop-operator.lauger.de_crashlooppolicies.yaml

@@ -133,6 +133,23 @@ spec:
                     type: object
                 type: object
                 x-kubernetes-map-type: atomic
+              failureCorrelation:
+                default: Pod
+                description: |-
+                  FailureCorrelation controls how AllReplicasFailing compares replicas.
+
+                  Pod, the default, requires every replica to be failing in some way.
+                  Container additionally requires the same container name to be the
+                  failing one in every replica, which distinguishes a systematic problem
+                  such as a bad image or a missing Secret from unrelated failures that
+                  happen to coincide. It is the stricter setting and acts in fewer
+                  situations.
+
+                  Has no effect when AllReplicasFailing is false.
+                enum:
+                - Pod
+                - Container
+                type: string
               namespaceSelector:
                 description: |-
                   NamespaceSelector selects namespaces by labels. If set, only pods in

internal/controller/crashlooppolicy_controller_test.go

@@ -1493,3 +1493,92 @@ func TestScaleWorkloadToZero_DryRunTouchesNothing(t *testing.T) {
 		t.Errorf("dry run must not write annotations, got %v", updated.Annotations)
 	}
 }
+
+// newMultiContainerFailingPod builds a pod where exactly one named container is
+// in a watched waiting state and the other is healthy.
+func newMultiContainerFailingPod(name, failingContainer string) *corev1.Pod {
+	statusFor := func(cname string, failing bool) corev1.ContainerStatus {
+		cs := corev1.ContainerStatus{Name: cname, RestartCount: 12, Started: new(true)}
+		if failing {
+			cs.State = corev1.ContainerState{
+				Waiting: &corev1.ContainerStateWaiting{Reason: "CrashLoopBackOff"},
+			}
+		} else {
+			cs.Ready = true
+			cs.State = corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}
+		}
+		return cs
+	}
+	return &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            name,
+			Namespace:       testNamespace,
+			Labels:          map[string]string{"app": "my-app"},
+			OwnerReferences: []metav1.OwnerReference{rsOwnerRef()},
+		},
+		Status: corev1.PodStatus{
+			Phase: corev1.PodRunning,
+			Conditions: []corev1.PodCondition{{
+				Type:               corev1.ContainersReady,
+				Status:             corev1.ConditionFalse,
+				LastTransitionTime: metav1.NewTime(metav1.Now().Add(-2 * time.Hour)),
+			}},
+			ContainerStatuses: []corev1.ContainerStatus{
+				statusFor("app", failingContainer == "app"),
+				statusFor("sidecar", failingContainer == "sidecar"),
+			},
+		},
+	}
+}
+
+func TestReplicasAllFailing_Correlation(t *testing.T) {
+	sameContainer := []corev1.Pod{
+		*newMultiContainerFailingPod("p1", "app"),
+		*newMultiContainerFailingPod("p2", "app"),
+	}
+	differentContainers := []corev1.Pod{
+		*newMultiContainerFailingPod("p1", "app"),
+		*newMultiContainerFailingPod("p2", "sidecar"),
+	}
+
+	podMode := newCrashLoopPolicy("pod-mode")
+	containerMode := newCrashLoopPolicy("container-mode")
+	containerMode.Spec.FailureCorrelation = CorrelationContainer
+
+	tests := []struct {
+		name   string
+		policy *crashloopv1alpha1.CrashLoopPolicy
+		pods   []corev1.Pod
+		want   bool
+	}{
+		{"pod mode, same container", podMode, sameContainer, true},
+		// Every pod is broken, so pod correlation acts.
+		{"pod mode, different containers", podMode, differentContainers, true},
+		{"container mode, same container", containerMode, sameContainer, true},
+		// The failures do not share a container, so this looks coincidental
+		// rather than systematic and the stricter mode holds off.
+		{"container mode, different containers", containerMode, differentContainers, false},
+		{"no pods", podMode, nil, false},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := replicasAllFailing(tc.policy, tc.pods); got != tc.want {
+				t.Errorf("replicasAllFailing() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestIsMoreRestrictive_PodCorrelationBeatsContainer(t *testing.T) {
+	podMode := newCrashLoopPolicy("aaa")
+	containerMode := newCrashLoopPolicy("aaa")
+	containerMode.Spec.FailureCorrelation = CorrelationContainer
+
+	if !isMoreRestrictive(podMode, containerMode) {
+		t.Error("pod correlation acts in more situations and must rank as more restrictive")
+	}
+	if isMoreRestrictive(containerMode, podMode) {
+		t.Error("the ordering must not be symmetric")
+	}
+}

internal/controller/helpers.go

@@ -353,12 +353,7 @@ func allReplicasFailing(ctx context.Context, c client.Client, owner *ownerWorklo
 		if len(podList.Items) == 0 {
 			return false, nil
 		}
-		for i := range podList.Items {
-			if !podMatchesPolicy(policy, &podList.Items[i]) {
-				return false, nil
-			}
-		}
-		return true, nil
+		return replicasAllFailing(policy, podList.Items), nil
 
 	case "StatefulSet":
 		sts := &appsv1.StatefulSet{}
@@ -383,12 +378,7 @@ func allReplicasFailing(ctx context.Context, c client.Client, owner *ownerWorklo
 		if len(podList.Items) == 0 {
 			return false, nil
 		}
-		for i := range podList.Items {
-			if !podMatchesPolicy(policy, &podList.Items[i]) {
-				return false, nil
-			}
-		}
-		return true, nil
+		return replicasAllFailing(policy, podList.Items), nil
 
 	case "CronJob":
 		cj := &batchv1.CronJob{}
@@ -435,12 +425,7 @@ func allReplicasFailing(ctx context.Context, c client.Client, owner *ownerWorklo
 		if len(jobPods) == 0 {
 			return false, nil
 		}
-		for i := range jobPods {
-			if !podMatchesPolicy(policy, &jobPods[i]) {
-				return false, nil
-			}
-		}
-		return true, nil
+		return replicasAllFailing(policy, jobPods), nil
 	}
 	return false, nil
 }

internal/controller/policyresolve.go

@@ -65,6 +65,95 @@ func effectiveDryRun(p *crashloopv1alpha1.CrashLoopPolicy) bool {
 	return p.Spec.DryRun != nil && *p.Spec.DryRun
 }
 
+// FailureCorrelation modes.
+const (
+	CorrelationPod       = "Pod"
+	CorrelationContainer = "Container"
+)
+
+func effectiveFailureCorrelation(p *crashloopv1alpha1.CrashLoopPolicy) string {
+	if p.Spec.FailureCorrelation == CorrelationContainer {
+		return CorrelationContainer
+	}
+	return CorrelationPod
+}
+
+// failingContainerNames returns the names of the containers that make this pod
+// count as failing for the policy. Used to tell a systematic failure, the same
+// container broken everywhere, from unrelated ones that coincide.
+func failingContainerNames(p *crashloopv1alpha1.CrashLoopPolicy, pod *corev1.Pod) []string {
+	watchReasons := effectiveWatchReasons(p)
+	terminationReasons := effectiveTerminationReasons(p)
+	threshold := effectiveRestartThreshold(p)
+	window := effectiveRestartWindow(p)
+
+	var names []string
+	for _, statuses := range [][]corev1.ContainerStatus{
+		pod.Status.ContainerStatuses, pod.Status.InitContainerStatuses,
+	} {
+		for i := range statuses {
+			single := &corev1.Pod{
+				ObjectMeta: pod.ObjectMeta,
+				Spec:       pod.Spec,
+				Status: corev1.PodStatus{
+					Phase:             pod.Status.Phase,
+					ContainerStatuses: statuses[i : i+1],
+				},
+			}
+			if _, ok := podHasFailureReason(single, watchReasons); ok {
+				names = append(names, statuses[i].Name)
+				continue
+			}
+			if _, ok := podIsRestartLooping(single, terminationReasons, threshold, window); ok {
+				names = append(names, statuses[i].Name)
+			}
+		}
+	}
+	return names
+}
+
+// replicasAllFailing reports whether every replica counts as failing under the
+// policy's correlation mode.
+func replicasAllFailing(p *crashloopv1alpha1.CrashLoopPolicy, pods []corev1.Pod) bool {
+	if len(pods) == 0 {
+		return false
+	}
+	if effectiveFailureCorrelation(p) == CorrelationPod {
+		for i := range pods {
+			if !podMatchesPolicy(p, &pods[i]) {
+				return false
+			}
+		}
+		return true
+	}
+
+	// Container mode: some container name must be failing in every replica.
+	var shared map[string]struct{}
+	for i := range pods {
+		names := failingContainerNames(p, &pods[i])
+		if len(names) == 0 {
+			return false
+		}
+		current := make(map[string]struct{}, len(names))
+		for _, n := range names {
+			current[n] = struct{}{}
+		}
+		if shared == nil {
+			shared = current
+			continue
+		}
+		for n := range shared {
+			if _, ok := current[n]; !ok {
+				delete(shared, n)
+			}
+		}
+		if len(shared) == 0 {
+			return false
+		}
+	}
+	return len(shared) > 0
+}
+
 func effectiveTerminationReasons(p *crashloopv1alpha1.CrashLoopPolicy) []string {
 	return p.Spec.WatchTerminationReasons
 }
@@ -97,6 +186,7 @@ func podMatchesPolicy(p *crashloopv1alpha1.CrashLoopPolicy, pod *corev1.Pod) boo
 //  1. lower restartThreshold
 //  2. shorter durationThreshold
 //  3. allReplicasFailing false before true (acts on partial failure too)
+//     3a. pod correlation before container correlation (acts in more situations)
 //  4. dryRun false before true (a real action outranks a simulated one)
 //  5. name ascending, purely to break remaining ties
 func isMoreRestrictive(a, b *crashloopv1alpha1.CrashLoopPolicy) bool {
@@ -109,6 +199,11 @@ func isMoreRestrictive(a, b *crashloopv1alpha1.CrashLoopPolicy) bool {
 	if aa, ab := effectiveAllReplicasFailing(a), effectiveAllReplicasFailing(b); aa != ab {
 		return !aa
 	}
+	// Container correlation acts in fewer situations than pod correlation, so
+	// a policy using it is the less restrictive of the two.
+	if ca, cb := effectiveFailureCorrelation(a), effectiveFailureCorrelation(b); ca != cb {
+		return ca == CorrelationPod
+	}
 	if wa, wb := effectiveDryRun(a), effectiveDryRun(b); wa != wb {
 		return !wa
 	}