[misc] fully deprecate predictor (#53)

Author: slin1237

pkg/controller/v1beta1/inferenceservice/components/predictor.go

@@ -16,6 +16,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	v1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -335,6 +336,41 @@ func TestInferenceServiceReconcile(t *testing.T) {
 				err = c.Create(context.TODO(), baseModel)
 				g.Expect(err).NotTo(gomega.HaveOccurred())
 
+				// Create an old predictor deployment to simulate existing resources
+				oldDeployment := &appsv1.Deployment{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-legacy",
+						Namespace: "default",
+						Labels: map[string]string{
+							constants.InferenceServicePodLabelKey: "test-legacy",
+						},
+					},
+					Spec: appsv1.DeploymentSpec{
+						Selector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								constants.InferenceServicePodLabelKey: "test-legacy",
+							},
+						},
+						Template: v1.PodTemplateSpec{
+							ObjectMeta: metav1.ObjectMeta{
+								Labels: map[string]string{
+									constants.InferenceServicePodLabelKey: "test-legacy",
+								},
+							},
+							Spec: v1.PodSpec{
+								Containers: []v1.Container{
+									{
+										Name:  "predictor",
+										Image: "old-predictor-image:latest",
+									},
+								},
+							},
+						},
+					},
+				}
+				err = c.Create(context.TODO(), oldDeployment)
+				g.Expect(err).NotTo(gomega.HaveOccurred())
+
 				// Create sklearn runtime
 				rt := &v1beta1.ServingRuntime{
 					ObjectMeta: metav1.ObjectMeta{
@@ -371,34 +407,42 @@ func TestInferenceServiceReconcile(t *testing.T) {
 				g.Expect(err).NotTo(gomega.HaveOccurred())
 			},
 			validate: func(t *testing.T, c client.Client, isvc *v1beta1.InferenceService) {
-				// Check that predictor deployment was created
-				// The deployment name should be either "test-legacy-predictor-default" or "test-legacy"
-				deployment := &appsv1.Deployment{}
-
-				// Try the default predictor name first
+				// The migration should have happened, check that the resource was updated
+				updatedIsvc := &v1beta1.InferenceService{}
 				err := c.Get(context.TODO(), types.NamespacedName{
-					Name:      "test-legacy-predictor-default",
+					Name:      "test-legacy",
 					Namespace: "default",
-				}, deployment)
+				}, updatedIsvc)
+				g.Expect(err).NotTo(gomega.HaveOccurred())
 
-				if err != nil {
-					// If not found, try the shorter name
-					err = c.Get(context.TODO(), types.NamespacedName{
-						Name:      "test-legacy",
-						Namespace: "default",
-					}, deployment)
-				}
+				// Check that migration happened
+				g.Expect(updatedIsvc.Spec.Model).NotTo(gomega.BeNil(), "Expected model to be migrated")
+				g.Expect(updatedIsvc.Spec.Model.Name).To(gomega.Equal("sklearn-model"))
+				g.Expect(updatedIsvc.Spec.Engine).NotTo(gomega.BeNil(), "Expected engine to be created")
 
-				// The deployment should exist
-				g.Expect(err).NotTo(gomega.HaveOccurred(), "Expected predictor deployment to be created")
+				// Check that predictor is cleared
+				g.Expect(updatedIsvc.Spec.Predictor.Model).To(gomega.BeNil(), "Expected predictor.Model to be cleared")
 
-				// Verify it's using the sklearn runtime image
-				g.Expect(deployment.Spec.Template.Spec.Containers).To(gomega.HaveLen(1))
-				g.Expect(deployment.Spec.Template.Spec.Containers[0].Image).To(gomega.Equal("sklearn-server:v1"))
+				// Check for deprecation warning
+				g.Expect(updatedIsvc.ObjectMeta.Annotations).NotTo(gomega.BeNil())
+				g.Expect(updatedIsvc.ObjectMeta.Annotations[constants.DeprecationWarning]).To(gomega.Equal("The Predictor field is deprecated and will be removed in a future release. Please use Engine and Model fields instead."))
 
-				// Verify labels
-				g.Expect(deployment.Spec.Template.Labels).To(gomega.HaveKey(constants.InferenceServicePodLabelKey))
-				g.Expect(deployment.Spec.Template.Labels[constants.InferenceServicePodLabelKey]).To(gomega.Equal("test-legacy"))
+				// Check that old predictor deployment was deleted
+				deployment := &appsv1.Deployment{}
+				err = c.Get(context.TODO(), types.NamespacedName{
+					Name:      "test-legacy", // predictor deployment uses inference service name
+					Namespace: "default",
+				}, deployment)
+				g.Expect(err).To(gomega.HaveOccurred(), "Expected old predictor deployment to be deleted")
+				g.Expect(apierrors.IsNotFound(err)).To(gomega.BeTrue(), "Expected deployment to not be found")
+
+				// Check that new engine deployment was created
+				engineDeployment := &appsv1.Deployment{}
+				err = c.Get(context.TODO(), types.NamespacedName{
+					Name:      "test-legacy-engine",
+					Namespace: "default",
+				}, engineDeployment)
+				g.Expect(err).NotTo(gomega.HaveOccurred(), "Expected engine deployment to be created")
 			},
 		},
 		{

pkg/controller/v1beta1/inferenceservice/components/predictor_v2.go

@@ -27,41 +27,41 @@ func NewStatusReconciler() *StatusReconciler {
 }
 
 // PropagateRawStatus propagates status from raw Kubernetes deployment
-func (sm *StatusReconciler) PropagateRawStatus(
+func (sr *StatusReconciler) PropagateRawStatus(
 	status *v1beta1.InferenceServiceStatus,
 	component v1beta1.ComponentType,
 	deployment *appsv1.Deployment,
 	url *apis.URL) {
 
-	statusSpec := sm.initializeComponentStatus(status, component)
+	statusSpec := sr.initializeComponentStatus(status, component)
 
 	statusSpec.LatestCreatedRevision = deployment.GetObjectMeta().GetAnnotations()["deployment.kubernetes.io/revision"]
-	condition := sm.getDeploymentCondition(deployment, appsv1.DeploymentAvailable)
+	condition := sr.getDeploymentCondition(deployment, appsv1.DeploymentAvailable)
 	if condition != nil && condition.Status == v1.ConditionTrue {
 		statusSpec.URL = url
 	}
-	readyCondition := sm.getReadyConditionsMap()[component]
-	sm.setCondition(status, readyCondition, condition)
+	readyCondition := sr.getReadyConditionsMap()[component]
+	sr.setCondition(status, readyCondition, condition)
 	status.Components[component] = statusSpec
 	status.ObservedGeneration = deployment.Status.ObservedGeneration
 }
 
 // PropagateMultiNodeStatus propagates status from LeaderWorkerSet
-func (sm *StatusReconciler) PropagateMultiNodeStatus(
+func (sr *StatusReconciler) PropagateMultiNodeStatus(
 	status *v1beta1.InferenceServiceStatus,
 	component v1beta1.ComponentType,
 	lws *lwsspec.LeaderWorkerSet,
 	url *apis.URL) {
 
-	statusSpec := sm.initializeComponentStatus(status, component)
+	statusSpec := sr.initializeComponentStatus(status, component)
 
 	statusSpec.LatestCreatedRevision = lws.GetObjectMeta().GetAnnotations()["resourceVersion"]
-	lwsCondition := sm.getLWSConditions(lws, lwsspec.LeaderWorkerSetAvailable)
+	lwsCondition := sr.getLWSConditions(lws, lwsspec.LeaderWorkerSetAvailable)
 	if lwsCondition != nil && lwsCondition.Status == v1.ConditionTrue {
 		statusSpec.URL = url
 	}
 
-	readyCondition := sm.getReadyConditionsMap()[component]
+	readyCondition := sr.getReadyConditionsMap()[component]
 
 	// Create a new condition with the correct component ready condition type
 	// instead of using the LWS condition type directly
@@ -73,27 +73,27 @@ func (sm *StatusReconciler) PropagateMultiNodeStatus(
 			Reason:             lwsCondition.Reason,
 			LastTransitionTime: lwsCondition.LastTransitionTime,
 		}
-		sm.setCondition(status, readyCondition, componentCondition)
+		sr.setCondition(status, readyCondition, componentCondition)
 	}
 
 	status.Components[component] = statusSpec
 	status.ObservedGeneration = lws.Generation
 }
 
 // PropagateMultiNodeRayVLLMStatus propagates status from multiple deployments
-func (sm *StatusReconciler) PropagateMultiNodeRayVLLMStatus(
+func (sr *StatusReconciler) PropagateMultiNodeRayVLLMStatus(
 	status *v1beta1.InferenceServiceStatus,
 	component v1beta1.ComponentType,
 	deployments []*appsv1.Deployment,
 	url *apis.URL) {
 
-	statusSpec := sm.initializeComponentStatus(status, component)
+	statusSpec := sr.initializeComponentStatus(status, component)
 
-	firstDeployment, err := sm.getFirstDeployment(deployments)
+	firstDeployment, err := sr.getFirstDeployment(deployments)
 	if err != nil {
 		// Handle error case gracefully - set a default state
-		sm.setCondition(status, sm.getReadyConditionsMap()[component], &apis.Condition{
-			Type:    sm.getReadyConditionsMap()[component],
+		sr.setCondition(status, sr.getReadyConditionsMap()[component], &apis.Condition{
+			Type:    sr.getReadyConditionsMap()[component],
 			Status:  v1.ConditionFalse,
 			Reason:  "NoDeployments",
 			Message: "No deployments available",
@@ -103,23 +103,23 @@ func (sm *StatusReconciler) PropagateMultiNodeRayVLLMStatus(
 
 	statusSpec.LatestCreatedRevision = firstDeployment.GetObjectMeta().GetAnnotations()["deployment.kubernetes.io/revision"]
 
-	condition := sm.getMultiDeploymentCondition(deployments, appsv1.DeploymentAvailable)
+	condition := sr.getMultiDeploymentCondition(deployments, appsv1.DeploymentAvailable)
 	if condition != nil && condition.Status == v1.ConditionTrue {
 		statusSpec.URL = url
 	}
-	readyCondition := sm.getReadyConditionsMap()[component]
-	sm.setCondition(status, readyCondition, condition)
+	readyCondition := sr.getReadyConditionsMap()[component]
+	sr.setCondition(status, readyCondition, condition)
 	status.Components[component] = statusSpec
 	status.ObservedGeneration = firstDeployment.Status.ObservedGeneration
 }
 
 // PropagateStatus propagates status from Knative Service
-func (sm *StatusReconciler) PropagateStatus(
+func (sr *StatusReconciler) PropagateStatus(
 	status *v1beta1.InferenceServiceStatus,
 	component v1beta1.ComponentType,
 	serviceStatus *knservingv1.ServiceStatus) {
 
-	statusSpec := sm.initializeComponentStatus(status, component)
+	statusSpec := sr.initializeComponentStatus(status, component)
 
 	statusSpec.LatestCreatedRevision = serviceStatus.LatestCreatedRevisionName
 	revisionTraffic := map[string]int64{}
@@ -130,21 +130,21 @@ func (sm *StatusReconciler) PropagateStatus(
 	}
 
 	// Handle traffic routing logic
-	sm.handleTrafficRouting(&statusSpec, serviceStatus, revisionTraffic)
+	sr.handleTrafficRouting(&statusSpec, serviceStatus, revisionTraffic)
 
 	if serviceStatus.LatestReadyRevisionName != statusSpec.LatestReadyRevision {
 		statusSpec.LatestReadyRevision = serviceStatus.LatestReadyRevisionName
 	}
 
 	// Propagate conditions
-	sm.propagateServiceConditions(status, component, serviceStatus, &statusSpec)
+	sr.propagateServiceConditions(status, component, serviceStatus, &statusSpec)
 
 	status.Components[component] = statusSpec
 	status.ObservedGeneration = serviceStatus.ObservedGeneration
 }
 
 // PropagateModelStatus propagates model status from pod information
-func (sm *StatusReconciler) PropagateModelStatus(
+func (sr *StatusReconciler) PropagateModelStatus(
 	status *v1beta1.InferenceServiceStatus,
 	statusSpec v1beta1.ComponentStatusSpec,
 	podList *v1.PodList,
@@ -153,34 +153,34 @@ func (sm *StatusReconciler) PropagateModelStatus(
 	// Check at least one pod is running for the latest revision of inferenceservice
 	totalCopies := len(podList.Items)
 	if totalCopies == 0 {
-		sm.UpdateModelRevisionStates(status, v1beta1.Pending, totalCopies, nil)
+		sr.UpdateModelRevisionStates(status, v1beta1.Pending, totalCopies, nil)
 		return
 	}
 
 	// Use helper function to safely get the first pod
-	firstPod, err := sm.getFirstPod(podList)
+	firstPod, err := sr.getFirstPod(podList)
 	if err != nil {
-		sm.UpdateModelRevisionStates(status, v1beta1.Pending, totalCopies, nil)
+		sr.UpdateModelRevisionStates(status, v1beta1.Pending, totalCopies, nil)
 		return
 	}
 
 	// Update model state to 'Loaded' if inferenceservice status is ready.
 	if status.IsReady() {
 		if rawDeployment {
-			sm.UpdateModelRevisionStates(status, v1beta1.Loaded, totalCopies, nil)
+			sr.UpdateModelRevisionStates(status, v1beta1.Loaded, totalCopies, nil)
 			return
 		} else if statusSpec.LatestCreatedRevision == statusSpec.LatestReadyRevision {
-			sm.UpdateModelRevisionStates(status, v1beta1.Loaded, totalCopies, nil)
+			sr.UpdateModelRevisionStates(status, v1beta1.Loaded, totalCopies, nil)
 			return
 		}
 	}
 
 	// Check container statuses
-	sm.checkContainerStatuses(status, firstPod, totalCopies)
+	sr.checkContainerStatuses(status, firstPod, totalCopies)
 }
 
 // UpdateModelRevisionStates updates the model revision states
-func (sm *StatusReconciler) UpdateModelRevisionStates(
+func (sr *StatusReconciler) UpdateModelRevisionStates(
 	status *v1beta1.InferenceServiceStatus,
 	modelState v1beta1.ModelState,
 	totalCopies int,
@@ -205,12 +205,12 @@ func (sm *StatusReconciler) UpdateModelRevisionStates(
 	}
 
 	if info != nil {
-		sm.SetModelFailureInfo(status, info)
+		sr.SetModelFailureInfo(status, info)
 	}
 }
 
 // UpdateModelTransitionStatus updates the model transition status
-func (sm *StatusReconciler) UpdateModelTransitionStatus(
+func (sr *StatusReconciler) UpdateModelTransitionStatus(
 	status *v1beta1.InferenceServiceStatus,
 	transitionStatus v1beta1.TransitionStatus,
 	info *v1beta1.FailureInfo) {
@@ -227,12 +227,12 @@ func (sm *StatusReconciler) UpdateModelTransitionStatus(
 	}
 
 	if info != nil {
-		sm.SetModelFailureInfo(status, info)
+		sr.SetModelFailureInfo(status, info)
 	}
 }
 
 // SetModelFailureInfo sets the model failure information
-func (sm *StatusReconciler) SetModelFailureInfo(status *v1beta1.InferenceServiceStatus, info *v1beta1.FailureInfo) bool {
+func (sr *StatusReconciler) SetModelFailureInfo(status *v1beta1.InferenceServiceStatus, info *v1beta1.FailureInfo) bool {
 	if reflect.DeepEqual(info, status.ModelStatus.LastFailureInfo) {
 		return false
 	}
@@ -241,12 +241,12 @@ func (sm *StatusReconciler) SetModelFailureInfo(status *v1beta1.InferenceService
 }
 
 // PropagateCrossComponentStatus aggregates conditions across components
-func (sm *StatusReconciler) PropagateCrossComponentStatus(
+func (sr *StatusReconciler) PropagateCrossComponentStatus(
 	status *v1beta1.InferenceServiceStatus,
 	componentList []v1beta1.ComponentType,
 	conditionType apis.ConditionType) {
 
-	conditionsMap, ok := sm.getConditionsMapIndex()[conditionType]
+	conditionsMap, ok := sr.getConditionsMapIndex()[conditionType]
 	if !ok {
 		return
 	}
@@ -266,5 +266,5 @@ func (sm *StatusReconciler) PropagateCrossComponentStatus(
 		}
 	}
 
-	sm.setCondition(status, conditionType, crossComponentCondition)
+	sr.setCondition(status, conditionType, crossComponentCondition)
 }

pkg/controller/v1beta1/inferenceservice/components/predictor_v2_test.go

@@ -3,7 +3,6 @@ package utils
 import (
 	"strings"
 
-	"github.com/sgl-project/ome/pkg/apis/ome/v1beta1"
 	"github.com/sgl-project/ome/pkg/constants"
 	"github.com/sgl-project/ome/pkg/controller/v1beta1/controllerconfig"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -34,25 +33,6 @@ func IsOriginalModelVolumeMountNecessary(annotations map[string]string) bool {
 		annotations[constants.FTServingWithMergedWeightsAnnotationKey] != "true"
 }
 
-// IsEntrypointRouter checks if the InferenceService has the
-// entrypoint-component annotation set to router.
-// Returns true if the annotation is set to router, false otherwise.
-func IsEntrypointRouter(annotations map[string]string) bool {
-	componentValue, hasAnnotation := annotations[constants.EntrypointComponent]
-	if !hasAnnotation {
-		return false
-	}
-
-	// Validate against known component types
-	switch v1beta1.ComponentType(componentValue) {
-	case v1beta1.RouterComponent:
-		return true
-	default:
-		// Invalid component type
-		return false
-	}
-}
-
 func IsEmptyModelDirVolumeRequired(annotations map[string]string) bool {
 	modelInitInject := annotations[constants.ModelInitInjectionKey]
 	fineTunedAdapterInject := annotations[constants.FineTunedAdapterInjectionKey]