Custom container (kubeflow#40)

* Updates for vendor and fix rbac issue * Add a development kustomize overlay * revery dep ensure * initial custom handling and validation * Update tests and validation for custom kfservice * remove debugging stmts * push nil check into custom validator * add custom example
magdalenakuhn17 · Apr 27, 2019 · 34c6b75 · 34c6b75
1 parent 89d1b3d
commit 34c6b75
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 27 deletions.
diff --git a/Gopkg.toml b/Gopkg.toml
@@ -29,7 +29,7 @@ version="v1.4.7"
 
 [[constraint]]
   name = "github.com/knative/serving"
-  version = "v0.5.0"
+  revision = "a44e3578ce27cc1455b3e1c7f74b09b82dda57f5"
 
 [[override]]
   name="sigs.k8s.io/controller-runtime"
@@ -38,8 +38,8 @@ version="v1.4.7"
 
 [[override]]
   name = "github.com/knative/pkg"
-  # HEAD as of 2019-04-02
-  revision = "281cda84ceb316a70c2eafc5b3250a4b72c2d468"
+  # HEAD as of 2019-04-19
+  revision = "3c8c4a93547f62b542013c273068f51875b5c942"
 
 [[override]]
   name = "k8s.io/api"

diff --git a/config/samples/custom.yaml b/config/samples/custom.yaml
@@ -0,0 +1,14 @@
+apiVersion: serving.kubeflow.org/v1alpha1
+kind: KFService
+metadata:
+  labels:
+    controller-tools.k8s.io: "1.0"
+  name: service-custom-sample
+spec:
+  default:
+    custom:
+      container:
+        image: seldonio/mock_classifier:1.0
+        env:
+          - name: PREDICTIVE_UNIT_SERVICE_PORT
+            value: "8080"
diff --git a/pkg/apis/serving/v1alpha1/kfservice_validation.go b/pkg/apis/serving/v1alpha1/kfservice_validation.go
@@ -19,7 +19,9 @@ package v1alpha1
 import (
 	"fmt"
 
+	knserving "github.com/knative/serving/pkg/apis/serving"
 	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 const (
@@ -70,6 +72,7 @@ func validateKFService(kfsvc *KFService) error {
 	if err := validateCanarySpec(kfsvc.Spec.Canary); err != nil {
 		return err
 	}
+
 	return nil
 }
 
@@ -90,6 +93,9 @@ func validateDefaultSpec(defaultSpec ModelSpec) error {
 	if err := validateOneModelSpec(defaultSpec); err != nil {
 		return err
 	}
+	if err := validateContainer(defaultSpec.Custom); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -103,6 +109,9 @@ func validateCanarySpec(canarySpec *CanarySpec) error {
 	if canarySpec.TrafficPercent < 0 || canarySpec.TrafficPercent > 100 {
 		return fmt.Errorf(TrafficBoundsExceededError)
 	}
+	if err := validateContainer(canarySpec.Custom); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -125,3 +134,14 @@ func validateOneModelSpec(modelSpec ModelSpec) error {
 	}
 	return nil
 }
+
+func validateContainer(customSpec *CustomSpec) error {
+	if customSpec == nil {
+		return nil
+	}
+	knativeErrs := knserving.ValidateContainer(customSpec.Container, sets.String{})
+	if knativeErrs != nil {
+		return fmt.Errorf("Custom: " + knativeErrs.Error())
+	}
+	return nil
+}
diff --git a/pkg/apis/serving/v1alpha1/kfservice_validation_test.go b/pkg/apis/serving/v1alpha1/kfservice_validation_test.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"fmt"
+	"k8s.io/api/core/v1"
 	"testing"
 
 	"github.com/onsi/gomega"
@@ -75,3 +77,32 @@ func TestBadReplicaValues(t *testing.T) {
 	kfsvc.Spec.MaxReplicas = 1
 	g.Expect(kfsvc.ValidateCreate()).Should(gomega.MatchError(MinReplicasShouldBeLessThanMaxError))
 }
+
+func TestCustomBadFields(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	kfsvc := TFExampleKFService.DeepCopy()
+	kfsvc.Spec.Default.Tensorflow = nil
+	kfsvc.Spec.Default.Custom = &CustomSpec{
+		v1.Container{
+			Name:      "foo",
+			Image:     "custom:0.1",
+			Stdin:     true,
+			StdinOnce: true,
+		},
+	}
+	g.Expect(kfsvc.ValidateCreate()).Should(gomega.MatchError("Custom: must not set the field(s): name, stdin, stdinOnce"))
+}
+
+func TestCustomOK(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	kfsvc := TFExampleKFService.DeepCopy()
+	kfsvc.Spec.Default.Tensorflow = nil
+	kfsvc.Spec.Default.Custom = &CustomSpec{
+		v1.Container{
+			Image: "custom:0.1",
+		},
+	}
+	err := kfsvc.ValidateCreate()
+	fmt.Println(err)
+	g.Expect(kfsvc.ValidateCreate()).Should(gomega.Succeed())
+}
diff --git a/pkg/controller/kfservice/kfservice_controller_test.go b/pkg/controller/kfservice/kfservice_controller_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package service
 
 import (
+	"github.com/knative/serving/pkg/apis/serving/v1beta1"
 	"github.com/kubeflow/kfserving/pkg/frameworks/tensorflow"
 	"k8s.io/api/core/v1"
 	"testing"
@@ -133,9 +134,9 @@ func TestReconcile(t *testing.T) {
 			Release: &knservingv1alpha1.ReleaseType{
 				Revisions: []string{"@latest"},
 				Configuration: knservingv1alpha1.ConfigurationSpec{
-					RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+					RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 						Spec: knservingv1alpha1.RevisionSpec{
-							Container: v1.Container{
+							Container: &v1.Container{
 								Image: tensorflow.TensorflowServingImageName + ":" +
 									instance.Spec.Default.Tensorflow.RuntimeVersion,
 								Command: []string{tensorflow.TensorflowEntrypointCommand},
@@ -218,9 +219,9 @@ func TestCanaryReconcile(t *testing.T) {
 				Revisions:      []string{canary.Status.Default.Name, "@latest"},
 				RolloutPercent: 20,
 				Configuration: knservingv1alpha1.ConfigurationSpec{
-					RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+					RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 						Spec: knservingv1alpha1.RevisionSpec{
-							Container: v1.Container{
+							Container: &v1.Container{
 								Image: tensorflow.TensorflowServingImageName + ":" +
 									canary.Spec.Canary.Tensorflow.RuntimeVersion,
 								Command: []string{tensorflow.TensorflowEntrypointCommand},
@@ -248,14 +249,12 @@ func TestCanaryReconcile(t *testing.T) {
 	updateCanary.Status.LatestReadyRevisionName = "revision-v2"
 	updateCanary.Status.Traffic = []knservingv1alpha1.TrafficTarget{
 		{
-			Name:         "candidate",
-			RevisionName: "revision-v2",
-			Percent:      20,
+			Name:          "candidate",
+			TrafficTarget: v1beta1.TrafficTarget{RevisionName: "revision-v2", Percent: 20},
 		},
 		{
-			Name:         "current",
-			RevisionName: "revision-v1",
-			Percent:      80,
+			Name:          "current",
+			TrafficTarget: v1beta1.TrafficTarget{RevisionName: "revision-v1", Percent: 80},
 		},
 	}
 	g.Expect(c.Status().Update(context.TODO(), updateCanary)).NotTo(gomega.HaveOccurred())

diff --git a/pkg/frameworks/custom/custom_container.go b/pkg/frameworks/custom/custom_container.go
@@ -0,0 +1,10 @@
+package custom
+
+import (
+	"github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha1"
+	v1 "k8s.io/api/core/v1"
+)
+
+func CreateCustomContainer(customSpec *v1alpha1.CustomSpec) *v1.Container {
+	return &customSpec.Container
+}
diff --git a/pkg/reconciler/ksvc/reconciler_test.go b/pkg/reconciler/ksvc/reconciler_test.go
@@ -37,9 +37,9 @@ func TestKnativeServiceReconcile(t *testing.T) {
 			Release: &knservingv1alpha1.ReleaseType{
 				Revisions: []string{"@latest"},
 				Configuration: knservingv1alpha1.ConfigurationSpec{
-					RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+					RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 						Spec: knservingv1alpha1.RevisionSpec{
-							Container: v1.Container{
+							Container: &v1.Container{
 								Image:   "tensorflow/serving:1.13",
 								Command: []string{tensorflow.TensorflowEntrypointCommand},
 								Args: []string{
@@ -72,9 +72,9 @@ func TestKnativeServiceReconcile(t *testing.T) {
 					Release: &knservingv1alpha1.ReleaseType{
 						Revisions: []string{"@latest"},
 						Configuration: knservingv1alpha1.ConfigurationSpec{
-							RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+							RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 								Spec: knservingv1alpha1.RevisionSpec{
-									Container: v1.Container{
+									Container: &v1.Container{
 										Image:   "tensorflow/serving:1.13",
 										Command: []string{tensorflow.TensorflowEntrypointCommand},
 										Args: []string{
@@ -102,9 +102,9 @@ func TestKnativeServiceReconcile(t *testing.T) {
 					Release: &knservingv1alpha1.ReleaseType{
 						Revisions: []string{"@latest"},
 						Configuration: knservingv1alpha1.ConfigurationSpec{
-							RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+							RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 								Spec: knservingv1alpha1.RevisionSpec{
-									Container: v1.Container{
+									Container: &v1.Container{
 										Image:   "tensorflow/serving:1.13",
 										Command: []string{tensorflow.TensorflowEntrypointCommand},
 										Args: []string{

diff --git a/pkg/reconciler/ksvc/resources/knative_service.go b/pkg/reconciler/ksvc/resources/knative_service.go
@@ -19,6 +19,7 @@ package resources
 import (
 	knservingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha1"
+	"github.com/kubeflow/kfserving/pkg/frameworks/custom"
 	"github.com/kubeflow/kfserving/pkg/frameworks/tensorflow"
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -27,6 +28,8 @@ import (
 func CreateModelServingContainer(modelName string, modelSpec *v1alpha1.ModelSpec) *v1.Container {
 	if modelSpec.Tensorflow != nil {
 		return tensorflow.CreateTensorflowContainer(modelName, modelSpec.Tensorflow)
+	} else if modelSpec.Custom != nil {
+		return custom.CreateCustomContainer(modelSpec.Custom)
 	} else {
 		//TODO(@yuzisun) handle other model types
 		return &v1.Container{}
@@ -58,9 +61,9 @@ func CreateKnativeService(kfsvc *v1alpha1.KFService) (*knservingv1alpha1.Service
 				Revisions:      revisions,
 				RolloutPercent: int(routingPercent),
 				Configuration: knservingv1alpha1.ConfigurationSpec{
-					RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+					RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 						Spec: knservingv1alpha1.RevisionSpec{
-							Container: *container,
+							Container: container,
 						},
 					},
 				},

diff --git a/pkg/reconciler/ksvc/resources/knative_service_test.go b/pkg/reconciler/ksvc/resources/knative_service_test.go
@@ -44,9 +44,9 @@ var kfsvc = &v1alpha1.KFService{
 }
 
 var ksvcConfiguration = knservingv1alpha1.ConfigurationSpec{
-	RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+	RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 		Spec: knservingv1alpha1.RevisionSpec{
-			Container: v1.Container{
+			Container: &v1.Container{
 				Image:   tensorflow.TensorflowServingImageName + ":" + kfsvc.Spec.Default.Tensorflow.RuntimeVersion,
 				Command: []string{tensorflow.TensorflowEntrypointCommand},
 				Args: []string{
@@ -61,9 +61,9 @@ var ksvcConfiguration = knservingv1alpha1.ConfigurationSpec{
 }
 
 var ksvcCanaryConfiguration = knservingv1alpha1.ConfigurationSpec{
-	RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+	RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 		Spec: knservingv1alpha1.RevisionSpec{
-			Container: v1.Container{
+			Container: &v1.Container{
 				Image:   tensorflow.TensorflowServingImageName + ":" + kfsvc.Spec.Default.Tensorflow.RuntimeVersion,
 				Command: []string{tensorflow.TensorflowEntrypointCommand},
 				Args: []string{
@@ -306,9 +306,9 @@ func TestKnativeServiceSpec(t *testing.T) {
 					Release: &knservingv1alpha1.ReleaseType{
 						Revisions: []string{"@latest"},
 						Configuration: knservingv1alpha1.ConfigurationSpec{
-							RevisionTemplate: knservingv1alpha1.RevisionTemplateSpec{
+							RevisionTemplate: &knservingv1alpha1.RevisionTemplateSpec{
 								Spec: knservingv1alpha1.RevisionSpec{
-									Container: v1.Container{
+									Container: &v1.Container{
 										//TODO(@yuzisun) fill in once scikit is implemented
 									},
 								},