Skip to content

Commit 8cec2b5

Browse files
jlostjlost
authored
fix: incl. port 8080 in status.address.url for headless svcs (kserve#4998)
1 parent 37b1f9a commit 8cec2b5

6 files changed

Lines changed: 268 additions & 20 deletions

File tree

‎pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9644,6 +9644,173 @@ var _ = Describe("v1beta1 inference service controller", func() {
96449644
}, timeout, interval).Should(BeTrue())
96459645
})
96469646
})
9647+
9648+
Context("When creating inference service with headless service (ServiceClusterIPNone)", func() {
9649+
It("Should include port 8080 in status.address.url", func() {
9650+
By("By creating a new InferenceService with headless service enabled")
9651+
ctx := context.Background()
9652+
9653+
// Create configmap with serviceClusterIPNone enabled
9654+
configs := map[string]string{
9655+
"ingress": `{
9656+
"ingressGateway": "knative-serving/knative-ingress-gateway",
9657+
"localGateway": "knative-serving/knative-local-gateway",
9658+
"localGatewayService": "knative-local-gateway.istio-system.svc.cluster.local"
9659+
}`,
9660+
"storageInitializer": `{
9661+
"image": "kserve/storage-initializer:latest",
9662+
"memoryRequest": "100Mi",
9663+
"memoryLimit": "1Gi",
9664+
"cpuRequest": "100m",
9665+
"cpuLimit": "1",
9666+
"caBundleConfigMapName": "",
9667+
"caBundleVolumeMountPath": "/etc/ssl/custom-certs",
9668+
"cpuModelcar": "10m",
9669+
"memoryModelcar": "15Mi"
9670+
}`,
9671+
"service": `{"serviceClusterIPNone": true}`,
9672+
}
9673+
configMap := createInferenceServiceConfigMap(configs)
9674+
Expect(k8sClient.Create(ctx, configMap)).NotTo(HaveOccurred())
9675+
defer k8sClient.Delete(ctx, configMap)
9676+
9677+
// Create ServingRuntime
9678+
servingRuntime := &v1alpha1.ServingRuntime{
9679+
ObjectMeta: metav1.ObjectMeta{
9680+
Name: "tf-serving-raw",
9681+
Namespace: "default",
9682+
},
9683+
Spec: v1alpha1.ServingRuntimeSpec{
9684+
SupportedModelFormats: []v1alpha1.SupportedModelFormat{
9685+
{
9686+
Name: "tensorflow",
9687+
Version: ptr.To("1"),
9688+
AutoSelect: ptr.To(true),
9689+
},
9690+
},
9691+
ServingRuntimePodSpec: v1alpha1.ServingRuntimePodSpec{
9692+
Containers: []corev1.Container{
9693+
{
9694+
Name: "kserve-container",
9695+
Image: "tensorflow/serving:1.14.0",
9696+
Command: []string{"/usr/bin/tensorflow_model_server"},
9697+
Args: []string{
9698+
"--port=9000",
9699+
"--rest_api_port=8080",
9700+
"--model_base_path=/mnt/models",
9701+
"--rest_api_timeout_in_ms=60000",
9702+
},
9703+
Resources: defaultResource,
9704+
},
9705+
},
9706+
},
9707+
Disabled: ptr.To(false),
9708+
},
9709+
}
9710+
k8sClient.Create(ctx, servingRuntime)
9711+
defer k8sClient.Delete(ctx, servingRuntime)
9712+
9713+
serviceName := "raw-headless-port"
9714+
expectedRequest := reconcile.Request{NamespacedName: types.NamespacedName{Name: serviceName, Namespace: "default"}}
9715+
serviceKey := expectedRequest.NamespacedName
9716+
storageUri := "s3://test/mnist/export"
9717+
9718+
isvc := &v1beta1.InferenceService{
9719+
ObjectMeta: metav1.ObjectMeta{
9720+
Name: serviceKey.Name,
9721+
Namespace: serviceKey.Namespace,
9722+
Annotations: map[string]string{
9723+
constants.DeploymentMode: string(constants.Standard),
9724+
},
9725+
},
9726+
Spec: v1beta1.InferenceServiceSpec{
9727+
Predictor: v1beta1.PredictorSpec{
9728+
ComponentExtensionSpec: v1beta1.ComponentExtensionSpec{
9729+
MinReplicas: ptr.To(int32(1)),
9730+
MaxReplicas: 3,
9731+
},
9732+
Tensorflow: &v1beta1.TFServingSpec{
9733+
PredictorExtensionSpec: v1beta1.PredictorExtensionSpec{
9734+
StorageURI: &storageUri,
9735+
RuntimeVersion: ptr.To("1.14.0"),
9736+
Container: corev1.Container{
9737+
Name: constants.InferenceServiceContainerName,
9738+
Resources: defaultResource,
9739+
},
9740+
},
9741+
},
9742+
},
9743+
},
9744+
}
9745+
isvc.DefaultInferenceService(nil, nil, &v1beta1.SecurityConfig{AutoMountServiceAccountToken: false}, nil)
9746+
Expect(k8sClient.Create(ctx, isvc)).Should(Succeed())
9747+
defer k8sClient.Delete(ctx, isvc)
9748+
9749+
inferenceService := &v1beta1.InferenceService{}
9750+
Eventually(func() bool {
9751+
err := k8sClient.Get(ctx, serviceKey, inferenceService)
9752+
return err == nil
9753+
}, timeout, interval).Should(BeTrue())
9754+
9755+
// Wait for deployment to be created
9756+
actualDeployment := &appsv1.Deployment{}
9757+
predictorDeploymentKey := types.NamespacedName{
9758+
Name: constants.PredictorServiceName(serviceKey.Name),
9759+
Namespace: serviceKey.Namespace,
9760+
}
9761+
Eventually(func() error {
9762+
return k8sClient.Get(ctx, predictorDeploymentKey, actualDeployment)
9763+
}, timeout, interval).Should(Succeed())
9764+
9765+
// Wait for service to be created and verify it's headless
9766+
actualService := &corev1.Service{}
9767+
predictorServiceKey := types.NamespacedName{
9768+
Name: constants.PredictorServiceName(serviceKey.Name),
9769+
Namespace: serviceKey.Namespace,
9770+
}
9771+
Eventually(func() error {
9772+
return k8sClient.Get(ctx, predictorServiceKey, actualService)
9773+
}, timeout, interval).Should(Succeed())
9774+
9775+
// Verify service is headless (ClusterIP: None)
9776+
Expect(actualService.Spec.ClusterIP).To(Equal(corev1.ClusterIPNone),
9777+
"Service should be headless (ClusterIP: None) when serviceClusterIPNone is enabled")
9778+
9779+
// Update deployment status to trigger status reconciliation
9780+
updatedDeployment := actualDeployment.DeepCopy()
9781+
updatedDeployment.Status.Conditions = []appsv1.DeploymentCondition{
9782+
{
9783+
Type: appsv1.DeploymentAvailable,
9784+
Status: corev1.ConditionTrue,
9785+
},
9786+
}
9787+
Expect(k8sClient.Status().Update(ctx, updatedDeployment)).NotTo(HaveOccurred())
9788+
9789+
// Verify status.address.url includes port 8080
9790+
expectedIsvcStatus := getExpectedIsvcStatusWithPort(serviceKey, constants.InferenceServiceDefaultHttpPort)
9791+
Eventually(func() string {
9792+
isvc := &v1beta1.InferenceService{}
9793+
if err := k8sClient.Get(ctx, serviceKey, isvc); err != nil {
9794+
return err.Error()
9795+
}
9796+
return cmp.Diff(expectedIsvcStatus, isvc.Status, cmpopts.IgnoreTypes(apis.VolatileTime{}))
9797+
}, timeout, interval).Should(BeEmpty())
9798+
9799+
// Double-check the address URL contains :8080
9800+
Eventually(func() bool {
9801+
isvc := &v1beta1.InferenceService{}
9802+
if err := k8sClient.Get(ctx, serviceKey, isvc); err != nil {
9803+
return false
9804+
}
9805+
if isvc.Status.Address == nil || isvc.Status.Address.URL == nil {
9806+
return false
9807+
}
9808+
expectedHost := serviceKey.Name + "-predictor." + serviceKey.Namespace + ".svc.cluster.local:8080"
9809+
return isvc.Status.Address.URL.Host == expectedHost
9810+
}, timeout, interval).Should(BeTrue(),
9811+
"status.address.url should include :8080 for headless service")
9812+
})
9813+
})
96479814
})
96489815

96499816
func verifyEnvKeyValueDeployments(actualDefaultDeployment *appsv1.Deployment, envKey string, expectedEnvValue string) {

‎pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/httproute_reconciler.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"k8s.io/apimachinery/pkg/types"
3030
"k8s.io/utils/ptr"
3131
knapis "knative.dev/pkg/apis"
32-
duckv1 "knative.dev/pkg/apis/duck/v1"
3332
"knative.dev/pkg/network"
3433
ctrl "sigs.k8s.io/controller-runtime"
3534
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -908,12 +907,9 @@ func (r *RawHTTPRouteReconciler) Reconcile(ctx context.Context, isvc *v1beta1.In
908907
if isvc.Status.URL, err = createRawURL(isvc, r.ingressConfig); err != nil {
909908
return ctrl.Result{}, err
910909
}
911-
isvc.Status.Address = &duckv1.Addressable{
912-
URL: &knapis.URL{
913-
Host: getRawServiceHost(isvc),
914-
Scheme: r.ingressConfig.UrlScheme,
915-
Path: "",
916-
},
910+
isvc.Status.Address, err = createAddress(ctx, r.client, isvc, r.ingressConfig)
911+
if err != nil {
912+
return ctrl.Result{}, err
917913
}
918914
return ctrl.Result{}, nil
919915
}

‎pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/httproute_reconciler_test.go‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,6 +2416,19 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
24162416
}
24172417
}
24182418

2419+
// Helper to create a predictor service (needed by createAddress)
2420+
predictorService := func(isvcName, namespace string) *corev1.Service {
2421+
return &corev1.Service{
2422+
ObjectMeta: metav1.ObjectMeta{
2423+
Name: constants.PredictorServiceName(isvcName),
2424+
Namespace: namespace,
2425+
},
2426+
Spec: corev1.ServiceSpec{
2427+
ClusterIP: "10.0.0.1",
2428+
},
2429+
}
2430+
}
2431+
24192432
t.Run("Reconcile disables ingress creation for cluster-local label", func(t *testing.T) {
24202433
isvc := &v1beta1.InferenceService{
24212434
ObjectMeta: metav1.ObjectMeta{
@@ -2430,7 +2443,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
24302443
},
24312444
Status: v1beta1.InferenceServiceStatus{},
24322445
}
2433-
client := fake.NewClientBuilder().WithScheme(s).Build()
2446+
client := fake.NewClientBuilder().WithScheme(s).WithObjects(predictorService("test-isvc", "default")).Build()
24342447
reconciler := NewRawHTTPRouteReconciler(client, s, ingressConfig, isvcConfig)
24352448
result, err := reconciler.Reconcile(t.Context(), isvc)
24362449
g.Expect(err).ToNot(HaveOccurred())
@@ -2453,7 +2466,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
24532466
}
24542467
clusterLocalConfig := *ingressConfig
24552468
clusterLocalConfig.IngressDomain = constants.ClusterLocalDomain
2456-
client := fake.NewClientBuilder().WithScheme(s).Build()
2469+
client := fake.NewClientBuilder().WithScheme(s).WithObjects(predictorService("test-isvc", "default")).Build()
24572470
reconciler := NewRawHTTPRouteReconciler(client, s, &clusterLocalConfig, isvcConfig)
24582471
result, err := reconciler.Reconcile(t.Context(), isvc)
24592472
g.Expect(err).ToNot(HaveOccurred())
@@ -2478,7 +2491,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
24782491
Type: v1beta1.PredictorReady,
24792492
Status: corev1.ConditionTrue,
24802493
})
2481-
client := fake.NewClientBuilder().WithScheme(s).Build()
2494+
client := fake.NewClientBuilder().WithScheme(s).WithObjects(predictorService("test-isvc", "default")).Build()
24822495
reconciler := NewRawHTTPRouteReconciler(client, s, ingressConfig, isvcConfig)
24832496
result, err := reconciler.Reconcile(t.Context(), isvc)
24842497
g.Expect(err).ToNot(HaveOccurred())
@@ -2633,7 +2646,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
26332646
})
26342647

26352648
// Create client without HTTPRoute objects - HTTPRoutes get created during reconciliation but have empty status
2636-
client := fake.NewClientBuilder().WithScheme(s).Build()
2649+
client := fake.NewClientBuilder().WithScheme(s).WithObjects(predictorService("test-isvc-requeue", "default")).Build()
26372650
reconciler := NewRawHTTPRouteReconciler(client, s, ingressConfig, isvcConfig)
26382651
result, err := reconciler.Reconcile(t.Context(), isvc)
26392652
g.Expect(err).ToNot(HaveOccurred())
@@ -2693,6 +2706,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
26932706
WithObjects(
26942707
notReadyHTTPRoute,
26952708
readyHTTPRoute(isvc.Name, isvc.Namespace), // Top-level route is ready
2709+
predictorService("test-isvc-not-ready", "default"),
26962710
).
26972711
Build()
26982712

@@ -2820,12 +2834,22 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
28202834
},
28212835
}
28222836

2837+
transformerService := &corev1.Service{
2838+
ObjectMeta: metav1.ObjectMeta{
2839+
Name: constants.TransformerServiceName("test-isvc-transformer"),
2840+
Namespace: "default",
2841+
},
2842+
Spec: corev1.ServiceSpec{
2843+
ClusterIP: "10.0.0.2",
2844+
},
2845+
}
28232846
client := fake.NewClientBuilder().
28242847
WithScheme(s).
28252848
WithObjects(
28262849
readyPredictorHTTPRoute,
28272850
notReadyTransformerHTTPRoute,
28282851
readyTopLevelHTTPRoute,
2852+
transformerService,
28292853
).
28302854
Build()
28312855

@@ -2947,6 +2971,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
29472971
readyPredictorHTTPRoute,
29482972
notReadyExplainerHTTPRoute,
29492973
readyTopLevelHTTPRoute,
2974+
predictorService("test-isvc-explainer", "default"),
29502975
).
29512976
Build()
29522977

@@ -3039,6 +3064,7 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
30393064
WithObjects(
30403065
readyPredictorHTTPRoute,
30413066
notReadyTopLevelHTTPRoute,
3067+
predictorService("test-isvc-toplevel", "default"),
30423068
).
30433069
Build()
30443070

@@ -3183,13 +3209,23 @@ func TestRawHTTPRouteReconciler_Reconcile(t *testing.T) {
31833209
},
31843210
}
31853211

3212+
transformerService := &corev1.Service{
3213+
ObjectMeta: metav1.ObjectMeta{
3214+
Name: constants.TransformerServiceName("test-isvc-all-ready"),
3215+
Namespace: "default",
3216+
},
3217+
Spec: corev1.ServiceSpec{
3218+
ClusterIP: "10.0.0.2",
3219+
},
3220+
}
31863221
client := fake.NewClientBuilder().
31873222
WithScheme(s).
31883223
WithObjects(
31893224
readyPredictorHTTPRoute,
31903225
readyTransformerHTTPRoute,
31913226
readyExplainerHTTPRoute,
31923227
readyTopLevelHTTPRoute,
3228+
transformerService,
31933229
).
31943230
Build()
31953231

‎pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,9 @@ func (r *RawIngressReconciler) Reconcile(ctx context.Context, isvc *v1beta1.Infe
131131
if err != nil {
132132
return ctrl.Result{}, err
133133
}
134-
isvc.Status.Address = &duckv1.Addressable{
135-
URL: &apis.URL{
136-
Host: getRawServiceHost(isvc),
137-
Scheme: r.ingressConfig.UrlScheme,
138-
Path: "",
139-
},
134+
isvc.Status.Address, err = createAddress(ctx, r.client, isvc, r.ingressConfig)
135+
if err != nil {
136+
return ctrl.Result{}, err
140137
}
141138
isvc.Status.SetCondition(v1beta1.IngressReady, &apis.Condition{
142139
Type: v1beta1.IngressReady,
@@ -146,6 +143,34 @@ func (r *RawIngressReconciler) Reconcile(ctx context.Context, isvc *v1beta1.Infe
146143
return ctrl.Result{}, nil
147144
}
148145

146+
func createAddress(ctx context.Context, cl client.Client, isvc *v1beta1.InferenceService, ingressConfig *v1beta1.IngressConfig) (*duckv1.Addressable, error) {
147+
host := getRawServiceHost(isvc)
148+
// Determine the entry point service name.
149+
// If a transformer exists, it becomes the entry point; otherwise, the predictor is.
150+
entryPointSvcName := constants.PredictorServiceName(isvc.Name)
151+
if isvc.Spec.Transformer != nil {
152+
entryPointSvcName = constants.TransformerServiceName(isvc.Name)
153+
}
154+
// Check if the entry point service is headless
155+
entryPointSvc := &corev1.Service{}
156+
if err := cl.Get(ctx, types.NamespacedName{
157+
Namespace: isvc.Namespace,
158+
Name: entryPointSvcName,
159+
}, entryPointSvc); err != nil {
160+
return nil, fmt.Errorf("failed to get entry point service %s: %w", entryPointSvcName, err)
161+
}
162+
if entryPointSvc.Spec.ClusterIP == corev1.ClusterIPNone {
163+
host = host + ":" + constants.InferenceServiceDefaultHttpPort
164+
}
165+
return &duckv1.Addressable{
166+
URL: &apis.URL{
167+
Host: host,
168+
Scheme: ingressConfig.UrlScheme,
169+
Path: "",
170+
},
171+
}, nil
172+
}
173+
149174
func generateRule(ingressHost string, componentName string, path string, port int32) netv1.IngressRule { //nolint:unparam
150175
pathType := netv1.PathTypePrefix
151176
rule := netv1.IngressRule{

0 commit comments

Comments
 (0)