Remove redundant code which is confusing (kserve#528)

SeldonIO · Nov 8, 2019 · 948d1bc · 948d1bc
1 parent 1792286
commit 948d1bc
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 34 deletions.
diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
@@ -23,7 +23,6 @@ import (
 
 	"k8s.io/api/admissionregistration/v1beta1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"knative.dev/serving/pkg/network"
 )
 
 // KFServing Constants
@@ -202,6 +201,13 @@ func PredictorURL(metadata v1.ObjectMeta, isCanary bool) string {
 	if isCanary {
 		serviceName = CanaryPredictorServiceName(metadata.Name)
 	}
-	serviceHostname := network.GetServiceHostname(serviceName, metadata.Namespace)
-	return fmt.Sprintf("http://%s", serviceHostname)
+	return fmt.Sprintf("%s.%s", serviceName, metadata.Namespace)
+}
+
+func TransformerURL(metadata v1.ObjectMeta, isCanary bool) string {
+	serviceName := DefaultTransformerServiceName(metadata.Name)
+	if isCanary {
+		serviceName = CanaryTransformerServiceName(metadata.Name)
+	}
+	return fmt.Sprintf("%s.%s", serviceName, metadata.Namespace)
 }
diff --git a/pkg/controller/inferenceservice/resources/knative/service.go b/pkg/controller/inferenceservice/resources/knative/service.go
@@ -25,7 +25,7 @@ import (
 	"github.com/kubeflow/kfserving/pkg/constants"
 	"github.com/kubeflow/kfserving/pkg/controller/inferenceservice/resources/credentials"
 	"github.com/kubeflow/kfserving/pkg/utils"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/serving/pkg/apis/autoscaling"
 	"knative.dev/serving/pkg/apis/serving"
@@ -87,21 +87,14 @@ func (c *ServiceBuilder) CreateInferenceServiceComponent(isvc *v1alpha2.Inferenc
 		return c.CreateTransformerService(serviceName, isvc.ObjectMeta, transformerSpec, isCanary)
 	case constants.Explainer:
 		explainerSpec := isvc.Spec.Default.Explainer
-		predictorService := constants.DefaultPredictorServiceName(isvc.Name) + "." + isvc.Namespace
+		predictorService := constants.PredictorURL(isvc.ObjectMeta, isCanary)
 		if isvc.Spec.Default.Transformer != nil {
-			predictorService = constants.DefaultTransformerServiceName(isvc.Name) + "." + isvc.Namespace
-		}
-		if isCanary {
-			explainerSpec = isvc.Spec.Canary.Explainer
-			predictorService = constants.CanaryPredictorServiceName(isvc.Name) + "." + isvc.Namespace
-			if isvc.Spec.Canary.Transformer != nil {
-				predictorService = constants.CanaryTransformerServiceName(isvc.Name) + "." + isvc.Namespace
-			}
+			predictorService = constants.TransformerURL(isvc.ObjectMeta, isCanary)
 		}
 		if explainerSpec == nil {
 			return nil, nil
 		}
-		return c.CreateExplainerService(serviceName, isvc.ObjectMeta, explainerSpec, predictorService, isCanary)
+		return c.CreateExplainerService(serviceName, isvc.ObjectMeta, explainerSpec, predictorService)
 	}
 	return nil, fmt.Errorf("Invalid Component")
 }
@@ -168,20 +161,7 @@ func (c *ServiceBuilder) CreateTransformerService(name string, metadata metav1.O
 	if err != nil {
 		return nil, err
 	}
-
-	predictorHostName := fmt.Sprintf("%s.%s", constants.DefaultPredictorServiceName(metadata.Name), metadata.Namespace)
-
-	if isCanary {
-		predictorHostName = fmt.Sprintf("%s.%s", constants.CanaryPredictorServiceName(metadata.Name), metadata.Namespace)
-	}
-	container := transformerSpec.Custom.Container
-	predefinedArgs := []string{
-		constants.ArgumentModelName,
-		metadata.Name,
-		constants.ArgumentPredictorHost,
-		predictorHostName,
-	}
-	container.Args = append(container.Args, predefinedArgs...)
+	container := transformerSpec.GetContainerSpec(metadata, isCanary)
 	service := &knservingv1alpha1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -205,7 +185,7 @@ func (c *ServiceBuilder) CreateTransformerService(name string, metadata metav1.O
 							PodSpec: v1.PodSpec{
 								ServiceAccountName: transformerSpec.ServiceAccountName,
 								Containers: []v1.Container{
-									container,
+									*container,
 								},
 							},
 						},
@@ -227,7 +207,7 @@ func (c *ServiceBuilder) CreateTransformerService(name string, metadata metav1.O
 	return service, nil
 }
 
-func (c *ServiceBuilder) CreateExplainerService(name string, metadata metav1.ObjectMeta, explainerSpec *v1alpha2.ExplainerSpec, predictorService string, isCanary bool) (*knservingv1alpha1.Service, error) {
+func (c *ServiceBuilder) CreateExplainerService(name string, metadata metav1.ObjectMeta, explainerSpec *v1alpha2.ExplainerSpec, predictorService string) (*knservingv1alpha1.Service, error) {
 	annotations, err := c.buildAnnotations(metadata, explainerSpec.MinReplicas, explainerSpec.MaxReplicas)
 	if err != nil {
 		return nil, err

diff --git a/pkg/controller/inferenceservice/resources/knative/service_test.go b/pkg/controller/inferenceservice/resources/knative/service_test.go
@@ -866,8 +866,7 @@ func TestExplainerToKnativeService(t *testing.T) {
 			constants.DefaultExplainerServiceName(scenario.inferenceService.Name),
 			scenario.inferenceService.ObjectMeta,
 			scenario.inferenceService.Spec.Default.Explainer,
-			constants.DefaultPredictorServiceName(scenario.inferenceService.Name)+"."+scenario.inferenceService.Namespace,
-			false)
+			constants.DefaultPredictorServiceName(scenario.inferenceService.Name)+"."+scenario.inferenceService.Namespace)
 		if err != nil {
 			t.Errorf("Test %q unexpected error %s", name, err.Error())
 		}
@@ -881,8 +880,7 @@ func TestExplainerToKnativeService(t *testing.T) {
 				constants.CanaryTransformerServiceName(isvc.Name),
 				scenario.inferenceService.ObjectMeta,
 				scenario.inferenceService.Spec.Canary.Explainer,
-				constants.CanaryPredictorServiceName(scenario.inferenceService.Name)+"."+scenario.inferenceService.Namespace,
-				true)
+				constants.CanaryPredictorServiceName(scenario.inferenceService.Name)+"."+scenario.inferenceService.Namespace)
 			if err != nil {
 				t.Errorf("Test %q unexpected error %s", name, err.Error())
 			}