diff --git a/config/crd/external/gateways.networking.istio.io.yaml b/config/crd/external/gateways.networking.istio.io.yaml new file mode 100644 index 00000000..0213bc9f --- /dev/null +++ b/config/crd/external/gateways.networking.istio.io.yaml @@ -0,0 +1,258 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + labels: + app: istio-pilot + chart: istio + heritage: Tiller + maistra-version: 2.5.2 + release: istio + name: gateways.networking.istio.io +spec: + conversion: + strategy: None + group: networking.istio.io + names: + categories: + - istio-io + - networking-istio-io + kind: Gateway + listKind: GatewayList + plural: gateways + shortNames: + - gw + singular: gateway + scope: Namespaced + versions: + - name: v1alpha3 + schema: + openAPIV3Schema: + properties: + spec: + description: 'Configuration affecting edge load balancer. See more details + at: https://istio.io/docs/reference/config/networking/gateway.html' + properties: + selector: + additionalProperties: + type: string + type: object + servers: + description: A list of server specifications. + items: + properties: + bind: + type: string + defaultEndpoint: + type: string + hosts: + description: One or more hosts exposed by this gateway. + items: + type: string + type: array + name: + description: An optional name of the server, when set must be + unique across all servers. + type: string + port: + properties: + name: + description: Label assigned to the port. + type: string + number: + description: A valid non-negative integer port number. + type: integer + protocol: + description: The protocol exposed on the port. + type: string + targetPort: + type: integer + type: object + tls: + description: Set of TLS related options that govern the server's + behavior. + properties: + caCertificates: + description: REQUIRED if mode is `MUTUAL`. + type: string + cipherSuites: + description: 'Optional: If specified, only support the specified + cipher list.' + items: + type: string + type: array + credentialName: + type: string + httpsRedirect: + type: boolean + maxProtocolVersion: + description: 'Optional: Maximum TLS protocol version.' + enum: + - TLS_AUTO + - TLSV1_0 + - TLSV1_1 + - TLSV1_2 + - TLSV1_3 + type: string + minProtocolVersion: + description: 'Optional: Minimum TLS protocol version.' + enum: + - TLS_AUTO + - TLSV1_0 + - TLSV1_1 + - TLSV1_2 + - TLSV1_3 + type: string + mode: + enum: + - PASSTHROUGH + - SIMPLE + - MUTUAL + - AUTO_PASSTHROUGH + - ISTIO_MUTUAL + type: string + privateKey: + description: REQUIRED if mode is `SIMPLE` or `MUTUAL`. + type: string + serverCertificate: + description: REQUIRED if mode is `SIMPLE` or `MUTUAL`. + type: string + subjectAltNames: + items: + type: string + type: array + verifyCertificateHash: + items: + type: string + type: array + verifyCertificateSpki: + items: + type: string + type: array + type: object + type: object + type: array + type: object + status: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + served: true + storage: true + subresources: + status: {} + - name: v1beta1 + schema: + openAPIV3Schema: + properties: + spec: + description: 'Configuration affecting edge load balancer. See more details + at: https://istio.io/docs/reference/config/networking/gateway.html' + properties: + selector: + additionalProperties: + type: string + type: object + servers: + description: A list of server specifications. + items: + properties: + bind: + type: string + defaultEndpoint: + type: string + hosts: + description: One or more hosts exposed by this gateway. + items: + type: string + type: array + name: + description: An optional name of the server, when set must be + unique across all servers. + type: string + port: + properties: + name: + description: Label assigned to the port. + type: string + number: + description: A valid non-negative integer port number. + type: integer + protocol: + description: The protocol exposed on the port. + type: string + targetPort: + type: integer + type: object + tls: + description: Set of TLS related options that govern the server's + behavior. + properties: + caCertificates: + description: REQUIRED if mode is `MUTUAL`. + type: string + cipherSuites: + description: 'Optional: If specified, only support the specified + cipher list.' + items: + type: string + type: array + credentialName: + type: string + httpsRedirect: + type: boolean + maxProtocolVersion: + description: 'Optional: Maximum TLS protocol version.' + enum: + - TLS_AUTO + - TLSV1_0 + - TLSV1_1 + - TLSV1_2 + - TLSV1_3 + type: string + minProtocolVersion: + description: 'Optional: Minimum TLS protocol version.' + enum: + - TLS_AUTO + - TLSV1_0 + - TLSV1_1 + - TLSV1_2 + - TLSV1_3 + type: string + mode: + enum: + - PASSTHROUGH + - SIMPLE + - MUTUAL + - AUTO_PASSTHROUGH + - ISTIO_MUTUAL + type: string + privateKey: + description: REQUIRED if mode is `SIMPLE` or `MUTUAL`. + type: string + serverCertificate: + description: REQUIRED if mode is `SIMPLE` or `MUTUAL`. + type: string + subjectAltNames: + items: + type: string + type: array + verifyCertificateHash: + items: + type: string + type: array + verifyCertificateSpki: + items: + type: string + type: array + type: object + type: object + type: array + type: object + status: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + served: true + storage: false + subresources: + status: {} diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 8ee25655..f033e3ee 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,10 +1,11 @@ resources: - - manager.yaml +- manager.yaml generatorOptions: disableNameSuffixHash: true configMapGenerator: - - files: - - controller_manager_config.yaml - name: manager-config +- files: + - controller_manager_config.yaml + name: manager-config + diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 25076276..67414f16 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -122,6 +122,16 @@ rules: - patch - update - watch +- apiGroups: + - networking.istio.io + resources: + - gateways + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - networking.istio.io resources: diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml index ed4d980b..f8a9664b 100644 --- a/config/webhook/kustomization.yaml +++ b/config/webhook/kustomization.yaml @@ -5,7 +5,6 @@ resources: - manifests.yaml - service.yaml - patches: - path: webhook_patch.yaml target: @@ -19,4 +18,3 @@ patches: kind: ValidatingWebhookConfiguration name: validating-webhook-configuration version: v1 - diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml index 1837a784..e545697f 100644 --- a/config/webhook/webhook_patch.yaml +++ b/config/webhook/webhook_patch.yaml @@ -13,4 +13,3 @@ webhooks: matchExpressions: - key: serving.kserve.io/inferenceservice operator: Exists - diff --git a/controllers/comparators/gateway_comparator.go b/controllers/comparators/gateway_comparator.go new file mode 100644 index 00000000..2548b474 --- /dev/null +++ b/controllers/comparators/gateway_comparator.go @@ -0,0 +1,44 @@ +/* + +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 comparators + +import ( + istiov1beta1 "istio.io/api/networking/v1beta1" + istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func GetGatewayComparator() ResourceComparator { + return func(existing client.Object, desired client.Object) bool { + existingGateway := existing.(*istioclientv1beta1.Gateway) + desiredGateway := desired.(*istioclientv1beta1.Gateway) + + exists := false + for _, server := range existingGateway.Spec.Servers { + if serversEqual(server, desiredGateway.Spec.Servers[0]) { + exists = true + break + } + } + + return exists + } +} + +// serversEquals compare if the inferenceservice name matches got the given resources +func serversEqual(s1, s2 *istiov1beta1.Server) bool { + return s1.Port.Name == s2.Port.Name +} diff --git a/controllers/constants/constants.go b/controllers/constants/constants.go index 51c861ba..1d7e1356 100644 --- a/controllers/constants/constants.go +++ b/controllers/constants/constants.go @@ -49,6 +49,7 @@ const ( KServeCACertConfigMapName = "odh-kserve-custom-ca-bundle" ODHGlobalCertConfigMapName = "odh-trusted-ca-bundle" ODHCustomCACertFileName = "odh-ca-bundle.crt" + KServeGatewayName = "kserve-local-gateway" ) const ( @@ -59,3 +60,8 @@ const ( VllmImageName = "vllm" CaikitImageName = "caikit-nlp" ) + +// openshift +const ( + ServingCertAnnotationKey = "service.beta.openshift.io/serving-cert-secret-name" +) diff --git a/controllers/inferenceservice_controller.go b/controllers/inferenceservice_controller.go index 1b370bc2..58fc5fbe 100644 --- a/controllers/inferenceservice_controller.go +++ b/controllers/inferenceservice_controller.go @@ -32,6 +32,8 @@ import ( apierrs "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + + // "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -40,6 +42,7 @@ import ( // OpenshiftInferenceServiceReconciler holds the controller configuration. type OpenshiftInferenceServiceReconciler struct { client client.Client + clientReader client.Reader log logr.Logger MeshDisabled bool mmISVCReconciler *reconcilers.ModelMeshInferenceServiceReconciler @@ -47,13 +50,14 @@ type OpenshiftInferenceServiceReconciler struct { kserveRawISVCReconciler *reconcilers.KserveRawInferenceServiceReconciler } -func NewOpenshiftInferenceServiceReconciler(client client.Client, log logr.Logger, meshDisabled bool) *OpenshiftInferenceServiceReconciler { +func NewOpenshiftInferenceServiceReconciler(client client.Client, clientReader client.Reader, log logr.Logger, meshDisabled bool) *OpenshiftInferenceServiceReconciler { return &OpenshiftInferenceServiceReconciler{ client: client, + clientReader: clientReader, log: log, MeshDisabled: meshDisabled, mmISVCReconciler: reconcilers.NewModelMeshInferenceServiceReconciler(client), - kserveServerlessISVCReconciler: reconcilers.NewKServeServerlessInferenceServiceReconciler(client), + kserveServerlessISVCReconciler: reconcilers.NewKServeServerlessInferenceServiceReconciler(client, clientReader), kserveRawISVCReconciler: reconcilers.NewKServeRawInferenceServiceReconciler(client), } } @@ -149,6 +153,23 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) }) } return reconcileRequests + })). + Watches(&corev1.Secret{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + r.log.Info("Reconcile event triggered by Secret: " + o.GetName()) + isvc := &kservev1beta1.InferenceService{} + err := r.client.Get(ctx, types.NamespacedName{Name: o.GetName(), Namespace: o.GetNamespace()}, isvc) + if err != nil { + if apierrs.IsNotFound(err) { + return []reconcile.Request{} + } + r.log.Error(err, "Error getting the inferenceService", "name", o.GetName()) + return []reconcile.Request{} + } + + return []reconcile.Request{ + {NamespacedName: types.NamespacedName{Name: o.GetName(), Namespace: o.GetNamespace()}}, + } })) kserveWithMeshEnabled, kserveWithMeshEnabledErr := utils.VerifyIfComponentIsEnabled(context.Background(), mgr.GetClient(), utils.KServeWithServiceMeshComponent) diff --git a/controllers/kserve_inferenceservice_controller_test.go b/controllers/kserve_inferenceservice_controller_test.go index cdf3d825..a5008dd3 100644 --- a/controllers/kserve_inferenceservice_controller_test.go +++ b/controllers/kserve_inferenceservice_controller_test.go @@ -17,6 +17,7 @@ package controllers import ( "context" + "fmt" "strings" @@ -28,14 +29,22 @@ import ( gomegatypes "github.com/onsi/gomega/types" "github.com/opendatahub-io/odh-model-controller/controllers/constants" routev1 "github.com/openshift/api/route/v1" + istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) +const ( + testIsvcSvcPath = "./testdata/servingcert-service/test-isvc-svc.yaml" + kserveLocalGatewayPath = "./testdata/gateway/kserve-local-gateway.yaml" + testIsvcSvcSecretPath = "./testdata/gateway/test-isvc-svc-secret.yaml" +) + var _ = Describe("The Openshift Kserve model controller", func() { When("creating a Kserve ServiceRuntime & InferenceService", func() { @@ -94,6 +103,112 @@ var _ = Describe("The Openshift Kserve model controller", func() { return err }, timeout, interval).Should(Succeed()) }) + It("With a new Kserve InferenceService, serving cert annotation should be added to the runtime Service object.", func() { + // We need to stub the cluster state and indicate where is istio namespace (reusing authConfig test data) + if dsciErr := createDSCI(DSCIWithoutAuthorization); dsciErr != nil && !errors.IsAlreadyExists(dsciErr) { + Fail(dsciErr.Error()) + } + // Create a new InferenceService + inferenceService := &kservev1beta1.InferenceService{} + err := convertToStructuredResource(KserveInferenceServicePath1, inferenceService) + Expect(err).NotTo(HaveOccurred()) + inferenceService.SetNamespace(testNs) + Expect(cli.Create(ctx, inferenceService)).Should(Succeed()) + // Update the URL of the InferenceService to indicate it is ready. + deployedInferenceService := &kservev1beta1.InferenceService{} + err = cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: inferenceService.Namespace}, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + // url, err := apis.ParseURL("https://example-onnx-mnist-default.test.com") + Expect(err).NotTo(HaveOccurred()) + newAddress := &duckv1.Addressable{ + URL: apis.HTTPS("example-onnx-mnist-default.test.com"), + } + deployedInferenceService.Status.Address = newAddress + err = cli.Status().Update(ctx, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + // Stub: Create a Kserve Service, which must be created by the KServe operator. + svc := &corev1.Service{} + err = convertToStructuredResource(testIsvcSvcPath, svc) + Expect(err).NotTo(HaveOccurred()) + svc.SetNamespace(inferenceService.Namespace) + Expect(cli.Create(ctx, svc)).Should(Succeed()) + err = cli.Status().Update(ctx, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + // isvcService, err := waitForService(cli, testNs, inferenceService.Name, 5, 2*time.Second) + // Expect(err).NotTo(HaveOccurred()) + + isvcService := &corev1.Service{} + Eventually(func() error { + err := cli.Get(ctx, client.ObjectKey{Namespace: inferenceService.Namespace, Name: inferenceService.Name}, isvcService) + if err != nil { + return err + } + if isvcService.Annotations == nil || isvcService.Annotations[constants.ServingCertAnnotationKey] == "" { + + return fmt.Errorf("Annotation[constants.ServingCertAnnotationKey] is not added yet") + } + return nil + }, timeout, interval).Should(Succeed()) + + Expect(isvcService.Annotations[constants.ServingCertAnnotationKey]).Should(Equal(inferenceService.Name)) + }) + + It("should create a secret for runtime and update kserve local gateway in the istio-system namespace", func() { + // We need to stub the cluster state and indicate where is istio namespace (reusing authConfig test data) + if dsciErr := createDSCI(DSCIWithoutAuthorization); dsciErr != nil && !errors.IsAlreadyExists(dsciErr) { + Fail(dsciErr.Error()) + } + // Stub: Create a kserve-local-gateway, which must be created by the OpenDataHub operator. + kserveLocalGateway := &istioclientv1beta1.Gateway{} + err := convertToStructuredResource(kserveLocalGatewayPath, kserveLocalGateway) + Expect(err).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, kserveLocalGateway)).Should(Succeed()) + + // Stub: Create a certificate Secret, which must be created by the openshift service-ca operator. + secret := &corev1.Secret{} + err = convertToStructuredResource(testIsvcSvcSecretPath, secret) + Expect(err).NotTo(HaveOccurred()) + secret.SetNamespace(testNs) + Expect(cli.Create(ctx, secret)).Should(Succeed()) + + // Create a new InferenceService + inferenceService := &kservev1beta1.InferenceService{} + err = convertToStructuredResource(KserveInferenceServicePath1, inferenceService) + Expect(err).NotTo(HaveOccurred()) + inferenceService.SetNamespace(testNs) + + Expect(cli.Create(ctx, inferenceService)).Should(Succeed()) + + // Update the URL of the InferenceService to indicate it is ready. + deployedInferenceService := &kservev1beta1.InferenceService{} + err = cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: inferenceService.Namespace}, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + + newAddress := &duckv1.Addressable{ + URL: apis.HTTPS("example-onnx-mnist-default.test.com"), + } + deployedInferenceService.Status.Address = newAddress + + err = cli.Status().Update(ctx, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + + // Verify that the certificate secret is created in the istio-system namespace. + Eventually(func() error { + secret := &corev1.Secret{} + return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret) + }, timeout, interval).Should(Succeed()) + + // Verify that the gateway is updated in the istio-system namespace. + var gateway *istioclientv1beta1.Gateway + Eventually(func() error { + gateway, err = waitForUpdatedGatewayCompletion(cli, "add", constants.IstioNamespace, constants.KServeGatewayName, inferenceService.Name) + return err + }, timeout, interval).Should(Succeed()) + + // Ensure that the server is successfully added to the KServe local gateway within the istio-system namespace. + targetServerExist := hasServerFromGateway(gateway, fmt.Sprintf("%s-%s", "https", inferenceService.Name)) + Expect(targetServerExist).Should(BeTrue()) + }) It("should create required network policies when KServe is used", func() { // given @@ -121,8 +236,156 @@ var _ = Describe("The Openshift Kserve model controller", func() { ), ) }) + }) + + Context("when there is a existing inferenceService", func() { + var testNs string + var isvcName string + + BeforeEach(func() { + ctx := context.Background() + testNamespace := Namespaces.Create(cli) + testNs = testNamespace.Name + + inferenceServiceConfig := &corev1.ConfigMap{} + Expect(convertToStructuredResource(InferenceServiceConfigPath1, inferenceServiceConfig)).To(Succeed()) + if err := cli.Create(ctx, inferenceServiceConfig); err != nil && !errors.IsAlreadyExists(err) { + Fail(err.Error()) + } + + // We need to stub the cluster state and indicate where is istio namespace (reusing authConfig test data) + if dsciErr := createDSCI(DSCIWithoutAuthorization); dsciErr != nil && !errors.IsAlreadyExists(dsciErr) { + Fail(dsciErr.Error()) + } + + servingRuntime := &kservev1alpha1.ServingRuntime{} + Expect(convertToStructuredResource(KserveServingRuntimePath1, servingRuntime)).To(Succeed()) + if err := cli.Create(ctx, servingRuntime); err != nil && !errors.IsAlreadyExists(err) { + Fail(err.Error()) + } + + // Stub: Create a kserve-local-gateway, which must be created by the OpenDataHub operator. + kserveLocalGateway := &istioclientv1beta1.Gateway{} + err := convertToStructuredResource(kserveLocalGatewayPath, kserveLocalGateway) + Expect(err).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, kserveLocalGateway)).Should(Succeed()) + + // Stub: Create a certificate Secret, which must be created by the openshift service-ca operator. + secret := &corev1.Secret{} + err = convertToStructuredResource(testIsvcSvcSecretPath, secret) + Expect(err).NotTo(HaveOccurred()) + secret.SetNamespace(testNs) + Expect(cli.Create(ctx, secret)).Should(Succeed()) + + // Create a new InferenceService + inferenceService := &kservev1beta1.InferenceService{} + err = convertToStructuredResource(KserveInferenceServicePath1, inferenceService) + Expect(err).NotTo(HaveOccurred()) + inferenceService.SetNamespace(testNs) + // Ensure the Delete method is called when the InferenceService (ISVC) is deleted. + inferenceService.SetFinalizers([]string{"finalizer.inferenceservice"}) + + Expect(cli.Create(ctx, inferenceService)).Should(Succeed()) + isvcName = inferenceService.Name + + // Update the URL of the InferenceService to indicate it is ready. + deployedInferenceService := &kservev1beta1.InferenceService{} + err = cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: testNs}, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + + newAddress := &duckv1.Addressable{ + URL: apis.HTTPS("example-onnx-mnist-default.test.com"), + } + deployedInferenceService.Status.Address = newAddress + + err = cli.Status().Update(ctx, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + + // Verify that the certificate secret is created in the istio-system namespace. + Eventually(func() error { + secret := &corev1.Secret{} + return cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: inferenceService.Namespace}, secret) + }, timeout, interval).Should(Succeed()) + + Eventually(func() error { + return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret) + }, timeout, interval).Should(Succeed()) + + // Verify that the gateway is updated in the istio-system namespace. + var gateway *istioclientv1beta1.Gateway + Eventually(func() error { + gateway, err = waitForUpdatedGatewayCompletion(cli, "add", constants.IstioNamespace, constants.KServeGatewayName, inferenceService.Name) + return err + }, timeout, interval).Should(Succeed()) + + // Ensure that the server is successfully added to the KServe local gateway within the istio-system namespace. + targetServerExist := hasServerFromGateway(gateway, fmt.Sprintf("%s-%s", "https", inferenceService.Name)) + Expect(targetServerExist).Should(BeTrue()) + }) + + When("serving cert Secret is rotated", func() { + It("should re-sync serving cert Secret to istio-system", func() { + deployedInferenceService := &kservev1beta1.InferenceService{} + err := cli.Get(ctx, types.NamespacedName{Name: isvcName, Namespace: testNs}, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + + // Get source secret + srcSecret := &corev1.Secret{} + err = cli.Get(ctx, client.ObjectKey{Namespace: testNs, Name: deployedInferenceService.Name}, srcSecret) + Expect(err).NotTo(HaveOccurred()) + + // Update source secret + updatedDataString := "updateData" + srcSecret.Data["tls.crt"] = []byte(updatedDataString) + srcSecret.Data["tls.key"] = []byte(updatedDataString) + Expect(cli.Update(ctx, srcSecret)).Should(Succeed()) + + // Get destination secret + err = cli.Get(ctx, client.ObjectKey{Namespace: testNs, Name: deployedInferenceService.Name}, srcSecret) + Expect(err).NotTo(HaveOccurred()) + + // Verify that the certificate secret in the istio-system namespace is updated. + destSecret := &corev1.Secret{} + Eventually(func() error { + Expect(cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", deployedInferenceService.Name, deployedInferenceService.Namespace)}, destSecret)).Should(Succeed()) + if string(destSecret.Data["tls.crt"]) != updatedDataString { + return fmt.Errorf("destSecret is not updated yet") + } + return nil + }, timeout, interval).Should(Succeed()) + + Expect(destSecret.Data).To(Equal(srcSecret.Data)) + }) + }) + + When("infereceService is deleted", func() { + It("should remove the Server from the kserve local gateway in istio-system and delete the created Secret", func() { + // Delete the existing ISVC + deployedInferenceService := &kservev1beta1.InferenceService{} + err := cli.Get(ctx, types.NamespacedName{Name: isvcName, Namespace: testNs}, deployedInferenceService) + Expect(err).NotTo(HaveOccurred()) + Expect(cli.Delete(ctx, deployedInferenceService)).Should(Succeed()) + + // Verify that the gateway is updated in the istio-system namespace. + var gateway *istioclientv1beta1.Gateway + Eventually(func() error { + gateway, err = waitForUpdatedGatewayCompletion(cli, "delete", constants.IstioNamespace, constants.KServeGatewayName, isvcName) + return err + }, timeout, interval).Should(Succeed()) + + // Ensure that the server is successfully removed from the KServe local gateway within the istio-system namespace. + targetServerExist := hasServerFromGateway(gateway, isvcName) + Expect(targetServerExist).Should(BeFalse()) + // Ensure that the synced Secret is successfully deleted within the istio-system namespace. + secret := &corev1.Secret{} + Eventually(func() error { + return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", isvcName, constants.IstioNamespace)}, secret) + }, timeout, interval).ShouldNot(Succeed()) + }) + }) }) + }) func withMatchingNestedField(path string, matcher gomegatypes.GomegaMatcher) gomegatypes.GomegaMatcher { @@ -151,3 +414,43 @@ func withMatchingNestedField(path string, matcher gomegatypes.GomegaMatcher) gom func getKServeRouteName(isvc *kservev1beta1.InferenceService) string { return isvc.Name + "-" + isvc.Namespace } + +func waitForUpdatedGatewayCompletion(cli client.Client, op string, namespace, gatewayName string, isvcName string) (*istioclientv1beta1.Gateway, error) { + ctx := context.Background() + portName := fmt.Sprintf("%s-%s", "https", isvcName) + gateway := &istioclientv1beta1.Gateway{} + + // Get the Gateway resource + err := cli.Get(ctx, client.ObjectKey{Namespace: namespace, Name: gatewayName}, gateway) + if err != nil { + return nil, fmt.Errorf("failed to get Gateway: %w", err) + } + + // Check conditions based on operation (op) + switch op { + case "add": + if !hasServerFromGateway(gateway, portName) { + return nil, fmt.Errorf("server %s not found in Gateway %s", portName, gatewayName) + } + case "delete": + if hasServerFromGateway(gateway, portName) { + return nil, fmt.Errorf("server %s still exists in Gateway %s", portName, gatewayName) + } + default: + return nil, fmt.Errorf("unsupported operation: %s", op) + } + + return gateway, nil +} + +// checks if the server exists for the given gateway +func hasServerFromGateway(gateway *istioclientv1beta1.Gateway, portName string) bool { + targetServerExist := false + for _, server := range gateway.Spec.Servers { + if server.Port.Name == portName { + targetServerExist = true + break + } + } + return targetServerExist +} diff --git a/controllers/reconcilers/kserve_isvc_gateway_reconciler.go b/controllers/reconcilers/kserve_isvc_gateway_reconciler.go new file mode 100644 index 00000000..909deeda --- /dev/null +++ b/controllers/reconcilers/kserve_isvc_gateway_reconciler.go @@ -0,0 +1,302 @@ +/* + +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 reconcilers + +import ( + "context" + "fmt" + "net/url" + "reflect" + + "github.com/go-logr/logr" + kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" + "github.com/opendatahub-io/odh-model-controller/controllers/comparators" + "github.com/opendatahub-io/odh-model-controller/controllers/constants" + "github.com/opendatahub-io/odh-model-controller/controllers/processors" + "github.com/opendatahub-io/odh-model-controller/controllers/resources" + "github.com/opendatahub-io/odh-model-controller/controllers/utils" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + istiov1beta1 "istio.io/api/networking/v1beta1" + istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" +) + +var _ SubResourceReconciler = (*KserveGatewayReconciler)(nil) +var meshNamespace string + +type KserveGatewayReconciler struct { + client client.Client + clientReader client.Reader + secretHandler resources.SecretHandler + gatewayHandler resources.GatewayHandler + deltaProcessor processors.DeltaProcessor +} + +// The clientReader uses the API server to retrieve Secrets that are not cached. By default, only Secrets with the specific label "opendatahub.io/managed: true" are cached. +func NewKserveGatewayReconciler(client client.Client, clientReader client.Reader) *KserveGatewayReconciler { + + return &KserveGatewayReconciler{ + client: client, + clientReader: clientReader, + secretHandler: resources.NewSecretHandler(client), + gatewayHandler: resources.NewGatewayHandler(client), + deltaProcessor: processors.NewDeltaProcessor(), + } +} + +func (r *KserveGatewayReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error { + log.V(1).Info("Reconciling KServe local gateway for Kserve InferenceService") + + _, meshNamespace = utils.GetIstioControlPlaneName(ctx, r.client) + + // return if Address.URL is not set + if isvc.Status.Address != nil && isvc.Status.Address.URL == nil { + log.V(1).Info("Waiting for the URL as the InferenceService is not ready yet") + return nil + } + + // return if serving cert secret in the source namespace is not created + srcCertSecret := &corev1.Secret{} + err := r.clientReader.Get(ctx, types.NamespacedName{Name: isvc.Name, Namespace: isvc.Namespace}, srcCertSecret) + if err != nil { + if errors.IsNotFound(err) { + log.V(1).Info(fmt.Sprintf("Waiting for the creation of the serving certificate Secret(%s) in %s namespace", isvc.Name, isvc.Namespace)) + return nil + } + return err + } + + // Copy src secret to destination namespace when there is not the synced secret. + // This use clientReader because the secret that it looks for is not cached. + copiedCertSecret := &corev1.Secret{} + err = r.clientReader.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), Namespace: meshNamespace}, copiedCertSecret) + if err != nil { + if errors.IsNotFound(err) { + if err := r.copyServingCertSecretFromIsvcNamespace(ctx, srcCertSecret, nil); err != nil { + log.V(1).Error(err, fmt.Sprintf("Failed to copy the serving certificate Secret(%s) to %s namespace", srcCertSecret.Name, meshNamespace)) + return err + } + } + return err + } else { + // Recreate copied secrt when src secret is updated + if !reflect.DeepEqual(srcCertSecret.Data, copiedCertSecret.Data) { + log.V(2).Info(fmt.Sprintf("Recreating for serving certificate Secret(%s) in %s namespace", copiedCertSecret.Name, meshNamespace)) + if err := r.copyServingCertSecretFromIsvcNamespace(ctx, srcCertSecret, copiedCertSecret); err != nil { + log.V(1).Error(err, fmt.Sprintf("Failed to copy the Secret(%s) for InferenceService in %s namespace", copiedCertSecret.Name, meshNamespace)) + return err + } + } + } + + // Create Desired resource + desiredResource, err := r.getDesiredResource(isvc) + if err != nil { + return err + } + + // Get Existing resource + existingResource, err := r.getExistingResource(ctx) + if err != nil { + if errors.IsNotFound(err) { + log.Error(err, fmt.Sprintf("Failed to find KServe local gateway in %s namespace", meshNamespace)) + } + return err + } + + // Process Delta + if err = r.processDelta(ctx, log, desiredResource, existingResource); err != nil { + return err + } + return nil +} + +func (r *KserveGatewayReconciler) getDesiredResource(isvc *kservev1beta1.InferenceService) (*istioclientv1beta1.Gateway, error) { + hostname, err := getURLWithoutScheme(isvc) + if err != nil { + return nil, err + } + + desiredGateway := &istioclientv1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.KServeGatewayName, + Namespace: meshNamespace, + }, + Spec: istiov1beta1.Gateway{ + Servers: []*istiov1beta1.Server{ + { + Hosts: []string{hostname}, + Port: &istiov1beta1.Port{ + Name: fmt.Sprintf("%s-%s", "https", isvc.Name), + Number: 8445, + Protocol: "HTTPS", + }, + Tls: &istiov1beta1.ServerTLSSettings{ + CredentialName: fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), + Mode: istiov1beta1.ServerTLSSettings_SIMPLE, + }, + }, + }, + }, + } + + return desiredGateway, nil +} + +func (r *KserveGatewayReconciler) getExistingResource(ctx context.Context) (*istioclientv1beta1.Gateway, error) { + return r.gatewayHandler.Get(ctx, types.NamespacedName{Name: constants.KServeGatewayName, Namespace: meshNamespace}) +} + +func (r *KserveGatewayReconciler) Delete(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error { + var errs []error + + log.V(1).Info(fmt.Sprintf("Deleting serving certificate Secret(%s) in %s namespace", fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), isvc.Namespace)) + if err := r.deleteServingCertSecretInIstioNamespace(ctx, fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace)); err != nil { + log.V(1).Error(err, fmt.Sprintf("Failed to delete the copied serving certificate Secret(%s) in %s namespace", fmt.Sprintf("%s-%s", isvc.Name, isvc.Namespace), isvc.Namespace)) + errs = append(errs, err) + } + + log.V(1).Info(fmt.Sprintf("Deleting the Server(%s) from KServe local gateway in the %s namespace", fmt.Sprintf("%s-%s", "https", isvc.Name), meshNamespace)) + if err := r.removeServerFromGateway(ctx, log, fmt.Sprintf("%s-%s", "https", isvc.Name)); err != nil { + log.V(1).Error(err, fmt.Sprintf("Failed to remove the Server(%s) from KServe local gateway in the %s namespace", fmt.Sprintf("%s-%s", "https", isvc.Name), meshNamespace)) + errs = append(errs, err) + } + + if len(errs) > 0 { + return fmt.Errorf("multiple errors: %v", errs) + } + + return nil +} + +func (r *KserveGatewayReconciler) Cleanup(ctx context.Context, log logr.Logger, isvcName string) error { + // NOOP - Resources should not be deleted until the kserve component is uninstalled. + return nil +} + +func (r *KserveGatewayReconciler) processDelta(ctx context.Context, log logr.Logger, desiredGateway *istioclientv1beta1.Gateway, existingGateway *istioclientv1beta1.Gateway) (err error) { + comparator := comparators.GetGatewayComparator() + delta := r.deltaProcessor.ComputeDelta(comparator, desiredGateway, existingGateway) + + if delta.IsUpdated() { + log.V(1).Info("Delta found", "update", desiredGateway.GetName()) + gw := existingGateway.DeepCopy() + gw.Spec.Servers = append(existingGateway.Spec.Servers, desiredGateway.Spec.Servers[0]) + + if err = r.gatewayHandler.Update(ctx, gw); err != nil { + log.V(1).Error(err, fmt.Sprintf("Failed to add the Server(%s) from KServe local gateway in the istio-system namespace", desiredGateway.Spec.Servers[0].Port.Name)) + return err + } + return nil + } + return nil +} + +func (r *KserveGatewayReconciler) removeServerFromGateway(ctx context.Context, log logr.Logger, serverToRemove string) error { + gateway, err := r.gatewayHandler.Get(ctx, types.NamespacedName{Name: constants.KServeGatewayName, Namespace: meshNamespace}) + if err != nil { + log.V(1).Error(err, "Failed to retrieve KServe local gateway in istio-system namespace") + return err + } + + newServers := []*istiov1beta1.Server{} + for _, server := range gateway.Spec.Servers { + if server.Port.Name != serverToRemove { + newServers = append(newServers, server) + } + } + + gateway.Spec.Servers = newServers + if err := r.gatewayHandler.Update(ctx, gateway); err != nil { + log.V(1).Error(err, "Failed to update KServe local gateway in istio-system namespace") + return err + } + + return nil +} + +func (r *KserveGatewayReconciler) copyServingCertSecretFromIsvcNamespace(ctx context.Context, sourceSecret *corev1.Secret, preDestSecret *corev1.Secret) error { + destinationSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-%s", sourceSecret.Name, sourceSecret.Namespace), + Namespace: meshNamespace, + Labels: map[string]string{ + "opendatahub.io/managed": "true", + "app.kubernetes.io/name": "odh-model-controller", + "app.kubernetes.io/component": "kserve", + "app.kubernetes.io/part-of": "odh-model-serving", + "app.kubernetes.io/managed-by": "odh-model-controller", + }, + }, + Data: sourceSecret.Data, + Type: sourceSecret.Type, + } + + // Remove old secret if src secret is updated + if preDestSecret != nil { + if err := r.client.Delete(ctx, preDestSecret); err != nil { + return err + } + } + + if err := r.client.Create(ctx, destinationSecret); err != nil { + return err + } + + // add label 'opendatahub.io/managed=true' to original Secret for caching + if err := r.addServingCertSecretLabel(ctx, sourceSecret); err != nil { + return err + } + return nil +} + +func (r *KserveGatewayReconciler) addServingCertSecretLabel(ctx context.Context, sourceSecret *corev1.Secret) error { + service := sourceSecret.DeepCopy() + if service.Labels == nil { + service.Labels = make(map[string]string) + } + + service.Labels["opendatahub.io/managed"] = "true" + + err := r.client.Update(ctx, service) + + return err +} + +func (r *KserveGatewayReconciler) deleteServingCertSecretInIstioNamespace(ctx context.Context, targetSecretName string) error { + secret, err := r.secretHandler.Get(ctx, types.NamespacedName{Name: targetSecretName, Namespace: meshNamespace}) + if err != nil && errors.IsNotFound(err) { + return nil + } + + if err := r.client.Delete(ctx, secret); err != nil { + return err + } + return nil +} + +func getURLWithoutScheme(isvc *kservev1beta1.InferenceService) (string, error) { + parsedURL, err := url.Parse(isvc.Status.Address.URL.String()) + if err != nil { + return "", err + } + + return parsedURL.Host + parsedURL.Path, nil +} diff --git a/controllers/reconcilers/kserve_isvc_service_cert_reconciler.go b/controllers/reconcilers/kserve_isvc_service_cert_reconciler.go new file mode 100644 index 00000000..ec083ca7 --- /dev/null +++ b/controllers/reconcilers/kserve_isvc_service_cert_reconciler.go @@ -0,0 +1,135 @@ +/* + +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 reconcilers + +import ( + "context" + + "github.com/go-logr/logr" + kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" + "github.com/opendatahub-io/odh-model-controller/controllers/constants" + "github.com/opendatahub-io/odh-model-controller/controllers/resources" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ SubResourceReconciler = (*KserveIsvcServiceReconciler)(nil) + +type KserveIsvcServiceReconciler struct { + client client.Client + serviceHandler resources.ServiceHandler +} + +func NewKserveIsvcServiceReconciler(client client.Client) *KserveIsvcServiceReconciler { + return &KserveIsvcServiceReconciler{ + client: client, + serviceHandler: resources.NewServiceHandler(client), + } +} + +func (r *KserveIsvcServiceReconciler) Delete(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error { + // NOOP - Resources are deleted along with the deletion of InferenceServices + return nil +} + +func (r *KserveIsvcServiceReconciler) Cleanup(_ context.Context, _ logr.Logger, _ string) error { + // NOOP - Resources are deleted along with the deletion of InferenceServices + return nil +} + +// To support KServe local gateway using HTTPS, each InferenceService (ISVC) needs a certificate. This reconciliation process helps add a serving certificate annotation to the ISVC service. +func (r *KserveIsvcServiceReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error { + log.V(1).Info("Reconciling InferenceService Service serving cert") + + // return if Address.URL is not set + if isvc.Status.Address != nil && isvc.Status.Address.URL == nil { + log.V(1).Info("Waiting for the URL as the InferenceService is not ready yet") + return nil + } + + // Create Desired resource + desiredResource, err := r.createDesiredResource(isvc) + if err != nil { + return err + } + + // Get Existing resource + existingResource, err := r.getExistingResource(ctx, log, isvc) + if err != nil { + return err + } + + // Process Delta + if err = r.processDelta(ctx, log, desiredResource, existingResource); err != nil { + return err + } + return nil +} + +func (r *KserveIsvcServiceReconciler) createDesiredResource(isvc *kservev1beta1.InferenceService) (*v1.Service, error) { + service := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "isvc-service", + Annotations: map[string]string{ + constants.ServingCertAnnotationKey: isvc.Name, + }, + }, + } + return service, nil +} + +func (r *KserveIsvcServiceReconciler) getExistingResource(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) (*v1.Service, error) { + return r.serviceHandler.FetchService(ctx, log, types.NamespacedName{Name: isvc.Name, Namespace: isvc.Namespace}) +} + +func (r *KserveIsvcServiceReconciler) processDelta(ctx context.Context, log logr.Logger, desiredService *v1.Service, existingService *v1.Service) (err error) { + if isUpdated(desiredService, existingService, log) { + log.V(1).Info("Delta found", "update", existingService.GetName()) + service := existingService.DeepCopy() + if service.Annotations == nil { + service.Annotations = make(map[string]string) + } + + for key, value := range desiredService.Annotations { + service.Annotations[key] = value + } + + if err = r.client.Update(ctx, service); err != nil { + return err + } + } + return nil +} + +func isUpdated(desiredService *v1.Service, existingService *v1.Service, log logr.Logger) bool { + if existingService == nil { + log.Info("The service for the InferenceService has not been created yet") + return false + } + deployedAnnotations := existingService.GetAnnotations() + + if len(deployedAnnotations) != 0 { + if val, exists := existingService.Annotations[constants.ServingCertAnnotationKey]; exists { + if val == desiredService.Annotations[constants.ServingCertAnnotationKey] { + return false + } + } + } + + return true +} diff --git a/controllers/reconcilers/kserve_serverless_inferenceservice_reconciler.go b/controllers/reconcilers/kserve_serverless_inferenceservice_reconciler.go index 9a5b8257..00365f0d 100644 --- a/controllers/reconcilers/kserve_serverless_inferenceservice_reconciler.go +++ b/controllers/reconcilers/kserve_serverless_inferenceservice_reconciler.go @@ -33,7 +33,8 @@ type KserveServerlessInferenceServiceReconciler struct { subResourceReconcilers []SubResourceReconciler } -func NewKServeServerlessInferenceServiceReconciler(client client.Client) *KserveServerlessInferenceServiceReconciler { +func NewKServeServerlessInferenceServiceReconciler(client client.Client, clientReader client.Reader) *KserveServerlessInferenceServiceReconciler { + subResourceReconciler := []SubResourceReconciler{ NewKserveServiceMeshMemberReconciler(client), NewKserveRouteReconciler(client), @@ -46,6 +47,8 @@ func NewKServeServerlessInferenceServiceReconciler(client client.Client) *Kserve NewKServeIstioPeerAuthenticationReconciler(client), NewKServeNetworkPolicyReconciler(client), NewKserveAuthConfigReconciler(client), + NewKserveIsvcServiceReconciler(client), + NewKserveGatewayReconciler(client, clientReader), NewKserveMetricsDashboardReconciler(client), } diff --git a/controllers/resources/gateway.go b/controllers/resources/gateway.go new file mode 100644 index 00000000..95901842 --- /dev/null +++ b/controllers/resources/gateway.go @@ -0,0 +1,55 @@ +/* + +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 resources + +import ( + "context" + "fmt" + + istionetworkclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type GatewayHandler interface { + Get(ctx context.Context, key types.NamespacedName) (*istionetworkclientv1beta1.Gateway, error) + Update(ctx context.Context, gateway *istionetworkclientv1beta1.Gateway) error +} + +type gatewayHandler struct { + client client.Client +} + +func NewGatewayHandler(client client.Client) GatewayHandler { + return &gatewayHandler{ + client: client, + } +} + +func (g *gatewayHandler) Get(ctx context.Context, key types.NamespacedName) (*istionetworkclientv1beta1.Gateway, error) { + gateway := &istionetworkclientv1beta1.Gateway{} + if err := g.client.Get(ctx, key, gateway); err != nil { + return nil, err + } + return gateway, nil +} + +func (g *gatewayHandler) Update(ctx context.Context, gateway *istionetworkclientv1beta1.Gateway) error { + if err := g.client.Update(ctx, gateway); err != nil { + return fmt.Errorf("could not UPDATE gateway %s/%s. cause %w", gateway.Namespace, gateway.Name, err) + } + return nil +} diff --git a/controllers/resources/secret.go b/controllers/resources/secret.go new file mode 100644 index 00000000..224e6cb6 --- /dev/null +++ b/controllers/resources/secret.go @@ -0,0 +1,47 @@ +/* + +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 resources + +import ( + "context" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type SecretHandler interface { + Get(ctx context.Context, key types.NamespacedName) (*v1.Secret, error) +} + +type secretHandler struct { + client client.Client +} + +func NewSecretHandler(client client.Client) SecretHandler { + return &secretHandler{ + client: client, + } +} + +func (s *secretHandler) Get(ctx context.Context, key types.NamespacedName) (*v1.Secret, error) { + secret := &v1.Secret{} + if err := s.client.Get(ctx, key, secret); err != nil { + return nil, err + } + + return secret, nil +} diff --git a/controllers/resources/service.go b/controllers/resources/service.go index c3ac39ff..678b2104 100644 --- a/controllers/resources/service.go +++ b/controllers/resources/service.go @@ -17,6 +17,7 @@ package resources import ( "context" + "github.com/go-logr/logr" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -39,8 +40,8 @@ func NewServiceHandler(client client.Client) ServiceHandler { } func (r *serviceHandler) FetchService(ctx context.Context, log logr.Logger, key types.NamespacedName) (*v1.Service, error) { - route := &v1.Service{} - err := r.client.Get(ctx, key, route) + svc := &v1.Service{} + err := r.client.Get(ctx, key, svc) if err != nil && errors.IsNotFound(err) { log.V(1).Info("Service not found.") return nil, nil @@ -48,5 +49,5 @@ func (r *serviceHandler) FetchService(ctx context.Context, log logr.Logger, key return nil, err } log.V(1).Info("Successfully fetch deployed Service") - return route, nil + return svc, nil } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 44ea753e..afc3df02 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -41,7 +41,10 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" routev1 "github.com/openshift/api/route/v1" + istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -125,6 +128,7 @@ var _ = BeforeSuite(func() { testScheme := runtime.NewScheme() utils.RegisterSchemes(testScheme) utilruntime.Must(authorinov1beta2.AddToScheme(testScheme)) + utilruntime.Must(istioclientv1beta1.AddToScheme(testScheme)) // +kubebuilder:scaffold:scheme @@ -155,6 +159,7 @@ var _ = BeforeSuite(func() { err = (NewOpenshiftInferenceServiceReconciler( mgr.GetClient(), + mgr.GetAPIReader(), ctrl.Log.WithName("controllers").WithName("InferenceService-controller"), false)). SetupWithManager(mgr) @@ -205,6 +210,7 @@ var _ = AfterSuite(func() { var _ = AfterEach(func() { cleanUp := func(namespace string, cli client.Client) { inNamespace := client.InNamespace(namespace) + istioNamespace := client.InNamespace(constants.IstioNamespace) Expect(cli.DeleteAllOf(context.TODO(), &kservev1alpha1.ServingRuntime{}, inNamespace)).ToNot(HaveOccurred()) Expect(cli.DeleteAllOf(context.TODO(), &kservev1beta1.InferenceService{}, inNamespace)).ToNot(HaveOccurred()) Expect(cli.DeleteAllOf(context.TODO(), &routev1.Route{}, inNamespace)).ToNot(HaveOccurred()) @@ -213,6 +219,8 @@ var _ = AfterEach(func() { Expect(cli.DeleteAllOf(context.TODO(), &corev1.Secret{}, inNamespace)).ToNot(HaveOccurred()) Expect(cli.DeleteAllOf(context.TODO(), &authorinov1beta2.AuthConfig{}, inNamespace)).ToNot(HaveOccurred()) Expect(cli.DeleteAllOf(context.TODO(), &corev1.ConfigMap{}, inNamespace)).ToNot(HaveOccurred()) + Expect(cli.DeleteAllOf(context.TODO(), &corev1.Service{}, inNamespace)).ToNot(HaveOccurred()) + Expect(cli.DeleteAllOf(context.TODO(), &istioclientv1beta1.Gateway{}, istioNamespace)).ToNot(HaveOccurred()) } cleanUp(WorkingNamespace, cli) for _, ns := range Namespaces.All() { @@ -279,3 +287,7 @@ func createTestNamespaceName() string { } return "test-ns-" + string(b) } + +func NewFakeClientsetWrapper(fakeClient *fake.Clientset) kubernetes.Interface { + return fakeClient +} diff --git a/controllers/testdata/gateway/kserve-local-gateway.yaml b/controllers/testdata/gateway/kserve-local-gateway.yaml new file mode 100644 index 00000000..149e9bd6 --- /dev/null +++ b/controllers/testdata/gateway/kserve-local-gateway.yaml @@ -0,0 +1,15 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: kserve-local-gateway + namespace: istio-system +spec: + selector: + knative: ingressgateway + servers: + - hosts: + - 'demo.default.svc.cluster.local' + port: + name: http + number: 8080 + protocol: HTTP diff --git a/controllers/testdata/gateway/test-isvc-svc-secret.yaml b/controllers/testdata/gateway/test-isvc-svc-secret.yaml new file mode 100644 index 00000000..a13660db --- /dev/null +++ b/controllers/testdata/gateway/test-isvc-svc-secret.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +data: + tls.crt: dGxzLmNydAo= + tls.key: dGxzLmtleQo= +kind: Secret +metadata: + annotations: + openshift.io/description: 'Secret contains a pair signed serving certificate/key + that is generated by Service CA operator for service/example-onnx-mnist with + hostname example-onnx-mnist.default.svc and is annotated to the service with + annotating a service resource with ''service.beta.openshift.io/serving-cert-secret-name: + example-onnx-mnist''. The certificate is valid for 2 years.' + openshift.io/owning-component: service-ca + service.alpha.openshift.io/expiry: "2026-06-11T17:58:35Z" + service.beta.openshift.io/expiry: "2026-06-11T17:58:35Z" + service.beta.openshift.io/originating-service-name: example-onnx-mnist + name: example-onnx-mnist + namespace: default +type: kubernetes.io/tls diff --git a/controllers/testdata/servingcert-service/test-isvc-svc.yaml b/controllers/testdata/servingcert-service/test-isvc-svc.yaml new file mode 100644 index 00000000..94ccecd4 --- /dev/null +++ b/controllers/testdata/servingcert-service/test-isvc-svc.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Service +metadata: + name: example-onnx-mnist + namespace: default +spec: + externalName: kserve-local-gateway.istio-system.svc.cluster.local + sessionAffinity: None + type: ExternalName diff --git a/controllers/utils/init.go b/controllers/utils/init.go index d6127515..75a708e0 100644 --- a/controllers/utils/init.go +++ b/controllers/utils/init.go @@ -6,6 +6,7 @@ import ( authorinov1beta2 "github.com/kuadrant/authorino/api/v1beta2" routev1 "github.com/openshift/api/route/v1" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1" telemetryv1alpha1 "istio.io/client-go/pkg/apis/telemetry/v1alpha1" corev1 "k8s.io/api/core/v1" @@ -34,6 +35,7 @@ func RegisterSchemes(s *runtime.Scheme) { utilruntime.Must(maistrav1.SchemeBuilder.AddToScheme(s)) utilruntime.Must(knservingv1.AddToScheme(s)) utilruntime.Must(authorinov1beta2.SchemeBuilder.AddToScheme(s)) + utilruntime.Must(istioclientv1beta1.SchemeBuilder.AddToScheme(s)) // The following are related to Service Mesh, uncomment this and other // similar blocks to use with Service Mesh diff --git a/go.mod b/go.mod index c71d0843..72d85158 100644 --- a/go.mod +++ b/go.mod @@ -41,6 +41,7 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect + github.com/evanphx/json-patch v5.7.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.7.0 // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/go-logr/zapr v1.3.0 // indirect diff --git a/main.go b/main.go index fca3703b..5b3e8753 100644 --- a/main.go +++ b/main.go @@ -23,7 +23,7 @@ import ( "strconv" "github.com/opendatahub-io/odh-model-controller/controllers/webhook" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" knservingv1 "knative.dev/serving/pkg/apis/serving/v1" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -32,6 +32,7 @@ import ( // to ensure that exec-entrypoint and run can make use of them. // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) + _ "k8s.io/client-go/plugin/pkg/client/auth" "istio.io/client-go/pkg/apis/security/v1beta1" @@ -63,6 +64,7 @@ func init() { //nolint:gochecknoinits //reason this way we ensure schemes are al // +kubebuilder:rbac:groups=serving.kserve.io,resources=servingruntimes/finalizers,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=networking.istio.io,resources=virtualservices,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=networking.istio.io,resources=virtualservices/finalizers,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=networking.istio.io,resources=gateways,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=security.istio.io,resources=peerauthentications,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=security.istio.io,resources=authorizationpolicies,verbs=get;list // +kubebuilder:rbac:groups=telemetry.istio.io,resources=telemetries,verbs=get;list;watch;create;update;patch;delete @@ -131,12 +133,12 @@ func main() { }, Cache: cache.Options{ ByObject: map[client.Object]cache.ByObject{ - &v1.Secret{}: { + &corev1.Secret{}: { Label: labels.SelectorFromSet(labels.Set{ "opendatahub.io/managed": "true", }), }, - &v1.ConfigMap{}: { + &corev1.ConfigMap{}: { Label: labels.SelectorFromSet(labels.Set{ "app.opendatahub.io/kserve": "true", }), @@ -144,6 +146,7 @@ func main() { }, }, }) + if err != nil { setupLog.Error(err, "unable to start manager") os.Exit(1) @@ -152,6 +155,7 @@ func main() { //Setup InferenceService controller if err = (controllers.NewOpenshiftInferenceServiceReconciler( mgr.GetClient(), + mgr.GetAPIReader(), ctrl.Log.WithName("controllers").WithName("InferenceService"), getEnvAsBool("MESH_DISABLED", false))). SetupWithManager(mgr); err != nil { @@ -214,8 +218,9 @@ func main() { setupLog.Error(err, "unable to setup Knative Service validating Webhook") os.Exit(1) } + } else { - setupLog.Info("Skipping setup of Knative Service validating Webhook, because KServe Serverless setup seems to be disabled in the DataScienceCluster resource.") + setupLog.Info("Skipping setup of Knative Service validating/mutating Webhook, because KServe Serverless setup seems to be disabled in the DataScienceCluster resource.") } //+kubebuilder:scaffold:builder