diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a282400787f..db02f051ebc 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -117,6 +117,7 @@ rules: - routes verbs: - create + - delete - get - list - patch diff --git a/pkg/controller/v1alpha1/inferencegraph/controller.go b/pkg/controller/v1alpha1/inferencegraph/controller.go index 1aa32199fa1..2a3e80ec145 100644 --- a/pkg/controller/v1alpha1/inferencegraph/controller.go +++ b/pkg/controller/v1alpha1/inferencegraph/controller.go @@ -19,7 +19,7 @@ limitations under the License. // +kubebuilder:rbac:groups=serving.knative.dev,resources=services,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=serving.knative.dev,resources=services/finalizers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=serving.knative.dev,resources=services/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=create;get;update;patch;watch +// +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=create;get;update;patch;watch;delete // +kubebuilder:rbac:groups=route.openshift.io,resources=routes/status,verbs=get package inferencegraph diff --git a/pkg/controller/v1alpha1/inferencegraph/controller_test.go b/pkg/controller/v1alpha1/inferencegraph/controller_test.go index a576964925f..7f0cdc3e7ae 100644 --- a/pkg/controller/v1alpha1/inferencegraph/controller_test.go +++ b/pkg/controller/v1alpha1/inferencegraph/controller_test.go @@ -27,6 +27,7 @@ import ( "google.golang.org/protobuf/proto" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -696,6 +697,149 @@ var _ = Describe("Inference Graph controller test", func() { return inferenceGraphSubmitted.Status.URL.Host }, timeout, interval).Should(Equal(osRoute.Status.Ingress[0].Host)) }) + + It("Should not create ingress when cluster-local visibility is configured", func() { + By("By creating a new InferenceGraph") + var configMap = &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.InferenceServiceConfigMapName, + Namespace: constants.KServeNamespace, + }, + Data: configs, + } + Expect(k8sClient.Create(context.TODO(), configMap)).NotTo(HaveOccurred()) + defer func() { _ = k8sClient.Delete(context.TODO(), configMap) }() + graphName := "igraw-private" + var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: graphName, Namespace: "default"}} + var serviceKey = expectedRequest.NamespacedName + ctx := context.Background() + ig := &v1alpha1.InferenceGraph{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceKey.Name, + Namespace: serviceKey.Namespace, + Annotations: map[string]string{ + "serving.kserve.io/deploymentMode": string(constants.RawDeployment), + }, + Labels: map[string]string{ + constants.NetworkVisibility: constants.ClusterLocalVisibility, + }, + }, + Spec: v1alpha1.InferenceGraphSpec{ + Nodes: map[string]v1alpha1.InferenceRouter{ + v1alpha1.GraphRootNodeName: { + RouterType: v1alpha1.Sequence, + Steps: []v1alpha1.InferenceStep{ + { + InferenceTarget: v1alpha1.InferenceTarget{ + ServiceURL: "http://someservice.exmaple.com", + }, + }, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, ig)).Should(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, ig) }() + + // The OpenShift route must not be created + actualK8sDeploymentCreated := &appsv1.Deployment{} + Eventually(func() error { + return k8sClient.Get(ctx, serviceKey, actualK8sDeploymentCreated) + }, timeout, interval).Should(Succeed()) + actualK8sDeploymentCreated.Status.Conditions = []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentAvailable}, + } + Expect(k8sClient.Status().Update(ctx, actualK8sDeploymentCreated)).Should(Succeed()) + osRoute := osv1.Route{} + Consistently(func() error { + osRouteKey := types.NamespacedName{Name: ig.GetName() + "-route", Namespace: ig.GetNamespace()} + return k8sClient.Get(ctx, osRouteKey, &osRoute) + }, timeout, interval).Should(WithTransform(errors.IsNotFound, BeTrue())) + + // The InferenceGraph should have a cluster-internal hostname + Eventually(func() string { + _ = k8sClient.Get(ctx, serviceKey, ig) + return ig.Status.URL.Host + }, timeout, interval).Should(Equal(fmt.Sprintf("%s.%s.svc.cluster.local", graphName, "default"))) + }) + + It("Should reconfigure InferenceGraph as private when cluster-local visibility is configured", func() { + By("By creating a new InferenceGraph") + var configMap = &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.InferenceServiceConfigMapName, + Namespace: constants.KServeNamespace, + }, + Data: configs, + } + Expect(k8sClient.Create(context.TODO(), configMap)).NotTo(HaveOccurred()) + defer func() { _ = k8sClient.Delete(context.TODO(), configMap) }() + graphName := "igraw-exposed-to-private" + var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: graphName, Namespace: "default"}} + var serviceKey = expectedRequest.NamespacedName + ctx := context.Background() + ig := &v1alpha1.InferenceGraph{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceKey.Name, + Namespace: serviceKey.Namespace, + Annotations: map[string]string{ + "serving.kserve.io/deploymentMode": string(constants.RawDeployment), + }, + }, + Spec: v1alpha1.InferenceGraphSpec{ + Nodes: map[string]v1alpha1.InferenceRouter{ + v1alpha1.GraphRootNodeName: { + RouterType: v1alpha1.Sequence, + Steps: []v1alpha1.InferenceStep{ + { + InferenceTarget: v1alpha1.InferenceTarget{ + ServiceURL: "http://someservice.exmaple.com", + }, + }, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, ig)).Should(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, ig) }() + + // Wait the OpenShift route to be created + actualK8sDeploymentCreated := &appsv1.Deployment{} + Eventually(func() error { + return k8sClient.Get(ctx, serviceKey, actualK8sDeploymentCreated) + }, timeout, interval).Should(Succeed()) + actualK8sDeploymentCreated.Status.Conditions = []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentAvailable}, + } + Expect(k8sClient.Status().Update(ctx, actualK8sDeploymentCreated)).Should(Succeed()) + osRoute := osv1.Route{} + Eventually(func() error { + osRouteKey := types.NamespacedName{Name: ig.GetName() + "-route", Namespace: ig.GetNamespace()} + return k8sClient.Get(ctx, osRouteKey, &osRoute) + }, timeout, interval).Should(Succeed()) + + // Reconfigure as private + Expect(k8sClient.Get(ctx, serviceKey, ig)).Should(Succeed()) + if ig.Labels == nil { + ig.Labels = map[string]string{} + } + ig.Labels[constants.NetworkVisibility] = constants.ClusterLocalVisibility + Expect(k8sClient.Update(ctx, ig)).Should(Succeed()) + + // The OpenShift route should be deleted + Eventually(func() error { + osRouteKey := types.NamespacedName{Name: ig.GetName() + "-route", Namespace: ig.GetNamespace()} + return k8sClient.Get(ctx, osRouteKey, &osRoute) + }).Should(WithTransform(errors.IsNotFound, BeTrue())) + + // The InferenceGraph should have a cluster-internal hostname + Eventually(func() string { + _ = k8sClient.Get(ctx, serviceKey, ig) + return ig.Status.URL.Host + }, timeout, interval).Should(Equal(fmt.Sprintf("%s.%s.svc.cluster.local", graphName, "default"))) + }) }) Context("When creating an InferenceGraph in Serverless mode", func() { diff --git a/pkg/controller/v1alpha1/inferencegraph/openshift_route_reconciler.go b/pkg/controller/v1alpha1/inferencegraph/openshift_route_reconciler.go index a970ac99996..2f8f23f9dd8 100644 --- a/pkg/controller/v1alpha1/inferencegraph/openshift_route_reconciler.go +++ b/pkg/controller/v1alpha1/inferencegraph/openshift_route_reconciler.go @@ -1,3 +1,19 @@ +/* +Copyright 2023 The KServe 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. +*/ + package inferencegraph import ( @@ -10,11 +26,13 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "knative.dev/pkg/network" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ctrlLog "sigs.k8s.io/controller-runtime/pkg/log" "github.com/kserve/kserve/pkg/apis/serving/v1alpha1" + "github.com/kserve/kserve/pkg/constants" ) type OpenShiftRouteReconciler struct { @@ -41,6 +59,21 @@ func (r *OpenShiftRouteReconciler) Reconcile(ctx context.Context, inferenceGraph return "", err } + if val, ok := inferenceGraph.Labels[constants.NetworkVisibility]; ok && val == constants.ClusterLocalVisibility { + privateHost := network.GetServiceHostname(inferenceGraph.GetName(), inferenceGraph.GetNamespace()) + // The IG is private. Remove the route, if needed. + if len(actualRoute.Name) != 0 { + logger.Info("Deleting OpenShift Route for InferenceGraph", "namespace", desiredRoute.Namespace, "name", desiredRoute.Name) + err = r.Client.Delete(ctx, &actualRoute) + if err != nil { + return privateHost, err + } + } + + // Return private hostname. + return privateHost, nil + } + if len(actualRoute.Name) == 0 { logger.Info("Creating a new OpenShift Route for InferenceGraph", "namespace", desiredRoute.Namespace, "name", desiredRoute.Name) err = r.Client.Create(ctx, &desiredRoute) diff --git a/pkg/webhook/validation/inferenceservice/inferenceservice_webhook_test.go b/pkg/webhook/validation/inferenceservice/inferenceservice_webhook_test.go index 975beef71ef..1cb9fdd7cc0 100644 --- a/pkg/webhook/validation/inferenceservice/inferenceservice_webhook_test.go +++ b/pkg/webhook/validation/inferenceservice/inferenceservice_webhook_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2023 The KServe 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. +*/ + package inferenceservice import (