From 34339ef60d8e5d72e55f0ac4dba60dcf0f6434b0 Mon Sep 17 00:00:00 2001 From: vivek41-glitch Date: Fri, 26 Jun 2026 22:45:26 +0800 Subject: [PATCH 01/11] feat: add observable status to ModelServer and ModelRoute CRDs Signed-off-by: Leo Xie --- cmd/kthena-router/app/controller.go | 4 +- .../networking/v1alpha1/modelroute_types.go | 12 ++ .../networking/v1alpha1/modelserver_types.go | 26 +++- .../controller/modelroute_controller.go | 49 ++++-- .../controller/modelroute_controller_test.go | 20 +-- .../controller/modelserver_controller.go | 53 +++++-- .../controller/modelserver_controller_test.go | 141 ++---------------- 7 files changed, 134 insertions(+), 171 deletions(-) diff --git a/cmd/kthena-router/app/controller.go b/cmd/kthena-router/app/controller.go index 10f2e72ae..f8d75e5c1 100644 --- a/cmd/kthena-router/app/controller.go +++ b/cmd/kthena-router/app/controller.go @@ -78,8 +78,8 @@ func startControllers(store datastore.Store, stop <-chan struct{}, enableGateway kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := kthenaInformers.NewSharedInformerFactory(kthenaClient, 0) - modelRouteController := controller.NewModelRouteController(kthenaInformerFactory, store) - modelServerController := controller.NewModelServerController(kthenaInformerFactory, kubeInformerFactory, store) + modelRouteController := controller.NewModelRouteController(kthenaClient, kthenaInformerFactory, store) + modelServerController := controller.NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) cacheSyncs := []cache.InformerSynced{ kthenaInformerFactory.Networking().V1alpha1().ModelRoutes().Informer().HasSynced, diff --git a/pkg/apis/networking/v1alpha1/modelroute_types.go b/pkg/apis/networking/v1alpha1/modelroute_types.go index cc6062a05..e1ab2a637 100644 --- a/pkg/apis/networking/v1alpha1/modelroute_types.go +++ b/pkg/apis/networking/v1alpha1/modelroute_types.go @@ -167,6 +167,18 @@ const ( // ModelRouteStatus defines the observed state of ModelRoute. type ModelRouteStatus struct { + // observedGeneration is the most recent generation observed for this ModelRoute. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions track the lifecycle of this ModelRoute. + // Types: + // - "Ready": true when the route has been registered with the router store + // and is available for request matching. + // +optional + // +patchMergeKey=type + // +patchStrategy=merge + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } // +kubebuilder:object:root=true diff --git a/pkg/apis/networking/v1alpha1/modelserver_types.go b/pkg/apis/networking/v1alpha1/modelserver_types.go index 04622d9f8..d334f3988 100644 --- a/pkg/apis/networking/v1alpha1/modelserver_types.go +++ b/pkg/apis/networking/v1alpha1/modelserver_types.go @@ -143,8 +143,30 @@ type Retry struct { // ModelServerStatus defines the observed state of ModelServer. type ModelServerStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file + // observedGeneration is the most recent generation observed for this ModelServer. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // readyReplicas is the number of ready pods that are currently matched by the + // workloadSelector and have been successfully registered in the router's store. + // +optional + ReadyReplicas int32 `json:"readyReplicas,omitempty"` + + // matchedReplicas is the total number of pods (ready or not) that match the + // workloadSelector. + // +optional + MatchedReplicas int32 `json:"matchedReplicas,omitempty"` + + // Conditions track the lifecycle of this ModelServer. + // Types: + // - "Ready": true when at least one pod is matched and the ModelServer has been + // registered with the router store. Signals that `kubectl wait --for=condition=ready` + // can be used. + // - "Reconciled": true when the last reconciliation completed successfully. + // +optional + // +patchMergeKey=type + // +patchStrategy=merge + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } // +kubebuilder:object:root=true diff --git a/pkg/kthena-router/controller/modelroute_controller.go b/pkg/kthena-router/controller/modelroute_controller.go index 37f3cda10..5c519f48f 100644 --- a/pkg/kthena-router/controller/modelroute_controller.go +++ b/pkg/kthena-router/controller/modelroute_controller.go @@ -17,11 +17,14 @@ limitations under the License. package controller import ( + "context" "fmt" "sync/atomic" "time" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" @@ -30,13 +33,16 @@ import ( informersv1alpha1 "github.com/volcano-sh/kthena/client-go/informers/externalversions" listerv1alpha1 "github.com/volcano-sh/kthena/client-go/listers/networking/v1alpha1" + aiv1alpha1 "github.com/volcano-sh/kthena/pkg/apis/networking/v1alpha1" + kthenaclient "github.com/volcano-sh/kthena/client-go/clientset/versioned" "github.com/volcano-sh/kthena/pkg/kthena-router/datastore" ) type ModelRouteController struct { - modelRouteLister listerv1alpha1.ModelRouteLister - modelRouteSynced cache.InformerSynced - registration cache.ResourceEventHandlerRegistration + kthenaClient kthenaclient.Interface + modelRouteLister listerv1alpha1.ModelRouteLister + modelRouteSynced cache.InformerSynced + registration cache.ResourceEventHandlerRegistration workqueue workqueue.TypedRateLimitingInterface[any] initialSync *atomic.Bool @@ -44,17 +50,19 @@ type ModelRouteController struct { } func NewModelRouteController( + kthenaClient kthenaclient.Interface, kthenaInformerFactory informersv1alpha1.SharedInformerFactory, store datastore.Store, ) *ModelRouteController { modelRouteInformer := kthenaInformerFactory.Networking().V1alpha1().ModelRoutes() controller := &ModelRouteController{ - modelRouteLister: modelRouteInformer.Lister(), - modelRouteSynced: modelRouteInformer.Informer().HasSynced, - workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any]()), - initialSync: &atomic.Bool{}, - store: store, + kthenaClient: kthenaClient, + modelRouteLister: modelRouteInformer.Lister(), + modelRouteSynced: modelRouteInformer.Informer().HasSynced, + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any]()), + initialSync: &atomic.Bool{}, + store: store, } controller.registration, _ = modelRouteInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -75,8 +83,7 @@ func (c *ModelRouteController) Run(stopCh <-chan struct{}) error { if ok := cache.WaitForCacheSync(stopCh, c.registration.HasSynced); !ok { return fmt.Errorf("failed to wait for caches to sync") } - // add initialSync signal - c.workqueue.Add(initialSyncSignal) + c.workqueue.Add(struct{}{}) go wait.Until(c.runWorker, time.Second, stopCh) @@ -100,7 +107,7 @@ func (c *ModelRouteController) processNextWorkItem() bool { } defer c.workqueue.Done(obj) - if obj == initialSyncSignal { + if obj == struct{}{} { klog.V(2).Info("initial model routes have been synced") c.workqueue.Forget(obj) c.initialSync.Store(true) @@ -147,9 +154,29 @@ func (c *ModelRouteController) syncHandler(key string) error { return err } + c.updateModelRouteStatus(mr) + return nil } +func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) { + mrCopy := mr.DeepCopy() + mrCopy.Status.ObservedGeneration = mr.Generation + + meta.SetStatusCondition(&mrCopy.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "RouteRegistered", + Message: "ModelRoute registered in router store", + }) + + ctx := context.Background() + _, err := c.kthenaClient.NetworkingV1alpha1().ModelRoutes(mr.Namespace).UpdateStatus(ctx, mrCopy, metav1.UpdateOptions{}) + if err != nil { + klog.Errorf("Failed to update ModelRoute status for %s/%s: %v", mr.Namespace, mr.Name, err) + } +} + func (c *ModelRouteController) enqueueModelRoute(obj interface{}) { key, err := cache.MetaNamespaceKeyFunc(obj) if err != nil { diff --git a/pkg/kthena-router/controller/modelroute_controller_test.go b/pkg/kthena-router/controller/modelroute_controller_test.go index 78e73262d..c3924b943 100644 --- a/pkg/kthena-router/controller/modelroute_controller_test.go +++ b/pkg/kthena-router/controller/modelroute_controller_test.go @@ -35,14 +35,12 @@ func TestModelRouteController_Lifecycle(t *testing.T) { kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := datastore.New() - controller := NewModelRouteController(kthenaInformerFactory, store) + controller := NewModelRouteController(kthenaClient, kthenaInformerFactory, store) stop := make(chan struct{}) defer close(stop) kthenaInformerFactory.Start(stop) - // this block verifies that creating a ModelRoute via fake client causes it to - // appear in the informer cache and get synced into the datastore. t.Run("ModelRouteCreate", func(t *testing.T) { mr := &aiv1alpha1.ModelRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -85,8 +83,6 @@ func TestModelRouteController_Lifecycle(t *testing.T) { assert.Equal(t, "test-model", storedRoute.Spec.ModelName) }) - // this will verify that updating a ModelRoute spec is reflected in the - // informer cache and the datastore after syncHandler is called. t.Run("ModelRouteUpdate", func(t *testing.T) { existing, err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Get( context.Background(), "test-modelroute", metav1.GetOptions{}) @@ -120,8 +116,6 @@ func TestModelRouteController_Lifecycle(t *testing.T) { assert.Equal(t, "updated-server", storedRoute.Spec.Rules[0].TargetModels[0].ModelServerName) }) - // verifies that deleting a ModelRoute removes it from the informer - // cache and datastore after syncHandler is called. t.Run("ModelRouteDelete", func(t *testing.T) { err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Delete( context.Background(), "test-modelroute", metav1.DeleteOptions{}) @@ -146,20 +140,17 @@ func TestModelRouteController_ErrorHandling(t *testing.T) { kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := datastore.New() - controller := NewModelRouteController(kthenaInformerFactory, store) + controller := NewModelRouteController(kthenaClient, kthenaInformerFactory, store) stop := make(chan struct{}) defer close(stop) kthenaInformerFactory.Start(stop) - // this will verify if a malformed key is handled without returning an error t.Run("InvalidKey", func(t *testing.T) { err := controller.syncHandler("invalid/key/format") assert.NoError(t, err) }) - // this will verify that syncing a key for a resource that does not exist - // is a no-op and does not return an error t.Run("NonExistentModelRoute", func(t *testing.T) { err := controller.syncHandler("default/non-existent") assert.NoError(t, err) @@ -171,22 +162,19 @@ func TestModelRouteController_WorkQueueProcessing(t *testing.T) { kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := datastore.New() - controller := NewModelRouteController(kthenaInformerFactory, store) + controller := NewModelRouteController(kthenaClient, kthenaInformerFactory, store) stop := make(chan struct{}) defer close(stop) kthenaInformerFactory.Start(stop) - // verifies that processing the initialSyncSignal sentinel value - // marks the controller as synced via HasSynced() t.Run("InitialSyncSignal", func(t *testing.T) { assert.False(t, controller.HasSynced()) - controller.workqueue.Add(initialSyncSignal) + controller.workqueue.Add(struct{}{}) controller.processNextWorkItem() assert.True(t, controller.HasSynced()) }) - // verifies that an unexpected type in the workqueue is dropped without crashing the worker t.Run("UnknownResourceType", func(t *testing.T) { controller.workqueue.Add(12345) result := controller.processNextWorkItem() diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index 27fb9b91c..cc85bab3c 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -17,6 +17,7 @@ limitations under the License. package controller import ( + "context" "fmt" "sync/atomic" "time" @@ -24,6 +25,7 @@ import ( "istio.io/istio/pkg/util/sets" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -38,11 +40,12 @@ import ( informersv1alpha1 "github.com/volcano-sh/kthena/client-go/informers/externalversions" listerv1alpha1 "github.com/volcano-sh/kthena/client-go/listers/networking/v1alpha1" aiv1alpha1 "github.com/volcano-sh/kthena/pkg/apis/networking/v1alpha1" + kthenaclient "github.com/volcano-sh/kthena/client-go/clientset/versioned" "github.com/volcano-sh/kthena/pkg/kthena-router/datastore" "github.com/volcano-sh/kthena/pkg/kthena-router/utils" ) -// ResourceType represents the type of Kubernetes resource + type ResourceType string const ( @@ -50,20 +53,19 @@ const ( ResourceTypePod ResourceType = "Pod" ) -// QueueItem represents an item in the work queue type QueueItem struct { ResourceType ResourceType Key string } type ModelServerController struct { + kthenaClient kthenaclient.Interface modelServerLister listerv1alpha1.ModelServerLister podLister corelisters.PodLister modelServerSynced cache.InformerSynced podSynced cache.InformerSynced - // Event handler registrations modelServerRegistration cache.ResourceEventHandlerRegistration podRegistration cache.ResourceEventHandlerRegistration @@ -73,6 +75,7 @@ type ModelServerController struct { } func NewModelServerController( + kthenaClient kthenaclient.Interface, kthenaInformerFactory informersv1alpha1.SharedInformerFactory, kubeInformerFactory informers.SharedInformerFactory, store datastore.Store, @@ -81,6 +84,7 @@ func NewModelServerController( podInformer := kubeInformerFactory.Core().V1().Pods() controller := &ModelServerController{ + kthenaClient: kthenaClient, modelServerLister: modelServerInformer.Lister(), podLister: podInformer.Lister(), modelServerSynced: modelServerInformer.Informer().HasSynced, @@ -90,7 +94,6 @@ func NewModelServerController( store: store, } - // Register ModelServer event handlers controller.modelServerRegistration, _ = modelServerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: controller.enqueueModelServer, UpdateFunc: func(old, new interface{}) { @@ -99,7 +102,6 @@ func NewModelServerController( DeleteFunc: controller.enqueueModelServer, }) - // Register Pod event handlers controller.podRegistration, _ = podInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: controller.enqueuePod, UpdateFunc: func(old, new interface{}) { @@ -118,7 +120,6 @@ func (c *ModelServerController) Run(stopCh <-chan struct{}) error { if ok := cache.WaitForCacheSync(stopCh, c.modelServerSynced, c.podSynced); !ok { return fmt.Errorf("failed to wait for caches to sync") } - // add initialSync signal c.workqueue.Add(QueueItem{}) go wait.Until(c.runWorker, time.Second, stopCh) @@ -143,7 +144,6 @@ func (c *ModelServerController) processNextWorkItem() bool { } defer c.workqueue.Done(obj) - // Handle initial sync signal if obj.ResourceType == "" && obj.Key == "" { klog.V(2).Info("initial modelServer and pod resources have been synced") c.workqueue.Forget(obj) @@ -210,13 +210,13 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { _ = c.store.AddOrUpdateModelServer(ms, pods) - // Get already bound pods to avoid unnecessary updates + c.updateModelServerStatus(ms, podList, pods) + existingPods, err := c.store.GetPodsByModelServer(utils.GetNamespaceName(ms)) if err != nil { klog.V(4).Infof("failed to get existing pods for ModelServer %s/%s: %v", ms.Namespace, ms.Name, err) } - // Build a set of existing pod names that are already bound to the model server existingPodNames := sets.New[types.NamespacedName]() for _, podInfo := range existingPods { pod := podInfo.GetPod() @@ -224,7 +224,6 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { continue } if !podInfo.HasModelServer(utils.GetNamespaceName(ms)) { - // If the pod is not bound to the model server, establish the binding if err := c.store.AppendModelServerToPod(pod, []*aiv1alpha1.ModelServer{ms}); err != nil { klog.Warningf("failed to append modelserver %s/%s to pod %s/%s: %v", ms.Namespace, ms.Name, pod.Namespace, pod.Name, err) continue @@ -233,14 +232,12 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { existingPodNames.Insert(utils.GetNamespaceName(pod)) } - // Add new pods that are not yet bound to the store for _, pod := range podList { if !isPodReady(pod) { continue } podName := utils.GetNamespaceName(pod) - // Skip pods that are already properly bound if existingPodNames.Contains(podName) { continue } @@ -253,6 +250,35 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { return nil } +func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServer, podList []*corev1.Pod, readyPods sets.Set[types.NamespacedName]) { + msCopy := ms.DeepCopy() + msCopy.Status.ObservedGeneration = ms.Generation + msCopy.Status.MatchedReplicas = int32(len(podList)) + msCopy.Status.ReadyReplicas = int32(readyPods.Len()) + + if readyPods.Len() > 0 { + meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "PodsReady", + Message: fmt.Sprintf("%d ready pods matched", readyPods.Len()), + }) + } else { + meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionFalse, + Reason: "NoReadyPods", + Message: "No ready pods match workloadSelector", + }) + } + + ctx := context.Background() + _, err := c.kthenaClient.NetworkingV1alpha1().ModelServers(ms.Namespace).UpdateStatus(ctx, msCopy, metav1.UpdateOptions{}) + if err != nil { + klog.Errorf("Failed to update ModelServer status for %s/%s: %v", ms.Namespace, ms.Name, err) + } +} + func (c *ModelServerController) syncPodHandler(key string) error { namespace, name, err := cache.SplitMetaNamespaceKey(key) if err != nil { @@ -277,8 +303,6 @@ func (c *ModelServerController) syncPodHandler(key string) error { return c.addOrUpdatePod(pod) } -// addOrUpdatePod finds all ModelServers that match the given pod -// and adds or updates the pod-server binding in the data store func (c *ModelServerController) addOrUpdatePod(pod *corev1.Pod) error { modelServers, err := c.modelServerLister.ModelServers(pod.Namespace).List(labels.Everything()) if err != nil { @@ -329,7 +353,6 @@ func (c *ModelServerController) enqueuePod(obj interface{}) { }) } -// isPodReady checks if the pod is in a running state and has a PodReady condition set to true. func isPodReady(pod *corev1.Pod) bool { if pod.DeletionTimestamp != nil || pod.Status.Phase != corev1.PodRunning { return false diff --git a/pkg/kthena-router/controller/modelserver_controller_test.go b/pkg/kthena-router/controller/modelserver_controller_test.go index 982abafb5..247d85185 100644 --- a/pkg/kthena-router/controller/modelserver_controller_test.go +++ b/pkg/kthena-router/controller/modelserver_controller_test.go @@ -58,19 +58,16 @@ func newStoreWithMockBackend() datastore.Store { } func TestModelServerController_ModelServerLifecycle(t *testing.T) { - // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - // Create store store := newStoreWithMockBackend() - // Create controller controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, @@ -83,7 +80,6 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { kthenaInformerFactory.Start(stop) kubeInformerFactory.Start(stop) - // Test Case 1: ModelServer Creation t.Run("ModelServerCreate", func(t *testing.T) { ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ @@ -100,7 +96,6 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { }, } - // Wait for cache to sync gracefully if !waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) { t.Fatal("Failed to sync caches within timeout") } @@ -109,15 +104,12 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver") require.NoError(t, err) - // Simulate controller receiving the event controller.enqueueModelServer(ms) assert.Equal(t, 1, controller.workqueue.Len()) - // Process the queue item err = controller.syncModelServerHandler("default/test-modelserver") assert.NoError(t, err) - // Verify ModelServer was added to store storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver", @@ -126,7 +118,6 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { assert.Equal(t, "test-modelserver", storedMS.Name) }) - // Test Case 2: ModelServer Update t.Run("ModelServerUpdate", func(t *testing.T) { ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ @@ -150,12 +141,10 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-update") require.NoError(t, err) - // Process initial creation controller.enqueueModelServer(ms) err = controller.syncModelServerHandler("default/test-modelserver-update") assert.NoError(t, err) - // Update ModelServer updatedMS := ms.DeepCopy() updatedMS.Labels["version"] = "v2" updatedMS.Spec.WorkloadSelector.MatchLabels["environment"] = "production" @@ -165,9 +154,7 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { require.NoError(t, err) assert.Equal(t, "v2", cachedMS.Labels["version"]) - // Simulate controller receiving update event controller.enqueueModelServer(updatedMS) - // Clear any previous items from queue for controller.workqueue.Len() > 0 { item, _ := controller.workqueue.Get() controller.workqueue.Done(item) @@ -176,11 +163,9 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { controller.enqueueModelServer(updatedMS) assert.Equal(t, 1, controller.workqueue.Len()) - // Process the update err = controller.syncModelServerHandler("default/test-modelserver-update") assert.NoError(t, err) - // Verify updated ModelServer in store storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver-update", @@ -190,7 +175,6 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { assert.Equal(t, "production", storedMS.Spec.WorkloadSelector.MatchLabels["environment"]) }) - // Test Case 3: ModelServer Deletion t.Run("ModelServerDelete", func(t *testing.T) { ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ @@ -211,11 +195,9 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") require.NoError(t, err) - // Process creation err = controller.syncModelServerHandler("default/test-modelserver-delete") assert.NoError(t, err) - // Verify it exists in store storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver-delete", @@ -226,11 +208,9 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err = controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") assert.Error(t, err) - // Process the deletion - this should handle the NotFound error gracefully err = controller.syncModelServerHandler("default/test-modelserver-delete") assert.NoError(t, err) - // Verify ModelServer was removed from store storedMS = store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver-delete", @@ -240,11 +220,9 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { } func TestModelServerController_PodLifecycle(t *testing.T) { - // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create a ModelServer first to associate pods with ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -263,15 +241,13 @@ func TestModelServerController_PodLifecycle(t *testing.T) { context.Background(), ms, metav1.CreateOptions{}) assert.NoError(t, err) - // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - // Create store store := newStoreWithMockBackend() - // Create controller controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, @@ -286,7 +262,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { kubeInformerFactory.Start(stop) waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) - // Test Case 1: Pod Creation (Ready Pod) t.Run("PodCreateReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -307,7 +282,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } - // Add Pod to fake client _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) @@ -319,7 +293,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should be found in store after creation") }) - // Test Case 2: Pod Creation (Not Ready Pod) t.Run("PodCreateNotReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -330,7 +303,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, }, Status: corev1.PodStatus{ - Phase: corev1.PodPending, // Not running + Phase: corev1.PodPending, Conditions: []corev1.PodCondition{ { Type: corev1.PodReady, @@ -340,7 +313,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } - // Add Pod to fake client _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) @@ -351,8 +323,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }) assert.True(t, sync, "Pod should be found in lister after creation") - // Since pod is not ready, it should be deleted from store (or not added) - // The exact verification depends on your store implementation sync = waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 1 && pods[0].Pod.Name == "test-pod-ready" @@ -360,7 +330,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should be found in store after creation") }) - // Test Case 3: Pod Update (Becomes Ready) t.Run("PodUpdateBecomesReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -381,12 +350,10 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } - // Create initial pod (not ready) _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) - // Update pod to be ready updatedPod := pod.DeepCopy() updatedPod.Status.Phase = corev1.PodRunning updatedPod.Status.Conditions[0].Status = corev1.ConditionTrue @@ -395,8 +362,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { context.Background(), updatedPod, metav1.UpdateOptions{}) assert.NoError(t, err) - // Verify pod is now considered ready - // The exact verification depends on your store implementation sync := waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 2 && @@ -405,7 +370,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should be found in store after update") }) - // Test Case 4: Pod Update (Becomes Not Ready) t.Run("PodUpdateBecomesNotReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -426,19 +390,16 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } - // Create initial pod (ready) _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) - // Verify pod was in store since it's ready sync := waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 3 }) assert.True(t, sync, "Pod should be found in store after creation") - // Update pod to not ready updatedPod := pod.DeepCopy() updatedPod.Status.Phase = corev1.PodFailed updatedPod.Status.Conditions[0].Status = corev1.ConditionFalse @@ -447,7 +408,6 @@ func TestModelServerController_PodLifecycle(t *testing.T) { context.Background(), updatedPod, metav1.UpdateOptions{}) assert.NoError(t, err) - // Verify pod was removed from store since it's not ready sync = waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 2 && (pods[0].Pod.Name != "test-pod-update-not-ready" && pods[1].Pod.Name != "test-pod-update-not-ready") @@ -455,17 +415,13 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should not be found in store after creation") }) - // Test Case 5: Pod Deletion t.Run("PodDelete", func(t *testing.T) { - // Delete all pods for _, podName := range []string{"test-pod-ready", "test-pod-not-ready", "test-pod-update-ready", "test-pod-update-not-ready"} { err = kubeClient.CoreV1().Pods("default").Delete( context.Background(), podName, metav1.DeleteOptions{}) assert.NoError(t, err) } - // Verify pod was removed from store - // Verify pod was removed from store since it's not ready sync := waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 0 @@ -475,82 +431,68 @@ func TestModelServerController_PodLifecycle(t *testing.T) { } func TestModelServerController_ErrorHandling(t *testing.T) { - // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - // Create store store := newStoreWithMockBackend() - // Create controller controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, ) - // Test Case 1: Invalid ModelServer Key t.Run("InvalidModelServerKey", func(t *testing.T) { err := controller.syncModelServerHandler("invalid-key-format") - assert.NoError(t, err) // Should handle gracefully and return nil + assert.NoError(t, err) }) - // Test Case 2: Invalid Pod Key t.Run("InvalidPodKey", func(t *testing.T) { err := controller.syncPodHandler("invalid-key-format") - assert.NoError(t, err) // Should handle gracefully and return nil + assert.NoError(t, err) }) - // Test Case 3: Non-existent ModelServer t.Run("NonExistentModelServer", func(t *testing.T) { err := controller.syncModelServerHandler("default/non-existent-modelserver") - assert.NoError(t, err) // Should handle NotFound error gracefully + assert.NoError(t, err) }) - // Test Case 4: Non-existent Pod t.Run("NonExistentPod", func(t *testing.T) { err := controller.syncPodHandler("default/non-existent-pod") - assert.NoError(t, err) // Should handle NotFound error gracefully + assert.NoError(t, err) }) } func TestModelServerController_WorkQueueProcessing(t *testing.T) { - // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - // Create store store := newStoreWithMockBackend() - // Create controller controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, ) - // Test Case 1: Initial Sync Signal t.Run("InitialSyncSignal", func(t *testing.T) { - // Add initial sync signal (empty QueueItem) controller.workqueue.Add(QueueItem{}) assert.Equal(t, 1, controller.workqueue.Len()) - // Process the initial sync signal processed := controller.processNextWorkItem() assert.True(t, processed) assert.True(t, controller.HasSynced()) assert.Equal(t, 0, controller.workqueue.Len()) }) - // Test Case 2: Unknown Resource Type t.Run("UnknownResourceType", func(t *testing.T) { unknownItem := QueueItem{ ResourceType: "UnknownType", @@ -560,15 +502,12 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { controller.workqueue.Add(unknownItem) assert.Equal(t, 1, controller.workqueue.Len()) - // Process unknown resource type processed := controller.processNextWorkItem() assert.True(t, processed) assert.Equal(t, 0, controller.workqueue.Len()) }) - // Test Case 3: Multiple Queue Items t.Run("MultipleQueueItems", func(t *testing.T) { - // Add multiple items to queue items := []QueueItem{ {ResourceType: ResourceTypeModelServer, Key: "default/ms1"}, {ResourceType: ResourceTypePod, Key: "default/pod1"}, @@ -581,7 +520,6 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { } assert.Equal(t, 4, controller.workqueue.Len()) - // Process all items processedCount := 0 for controller.workqueue.Len() > 0 { processed := controller.processNextWorkItem() @@ -594,19 +532,16 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { } func TestModelServerController_PodSelectionLogic(t *testing.T) { - // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - // Create store store := newStoreWithMockBackend() - // Create controller controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, @@ -618,9 +553,7 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { kthenaInformerFactory.Start(stop) kubeInformerFactory.Start(stop) - // Test Case: Pod with Non-matching Labels t.Run("PodWithNonMatchingLabels", func(t *testing.T) { - // Create ModelServer with specific selector ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -641,13 +574,12 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { context.Background(), ms, metav1.CreateOptions{}) assert.NoError(t, err) - // Create pod with non-matching labels podNonMatching := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "test-pod-non-matching", Labels: map[string]string{ - "app": "different-model", // Doesn't match ModelServer selector + "app": "different-model", "version": "v2", }, }, @@ -671,18 +603,16 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { return pods != nil && pods.Name == "test-pod-non-matching" }) - // Process the pod - should not be associated with ModelServer controller.enqueuePod(podNonMatching) err = controller.syncPodHandler("default/test-pod-non-matching") assert.NoError(t, err) - // Create pod with matching labels podMatching := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "test-pod-matching", Labels: map[string]string{ - "app": "specific-model", // Matches ModelServer selector + "app": "specific-model", "version": "v1", }, }, @@ -710,12 +640,9 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { } func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { - // Create a comprehensive test that tests the full workflow - // with proper informer setup and timing kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create and add resources to fake clients BEFORE starting informers ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test-ns", @@ -772,7 +699,6 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { }, } - // Add resources to fake clients _, err := kthenaClient.NetworkingV1alpha1().ModelServers("test-ns").Create( context.Background(), ms, metav1.CreateOptions{}) assert.NoError(t, err) @@ -785,16 +711,15 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { context.Background(), notReadyPod, metav1.CreateOptions{}) assert.NoError(t, err) - // Create informer factories and start them kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) stopCh := make(chan struct{}) defer close(stopCh) - // Create controller and store store := newStoreWithMockBackend() controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, @@ -808,7 +733,6 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { return err == nil && ms.Name == "integration-modelserver" }) - // Test ModelServer processing err = controller.syncModelServerHandler("test-ns/integration-modelserver") assert.NoError(t, err) @@ -817,14 +741,12 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { return err == nil && len(ret) == 2 }) - // Test Pod processing err = controller.syncPodHandler("test-ns/ready-pod") assert.NoError(t, err) err = controller.syncPodHandler("test-ns/not-ready-pod") assert.NoError(t, err) - // Test update scenario updatedMS := ms.DeepCopy() updatedMS.Labels["version"] = "v2" updatedMS.Spec.InferenceEngine = aiv1alpha1.SGLang @@ -833,27 +755,22 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { context.Background(), updatedMS, metav1.UpdateOptions{}) assert.NoError(t, err) - // Wait for update to propagate gracefully if !waitForCacheSync(t, 5*time.Second, controller.modelServerSynced) { t.Log("Cache sync timeout after update - proceeding anyway") } - // Wait for the updated object to be available in cache found := waitForObjectInCache(t, 2*time.Second, func() bool { ms, err := controller.modelServerLister.ModelServers("test-ns").Get("integration-modelserver") if err != nil { return false } - // Check if the update is reflected return ms.Labels["version"] == "v2" }) assert.True(t, found, "Updated ModelServer should be found in cache after update") - // Process the update err = controller.syncModelServerHandler("test-ns/integration-modelserver") assert.NoError(t, err) - // Verify update storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "test-ns", Name: "integration-modelserver", @@ -863,25 +780,17 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { assert.Equal(t, aiv1alpha1.SGLang, storedMS.Spec.InferenceEngine) } - // Test error handling for non-existent resources err = controller.syncModelServerHandler("test-ns/non-existent-modelserver") - assert.NoError(t, err) // This should work fine for pods + assert.NoError(t, err) err = controller.syncPodHandler("test-ns/non-existent-pod") - assert.NoError(t, err) // This should work fine for pods + assert.NoError(t, err) } -// TestModelServerController_SharedPods tests a scenario where: -// 1. We sync a modelserver (ms1) with corresponding pods -// 2. These pods also belong to another modelserver (ms2) -// 3. Then we sync the second modelserver (ms2) -// 4. Verify that GetPodsByModelServer(ms2) returns all pods correctly func TestModelServerController_SharedPods(t *testing.T) { - // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() - // Create two ModelServers with the same selector (so they match the same pods) ms1 := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -911,7 +820,6 @@ func TestModelServerController_SharedPods(t *testing.T) { }, } - // Create pods that match both ModelServers' selectors pod1 := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -949,7 +857,6 @@ func TestModelServerController_SharedPods(t *testing.T) { }, } - // Add resources to fake clients _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( context.Background(), ms1, metav1.CreateOptions{}) assert.NoError(t, err) @@ -962,15 +869,13 @@ func TestModelServerController_SharedPods(t *testing.T) { context.Background(), pod2, metav1.CreateOptions{}) assert.NoError(t, err) - // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - // Create store store := newStoreWithMockBackend() - // Create controller controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, @@ -982,12 +887,10 @@ func TestModelServerController_SharedPods(t *testing.T) { kthenaInformerFactory.Start(stop) kubeInformerFactory.Start(stop) - // Wait for caches to sync if !waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) { t.Fatal("Failed to sync caches within timeout") } - // Wait for objects to be available in cache waitForObjectInCache(t, 2*time.Second, func() bool { _, err := controller.modelServerLister.ModelServers("default").Get("model1") return err == nil @@ -1007,17 +910,14 @@ func TestModelServerController_SharedPods(t *testing.T) { pod1Name := utils.GetNamespaceName(pod1) pod2Name := utils.GetNamespaceName(pod2) - // Step 1: Sync first modelserver (ms1) using syncModelServerHandler err = controller.syncModelServerHandler("default/model1") assert.NoError(t, err) - // Step 2: Sync pods using syncPodHandler (this will make pods belong to both ms1 and ms2) err = controller.syncPodHandler("default/pod1") assert.NoError(t, err) err = controller.syncPodHandler("default/pod2") assert.NoError(t, err) - // Step 3: Add ms2 to fake client and sync it _, err = kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( context.Background(), ms2, metav1.CreateOptions{}) assert.NoError(t, err) @@ -1027,17 +927,14 @@ func TestModelServerController_SharedPods(t *testing.T) { return err == nil }) - // Sync second modelserver (ms2) using syncModelServerHandler ms2Name := utils.GetNamespaceName(ms2) err = controller.syncModelServerHandler("default/model2") assert.NoError(t, err) - // Step 4: Verify GetPodsByModelServer(ms2) returns all pods pods, err := store.GetPodsByModelServer(ms2Name) assert.NoError(t, err) assert.Len(t, pods, 2, "ms2 should have 2 pods") - // Verify both pods are present podNames := make(map[types.NamespacedName]bool) for _, pod := range pods { podNames[utils.GetNamespaceName(pod.Pod)] = true @@ -1045,12 +942,10 @@ func TestModelServerController_SharedPods(t *testing.T) { assert.True(t, podNames[pod1Name], "pod1 should be returned for ms2") assert.True(t, podNames[pod2Name], "pod2 should be returned for ms2") - // Verify ms1 also has both pods podsMS1, err := store.GetPodsByModelServer(ms1Name) assert.NoError(t, err) assert.Len(t, podsMS1, 2, "ms1 should also have 2 pods") - // Verify pods reference both model servers pod1Info := store.GetPodInfo(pod1Name) assert.NotNil(t, pod1Info) assert.True(t, pod1Info.HasModelServer(ms1Name), "pod1 should reference ms1") @@ -1062,9 +957,6 @@ func TestModelServerController_SharedPods(t *testing.T) { assert.True(t, pod2Info.HasModelServer(ms2Name), "pod2 should reference ms2") } -// Helper functions for testing - -// waitForCacheSync waits for the informer caches to sync with a timeout func waitForCacheSync(t *testing.T, timeout time.Duration, cacheSyncWaiters ...cache.InformerSynced) bool { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -1076,7 +968,6 @@ func waitForCacheSync(t *testing.T, timeout time.Duration, cacheSyncWaiters ...c return true } -// waitForObjectInCache waits for a specific object to appear in the cache func waitForObjectInCache(t *testing.T, timeout time.Duration, checkFunc func() bool) bool { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() From 40cbf0cd2a671d357bb664c368b929a36763babf Mon Sep 17 00:00:00 2001 From: vivek41-glitch Date: Fri, 26 Jun 2026 20:39:00 +0530 Subject: [PATCH 02/11] fix: add retry.RetryOnConflict for status updates and fix imports Signed-off-by: Leo Xie --- .../networking/v1alpha1/modelserver_types.go | 3 +- .../controller/modelroute_controller.go | 48 ++++++++---- .../controller/modelserver_controller.go | 77 +++++++++++++------ 3 files changed, 84 insertions(+), 44 deletions(-) diff --git a/pkg/apis/networking/v1alpha1/modelserver_types.go b/pkg/apis/networking/v1alpha1/modelserver_types.go index d334f3988..fe4dae5bd 100644 --- a/pkg/apis/networking/v1alpha1/modelserver_types.go +++ b/pkg/apis/networking/v1alpha1/modelserver_types.go @@ -159,10 +159,9 @@ type ModelServerStatus struct { // Conditions track the lifecycle of this ModelServer. // Types: - // - "Ready": true when at least one pod is matched and the ModelServer has been + // - "Ready": true when at least one ready pod is matched and the ModelServer has been // registered with the router store. Signals that `kubectl wait --for=condition=ready` // can be used. - // - "Reconciled": true when the last reconciliation completed successfully. // +optional // +patchMergeKey=type // +patchStrategy=merge diff --git a/pkg/kthena-router/controller/modelroute_controller.go b/pkg/kthena-router/controller/modelroute_controller.go index 5c519f48f..f5776ed34 100644 --- a/pkg/kthena-router/controller/modelroute_controller.go +++ b/pkg/kthena-router/controller/modelroute_controller.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" @@ -154,27 +155,40 @@ func (c *ModelRouteController) syncHandler(key string) error { return err } - c.updateModelRouteStatus(mr) - - return nil + return c.updateModelRouteStatus(mr) } -func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) { - mrCopy := mr.DeepCopy() - mrCopy.Status.ObservedGeneration = mr.Generation +func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) error { + // Check if status is already up-to-date + if mr.Status.ObservedGeneration == mr.Generation { + if cond := meta.FindStatusCondition(mr.Status.Conditions, "Ready"); cond != nil && + cond.Status == metav1.ConditionTrue && + cond.Reason == "RouteRegistered" { + return nil + } + } - meta.SetStatusCondition(&mrCopy.Status.Conditions, metav1.Condition{ - Type: "Ready", - Status: metav1.ConditionTrue, - Reason: "RouteRegistered", - Message: "ModelRoute registered in router store", - }) + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + // Get latest version to avoid conflicts + latest, err := c.modelRouteLister.ModelRoutes(mr.Namespace).Get(mr.Name) + if err != nil { + return err + } - ctx := context.Background() - _, err := c.kthenaClient.NetworkingV1alpha1().ModelRoutes(mr.Namespace).UpdateStatus(ctx, mrCopy, metav1.UpdateOptions{}) - if err != nil { - klog.Errorf("Failed to update ModelRoute status for %s/%s: %v", mr.Namespace, mr.Name, err) - } + mrCopy := latest.DeepCopy() + mrCopy.Status.ObservedGeneration = latest.Generation + + meta.SetStatusCondition(&mrCopy.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "RouteRegistered", + Message: "ModelRoute registered in router store", + }) + + ctx := context.Background() + _, err = c.kthenaClient.NetworkingV1alpha1().ModelRoutes(mr.Namespace).UpdateStatus(ctx, mrCopy, metav1.UpdateOptions{}) + return err + }) } func (c *ModelRouteController) enqueueModelRoute(obj interface{}) { diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index cc85bab3c..1f8ffbc09 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" @@ -45,7 +46,6 @@ import ( "github.com/volcano-sh/kthena/pkg/kthena-router/utils" ) - type ResourceType string const ( @@ -210,7 +210,10 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { _ = c.store.AddOrUpdateModelServer(ms, pods) - c.updateModelServerStatus(ms, podList, pods) + if err := c.updateModelServerStatus(ms, podList, pods); err != nil { + klog.Errorf("Failed to update ModelServer status for %s/%s: %v", ms.Namespace, ms.Name, err) + return err + } existingPods, err := c.store.GetPodsByModelServer(utils.GetNamespaceName(ms)) if err != nil { @@ -250,33 +253,57 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { return nil } -func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServer, podList []*corev1.Pod, readyPods sets.Set[types.NamespacedName]) { - msCopy := ms.DeepCopy() - msCopy.Status.ObservedGeneration = ms.Generation - msCopy.Status.MatchedReplicas = int32(len(podList)) - msCopy.Status.ReadyReplicas = int32(readyPods.Len()) - +func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServer, podList []*corev1.Pod, readyPods sets.Set[types.NamespacedName]) error { + expectedReadyStatus := metav1.ConditionFalse + expectedReason := "NoReadyPods" if readyPods.Len() > 0 { - meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ - Type: "Ready", - Status: metav1.ConditionTrue, - Reason: "PodsReady", - Message: fmt.Sprintf("%d ready pods matched", readyPods.Len()), - }) - } else { - meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ - Type: "Ready", - Status: metav1.ConditionFalse, - Reason: "NoReadyPods", - Message: "No ready pods match workloadSelector", - }) + expectedReadyStatus = metav1.ConditionTrue + expectedReason = "PodsReady" } - ctx := context.Background() - _, err := c.kthenaClient.NetworkingV1alpha1().ModelServers(ms.Namespace).UpdateStatus(ctx, msCopy, metav1.UpdateOptions{}) - if err != nil { - klog.Errorf("Failed to update ModelServer status for %s/%s: %v", ms.Namespace, ms.Name, err) + // Check if status is already up-to-date + if ms.Status.ObservedGeneration == ms.Generation && + ms.Status.MatchedReplicas == int32(len(podList)) && + ms.Status.ReadyReplicas == int32(readyPods.Len()) { + if cond := meta.FindStatusCondition(ms.Status.Conditions, "Ready"); cond != nil && + cond.Status == expectedReadyStatus && + cond.Reason == expectedReason { + return nil + } } + + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + // Get latest version to avoid conflicts + latest, err := c.modelServerLister.ModelServers(ms.Namespace).Get(ms.Name) + if err != nil { + return err + } + + msCopy := latest.DeepCopy() + msCopy.Status.ObservedGeneration = latest.Generation + msCopy.Status.MatchedReplicas = int32(len(podList)) + msCopy.Status.ReadyReplicas = int32(readyPods.Len()) + + if readyPods.Len() > 0 { + meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "PodsReady", + Message: fmt.Sprintf("%d ready pods matched", readyPods.Len()), + }) + } else { + meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionFalse, + Reason: "NoReadyPods", + Message: "No ready pods match workloadSelector", + }) + } + + ctx := context.Background() + _, err = c.kthenaClient.NetworkingV1alpha1().ModelServers(ms.Namespace).UpdateStatus(ctx, msCopy, metav1.UpdateOptions{}) + return err + }) } func (c *ModelServerController) syncPodHandler(key string) error { From 7f898a26f7065979b47f0626488661603fd10fac Mon Sep 17 00:00:00 2001 From: vivek41-glitch Date: Tue, 30 Jun 2026 16:00:06 +0800 Subject: [PATCH 03/11] fix: restore initialSyncSignal, use API client in retry loop, add error handling Signed-off-by: Leo Xie --- .../controller/modelroute_controller.go | 10 +++++----- .../controller/modelroute_controller_test.go | 12 ++++++++++++ .../controller/modelserver_controller.go | 10 ++++++---- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pkg/kthena-router/controller/modelroute_controller.go b/pkg/kthena-router/controller/modelroute_controller.go index f5776ed34..83629bbcb 100644 --- a/pkg/kthena-router/controller/modelroute_controller.go +++ b/pkg/kthena-router/controller/modelroute_controller.go @@ -84,7 +84,7 @@ func (c *ModelRouteController) Run(stopCh <-chan struct{}) error { if ok := cache.WaitForCacheSync(stopCh, c.registration.HasSynced); !ok { return fmt.Errorf("failed to wait for caches to sync") } - c.workqueue.Add(struct{}{}) + c.workqueue.Add(initialSyncSignal) go wait.Until(c.runWorker, time.Second, stopCh) @@ -108,7 +108,7 @@ func (c *ModelRouteController) processNextWorkItem() bool { } defer c.workqueue.Done(obj) - if obj == struct{}{} { + if obj == initialSyncSignal { klog.V(2).Info("initial model routes have been synced") c.workqueue.Forget(obj) c.initialSync.Store(true) @@ -169,8 +169,9 @@ func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) } return retry.RetryOnConflict(retry.DefaultRetry, func() error { - // Get latest version to avoid conflicts - latest, err := c.modelRouteLister.ModelRoutes(mr.Namespace).Get(mr.Name) + // Get latest version from API server to avoid conflicts + ctx := context.Background() + latest, err := c.kthenaClient.NetworkingV1alpha1().ModelRoutes(mr.Namespace).Get(ctx, mr.Name, metav1.GetOptions{}) if err != nil { return err } @@ -185,7 +186,6 @@ func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) Message: "ModelRoute registered in router store", }) - ctx := context.Background() _, err = c.kthenaClient.NetworkingV1alpha1().ModelRoutes(mr.Namespace).UpdateStatus(ctx, mrCopy, metav1.UpdateOptions{}) return err }) diff --git a/pkg/kthena-router/controller/modelroute_controller_test.go b/pkg/kthena-router/controller/modelroute_controller_test.go index c3924b943..698266f97 100644 --- a/pkg/kthena-router/controller/modelroute_controller_test.go +++ b/pkg/kthena-router/controller/modelroute_controller_test.go @@ -41,6 +41,8 @@ func TestModelRouteController_Lifecycle(t *testing.T) { defer close(stop) kthenaInformerFactory.Start(stop) + // this block verifies that creating a ModelRoute via fake client causes it to + // appear in the informer cache and get synced into the datastore. t.Run("ModelRouteCreate", func(t *testing.T) { mr := &aiv1alpha1.ModelRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -83,6 +85,8 @@ func TestModelRouteController_Lifecycle(t *testing.T) { assert.Equal(t, "test-model", storedRoute.Spec.ModelName) }) + // this will verify that updating a ModelRoute spec is reflected in the + // informer cache and the datastore after syncHandler is called. t.Run("ModelRouteUpdate", func(t *testing.T) { existing, err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Get( context.Background(), "test-modelroute", metav1.GetOptions{}) @@ -116,6 +120,8 @@ func TestModelRouteController_Lifecycle(t *testing.T) { assert.Equal(t, "updated-server", storedRoute.Spec.Rules[0].TargetModels[0].ModelServerName) }) + // verifies that deleting a ModelRoute removes it from the informer + // cache and datastore after syncHandler is called. t.Run("ModelRouteDelete", func(t *testing.T) { err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Delete( context.Background(), "test-modelroute", metav1.DeleteOptions{}) @@ -146,11 +152,14 @@ func TestModelRouteController_ErrorHandling(t *testing.T) { defer close(stop) kthenaInformerFactory.Start(stop) + // this will verify if a malformed key is handled without returning an error t.Run("InvalidKey", func(t *testing.T) { err := controller.syncHandler("invalid/key/format") assert.NoError(t, err) }) + // this will verify that syncing a key for a resource that does not exist + // is a no-op and does not return an error t.Run("NonExistentModelRoute", func(t *testing.T) { err := controller.syncHandler("default/non-existent") assert.NoError(t, err) @@ -168,6 +177,8 @@ func TestModelRouteController_WorkQueueProcessing(t *testing.T) { defer close(stop) kthenaInformerFactory.Start(stop) + // verifies that processing the initialSyncSignal sentinel value + // marks the controller as synced via HasSynced() t.Run("InitialSyncSignal", func(t *testing.T) { assert.False(t, controller.HasSynced()) controller.workqueue.Add(struct{}{}) @@ -175,6 +186,7 @@ func TestModelRouteController_WorkQueueProcessing(t *testing.T) { assert.True(t, controller.HasSynced()) }) + // verifies that an unexpected type in the workqueue is dropped without crashing the worker t.Run("UnknownResourceType", func(t *testing.T) { controller.workqueue.Add(12345) result := controller.processNextWorkItem() diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index 1f8ffbc09..f551836e5 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -208,7 +208,9 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { } } - _ = c.store.AddOrUpdateModelServer(ms, pods) + if err := c.store.AddOrUpdateModelServer(ms, pods); err != nil { + return err + } if err := c.updateModelServerStatus(ms, podList, pods); err != nil { klog.Errorf("Failed to update ModelServer status for %s/%s: %v", ms.Namespace, ms.Name, err) @@ -273,8 +275,9 @@ func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServ } return retry.RetryOnConflict(retry.DefaultRetry, func() error { - // Get latest version to avoid conflicts - latest, err := c.modelServerLister.ModelServers(ms.Namespace).Get(ms.Name) + // Get latest version from API server to avoid conflicts + ctx := context.Background() + latest, err := c.kthenaClient.NetworkingV1alpha1().ModelServers(ms.Namespace).Get(ctx, ms.Name, metav1.GetOptions{}) if err != nil { return err } @@ -300,7 +303,6 @@ func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServ }) } - ctx := context.Background() _, err = c.kthenaClient.NetworkingV1alpha1().ModelServers(ms.Namespace).UpdateStatus(ctx, msCopy, metav1.UpdateOptions{}) return err }) From d83217f3a9c9ff65f1a0e313fbdf7b6649ee4137 Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Fri, 3 Jul 2026 16:06:06 +0800 Subject: [PATCH 04/11] chore: restore old tests Signed-off-by: Leo Xie --- .../controller/modelroute_controller.go | 25 ++-- .../controller/modelroute_controller_test.go | 2 +- .../controller/modelserver_controller.go | 12 +- .../controller/modelserver_controller_test.go | 134 ++++++++++++++++-- 4 files changed, 149 insertions(+), 24 deletions(-) diff --git a/pkg/kthena-router/controller/modelroute_controller.go b/pkg/kthena-router/controller/modelroute_controller.go index 83629bbcb..2d5bd7d2b 100644 --- a/pkg/kthena-router/controller/modelroute_controller.go +++ b/pkg/kthena-router/controller/modelroute_controller.go @@ -25,25 +25,25 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/util/retry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/retry" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" + kthenaclient "github.com/volcano-sh/kthena/client-go/clientset/versioned" informersv1alpha1 "github.com/volcano-sh/kthena/client-go/informers/externalversions" listerv1alpha1 "github.com/volcano-sh/kthena/client-go/listers/networking/v1alpha1" aiv1alpha1 "github.com/volcano-sh/kthena/pkg/apis/networking/v1alpha1" - kthenaclient "github.com/volcano-sh/kthena/client-go/clientset/versioned" "github.com/volcano-sh/kthena/pkg/kthena-router/datastore" ) type ModelRouteController struct { - kthenaClient kthenaclient.Interface - modelRouteLister listerv1alpha1.ModelRouteLister - modelRouteSynced cache.InformerSynced - registration cache.ResourceEventHandlerRegistration + kthenaClient kthenaclient.Interface + modelRouteLister listerv1alpha1.ModelRouteLister + modelRouteSynced cache.InformerSynced + registration cache.ResourceEventHandlerRegistration workqueue workqueue.TypedRateLimitingInterface[any] initialSync *atomic.Bool @@ -58,12 +58,12 @@ func NewModelRouteController( modelRouteInformer := kthenaInformerFactory.Networking().V1alpha1().ModelRoutes() controller := &ModelRouteController{ - kthenaClient: kthenaClient, - modelRouteLister: modelRouteInformer.Lister(), - modelRouteSynced: modelRouteInformer.Informer().HasSynced, - workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any]()), - initialSync: &atomic.Bool{}, - store: store, + kthenaClient: kthenaClient, + modelRouteLister: modelRouteInformer.Lister(), + modelRouteSynced: modelRouteInformer.Informer().HasSynced, + workqueue: workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any]()), + initialSync: &atomic.Bool{}, + store: store, } controller.registration, _ = modelRouteInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -84,6 +84,7 @@ func (c *ModelRouteController) Run(stopCh <-chan struct{}) error { if ok := cache.WaitForCacheSync(stopCh, c.registration.HasSynced); !ok { return fmt.Errorf("failed to wait for caches to sync") } + // add initialSync signal c.workqueue.Add(initialSyncSignal) go wait.Until(c.runWorker, time.Second, stopCh) diff --git a/pkg/kthena-router/controller/modelroute_controller_test.go b/pkg/kthena-router/controller/modelroute_controller_test.go index 698266f97..4cbd94e60 100644 --- a/pkg/kthena-router/controller/modelroute_controller_test.go +++ b/pkg/kthena-router/controller/modelroute_controller_test.go @@ -181,7 +181,7 @@ func TestModelRouteController_WorkQueueProcessing(t *testing.T) { // marks the controller as synced via HasSynced() t.Run("InitialSyncSignal", func(t *testing.T) { assert.False(t, controller.HasSynced()) - controller.workqueue.Add(struct{}{}) + controller.workqueue.Add(initialSyncSignal) controller.processNextWorkItem() assert.True(t, controller.HasSynced()) }) diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index f551836e5..90fa5c497 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -29,19 +29,19 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/retry" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" + kthenaclient "github.com/volcano-sh/kthena/client-go/clientset/versioned" informersv1alpha1 "github.com/volcano-sh/kthena/client-go/informers/externalversions" listerv1alpha1 "github.com/volcano-sh/kthena/client-go/listers/networking/v1alpha1" aiv1alpha1 "github.com/volcano-sh/kthena/pkg/apis/networking/v1alpha1" - kthenaclient "github.com/volcano-sh/kthena/client-go/clientset/versioned" "github.com/volcano-sh/kthena/pkg/kthena-router/datastore" "github.com/volcano-sh/kthena/pkg/kthena-router/utils" ) @@ -217,11 +217,13 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { return err } + // Get already bound pods to avoid unnecessary updates existingPods, err := c.store.GetPodsByModelServer(utils.GetNamespaceName(ms)) if err != nil { klog.V(4).Infof("failed to get existing pods for ModelServer %s/%s: %v", ms.Namespace, ms.Name, err) } + // Build a set of existing pod names that are already bound to the model server existingPodNames := sets.New[types.NamespacedName]() for _, podInfo := range existingPods { pod := podInfo.GetPod() @@ -229,6 +231,7 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { continue } if !podInfo.HasModelServer(utils.GetNamespaceName(ms)) { + // If the pod is not bound to the model server, establish the binding if err := c.store.AppendModelServerToPod(pod, []*aiv1alpha1.ModelServer{ms}); err != nil { klog.Warningf("failed to append modelserver %s/%s to pod %s/%s: %v", ms.Namespace, ms.Name, pod.Namespace, pod.Name, err) continue @@ -237,12 +240,14 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { existingPodNames.Insert(utils.GetNamespaceName(pod)) } + // Add new pods that are not yet bound to the store for _, pod := range podList { if !isPodReady(pod) { continue } podName := utils.GetNamespaceName(pod) + // Skip pods that are already properly bound if existingPodNames.Contains(podName) { continue } @@ -332,6 +337,8 @@ func (c *ModelServerController) syncPodHandler(key string) error { return c.addOrUpdatePod(pod) } +// addOrUpdatePod finds all ModelServers that match the given pod +// and adds or updates the pod-server binding in the data store func (c *ModelServerController) addOrUpdatePod(pod *corev1.Pod) error { modelServers, err := c.modelServerLister.ModelServers(pod.Namespace).List(labels.Everything()) if err != nil { @@ -382,6 +389,7 @@ func (c *ModelServerController) enqueuePod(obj interface{}) { }) } +// isPodReady checks if the pod is in a running state and has a PodReady condition set to true. func isPodReady(pod *corev1.Pod) bool { if pod.DeletionTimestamp != nil || pod.Status.Phase != corev1.PodRunning { return false diff --git a/pkg/kthena-router/controller/modelserver_controller_test.go b/pkg/kthena-router/controller/modelserver_controller_test.go index 247d85185..174a3e9ee 100644 --- a/pkg/kthena-router/controller/modelserver_controller_test.go +++ b/pkg/kthena-router/controller/modelserver_controller_test.go @@ -58,14 +58,18 @@ func newStoreWithMockBackend() datastore.Store { } func TestModelServerController_ModelServerLifecycle(t *testing.T) { + // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + // Create store store := newStoreWithMockBackend() + // Create controller controller := NewModelServerController( kthenaClient, kthenaInformerFactory, @@ -80,6 +84,7 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { kthenaInformerFactory.Start(stop) kubeInformerFactory.Start(stop) + // Test Case 1: ModelServer Creation t.Run("ModelServerCreate", func(t *testing.T) { ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ @@ -96,6 +101,7 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { }, } + // Wait for cache to sync gracefully if !waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) { t.Fatal("Failed to sync caches within timeout") } @@ -104,12 +110,15 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver") require.NoError(t, err) + // Simulate controller receiving the event controller.enqueueModelServer(ms) assert.Equal(t, 1, controller.workqueue.Len()) + // Process the queue item err = controller.syncModelServerHandler("default/test-modelserver") assert.NoError(t, err) + // Verify ModelServer was added to store storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver", @@ -118,6 +127,7 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { assert.Equal(t, "test-modelserver", storedMS.Name) }) + // Test Case 2: ModelServer Update t.Run("ModelServerUpdate", func(t *testing.T) { ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ @@ -141,10 +151,12 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-update") require.NoError(t, err) + // Process initial creation controller.enqueueModelServer(ms) err = controller.syncModelServerHandler("default/test-modelserver-update") assert.NoError(t, err) + // Update ModelServer updatedMS := ms.DeepCopy() updatedMS.Labels["version"] = "v2" updatedMS.Spec.WorkloadSelector.MatchLabels["environment"] = "production" @@ -154,7 +166,9 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { require.NoError(t, err) assert.Equal(t, "v2", cachedMS.Labels["version"]) + // Simulate controller receiving update event controller.enqueueModelServer(updatedMS) + // Clear any previous items from queue for controller.workqueue.Len() > 0 { item, _ := controller.workqueue.Get() controller.workqueue.Done(item) @@ -163,9 +177,11 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { controller.enqueueModelServer(updatedMS) assert.Equal(t, 1, controller.workqueue.Len()) + // Process the update err = controller.syncModelServerHandler("default/test-modelserver-update") assert.NoError(t, err) + // Verify updated ModelServer in store storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver-update", @@ -175,6 +191,7 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { assert.Equal(t, "production", storedMS.Spec.WorkloadSelector.MatchLabels["environment"]) }) + // Test Case 3: ModelServer Deletion t.Run("ModelServerDelete", func(t *testing.T) { ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ @@ -195,9 +212,11 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") require.NoError(t, err) + // Process creation err = controller.syncModelServerHandler("default/test-modelserver-delete") assert.NoError(t, err) + // Verify it exists in store storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver-delete", @@ -208,9 +227,11 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { _, err = controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") assert.Error(t, err) + // Process the deletion - this should handle the NotFound error gracefully err = controller.syncModelServerHandler("default/test-modelserver-delete") assert.NoError(t, err) + // Verify ModelServer was removed from store storedMS = store.GetModelServer(types.NamespacedName{ Namespace: "default", Name: "test-modelserver-delete", @@ -220,9 +241,11 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { } func TestModelServerController_PodLifecycle(t *testing.T) { + // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create a ModelServer first to associate pods with ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -241,11 +264,14 @@ func TestModelServerController_PodLifecycle(t *testing.T) { context.Background(), ms, metav1.CreateOptions{}) assert.NoError(t, err) + // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + // Create store store := newStoreWithMockBackend() + // Create controller controller := NewModelServerController( kthenaClient, kthenaInformerFactory, @@ -262,6 +288,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { kubeInformerFactory.Start(stop) waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) + // Test Case 1: Pod Creation (Ready Pod) t.Run("PodCreateReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -282,6 +309,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } + // Add Pod to fake client _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) @@ -293,6 +321,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should be found in store after creation") }) + // Test Case 2: Pod Creation (Not Ready Pod) t.Run("PodCreateNotReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -303,7 +332,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, }, Status: corev1.PodStatus{ - Phase: corev1.PodPending, + Phase: corev1.PodPending, // Not running Conditions: []corev1.PodCondition{ { Type: corev1.PodReady, @@ -313,6 +342,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } + // Add Pod to fake client _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) @@ -323,6 +353,8 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }) assert.True(t, sync, "Pod should be found in lister after creation") + // Since pod is not ready, it should be deleted from store (or not added) + // The exact verification depends on your store implementation sync = waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 1 && pods[0].Pod.Name == "test-pod-ready" @@ -330,6 +362,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should be found in store after creation") }) + // Test Case 3: Pod Update (Becomes Ready) t.Run("PodUpdateBecomesReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -350,10 +383,12 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } + // Create initial pod (not ready) _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) + // Update pod to be ready updatedPod := pod.DeepCopy() updatedPod.Status.Phase = corev1.PodRunning updatedPod.Status.Conditions[0].Status = corev1.ConditionTrue @@ -362,6 +397,8 @@ func TestModelServerController_PodLifecycle(t *testing.T) { context.Background(), updatedPod, metav1.UpdateOptions{}) assert.NoError(t, err) + // Verify pod is now considered ready + // The exact verification depends on your store implementation sync := waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 2 && @@ -370,6 +407,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should be found in store after update") }) + // Test Case 4: Pod Update (Becomes Not Ready) t.Run("PodUpdateBecomesNotReady", func(t *testing.T) { pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -390,16 +428,19 @@ func TestModelServerController_PodLifecycle(t *testing.T) { }, } + // Create initial pod (ready) _, err := kubeClient.CoreV1().Pods("default").Create( context.Background(), pod, metav1.CreateOptions{}) assert.NoError(t, err) + // Verify pod was in store since it's ready sync := waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 3 }) assert.True(t, sync, "Pod should be found in store after creation") + // Update pod to not ready updatedPod := pod.DeepCopy() updatedPod.Status.Phase = corev1.PodFailed updatedPod.Status.Conditions[0].Status = corev1.ConditionFalse @@ -408,6 +449,7 @@ func TestModelServerController_PodLifecycle(t *testing.T) { context.Background(), updatedPod, metav1.UpdateOptions{}) assert.NoError(t, err) + // Verify pod was removed from store since it's not ready sync = waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 2 && (pods[0].Pod.Name != "test-pod-update-not-ready" && pods[1].Pod.Name != "test-pod-update-not-ready") @@ -415,13 +457,17 @@ func TestModelServerController_PodLifecycle(t *testing.T) { assert.True(t, sync, "Pod should not be found in store after creation") }) + // Test Case 5: Pod Deletion t.Run("PodDelete", func(t *testing.T) { + // Delete all pods for _, podName := range []string{"test-pod-ready", "test-pod-not-ready", "test-pod-update-ready", "test-pod-update-not-ready"} { err = kubeClient.CoreV1().Pods("default").Delete( context.Background(), podName, metav1.DeleteOptions{}) assert.NoError(t, err) } + // Verify pod was removed from store + // Verify pod was removed from store since it's not ready sync := waitForObjectInCache(t, 2*time.Second, func() bool { pods, _ := store.GetPodsByModelServer(utils.GetNamespaceName(ms)) return len(pods) == 0 @@ -431,14 +477,18 @@ func TestModelServerController_PodLifecycle(t *testing.T) { } func TestModelServerController_ErrorHandling(t *testing.T) { + // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + // Create store store := newStoreWithMockBackend() + // Create controller controller := NewModelServerController( kthenaClient, kthenaInformerFactory, @@ -446,36 +496,44 @@ func TestModelServerController_ErrorHandling(t *testing.T) { store, ) + // Test Case 1: Invalid ModelServer Key t.Run("InvalidModelServerKey", func(t *testing.T) { err := controller.syncModelServerHandler("invalid-key-format") - assert.NoError(t, err) + assert.NoError(t, err) // Should handle gracefully and return nil }) + // Test Case 2: Invalid Pod Key t.Run("InvalidPodKey", func(t *testing.T) { err := controller.syncPodHandler("invalid-key-format") - assert.NoError(t, err) + assert.NoError(t, err) // Should handle gracefully and return nil }) + // Test Case 3: Non-existent ModelServer t.Run("NonExistentModelServer", func(t *testing.T) { err := controller.syncModelServerHandler("default/non-existent-modelserver") - assert.NoError(t, err) + assert.NoError(t, err) // Should handle NotFound error gracefully }) + // Test Case 4: Non-existent Pod t.Run("NonExistentPod", func(t *testing.T) { err := controller.syncPodHandler("default/non-existent-pod") - assert.NoError(t, err) + assert.NoError(t, err) // Should handle NotFound error gracefully }) } func TestModelServerController_WorkQueueProcessing(t *testing.T) { + // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + // Create store store := newStoreWithMockBackend() + // Create controller controller := NewModelServerController( kthenaClient, kthenaInformerFactory, @@ -483,16 +541,20 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { store, ) + // Test Case 1: Initial Sync Signal t.Run("InitialSyncSignal", func(t *testing.T) { + // Add initial sync signal (empty QueueItem) controller.workqueue.Add(QueueItem{}) assert.Equal(t, 1, controller.workqueue.Len()) + // Process the initial sync signal processed := controller.processNextWorkItem() assert.True(t, processed) assert.True(t, controller.HasSynced()) assert.Equal(t, 0, controller.workqueue.Len()) }) + // Test Case 2: Unknown Resource Type t.Run("UnknownResourceType", func(t *testing.T) { unknownItem := QueueItem{ ResourceType: "UnknownType", @@ -502,12 +564,15 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { controller.workqueue.Add(unknownItem) assert.Equal(t, 1, controller.workqueue.Len()) + // Process unknown resource type processed := controller.processNextWorkItem() assert.True(t, processed) assert.Equal(t, 0, controller.workqueue.Len()) }) + // Test Case 3: Multiple Queue Items t.Run("MultipleQueueItems", func(t *testing.T) { + // Add multiple items to queue items := []QueueItem{ {ResourceType: ResourceTypeModelServer, Key: "default/ms1"}, {ResourceType: ResourceTypePod, Key: "default/pod1"}, @@ -520,6 +585,7 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { } assert.Equal(t, 4, controller.workqueue.Len()) + // Process all items processedCount := 0 for controller.workqueue.Len() > 0 { processed := controller.processNextWorkItem() @@ -532,14 +598,18 @@ func TestModelServerController_WorkQueueProcessing(t *testing.T) { } func TestModelServerController_PodSelectionLogic(t *testing.T) { + // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + // Create store store := newStoreWithMockBackend() + // Create controller controller := NewModelServerController( kthenaClient, kthenaInformerFactory, @@ -553,7 +623,9 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { kthenaInformerFactory.Start(stop) kubeInformerFactory.Start(stop) + // Test Case: Pod with Non-matching Labels t.Run("PodWithNonMatchingLabels", func(t *testing.T) { + // Create ModelServer with specific selector ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -574,12 +646,13 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { context.Background(), ms, metav1.CreateOptions{}) assert.NoError(t, err) + // Create pod with non-matching labels podNonMatching := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "test-pod-non-matching", Labels: map[string]string{ - "app": "different-model", + "app": "different-model", // Doesn't match ModelServer selector "version": "v2", }, }, @@ -603,16 +676,18 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { return pods != nil && pods.Name == "test-pod-non-matching" }) + // Process the pod - should not be associated with ModelServer controller.enqueuePod(podNonMatching) err = controller.syncPodHandler("default/test-pod-non-matching") assert.NoError(t, err) + // Create pod with matching labels podMatching := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "test-pod-matching", Labels: map[string]string{ - "app": "specific-model", + "app": "specific-model", // Matches ModelServer selector "version": "v1", }, }, @@ -640,9 +715,12 @@ func TestModelServerController_PodSelectionLogic(t *testing.T) { } func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { + // Create a comprehensive test that tests the full workflow + // with proper informer setup and timing kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create and add resources to fake clients BEFORE starting informers ms := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test-ns", @@ -699,6 +777,7 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { }, } + // Add resources to fake clients _, err := kthenaClient.NetworkingV1alpha1().ModelServers("test-ns").Create( context.Background(), ms, metav1.CreateOptions{}) assert.NoError(t, err) @@ -711,12 +790,14 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { context.Background(), notReadyPod, metav1.CreateOptions{}) assert.NoError(t, err) + // Create informer factories and start them kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) stopCh := make(chan struct{}) defer close(stopCh) + // Create controller and store store := newStoreWithMockBackend() controller := NewModelServerController( kthenaClient, @@ -733,6 +814,7 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { return err == nil && ms.Name == "integration-modelserver" }) + // Test ModelServer processing err = controller.syncModelServerHandler("test-ns/integration-modelserver") assert.NoError(t, err) @@ -741,12 +823,14 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { return err == nil && len(ret) == 2 }) + // Test Pod processing err = controller.syncPodHandler("test-ns/ready-pod") assert.NoError(t, err) err = controller.syncPodHandler("test-ns/not-ready-pod") assert.NoError(t, err) + // Test update scenario updatedMS := ms.DeepCopy() updatedMS.Labels["version"] = "v2" updatedMS.Spec.InferenceEngine = aiv1alpha1.SGLang @@ -755,22 +839,27 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { context.Background(), updatedMS, metav1.UpdateOptions{}) assert.NoError(t, err) + // Wait for update to propagate gracefully if !waitForCacheSync(t, 5*time.Second, controller.modelServerSynced) { t.Log("Cache sync timeout after update - proceeding anyway") } + // Wait for the updated object to be available in cache found := waitForObjectInCache(t, 2*time.Second, func() bool { ms, err := controller.modelServerLister.ModelServers("test-ns").Get("integration-modelserver") if err != nil { return false } + // Check if the update is reflected return ms.Labels["version"] == "v2" }) assert.True(t, found, "Updated ModelServer should be found in cache after update") + // Process the update err = controller.syncModelServerHandler("test-ns/integration-modelserver") assert.NoError(t, err) + // Verify update storedMS := store.GetModelServer(types.NamespacedName{ Namespace: "test-ns", Name: "integration-modelserver", @@ -780,17 +869,25 @@ func TestModelServerController_ComprehensiveLifecycleTest(t *testing.T) { assert.Equal(t, aiv1alpha1.SGLang, storedMS.Spec.InferenceEngine) } + // Test error handling for non-existent resources err = controller.syncModelServerHandler("test-ns/non-existent-modelserver") - assert.NoError(t, err) + assert.NoError(t, err) // This should work fine for pods err = controller.syncPodHandler("test-ns/non-existent-pod") - assert.NoError(t, err) + assert.NoError(t, err) // This should work fine for pods } +// TestModelServerController_SharedPods tests a scenario where: +// 1. We sync a modelserver (ms1) with corresponding pods +// 2. These pods also belong to another modelserver (ms2) +// 3. Then we sync the second modelserver (ms2) +// 4. Verify that GetPodsByModelServer(ms2) returns all pods correctly func TestModelServerController_SharedPods(t *testing.T) { + // Create fake clients kubeClient := kubefake.NewSimpleClientset() kthenaClient := kthenafake.NewSimpleClientset() + // Create two ModelServers with the same selector (so they match the same pods) ms1 := &aiv1alpha1.ModelServer{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -820,6 +917,7 @@ func TestModelServerController_SharedPods(t *testing.T) { }, } + // Create pods that match both ModelServers' selectors pod1 := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -857,6 +955,7 @@ func TestModelServerController_SharedPods(t *testing.T) { }, } + // Add resources to fake clients _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( context.Background(), ms1, metav1.CreateOptions{}) assert.NoError(t, err) @@ -869,11 +968,14 @@ func TestModelServerController_SharedPods(t *testing.T) { context.Background(), pod2, metav1.CreateOptions{}) assert.NoError(t, err) + // Create informer factories kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + // Create store store := newStoreWithMockBackend() + // Create controller controller := NewModelServerController( kthenaClient, kthenaInformerFactory, @@ -887,10 +989,12 @@ func TestModelServerController_SharedPods(t *testing.T) { kthenaInformerFactory.Start(stop) kubeInformerFactory.Start(stop) + // Wait for caches to sync if !waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) { t.Fatal("Failed to sync caches within timeout") } + // Wait for objects to be available in cache waitForObjectInCache(t, 2*time.Second, func() bool { _, err := controller.modelServerLister.ModelServers("default").Get("model1") return err == nil @@ -910,14 +1014,17 @@ func TestModelServerController_SharedPods(t *testing.T) { pod1Name := utils.GetNamespaceName(pod1) pod2Name := utils.GetNamespaceName(pod2) + // Step 1: Sync first modelserver (ms1) using syncModelServerHandler err = controller.syncModelServerHandler("default/model1") assert.NoError(t, err) + // Step 2: Sync pods using syncPodHandler (this will make pods belong to both ms1 and ms2) err = controller.syncPodHandler("default/pod1") assert.NoError(t, err) err = controller.syncPodHandler("default/pod2") assert.NoError(t, err) + // Step 3: Add ms2 to fake client and sync it _, err = kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( context.Background(), ms2, metav1.CreateOptions{}) assert.NoError(t, err) @@ -927,14 +1034,17 @@ func TestModelServerController_SharedPods(t *testing.T) { return err == nil }) + // Sync second modelserver (ms2) using syncModelServerHandler ms2Name := utils.GetNamespaceName(ms2) err = controller.syncModelServerHandler("default/model2") assert.NoError(t, err) + // Step 4: Verify GetPodsByModelServer(ms2) returns all pods pods, err := store.GetPodsByModelServer(ms2Name) assert.NoError(t, err) assert.Len(t, pods, 2, "ms2 should have 2 pods") + // Verify both pods are present podNames := make(map[types.NamespacedName]bool) for _, pod := range pods { podNames[utils.GetNamespaceName(pod.Pod)] = true @@ -942,10 +1052,12 @@ func TestModelServerController_SharedPods(t *testing.T) { assert.True(t, podNames[pod1Name], "pod1 should be returned for ms2") assert.True(t, podNames[pod2Name], "pod2 should be returned for ms2") + // Verify ms1 also has both pods podsMS1, err := store.GetPodsByModelServer(ms1Name) assert.NoError(t, err) assert.Len(t, podsMS1, 2, "ms1 should also have 2 pods") + // Verify pods reference both model servers pod1Info := store.GetPodInfo(pod1Name) assert.NotNil(t, pod1Info) assert.True(t, pod1Info.HasModelServer(ms1Name), "pod1 should reference ms1") @@ -957,6 +1069,9 @@ func TestModelServerController_SharedPods(t *testing.T) { assert.True(t, pod2Info.HasModelServer(ms2Name), "pod2 should reference ms2") } +// Helper functions for testing + +// waitForCacheSync waits for the informer caches to sync with a timeout func waitForCacheSync(t *testing.T, timeout time.Duration, cacheSyncWaiters ...cache.InformerSynced) bool { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -968,6 +1083,7 @@ func waitForCacheSync(t *testing.T, timeout time.Duration, cacheSyncWaiters ...c return true } +// waitForObjectInCache waits for a specific object to appear in the cache func waitForObjectInCache(t *testing.T, timeout time.Duration, checkFunc func() bool) bool { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() From 92baddfcd4a8e370ae7425e4214237dbae6aeece Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Fri, 3 Jul 2026 16:12:03 +0800 Subject: [PATCH 05/11] chore: make generate for `ModelRouteStatus` and `ModelServerStatus` Signed-off-by: Leo Xie --- ...orking.serving.volcano.sh_modelroutes.yaml | 67 ++++++++++++++++ ...rking.serving.volcano.sh_modelservers.yaml | 80 +++++++++++++++++++ .../networking/v1alpha1/modelroute.go | 9 +-- .../networking/v1alpha1/modelroutestatus.go | 57 +++++++++++++ .../networking/v1alpha1/modelserver.go | 9 +-- .../networking/v1alpha1/modelserverstatus.go | 75 +++++++++++++++++ client-go/applyconfiguration/utils.go | 4 + .../crd/networking.serving.volcano.sh.md | 8 ++ .../v1alpha1/zz_generated.deepcopy.go | 18 ++++- 9 files changed, 315 insertions(+), 12 deletions(-) create mode 100644 client-go/applyconfiguration/networking/v1alpha1/modelroutestatus.go create mode 100644 client-go/applyconfiguration/networking/v1alpha1/modelserverstatus.go diff --git a/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelroutes.yaml b/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelroutes.yaml index ed84f4132..d644a7454 100644 --- a/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelroutes.yaml +++ b/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelroutes.yaml @@ -355,6 +355,73 @@ spec: rule: self.modelName != "" || size(self.loraAdapters) > 0 status: description: ModelRouteStatus defines the observed state of ModelRoute. + properties: + conditions: + description: |- + Conditions track the lifecycle of this ModelRoute. + Types: + - "Ready": true when the route has been registered with the router store + and is available for request matching. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + observedGeneration: + description: observedGeneration is the most recent generation observed + for this ModelRoute. + format: int64 + type: integer type: object required: - spec diff --git a/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml b/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml index f9f32bd31..e02931a81 100644 --- a/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml +++ b/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml @@ -159,6 +159,86 @@ spec: type: object status: description: ModelServerStatus defines the observed state of ModelServer. + properties: + conditions: + description: |- + Conditions track the lifecycle of this ModelServer. + Types: + - "Ready": true when at least one ready pod is matched and the ModelServer has been + registered with the router store. Signals that `kubectl wait --for=condition=ready` + can be used. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + matchedReplicas: + description: |- + matchedReplicas is the total number of pods (ready or not) that match the + workloadSelector. + format: int32 + type: integer + observedGeneration: + description: observedGeneration is the most recent generation observed + for this ModelServer. + format: int64 + type: integer + readyReplicas: + description: |- + readyReplicas is the number of ready pods that are currently matched by the + workloadSelector and have been successfully registered in the router's store. + format: int32 + type: integer type: object type: object served: true diff --git a/client-go/applyconfiguration/networking/v1alpha1/modelroute.go b/client-go/applyconfiguration/networking/v1alpha1/modelroute.go index b082e08c4..42357e0dd 100644 --- a/client-go/applyconfiguration/networking/v1alpha1/modelroute.go +++ b/client-go/applyconfiguration/networking/v1alpha1/modelroute.go @@ -19,7 +19,6 @@ limitations under the License. package v1alpha1 import ( - networkingv1alpha1 "github.com/volcano-sh/kthena/pkg/apis/networking/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" types "k8s.io/apimachinery/pkg/types" v1 "k8s.io/client-go/applyconfigurations/meta/v1" @@ -30,8 +29,8 @@ import ( type ModelRouteApplyConfiguration struct { v1.TypeMetaApplyConfiguration `json:",inline"` *v1.ObjectMetaApplyConfiguration `json:"metadata,omitempty"` - Spec *ModelRouteSpecApplyConfiguration `json:"spec,omitempty"` - Status *networkingv1alpha1.ModelRouteStatus `json:"status,omitempty"` + Spec *ModelRouteSpecApplyConfiguration `json:"spec,omitempty"` + Status *ModelRouteStatusApplyConfiguration `json:"status,omitempty"` } // ModelRoute constructs a declarative configuration of the ModelRoute type for use with @@ -215,8 +214,8 @@ func (b *ModelRouteApplyConfiguration) WithSpec(value *ModelRouteSpecApplyConfig // WithStatus sets the Status field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Status field is set to the value of the last call. -func (b *ModelRouteApplyConfiguration) WithStatus(value networkingv1alpha1.ModelRouteStatus) *ModelRouteApplyConfiguration { - b.Status = &value +func (b *ModelRouteApplyConfiguration) WithStatus(value *ModelRouteStatusApplyConfiguration) *ModelRouteApplyConfiguration { + b.Status = value return b } diff --git a/client-go/applyconfiguration/networking/v1alpha1/modelroutestatus.go b/client-go/applyconfiguration/networking/v1alpha1/modelroutestatus.go new file mode 100644 index 000000000..3615fa569 --- /dev/null +++ b/client-go/applyconfiguration/networking/v1alpha1/modelroutestatus.go @@ -0,0 +1,57 @@ +/* +Copyright The Volcano Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + v1 "k8s.io/client-go/applyconfigurations/meta/v1" +) + +// ModelRouteStatusApplyConfiguration represents a declarative configuration of the ModelRouteStatus type for use +// with apply. +type ModelRouteStatusApplyConfiguration struct { + ObservedGeneration *int64 `json:"observedGeneration,omitempty"` + Conditions []v1.ConditionApplyConfiguration `json:"conditions,omitempty"` +} + +// ModelRouteStatusApplyConfiguration constructs a declarative configuration of the ModelRouteStatus type for use with +// apply. +func ModelRouteStatus() *ModelRouteStatusApplyConfiguration { + return &ModelRouteStatusApplyConfiguration{} +} + +// WithObservedGeneration sets the ObservedGeneration field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ObservedGeneration field is set to the value of the last call. +func (b *ModelRouteStatusApplyConfiguration) WithObservedGeneration(value int64) *ModelRouteStatusApplyConfiguration { + b.ObservedGeneration = &value + return b +} + +// WithConditions adds the given value to the Conditions field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the Conditions field. +func (b *ModelRouteStatusApplyConfiguration) WithConditions(values ...*v1.ConditionApplyConfiguration) *ModelRouteStatusApplyConfiguration { + for i := range values { + if values[i] == nil { + panic("nil value passed to WithConditions") + } + b.Conditions = append(b.Conditions, *values[i]) + } + return b +} diff --git a/client-go/applyconfiguration/networking/v1alpha1/modelserver.go b/client-go/applyconfiguration/networking/v1alpha1/modelserver.go index b52d80d60..e743bbba4 100644 --- a/client-go/applyconfiguration/networking/v1alpha1/modelserver.go +++ b/client-go/applyconfiguration/networking/v1alpha1/modelserver.go @@ -19,7 +19,6 @@ limitations under the License. package v1alpha1 import ( - networkingv1alpha1 "github.com/volcano-sh/kthena/pkg/apis/networking/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" types "k8s.io/apimachinery/pkg/types" v1 "k8s.io/client-go/applyconfigurations/meta/v1" @@ -30,8 +29,8 @@ import ( type ModelServerApplyConfiguration struct { v1.TypeMetaApplyConfiguration `json:",inline"` *v1.ObjectMetaApplyConfiguration `json:"metadata,omitempty"` - Spec *ModelServerSpecApplyConfiguration `json:"spec,omitempty"` - Status *networkingv1alpha1.ModelServerStatus `json:"status,omitempty"` + Spec *ModelServerSpecApplyConfiguration `json:"spec,omitempty"` + Status *ModelServerStatusApplyConfiguration `json:"status,omitempty"` } // ModelServer constructs a declarative configuration of the ModelServer type for use with @@ -215,8 +214,8 @@ func (b *ModelServerApplyConfiguration) WithSpec(value *ModelServerSpecApplyConf // WithStatus sets the Status field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Status field is set to the value of the last call. -func (b *ModelServerApplyConfiguration) WithStatus(value networkingv1alpha1.ModelServerStatus) *ModelServerApplyConfiguration { - b.Status = &value +func (b *ModelServerApplyConfiguration) WithStatus(value *ModelServerStatusApplyConfiguration) *ModelServerApplyConfiguration { + b.Status = value return b } diff --git a/client-go/applyconfiguration/networking/v1alpha1/modelserverstatus.go b/client-go/applyconfiguration/networking/v1alpha1/modelserverstatus.go new file mode 100644 index 000000000..b4ee96c0e --- /dev/null +++ b/client-go/applyconfiguration/networking/v1alpha1/modelserverstatus.go @@ -0,0 +1,75 @@ +/* +Copyright The Volcano Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1alpha1 + +import ( + v1 "k8s.io/client-go/applyconfigurations/meta/v1" +) + +// ModelServerStatusApplyConfiguration represents a declarative configuration of the ModelServerStatus type for use +// with apply. +type ModelServerStatusApplyConfiguration struct { + ObservedGeneration *int64 `json:"observedGeneration,omitempty"` + ReadyReplicas *int32 `json:"readyReplicas,omitempty"` + MatchedReplicas *int32 `json:"matchedReplicas,omitempty"` + Conditions []v1.ConditionApplyConfiguration `json:"conditions,omitempty"` +} + +// ModelServerStatusApplyConfiguration constructs a declarative configuration of the ModelServerStatus type for use with +// apply. +func ModelServerStatus() *ModelServerStatusApplyConfiguration { + return &ModelServerStatusApplyConfiguration{} +} + +// WithObservedGeneration sets the ObservedGeneration field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ObservedGeneration field is set to the value of the last call. +func (b *ModelServerStatusApplyConfiguration) WithObservedGeneration(value int64) *ModelServerStatusApplyConfiguration { + b.ObservedGeneration = &value + return b +} + +// WithReadyReplicas sets the ReadyReplicas field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ReadyReplicas field is set to the value of the last call. +func (b *ModelServerStatusApplyConfiguration) WithReadyReplicas(value int32) *ModelServerStatusApplyConfiguration { + b.ReadyReplicas = &value + return b +} + +// WithMatchedReplicas sets the MatchedReplicas field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the MatchedReplicas field is set to the value of the last call. +func (b *ModelServerStatusApplyConfiguration) WithMatchedReplicas(value int32) *ModelServerStatusApplyConfiguration { + b.MatchedReplicas = &value + return b +} + +// WithConditions adds the given value to the Conditions field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the Conditions field. +func (b *ModelServerStatusApplyConfiguration) WithConditions(values ...*v1.ConditionApplyConfiguration) *ModelServerStatusApplyConfiguration { + for i := range values { + if values[i] == nil { + panic("nil value passed to WithConditions") + } + b.Conditions = append(b.Conditions, *values[i]) + } + return b +} diff --git a/client-go/applyconfiguration/utils.go b/client-go/applyconfiguration/utils.go index 9247efca7..7b266f57c 100644 --- a/client-go/applyconfiguration/utils.go +++ b/client-go/applyconfiguration/utils.go @@ -46,10 +46,14 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &networkingv1alpha1.ModelRouteApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("ModelRouteSpec"): return &networkingv1alpha1.ModelRouteSpecApplyConfiguration{} + case v1alpha1.SchemeGroupVersion.WithKind("ModelRouteStatus"): + return &networkingv1alpha1.ModelRouteStatusApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("ModelServer"): return &networkingv1alpha1.ModelServerApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("ModelServerSpec"): return &networkingv1alpha1.ModelServerSpecApplyConfiguration{} + case v1alpha1.SchemeGroupVersion.WithKind("ModelServerStatus"): + return &networkingv1alpha1.ModelServerStatusApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("PDGroup"): return &networkingv1alpha1.PDGroupApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("RateLimit"): diff --git a/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md b/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md index fa7bc47a6..38b6d3ed2 100644 --- a/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md +++ b/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md @@ -188,6 +188,9 @@ ModelRouteStatus defines the observed state of ModelRoute. _Appears in:_ - [ModelRoute](#modelroute) +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `observedGeneration` _integer_ | observedGeneration is the most recent generation observed for this ModelRoute. | | | #### ModelServer @@ -258,6 +261,11 @@ ModelServerStatus defines the observed state of ModelServer. _Appears in:_ - [ModelServer](#modelserver) +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `observedGeneration` _integer_ | observedGeneration is the most recent generation observed for this ModelServer. | | | +| `readyReplicas` _integer_ | readyReplicas is the number of ready pods that are currently matched by the
workloadSelector and have been successfully registered in the router's store. | | | +| `matchedReplicas` _integer_ | matchedReplicas is the total number of pods (ready or not) that match the
workloadSelector. | | | #### PDGroup diff --git a/pkg/apis/networking/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/networking/v1alpha1/zz_generated.deepcopy.go index 26f1f0a88..ab7b4f698 100644 --- a/pkg/apis/networking/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/networking/v1alpha1/zz_generated.deepcopy.go @@ -128,7 +128,7 @@ func (in *ModelRoute) DeepCopyInto(out *ModelRoute) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelRoute. @@ -227,6 +227,13 @@ func (in *ModelRouteSpec) DeepCopy() *ModelRouteSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ModelRouteStatus) DeepCopyInto(out *ModelRouteStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelRouteStatus. @@ -245,7 +252,7 @@ func (in *ModelServer) DeepCopyInto(out *ModelServer) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelServer. @@ -337,6 +344,13 @@ func (in *ModelServerSpec) DeepCopy() *ModelServerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ModelServerStatus) DeepCopyInto(out *ModelServerStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModelServerStatus. From 0c3633590927866639e886b827281697645437df Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Fri, 3 Jul 2026 21:29:15 +0800 Subject: [PATCH 06/11] feat: Sync modelserver state through `syncPodHandler` Signed-off-by: Leo Xie --- .../controller/modelserver_controller.go | 71 ++++++++++++++++--- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index 90fa5c497..2781cb16f 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -46,6 +46,7 @@ import ( "github.com/volcano-sh/kthena/pkg/kthena-router/utils" ) +// ResourceType represents the type of Kubernetes resource type ResourceType string const ( @@ -53,6 +54,7 @@ const ( ResourceTypePod ResourceType = "Pod" ) +// QueueItem represents an item in the work queue type QueueItem struct { ResourceType ResourceType Key string @@ -66,6 +68,7 @@ type ModelServerController struct { modelServerSynced cache.InformerSynced podSynced cache.InformerSynced + // Event handler registrations modelServerRegistration cache.ResourceEventHandlerRegistration podRegistration cache.ResourceEventHandlerRegistration @@ -94,6 +97,7 @@ func NewModelServerController( store: store, } + // Register ModelServer event handlers controller.modelServerRegistration, _ = modelServerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: controller.enqueueModelServer, UpdateFunc: func(old, new interface{}) { @@ -102,6 +106,7 @@ func NewModelServerController( DeleteFunc: controller.enqueueModelServer, }) + // Register Pod event handlers controller.podRegistration, _ = podInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: controller.enqueuePod, UpdateFunc: func(old, new interface{}) { @@ -120,6 +125,7 @@ func (c *ModelServerController) Run(stopCh <-chan struct{}) error { if ok := cache.WaitForCacheSync(stopCh, c.modelServerSynced, c.podSynced); !ok { return fmt.Errorf("failed to wait for caches to sync") } + // add initialSync signal c.workqueue.Add(QueueItem{}) go wait.Until(c.runWorker, time.Second, stopCh) @@ -144,6 +150,7 @@ func (c *ModelServerController) processNextWorkItem() bool { } defer c.workqueue.Done(obj) + // Handle initial sync signal if obj.ResourceType == "" && obj.Key == "" { klog.V(2).Info("initial modelServer and pod resources have been synced") c.workqueue.Forget(obj) @@ -323,6 +330,9 @@ func (c *ModelServerController) syncPodHandler(key string) error { pod, err := c.podLister.Pods(namespace).Get(name) if errors.IsNotFound(err) { _ = c.store.DeletePod(types.NamespacedName{Namespace: namespace, Name: name}) + // When a pod is deleted, enqueue all ModelServers in the namespace + // to refresh their status (we no longer have the pod to check labels). + c.enqueueModelServersInNamespace(namespace) return nil } if err != nil { @@ -331,36 +341,79 @@ func (c *ModelServerController) syncPodHandler(key string) error { if !isPodReady(pod) { _ = c.store.DeletePod(types.NamespacedName{Namespace: namespace, Name: name}) + // Pod became not ready: enqueue matching ModelServers to refresh status + c.enqueueMatchingModelServers(pod) return nil } - return c.addOrUpdatePod(pod) + if err := c.addOrUpdatePod(pod); err != nil { + return err + } + // Pod became ready: enqueue matching ModelServers to refresh status + c.enqueueMatchingModelServers(pod) + return nil } // addOrUpdatePod finds all ModelServers that match the given pod // and adds or updates the pod-server binding in the data store func (c *ModelServerController) addOrUpdatePod(pod *corev1.Pod) error { + servers, err := c.getMatchingModelServers(pod) + if err != nil { + return err + } + + if len(servers) > 0 { + if err := c.store.AddOrUpdatePod(pod, servers); err != nil { + return fmt.Errorf("failed to add or update pod %s/%s in data store: %v", pod.Namespace, pod.Name, err) + } + } + + return nil +} + +// getMatchingModelServers returns all ModelServers in the same namespace whose +// workloadSelector matches the pod's labels. +func (c *ModelServerController) getMatchingModelServers(pod *corev1.Pod) ([]*aiv1alpha1.ModelServer, error) { modelServers, err := c.modelServerLister.ModelServers(pod.Namespace).List(labels.Everything()) if err != nil { - return fmt.Errorf("failed to list ModelServers for pod %s/%s: %v", pod.Namespace, pod.Name, err) + return nil, fmt.Errorf("failed to list ModelServers for pod %s/%s: %v", pod.Namespace, pod.Name, err) } - servers := []*aiv1alpha1.ModelServer{} + var matching []*aiv1alpha1.ModelServer for _, item := range modelServers { selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchLabels: item.Spec.WorkloadSelector.MatchLabels}) if err != nil || !selector.Matches(labels.Set(pod.Labels)) { continue } - servers = append(servers, item) + matching = append(matching, item) } + return matching, nil +} - if len(servers) > 0 { - if err := c.store.AddOrUpdatePod(pod, servers); err != nil { - return fmt.Errorf("failed to add or update pod %s/%s in data store: %v", pod.Namespace, pod.Name, err) - } +// enqueueMatchingModelServers enqueues all ModelServers whose workloadSelector +// matches the pod, triggering a status re-evaluation. +func (c *ModelServerController) enqueueMatchingModelServers(pod *corev1.Pod) { + matching, err := c.getMatchingModelServers(pod) + if err != nil { + klog.V(4).Infof("failed to find matching ModelServers for pod %s/%s: %v", pod.Namespace, pod.Name, err) + return } + for _, ms := range matching { + c.enqueueModelServer(ms) + } +} - return nil +// enqueueModelServersInNamespace enqueues all ModelServers in the given namespace. +// Used when a pod is deleted and we no longer have pod labels to narrow the search. +func (c *ModelServerController) enqueueModelServersInNamespace(namespace string) { + msList, err := c.modelServerLister.ModelServers(namespace).List(labels.Everything()) + if err != nil { + klog.V(4).Infof("failed to list ModelServers in namespace %s: %v", namespace, err) + return + } + for _, ms := range msList { + c.enqueueModelServer(ms) + } } func (c *ModelServerController) enqueueModelServer(obj interface{}) { From e10ae6c513b3612feea3405597bf8b381e4bf640 Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Fri, 3 Jul 2026 21:41:03 +0800 Subject: [PATCH 07/11] test: Unittest for modelroute and modelserver status updating Signed-off-by: Leo Xie --- .../controller/modelroute_controller_test.go | 99 +++++++++ .../controller/modelserver_controller_test.go | 199 ++++++++++++++++++ 2 files changed, 298 insertions(+) diff --git a/pkg/kthena-router/controller/modelroute_controller_test.go b/pkg/kthena-router/controller/modelroute_controller_test.go index 4cbd94e60..f3c798ed7 100644 --- a/pkg/kthena-router/controller/modelroute_controller_test.go +++ b/pkg/kthena-router/controller/modelroute_controller_test.go @@ -22,6 +22,8 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kthenafake "github.com/volcano-sh/kthena/client-go/clientset/versioned/fake" @@ -193,3 +195,100 @@ func TestModelRouteController_WorkQueueProcessing(t *testing.T) { assert.True(t, result) }) } + +// TestModelRouteController_StatusUpdate verifies that syncHandler sets +// the Ready condition on the ModelRoute status. +func TestModelRouteController_StatusUpdate(t *testing.T) { + kthenaClient := kthenafake.NewSimpleClientset() + kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + store := datastore.New() + + controller := NewModelRouteController(kthenaClient, kthenaInformerFactory, store) + + stop := make(chan struct{}) + defer close(stop) + kthenaInformerFactory.Start(stop) + + // sync of a newly created ModelRoute writes the Ready condition + t.Run("SetsReadyConditionAfterSync", func(t *testing.T) { + mr := &aiv1alpha1.ModelRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test-status-route", + }, + Spec: aiv1alpha1.ModelRouteSpec{ + ModelName: "status-test-model", + Rules: []*aiv1alpha1.Rule{ + { + Name: "rule-1", + TargetModels: []*aiv1alpha1.TargetModel{ + {ModelServerName: "test-server"}, + }, + }, + }, + }, + } + + _, err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Create( + context.Background(), mr, metav1.CreateOptions{}) + assert.NoError(t, err) + + waitForCacheSync(t, 5*time.Second, controller.modelRouteSynced) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelRouteLister.ModelRoutes("default").Get("test-status-route") + return err == nil + }) + + err = controller.syncHandler("default/test-status-route") + assert.NoError(t, err) + + // verify status was written to the fake API server + updated, err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Get( + context.Background(), "test-status-route", metav1.GetOptions{}) + assert.NoError(t, err) + + cond := meta.FindStatusCondition(updated.Status.Conditions, "Ready") + require.NotNil(t, cond) + assert.Equal(t, metav1.ConditionTrue, cond.Status) + assert.Equal(t, "RouteRegistered", cond.Reason) + }) + + // sync skips status update when already up-to-date + t.Run("SkipsUpdateWhenAlreadyReady", func(t *testing.T) { + mr := &aiv1alpha1.ModelRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test-status-skip", + }, + Spec: aiv1alpha1.ModelRouteSpec{ + ModelName: "skip-test-model", + Rules: []*aiv1alpha1.Rule{ + { + Name: "rule-1", + TargetModels: []*aiv1alpha1.TargetModel{ + {ModelServerName: "test-server"}, + }, + }, + }, + }, + } + + _, err := kthenaClient.NetworkingV1alpha1().ModelRoutes("default").Create( + context.Background(), mr, metav1.CreateOptions{}) + assert.NoError(t, err) + + waitForCacheSync(t, 5*time.Second, controller.modelRouteSynced) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelRouteLister.ModelRoutes("default").Get("test-status-skip") + return err == nil + }) + + // first sync: sets status + err = controller.syncHandler("default/test-status-skip") + assert.NoError(t, err) + + // second sync: should skip (already ready) and not error + err = controller.syncHandler("default/test-status-skip") + assert.NoError(t, err) + }) +} diff --git a/pkg/kthena-router/controller/modelserver_controller_test.go b/pkg/kthena-router/controller/modelserver_controller_test.go index 174a3e9ee..fd98f65a8 100644 --- a/pkg/kthena-router/controller/modelserver_controller_test.go +++ b/pkg/kthena-router/controller/modelserver_controller_test.go @@ -1103,3 +1103,202 @@ func waitForObjectInCache(t *testing.T, timeout time.Duration, checkFunc func() } } } + +// drainModelServerKeys drains the workqueue and returns keys of enqueued ModelServer items. +func drainModelServerKeys(controller *ModelServerController) []string { + var keys []string + for controller.workqueue.Len() > 0 { + item, _ := controller.workqueue.Get() + controller.workqueue.Done(item) + controller.workqueue.Forget(item) + if item.ResourceType == ResourceTypeModelServer { + keys = append(keys, item.Key) + } + } + return keys +} + +// TestModelServerController_PodEnqueuesModelServer verifies that syncPodHandler +// enqueues matching ModelServers after pod state changes. +func TestModelServerController_PodEnqueuesModelServer(t *testing.T) { + kubeClient := kubefake.NewSimpleClientset() + kthenaClient := kthenafake.NewSimpleClientset() + + ms := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "test-ms-enq"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: map[string]string{"app": "enq-test"}, + }, + }, + } + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) + assert.NoError(t, err) + + kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) + kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + store := newStoreWithMockBackend() + controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, + ) + + stop := make(chan struct{}) + defer close(stop) + kthenaInformerFactory.Start(stop) + kubeInformerFactory.Start(stop) + waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelServerLister.ModelServers("default").Get("test-ms-enq") + return err == nil + }) + + // ready pod sync enqueues its matching ModelServer + t.Run("ReadyPodEnqueuesMatchingModelServer", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "ready-pod-enq", + Labels: map[string]string{"app": "enq-test"}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionTrue}, + }, + }, + } + _, err := kubeClient.CoreV1().Pods("default").Create( + context.Background(), pod, metav1.CreateOptions{}) + assert.NoError(t, err) + + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("ready-pod-enq") + return e == nil && p.Name == "ready-pod-enq" + }) + // drain any pending items before sync + drainModelServerKeys(controller) + + err = controller.syncPodHandler("default/ready-pod-enq") + assert.NoError(t, err) + + keys := drainModelServerKeys(controller) + assert.Contains(t, keys, "default/test-ms-enq") + }) + + // not-ready pod sync deletes from store and enqueues matching ModelServer + t.Run("NotReadyPodEnqueuesMatchingModelServer", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "notready-pod-enq", + Labels: map[string]string{"app": "enq-test"}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodPending, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: corev1.ConditionFalse}, + }, + }, + } + _, err := kubeClient.CoreV1().Pods("default").Create( + context.Background(), pod, metav1.CreateOptions{}) + assert.NoError(t, err) + + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("notready-pod-enq") + return e == nil && p.Name == "notready-pod-enq" + }) + drainModelServerKeys(controller) + + err = controller.syncPodHandler("default/notready-pod-enq") + assert.NoError(t, err) + + keys := drainModelServerKeys(controller) + assert.Contains(t, keys, "default/test-ms-enq") + }) + + // deleted pod (not found in lister) enqueues all namespace ModelServers + t.Run("DeletedPodEnqueuesAllNamespaceModelServers", func(t *testing.T) { + drainModelServerKeys(controller) + + err := controller.syncPodHandler("default/non-existent-pod-enq") + assert.NoError(t, err) + + keys := drainModelServerKeys(controller) + assert.Contains(t, keys, "default/test-ms-enq") + }) +} + +// TestModelServerController_GetMatchingModelServers verifies the helper +// returns correct ModelServers based on pod label matching. +func TestModelServerController_GetMatchingModelServers(t *testing.T) { + kthenaClient := kthenafake.NewSimpleClientset() + + ms1 := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "ms-match"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: map[string]string{"app": "myapp"}, + }, + }, + } + ms2 := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "ms-nomatch"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: map[string]string{"app": "other"}, + }, + }, + } + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms1, metav1.CreateOptions{}) + assert.NoError(t, err) + _, err = kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms2, metav1.CreateOptions{}) + assert.NoError(t, err) + + kubeInformerFactory := informers.NewSharedInformerFactory(kubefake.NewSimpleClientset(), 0) + kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + controller := NewModelServerController( + kthenaClient, kthenaInformerFactory, kubeInformerFactory, newStoreWithMockBackend(), + ) + + stop := make(chan struct{}) + defer close(stop) + kthenaInformerFactory.Start(stop) + kubeInformerFactory.Start(stop) + waitForCacheSync(t, 5*time.Second, controller.modelServerSynced) + + // pod labels match one ModelServer's workloadSelector + t.Run("MatchingLabelsReturnsModelServer", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "matching-pod", + Labels: map[string]string{"app": "myapp"}, + }, + } + result, err := controller.getMatchingModelServers(pod) + assert.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, "ms-match", result[0].Name) + }) + + // pod labels match no ModelServer + t.Run("NonMatchingLabelsReturnsEmpty", func(t *testing.T) { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "other-pod", + Labels: map[string]string{"app": "nonexistent"}, + }, + } + result, err := controller.getMatchingModelServers(pod) + assert.NoError(t, err) + assert.Empty(t, result) + }) +} From e739430f455c44c85a8e1a3f012da5047fa00eb2 Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Fri, 3 Jul 2026 22:49:33 +0800 Subject: [PATCH 08/11] test: Test lifecycle through fake API Signed-off-by: Leo Xie --- .../controller/modelserver_controller_test.go | 68 +++++++++++-------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/pkg/kthena-router/controller/modelserver_controller_test.go b/pkg/kthena-router/controller/modelserver_controller_test.go index fd98f65a8..d331617fe 100644 --- a/pkg/kthena-router/controller/modelserver_controller_test.go +++ b/pkg/kthena-router/controller/modelserver_controller_test.go @@ -76,7 +76,6 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { kubeInformerFactory, store, ) - modelServerIndexer := kthenaInformerFactory.Networking().V1alpha1().ModelServers().Informer().GetIndexer() stop := make(chan struct{}) defer close(stop) @@ -106,13 +105,15 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { t.Fatal("Failed to sync caches within timeout") } - require.NoError(t, modelServerIndexer.Add(ms.DeepCopy())) - _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver") + // write through fake API so status update can Get + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) require.NoError(t, err) - // Simulate controller receiving the event - controller.enqueueModelServer(ms) - assert.Equal(t, 1, controller.workqueue.Len()) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver") + return err == nil + }) // Process the queue item err = controller.syncModelServerHandler("default/test-modelserver") @@ -147,35 +148,31 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { }, } - require.NoError(t, modelServerIndexer.Add(ms.DeepCopy())) - _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-update") + // update via fake API, verify store reflects changes + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) require.NoError(t, err) - // Process initial creation - controller.enqueueModelServer(ms) - err = controller.syncModelServerHandler("default/test-modelserver-update") - assert.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-update") + return err == nil + }) + + assert.NoError(t, controller.syncModelServerHandler("default/test-modelserver-update")) // Update ModelServer updatedMS := ms.DeepCopy() updatedMS.Labels["version"] = "v2" updatedMS.Spec.WorkloadSelector.MatchLabels["environment"] = "production" - require.NoError(t, modelServerIndexer.Update(updatedMS.DeepCopy())) - cachedMS, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-update") + _, err = kthenaClient.NetworkingV1alpha1().ModelServers("default").Update( + context.Background(), updatedMS, metav1.UpdateOptions{}) require.NoError(t, err) - assert.Equal(t, "v2", cachedMS.Labels["version"]) - // Simulate controller receiving update event - controller.enqueueModelServer(updatedMS) - // Clear any previous items from queue - for controller.workqueue.Len() > 0 { - item, _ := controller.workqueue.Get() - controller.workqueue.Done(item) - controller.workqueue.Forget(item) - } - controller.enqueueModelServer(updatedMS) - assert.Equal(t, 1, controller.workqueue.Len()) + waitForObjectInCache(t, 2*time.Second, func() bool { + cached, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-update") + return err == nil && cached.Labels["version"] == "v2" + }) // Process the update err = controller.syncModelServerHandler("default/test-modelserver-update") @@ -208,10 +205,16 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { }, } - require.NoError(t, modelServerIndexer.Add(ms.DeepCopy())) - _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") + // delete via fake API, verify removed from store + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") + return err == nil + }) + // Process creation err = controller.syncModelServerHandler("default/test-modelserver-delete") assert.NoError(t, err) @@ -223,9 +226,14 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { }) require.NotNil(t, storedMS, "ModelServer should be found in store before deletion") - require.NoError(t, modelServerIndexer.Delete(ms.DeepCopy())) - _, err = controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") - assert.Error(t, err) + err = kthenaClient.NetworkingV1alpha1().ModelServers("default").Delete( + context.Background(), "test-modelserver-delete", metav1.DeleteOptions{}) + assert.NoError(t, err) + + waitForObjectInCache(t, 2*time.Second, func() bool { + _, err := controller.modelServerLister.ModelServers("default").Get("test-modelserver-delete") + return err != nil + }) // Process the deletion - this should handle the NotFound error gracefully err = controller.syncModelServerHandler("default/test-modelserver-delete") From 2c2eb722ec6c6f3f2c903413cf615bdfe02b4494 Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Thu, 9 Jul 2026 14:55:28 +0800 Subject: [PATCH 09/11] api: introduce Ready condition for server Signed-off-by: Leo Xie --- .../docs/reference/crd/networking.serving.volcano.sh.md | 4 ++++ pkg/apis/networking/v1alpha1/modelroute_types.go | 8 ++++++++ pkg/apis/networking/v1alpha1/modelserver_types.go | 8 ++++++++ pkg/kthena-router/controller/modelroute_controller.go | 4 ++-- .../controller/modelroute_controller_test.go | 2 +- pkg/kthena-router/controller/modelserver_controller.go | 6 +++--- 6 files changed, 26 insertions(+), 6 deletions(-) diff --git a/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md b/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md index 38b6d3ed2..0f6f45e9d 100644 --- a/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md +++ b/docs/kthena/docs/reference/crd/networking.serving.volcano.sh.md @@ -140,6 +140,8 @@ _Appears in:_ | `status` _[ModelRouteStatus](#modelroutestatus)_ | | | | + + #### ModelRouteList @@ -212,6 +214,8 @@ _Appears in:_ | `status` _[ModelServerStatus](#modelserverstatus)_ | | | | + + #### ModelServerList diff --git a/pkg/apis/networking/v1alpha1/modelroute_types.go b/pkg/apis/networking/v1alpha1/modelroute_types.go index e1ab2a637..21632ff66 100644 --- a/pkg/apis/networking/v1alpha1/modelroute_types.go +++ b/pkg/apis/networking/v1alpha1/modelroute_types.go @@ -165,6 +165,14 @@ const ( Month RateLimitUnit = "month" ) +// ModelRouteConditionType is the type of a status condition on a ModelRoute. +type ModelRouteConditionType string + +const ( + // ModelRouteConditionReady indicates the route has been registered in the router store. + ModelRouteConditionReady ModelRouteConditionType = "Ready" +) + // ModelRouteStatus defines the observed state of ModelRoute. type ModelRouteStatus struct { // observedGeneration is the most recent generation observed for this ModelRoute. diff --git a/pkg/apis/networking/v1alpha1/modelserver_types.go b/pkg/apis/networking/v1alpha1/modelserver_types.go index fe4dae5bd..817de845f 100644 --- a/pkg/apis/networking/v1alpha1/modelserver_types.go +++ b/pkg/apis/networking/v1alpha1/modelserver_types.go @@ -110,6 +110,14 @@ const ( ConnectorTypeMoonCake KVConnectorType = "mooncake" // Indicates `MoonCakeConnector` in vllm-ascend ) +// ModelServerConditionType is the type of a status condition on a ModelServer. +type ModelServerConditionType string + +const ( + // ModelServerConditionReady indicates at least one ready pod is registered in the router store. + ModelServerConditionReady ModelServerConditionType = "Ready" +) + // KVConnectorSpec defines KV connector configuration for PD disaggregated routing type KVConnectorSpec struct { // Type specifies the connector type. diff --git a/pkg/kthena-router/controller/modelroute_controller.go b/pkg/kthena-router/controller/modelroute_controller.go index 2d5bd7d2b..d94435bf5 100644 --- a/pkg/kthena-router/controller/modelroute_controller.go +++ b/pkg/kthena-router/controller/modelroute_controller.go @@ -162,7 +162,7 @@ func (c *ModelRouteController) syncHandler(key string) error { func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) error { // Check if status is already up-to-date if mr.Status.ObservedGeneration == mr.Generation { - if cond := meta.FindStatusCondition(mr.Status.Conditions, "Ready"); cond != nil && + if cond := meta.FindStatusCondition(mr.Status.Conditions, string(aiv1alpha1.ModelRouteConditionReady)); cond != nil && cond.Status == metav1.ConditionTrue && cond.Reason == "RouteRegistered" { return nil @@ -181,7 +181,7 @@ func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) mrCopy.Status.ObservedGeneration = latest.Generation meta.SetStatusCondition(&mrCopy.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: string(aiv1alpha1.ModelRouteConditionReady), Status: metav1.ConditionTrue, Reason: "RouteRegistered", Message: "ModelRoute registered in router store", diff --git a/pkg/kthena-router/controller/modelroute_controller_test.go b/pkg/kthena-router/controller/modelroute_controller_test.go index f3c798ed7..c17947669 100644 --- a/pkg/kthena-router/controller/modelroute_controller_test.go +++ b/pkg/kthena-router/controller/modelroute_controller_test.go @@ -247,7 +247,7 @@ func TestModelRouteController_StatusUpdate(t *testing.T) { context.Background(), "test-status-route", metav1.GetOptions{}) assert.NoError(t, err) - cond := meta.FindStatusCondition(updated.Status.Conditions, "Ready") + cond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelRouteConditionReady)) require.NotNil(t, cond) assert.Equal(t, metav1.ConditionTrue, cond.Status) assert.Equal(t, "RouteRegistered", cond.Reason) diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index 2781cb16f..02c69bade 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -279,7 +279,7 @@ func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServ if ms.Status.ObservedGeneration == ms.Generation && ms.Status.MatchedReplicas == int32(len(podList)) && ms.Status.ReadyReplicas == int32(readyPods.Len()) { - if cond := meta.FindStatusCondition(ms.Status.Conditions, "Ready"); cond != nil && + if cond := meta.FindStatusCondition(ms.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)); cond != nil && cond.Status == expectedReadyStatus && cond.Reason == expectedReason { return nil @@ -301,14 +301,14 @@ func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServ if readyPods.Len() > 0 { meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: string(aiv1alpha1.ModelServerConditionReady), Status: metav1.ConditionTrue, Reason: "PodsReady", Message: fmt.Sprintf("%d ready pods matched", readyPods.Len()), }) } else { meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ - Type: "Ready", + Type: string(aiv1alpha1.ModelServerConditionReady), Status: metav1.ConditionFalse, Reason: "NoReadyPods", Message: "No ready pods match workloadSelector", From 96db8f49dbc9ba1b25d780e51e1f179d130ec263 Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Thu, 9 Jul 2026 15:50:48 +0800 Subject: [PATCH 10/11] api: introduce AllPodsReady condition Signed-off-by: Leo Xie --- ...rking.serving.volcano.sh_modelservers.yaml | 5 +- .../networking/v1alpha1/modelserver_types.go | 8 +- .../controller/modelserver_controller.go | 84 +++-- .../controller/modelserver_controller_test.go | 323 ++++++++++++++++++ 4 files changed, 388 insertions(+), 32 deletions(-) diff --git a/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml b/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml index e02931a81..de538f23b 100644 --- a/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml +++ b/charts/kthena/charts/networking/crds/networking.serving.volcano.sh_modelservers.yaml @@ -165,8 +165,9 @@ spec: Conditions track the lifecycle of this ModelServer. Types: - "Ready": true when at least one ready pod is matched and the ModelServer has been - registered with the router store. Signals that `kubectl wait --for=condition=ready` - can be used. + registered with the router store. Signals that the ModelServer can serve traffic. + - "AllPodsReady": true when at least one pod is matched and every matched pod is ready + and registered with the router store. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/pkg/apis/networking/v1alpha1/modelserver_types.go b/pkg/apis/networking/v1alpha1/modelserver_types.go index 817de845f..73e326f05 100644 --- a/pkg/apis/networking/v1alpha1/modelserver_types.go +++ b/pkg/apis/networking/v1alpha1/modelserver_types.go @@ -116,6 +116,9 @@ type ModelServerConditionType string const ( // ModelServerConditionReady indicates at least one ready pod is registered in the router store. ModelServerConditionReady ModelServerConditionType = "Ready" + + // ModelServerConditionAllPodsReady indicates every matched pod is ready and registered in the router store. + ModelServerConditionAllPodsReady ModelServerConditionType = "AllPodsReady" ) // KVConnectorSpec defines KV connector configuration for PD disaggregated routing @@ -168,8 +171,9 @@ type ModelServerStatus struct { // Conditions track the lifecycle of this ModelServer. // Types: // - "Ready": true when at least one ready pod is matched and the ModelServer has been - // registered with the router store. Signals that `kubectl wait --for=condition=ready` - // can be used. + // registered with the router store. Signals that the ModelServer can serve traffic. + // - "AllPodsReady": true when at least one pod is matched and every matched pod is ready + // and registered with the router store. // +optional // +patchMergeKey=type // +patchStrategy=merge diff --git a/pkg/kthena-router/controller/modelserver_controller.go b/pkg/kthena-router/controller/modelserver_controller.go index 02c69bade..726fcbef8 100644 --- a/pkg/kthena-router/controller/modelserver_controller.go +++ b/pkg/kthena-router/controller/modelserver_controller.go @@ -54,6 +54,15 @@ const ( ResourceTypePod ResourceType = "Pod" ) +// Condition reasons for ModelServer status. +const ( + ReasonNoReadyPods = "NoReadyPods" + ReasonPodsReady = "PodsReady" + ReasonNoMatchedPods = "NoMatchedPods" + ReasonPodsNotFullyReady = "PodsNotFullyReady" + ReasonAllPodsReady = "AllPodsReady" +) + // QueueItem represents an item in the work queue type QueueItem struct { ResourceType ResourceType @@ -268,20 +277,41 @@ func (c *ModelServerController) syncModelServerHandler(key string) error { } func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServer, podList []*corev1.Pod, readyPods sets.Set[types.NamespacedName]) error { - expectedReadyStatus := metav1.ConditionFalse - expectedReason := "NoReadyPods" - if readyPods.Len() > 0 { - expectedReadyStatus = metav1.ConditionTrue - expectedReason = "PodsReady" + matchedReplicas := int32(len(podList)) + readyReplicas := int32(readyPods.Len()) + + // Ready condition (at least one pod ready) + readyStatus := metav1.ConditionFalse + readyReason := ReasonNoReadyPods + readyMessage := "No ready pods match workloadSelector" + if readyReplicas > 0 { + readyStatus = metav1.ConditionTrue + readyReason = ReasonPodsReady + readyMessage = fmt.Sprintf("%d ready pods matched", readyReplicas) + } + + // AllPodsReady condition (every matched pod ready) + allPodsReadyStatus := metav1.ConditionFalse + allPodsReadyReason := ReasonNoMatchedPods + allPodsReadyMessage := "No pods match workloadSelector" + if matchedReplicas > 0 { + allPodsReadyReason = ReasonPodsNotFullyReady + allPodsReadyMessage = fmt.Sprintf("%d of %d matched pods are ready", readyReplicas, matchedReplicas) + if readyReplicas == matchedReplicas { + allPodsReadyStatus = metav1.ConditionTrue + allPodsReadyReason = ReasonAllPodsReady + allPodsReadyMessage = fmt.Sprintf("All %d matched pods are ready", matchedReplicas) + } } // Check if status is already up-to-date if ms.Status.ObservedGeneration == ms.Generation && - ms.Status.MatchedReplicas == int32(len(podList)) && - ms.Status.ReadyReplicas == int32(readyPods.Len()) { - if cond := meta.FindStatusCondition(ms.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)); cond != nil && - cond.Status == expectedReadyStatus && - cond.Reason == expectedReason { + ms.Status.MatchedReplicas == matchedReplicas && + ms.Status.ReadyReplicas == readyReplicas { + readyCond := meta.FindStatusCondition(ms.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + allReadyCond := meta.FindStatusCondition(ms.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + if readyCond != nil && readyCond.Status == readyStatus && readyCond.Reason == readyReason && + allReadyCond != nil && allReadyCond.Status == allPodsReadyStatus && allReadyCond.Reason == allPodsReadyReason { return nil } } @@ -296,24 +326,22 @@ func (c *ModelServerController) updateModelServerStatus(ms *aiv1alpha1.ModelServ msCopy := latest.DeepCopy() msCopy.Status.ObservedGeneration = latest.Generation - msCopy.Status.MatchedReplicas = int32(len(podList)) - msCopy.Status.ReadyReplicas = int32(readyPods.Len()) - - if readyPods.Len() > 0 { - meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ - Type: string(aiv1alpha1.ModelServerConditionReady), - Status: metav1.ConditionTrue, - Reason: "PodsReady", - Message: fmt.Sprintf("%d ready pods matched", readyPods.Len()), - }) - } else { - meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ - Type: string(aiv1alpha1.ModelServerConditionReady), - Status: metav1.ConditionFalse, - Reason: "NoReadyPods", - Message: "No ready pods match workloadSelector", - }) - } + msCopy.Status.MatchedReplicas = matchedReplicas + msCopy.Status.ReadyReplicas = readyReplicas + + meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ + Type: string(aiv1alpha1.ModelServerConditionReady), + Status: readyStatus, + Reason: readyReason, + Message: readyMessage, + }) + + meta.SetStatusCondition(&msCopy.Status.Conditions, metav1.Condition{ + Type: string(aiv1alpha1.ModelServerConditionAllPodsReady), + Status: allPodsReadyStatus, + Reason: allPodsReadyReason, + Message: allPodsReadyMessage, + }) _, err = c.kthenaClient.NetworkingV1alpha1().ModelServers(ms.Namespace).UpdateStatus(ctx, msCopy, metav1.UpdateOptions{}) return err diff --git a/pkg/kthena-router/controller/modelserver_controller_test.go b/pkg/kthena-router/controller/modelserver_controller_test.go index d331617fe..6142f8293 100644 --- a/pkg/kthena-router/controller/modelserver_controller_test.go +++ b/pkg/kthena-router/controller/modelserver_controller_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -1310,3 +1311,325 @@ func TestModelServerController_GetMatchingModelServers(t *testing.T) { assert.Empty(t, result) }) } + +func helperPod(name string, labels map[string]string, ready bool) *corev1.Pod { + phase := corev1.PodPending + condStatus := corev1.ConditionFalse + if ready { + phase = corev1.PodRunning + condStatus = corev1.ConditionTrue + } + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: name, + Labels: labels, + }, + Status: corev1.PodStatus{ + Phase: phase, + Conditions: []corev1.PodCondition{ + {Type: corev1.PodReady, Status: condStatus}, + }, + }, + } +} + +// TestModelServerController_ReadinessConditions verifies that the new AllPodsReady +// condition is set alongside Ready for empty / partial / full pod readiness. +func TestModelServerController_ReadinessConditions(t *testing.T) { + kthenaClient := kthenafake.NewSimpleClientset() + kubeClient := kubefake.NewSimpleClientset() + kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) + kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + store := newStoreWithMockBackend() + controller := NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) + + stop := make(chan struct{}) + defer close(stop) + kthenaInformerFactory.Start(stop) + kubeInformerFactory.Start(stop) + waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) + + // zero matched pods: Ready=False, AllPodsReady=False + t.Run(ReasonNoMatchedPods, func(t *testing.T) { + noMatchLabels := map[string]string{"app": "readiness-no-match"} + ms := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "ms-no-pods"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: noMatchLabels, + }, + }, + } + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, e := controller.modelServerLister.ModelServers("default").Get("ms-no-pods") + return e == nil + }) + + assert.NoError(t, controller.syncModelServerHandler("default/ms-no-pods")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-no-pods", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, int32(0), updated.Status.MatchedReplicas) + assert.Equal(t, int32(0), updated.Status.ReadyReplicas) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionFalse, readyCond.Status) + assert.Equal(t, ReasonNoReadyPods, readyCond.Reason) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionFalse, allReadyCond.Status) + assert.Equal(t, ReasonNoMatchedPods, allReadyCond.Reason) + }) + + // 1/2 pods ready: Ready=True, AllPodsReady=False + t.Run("PartialReadiness", func(t *testing.T) { + partialLabels := map[string]string{"app": "readiness-partial"} + ms := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "ms-partial"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: partialLabels, + }, + }, + } + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, e := controller.modelServerLister.ModelServers("default").Get("ms-partial") + return e == nil + }) + + // Create 1 ready + 1 not-ready pod + readyPod := helperPod("ready-a", partialLabels, true) + notReadyPod := helperPod("notready-a", partialLabels, false) + _, _ = kubeClient.CoreV1().Pods("default").Create(context.Background(), readyPod, metav1.CreateOptions{}) + _, _ = kubeClient.CoreV1().Pods("default").Create(context.Background(), notReadyPod, metav1.CreateOptions{}) + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("notready-a") + return e == nil && p.Name == "notready-a" + }) + + assert.NoError(t, controller.syncModelServerHandler("default/ms-partial")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-partial", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, int32(2), updated.Status.MatchedReplicas) + assert.Equal(t, int32(1), updated.Status.ReadyReplicas) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionTrue, readyCond.Status) + assert.Equal(t, ReasonPodsReady, readyCond.Reason) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionFalse, allReadyCond.Status) + assert.Equal(t, ReasonPodsNotFullyReady, allReadyCond.Reason) + }) + + // 2/2 pods ready: Ready=True, AllPodsReady=True + t.Run("FullReadiness", func(t *testing.T) { + fullLabels := map[string]string{"app": "readiness-full"} + ms := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "ms-full"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: fullLabels, + }, + }, + } + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, e := controller.modelServerLister.ModelServers("default").Get("ms-full") + return e == nil + }) + + pod1 := helperPod("ready-full-1", fullLabels, true) + pod2 := helperPod("ready-full-2", fullLabels, true) + _, _ = kubeClient.CoreV1().Pods("default").Create(context.Background(), pod1, metav1.CreateOptions{}) + _, _ = kubeClient.CoreV1().Pods("default").Create(context.Background(), pod2, metav1.CreateOptions{}) + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("ready-full-2") + return e == nil && p.Name == "ready-full-2" + }) + + assert.NoError(t, controller.syncModelServerHandler("default/ms-full")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-full", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, int32(2), updated.Status.MatchedReplicas) + assert.Equal(t, int32(2), updated.Status.ReadyReplicas) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionTrue, readyCond.Status) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionTrue, allReadyCond.Status) + assert.Equal(t, ReasonAllPodsReady, allReadyCond.Reason) + }) +} + +// TestModelServerController_ReadinessConditionTransitions verifies condition +// values across a lifecycle: 0 pods → 1/1 ready → 1/2 ready → 2/2 ready. +func TestModelServerController_ReadinessConditionTransitions(t *testing.T) { + kthenaClient := kthenafake.NewSimpleClientset() + kubeClient := kubefake.NewSimpleClientset() + kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) + kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) + store := newStoreWithMockBackend() + controller := NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) + + stop := make(chan struct{}) + defer close(stop) + kthenaInformerFactory.Start(stop) + kubeInformerFactory.Start(stop) + waitForCacheSync(t, 5*time.Second, controller.modelServerSynced, controller.podSynced) + + tLabels := map[string]string{"app": "readiness-trans"} + + ms := &aiv1alpha1.ModelServer{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "ms-trans"}, + Spec: aiv1alpha1.ModelServerSpec{ + InferenceEngine: aiv1alpha1.VLLM, + WorkloadSelector: &aiv1alpha1.WorkloadSelector{ + MatchLabels: tLabels, + }, + }, + } + _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + context.Background(), ms, metav1.CreateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + _, e := controller.modelServerLister.ModelServers("default").Get("ms-trans") + return e == nil + }) + + // Step 1: no matching pods + t.Run("Step1_NoMatchingPods", func(t *testing.T) { + assert.NoError(t, controller.syncModelServerHandler("default/ms-trans")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-trans", metav1.GetOptions{}) + require.NoError(t, err) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionFalse, readyCond.Status) + assert.Equal(t, ReasonNoReadyPods, readyCond.Reason) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionFalse, allReadyCond.Status) + assert.Equal(t, ReasonNoMatchedPods, allReadyCond.Reason) + assert.Equal(t, int32(0), updated.Status.MatchedReplicas) + }) + + // Step 2: add one ready pod → Ready=True, AllPodsReady=True (only 1 matched) + t.Run("Step2_AllPodsReadyWithOnePod", func(t *testing.T) { + readyPod := helperPod("trans-pod-1", tLabels, true) + _, err := kubeClient.CoreV1().Pods("default").Create(context.Background(), readyPod, metav1.CreateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("trans-pod-1") + return e == nil && p.Name == "trans-pod-1" + }) + + assert.NoError(t, controller.syncModelServerHandler("default/ms-trans")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-trans", metav1.GetOptions{}) + require.NoError(t, err) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionTrue, readyCond.Status) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionTrue, allReadyCond.Status) + assert.Equal(t, ReasonAllPodsReady, allReadyCond.Reason) + + assert.Equal(t, int32(1), updated.Status.MatchedReplicas) + assert.Equal(t, int32(1), updated.Status.ReadyReplicas) + }) + + // Step 3: add one not-ready pod → Ready=True, AllPodsReady=False (1/2) + t.Run("Step3_PartialAfterNotReady", func(t *testing.T) { + notReadyPod := helperPod("trans-pod-2", tLabels, false) + _, err := kubeClient.CoreV1().Pods("default").Create(context.Background(), notReadyPod, metav1.CreateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("trans-pod-2") + return e == nil && p.Name == "trans-pod-2" + }) + + assert.NoError(t, controller.syncModelServerHandler("default/ms-trans")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-trans", metav1.GetOptions{}) + require.NoError(t, err) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionTrue, readyCond.Status) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionFalse, allReadyCond.Status) + assert.Equal(t, ReasonPodsNotFullyReady, allReadyCond.Reason) + + assert.Equal(t, int32(2), updated.Status.MatchedReplicas) + assert.Equal(t, int32(1), updated.Status.ReadyReplicas) + }) + + // Step 4: make the second pod ready → Ready=True, AllPodsReady=True (2/2) + t.Run("Step4_FullReadyAfterTransition", func(t *testing.T) { + // Update not-ready pod to ready + notReadyPod, err := kubeClient.CoreV1().Pods("default").Get(context.Background(), "trans-pod-2", metav1.GetOptions{}) + require.NoError(t, err) + updatedPod := notReadyPod.DeepCopy() + updatedPod.Status.Phase = corev1.PodRunning + updatedPod.Status.Conditions[0].Status = corev1.ConditionTrue + _, err = kubeClient.CoreV1().Pods("default").Update(context.Background(), updatedPod, metav1.UpdateOptions{}) + require.NoError(t, err) + waitForObjectInCache(t, 2*time.Second, func() bool { + p, e := controller.podLister.Pods("default").Get("trans-pod-2") + return e == nil && isPodReady(p) + }) + + assert.NoError(t, controller.syncModelServerHandler("default/ms-trans")) + + updated, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Get( + context.Background(), "ms-trans", metav1.GetOptions{}) + require.NoError(t, err) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionReady)) + require.NotNil(t, readyCond) + assert.Equal(t, metav1.ConditionTrue, readyCond.Status) + + allReadyCond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelServerConditionAllPodsReady)) + require.NotNil(t, allReadyCond) + assert.Equal(t, metav1.ConditionTrue, allReadyCond.Status) + assert.Equal(t, ReasonAllPodsReady, allReadyCond.Reason) + + assert.Equal(t, int32(2), updated.Status.MatchedReplicas) + assert.Equal(t, int32(2), updated.Status.ReadyReplicas) + }) +} From f49b42c9e65644af396291fbff891890f9e6b3ff Mon Sep 17 00:00:00 2001 From: Leo Xie Date: Fri, 10 Jul 2026 17:01:44 +0800 Subject: [PATCH 11/11] chore: introduce reason constant Signed-off-by: Leo Xie --- .../networking/v1alpha1/modelroute_types.go | 4 ++++ .../controller/modelroute_controller.go | 21 ++++++++++++------- .../controller/modelroute_controller_test.go | 5 +++-- .../controller/modelserver_controller_test.go | 16 ++++++++------ 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/pkg/apis/networking/v1alpha1/modelroute_types.go b/pkg/apis/networking/v1alpha1/modelroute_types.go index 21632ff66..2b0202834 100644 --- a/pkg/apis/networking/v1alpha1/modelroute_types.go +++ b/pkg/apis/networking/v1alpha1/modelroute_types.go @@ -171,6 +171,10 @@ type ModelRouteConditionType string const ( // ModelRouteConditionReady indicates the route has been registered in the router store. ModelRouteConditionReady ModelRouteConditionType = "Ready" + + // ReasonRouteRegistered is the reason used when a ModelRoute has been successfully + // registered in the router store. + ReasonRouteRegistered = "RouteRegistered" ) // ModelRouteStatus defines the observed state of ModelRoute. diff --git a/pkg/kthena-router/controller/modelroute_controller.go b/pkg/kthena-router/controller/modelroute_controller.go index 22c7b83f4..e29f534cc 100644 --- a/pkg/kthena-router/controller/modelroute_controller.go +++ b/pkg/kthena-router/controller/modelroute_controller.go @@ -164,13 +164,18 @@ func (c *ModelRouteController) syncHandler(key string) error { } func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) error { - // Check if status is already up-to-date - if mr.Status.ObservedGeneration == mr.Generation { - if cond := meta.FindStatusCondition(mr.Status.Conditions, string(aiv1alpha1.ModelRouteConditionReady)); cond != nil && - cond.Status == metav1.ConditionTrue && - cond.Reason == "RouteRegistered" { - return nil - } + // If the Ready condition is already set with the expected values, + // no update is needed. We check the condition content rather than + // Status.ObservedGeneration to avoid unnecessary status writes on + // spec-only changes (e.g. rate limit updates). Unnecessary writes + // increment the resourceVersion and cause conflicts with clients + // that are simultaneously updating the same ModelRoute. + readyCond := meta.FindStatusCondition(mr.Status.Conditions, string(aiv1alpha1.ModelRouteConditionReady)) + if readyCond != nil && + readyCond.Status == metav1.ConditionTrue && + readyCond.Reason == aiv1alpha1.ReasonRouteRegistered && + readyCond.ObservedGeneration == mr.Generation { + return nil } return retry.RetryOnConflict(retry.DefaultRetry, func() error { @@ -187,7 +192,7 @@ func (c *ModelRouteController) updateModelRouteStatus(mr *aiv1alpha1.ModelRoute) meta.SetStatusCondition(&mrCopy.Status.Conditions, metav1.Condition{ Type: string(aiv1alpha1.ModelRouteConditionReady), Status: metav1.ConditionTrue, - Reason: "RouteRegistered", + Reason: aiv1alpha1.ReasonRouteRegistered, Message: "ModelRoute registered in router store", }) diff --git a/pkg/kthena-router/controller/modelroute_controller_test.go b/pkg/kthena-router/controller/modelroute_controller_test.go index 8a6c190ac..f866ef09d 100644 --- a/pkg/kthena-router/controller/modelroute_controller_test.go +++ b/pkg/kthena-router/controller/modelroute_controller_test.go @@ -206,7 +206,8 @@ func TestModelRouteController_StatusUpdate(t *testing.T) { kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := datastore.New() - controller := NewModelRouteController(kthenaClient, kthenaInformerFactory, store) + controller, err := NewModelRouteController(kthenaClient, kthenaInformerFactory, store) + require.NoError(t, err) stop := make(chan struct{}) defer close(stop) @@ -253,7 +254,7 @@ func TestModelRouteController_StatusUpdate(t *testing.T) { cond := meta.FindStatusCondition(updated.Status.Conditions, string(aiv1alpha1.ModelRouteConditionReady)) require.NotNil(t, cond) assert.Equal(t, metav1.ConditionTrue, cond.Status) - assert.Equal(t, "RouteRegistered", cond.Reason) + assert.Equal(t, aiv1alpha1.ReasonRouteRegistered, cond.Reason) }) // sync skips status update when already up-to-date diff --git a/pkg/kthena-router/controller/modelserver_controller_test.go b/pkg/kthena-router/controller/modelserver_controller_test.go index 4b9bf79b3..f305a11ac 100644 --- a/pkg/kthena-router/controller/modelserver_controller_test.go +++ b/pkg/kthena-router/controller/modelserver_controller_test.go @@ -78,7 +78,7 @@ func TestModelServerController_ModelServerLifecycle(t *testing.T) { store, ) require.NoError(t, err) - modelServerIndexer := kthenaInformerFactory.Networking().V1alpha1().ModelServers().Informer().GetIndexer() + _ = kthenaInformerFactory.Networking().V1alpha1().ModelServers().Informer().GetIndexer() stop := make(chan struct{}) defer close(stop) @@ -1157,9 +1157,10 @@ func TestModelServerController_PodEnqueuesModelServer(t *testing.T) { kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := newStoreWithMockBackend() - controller := NewModelServerController( + controller, err := NewModelServerController( kthenaClient, kthenaInformerFactory, kubeInformerFactory, store, ) + require.NoError(t, err) stop := make(chan struct{}) defer close(stop) @@ -1280,9 +1281,10 @@ func TestModelServerController_GetMatchingModelServers(t *testing.T) { kubeInformerFactory := informers.NewSharedInformerFactory(kubefake.NewSimpleClientset(), 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) - controller := NewModelServerController( + controller, err := NewModelServerController( kthenaClient, kthenaInformerFactory, kubeInformerFactory, newStoreWithMockBackend(), ) + require.NoError(t, err) stop := make(chan struct{}) defer close(stop) @@ -1350,7 +1352,8 @@ func TestModelServerController_ReadinessConditions(t *testing.T) { kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := newStoreWithMockBackend() - controller := NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) + controller, err := NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) + require.NoError(t, err) stop := make(chan struct{}) defer close(stop) @@ -1502,7 +1505,8 @@ func TestModelServerController_ReadinessConditionTransitions(t *testing.T) { kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0) kthenaInformerFactory := informersv1alpha1.NewSharedInformerFactory(kthenaClient, 0) store := newStoreWithMockBackend() - controller := NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) + controller, err := NewModelServerController(kthenaClient, kthenaInformerFactory, kubeInformerFactory, store) + require.NoError(t, err) stop := make(chan struct{}) defer close(stop) @@ -1521,7 +1525,7 @@ func TestModelServerController_ReadinessConditionTransitions(t *testing.T) { }, }, } - _, err := kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( + _, err = kthenaClient.NetworkingV1alpha1().ModelServers("default").Create( context.Background(), ms, metav1.CreateOptions{}) require.NoError(t, err) waitForObjectInCache(t, 2*time.Second, func() bool {