From 9a4f13b76e8141fd968f990512c52657def1c25b Mon Sep 17 00:00:00 2001 From: Michalis Kargakis Date: Sat, 17 Oct 2026 21:39:24 +0200 Subject: [PATCH] Count a stand-in only once it is available, honouring the target's minReadySeconds --- docs/how-it-works.md | 9 +++-- internal/controller/evictionwebhook_test.go | 20 ++++++++++ internal/controller/understudy_controller.go | 12 +----- .../controller/understudy_controller_test.go | 14 +++++-- internal/signal/evictionwebhook/adapter.go | 8 ++-- internal/workload/target.go | 24 ++++++++++++ internal/workload/target_test.go | 38 +++++++++++++++++++ 7 files changed, 103 insertions(+), 22 deletions(-) diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 9660036..03a26ea 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -19,10 +19,11 @@ sentinel adapters normalize their observations into a `NodeDoom`. **3. Surge.** The controller increases the target workload's replica count and marks the old pod with a negative `controller.kubernetes.io/pod-deletion-cost`. -**4. Wait for traffic readiness.** A stand-in counts only when it is Ready, is -not deleting, and runs on neither the victim node nor another doomed node. -Readiness gates therefore include load balancer registration. Once enough -healthy stand-ins exist, the webhook admits the drainer's next retry. +**4. Wait for traffic readiness.** A stand-in counts only when it has been Ready +for the target's `minReadySeconds`, is not deleting, and runs on neither the +victim node nor another doomed node. Readiness gates therefore include load +balancer registration. Once enough healthy stand-ins exist, the webhook admits +the drainer's next retry. **5. Scale back.** After the old pod leaves, the workload returns to its original replica count, or the latest requested `baselineReplicas` when capacity diff --git a/internal/controller/evictionwebhook_test.go b/internal/controller/evictionwebhook_test.go index f043e4e..da24697 100644 --- a/internal/controller/evictionwebhook_test.go +++ b/internal/controller/evictionwebhook_test.go @@ -176,6 +176,26 @@ var _ = Describe("Eviction webhook adapter", func() { Eventually(func() error { return evict(f.pod, false) }).Should(Succeed()) }) + It("counts a stand-in only once it has been ready for the target's minReadySeconds", func() { + f := newFixture(nil, func(dep *appsv1.Deployment) { dep.Spec.MinReadySeconds = 30 }) + cordonNode(f.node) + expectSurged(f) + + By("holding while the stand-in has been ready for less than minReadySeconds") + clearNode := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: f.ns + "-clear"}} + Expect(k8sClient.Create(ctx, clearNode)).To(Succeed()) + standIn := makePod(f.ns, "lead-understudy", f.dep.Name, clearNode.Name, true) + Eventually(func() appsv1alpha1.Phase { return getCR(f).Status.Phase }).Should(Equal(appsv1alpha1.PhaseSurging)) + Consistently(func() bool { + return apierrors.IsTooManyRequests(evict(f.pod, false)) && getCR(f).Status.Phase == appsv1alpha1.PhaseSurging + }).Should(BeTrue()) + + By("releasing once the stand-in is available") + markPodReadySince(standIn, time.Now().Add(-31*time.Second)) + Eventually(func() appsv1alpha1.Phase { return getCR(f).Status.Phase }).Should(Equal(appsv1alpha1.PhaseReleasing)) + Eventually(func() error { return evict(f.pod, false) }).Should(Succeed()) + }) + It("never holds an unsupported target", func() { f := newFixture(func(cr *appsv1alpha1.Understudy) { cr.Spec.TargetRef.Kind = "StatefulSet" diff --git a/internal/controller/understudy_controller.go b/internal/controller/understudy_controller.go index 82a0318..e8fade7 100644 --- a/internal/controller/understudy_controller.go +++ b/internal/controller/understudy_controller.go @@ -205,6 +205,7 @@ func (r *UnderstudyReconciler) assessPods(ctx context.Context, cr *appsv1alpha1. return a, err } a.all = podList.Items + now := time.Now() for i := range podList.Items { pod := &podList.Items[i] if !serving.Matches(labels.Set(pod.Labels)) || pod.Spec.NodeName == "" || pod.DeletionTimestamp != nil { @@ -218,7 +219,7 @@ func (r *UnderstudyReconciler) assessPods(ctx context.Context, cr *appsv1alpha1. } continue } - if podReady(pod) { + if target.PodAvailable(pod, now) { a.viable++ } } @@ -493,15 +494,6 @@ func (r *UnderstudyReconciler) requestsInNamespace(ctx context.Context, namespac return reqs } -func podReady(pod *corev1.Pod) bool { - for _, c := range pod.Status.Conditions { - if c.Type == corev1.PodReady { - return c.Status == corev1.ConditionTrue - } - } - return false -} - func setCond(status *appsv1alpha1.UnderstudyStatus, cr *appsv1alpha1.Understudy, condType string, condStatus metav1.ConditionStatus, reason, msg string) { meta.SetStatusCondition(&status.Conditions, metav1.Condition{ diff --git a/internal/controller/understudy_controller_test.go b/internal/controller/understudy_controller_test.go index c505710..de3bb56 100644 --- a/internal/controller/understudy_controller_test.go +++ b/internal/controller/understudy_controller_test.go @@ -48,7 +48,7 @@ type fixture struct { cr *appsv1alpha1.Understudy } -func newFixture(mutateCR func(*appsv1alpha1.Understudy)) *fixture { +func newFixture(mutateCR func(*appsv1alpha1.Understudy), mutateDep ...func(*appsv1.Deployment)) *fixture { nsCounter++ f := &fixture{ns: fmt.Sprintf("test-%d-%d", GinkgoRandomSeed(), nsCounter)} @@ -60,6 +60,9 @@ func newFixture(mutateCR func(*appsv1alpha1.Understudy)) *fixture { Expect(k8sClient.Create(ctx, f.node)).To(Succeed()) f.dep = makeDeployment(f.ns, "lead", 1) + for _, mutate := range mutateDep { + mutate(f.dep) + } Expect(k8sClient.Create(ctx, f.dep)).To(Succeed()) settleDeployment(f.dep) @@ -144,6 +147,10 @@ func makePod(ns, name, app, node string, ready bool) *corev1.Pod { } func markPodReady(pod *corev1.Pod) { + markPodReadySince(pod, time.Now()) +} + +func markPodReadySince(pod *corev1.Pod, since time.Time) { Eventually(func() error { current := &corev1.Pod{} if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(pod), current); err != nil { @@ -151,8 +158,9 @@ func markPodReady(pod *corev1.Pod) { } current.Status.Phase = corev1.PodRunning current.Status.Conditions = []corev1.PodCondition{{ - Type: corev1.PodReady, - Status: corev1.ConditionTrue, + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.NewTime(since), }} return k8sClient.Status().Update(ctx, current) }).Should(Succeed()) diff --git a/internal/signal/evictionwebhook/adapter.go b/internal/signal/evictionwebhook/adapter.go index b0d7840..022ee95 100644 --- a/internal/signal/evictionwebhook/adapter.go +++ b/internal/signal/evictionwebhook/adapter.go @@ -176,6 +176,7 @@ func (a *Adapter) standInReady(ctx context.Context, target *workload.Target, vic return false } healthy := 0 + now := time.Now() for i := range pods.Items { p := &pods.Items[i] if p.DeletionTimestamp != nil || p.Spec.NodeName == "" { @@ -187,11 +188,8 @@ func (a *Adapter) standInReady(ctx context.Context, target *workload.Target, vic if _, doomed := a.registry.Lookup(p.Spec.NodeName); doomed { continue } - for _, c := range p.Status.Conditions { - if c.Type == corev1.PodReady && c.Status == corev1.ConditionTrue { - healthy++ - break - } + if target.PodAvailable(p, now) { + healthy++ } } return healthy >= int(base) diff --git a/internal/workload/target.go b/internal/workload/target.go index 11f235e..156cee8 100644 --- a/internal/workload/target.go +++ b/internal/workload/target.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "strconv" + "time" rolloutsv1alpha1 "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" appsv1 "k8s.io/api/apps/v1" @@ -91,6 +92,29 @@ func (t *Target) Replicas() int32 { return *replicas } +// PodAvailable applies the workload controller's own availability rule: Ready +// for longer than the target's minReadySeconds. +func (t *Target) PodAvailable(pod *corev1.Pod, now time.Time) bool { + minReady := time.Duration(t.minReadySeconds()) * time.Second + for _, c := range pod.Status.Conditions { + if c.Type == corev1.PodReady { + return c.Status == corev1.ConditionTrue && + (minReady == 0 || !c.LastTransitionTime.IsZero() && c.LastTransitionTime.Add(minReady).Before(now)) + } + } + return false +} + +func (t *Target) minReadySeconds() int32 { + switch obj := t.Object.(type) { + case *appsv1.Deployment: + return obj.Spec.MinReadySeconds + case *rolloutsv1alpha1.Rollout: + return obj.Spec.MinReadySeconds + } + return 0 +} + func (t *Target) revision() string { if ro, ok := t.Object.(*rolloutsv1alpha1.Rollout); ok { return ro.Status.CurrentPodHash diff --git a/internal/workload/target_test.go b/internal/workload/target_test.go index 4ee4a8c..2b2a29c 100644 --- a/internal/workload/target_test.go +++ b/internal/workload/target_test.go @@ -18,8 +18,11 @@ package workload import ( "testing" + "time" rolloutsv1alpha1 "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" ) @@ -81,3 +84,38 @@ func TestRolloutStandDown(t *testing.T) { }) } } + +func TestPodAvailable(t *testing.T) { + now := time.Now() + ready := func(status corev1.ConditionStatus, since time.Time) *corev1.Pod { + return &corev1.Pod{Status: corev1.PodStatus{Conditions: []corev1.PodCondition{{ + Type: corev1.PodReady, Status: status, LastTransitionTime: metav1.NewTime(since), + }}}} + } + tests := []struct { + name string + pod *corev1.Pod + minReadySeconds int32 + want bool + }{ + {"no ready condition", &corev1.Pod{}, 0, false}, + {"not ready", ready(corev1.ConditionFalse, now.Add(-time.Hour)), 0, false}, + {"ready without minReadySeconds", ready(corev1.ConditionTrue, now), 0, true}, + {"ready for less than minReadySeconds", ready(corev1.ConditionTrue, now.Add(-29*time.Second)), 30, false}, + {"ready for exactly minReadySeconds", ready(corev1.ConditionTrue, now.Add(-30*time.Second)), 30, false}, + {"ready for longer than minReadySeconds", ready(corev1.ConditionTrue, now.Add(-31*time.Second)), 30, true}, + {"ready since an unknown time", ready(corev1.ConditionTrue, time.Time{}), 30, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, target := range []*Target{ + {Object: &appsv1.Deployment{Spec: appsv1.DeploymentSpec{MinReadySeconds: tt.minReadySeconds}}}, + {Object: &rolloutsv1alpha1.Rollout{Spec: rolloutsv1alpha1.RolloutSpec{MinReadySeconds: tt.minReadySeconds}}}, + } { + if got := target.PodAvailable(tt.pod, now); got != tt.want { + t.Errorf("%s PodAvailable() = %v, want %v", target.Kind(), got, tt.want) + } + } + }) + } +}