From d816da7ed807237a5a1b4541f423a1b6fd13d6af Mon Sep 17 00:00:00 2001 From: Modassar Rana Date: Mon, 2 Dec 2024 21:54:57 +0530 Subject: [PATCH 1/2] build: fix workflow details making it compatible on s390x (#602) * Update Makefile Signed-off-by: root * Update install_protoc.sh Signed-off-by: root --------- Signed-off-by: root --- Makefile | 8 ++++---- scripts/install_protoc.sh | 10 +++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 5566a331d..972072b7b 100644 --- a/Makefile +++ b/Makefile @@ -203,8 +203,8 @@ gen: deps gen/grpc gen/openapi gen/openapi-server gen/converter .PHONY: lint lint: - ${GOLANGCI_LINT} run main.go - ${GOLANGCI_LINT} run cmd/... internal/... ./pkg/... + ${GOLANGCI_LINT} run main.go --timeout 3m + ${GOLANGCI_LINT} run cmd/... internal/... ./pkg/... --timeout 3m .PHONY: test test: gen bin/envtest @@ -256,12 +256,12 @@ ifeq ($(DOCKER),docker) # docker uses builder containers - $(DOCKER) buildx rm model-registry-builder $(DOCKER) buildx create --use --name model-registry-builder --platform=$(PLATFORMS) - $(DOCKER) buildx build --push --platform=$(PLATFORMS) --tag ${IMG} -f ${DOCKERFILE} . + $(DOCKER) buildx build --push --platform=$(PLATFORMS) --tag ${IMG}:$(IMG_VERSION) -f ${DOCKERFILE} . $(DOCKER) buildx rm model-registry-builder else ifeq ($(DOCKER),podman) # podman uses image manifests $(DOCKER) manifest create -a ${IMG} - $(DOCKER) buildx build --platform=$(PLATFORMS) --manifest ${IMG} -f ${DOCKERFILE} . + $(DOCKER) buildx build --platform=$(PLATFORMS) --manifest ${IMG}:$(IMG_VERSION) -f ${DOCKERFILE} . $(DOCKER) manifest push ${IMG} $(DOCKER) manifest rm ${IMG} else diff --git a/scripts/install_protoc.sh b/scripts/install_protoc.sh index 6f8fc3b50..fd66de66e 100755 --- a/scripts/install_protoc.sh +++ b/scripts/install_protoc.sh @@ -7,9 +7,13 @@ if [[ ${OSTYPE,,} =~ darwin ]]; then OS="osx" fi -ARCH="x86_64" -if [[ $(uname -m) =~ arm ]]; then - ARCH="aarch_64" +ARCH=$(uname -m) +if [[ "$ARCH" == "arm"* ]]; then + ARCH="aarch_64" +elif [[ "$ARCH" == "s390x" ]]; then + ARCH="s390_64" +elif [[ "$ARCH" == "ppc64le" ]] ; then + ARCH="ppcle_64" fi PROJECT_ROOT=$(realpath "$(dirname "$0")"/..) 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 2/2] 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 a66a8a9d2..fc4ac4454 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 cbfd31bb4..ab951b068 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, )