Merge pull request #955 from cliveseldon/issue_954_explainer_args

Fix explainer and endpoint defaulting in webhook
SeldonIO · Oct 16, 2019 · c3e7e1d · c3e7e1d
2 parents 83f2e65 + 7feff6d
commit c3e7e1d
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 14 deletions.
diff --git a/operator/api/v1alpha2/seldondeployment_webhook.go b/operator/api/v1alpha2/seldondeployment_webhook.go
@@ -273,6 +273,19 @@ func (r *SeldonDeployment) DefaultSeldonDeployment() {
 					}
 				}
 
+				// Add a default REST endpoint if none provided
+				// pu needs to have an endpoint as engine reads it from SDep in order to direct graph traffic
+				// probes etc will be added later by controller
+				if pu.Endpoint == nil {
+					pu.Endpoint = &Endpoint{Type: REST}
+				}
+				var portType string
+				if pu.Endpoint.Type == GRPC {
+					portType = "grpc"
+				} else {
+					portType = "http"
+				}
+
 				SetImageNameForPrepackContainer(pu, con)
 
 				// if new Add container to componentSpecs
@@ -295,18 +308,6 @@ func (r *SeldonDeployment) DefaultSeldonDeployment() {
 
 				getUpdatePortNumMap(con.Name, &nextPortNum, portMap)
 				portNum := portMap[pu.Name]
-				// Add a default REST endpoint if none provided
-				// pu needs to have an endpoint as engine reads it from SDep in order to direct graph traffic
-				// probes etc will be added later by controller
-				if pu.Endpoint == nil {
-					pu.Endpoint = &Endpoint{Type: REST}
-				}
-				var portType string
-				if pu.Endpoint.Type == GRPC {
-					portType = "grpc"
-				} else {
-					portType = "http"
-				}
 
 				if con != nil {
 					existingPort := GetPort(portType, con.Ports)
@@ -333,7 +334,6 @@ func (r *SeldonDeployment) DefaultSeldonDeployment() {
 					if _, hasSeparateEnginePod := r.Spec.Annotations[ANNOTATION_SEPARATE_ENGINE]; !hasSeparateEnginePod {
 						pu.Endpoint.ServiceHost = "localhost"
 					} else {
-						//FIXME
 						containerServiceValue := GetContainerServiceName(r, p, con)
 						pu.Endpoint.ServiceHost = containerServiceValue + "." + r.ObjectMeta.Namespace + ".svc.cluster.local."
 					}

diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go
@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"knative.dev/pkg/apis/istio/common/v1alpha1"
 	istio "knative.dev/pkg/apis/istio/v1alpha3"
+	"sort"
 	"strconv"
 	"strings"
 )
@@ -116,7 +117,15 @@ func createExplainer(r *SeldonDeploymentReconciler, mlDep *machinelearningv1alph
 		if p.Explainer.Type == "anchor_images" {
 			explainerContainer.Args = append(explainerContainer.Args, "--tf_data_type=float32")
 		}
-		for k, v := range p.Explainer.Config {
+
+		// Order explainer config map keys
+		var keys []string
+		for k, _ := range p.Explainer.Config {
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		for _, k := range keys {
+			v := p.Explainer.Config[k]
 			//remote files in model location should get downloaded by initializer
 			if p.Explainer.ModelUri != "" {
 				v = strings.Replace(v, p.Explainer.ModelUri, "/mnt/models", 1)