Fixes issues with Validation not applying to transformers/explainers …

…and unified predictor concept with explainer/transformer (kubeflow#397)

* Fixes issues with Validation not applying to transformers/explainers

* Added Validation classes

* more changes

* Standardized terminology around predictors

* Renames

* Added predictor

* other renames and fixes

* adding swagger

* Moved more validation logic to predictor

* Fixed validation

* Fixed validation

* Fixed configmap

config/default/configmap/kfservice.yaml

@@ -4,7 +4,7 @@ metadata:
   name: kfservice-config
   namespace: kfserving-system
 data:
-  frameworks: |-
+  predictors: |-
     {
         "tensorflow": {
             "image": "tensorflow/serving"

docs/apis/README.md

@@ -479,11 +479,11 @@ ExplainerConfig
 </tr>
 </tbody>
 </table>
-<h3 id="serving.kubeflow.org/v1alpha2.FrameworkConfig">FrameworkConfig
+<h3 id="serving.kubeflow.org/v1alpha2.PredictorConfig">PredictorConfig
 </h3>
 <p>
 (<em>Appears on:</em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworksConfig">FrameworksConfig</a>)
+<a href="#serving.kubeflow.org/v1alpha2.PredictorsConfig">PredictorsConfig</a>)
 </p>
 <p>
 </p>
@@ -507,11 +507,11 @@ string
 </tr>
 </tbody>
 </table>
-<h3 id="serving.kubeflow.org/v1alpha2.FrameworkHandler">FrameworkHandler
+<h3 id="serving.kubeflow.org/v1alpha2.Predictor">Predictor
 </h3>
 <p>
 </p>
-<h3 id="serving.kubeflow.org/v1alpha2.FrameworksConfig">FrameworksConfig
+<h3 id="serving.kubeflow.org/v1alpha2.PredictorsConfig">PredictorsConfig
 </h3>
 <p>
 </p>
@@ -527,8 +527,8 @@ string
 <td>
 <code>tensorflow</code></br>
 <em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworkConfig">
-FrameworkConfig
+<a href="#serving.kubeflow.org/v1alpha2.PredictorConfig">
+PredictorConfig
 </a>
 </em>
 </td>
@@ -539,8 +539,8 @@ FrameworkConfig
 <td>
 <code>tensorrt</code></br>
 <em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworkConfig">
-FrameworkConfig
+<a href="#serving.kubeflow.org/v1alpha2.PredictorConfig">
+PredictorConfig
 </a>
 </em>
 </td>
@@ -551,8 +551,8 @@ FrameworkConfig
 <td>
 <code>xgboost</code></br>
 <em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworkConfig">
-FrameworkConfig
+<a href="#serving.kubeflow.org/v1alpha2.PredictorConfig">
+PredictorConfig
 </a>
 </em>
 </td>
@@ -563,8 +563,8 @@ FrameworkConfig
 <td>
 <code>sklearn</code></br>
 <em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworkConfig">
-FrameworkConfig
+<a href="#serving.kubeflow.org/v1alpha2.PredictorConfig">
+PredictorConfig
 </a>
 </em>
 </td>
@@ -575,8 +575,8 @@ FrameworkConfig
 <td>
 <code>pytorch</code></br>
 <em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworkConfig">
-FrameworkConfig
+<a href="#serving.kubeflow.org/v1alpha2.PredictorConfig">
+PredictorConfig
 </a>
 </em>
 </td>
@@ -587,8 +587,8 @@ FrameworkConfig
 <td>
 <code>onnx</code></br>
 <em>
-<a href="#serving.kubeflow.org/v1alpha2.FrameworkConfig">
-FrameworkConfig
+<a href="#serving.kubeflow.org/v1alpha2.PredictorConfig">
+PredictorConfig
 </a>
 </em>
 </td>

pkg/apis/serving/v1alpha2/explainer.go

@@ -15,43 +15,42 @@ package v1alpha2
 
 import (
 	"fmt"
+
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/klog"
 )
 
-type ExplainerHandler interface {
+type Explainer interface {
 	GetStorageUri() string
-	CreateExplainerServingContainer(modelName string, predictorHost string, config *ExplainersConfig) *v1.Container
+	CreateExplainerContainer(modelName string, predictorHost string, config *ExplainersConfig) *v1.Container
 	ApplyDefaults()
 	Validate() error
 }
 
 const (
-	// ExactlyOneModelSpecViolatedError is a known error message
-	ExactlyOneExplainerSpecViolatedError = "Exactly one of [Custom, Alibi] must be specified in ExplainerSpec"
-	// AtLeastOneModelSpecViolatedError is a known error message
-	AtLeastOneExplainerSpecViolatedError = "At least one of [Custom, Alibi] must be specified in ExplainerSpec"
+	// ExactlyOneExplainerViolatedError is a known error message
+	ExactlyOneExplainerViolatedError = "Exactly one of [Custom, Alibi] must be specified in ExplainerSpec"
 )
 
 // Returns a URI to the explainer. This URI is passed to the model-initializer via the ModelInitializerSourceUriInternalAnnotationKey
 func (m *ExplainerSpec) GetStorageUri() string {
 	return getExplainerHandler(m).GetStorageUri()
 }
 
-func (m *ExplainerSpec) CreateExplainerServingContainer(modelName string, predictorHost string, config *ExplainersConfig) *v1.Container {
-	return getExplainerHandler(m).CreateExplainerServingContainer(modelName, predictorHost, config)
+func (m *ExplainerSpec) CreateExplainerContainer(modelName string, predictorHost string, config *ExplainersConfig) *v1.Container {
+	return getExplainerHandler(m).CreateExplainerContainer(modelName, predictorHost, config)
 }
 
 func (m *ExplainerSpec) ApplyDefaults() {
 	getExplainerHandler(m).ApplyDefaults()
 }
 
 func (m *ExplainerSpec) Validate() error {
-	handler, err := makeExplainerHandler(m)
+	explainer, err := makeExplainer(m)
 	if err != nil {
 		return err
 	}
-	return handler.Validate()
+	return explainer.Validate()
 }
 
 type ExplainerConfig struct {
@@ -63,29 +62,25 @@ type ExplainersConfig struct {
 	AlibiExplainer ExplainerConfig `json:"alibi,omitempty"`
 }
 
-func getExplainerHandler(modelSpec *ExplainerSpec) ExplainerHandler {
-	handler, err := makeExplainerHandler(modelSpec)
+func getExplainerHandler(modelSpec *ExplainerSpec) Explainer {
+	explainer, err := makeExplainer(modelSpec)
 	if err != nil {
 		klog.Fatal(err)
 	}
 
-	return handler
+	return explainer
 }
 
-func makeExplainerHandler(explainerSpec *ExplainerSpec) (ExplainerHandler, error) {
-	handlers := []ExplainerHandler{}
+func makeExplainer(explainerSpec *ExplainerSpec) (Explainer, error) {
+	handlers := []Explainer{}
 	if explainerSpec.Custom != nil {
 		handlers = append(handlers, explainerSpec.Custom)
 	}
 	if explainerSpec.Alibi != nil {
 		handlers = append(handlers, explainerSpec.Alibi)
 	}
-
-	if len(handlers) == 0 {
-		return nil, fmt.Errorf(AtLeastOneExplainerSpecViolatedError)
-	}
 	if len(handlers) != 1 {
-		return nil, fmt.Errorf(ExactlyOneExplainerSpecViolatedError)
+		return nil, fmt.Errorf(ExactlyOneExplainerViolatedError)
 	}
 	return handlers[0], nil
 }

pkg/apis/serving/v1alpha2/explainer_alibi.go

@@ -2,10 +2,11 @@ package v1alpha2
 
 import (
 	"fmt"
+	"strings"
+
 	"github.com/kubeflow/kfserving/pkg/constants"
 	"github.com/kubeflow/kfserving/pkg/utils"
 	v1 "k8s.io/api/core/v1"
-	"strings"
 )
 
 var (
@@ -21,15 +22,15 @@ func (s *AlibiExplainerSpec) GetStorageUri() string {
 	return s.StorageURI
 }
 
-func (s *AlibiExplainerSpec) CreateExplainerServingContainer(modelName string, predictorHost string, config *ExplainersConfig) *v1.Container {
+func (s *AlibiExplainerSpec) CreateExplainerContainer(modelName string, predictorHost string, config *ExplainersConfig) *v1.Container {
 	imageName := AlibiImageName
 	if config.AlibiExplainer.ContainerImage != "" {
 		imageName = config.AlibiExplainer.ContainerImage
 	}
 
 	var args = []string{
-		constants.ModelServerArgsModelName, modelName,
-		constants.ModelServerArgsPredictorHost, predictorHost,
+		constants.ArgumentModelName, modelName,
+		constants.ArgumentPredictorHost, predictorHost,
 	}
 
 	if s.StorageURI != "" {

pkg/apis/serving/v1alpha2/framework_custom.go

@@ -32,10 +32,10 @@ func (c *CustomSpec) GetStorageUri() string {
 	return ""
 }
 
-func (c *CustomSpec) CreateModelServingContainer(modelName string, config *FrameworksConfig) *v1.Container {
+func (c *CustomSpec) GetContainer(modelName string, config *PredictorsConfig) *v1.Container {
 	return &c.Container
 }
-func (c *CustomSpec) CreateExplainerServingContainer(modelName string, predictUrl string, config *ExplainersConfig) *v1.Container {
+func (c *CustomSpec) CreateExplainerContainer(modelName string, predictUrl string, config *ExplainersConfig) *v1.Container {
 	return &c.Container
 }
 

pkg/apis/serving/v1alpha2/framework_onnx.go

@@ -39,7 +39,7 @@ func (s *ONNXSpec) GetStorageUri() string {
 	return s.StorageURI
 }
 
-func (s *ONNXSpec) CreateModelServingContainer(modelName string, config *FrameworksConfig) *v1.Container {
+func (s *ONNXSpec) GetContainer(modelName string, config *PredictorsConfig) *v1.Container {
 	imageName := ONNXServingImageName
 	if config.ONNX.ContainerImage != "" {
 		imageName = config.ONNX.ContainerImage