From 3eb4e9c050c463ef293797b49981b15e9a8962ae Mon Sep 17 00:00:00 2001 From: Alessio Pragliola <83355398+Al-Pragliola@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:36:58 +0100 Subject: [PATCH] fix(isvc): post testing (#603) * fix(isvc): bugs from local testing Signed-off-by: Alessio Pragliola * chore(isvc): nit rename skip tls verify var in tests Signed-off-by: Alessio Pragliola * fix(isvc): wrongly overriding the custom http client config Signed-off-by: Alessio Pragliola * feat(isvc): make skip tls verify controller wise Signed-off-by: Alessio Pragliola --------- Signed-off-by: Alessio Pragliola --- pkg/inferenceservice-controller/controller.go | 45 +++++++++++++++---- pkg/inferenceservice-controller/suite_test.go | 6 ++- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/pkg/inferenceservice-controller/controller.go b/pkg/inferenceservice-controller/controller.go index a66a8a9d..fc4ac445 100644 --- a/pkg/inferenceservice-controller/controller.go +++ b/pkg/inferenceservice-controller/controller.go @@ -2,6 +2,7 @@ package inferenceservicecontroller import ( "context" + "crypto/tls" "fmt" "net/http" @@ -18,7 +19,7 @@ import ( type InferenceServiceController struct { client client.Client - customHTTPClient *http.Client + httpClient *http.Client log logr.Logger bearerToken string inferenceServiceIDLabel string @@ -26,14 +27,36 @@ type InferenceServiceController struct { modelVersionIDLabel string modelRegistryNamespaceLabel string modelRegistryNameLabel string - modelRegistryURLLabel string + modelRegistryURLAnnotation string modelRegistryFinalizer string defaultModelRegistryNamespace string } -func NewInferenceServiceController(client client.Client, log logr.Logger, bearerToken, isIDLabel, regModelIDLabel, modelVerIDLabel, mrNamespaceLabel, mrNameLabel, mrURLLabel, mrFinalizer, defaultMRNamespace string) *InferenceServiceController { +func NewInferenceServiceController( + client client.Client, + log logr.Logger, + skipTLSVerify bool, + bearerToken, + isIDLabel, + regModelIDLabel, + modelVerIDLabel, + mrNamespaceLabel, + mrNameLabel, + mrURLAnnotation, + mrFinalizer, + defaultMRNamespace string, +) *InferenceServiceController { + httpClient := http.DefaultClient + + if skipTLSVerify { + httpClient.Transport = &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + } + return &InferenceServiceController{ client: client, + httpClient: httpClient, log: log, bearerToken: bearerToken, inferenceServiceIDLabel: isIDLabel, @@ -41,14 +64,14 @@ func NewInferenceServiceController(client client.Client, log logr.Logger, bearer modelVersionIDLabel: modelVerIDLabel, modelRegistryNamespaceLabel: mrNamespaceLabel, modelRegistryNameLabel: mrNameLabel, - modelRegistryURLLabel: mrURLLabel, + modelRegistryURLAnnotation: mrURLAnnotation, modelRegistryFinalizer: mrFinalizer, defaultModelRegistryNamespace: defaultMRNamespace, } } func (r *InferenceServiceController) OverrideHTTPClient(client *http.Client) { - r.customHTTPClient = client + r.httpClient = client } // Reconcile performs the reconciliation of the model registry based on Kubeflow InferenceService CRs @@ -80,14 +103,20 @@ func (r *InferenceServiceController) Reconcile(ctx context.Context, req ctrl.Req registeredModelId, okRegisteredModelId := isvc.Labels[r.registeredModelIDLabel] modelVersionId := isvc.Labels[r.modelVersionIDLabel] mrName, okMrName := isvc.Labels[r.modelRegistryNameLabel] - mrUrl := isvc.Labels[r.modelRegistryURLLabel] + mrUrl, okMrUrl := isvc.Annotations[r.modelRegistryURLAnnotation] - if !okMrIsvcId && !okRegisteredModelId && !okMrName { + if !okMrIsvcId && !okRegisteredModelId { // Early check: no model registry specific labels set in the ISVC, ignore the CR log.Error(fmt.Errorf("missing model registry specific label, unable to link ISVC to Model Registry, skipping InferenceService"), "Stop ModelRegistry InferenceService reconciliation") return ctrl.Result{}, nil } + if !okMrName && !okMrUrl { + // Early check: it's required to have the model registry name or url set in the ISVC + log.Error(fmt.Errorf("missing model registry name or url, unable to link ISVC to Model Registry, skipping InferenceService"), "Stop ModelRegistry InferenceService reconciliation") + return ctrl.Result{}, nil + } + if mrNSFromISVC, ok := isvc.Labels[r.modelRegistryNamespaceLabel]; ok { mrNamespace = mrNSFromISVC } @@ -210,7 +239,7 @@ func (r *InferenceServiceController) initModelRegistryService(ctx context.Contex } cfg := &openapi.Configuration{ - HTTPClient: r.customHTTPClient, + HTTPClient: r.httpClient, Servers: openapi.ServerConfigurations{ { URL: url, diff --git a/pkg/inferenceservice-controller/suite_test.go b/pkg/inferenceservice-controller/suite_test.go index cbfd31bb..ab951b06 100644 --- a/pkg/inferenceservice-controller/suite_test.go +++ b/pkg/inferenceservice-controller/suite_test.go @@ -41,7 +41,8 @@ const ( modelVersionIDLabel = "modelregistry.kubeflow.org/model-version-id" namespaceLabel = "modelregistry.kubeflow.org/namespace" nameLabel = "modelregistry.kubeflow.org/name" - urlLabel = "modelregistry.kubeflow.org/url" + skipTLSVerify = true + urlAnnotation = "modelregistry.kubeflow.org/url" finalizerLabel = "modelregistry.kubeflow.org/finalizer" defaultNamespace = "default" accessToken = "" @@ -135,13 +136,14 @@ var _ = BeforeSuite(func() { inferenceServiceController := inferenceservicecontroller.NewInferenceServiceController( cli, ctrl.Log.WithName("controllers").WithName("ModelRegistry-InferenceService-Controller"), + skipTLSVerify, accessToken, inferenceServiceIDLabel, registeredModelIDLabel, modelVersionIDLabel, namespaceLabel, nameLabel, - urlLabel, + urlAnnotation, finalizerLabel, defaultNamespace, )