From c9ee03bdae8ed15c9c6dcec5b60eb4d33660a3d5 Mon Sep 17 00:00:00 2001 From: oona Date: Thu, 25 Apr 2024 17:18:13 +0800 Subject: [PATCH] Feature: support multi hosts in vs (#5430) * Update controller_utils.go add GetAnnotations() to support multi hosts * Update istio.go add env ANNOTATION_ISTIO_HOST_SEPARATOR to support multi hosts * Update seldondeployment_controller.go Update controller to support multi hosts * Update seldondeployment_explainers.go Update explianer to support multi hosts * Update istio.go Remove seperator defination * Update controller_utils.go Rename GetAnnotation to HostsFromAnnotation, and remove the redundant logic * Update seldondeployment_explainers.go * Update seldondeployment_controller.go * Update operator/controllers/seldondeployment_controller.go * add test and trimp whitespaces --------- Co-authored-by: Rafal Skolasinski --- .../seldondeployment_controller.go | 2 +- .../seldondeployment_explainers.go | 4 +- .../controllers/utils/controller_utils.go | 14 ++++ .../utils/controller_utils_test.go | 69 +++++++++++++++++++ 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/operator/controllers/seldondeployment_controller.go b/operator/controllers/seldondeployment_controller.go index 7e59d4c539..080b9cd376 100644 --- a/operator/controllers/seldondeployment_controller.go +++ b/operator/controllers/seldondeployment_controller.go @@ -262,7 +262,7 @@ func createIstioResources(mlDep *machinelearningv1.SeldonDeployment, Namespace: namespace, }, Spec: istio_networking.VirtualService{ - Hosts: []string{utils2.GetAnnotation(mlDep, ANNOTATION_ISTIO_HOST, "*")}, + Hosts: utils2.HostsFromAnnotation(mlDep, ANNOTATION_ISTIO_HOST, "*"), Gateways: []string{utils2.GetAnnotation(mlDep, ANNOTATION_ISTIO_GATEWAY, istio_gateway)}, Http: []*istio_networking.HTTPRoute{ { diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go index 8df3920418..46ca05f9a2 100644 --- a/operator/controllers/seldondeployment_explainers.go +++ b/operator/controllers/seldondeployment_explainers.go @@ -328,7 +328,7 @@ func createExplainerIstioResources(pSvcName string, p *machinelearningv1.Predict Namespace: namespace, }, Spec: istio_networking.VirtualService{ - Hosts: []string{utils2.GetAnnotation(mlDep, ANNOTATION_ISTIO_HOST, "*")}, + Hosts: utils2.HostsFromAnnotation(mlDep, ANNOTATION_ISTIO_HOST, "*"), Gateways: []string{utils2.GetAnnotation(mlDep, ANNOTATION_ISTIO_GATEWAY, istio_gateway)}, Http: []*istio_networking.HTTPRoute{ { @@ -349,7 +349,7 @@ func createExplainerIstioResources(pSvcName string, p *machinelearningv1.Predict Namespace: namespace, }, Spec: istio_networking.VirtualService{ - Hosts: []string{utils2.GetAnnotation(mlDep, ANNOTATION_ISTIO_HOST, "*")}, + Hosts: utils2.HostsFromAnnotation(mlDep, ANNOTATION_ISTIO_HOST, "*"), Gateways: []string{utils2.GetAnnotation(mlDep, ANNOTATION_ISTIO_GATEWAY, istio_gateway)}, Http: []*istio_networking.HTTPRoute{ { diff --git a/operator/controllers/utils/controller_utils.go b/operator/controllers/utils/controller_utils.go index a3a27f116b..62a1e17a3f 100644 --- a/operator/controllers/utils/controller_utils.go +++ b/operator/controllers/utils/controller_utils.go @@ -51,6 +51,20 @@ func GetAnnotation(mlDep *machinelearningv1.SeldonDeployment, annotationKey stri } } +// Get an annotation array for hosts from the Seldon Deployment given by annotationKey or return the array with fallback. +func HostsFromAnnotation(mlDep *machinelearningv1.SeldonDeployment, annotationKey string, fallback string) []string { + annotation := GetAnnotation(mlDep, annotationKey, fallback) + if strings.Contains(annotation, ",") { + hosts := strings.Split(annotation, ",") + for i, host := range hosts { + hosts[i] = strings.TrimSpace(host) + } + return hosts + } else { + return []string{strings.TrimSpace(annotation)} + } +} + // get annotations that start with seldon.io/engine func GetEngineEnvAnnotations(mlDep *machinelearningv1.SeldonDeployment) []corev1.EnvVar { diff --git a/operator/controllers/utils/controller_utils_test.go b/operator/controllers/utils/controller_utils_test.go index 80c615aa0b..405ce158cf 100644 --- a/operator/controllers/utils/controller_utils_test.go +++ b/operator/controllers/utils/controller_utils_test.go @@ -1,12 +1,16 @@ package utils import ( + "testing" + . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" machinelearningv1 "github.com/seldonio/seldon-core/operator/apis/machinelearning.seldon.io/v1" ) +const HOST_ANNOTATION = "host-annotation" + var _ = Describe("Controller utils", func() { DescribeTable( "isEmptyExplainer", @@ -23,3 +27,68 @@ var _ = Describe("Controller utils", func() { ), ) }) + +func TestHostsFromAnnotation(t *testing.T) { + var key = HOST_ANNOTATION + var fallback = "*" + + tests := []struct { + name string + mldep *machinelearningv1.SeldonDeployment + want []string + }{ + { + name: "No host", + mldep: &machinelearningv1.SeldonDeployment{ + Spec: machinelearningv1.SeldonDeploymentSpec{ + Annotations: map[string]string{}, + }, + }, + want: []string{fallback}, + }, + { + name: "Single host", + mldep: &machinelearningv1.SeldonDeployment{ + Spec: machinelearningv1.SeldonDeploymentSpec{ + Annotations: map[string]string{ + key: "prod.svc.cluster.local", + }, + }, + }, + want: []string{"prod.svc.cluster.local"}, + }, + { + name: "Multiple hosts", + mldep: &machinelearningv1.SeldonDeployment{ + Spec: machinelearningv1.SeldonDeploymentSpec{ + Annotations: map[string]string{ + key: "prod.svc.cluster.local,dev.svc.cluster.local", + }, + }, + }, + want: []string{"prod.svc.cluster.local", "dev.svc.cluster.local"}, + }, + { + name: "Multiple hosts with space", + mldep: &machinelearningv1.SeldonDeployment{ + Spec: machinelearningv1.SeldonDeploymentSpec{ + Annotations: map[string]string{ + key: "prod.svc.cluster.local, dev.svc.cluster.local", + }, + }, + }, + want: []string{"prod.svc.cluster.local", "dev.svc.cluster.local"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hosts := HostsFromAnnotation(tt.mldep, key, fallback) + for i, host := range hosts { + if host != tt.want[i] { + t.Errorf("got: %v; want %v", host, tt.want[i]) + } + } + }) + } +}