Skip to content

Commit 4b24b7d

Browse files
authored
fix(validator): wait for KAI deployments ready in gang-scheduling check (#1514)
Signed-off-by: Mark Chmarny <mark@chmarny.com>
1 parent c1d2468 commit 4b24b7d

3 files changed

Lines changed: 220 additions & 4 deletions

File tree

validators/conformance/gang_scheduling_check.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,20 @@ func CheckGangScheduling(ctx *validators.Context) error {
109109
return validators.Skip("KAI scheduler not found — cluster may use a different scheduler")
110110
}
111111

112-
// 1. All KAI scheduler deployments available.
112+
// 1. All KAI scheduler deployments available. Wait (bounded) for each to
113+
// become Available rather than sampling instantaneously: the single-replica
114+
// admission webhook can be transiently unavailable (rollout, restart, node
115+
// pressure), and a point-in-time 0/1 read would flake the whole conformance
116+
// phase. A genuinely-down deployment still fails after the bound.
113117
var deploymentsSummary strings.Builder
114118
for _, name := range kaiSchedulerDeployments {
115-
deploy, err := getDeploymentIfAvailable(ctx, "kai-scheduler", name)
119+
deploy, err := waitForDeploymentAvailable(ctx, "kai-scheduler", name, defaults.K8sPodReadyTimeout)
116120
if err != nil {
117-
return errors.Wrap(errors.ErrCodeNotFound,
118-
fmt.Sprintf("KAI scheduler component %s check failed", name), err)
121+
// Preserve the helper's code (NotFound for missing, Internal for
122+
// not-available/API failure, Timeout for cancellation) instead of
123+
// flattening every failure to NotFound.
124+
return errors.PropagateOrWrap(err, errors.ErrCodeNotFound,
125+
fmt.Sprintf("KAI scheduler component %s check failed", name))
119126
}
120127
expected := int32(1)
121128
if deploy.Spec.Replicas != nil {

validators/conformance/helpers.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"log/slog"
2222
"net/http"
2323
"strings"
24+
"time"
2425

2526
"github.com/NVIDIA/aicr/pkg/defaults"
2627
"github.com/NVIDIA/aicr/pkg/errors"
@@ -155,6 +156,74 @@ func getDeploymentIfAvailable(ctx *validators.Context, namespace, name string) (
155156
return deploy, nil
156157
}
157158

159+
// waitForDeploymentAvailable polls until the named Deployment reports at least
160+
// one available replica, or the timeout elapses. Returns the last-observed
161+
// Deployment so callers can capture diagnostics.
162+
//
163+
// Use this instead of getDeploymentIfAvailable for components that can be
164+
// transiently unavailable — single-replica webhooks, rolling updates,
165+
// restarts. An instantaneous check fails closed on a momentary 0/N blip; the
166+
// bounded wait tolerates the blip but still fails (after the bound) when a
167+
// deployment is genuinely down. A not-yet-created deployment is treated as
168+
// "keep waiting" within the bound rather than an immediate NotFound.
169+
//
170+
//nolint:unparam // namespace is part of the general helper API (mirrors getDeploymentIfAvailable); the sole caller today happens to use one namespace.
171+
func waitForDeploymentAvailable(ctx *validators.Context, namespace, name string, timeout time.Duration) (*appsv1.Deployment, error) {
172+
pollCtx, cancel := context.WithTimeout(ctx.Ctx, timeout)
173+
defer cancel()
174+
175+
var last *appsv1.Deployment
176+
err := wait.PollUntilContextCancel(pollCtx, defaults.PodPollInterval, true,
177+
func(c context.Context) (bool, error) {
178+
deploy, getErr := ctx.Clientset.AppsV1().Deployments(namespace).Get(c, name, metav1.GetOptions{})
179+
if getErr != nil {
180+
if k8serrors.IsNotFound(getErr) {
181+
return false, nil // not created yet — keep waiting within the bound
182+
}
183+
return false, errors.Wrap(errors.ErrCodeInternal,
184+
fmt.Sprintf("failed to get deployment %s/%s", namespace, name), getErr)
185+
}
186+
last = deploy
187+
return deploy.Status.AvailableReplicas >= 1, nil
188+
},
189+
)
190+
if err == nil {
191+
return last, nil
192+
}
193+
194+
// Caller cancellation is an external abort, not a readiness failure.
195+
// pollCtx derives from ctx.Ctx, so pollCtx.Err() is also set when the parent
196+
// is canceled — check the parent explicitly first and propagate it as a
197+
// transient timeout rather than reporting the deployment as not-available.
198+
if ctx.Ctx.Err() != nil {
199+
return last, errors.Wrap(errors.ErrCodeTimeout,
200+
fmt.Sprintf("waiting for deployment %s/%s canceled", namespace, name), ctx.Ctx.Err())
201+
}
202+
203+
expected := int32(1)
204+
var avail int32
205+
if last != nil {
206+
avail = last.Status.AvailableReplicas
207+
if last.Spec.Replicas != nil {
208+
expected = *last.Spec.Replicas
209+
}
210+
}
211+
// Our own bound elapsed (parent still live): the deployment never became
212+
// available in time. Surface the NotFound-shaped not-available message the
213+
// caller wraps. A non-deadline error is a genuine API failure — propagate it.
214+
if pollCtx.Err() != nil {
215+
if last == nil {
216+
return nil, errors.New(errors.ErrCodeNotFound,
217+
fmt.Sprintf("deployment %s/%s not found after %s", namespace, name, timeout))
218+
}
219+
return last, errors.New(errors.ErrCodeInternal,
220+
fmt.Sprintf("deployment %s/%s not available after %s: %d/%d replicas",
221+
namespace, name, timeout, avail, expected))
222+
}
223+
return last, errors.PropagateOrWrap(err, errors.ErrCodeInternal,
224+
fmt.Sprintf("failed waiting for deployment %s/%s", namespace, name))
225+
}
226+
158227
// verifyDaemonSetReady checks that a DaemonSet has at least one ready pod.
159228
func verifyDaemonSetReady(ctx *validators.Context, namespace, name string) error {
160229
_, err := getDaemonSetIfReady(ctx, namespace, name)
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
// Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package main
16+
17+
import (
18+
"context"
19+
stderrors "errors"
20+
"sync/atomic"
21+
"testing"
22+
"time"
23+
24+
"github.com/NVIDIA/aicr/pkg/errors"
25+
"github.com/NVIDIA/aicr/validators"
26+
appsv1 "k8s.io/api/apps/v1"
27+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
"k8s.io/apimachinery/pkg/runtime"
29+
k8sfake "k8s.io/client-go/kubernetes/fake"
30+
k8stesting "k8s.io/client-go/testing"
31+
)
32+
33+
//nolint:unparam // name is a meaningful fixture input even though current cases all use "admission".
34+
func deployWithAvailable(name string, avail int32) *appsv1.Deployment {
35+
r := int32(1)
36+
return &appsv1.Deployment{
37+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "kai-scheduler"},
38+
Spec: appsv1.DeploymentSpec{Replicas: &r},
39+
Status: appsv1.DeploymentStatus{AvailableReplicas: avail},
40+
}
41+
}
42+
43+
// TestWaitForDeploymentAvailable_ImmediatelyReady returns without waiting when
44+
// the deployment is already Available.
45+
func TestWaitForDeploymentAvailable_ImmediatelyReady(t *testing.T) {
46+
t.Parallel()
47+
client := k8sfake.NewClientset(deployWithAvailable("admission", 1))
48+
vctx := &validators.Context{Ctx: context.Background(), Clientset: client}
49+
50+
got, err := waitForDeploymentAvailable(vctx, "kai-scheduler", "admission", 5*time.Second)
51+
if err != nil {
52+
t.Fatalf("unexpected error: %v", err)
53+
}
54+
if got == nil || got.Status.AvailableReplicas != 1 {
55+
t.Fatalf("expected available deployment, got %+v", got)
56+
}
57+
}
58+
59+
// TestWaitForDeploymentAvailable_TransientBlip is the regression test for the
60+
// gang-scheduling flake: a deployment that reports 0/1 on the first sample and
61+
// becomes Available shortly after must pass, not fail closed.
62+
func TestWaitForDeploymentAvailable_TransientBlip(t *testing.T) {
63+
t.Parallel()
64+
client := k8sfake.NewClientset(deployWithAvailable("admission", 0))
65+
66+
var calls int32
67+
client.PrependReactor("get", "deployments", func(k8stesting.Action) (bool, runtime.Object, error) {
68+
n := atomic.AddInt32(&calls, 1)
69+
avail := int32(0)
70+
if n >= 2 { // available from the second poll onward
71+
avail = 1
72+
}
73+
return true, deployWithAvailable("admission", avail), nil
74+
})
75+
76+
vctx := &validators.Context{Ctx: context.Background(), Clientset: client}
77+
got, err := waitForDeploymentAvailable(vctx, "kai-scheduler", "admission", 5*time.Second)
78+
if err != nil {
79+
t.Fatalf("expected success after transient 0/1, got error: %v", err)
80+
}
81+
if got == nil || got.Status.AvailableReplicas != 1 {
82+
t.Fatalf("expected available deployment, got %+v", got)
83+
}
84+
if atomic.LoadInt32(&calls) < 2 {
85+
t.Errorf("expected the helper to re-poll past the initial 0/1 (calls=%d)", calls)
86+
}
87+
}
88+
89+
// TestWaitForDeploymentAvailable_NeverReady fails closed (after the bound) when
90+
// the deployment stays unavailable.
91+
func TestWaitForDeploymentAvailable_NeverReady(t *testing.T) {
92+
t.Parallel()
93+
client := k8sfake.NewClientset(deployWithAvailable("admission", 0))
94+
vctx := &validators.Context{Ctx: context.Background(), Clientset: client}
95+
96+
_, err := waitForDeploymentAvailable(vctx, "kai-scheduler", "admission", 50*time.Millisecond)
97+
if err == nil {
98+
t.Fatal("expected error for never-ready deployment")
99+
}
100+
if !stderrors.Is(err, errors.New(errors.ErrCodeInternal, "")) {
101+
t.Errorf("expected ErrCodeInternal, got %v", err)
102+
}
103+
}
104+
105+
// TestWaitForDeploymentAvailable_ParentCanceled ensures an external abort
106+
// (parent context canceled) is propagated as a transient timeout, not
107+
// misclassified as a deployment readiness failure.
108+
func TestWaitForDeploymentAvailable_ParentCanceled(t *testing.T) {
109+
t.Parallel()
110+
client := k8sfake.NewClientset(deployWithAvailable("admission", 0))
111+
parent, cancel := context.WithCancel(context.Background())
112+
cancel() // external abort before the wait runs
113+
vctx := &validators.Context{Ctx: parent, Clientset: client}
114+
115+
// Generous bound so any failure must come from the parent cancellation,
116+
// not our own deadline.
117+
_, err := waitForDeploymentAvailable(vctx, "kai-scheduler", "admission", 5*time.Second)
118+
if err == nil {
119+
t.Fatal("expected error on canceled parent context")
120+
}
121+
if !stderrors.Is(err, errors.New(errors.ErrCodeTimeout, "")) {
122+
t.Errorf("expected ErrCodeTimeout for cancellation, got %v", err)
123+
}
124+
}
125+
126+
// TestWaitForDeploymentAvailable_NotFound surfaces a missing deployment as
127+
// NotFound after the bound (not a generic internal error).
128+
func TestWaitForDeploymentAvailable_NotFound(t *testing.T) {
129+
t.Parallel()
130+
client := k8sfake.NewClientset() // no deployments
131+
vctx := &validators.Context{Ctx: context.Background(), Clientset: client}
132+
133+
_, err := waitForDeploymentAvailable(vctx, "kai-scheduler", "admission", 50*time.Millisecond)
134+
if err == nil {
135+
t.Fatal("expected error for missing deployment")
136+
}
137+
if !stderrors.Is(err, errors.New(errors.ErrCodeNotFound, "")) {
138+
t.Errorf("expected ErrCodeNotFound, got %v", err)
139+
}
140+
}

0 commit comments

Comments
 (0)