From 9dec605f2a9c40a802dc7be77c60945d8d424f0e Mon Sep 17 00:00:00 2001 From: eunwoo song Date: Sun, 6 Sep 2026 18:43:30 +0900 Subject: [PATCH] fix(controller): requeue at duration threshold expiry --- README.md | 2 +- internal/controller/constants.go | 2 + .../controller/crashlooppolicy_controller.go | 26 ++++++++--- .../crashlooppolicy_controller_test.go | 45 +++++++++++++++++++ internal/controller/helpers.go | 16 ++++++- 5 files changed, 81 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index aaf558c..452f59c 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ Short name: `clp` (`kubectl get clp`). | `namespaceSelector` | `nil` | Label selector for namespaces to watch (nil = all) | | `excludeNamespaces` | `[kube-system, kube-public, kube-node-lease]` | Namespaces to ignore (applied after namespaceSelector) | | `excludeWorkloadSelector` | `nil` | Label selector to exclude matching workloads from scale-down | -| `reconcileInterval` | `60s` | How often the policy is evaluated. Same duration format as `durationThreshold` | +| `reconcileInterval` | `60s` | Maximum time between policy evaluations. A pod nearing `durationThreshold` is evaluated when that threshold expires. Same duration format as `durationThreshold` | | `dryRun` | `false` | Log actions without executing them | ### Status diff --git a/internal/controller/constants.go b/internal/controller/constants.go index b063ddc..340a5b9 100644 --- a/internal/controller/constants.go +++ b/internal/controller/constants.go @@ -5,6 +5,8 @@ import "time" // Requeue intervals for controller reconciliation loops. const ( RequeueIntervalDefault = 60 * time.Second + // Avoid hot-looping when a threshold expires while reconciliation is running. + minimumThresholdRequeue = time.Second ) // Default thresholds. These mirror the kubebuilder defaults on the CRD fields diff --git a/internal/controller/crashlooppolicy_controller.go b/internal/controller/crashlooppolicy_controller.go index aa74888..d7cd0ca 100644 --- a/internal/controller/crashlooppolicy_controller.go +++ b/internal/controller/crashlooppolicy_controller.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "strconv" + "time" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -79,6 +80,9 @@ func (r *CrashLoopPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Requ } durationThreshold := effectiveDurationThreshold(policy) + requeueAfter, _ := effectiveReconcileInterval(policy) + reconcileTime := time.Now() + var earliestThresholdExpiry time.Time watchReasons := effectiveWatchReasons(policy) restartThreshold := effectiveRestartThreshold(policy) targets := effectiveTargets(policy) @@ -146,9 +150,17 @@ func (r *CrashLoopPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Requ } if !waitingFailing { reason = loopReason - } else if !podExceedsRestartThreshold(pod, restartThreshold) && - !podExceedsDurationThreshold(pod, durationThreshold) && !looping { - continue + } else if !podExceedsRestartThreshold(pod, restartThreshold) && !looping { + remaining, known := durationThresholdRemaining(pod, durationThreshold, reconcileTime) + if !known || remaining > 0 { + if known { + expiresAt := reconcileTime.Add(remaining) + if earliestThresholdExpiry.IsZero() || expiresAt.Before(earliestThresholdExpiry) { + earliestThresholdExpiry = expiresAt + } + } + continue + } } // Resolve owner workload @@ -318,11 +330,11 @@ func (r *CrashLoopPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Requ }); err != nil { return ctrl.Result{}, err } + if !earliestThresholdExpiry.IsZero() { + requeueAfter = min(requeueAfter, thresholdRequeueAfter(time.Until(earliestThresholdExpiry))) + } - // Use per-policy reconcile interval if configured - requeueInterval, _ := effectiveReconcileInterval(policy) - - return ctrl.Result{RequeueAfter: requeueInterval}, nil + return ctrl.Result{RequeueAfter: requeueAfter}, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/internal/controller/crashlooppolicy_controller_test.go b/internal/controller/crashlooppolicy_controller_test.go index b7a4bea..88db57e 100644 --- a/internal/controller/crashlooppolicy_controller_test.go +++ b/internal/controller/crashlooppolicy_controller_test.go @@ -304,6 +304,51 @@ func TestReconcile_CustomReconcileInterval(t *testing.T) { } } +func TestReconcile_RequeuesAtPendingDurationThreshold(t *testing.T) { + policy := newCrashLoopPolicy("test-policy", + withRestartThreshold(20), + withDurationThreshold("1m"), + withReconcileInterval("30s"), + withAllReplicasFailing(false), + ) + deploy := newDeployment("my-app", testNamespace, 1) + rs := newReplicaSet("my-app-rs", testNamespace, "my-app") + pod := newFailingPod("my-app-pod-1", testNamespace, rsOwnerRef(), "CrashLoopBackOff", 5) + pod.Status.Conditions[0].LastTransitionTime = metav1.NewTime(time.Now().Add(-58 * time.Second)) + laterPod := newFailingPod("my-app-pod-2", testNamespace, rsOwnerRef(), "CrashLoopBackOff", 5) + laterPod.Status.Conditions[0].LastTransitionTime = metav1.NewTime(time.Now().Add(-40 * time.Second)) + + c := setupTestClient(policy, deploy, rs, pod, laterPod) + r := newReconciler(c) + + result, err := r.Reconcile(testCtx(), testRequest("test-policy")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.RequeueAfter < time.Second || result.RequeueAfter > 2*time.Second { + t.Errorf("expected requeue at the pending threshold in 1s..2s, got %v", result.RequeueAfter) + } +} + +func TestThresholdRequeueAfter(t *testing.T) { + tests := []struct { + name string + remaining time.Duration + want time.Duration + }{ + {name: "preserves later expiry", remaining: 2 * time.Second, want: 2 * time.Second}, + {name: "floors tiny expiry", remaining: 100 * time.Millisecond, want: time.Second}, + {name: "floors elapsed expiry", remaining: -100 * time.Millisecond, want: time.Second}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := thresholdRequeueAfter(tt.remaining); got != tt.want { + t.Errorf("thresholdRequeueAfter(%v) = %v, want %v", tt.remaining, got, tt.want) + } + }) + } +} + func TestPodHasFailureReason(t *testing.T) { tests := []struct { name string diff --git a/internal/controller/helpers.go b/internal/controller/helpers.go index 88586f6..891be6f 100644 --- a/internal/controller/helpers.go +++ b/internal/controller/helpers.go @@ -216,11 +216,23 @@ func podFailingSince(pod *corev1.Pod) (time.Time, bool) { // podExceedsDurationThreshold reports whether the pod has been failing for at // least the given duration. func podExceedsDurationThreshold(pod *corev1.Pod, duration time.Duration) bool { + remaining, ok := durationThresholdRemaining(pod, duration, time.Now()) + return ok && remaining <= 0 +} + +// durationThresholdRemaining returns the time until the pod reaches the given +// continuous-failure duration. The boolean is false when the failure start is +// not known yet. +func durationThresholdRemaining(pod *corev1.Pod, duration time.Duration, now time.Time) (time.Duration, bool) { failingSince, ok := podFailingSince(pod) if !ok { - return false + return 0, false } - return time.Since(failingSince) >= duration + return failingSince.Add(duration).Sub(now), true +} + +func thresholdRequeueAfter(remaining time.Duration) time.Duration { + return max(remaining, minimumThresholdRequeue) } // ownerWorkload represents a resolved top-level workload that owns a pod.