Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions pkg/model-booster-controller/controller/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ const (
ModelFailedReason = "ModelAbnormal"
)

// genericModelServingProgressReasons are the Reason values the model-serving-controller uses
// on ModelServing's Progressing/UpdateInProgress conditions for ordinary startup/rollout
// progress (see pkg/model-serving-controller/utils/utils.go's newCondition). Any other Reason
// on those condition types means model-serving-controller identified a specific, actionable
// Pod-level failure (e.g. scheduling, image-pull, or crash-loop) that's worth surfacing here
// instead of the generic "ModelBooster not ready yet" message.
var genericModelServingProgressReasons = map[string]bool{
"GroupProgressing": true,
"GroupsUpdating": true,
}

// setModelInitCondition sets model condition to initialized
func (mc *ModelBoosterController) setModelInitCondition(ctx context.Context, model *workloadv1alpha1.ModelBooster) error {
meta.SetStatusCondition(&model.Status.Conditions, newCondition(string(workloadv1alpha1.ModelStatusConditionTypeInitialized),
Expand Down Expand Up @@ -63,6 +74,24 @@ func (mc *ModelBoosterController) setModelFailedCondition(ctx context.Context, m
}
}

// setModelWorkloadDegradedCondition sets ModelBooster's Active condition to False using the
// Reason/Message propagated from the generated ModelServing's own condition, so a Pod-level
// failure (scheduling, image pull, crash loop, etc.) is visible on ModelBooster without
// inspecting the ModelServing separately. It reuses the existing Active condition rather than
// introducing a new one: the next reconcile naturally supersedes it, either with
// setModelActiveCondition (True, once the ModelServing recovers) or with a fresh call to this
// function (updated detail) or setModelProcessingCondition's generic message (once the
// ModelServing's condition Reason is no longer an actionable failure) — so it never goes stale.
func (mc *ModelBoosterController) setModelWorkloadDegradedCondition(ctx context.Context, model *workloadv1alpha1.ModelBooster, reason, message string) error {
meta.SetStatusCondition(&model.Status.Conditions, newCondition(string(workloadv1alpha1.ModelStatusConditionTypeActive),
metav1.ConditionFalse, reason, message))
if err := mc.updateModelBoosterStatus(ctx, model); err != nil {
klog.Errorf("update ModelBooster status failed: %v", err)
return err
}
return nil
}

// setModelActiveCondition sets ModelBooster conditions to active
func (mc *ModelBoosterController) setModelActiveCondition(ctx context.Context, model *workloadv1alpha1.ModelBooster) error {
meta.SetStatusCondition(&model.Status.Conditions, newCondition(string(workloadv1alpha1.ModelStatusConditionTypeActive),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,37 +219,60 @@ func (mc *ModelBoosterController) reconcile(ctx context.Context, namespaceAndNam
mc.setModelFailedCondition(ctx, model, err)
return err
}
modelServingActive, err := mc.isModelServingActive(model)
if err != nil || !modelServingActive {
modelServingActive, blockingReason, blockingMessage, err := mc.isModelServingActive(model)
if err != nil {
return err
}
if !modelServingActive {
// If the generated ModelServing's own condition already identifies an actionable,
// non-generic cause (e.g. a Pod scheduling/image-pull/crash failure surfaced by
// model-serving-controller), propagate it here so users don't have to inspect the
// ModelServing separately. Otherwise leave the generic "still initializing" Active
// condition set by setModelProcessingCondition above untouched.
if blockingReason != "" && !genericModelServingProgressReasons[blockingReason] {
if err := mc.setModelWorkloadDegradedCondition(ctx, model, blockingReason, blockingMessage); err != nil {
return err
}
}
return nil
}
if err := mc.setModelActiveCondition(ctx, model); err != nil {
return err
}

return nil
}

// isModelServingActive returns true if all ModelServings are available.
func (mc *ModelBoosterController) isModelServingActive(model *workload.ModelBooster) (bool, error) {
// isModelServingActive returns true if all ModelServings are available. When a ModelServing
// isn't available, it also returns the Reason/Message of that ModelServing's blocking
// Progressing or UpdateInProgress condition (if present), so the caller can decide whether it
// describes normal startup progress or an actionable failure worth surfacing on ModelBooster.
func (mc *ModelBoosterController) isModelServingActive(model *workload.ModelBooster) (active bool, blockingReason string, blockingMessage string, err error) {
modelServings, err := mc.listModelServingsByLabel(model)
if err != nil {
return false, err
return false, "", "", err
}
// Ensure exactly one ModelServing exists for the single backend
if len(modelServings) != 1 {
klog.Infof("Number of ModelServings: %d, expected: %d", len(modelServings), 1)
return false, fmt.Errorf("ModelServing number not equal to backend number")
return false, "", "", fmt.Errorf("ModelServing number not equal to backend number")
}
// Check if all ModelServings are available
for _, modelServing := range modelServings {
if !meta.IsStatusConditionPresentAndEqual(modelServing.Status.Conditions, string(workload.ModelServingAvailable), metav1.ConditionTrue) {
// requeue until all ModelServings are active
klog.InfoS("ModelServing is not available", "ModelServing", klog.KObj(modelServing))
return false, nil
cond := meta.FindStatusCondition(modelServing.Status.Conditions, string(workload.ModelServingProgressing))
if cond == nil {
cond = meta.FindStatusCondition(modelServing.Status.Conditions, string(workload.ModelServingUpdateInProgress))
}
if cond != nil {
return false, cond.Reason, cond.Message, nil
}
return false, "", "", nil
}
}
return true, nil
return true, "", "", nil
}

// updateModelBoosterStatus updates model status.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
kthenafake "github.com/volcano-sh/kthena/client-go/clientset/versioned/fake"
workload "github.com/volcano-sh/kthena/pkg/apis/workload/v1alpha1"
mbutils "github.com/volcano-sh/kthena/pkg/model-booster-controller/utils"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -170,6 +172,162 @@ func TestReconcile_ReturnsError(t *testing.T) {
})
}

// TestIsModelServingActivePropagatesBlockingCondition verifies that when the generated
// ModelServing isn't Available, isModelServingActive surfaces its blocking condition's
// Reason/Message only when that Reason is a specific, actionable one (as opposed to the
// generic "still starting up" reasons model-serving-controller uses for ordinary progress),
// so callers can distinguish "still starting" from "stuck due to a Pod-level failure".
func TestIsModelServingActivePropagatesBlockingCondition(t *testing.T) {
tests := []struct {
name string
conditions []metav1.Condition
expectActive bool
expectReason string
expectMessage string
}{
{
name: "available",
conditions: []metav1.Condition{{Type: string(workload.ModelServingAvailable), Status: metav1.ConditionTrue, Reason: "AllGroupsReady"}},
expectActive: true,
},
{
name: "generic progressing reason is not treated as a failure",
conditions: []metav1.Condition{{
Type: string(workload.ModelServingProgressing), Status: metav1.ConditionTrue,
Reason: "GroupProgressing", Message: "Some groups is progressing: [0]",
}},
expectActive: false,
expectReason: "GroupProgressing",
},
{
name: "pod-level failure reason is propagated",
conditions: []metav1.Condition{{
Type: string(workload.ModelServingProgressing), Status: metav1.ConditionTrue,
Reason: "ImagePullBackOff",
Message: "Some groups is progressing: [0]; pod p-0 init container downloader: back-off pulling image",
}},
expectActive: false,
expectReason: "ImagePullBackOff",
expectMessage: "downloader",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
kubeClient := fake.NewClientset()
kthenaClient := kthenafake.NewSimpleClientset()
controller := NewModelBoosterController(kubeClient, kthenaClient)

model := &workload.ModelBooster{
ObjectMeta: metav1.ObjectMeta{Name: "m1", Namespace: "default", UID: "model-uid-1"},
}
modelServing := &workload.ModelServing{
ObjectMeta: metav1.ObjectMeta{
Name: "ms1",
Namespace: "default",
Labels: map[string]string{mbutils.OwnerUIDKey: string(model.UID)},
},
Status: workload.ModelServingStatus{Conditions: tt.conditions},
}
assert.NoError(t, controller.modelServingInformer.GetIndexer().Add(modelServing))

active, reason, message, err := controller.isModelServingActive(model)
assert.NoError(t, err)
assert.Equal(t, tt.expectActive, active)
assert.Equal(t, tt.expectReason, reason)
if tt.expectMessage != "" {
assert.Contains(t, message, tt.expectMessage)
}
})
}
}

// TestReconcileSurfacesModelServingPodFailure verifies the end-to-end propagation: when
// reconcile finds the ModelServing blocked on an actionable (non-generic) Progressing reason,
// ModelBooster's Active condition is set to False with that same Reason/Message instead of the
// generic "ModelBooster not ready yet" message -- and that once the ModelServing recovers to
// Available, the next reconcile clears it back to Active=True (the condition never sticks).
func TestReconcileSurfacesModelServingPodFailure(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

kubeClient := fake.NewClientset()
kthenaClient := kthenafake.NewSimpleClientset()
controller := NewModelBoosterController(kubeClient, kthenaClient)
go controller.Run(ctx, 1)
assert.True(t, waitForControllerCacheSync(controller))

model := loadYaml[workload.ModelBooster](t, "../convert/testdata/input/model.yaml")
_, err := kthenaClient.WorkloadV1alpha1().ModelBoosters(model.Namespace).Create(ctx, model, metav1.CreateOptions{})
assert.NoError(t, err)

// Let the controller create the real ModelServing itself (correct name/labels/revision),
// matching how it would exist in a real cluster.
var modelServingName string
assert.True(t, waitForCondition(func() bool {
list, err := kthenaClient.WorkloadV1alpha1().ModelServings(model.Namespace).List(ctx, metav1.ListOptions{})
if err != nil || len(list.Items) != 1 {
return false
}
modelServingName = list.Items[0].Name
return true
}))

// Simulate model-serving-controller surfacing a Pod-level failure: patch only the Status
// subresource so the Spec/labels the running controller compares against on future passes
// are untouched (i.e. it will see nothing to update and leave this Status alone).
msToUpdate, err := kthenaClient.WorkloadV1alpha1().ModelServings(model.Namespace).Get(ctx, modelServingName, metav1.GetOptions{})
assert.NoError(t, err)
msToUpdate.Status.Conditions = []metav1.Condition{{
Type: string(workload.ModelServingProgressing), Status: metav1.ConditionTrue,
Reason: "CrashLoopBackOff",
Message: "Some groups is progressing: [0]; pod p-0 container engine: back-off restarting failed container",
}}
_, err = kthenaClient.WorkloadV1alpha1().ModelServings(model.Namespace).UpdateStatus(ctx, msToUpdate, metav1.UpdateOptions{})
assert.NoError(t, err)

// The ModelServing status update triggers the controller (via triggerModel) to reconcile the
// ModelBooster; wait for the propagated Reason/Message to land on its Active condition.
assert.True(t, waitForCondition(func() bool {
get, err := kthenaClient.WorkloadV1alpha1().ModelBoosters(model.Namespace).Get(ctx, model.Name, metav1.GetOptions{})
if err != nil {
return false
}
cond := meta.FindStatusCondition(get.Status.Conditions, string(workload.ModelStatusConditionTypeActive))
return cond != nil && cond.Reason == "CrashLoopBackOff"
}), "ModelBooster's Active condition should surface the ModelServing's Pod failure reason")

get, err := kthenaClient.WorkloadV1alpha1().ModelBoosters(model.Namespace).Get(ctx, model.Name, metav1.GetOptions{})
assert.NoError(t, err)
activeCond := meta.FindStatusCondition(get.Status.Conditions, string(workload.ModelStatusConditionTypeActive))
require.NotNil(t, activeCond)
assert.Equal(t, metav1.ConditionFalse, activeCond.Status)
assert.Contains(t, activeCond.Message, "back-off restarting")

// The ModelServing recovers: the controller must clear the propagated failure, not leave it
// stale on ModelBooster.
msToUpdate, err = kthenaClient.WorkloadV1alpha1().ModelServings(model.Namespace).Get(ctx, modelServingName, metav1.GetOptions{})
assert.NoError(t, err)
msToUpdate.Status.Conditions = []metav1.Condition{{Type: string(workload.ModelServingAvailable), Status: metav1.ConditionTrue, Reason: "AllGroupsReady"}}
_, err = kthenaClient.WorkloadV1alpha1().ModelServings(model.Namespace).UpdateStatus(ctx, msToUpdate, metav1.UpdateOptions{})
assert.NoError(t, err)

assert.True(t, waitForCondition(func() bool {
get, err := kthenaClient.WorkloadV1alpha1().ModelBoosters(model.Namespace).Get(ctx, model.Name, metav1.GetOptions{})
if err != nil {
return false
}
cond := meta.FindStatusCondition(get.Status.Conditions, string(workload.ModelStatusConditionTypeActive))
return cond != nil && cond.Status == metav1.ConditionTrue
}), "the stale failure reason must not survive recovery")

get, err = kthenaClient.WorkloadV1alpha1().ModelBoosters(model.Namespace).Get(ctx, model.Name, metav1.GetOptions{})
assert.NoError(t, err)
activeCond = meta.FindStatusCondition(get.Status.Conditions, string(workload.ModelStatusConditionTypeActive))
require.NotNil(t, activeCond)
assert.Equal(t, ModelActiveReason, activeCond.Reason)
}

func TestCreateModel(t *testing.T) {
kubeClient := fake.NewClientset()
kthenaClient := kthenafake.NewClientset()
Expand Down
Loading
Loading