From 8c95dac94f6442d5d092f2b4674336a6668d8862 Mon Sep 17 00:00:00 2001 From: Vedant Mahabaleshwarkar Date: Wed, 8 Jan 2025 15:06:43 +0530 Subject: [PATCH 1/4] [RHOAIENG-16851] fix scheme bugs in status.url and status.address.url for rawdeployment Signed-off-by: Vedant Mahabaleshwarkar --- .../inferenceservice/rawkube_controller_test.go | 2 +- .../reconcilers/ingress/kube_ingress_reconciler.go | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go b/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go index 5a91887d4c1..0cb768a7bec 100644 --- a/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go +++ b/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go @@ -2652,7 +2652,7 @@ var _ = Describe("v1beta1 inference service controller", func() { }, Address: &duckv1.Addressable{ URL: &apis.URL{ - Scheme: "http", + Scheme: "https", Host: fmt.Sprintf("%s-predictor.%s.svc.cluster.local:8443", serviceKey.Name, serviceKey.Namespace), }, }, diff --git a/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go b/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go index 5bf7da76d53..6bee574f821 100644 --- a/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go +++ b/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go @@ -80,9 +80,15 @@ func createRawURL(client client.Client, isvc *v1beta1.InferenceService, authEnab return nil, err } } else { + var scheme string + if authEnabled { + scheme = "https" + } else { + scheme = "http" + } url = &apis.URL{ Host: getRawServiceHost(isvc, client), - Scheme: "http", + Scheme: scheme, Path: "", } if authEnabled { @@ -337,13 +343,17 @@ func (r *RawIngressReconciler) Reconcile(isvc *v1beta1.InferenceService) error { return err } internalHost := getRawServiceHost(isvc, r.client) + var scheme string if authEnabled { internalHost += ":" + strconv.Itoa(constants.OauthProxyPort) + scheme = "https" + } else { + scheme = "http" } isvc.Status.Address = &duckv1.Addressable{ URL: &apis.URL{ Host: internalHost, - Scheme: r.ingressConfig.UrlScheme, + Scheme: scheme, Path: "", }, } From ef6b4d6503f266a5ba7b03b6b0d458f88054c552 Mon Sep 17 00:00:00 2001 From: Vedant Mahabaleshwarkar Date: Wed, 8 Jan 2025 15:23:12 +0530 Subject: [PATCH 2/4] [RHOAIENG-16851] Remove component url temporarily Signed-off-by: Vedant Mahabaleshwarkar --- .../v1beta1/inference_service_status.go | 9 ++-- .../rawkube_controller_test.go | 48 +++++++++---------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/pkg/apis/serving/v1beta1/inference_service_status.go b/pkg/apis/serving/v1beta1/inference_service_status.go index 0373c0e3d2f..469a8a42d34 100644 --- a/pkg/apis/serving/v1beta1/inference_service_status.go +++ b/pkg/apis/serving/v1beta1/inference_service_status.go @@ -336,9 +336,12 @@ func (ss *InferenceServiceStatus) PropagateRawStatus( } condition := getDeploymentCondition(deploymentList, appsv1.DeploymentAvailable) - if condition != nil && condition.Status == v1.ConditionTrue { - statusSpec.URL = url - } + // currently the component url is disabled as this url generated based on ingressConfig. This is incompatible with + // rawdeployment changes as the url depends on the route creation, if a route is requested. + // TODO: add back component url deterministicly + // if condition != nil && condition.Status == v1.ConditionTrue { + // statusSpec.URL = url + //} readyCondition := readyConditionsMap[component] ss.SetCondition(readyCondition, condition) ss.Components[component] = statusSpec diff --git a/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go b/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go index 0cb768a7bec..23303fc5a04 100644 --- a/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go +++ b/pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go @@ -425,10 +425,10 @@ var _ = Describe("v1beta1 inference service controller", func() { Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{ v1beta1.PredictorComponent: { LatestCreatedRevision: "", - URL: &apis.URL{ - Scheme: "http", - Host: "raw-foo-predictor-default.example.com", - }, + //URL: &apis.URL{ + // Scheme: "http", + // Host: "raw-foo-predictor-default.example.com", + //}, }, }, ModelStatus: v1beta1.ModelStatus{ @@ -840,10 +840,10 @@ var _ = Describe("v1beta1 inference service controller", func() { Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{ v1beta1.PredictorComponent: { LatestCreatedRevision: "", - URL: &apis.URL{ - Scheme: "http", - Host: "raw-foo-customized-predictor-default.example.com", - }, + //URL: &apis.URL{ + // Scheme: "http", + // Host: "raw-foo-customized-predictor-default.example.com", + //}, }, }, ModelStatus: v1beta1.ModelStatus{ @@ -1246,10 +1246,10 @@ var _ = Describe("v1beta1 inference service controller", func() { Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{ v1beta1.PredictorComponent: { LatestCreatedRevision: "", - URL: &apis.URL{ - Scheme: "http", - Host: "raw-foo-2-predictor-default.example.com", - }, + //URL: &apis.URL{ + // Scheme: "http", + // Host: "raw-foo-2-predictor-default.example.com", + //}, }, }, ModelStatus: v1beta1.ModelStatus{ @@ -1723,10 +1723,10 @@ var _ = Describe("v1beta1 inference service controller", func() { Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{ v1beta1.PredictorComponent: { LatestCreatedRevision: "", - URL: &apis.URL{ - Scheme: "http", - Host: fmt.Sprintf("%s-predictor-default.example.com", serviceName), - }, + //URL: &apis.URL{ + // Scheme: "http", + // Host: fmt.Sprintf("%s-predictor-default.example.com", serviceName), + //}, }, }, ModelStatus: v1beta1.ModelStatus{ @@ -2156,10 +2156,10 @@ var _ = Describe("v1beta1 inference service controller", func() { Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{ v1beta1.PredictorComponent: { LatestCreatedRevision: "", - URL: &apis.URL{ - Scheme: "http", - Host: fmt.Sprintf("%s-predictor.%s.%s", serviceName, serviceKey.Namespace, domain), - }, + //URL: &apis.URL{ + // Scheme: "http", + // Host: fmt.Sprintf("%s-predictor.%s.%s", serviceName, serviceKey.Namespace, domain), + //}, }, }, ModelStatus: v1beta1.ModelStatus{ @@ -2659,10 +2659,10 @@ var _ = Describe("v1beta1 inference service controller", func() { Components: map[v1beta1.ComponentType]v1beta1.ComponentStatusSpec{ v1beta1.PredictorComponent: { LatestCreatedRevision: "", - URL: &apis.URL{ - Scheme: "http", - Host: "raw-auth-predictor-default.example.com", - }, + //URL: &apis.URL{ + // Scheme: "http", + // Host: "raw-auth-predictor-default.example.com", + //}, }, }, ModelStatus: v1beta1.ModelStatus{ From be3f5f375bb7a745441e99ef3af34bb66b6e1ec1 Mon Sep 17 00:00:00 2001 From: Vedant Mahabaleshwarkar Date: Mon, 13 Jan 2025 16:40:48 -0500 Subject: [PATCH 3/4] [RHOAIENG-16851] Use transformer spec to set upstream port in oauth-proxy if a transformer-container is present Signed-off-by: Vedant Mahabaleshwarkar --- .../reconcilers/deployment/deployment_reconciler.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go b/pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go index ca0c1afb702..cc43124ca9a 100644 --- a/pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go +++ b/pkg/controller/v1beta1/inferenceservice/reconcilers/deployment/deployment_reconciler.go @@ -269,15 +269,24 @@ func createRawWorkerDeployment(componentMeta metav1.ObjectMeta, } func GetKServeContainerPort(podSpec *corev1.PodSpec) string { + var kserveContainerPort string + for _, container := range podSpec.Containers { - if container.Name == "kserve-container" { + if container.Name == "transformer-container" { if len(container.Ports) > 0 { return strconv.Itoa(int(container.Ports[0].ContainerPort)) } } + if container.Name == "kserve-container" { + if len(container.Ports) > 0 { + kserveContainerPort = strconv.Itoa(int(container.Ports[0].ContainerPort)) + } + } } - return "" + + return kserveContainerPort } + func generateOauthProxyContainer(clientset kubernetes.Interface, isvc string, namespace string, upstreamPort string, sa string) (*corev1.Container, error) { isvcConfigMap, err := clientset.CoreV1().ConfigMaps(constants.KServeNamespace).Get(context.TODO(), constants.InferenceServiceConfigMapName, metav1.GetOptions{}) if err != nil { From 3fc65e29c8f19c4ab8d6000df59a3ae7bae08328 Mon Sep 17 00:00:00 2001 From: Vedant Mahabaleshwarkar Date: Tue, 14 Jan 2025 14:49:08 -0500 Subject: [PATCH 4/4] [RHOAIENG-16851] address feedback Signed-off-by: Vedant Mahabaleshwarkar --- .../ingress/kube_ingress_reconciler.go | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go b/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go index 6bee574f821..28679560f00 100644 --- a/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go +++ b/pkg/controller/v1beta1/inferenceservice/reconcilers/ingress/kube_ingress_reconciler.go @@ -80,19 +80,14 @@ func createRawURL(client client.Client, isvc *v1beta1.InferenceService, authEnab return nil, err } } else { - var scheme string - if authEnabled { - scheme = "https" - } else { - scheme = "http" - } url = &apis.URL{ Host: getRawServiceHost(isvc, client), - Scheme: scheme, + Scheme: "http", Path: "", } if authEnabled { url.Host += ":" + strconv.Itoa(constants.OauthProxyPort) + url.Scheme = "https" } } return url, nil @@ -343,19 +338,18 @@ func (r *RawIngressReconciler) Reconcile(isvc *v1beta1.InferenceService) error { return err } internalHost := getRawServiceHost(isvc, r.client) - var scheme string + url := &apis.URL{ + Host: internalHost, + Scheme: "http", + Path: "", + } if authEnabled { internalHost += ":" + strconv.Itoa(constants.OauthProxyPort) - scheme = "https" - } else { - scheme = "http" + url.Host = internalHost + url.Scheme = "https" } isvc.Status.Address = &duckv1.Addressable{ - URL: &apis.URL{ - Host: internalHost, - Scheme: scheme, - Path: "", - }, + URL: url, } isvc.Status.SetCondition(v1beta1.IngressReady, &apis.Condition{ Type: v1beta1.IngressReady,