From ffc04667d168de5b2990dc121fb2fdd0c133d906 Mon Sep 17 00:00:00 2001 From: Spolti Date: Tue, 30 Apr 2024 16:12:14 -0300 Subject: [PATCH] review additions Signed-off-by: Spolti --- config/manager/manager.yaml | 12 ++++ .../kserve_istio_podmonitor_reconciler.go | 8 +-- .../kserve_istio_servicemonitor_reconciler.go | 8 +-- controllers/utils/utils.go | 57 +++++++++++++------ 4 files changed, 57 insertions(+), 28 deletions(-) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 55d5f46e..fdc929d6 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -59,6 +59,18 @@ spec: name: auth-refs key: AUTHORINO_LABEL optional: true + - name: CONTROL_PLANE_NAME + valueFrom: + configMapKeyRef: + name: service-mesh-refs + key: CONTROL_PLANE_NAME + optional: true + - name: MESH_NAMESPACE + valueFrom: + configMapKeyRef: + name: service-mesh-refs + key: MESH_NAMESPACE + optional: true livenessProbe: httpGet: path: /healthz diff --git a/controllers/reconcilers/kserve_istio_podmonitor_reconciler.go b/controllers/reconcilers/kserve_istio_podmonitor_reconciler.go index e2980245..57c18962 100644 --- a/controllers/reconcilers/kserve_istio_podmonitor_reconciler.go +++ b/controllers/reconcilers/kserve_istio_podmonitor_reconciler.go @@ -20,7 +20,6 @@ import ( "fmt" "github.com/go-logr/logr" kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" - "github.com/kuadrant/authorino/pkg/log" "github.com/opendatahub-io/odh-model-controller/controllers/comparators" "github.com/opendatahub-io/odh-model-controller/controllers/processors" "github.com/opendatahub-io/odh-model-controller/controllers/resources" @@ -80,10 +79,7 @@ func (r *KserveIstioPodMonitorReconciler) Cleanup(ctx context.Context, log logr. } func (r *KserveIstioPodMonitorReconciler) createDesiredResource(ctx context.Context, isvc *kservev1beta1.InferenceService) (*v1.PodMonitor, error) { - dsciName, err := utils.GetDSCIName(ctx, r.client) - if err != nil { - log.V(1).Error(err, "Error getting DSCI name, default value will be used.") - } + istioControlPlaneName, meshNamespace := utils.GetIstioControlPlaneName(ctx, r.client) desiredPodMonitor := &v1.PodMonitor{ ObjectMeta: metav1.ObjectMeta{ @@ -143,7 +139,7 @@ func (r *KserveIstioPodMonitorReconciler) createDesiredResource(ctx context.Cont }, { Action: "replace", - Replacement: fmt.Sprintf("%s-istio-system", dsciName), + Replacement: fmt.Sprintf("%s-%s", istioControlPlaneName, meshNamespace), TargetLabel: "mesh_id", }, }, diff --git a/controllers/reconcilers/kserve_istio_servicemonitor_reconciler.go b/controllers/reconcilers/kserve_istio_servicemonitor_reconciler.go index 2716e783..6abec22e 100644 --- a/controllers/reconcilers/kserve_istio_servicemonitor_reconciler.go +++ b/controllers/reconcilers/kserve_istio_servicemonitor_reconciler.go @@ -20,7 +20,6 @@ import ( "fmt" "github.com/go-logr/logr" kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" - "github.com/kuadrant/authorino/pkg/log" "github.com/opendatahub-io/odh-model-controller/controllers/comparators" "github.com/opendatahub-io/odh-model-controller/controllers/processors" "github.com/opendatahub-io/odh-model-controller/controllers/resources" @@ -80,10 +79,7 @@ func (r *KserveIstioServiceMonitorReconciler) Cleanup(ctx context.Context, log l } func (r *KserveIstioServiceMonitorReconciler) createDesiredResource(ctx context.Context, isvc *kservev1beta1.InferenceService) (*v1.ServiceMonitor, error) { - dsciName, err := utils.GetDSCIName(ctx, r.client) - if err != nil { - log.V(1).Error(err, "Error getting DSCI name, default value will be used.") - } + istioControlPlaneName, meshNamespace := utils.GetIstioControlPlaneName(ctx, r.client) desiredServiceMonitor := &v1.ServiceMonitor{ ObjectMeta: metav1.ObjectMeta{ @@ -104,7 +100,7 @@ func (r *KserveIstioServiceMonitorReconciler) createDesiredResource(ctx context. RelabelConfigs: []*v1.RelabelConfig{ { TargetLabel: "mesh_id", - Replacement: fmt.Sprintf("%s-istio-system", dsciName), + Replacement: fmt.Sprintf("%s-%s", istioControlPlaneName, meshNamespace), Action: "replace", }, }, diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index f96e22b0..969ca90a 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/kuadrant/authorino/pkg/log" "os" "reflect" @@ -27,7 +28,8 @@ var ( ) const ( - defaultDSCIName = "data-science-smcp" + defaultIstioControlPlaneName = "data-science-smcp" + defaultMeshNamespace = "istio-system" inferenceServiceDeploymentModeAnnotation = "serving.kserve.io/deploymentMode" KserveConfigMapName = "inferenceservice-config" KServeWithServiceMeshComponent = "kserve-service-mesh" @@ -168,23 +170,46 @@ func VerifyIfCapabilityIsEnabled(ctx context.Context, cli client.Client, capabil return false, nil } -// GetDSCIName returns the name of the DSCI in the cluster. If it fails to read the DSCI, -// it returns the default DSCI name. -func GetDSCIName(ctx context.Context, cli client.Client) (string, error) { - objectList, err := getDSCIObject(ctx, cli) - if err != nil { - return defaultDSCIName, fmt.Errorf("not able to read %s: %w, using the default DSCI %s", - objectList, err, defaultDSCIName) - } - for _, item := range objectList.Items { - statusField, found, err := unstructured.NestedString(item.Object, "spec", "serviceMesh", "controlPlane", "name") - if err != nil || !found { - return defaultDSCIName, nil - } else if statusField != "" { - return statusField, nil +// GetIstioControlPlaneName return the name of the Istio Control Plane and the mesh namespace. +// It will first try to read the environment variables, if not found will then try to read the DSCI +// If the required value is not available in the DSCI, it will return the default values +func GetIstioControlPlaneName(ctx context.Context, cli client.Client) (istioControlPlane string, meshNamespace string) { + // first try toi retrieve it from the envs, it should be available through the service-mesh-refs ConfigMap + istioControlPlane = os.Getenv("CONTROL_PLANE_NAME") + meshNamespace = os.Getenv("MESH_NAMESPACE") + + if len(istioControlPlane) == 0 || len(meshNamespace) == 0 { + objectList, _ := getDSCIObject(ctx, cli) + log.V(1).Info("Trying to read Istio Control Plane name and namespace from DSCI") + + for _, item := range objectList.Items { + if len(istioControlPlane) == 0 { + name, _, _ := unstructured.NestedString(item.Object, "spec", "serviceMesh", "controlPlane", "name") + if len(name) > 0 { + 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 = defaultIstioControlPlaneName + } + } + + if len(meshNamespace) == 0 { + namespace, _, _ := unstructured.NestedString(item.Object, "spec", "serviceMesh", "controlPlane", "name") + if len(namespace) > 0 { + 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 = defaultIstioControlPlaneName + } + } } + } else { + log.V(1).Info("Istio Control Plane name and namespace read from environment variables") } - return defaultDSCIName, nil + + return istioControlPlane, meshNamespace } // VerifyIfMeshAuthorizationIsEnabled func checks if Authorization has been configured for