diff --git a/Makefile b/Makefile index 2594469a..67a8f261 100644 --- a/Makefile +++ b/Makefile @@ -69,7 +69,8 @@ vet: ## Run go vet against code. .PHONY: test test: manifests generate fmt vet envtest ## Run tests. - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" POD_NAMESPACE=default go test -v ./controllers/... -coverprofile cover.out + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" POD_NAMESPACE=default \ + MESH_NAMESPACE=istio-system CONTROL_PLANE_NAME=istio-system go test -v ./controllers/... -coverprofile cover.out .PHONY: e2e-test e2e-test: manifests generate fmt vet ## Run e2e-tests. diff --git a/config/runtimes/vllm-template.yaml b/config/runtimes/vllm-template.yaml index efb7b110..54d4bf26 100644 --- a/config/runtimes/vllm-template.yaml +++ b/config/runtimes/vllm-template.yaml @@ -43,7 +43,6 @@ objects: - "--port=8080" - "--model=/mnt/models" - "--served-model-name={{.Name}}" - - "--distributed-executor-backend=mp" env: - name: HF_HOME value: /tmp/hf_home diff --git a/controllers/constants/constants.go b/controllers/constants/constants.go index a5d83dcc..9dcb11d0 100644 --- a/controllers/constants/constants.go +++ b/controllers/constants/constants.go @@ -18,8 +18,6 @@ package constants const ( InferenceServiceKind = "InferenceService" - IstioNamespace = "istio-system" - IstioControlPlaneName = "data-science-smcp" ServiceMeshMemberRollName = "default" ServiceMeshMemberName = "default" IstioIngressService = "istio-ingressgateway" diff --git a/controllers/kserve_inferenceservice_controller_authconfig_test.go b/controllers/kserve_inferenceservice_controller_authconfig_test.go index c3e21723..8dc775b7 100644 --- a/controllers/kserve_inferenceservice_controller_authconfig_test.go +++ b/controllers/kserve_inferenceservice_controller_authconfig_test.go @@ -332,7 +332,8 @@ func createAuthorizationPolicy(authPolicyFile string) error { gvr := istiosec_v1b1.SchemeGroupVersion.WithResource("authorizationpolicies") resource := dynamicClient.Resource(gvr) - _, createErr := resource.Namespace(constants.IstioNamespace).Create(context.TODO(), obj, metav1.CreateOptions{}) + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) + _, createErr := resource.Namespace(meshNamespace).Create(context.TODO(), obj, metav1.CreateOptions{}) return createErr } @@ -371,5 +372,6 @@ func deleteAuthorizationPolicy(authPolicyFile string) error { } gvr := istiosec_v1b1.SchemeGroupVersion.WithResource("authorizationpolicies") - return dynamicClient.Resource(gvr).Namespace(constants.IstioNamespace).Delete(context.TODO(), obj.GetName(), metav1.DeleteOptions{}) + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) + return dynamicClient.Resource(gvr).Namespace(meshNamespace).Delete(context.TODO(), obj.GetName(), metav1.DeleteOptions{}) } diff --git a/controllers/kserve_inferenceservice_controller_mesh_test.go b/controllers/kserve_inferenceservice_controller_mesh_test.go index d97d4b6a..720be204 100644 --- a/controllers/kserve_inferenceservice_controller_mesh_test.go +++ b/controllers/kserve_inferenceservice_controller_mesh_test.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/opendatahub-io/odh-model-controller/controllers/constants" + "github.com/opendatahub-io/odh-model-controller/controllers/utils" ) var _ = Describe("The KServe mesh reconciler", func() { @@ -43,6 +44,7 @@ var _ = Describe("The KServe mesh reconciler", func() { } createUserOwnedMeshEnrolment := func(namespace string) *maistrav1.ServiceMeshMember { + controlPlaneName, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) smm := &maistrav1.ServiceMeshMember{ ObjectMeta: metav1.ObjectMeta{ Name: constants.ServiceMeshMemberName, @@ -52,8 +54,8 @@ var _ = Describe("The KServe mesh reconciler", func() { }, Spec: maistrav1.ServiceMeshMemberSpec{ ControlPlaneRef: maistrav1.ServiceMeshControlPlaneRef{ - Name: constants.IstioControlPlaneName, - Namespace: constants.IstioNamespace, + Name: controlPlaneName, + Namespace: meshNamespace, }}, Status: maistrav1.ServiceMeshMemberStatus{}, } diff --git a/controllers/kserve_inferenceservice_controller_metrics_test.go b/controllers/kserve_inferenceservice_controller_metrics_test.go index 2f7c3c8e..c63ef12d 100644 --- a/controllers/kserve_inferenceservice_controller_metrics_test.go +++ b/controllers/kserve_inferenceservice_controller_metrics_test.go @@ -20,9 +20,11 @@ const ( KserveOvmsInferenceServiceName = "example-onnx-mnist" UnsupportedMetricsInferenceServiceName = "sklearn-v2-iris" NilRuntimeInferenceServiceName = "sklearn-v2-iris-no-runtime" + NilModelInferenceServiceName = "custom-runtime" UnsupportedMetricsInferenceServicePath = "./testdata/deploy/kserve-unsupported-metrics-inference-service.yaml" UnsupprtedMetricsServingRuntimePath = "./testdata/deploy/kserve-unsupported-metrics-serving-runtime.yaml" NilRuntimeInferenceServicePath = "./testdata/deploy/kserve-nil-runtime-inference-service.yaml" + NilModelInferenceServicePath = "./testdata/deploy/kserve-nil-model-inference-service.yaml" ) var _ = Describe("The KServe Dashboard reconciler", func() { @@ -126,6 +128,25 @@ var _ = Describe("The KServe Dashboard reconciler", func() { } Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue()) }) + + It("if the isvc does not have the model field specified, an unsupported metrics configmap should be created", func() { + _ = createInferenceService(testNs, NilModelInferenceServiceName, NilModelInferenceServicePath) + + metricsConfigMap, err := waitForConfigMap(cli, testNs, NilModelInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(metricsConfigMap).NotTo(BeNil()) + + expectedCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: NilModelInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, + Namespace: testNs, + }, + Data: map[string]string{ + "supported": "false", + }, + } + Expect(compareConfigMap(metricsConfigMap, expectedCM)).Should(BeTrue()) + }) }) When("deleting the deployed models", func() { @@ -147,7 +168,7 @@ var _ = Describe("The KServe Dashboard reconciler", func() { Expect(cli.Delete(ctx, SklearnInferenceService)).Should(Succeed()) Eventually(func() error { configmap := &corev1.ConfigMap{} - key := types.NamespacedName{Name: KserveOvmsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, Namespace: SklearnInferenceService.Namespace} + key := types.NamespacedName{Name: UnsupportedMetricsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, Namespace: SklearnInferenceService.Namespace} err := cli.Get(ctx, key, configmap) return err }, timeout, interval).ShouldNot(Succeed()) diff --git a/controllers/kserve_inferenceservice_controller_test.go b/controllers/kserve_inferenceservice_controller_test.go index a5008dd3..b1f94451 100644 --- a/controllers/kserve_inferenceservice_controller_test.go +++ b/controllers/kserve_inferenceservice_controller_test.go @@ -28,6 +28,7 @@ import ( . "github.com/onsi/gomega/gstruct" gomegatypes "github.com/onsi/gomega/types" "github.com/opendatahub-io/odh-model-controller/controllers/constants" + "github.com/opendatahub-io/odh-model-controller/controllers/utils" routev1 "github.com/openshift/api/route/v1" istioclientv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" corev1 "k8s.io/api/core/v1" @@ -69,6 +70,7 @@ var _ = Describe("The Openshift Kserve model controller", func() { }) It("With Kserve InferenceService a Route be created", func() { + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) inferenceService := &kservev1beta1.InferenceService{} err := convertToStructuredResource(KserveInferenceServicePath1, inferenceService) Expect(err).NotTo(HaveOccurred()) @@ -79,7 +81,7 @@ var _ = Describe("The Openshift Kserve model controller", func() { By("By checking that the controller has not created the Route") Consistently(func() error { route := &routev1.Route{} - key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: constants.IstioNamespace} + key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: meshNamespace} err = cli.Get(ctx, key, route) return err }, timeout, interval).Should(HaveOccurred()) @@ -98,7 +100,7 @@ var _ = Describe("The Openshift Kserve model controller", func() { By("By checking that the controller has created the Route") Eventually(func() error { route := &routev1.Route{} - key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: constants.IstioNamespace} + key := types.NamespacedName{Name: getKServeRouteName(inferenceService), Namespace: meshNamespace} err = cli.Get(ctx, key, route) return err }, timeout, interval).Should(Succeed()) @@ -192,16 +194,18 @@ var _ = Describe("The Openshift Kserve model controller", func() { err = cli.Status().Update(ctx, deployedInferenceService) Expect(err).NotTo(HaveOccurred()) + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) + // 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) + return cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, 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) + gateway, err = waitForUpdatedGatewayCompletion(cli, "add", meshNamespace, constants.KServeGatewayName, inferenceService.Name) return err }, timeout, interval).Should(Succeed()) @@ -246,6 +250,7 @@ var _ = Describe("The Openshift Kserve model controller", func() { ctx := context.Background() testNamespace := Namespaces.Create(cli) testNs = testNamespace.Name + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) inferenceServiceConfig := &corev1.ConfigMap{} Expect(convertToStructuredResource(InferenceServiceConfigPath1, inferenceServiceConfig)).To(Succeed()) @@ -308,13 +313,13 @@ var _ = Describe("The Openshift Kserve model controller", func() { }, 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) + return cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, 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) + gateway, err = waitForUpdatedGatewayCompletion(cli, "add", meshNamespace, constants.KServeGatewayName, inferenceService.Name) return err }, timeout, interval).Should(Succeed()) @@ -345,9 +350,10 @@ var _ = Describe("The Openshift Kserve model controller", func() { Expect(err).NotTo(HaveOccurred()) // Verify that the certificate secret in the istio-system namespace is updated. + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) 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()) + Expect(cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, 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") } @@ -366,10 +372,12 @@ var _ = Describe("The Openshift Kserve model controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(cli.Delete(ctx, deployedInferenceService)).Should(Succeed()) + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) + // 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) + gateway, err = waitForUpdatedGatewayCompletion(cli, "delete", meshNamespace, constants.KServeGatewayName, isvcName) return err }, timeout, interval).Should(Succeed()) @@ -380,7 +388,7 @@ var _ = Describe("The Openshift Kserve model controller", func() { // 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) + return cli.Get(ctx, client.ObjectKey{Namespace: meshNamespace, Name: fmt.Sprintf("%s-%s", isvcName, meshNamespace)}, secret) }, timeout, interval).ShouldNot(Succeed()) }) }) diff --git a/controllers/reconcilers/kserve_authconfig_reconciler.go b/controllers/reconcilers/kserve_authconfig_reconciler.go index 0c8ca412..515b4eb3 100644 --- a/controllers/reconcilers/kserve_authconfig_reconciler.go +++ b/controllers/reconcilers/kserve_authconfig_reconciler.go @@ -18,6 +18,7 @@ package reconcilers import ( "context" "fmt" + "github.com/opendatahub-io/odh-model-controller/controllers/utils" "github.com/go-logr/logr" @@ -57,7 +58,7 @@ func NewKserveAuthConfigReconciler(client client.Client) *KserveAuthConfigReconc func (r *KserveAuthConfigReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error { log.V(1).Info("Reconciling Authorino AuthConfig for InferenceService") - authorinoEnabled, capabilityErr := utils.VerifyIfMeshAuthorizationIsEnabled(context.Background(), r.client) + authorinoEnabled, capabilityErr := utils.VerifyIfMeshAuthorizationIsEnabled(ctx, r.client) if capabilityErr != nil { log.V(1).Error(capabilityErr, "Error while verifying if Authorino is enabled") return nil diff --git a/controllers/reconcilers/kserve_metrics_dashboard_reconciler.go b/controllers/reconcilers/kserve_metrics_dashboard_reconciler.go index 0eec5261..e07c243c 100644 --- a/controllers/reconcilers/kserve_metrics_dashboard_reconciler.go +++ b/controllers/reconcilers/kserve_metrics_dashboard_reconciler.go @@ -95,17 +95,24 @@ func (r *KserveMetricsDashboardReconciler) createDesiredResource(ctx context.Con var servingRuntime string runtime := &kservev1alpha1.ServingRuntime{} supported := false + + // there is the possibility to also have and nil model field, e.g: + //predictor: + // containers: + // - name: kserve-container + // image: user/custom-model:v1 + if nil == isvc.Spec.Predictor.Model { + log.V(1).Info("no `predictor.model` field found in InferenceService, no metrics will be available") + return r.createConfigMap(isvc, false, log) + } + // resolve SR isvcRuntime := isvc.Spec.Predictor.Model.Runtime if isvcRuntime == nil { runtime, err = utils.FindSupportingRuntimeForISvc(ctx, r.client, log, isvc) if err != nil { if errwrap.Contains(err, constants.NoSuitableRuntimeError) { - configmap, err := r.createConfigMap(isvc, false, log) - if err != nil { - return nil, err - } - return configmap, nil + return r.createConfigMap(isvc, false, log) } return nil, err } diff --git a/controllers/reconcilers/kserve_route_reconciler.go b/controllers/reconcilers/kserve_route_reconciler.go index b4d4a103..97491240 100644 --- a/controllers/reconcilers/kserve_route_reconciler.go +++ b/controllers/reconcilers/kserve_route_reconciler.go @@ -26,6 +26,7 @@ import ( constants2 "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" + utils2 "github.com/opendatahub-io/odh-model-controller/controllers/utils" v1 "github.com/openshift/api/route/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -75,7 +76,8 @@ func (r *KserveRouteReconciler) Reconcile(ctx context.Context, log logr.Logger, func (r *KserveRouteReconciler) Delete(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error { log.V(1).Info("Deleting Kserve inference service generic route") - return r.routeHandler.DeleteRoute(ctx, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: constants2.IstioNamespace}) + _, meshNamespace := utils2.GetIstioControlPlaneName(ctx, r.client) + return r.routeHandler.DeleteRoute(ctx, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: meshNamespace}) } func (r *KserveRouteReconciler) Cleanup(_ context.Context, _ logr.Logger, _ string) error { @@ -129,10 +131,12 @@ func (r *KserveRouteReconciler) createDesiredResource(isvc *kservev1beta1.Infere } } + _, meshNamespace := utils2.GetIstioControlPlaneName(context.Background(), r.client) + route := &v1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: getKServeRouteName(isvc), - Namespace: constants2.IstioNamespace, + Namespace: meshNamespace, Annotations: annotations, Labels: isvc.Labels, }, @@ -159,7 +163,8 @@ func (r *KserveRouteReconciler) createDesiredResource(isvc *kservev1beta1.Infere } func (r *KserveRouteReconciler) getExistingResource(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) (*v1.Route, error) { - return r.routeHandler.FetchRoute(ctx, log, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: constants2.IstioNamespace}) + _, meshNamespace := utils2.GetIstioControlPlaneName(ctx, r.client) + return r.routeHandler.FetchRoute(ctx, log, types.NamespacedName{Name: getKServeRouteName(isvc), Namespace: meshNamespace}) } func (r *KserveRouteReconciler) processDelta(ctx context.Context, log logr.Logger, desiredRoute *v1.Route, existingRoute *v1.Route) (err error) { diff --git a/controllers/storageconfig_controller.go b/controllers/storageconfig_controller.go index d9ae80eb..d111f4c7 100644 --- a/controllers/storageconfig_controller.go +++ b/controllers/storageconfig_controller.go @@ -108,7 +108,14 @@ func (r *StorageSecretReconciler) reconcileSecret(secret *corev1.Secret, Namespace: secret.Namespace, }, odhGlobalCertConfigMap) - if err == nil { + if err != nil { + if apierrs.IsNotFound(err) { + log.Info("unable to fetch the ODH Global Cert ConfigMap", "error", err) + + } else { + return err + } + } else { odhCustomCertData = odhGlobalCertConfigMap.Data[constants.ODHCustomCACertFileName] } diff --git a/controllers/storageconfig_controller_test.go b/controllers/storageconfig_controller_test.go index 5dc07674..8ab1ea68 100644 --- a/controllers/storageconfig_controller_test.go +++ b/controllers/storageconfig_controller_test.go @@ -34,6 +34,7 @@ import ( const ( dataconnectionStringPath = "./testdata/secrets/dataconnection-string.yaml" storageconfigEncodedPath = "./testdata/secrets/storageconfig-encoded.yaml" + storageconfigCertString = "./testdata/secrets/storageconfig-cert-string.yaml" storageconfigEncodedUnmanagedPath = "./testdata/secrets/storageconfig-encoded-unmanaged.yaml" storageconfigCertEncodedPath = "./testdata/secrets/storageconfig-cert-encoded.yaml" storageconfigUpdatedCertEncodedPath = "./testdata/secrets/storageconfig-updated-cert-encoded.yaml" @@ -183,6 +184,39 @@ var _ = Describe("StorageConfig controller", func() { Expect(compareSecrets(updatedStorageconfigSecret, expectedUpdatedStorageConfigSecret)).Should((BeTrue())) }) }) + + Context("when a configmap odh-trusted-ca-bundle does not exists", func() { + It("should not return error", func() { + dataconnectionStringSecret := &corev1.Secret{} + + By("creating data connection secret") + err := convertToStructuredResource(dataconnectionStringPath, dataconnectionStringSecret) + Expect(err).NotTo(HaveOccurred()) + Expect(cli.Create(ctx, dataconnectionStringSecret)).Should(Succeed()) + + storageconfigSecret, err := waitForSecret(cli, WorkingNamespace, constants.DefaultStorageConfig, 30, 1*time.Second) + Expect(err).NotTo(HaveOccurred()) + + // Check storage-config secret + expectedStorageConfigSecret := &corev1.Secret{} + err = convertToStructuredResource(storageconfigEncodedPath, expectedStorageConfigSecret) + Expect(err).NotTo(HaveOccurred()) + + stringMap := make(map[string]string) + for key, value := range storageconfigSecret.Data { + stringValue := string(value) + stringMap[key] = stringValue + } + fmt.Printf("storageconfigSecret %s\n", stringMap) + stringMap1 := make(map[string]string) + for key, value := range expectedStorageConfigSecret.Data { + stringValue := string(value) + stringMap1[key] = stringValue + } + Expect(compareSecrets(storageconfigSecret, expectedStorageConfigSecret)).Should((BeTrue())) + }) + }) + }) func updateSecretData(cli client.Client, namespace, secretName string, dataKey string, dataValue string) error { diff --git a/controllers/suite_test.go b/controllers/suite_test.go index b073adf2..4590760d 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -18,13 +18,14 @@ package controllers import ( "context" "fmt" - apierrs "k8s.io/apimachinery/pkg/api/errors" "math/rand" "os" "path/filepath" "testing" "time" + apierrs "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/metrics/server" @@ -52,7 +53,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - "github.com/opendatahub-io/odh-model-controller/controllers/constants" "github.com/opendatahub-io/odh-model-controller/controllers/utils" //+kubebuilder:scaffold:imports ) @@ -140,10 +140,11 @@ var _ = BeforeSuite(func() { Expect(cli).NotTo(BeNil()) // Create istio-system namespace + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) istioNamespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: constants.IstioNamespace, - Namespace: constants.IstioNamespace, + Name: meshNamespace, + Namespace: meshNamespace, }, } Expect(cli.Create(ctx, istioNamespace)).Should(Succeed()) @@ -212,7 +213,8 @@ var _ = AfterSuite(func() { var _ = AfterEach(func() { cleanUp := func(namespace string, cli client.Client) { inNamespace := client.InNamespace(namespace) - istioNamespace := client.InNamespace(constants.IstioNamespace) + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, cli) + istioNamespace := client.InNamespace(meshNamespace) 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()) diff --git a/controllers/testdata/deploy/kserve-nil-model-inference-service.yaml b/controllers/testdata/deploy/kserve-nil-model-inference-service.yaml new file mode 100644 index 00000000..774dc91b --- /dev/null +++ b/controllers/testdata/deploy/kserve-nil-model-inference-service.yaml @@ -0,0 +1,10 @@ +apiVersion: serving.kserve.io/v1beta1 +kind: InferenceService +metadata: + name: custom-model + namespace: default +spec: + predictor: + containers: + - name: kserve-container + image: user/custom-model:v1 \ No newline at end of file diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index 73780906..4fa955d1 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -15,6 +15,7 @@ import ( kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" "github.com/kuadrant/authorino/pkg/log" + "github.com/opendatahub-io/odh-model-controller/controllers/constants" v1beta12 "istio.io/api/security/v1beta1" "istio.io/client-go/pkg/apis/security/v1beta1" corev1 "k8s.io/api/core/v1" @@ -26,8 +27,6 @@ import ( "k8s.io/client-go/discovery" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/opendatahub-io/odh-model-controller/controllers/constants" ) type IsvcDeploymentMode string @@ -193,8 +192,8 @@ func GetIstioControlPlaneName(ctx context.Context, cli client.Client) (istioCont log.V(1).Info("Trying to read Istio Control Plane name and namespace from DSCI") objectList, err := getDSCIObject(ctx, cli) if err != nil { - log.V(0).Error(err, "Failed to fetch the DSCI object, using default values") - return constants.IstioControlPlaneName, constants.IstioNamespace + log.V(0).Error(err, "Failed to fetch the DSCI object") + panic("error reading Istio Control Plane name and namespace from DSCI.") } for _, item := range objectList.Items { if len(istioControlPlane) == 0 { @@ -203,8 +202,8 @@ func GetIstioControlPlaneName(ctx context.Context, cli client.Client) (istioCont istioControlPlane = name } else { log.V(1).Info("Istio Control Plane name is not set in DSCI") - // at this point, it is not set anywhere, lets just use the default - istioControlPlane = constants.IstioControlPlaneName + // at this point, it is not set anywhere, lets return an error + panic("error setting Istio Control Plane in DSCI.") } } @@ -214,8 +213,8 @@ func GetIstioControlPlaneName(ctx context.Context, cli client.Client) (istioCont meshNamespace = namespace } else { log.V(1).Info("Mesh Namespace is not set in DSCI") - // at this point, it is not set anywhere, lets just use the default - meshNamespace = constants.IstioNamespace + // at this point, it is not set anywhere, lets return an error + panic("error setting Mesh Namespace in DSCI.") } } } @@ -246,7 +245,8 @@ func VerifyIfMeshAuthorizationIsEnabled(ctx context.Context, cli client.Client) // is available, taking into account both managed an unmanaged configurations. authPolicies := &v1beta1.AuthorizationPolicyList{} - if err := cli.List(ctx, authPolicies, client.InNamespace(constants.IstioNamespace)); err != nil && !meta.IsNoMatchError(err) { + _, meshNamespace := GetIstioControlPlaneName(ctx, cli) + if err := cli.List(ctx, authPolicies, client.InNamespace(meshNamespace)); err != nil && !meta.IsNoMatchError(err) { return false, err } if len(authPolicies.Items) == 0 { diff --git a/controllers/webhook/ksvc_validator.go b/controllers/webhook/ksvc_validator.go index 1e629a8c..4f28a813 100644 --- a/controllers/webhook/ksvc_validator.go +++ b/controllers/webhook/ksvc_validator.go @@ -3,9 +3,10 @@ package webhook import ( "context" "fmt" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "strings" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" "k8s.io/apimachinery/pkg/runtime" types2 "k8s.io/apimachinery/pkg/types" @@ -15,6 +16,7 @@ import ( "github.com/opendatahub-io/odh-model-controller/controllers/constants" "github.com/opendatahub-io/odh-model-controller/controllers/resources" + "github.com/opendatahub-io/odh-model-controller/controllers/utils" ) // +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-serving-knative-dev-v1-service,mutating=false,failurePolicy=fail,groups="serving.knative.dev",resources=services,verbs=create,versions=v1,name=validating.ksvc.odh-model-controller.opendatahub.io,sideEffects=None @@ -76,13 +78,14 @@ func (v *ksvcValidator) ValidateCreate(ctx context.Context, obj runtime.Object) // member. If it is still not a member, reject creation of the Ksvc to prevent // creation of a Pod that would not be in the Mesh. smmrQuerier := resources.NewServiceMeshMemberRole(v.client) - smmr, fetchSmmrErr := smmrQuerier.FetchSMMR(ctx, log, types2.NamespacedName{Name: constants.ServiceMeshMemberRollName, Namespace: constants.IstioNamespace}) + _, meshNamespace := utils.GetIstioControlPlaneName(ctx, v.client) + smmr, fetchSmmrErr := smmrQuerier.FetchSMMR(ctx, log, types2.NamespacedName{Name: constants.ServiceMeshMemberRollName, Namespace: meshNamespace}) if fetchSmmrErr != nil { - log.Error(fetchSmmrErr, "Error when fetching ServiceMeshMemberRoll", "smmr.namespace", constants.IstioNamespace, "smmr.name", constants.ServiceMeshMemberRollName) + log.Error(fetchSmmrErr, "Error when fetching ServiceMeshMemberRoll", "smmr.namespace", meshNamespace, "smmr.name", constants.ServiceMeshMemberRollName) return nil, fetchSmmrErr } if smmr == nil { - log.Info("Rejecting Knative service because the ServiceMeshMemberRoll does not exist", "smmr.namespace", constants.IstioNamespace, "smmr.name", constants.ServiceMeshMemberRollName) + log.Info("Rejecting Knative service because the ServiceMeshMemberRoll does not exist", "smmr.namespace", meshNamespace, "smmr.name", constants.ServiceMeshMemberRollName) return nil, fmt.Errorf("rejecting creation of Knative service %s on namespace %s because the ServiceMeshMemberRoll does not exist", ksvc.Name, ksvc.Namespace) } diff --git a/controllers/webhook_ksvc_validator_test.go b/controllers/webhook_ksvc_validator_test.go index b8d00f81..2b09636f 100644 --- a/controllers/webhook_ksvc_validator_test.go +++ b/controllers/webhook_ksvc_validator_test.go @@ -5,6 +5,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/opendatahub-io/odh-model-controller/controllers/constants" + "github.com/opendatahub-io/odh-model-controller/controllers/utils" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -17,6 +18,7 @@ import ( var _ = Describe("Knative validator webhook", func() { var validator admission.CustomValidator + var meshNamespace string createKserveOwnedKsvc := func() *knservingv1.Service { return &knservingv1.Service{ @@ -39,7 +41,7 @@ var _ = Describe("Knative validator webhook", func() { smmr := &v1.ServiceMeshMemberRoll{ ObjectMeta: metav1.ObjectMeta{ Name: constants.ServiceMeshMemberRollName, - Namespace: constants.IstioNamespace, + Namespace: meshNamespace, }, } @@ -52,13 +54,14 @@ var _ = Describe("Knative validator webhook", func() { } BeforeEach(func() { + _, meshNamespace = utils.GetIstioControlPlaneName(ctx, cli) validator = webhook.NewKsvcValidator(cli) // Other tests may create a ServiceMeshMemberRoll. // If there is one, delete it because it conflicts with tests in this file. smmr := v1.ServiceMeshMemberRoll{} getErr := cli.Get(ctx, types.NamespacedName{ - Namespace: constants.IstioNamespace, + Namespace: meshNamespace, Name: constants.ServiceMeshMemberRollName, }, &smmr) if getErr != nil {