diff --git a/config/default/rbac/rbac_role.yaml b/config/default/rbac/rbac_role.yaml index b67cbd0d011..ea07f9ebeb9 100644 --- a/config/default/rbac/rbac_role.yaml +++ b/config/default/rbac/rbac_role.yaml @@ -7,7 +7,7 @@ rules: - apiGroups: - serving.knative.dev resources: - - configurations + - services verbs: - get - list @@ -19,7 +19,7 @@ rules: - apiGroups: - serving.knative.dev resources: - - configurations/status + - services/status verbs: - get - update diff --git a/pkg/apis/serving/v1alpha2/kfservice_status.go b/pkg/apis/serving/v1alpha2/kfservice_status.go index f106a22ec2e..29cfb1e9f5a 100644 --- a/pkg/apis/serving/v1alpha2/kfservice_status.go +++ b/pkg/apis/serving/v1alpha2/kfservice_status.go @@ -14,6 +14,7 @@ limitations under the License. package v1alpha2 import ( + "github.com/kubeflow/kfserving/pkg/constants" "k8s.io/api/core/v1" "knative.dev/pkg/apis" knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" @@ -53,43 +54,61 @@ func (ss *KFServiceStatus) GetCondition(t apis.ConditionType) *apis.Condition { return conditionSet.Manage(ss).GetCondition(t) } -// PropagateDefaultConfigurationStatus propagates the default Configuration status and applies its values +func (ss *KFServiceStatus) PropagateDefaultStatus(endpoint constants.KFServiceEndpoint, defaultStatus *knservingv1alpha1.ServiceStatus) { + switch endpoint { + case constants.Predictor: + ss.PropagateDefaultPredictorStatus(defaultStatus) + case constants.Explainer: + case constants.Transformer: + } +} + +func (ss *KFServiceStatus) PropagateCanaryStatus(endpoint constants.KFServiceEndpoint, canaryStatus *knservingv1alpha1.ServiceStatus) { + switch endpoint { + case constants.Predictor: + ss.PropagateCanaryPredictorStatus(canaryStatus) + case constants.Explainer: + case constants.Transformer: + } +} + +// PropagateDefaultPredictorStatus propagates the default predictor status and applies its values // to the Service status. -func (ss *KFServiceStatus) PropagateDefaultConfigurationStatus(defaultConfigurationStatus *knservingv1alpha1.ConfigurationStatus) { - ss.Default.Name = defaultConfigurationStatus.LatestCreatedRevisionName - configurationCondition := defaultConfigurationStatus.GetCondition(knservingv1alpha1.ConfigurationConditionReady) +func (ss *KFServiceStatus) PropagateDefaultPredictorStatus(defaultStatus *knservingv1alpha1.ServiceStatus) { + ss.Default.Name = defaultStatus.LatestCreatedRevisionName + serviceCondition := defaultStatus.GetCondition(knservingv1alpha1.ServiceConditionReady) switch { - case configurationCondition == nil: - case configurationCondition.Status == v1.ConditionUnknown: - conditionSet.Manage(ss).MarkUnknown(DefaultPredictorReady, configurationCondition.Reason, configurationCondition.Message) - case configurationCondition.Status == v1.ConditionTrue: + case serviceCondition == nil: + case serviceCondition.Status == v1.ConditionUnknown: + conditionSet.Manage(ss).MarkUnknown(DefaultPredictorReady, serviceCondition.Reason, serviceCondition.Message) + case serviceCondition.Status == v1.ConditionTrue: conditionSet.Manage(ss).MarkTrue(DefaultPredictorReady) - case configurationCondition.Status == v1.ConditionFalse: - conditionSet.Manage(ss).MarkFalse(DefaultPredictorReady, configurationCondition.Reason, configurationCondition.Message) + case serviceCondition.Status == v1.ConditionFalse: + conditionSet.Manage(ss).MarkFalse(DefaultPredictorReady, serviceCondition.Reason, serviceCondition.Message) } } -// PropagateCanaryConfigurationStatus propagates the canary Configuration status and applies its values +// PropagateCanaryPredictorStatus propagates the canary predictor status and applies its values // to the Service status. -func (ss *KFServiceStatus) PropagateCanaryConfigurationStatus(canaryConfigurationStatus *knservingv1alpha1.ConfigurationStatus) { - // reset status if canaryConfigurationStatus is nil - if canaryConfigurationStatus == nil { +func (ss *KFServiceStatus) PropagateCanaryPredictorStatus(canaryStatus *knservingv1alpha1.ServiceStatus) { + // reset status if canaryServiceStatus is nil + if canaryStatus == nil { ss.Canary = StatusConfigurationSpec{} conditionSet.Manage(ss).ClearCondition(CanaryPredictorReady) return } - ss.Canary.Name = canaryConfigurationStatus.LatestCreatedRevisionName - configurationCondition := canaryConfigurationStatus.GetCondition(knservingv1alpha1.ConfigurationConditionReady) + ss.Canary.Name = canaryStatus.LatestCreatedRevisionName + serviceCondition := canaryStatus.GetCondition(knservingv1alpha1.ServiceConditionReady) switch { - case configurationCondition == nil: - case configurationCondition.Status == v1.ConditionUnknown: - conditionSet.Manage(ss).MarkUnknown(CanaryPredictorReady, configurationCondition.Reason, configurationCondition.Message) - case configurationCondition.Status == v1.ConditionTrue: + case serviceCondition == nil: + case serviceCondition.Status == v1.ConditionUnknown: + conditionSet.Manage(ss).MarkUnknown(CanaryPredictorReady, serviceCondition.Reason, serviceCondition.Message) + case serviceCondition.Status == v1.ConditionTrue: conditionSet.Manage(ss).MarkTrue(CanaryPredictorReady) - case configurationCondition.Status == v1.ConditionFalse: - conditionSet.Manage(ss).MarkFalse(CanaryPredictorReady, configurationCondition.Reason, configurationCondition.Message) + case serviceCondition.Status == v1.ConditionFalse: + conditionSet.Manage(ss).MarkFalse(CanaryPredictorReady, serviceCondition.Reason, serviceCondition.Message) } } diff --git a/pkg/apis/serving/v1alpha2/kfservice_status_test.go b/pkg/apis/serving/v1alpha2/kfservice_status_test.go index b9ae18c1fac..4c783d02d84 100644 --- a/pkg/apis/serving/v1alpha2/kfservice_status_test.go +++ b/pkg/apis/serving/v1alpha2/kfservice_status_test.go @@ -45,19 +45,19 @@ func TestKFServiceDuckType(t *testing.T) { func TestKFServiceIsReady(t *testing.T) { cases := []struct { - name string - defaultConfigurationStatus v1alpha1.ConfigurationStatus - canaryConfigurationStatus v1alpha1.ConfigurationStatus - routeStatus v1alpha1.RouteStatus - isReady bool + name string + defaultServiceStatus v1alpha1.ServiceStatus + canaryServiceStatus v1alpha1.ServiceStatus + routeStatus v1alpha1.RouteStatus + isReady bool }{{ - name: "empty status should not be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{}, - routeStatus: v1alpha1.RouteStatus{}, - isReady: false, + name: "empty status should not be ready", + defaultServiceStatus: v1alpha1.ServiceStatus{}, + routeStatus: v1alpha1.RouteStatus{}, + isReady: false, }, { name: "Different condition type should not be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: "Foo", @@ -68,10 +68,10 @@ func TestKFServiceIsReady(t *testing.T) { isReady: false, }, { name: "False condition status should not be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ - Type: v1alpha1.ConfigurationConditionReady, + Type: v1alpha1.ServiceConditionReady, Status: v1.ConditionFalse, }}, }, @@ -79,10 +79,10 @@ func TestKFServiceIsReady(t *testing.T) { isReady: false, }, { name: "Unknown condition status should not be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ - Type: v1alpha1.ConfigurationConditionReady, + Type: v1alpha1.ServiceConditionReady, Status: v1.ConditionUnknown, }}, }, @@ -90,7 +90,7 @@ func TestKFServiceIsReady(t *testing.T) { isReady: false, }, { name: "Missing condition status should not be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: v1alpha1.ConfigurationConditionReady, @@ -100,7 +100,7 @@ func TestKFServiceIsReady(t *testing.T) { isReady: false, }, { name: "True condition status should be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: v1alpha1.ConfigurationConditionReady, @@ -118,8 +118,8 @@ func TestKFServiceIsReady(t *testing.T) { }, isReady: true, }, { - name: "Default configuration, route conditions with ready status should be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + name: "Default service, route conditions with ready status should be ready", + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: "Foo", @@ -141,8 +141,8 @@ func TestKFServiceIsReady(t *testing.T) { }, isReady: true, }, { - name: "Default/canary configuration, route conditions with ready status should be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + name: "Default/canary service, route conditions with ready status should be ready", + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: v1alpha1.ConfigurationConditionReady, @@ -151,7 +151,7 @@ func TestKFServiceIsReady(t *testing.T) { }, }, }, - canaryConfigurationStatus: v1alpha1.ConfigurationStatus{ + canaryServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: v1alpha1.ConfigurationConditionReady, @@ -171,7 +171,7 @@ func TestKFServiceIsReady(t *testing.T) { isReady: true, }, { name: "Multiple conditions with ready status false should not be ready", - defaultConfigurationStatus: v1alpha1.ConfigurationStatus{ + defaultServiceStatus: v1alpha1.ServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{{ Type: "Foo", @@ -196,8 +196,8 @@ func TestKFServiceIsReady(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { status := KFServiceStatus{} - status.PropagateDefaultConfigurationStatus(&tc.defaultConfigurationStatus) - status.PropagateCanaryConfigurationStatus(&tc.canaryConfigurationStatus) + status.PropagateDefaultPredictorStatus(&tc.defaultServiceStatus) + status.PropagateCanaryPredictorStatus(&tc.canaryServiceStatus) status.PropagateRouteStatus(&tc.routeStatus) if e, a := tc.isReady, status.IsReady(); e != a { t.Errorf("%q expected: %v got: %v", tc.name, e, a) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 232c0fd4249..bdb0f5de573 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -82,6 +82,37 @@ const ( CustomSpecModelUriEnvVarKey = "STORAGE_URI" ) +type KFServiceEndpoint string + +type KFServiceVerb string + +// KFService Endpoint enums +const ( + Predictor KFServiceEndpoint = "predictor" + Explainer KFServiceEndpoint = "explainer" + Transformer KFServiceEndpoint = "transformer" +) + +// KFService verb enums +const ( + Predict KFServiceVerb = "predict" + Explain KFServiceVerb = "explain" +) + +// KFService default/canary constants +const ( + KFServiceDefault = "default" + KFServiceCanary = "canary" +) + +func (e KFServiceEndpoint) String() string { + return string(e) +} + +func (v KFServiceVerb) String() string { + return string(v) +} + func getEnvOrDefault(key string, fallback string) string { if value, ok := os.LookupEnv(key); ok { return value @@ -89,10 +120,50 @@ func getEnvOrDefault(key string, fallback string) string { return fallback } -func DefaultConfigurationName(name string) string { - return name + "-default" +func DefaultPredictorServiceName(name string) string { + return name + "-" + string(Predictor) + "-" + KFServiceDefault +} + +func CanaryPredictorServiceName(name string) string { + return name + "-" + string(Predictor) + "-" + KFServiceCanary +} + +func PredictRouteName(name string) string { + return name + "-" + string(Predict) +} + +func DefaultExplainerServiceName(name string) string { + return name + "-" + string(Explainer) + "-" + KFServiceDefault +} + +func CanaryExplainerServiceName(name string) string { + return name + "-" + string(Explainer) + "-" + KFServiceCanary +} + +func ExplainRouteName(name string) string { + return name + "-" + string(Explain) +} + +func DefaultTransformerServiceName(name string) string { + return name + "-" + string(Transformer) + "-" + KFServiceDefault +} + +func CanaryTransformerServiceName(name string) string { + return name + "-" + string(Transformer) + "-" + KFServiceCanary +} + +func TransformerRouteName(name string) string { + return name + "-" + string(Transformer) +} + +func DefaultServiceName(name string, endpoint KFServiceEndpoint) string { + return name + "-" + endpoint.String() + "-" + KFServiceDefault +} + +func CanaryServiceName(name string, endpoint KFServiceEndpoint) string { + return name + "-" + endpoint.String() + "-" + KFServiceCanary } -func CanaryConfigurationName(name string) string { - return name + "-canary" +func RouteName(name string, verb KFServiceVerb) string { + return name + "-" + verb.String() } diff --git a/pkg/controller/kfservice/kfservice_controller.go b/pkg/controller/kfservice/kfservice_controller.go index cf8f0164002..f87c00c8106 100644 --- a/pkg/controller/kfservice/kfservice_controller.go +++ b/pkg/controller/kfservice/kfservice_controller.go @@ -83,8 +83,8 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { OwnerType: &kfserving.KFService{}, } - // Watch for changes to Knative Configuration - if err = c.Watch(&source.Kind{Type: &knservingv1alpha1.Configuration{}}, kfservingController); err != nil { + // Watch for changes to Knative Service + if err = c.Watch(&source.Kind{Type: &knservingv1alpha1.Service{}}, kfservingController); err != nil { return err } @@ -117,8 +117,8 @@ type Reconciler interface { // Reconcile reads that state of the cluster for a Service object and makes changes based on the state read // and what is in the Service.Spec -// +kubebuilder:rbac:groups=serving.knative.dev,resources=configurations,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=serving.knative.dev,resources=configurations/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=serving.knative.dev,resources=services,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=serving.knative.dev,resources=services/status,verbs=get;update;patch // +kubebuilder:rbac:groups=serving.knative.dev,resources=routes,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=serving.knative.dev,resources=routes/status,verbs=get;update;patch // +kubebuilder:rbac:groups=serving.kubeflow.org,resources=kfservices,verbs=get;list;watch;create;update;patch;delete @@ -147,7 +147,7 @@ func (r *ReconcileService) Reconcile(request reconcile.Request) (reconcile.Resul } reconcilers := []Reconciler{ - knative.NewConfigurationReconciler(r.Client, r.scheme, configMap), + knative.NewServiceReconciler(r.Client, r.scheme, configMap), knative.NewRouteReconciler(r.Client, r.scheme), } diff --git a/pkg/controller/kfservice/kfservice_controller_test.go b/pkg/controller/kfservice/kfservice_controller_test.go index 9d2f40eec75..7fa3dc88d32 100644 --- 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 ( + "k8s.io/apimachinery/pkg/api/errors" "testing" "time" @@ -31,7 +32,6 @@ import ( kfserving "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" "github.com/onsi/gomega" "golang.org/x/net/context" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" @@ -47,15 +47,9 @@ const timeout = time.Second * 10 var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: "foo", Namespace: "default"}} var serviceKey = expectedRequest.NamespacedName -var configurationKey = types.NamespacedName{Name: constants.DefaultConfigurationName(serviceKey.Name), - Namespace: serviceKey.Namespace} var expectedCanaryRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: "bar", Namespace: "default"}} var canaryServiceKey = expectedCanaryRequest.NamespacedName -var defaultConfigurationKey = types.NamespacedName{Name: constants.DefaultConfigurationName(canaryServiceKey.Name), - Namespace: canaryServiceKey.Namespace} -var canaryConfigurationKey = types.NamespacedName{Name: constants.CanaryConfigurationName(canaryServiceKey.Name), - Namespace: canaryServiceKey.Namespace} var instance = &kfserving.KFService{ ObjectMeta: metav1.ObjectMeta{ @@ -133,6 +127,10 @@ var configs = map[string]string{ } func TestReconcile(t *testing.T) { + var predictorService = types.NamespacedName{Name: constants.DefaultPredictorServiceName(serviceKey.Name), + Namespace: serviceKey.Namespace} + var routeName = types.NamespacedName{Name: constants.PredictRouteName(serviceKey.Name), + Namespace: serviceKey.Namespace} g := gomega.NewGomegaWithT(t) // Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a // channel when it is finished. @@ -161,7 +159,7 @@ func TestReconcile(t *testing.T) { g.Expect(c.Create(context.TODO(), configMap)).NotTo(gomega.HaveOccurred()) defer c.Delete(context.TODO(), configMap) - // Create the KFService object and expect the Reconcile and Knative configuration/routes to be created + // Create the KFService object and expect the Reconcile and Knative service/routes to be created defaultInstance := instance.DeepCopy() g.Expect(c.Create(context.TODO(), defaultInstance)).NotTo(gomega.HaveOccurred()) @@ -169,40 +167,42 @@ func TestReconcile(t *testing.T) { defer c.Delete(context.TODO(), defaultInstance) g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedRequest))) - configuration := &knservingv1alpha1.Configuration{} - g.Eventually(func() error { return c.Get(context.TODO(), configurationKey, configuration) }, timeout). + service := &knservingv1alpha1.Service{} + g.Eventually(func() error { return c.Get(context.TODO(), predictorService, service) }, timeout). Should(gomega.Succeed()) - expectedConfiguration := &knservingv1alpha1.Configuration{ + expectedService := &knservingv1alpha1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: constants.DefaultConfigurationName(defaultInstance.Name), + Name: constants.DefaultPredictorServiceName(defaultInstance.Name), Namespace: defaultInstance.Namespace, }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "foo"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/target": "1", - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/maxScale": "3", - "autoscaling.knative.dev/minScale": "1", - constants.ModelInitializerSourceUriInternalAnnotationKey: defaultInstance.Spec.Default.Predictor.Tensorflow.ModelURI, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "foo"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/target": "1", + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/maxScale": "3", + "autoscaling.knative.dev/minScale": "1", + constants.ModelInitializerSourceUriInternalAnnotationKey: defaultInstance.Spec.Default.Predictor.Tensorflow.ModelURI, + }, }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: kfserving.TensorflowServingImageName + ":" + - defaultInstance.Spec.Default.Predictor.Tensorflow.RuntimeVersion, - Command: []string{kfserving.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + kfserving.TensorflowServingGRPCPort, - "--rest_api_port=" + kfserving.TensorflowServingRestPort, - "--model_name=" + defaultInstance.Name, - "--model_base_path=" + constants.DefaultModelLocalMountPath, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: kfserving.TensorflowServingImageName + ":" + + defaultInstance.Spec.Default.Predictor.Tensorflow.RuntimeVersion, + Command: []string{kfserving.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + kfserving.TensorflowServingGRPCPort, + "--rest_api_port=" + kfserving.TensorflowServingRestPort, + "--model_name=" + defaultInstance.Name, + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, }, }, }, @@ -212,18 +212,18 @@ func TestReconcile(t *testing.T) { }, }, } - g.Expect(configuration.Spec).To(gomega.Equal(expectedConfiguration.Spec)) + g.Expect(service.Spec).To(gomega.Equal(expectedService.Spec)) route := &knservingv1alpha1.Route{} - g.Eventually(func() error { return c.Get(context.TODO(), serviceKey, route) }, timeout). + g.Eventually(func() error { return c.Get(context.TODO(), routeName, route) }, timeout). Should(gomega.Succeed()) - // mock update knative configuration/route status since knative serving controller is not running in test - updated := configuration.DeepCopy() + // mock update knative service/route status since knative serving controller is not running in test + updated := service.DeepCopy() updated.Status.LatestCreatedRevisionName = "revision-v1" updated.Status.LatestReadyRevisionName = "revision-v1" updated.Status.Conditions = duckv1beta1.Conditions{ { - Type: knservingv1alpha1.ConfigurationConditionReady, + Type: knservingv1alpha1.ServiceConditionReady, Status: "True", }, } @@ -274,6 +274,12 @@ func TestReconcile(t *testing.T) { } func TestCanaryReconcile(t *testing.T) { + var defaultPredictor = types.NamespacedName{Name: constants.DefaultPredictorServiceName(canaryServiceKey.Name), + Namespace: canaryServiceKey.Namespace} + var canaryPredictor = types.NamespacedName{Name: constants.CanaryPredictorServiceName(canaryServiceKey.Name), + Namespace: canaryServiceKey.Namespace} + var routeName = types.NamespacedName{Name: constants.PredictRouteName(canaryServiceKey.Name), + Namespace: canaryServiceKey.Namespace} g := gomega.NewGomegaWithT(t) mgr, err := manager.New(cfg, manager.Options{}) @@ -307,44 +313,46 @@ func TestCanaryReconcile(t *testing.T) { defer c.Delete(context.TODO(), canaryInstance) g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) - defaultConfiguration := &knservingv1alpha1.Configuration{} - g.Eventually(func() error { return c.Get(context.TODO(), defaultConfigurationKey, defaultConfiguration) }, timeout). + defaultService := &knservingv1alpha1.Service{} + g.Eventually(func() error { return c.Get(context.TODO(), defaultPredictor, defaultService) }, timeout). Should(gomega.Succeed()) - canaryConfiguration := &knservingv1alpha1.Configuration{} - g.Eventually(func() error { return c.Get(context.TODO(), canaryConfigurationKey, canaryConfiguration) }, timeout). + canaryService := &knservingv1alpha1.Service{} + g.Eventually(func() error { return c.Get(context.TODO(), canaryPredictor, canaryService) }, timeout). Should(gomega.Succeed()) - expectedCanaryConfiguration := &knservingv1alpha1.Configuration{ + expectedCanaryService := &knservingv1alpha1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: canaryInstance.Name, Namespace: canaryInstance.Namespace, }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "bar"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/target": "1", - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/maxScale": "3", - "autoscaling.knative.dev/minScale": "1", - constants.ModelInitializerSourceUriInternalAnnotationKey: canary.Spec.Canary.Predictor.Tensorflow.ModelURI, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "bar"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/target": "1", + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/maxScale": "3", + "autoscaling.knative.dev/minScale": "1", + constants.ModelInitializerSourceUriInternalAnnotationKey: canary.Spec.Canary.Predictor.Tensorflow.ModelURI, + }, }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: kfserving.TensorflowServingImageName + ":" + - canary.Spec.Canary.Predictor.Tensorflow.RuntimeVersion, - Command: []string{kfserving.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + kfserving.TensorflowServingGRPCPort, - "--rest_api_port=" + kfserving.TensorflowServingRestPort, - "--model_name=" + canary.Name, - "--model_base_path=" + constants.DefaultModelLocalMountPath, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: kfserving.TensorflowServingImageName + ":" + + canary.Spec.Canary.Predictor.Tensorflow.RuntimeVersion, + Command: []string{kfserving.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + kfserving.TensorflowServingGRPCPort, + "--rest_api_port=" + kfserving.TensorflowServingRestPort, + "--model_name=" + canary.Name, + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, }, }, }, @@ -354,26 +362,26 @@ func TestCanaryReconcile(t *testing.T) { }, }, } - g.Expect(cmp.Diff(canaryConfiguration.Spec, expectedCanaryConfiguration.Spec)).To(gomega.Equal("")) + g.Expect(cmp.Diff(canaryService.Spec, expectedCanaryService.Spec)).To(gomega.Equal("")) route := &knservingv1alpha1.Route{} - g.Eventually(func() error { return c.Get(context.TODO(), canaryServiceKey, route) }, timeout). + g.Eventually(func() error { return c.Get(context.TODO(), routeName, route) }, timeout). Should(gomega.Succeed()) expectedRoute := knservingv1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: canaryInstance.Name, + Name: constants.PredictRouteName(canaryInstance.Name), Namespace: canaryInstance.Namespace, }, Spec: knservingv1alpha1.RouteSpec{ Traffic: []knservingv1alpha1.TrafficTarget{ { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: constants.DefaultConfigurationName(canary.Name), + ConfigurationName: constants.DefaultPredictorServiceName(canary.Name), Percent: 80, }, }, { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: constants.CanaryConfigurationName(canary.Name), + ConfigurationName: constants.CanaryPredictorServiceName(canary.Name), Percent: 20, }, }, @@ -382,25 +390,25 @@ func TestCanaryReconcile(t *testing.T) { } g.Expect(route.Spec).To(gomega.Equal(expectedRoute.Spec)) - // mock update knative configuration status since knative serving controller is not running in test - updateDefault := defaultConfiguration.DeepCopy() + // mock update knative service status since knative serving controller is not running in test + updateDefault := defaultService.DeepCopy() updateDefault.Status.LatestCreatedRevisionName = "revision-v1" updateDefault.Status.LatestReadyRevisionName = "revision-v1" updateDefault.Status.Conditions = duckv1beta1.Conditions{ { - Type: knservingv1alpha1.ConfigurationConditionReady, + Type: knservingv1alpha1.ServiceConditionReady, Status: "True", }, } g.Expect(c.Status().Update(context.TODO(), updateDefault)).NotTo(gomega.HaveOccurred()) g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) - updateCanary := canaryConfiguration.DeepCopy() + updateCanary := canaryService.DeepCopy() updateCanary.Status.LatestCreatedRevisionName = "revision-v2" updateCanary.Status.LatestReadyRevisionName = "revision-v2" updateCanary.Status.Conditions = duckv1beta1.Conditions{ { - Type: knservingv1alpha1.ConfigurationConditionReady, + Type: knservingv1alpha1.ServiceConditionReady, Status: "True", }, } @@ -469,6 +477,16 @@ func TestCanaryReconcile(t *testing.T) { } func TestCanaryDelete(t *testing.T) { + serviceName := "canary-delete" + namespace := "default" + var defaultPredictor = types.NamespacedName{Name: constants.DefaultPredictorServiceName(serviceName), + Namespace: namespace} + var canaryPredictor = types.NamespacedName{Name: constants.CanaryPredictorServiceName(serviceName), + Namespace: namespace} + var routeName = types.NamespacedName{Name: constants.PredictRouteName(serviceName), + Namespace: namespace} + var expectedCanaryRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: serviceName, Namespace: namespace}} + var canaryServiceKey = expectedCanaryRequest.NamespacedName g := gomega.NewGomegaWithT(t) mgr, err := manager.New(cfg, manager.Options{}) @@ -497,20 +515,69 @@ func TestCanaryDelete(t *testing.T) { defer c.Delete(context.TODO(), configMap) // Create the KFService object and expect the Reconcile - // Default and Canary configuration should be present + // Default and Canary service should be present canaryInstance := canary.DeepCopy() + canaryInstance.Name = serviceName g.Expect(c.Create(context.TODO(), canaryInstance)).NotTo(gomega.HaveOccurred()) defer c.Delete(context.TODO(), canaryInstance) g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) - defaultConfiguration := &knservingv1alpha1.Configuration{} - g.Eventually(func() error { return c.Get(context.TODO(), defaultConfigurationKey, defaultConfiguration) }, timeout). + defaultService := &knservingv1alpha1.Service{} + g.Eventually(func() error { return c.Get(context.TODO(), defaultPredictor, defaultService) }, timeout). + Should(gomega.Succeed()) + + canaryService := &knservingv1alpha1.Service{} + g.Eventually(func() error { return c.Get(context.TODO(), canaryPredictor, canaryService) }, timeout). Should(gomega.Succeed()) - canaryConfiguration := &knservingv1alpha1.Configuration{} - g.Eventually(func() error { return c.Get(context.TODO(), canaryConfigurationKey, canaryConfiguration) }, timeout). + route := &knservingv1alpha1.Route{} + g.Eventually(func() error { return c.Get(context.TODO(), routeName, route) }, timeout). Should(gomega.Succeed()) + // mock update knative service status since knative serving controller is not running in test + updateDefault := defaultService.DeepCopy() + updateDefault.Status.LatestCreatedRevisionName = "revision-v1" + updateDefault.Status.LatestReadyRevisionName = "revision-v1" + updateDefault.Status.Conditions = duckv1beta1.Conditions{ + { + Type: knservingv1alpha1.ServiceConditionReady, + Status: "True", + }, + } + g.Expect(c.Status().Update(context.TODO(), updateDefault)).NotTo(gomega.HaveOccurred()) + g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) + + updateCanary := canaryService.DeepCopy() + updateCanary.Status.LatestCreatedRevisionName = "revision-v2" + updateCanary.Status.LatestReadyRevisionName = "revision-v2" + updateCanary.Status.Conditions = duckv1beta1.Conditions{ + { + Type: knservingv1alpha1.ServiceConditionReady, + Status: "True", + }, + } + g.Expect(c.Status().Update(context.TODO(), updateCanary)).NotTo(gomega.HaveOccurred()) + g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) + + updatedRoute := route.DeepCopy() + updatedRoute.Status.URL = &apis.URL{Scheme: "http", Host: canaryServiceKey.Name + ".svc.cluster.local"} + updatedRoute.Status.Traffic = []knservingv1alpha1.TrafficTarget{ + { + TrafficTarget: v1beta1.TrafficTarget{RevisionName: "revision-v2", Percent: 20}, + }, + { + TrafficTarget: v1beta1.TrafficTarget{RevisionName: "revision-v1", Percent: 80}, + }, + } + updatedRoute.Status.Conditions = duckv1beta1.Conditions{ + { + Type: knservingv1alpha1.RouteConditionReady, + Status: "True", + }, + } + g.Expect(c.Status().Update(context.TODO(), updatedRoute)).NotTo(gomega.HaveOccurred()) + g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) + // Verify if KFService status is updated routeUrl := &apis.URL{Scheme: "http", Host: canaryServiceKey.Name + ".svc.cluster.local"} expectedKfsvcStatus := kfserving.KFServiceStatus{ @@ -545,47 +612,34 @@ func TestCanaryDelete(t *testing.T) { Traffic: 20, }, } + + canaryUpdate := &kfserving.KFService{} g.Eventually(func() string { - if err := c.Get(context.TODO(), canaryServiceKey, canaryInstance); err != nil { + if err := c.Get(context.TODO(), canaryServiceKey, canaryUpdate); err != nil { return err.Error() } - return cmp.Diff(&expectedKfsvcStatus, &canaryInstance.Status, cmpopts.IgnoreTypes(apis.VolatileTime{})) + return cmp.Diff(&expectedKfsvcStatus, &canaryUpdate.Status, cmpopts.IgnoreTypes(apis.VolatileTime{})) }, timeout).Should(gomega.BeEmpty()) // Update instance to remove Canary Spec - // Canary configuration should be removed during reconcile - canaryInstance.Spec.Canary = nil - canaryInstance.Spec.CanaryTrafficPercent = 0 - g.Expect(c.Update(context.TODO(), canaryInstance)).NotTo(gomega.HaveOccurred()) + // Canary service should be removed during reconcile + canaryUpdate.Spec.Canary = nil + canaryUpdate.Spec.CanaryTrafficPercent = 0 + g.Expect(c.Update(context.TODO(), canaryUpdate)).NotTo(gomega.HaveOccurred()) g.Expect(err).NotTo(gomega.HaveOccurred()) g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) - defaultConfiguration = &knservingv1alpha1.Configuration{} - g.Eventually(func() error { return c.Get(context.TODO(), defaultConfigurationKey, defaultConfiguration) }, timeout). + defaultService = &knservingv1alpha1.Service{} + g.Eventually(func() error { return c.Get(context.TODO(), defaultPredictor, defaultService) }, timeout). Should(gomega.Succeed()) - canaryConfiguration = &knservingv1alpha1.Configuration{} + canaryService = &knservingv1alpha1.Service{} g.Eventually(func() bool { - err := c.Get(context.TODO(), canaryConfigurationKey, canaryConfiguration) + err := c.Get(context.TODO(), canaryPredictor, canaryService) return errors.IsNotFound(err) }, timeout).Should(gomega.BeTrue()) - // Verify if KFService status is updated with right status - // Canary status should be removed with condition set to unknown - route := &knservingv1alpha1.Route{} - g.Eventually(func() error { return c.Get(context.TODO(), canaryServiceKey, route) }, timeout). - Should(gomega.Succeed()) - - updatedRoute := route.DeepCopy() - updatedRoute.Status.Traffic = []knservingv1alpha1.TrafficTarget{ - { - TrafficTarget: v1beta1.TrafficTarget{RevisionName: "revision-v1"}, - }, - } - g.Expect(c.Status().Update(context.TODO(), updatedRoute)).NotTo(gomega.HaveOccurred()) - g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedCanaryRequest))) - expectedKfsvcStatus = kfserving.KFServiceStatus{ Status: duckv1beta1.Status{ Conditions: duckv1beta1.Conditions{ @@ -608,12 +662,12 @@ func TestCanaryDelete(t *testing.T) { Name: "revision-v1", }, } - g.Eventually(func() *kfserving.KFServiceStatus { + g.Eventually(func() *duckv1beta1.Conditions { kfsvc := &kfserving.KFService{} err := c.Get(context.TODO(), canaryServiceKey, kfsvc) if err != nil { return nil } - return &kfsvc.Status - }, timeout).Should(testutils.BeSematicEqual(&expectedKfsvcStatus)) + return &kfsvc.Status.Conditions + }, timeout).Should(testutils.BeSematicEqual(&expectedKfsvcStatus.Conditions)) } diff --git a/pkg/controller/kfservice/reconcilers/knative/configuration_reconciler.go b/pkg/controller/kfservice/reconcilers/knative/configuration_reconciler.go deleted file mode 100644 index e6a1072e7dd..00000000000 --- a/pkg/controller/kfservice/reconcilers/knative/configuration_reconciler.go +++ /dev/null @@ -1,172 +0,0 @@ -/* -Copyright 2019 kubeflow.org. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package knative - -import ( - "context" - "fmt" - - "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" - "github.com/kubeflow/kfserving/pkg/constants" - "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/knative" - "knative.dev/pkg/kmp" - knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" - - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" -) - -var log = logf.Log.WithName("Reconciler") - -type ConfigurationReconciler struct { - client client.Client - scheme *runtime.Scheme - configurationBuilder *knative.ConfigurationBuilder -} - -func NewConfigurationReconciler(client client.Client, scheme *runtime.Scheme, config *v1.ConfigMap) *ConfigurationReconciler { - return &ConfigurationReconciler{ - client: client, - scheme: scheme, - configurationBuilder: knative.NewConfigurationBuilder(client, config), - } -} - -func (r *ConfigurationReconciler) Reconcile(kfsvc *v1alpha2.KFService) error { - if err := r.reconcileDefault(kfsvc); err != nil { - return err - } - if err := r.reconcileCanary(kfsvc); err != nil { - return err - } - return nil -} - -func (r *ConfigurationReconciler) reconcileDefault(kfsvc *v1alpha2.KFService) error { - defaultConfiguration, err := r.configurationBuilder.CreateKnativeConfiguration( - constants.DefaultConfigurationName(kfsvc.Name), - kfsvc.ObjectMeta, - &kfsvc.Spec.Default.Predictor, - ) - if err != nil { - return err - } - - status, err := r.reconcileConfiguration(kfsvc, defaultConfiguration) - if err != nil { - return err - } - - kfsvc.Status.PropagateDefaultConfigurationStatus(status) - return nil -} - -func (r *ConfigurationReconciler) reconcileCanary(kfsvc *v1alpha2.KFService) error { - if kfsvc.Spec.Canary == nil { - if err := r.finalizeConfiguration(kfsvc); err != nil { - return err - } - kfsvc.Status.PropagateCanaryConfigurationStatus(nil) - return nil - } - - canaryConfiguration, err := r.configurationBuilder.CreateKnativeConfiguration( - constants.CanaryConfigurationName(kfsvc.Name), - kfsvc.ObjectMeta, - &kfsvc.Spec.Canary.Predictor, - ) - if err != nil { - return err - } - - status, err := r.reconcileConfiguration(kfsvc, canaryConfiguration) - if err != nil { - return err - } - - kfsvc.Status.PropagateCanaryConfigurationStatus(status) - return nil -} - -func (r *ConfigurationReconciler) finalizeConfiguration(kfsvc *v1alpha2.KFService) error { - canaryConfigurationName := constants.CanaryConfigurationName(kfsvc.Name) - existing := &knservingv1alpha1.Configuration{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: canaryConfigurationName, Namespace: kfsvc.Namespace}, existing); err != nil { - if !errors.IsNotFound(err) { - return err - } - } else { - log.Info("Deleting Knative Serving configuration", "namespace", kfsvc.Namespace, "name", canaryConfigurationName) - if err := r.client.Delete(context.TODO(), existing, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil { - if !errors.IsNotFound(err) { - return err - } - } - } - return nil -} - -func (r *ConfigurationReconciler) reconcileConfiguration(kfsvc *v1alpha2.KFService, desired *knservingv1alpha1.Configuration) (*knservingv1alpha1.ConfigurationStatus, error) { - if err := controllerutil.SetControllerReference(kfsvc, desired, r.scheme); err != nil { - return nil, err - } - // Create configuration if does not exist - existing := &knservingv1alpha1.Configuration{} - err := r.client.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, existing) - if err != nil { - if errors.IsNotFound(err) { - log.Info("Creating Knative Serving configuration", "namespace", desired.Namespace, "name", desired.Name) - return &desired.Status, r.client.Create(context.TODO(), desired) - } - return nil, err - } - - // Return if no differences to reconcile. - if semanticEquals(desired, existing) { - return &existing.Status, nil - } - - // Reconcile differences and update - diff, err := kmp.SafeDiff(desired.Spec, existing.Spec) - if err != nil { - return &existing.Status, fmt.Errorf("failed to diff configuration: %v", err) - } - log.Info("Reconciling configuration diff (-desired, +observed):", "diff", diff) - log.Info("Updating configuration", "namespace", desired.Namespace, "name", desired.Name) - existing.Spec = desired.Spec - existing.ObjectMeta.Labels = desired.ObjectMeta.Labels - existing.ObjectMeta.Annotations = desired.ObjectMeta.Annotations - if err := r.client.Update(context.TODO(), existing); err != nil { - return &existing.Status, err - } - - return &existing.Status, nil -} - -func semanticEquals(desiredConfiguration, configuration *knservingv1alpha1.Configuration) bool { - return equality.Semantic.DeepEqual(desiredConfiguration.Spec, configuration.Spec) && - equality.Semantic.DeepEqual(desiredConfiguration.ObjectMeta.Labels, configuration.ObjectMeta.Labels) && - equality.Semantic.DeepEqual(desiredConfiguration.ObjectMeta.Annotations, configuration.ObjectMeta.Annotations) -} diff --git a/pkg/controller/kfservice/reconcilers/knative/configuration_reconciler_test.go b/pkg/controller/kfservice/reconcilers/knative/configuration_reconciler_test.go deleted file mode 100644 index 5895389a2a5..00000000000 --- a/pkg/controller/kfservice/reconcilers/knative/configuration_reconciler_test.go +++ /dev/null @@ -1,250 +0,0 @@ -/* -Copyright 2019 kubeflow.org. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package knative - -import ( - "context" - "fmt" - "testing" - "time" - - "github.com/google/go-cmp/cmp" - "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" - "github.com/kubeflow/kfserving/pkg/constants" - testutils "github.com/kubeflow/kfserving/pkg/testing" - "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" - "knative.dev/serving/pkg/apis/serving/v1beta1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" -) - -const timeout = time.Second * 5 - -func TestKnativeConfigurationReconcile(t *testing.T) { - g := gomega.NewGomegaWithT(t) - - mgr, err := manager.New(cfg, manager.Options{}) - stopMgr, mgrStopped := testutils.StartTestManager(mgr, g) - g.Expect(err).NotTo(gomega.HaveOccurred()) - c := mgr.GetClient() - - defer func() { - close(stopMgr) - mgrStopped.Wait() - }() - - configurationReconciler := NewConfigurationReconciler(c, mgr.GetScheme(), &v1.ConfigMap{}) - scenarios := map[string]struct { - kfsvc v1alpha2.KFService - desiredDefault *knservingv1alpha1.Configuration - desiredCanary *knservingv1alpha1.Configuration - }{ - "Reconcile creates default and canary configurations": { - kfsvc: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", - Namespace: "default", - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - Tensorflow: &v1alpha2.TensorflowSpec{ - RuntimeVersion: v1alpha2.DefaultTensorflowRuntimeVersion, - ModelURI: "gs://testuri", - }, - }, - }, - Canary: &v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - Tensorflow: &v1alpha2.TensorflowSpec{ - RuntimeVersion: v1alpha2.DefaultTensorflowRuntimeVersion, - ModelURI: "gs://testuri2", - }, - }, - }, - }, - }, - desiredDefault: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist-default", - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - "internal.serving.kubeflow.org/model-initializer-sourceuri": "gs://testuri", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.TensorflowServingImageName + ":" + v1alpha2.DefaultTensorflowRuntimeVersion, - Command: []string{v1alpha2.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + v1alpha2.TensorflowServingGRPCPort, - "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, - "--model_name=mnist", - "--model_base_path=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - desiredCanary: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist-canary", - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - "internal.serving.kubeflow.org/model-initializer-sourceuri": "gs://testuri2", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.TensorflowServingImageName + ":" + v1alpha2.DefaultTensorflowRuntimeVersion, - Command: []string{v1alpha2.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + v1alpha2.TensorflowServingGRPCPort, - "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, - "--model_name=mnist", - "--model_base_path=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - "Reconcile ignores canary if unspecified": { - kfsvc: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", - Namespace: "default", - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - Tensorflow: &v1alpha2.TensorflowSpec{ - RuntimeVersion: v1alpha2.DefaultTensorflowRuntimeVersion, - ModelURI: "gs://testuri", - }, - }, - }, - }, - }, - desiredDefault: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist-default", - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - "internal.serving.kubeflow.org/model-initializer-sourceuri": "gs://testuri", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.TensorflowServingImageName + ":" + v1alpha2.DefaultTensorflowRuntimeVersion, - Command: []string{v1alpha2.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + v1alpha2.TensorflowServingGRPCPort, - "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, - "--model_name=mnist", - "--model_base_path=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - desiredCanary: nil, - }, - } - for name, scenario := range scenarios { - t.Logf("Scenario: %s", name) - g.Expect(c.Create(context.TODO(), &scenario.kfsvc)).NotTo(gomega.HaveOccurred()) - - if err := configurationReconciler.Reconcile(&scenario.kfsvc); err != nil { - t.Errorf("Test %q failed: returned error: %v", name, err) - } - - g.Eventually(func() error { return awaitDesired(c, scenario.desiredDefault) }, timeout).Should(gomega.Succeed()) - g.Eventually(func() error { return awaitDesired(c, scenario.desiredCanary) }, timeout).Should(gomega.Succeed()) - - g.Expect(c.Delete(context.TODO(), &scenario.kfsvc)).NotTo(gomega.HaveOccurred()) - } -} - -func awaitDesired(c client.Client, desired *knservingv1alpha1.Configuration) error { - if desired == nil { - return nil - } - actual := knservingv1alpha1.Configuration{} - if err := c.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, &actual); err != nil { - return err - } - if diff := cmp.Diff(desired.Spec, actual.Spec); diff != "" { - return fmt.Errorf("Unexpected configuration spec (-want +got): %v", diff) - } - if diff := cmp.Diff(desired.ObjectMeta.Labels, actual.ObjectMeta.Labels); diff != "" { - return fmt.Errorf("Unexpected configuration labels (-want +got): %v", diff) - } - if diff := cmp.Diff(desired.ObjectMeta.Annotations, actual.ObjectMeta.Annotations); diff != "" { - return fmt.Errorf("Unexpected configuration annotations (-want +got): %v", diff) - } - return nil -} diff --git a/pkg/controller/kfservice/reconcilers/knative/route_reconciler.go b/pkg/controller/kfservice/reconcilers/knative/route_reconciler.go index 02012e3257c..6da861f641f 100644 --- a/pkg/controller/kfservice/reconcilers/knative/route_reconciler.go +++ b/pkg/controller/kfservice/reconcilers/knative/route_reconciler.go @@ -19,6 +19,7 @@ package knative import ( "context" "fmt" + "github.com/kubeflow/kfserving/pkg/constants" "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/knative" @@ -45,7 +46,12 @@ func NewRouteReconciler(client client.Client, scheme *runtime.Scheme) *RouteReco } func (r *RouteReconciler) Reconcile(kfsvc *v1alpha2.KFService) error { - desired := knative.NewRouteBuilder().CreateKnativeRoute(kfsvc) + endpoint := constants.Predictor + if kfsvc.Spec.Default.Transformer != nil { + endpoint = constants.Transformer + } + + desired := knative.NewRouteBuilder().CreateKnativeRoute(kfsvc, endpoint, constants.Predict) status, err := r.reconcileRoute(kfsvc, desired) if err != nil { diff --git a/pkg/controller/kfservice/reconcilers/knative/route_reconciler_test.go b/pkg/controller/kfservice/reconcilers/knative/route_reconciler_test.go index c2c95e79a19..6d40626d860 100644 --- a/pkg/controller/kfservice/reconcilers/knative/route_reconciler_test.go +++ b/pkg/controller/kfservice/reconcilers/knative/route_reconciler_test.go @@ -3,6 +3,7 @@ package knative import ( "context" "fmt" + "github.com/kubeflow/kfserving/pkg/constants" "testing" "github.com/google/go-cmp/cmp" @@ -55,14 +56,14 @@ func TestKnativeRouteReconcile(t *testing.T) { }, desiredRoute: &knservingv1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", + Name: constants.PredictRouteName("mnist"), Namespace: "default", }, Spec: knservingv1alpha1.RouteSpec{ Traffic: []knservingv1alpha1.TrafficTarget{ { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: "mnist-default", + ConfigurationName: constants.DefaultPredictorServiceName("mnist"), Percent: 100, }, }, diff --git a/pkg/controller/kfservice/reconcilers/knative/service_reconciler.go b/pkg/controller/kfservice/reconcilers/knative/service_reconciler.go new file mode 100644 index 00000000000..81df7ada542 --- /dev/null +++ b/pkg/controller/kfservice/reconcilers/knative/service_reconciler.go @@ -0,0 +1,182 @@ +/* +Copyright 2019 kubeflow.org. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package knative + +import ( + "context" + "fmt" + + "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" + "github.com/kubeflow/kfserving/pkg/constants" + "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/knative" + "knative.dev/pkg/kmp" + knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" +) + +var log = logf.Log.WithName("Reconciler") + +type ServiceReconciler struct { + client client.Client + scheme *runtime.Scheme + serviceBuilder *knative.ServiceBuilder +} + +func NewServiceReconciler(client client.Client, scheme *runtime.Scheme, config *v1.ConfigMap) *ServiceReconciler { + return &ServiceReconciler{ + client: client, + scheme: scheme, + serviceBuilder: knative.NewServiceBuilder(client, config), + } +} + +func (r *ServiceReconciler) Reconcile(kfsvc *v1alpha2.KFService) error { + if err := r.reconcileDefault(kfsvc); err != nil { + return err + } + + if err := r.reconcileCanary(kfsvc); err != nil { + return err + } + return nil +} + +func (r *ServiceReconciler) reconcileDefault(kfsvc *v1alpha2.KFService) error { + for _, endpoint := range []constants.KFServiceEndpoint{constants.Predictor, constants.Transformer, constants.Explainer} { + if err := r.reconcileEndpoint(kfsvc, endpoint, false); err != nil { + return err + } + } + return nil +} + +func (r *ServiceReconciler) reconcileCanary(kfsvc *v1alpha2.KFService) error { + for _, endpoint := range []constants.KFServiceEndpoint{constants.Predictor, constants.Transformer, constants.Explainer} { + if err := r.reconcileEndpoint(kfsvc, endpoint, true); err != nil { + return err + } + } + return nil +} + +func (r *ServiceReconciler) reconcileEndpoint(kfsvc *v1alpha2.KFService, endpoint constants.KFServiceEndpoint, isCanary bool) error { + if isCanary { + if kfsvc.Spec.Canary == nil { + if err := r.finalizeCanaryService(kfsvc, endpoint); err != nil { + return err + } + kfsvc.Status.PropagateCanaryStatus(endpoint, nil) + return nil + } + } + + service, err := r.serviceBuilder.CreateEndpointService( + kfsvc, + endpoint, + isCanary, + ) + + if err != nil { + return err + } + + if service == nil { + return nil + } + + status, err := r.reconcileService(kfsvc, service) + if err != nil { + return err + } + if isCanary { + kfsvc.Status.PropagateCanaryStatus(endpoint, status) + } else { + kfsvc.Status.PropagateDefaultStatus(endpoint, status) + } + return nil +} + +func (r *ServiceReconciler) finalizeCanaryService(kfsvc *v1alpha2.KFService, endpoint constants.KFServiceEndpoint) error { + canaryServiceName := constants.CanaryServiceName(kfsvc.Name, endpoint) + existing := &knservingv1alpha1.Service{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: canaryServiceName, Namespace: kfsvc.Namespace}, existing); err != nil { + if !errors.IsNotFound(err) { + return err + } + } else { + log.Info("Deleting service", "namespace", kfsvc.Namespace, "name", canaryServiceName) + if err := r.client.Delete(context.TODO(), existing, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil { + if !errors.IsNotFound(err) { + return err + } + } + } + return nil +} + +func (r *ServiceReconciler) reconcileService(kfsvc *v1alpha2.KFService, desired *knservingv1alpha1.Service) (*knservingv1alpha1.ServiceStatus, error) { + if err := controllerutil.SetControllerReference(kfsvc, desired, r.scheme); err != nil { + return nil, err + } + // Create service if does not exist + existing := &knservingv1alpha1.Service{} + err := r.client.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, existing) + if err != nil { + if errors.IsNotFound(err) { + log.Info("Creating Knative Service", "namespace", desired.Namespace, "name", desired.Name) + return &desired.Status, r.client.Create(context.TODO(), desired) + } + return nil, err + } + + // Return if no differences to reconcile. + if semanticEquals(desired, existing) { + return &existing.Status, nil + } + + // Reconcile differences and update + diff, err := kmp.SafeDiff(desired.Spec, existing.Spec) + if err != nil { + return &existing.Status, fmt.Errorf("failed to diff service: %v", err) + } + log.Info("Reconciling service diff (-desired, +observed):", "diff", diff) + log.Info("Updating service", "namespace", desired.Namespace, "name", desired.Name) + existing.Spec = desired.Spec + existing.ObjectMeta.Labels = desired.ObjectMeta.Labels + existing.ObjectMeta.Annotations = desired.ObjectMeta.Annotations + if err := r.client.Update(context.TODO(), existing); err != nil { + return &existing.Status, err + } + + return &existing.Status, nil +} + +func semanticEquals(desiredService, service *knservingv1alpha1.Service) bool { + return equality.Semantic.DeepEqual(desiredService.Spec, service.Spec) && + equality.Semantic.DeepEqual(desiredService.ObjectMeta.Labels, service.ObjectMeta.Labels) && + equality.Semantic.DeepEqual(desiredService.ObjectMeta.Annotations, service.ObjectMeta.Annotations) +} diff --git a/pkg/controller/kfservice/reconcilers/knative/service_reconciler_test.go b/pkg/controller/kfservice/reconcilers/knative/service_reconciler_test.go new file mode 100644 index 00000000000..e9f316c5917 --- /dev/null +++ b/pkg/controller/kfservice/reconcilers/knative/service_reconciler_test.go @@ -0,0 +1,256 @@ +/* +Copyright 2019 kubeflow.org. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package knative + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" + "github.com/kubeflow/kfserving/pkg/constants" + testutils "github.com/kubeflow/kfserving/pkg/testing" + "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" + "knative.dev/serving/pkg/apis/serving/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" +) + +const timeout = time.Second * 5 + +func TestKnativeServiceReconcile(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + mgr, err := manager.New(cfg, manager.Options{}) + stopMgr, mgrStopped := testutils.StartTestManager(mgr, g) + g.Expect(err).NotTo(gomega.HaveOccurred()) + c := mgr.GetClient() + + defer func() { + close(stopMgr) + mgrStopped.Wait() + }() + + serviceReconciler := NewServiceReconciler(c, mgr.GetScheme(), &v1.ConfigMap{}) + scenarios := map[string]struct { + kfsvc v1alpha2.KFService + desiredDefault *knservingv1alpha1.Service + desiredCanary *knservingv1alpha1.Service + }{ + "Reconcile creates default and canary service": { + kfsvc: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mnist", + Namespace: "default", + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + Tensorflow: &v1alpha2.TensorflowSpec{ + RuntimeVersion: v1alpha2.DefaultTensorflowRuntimeVersion, + ModelURI: "gs://testuri", + }, + }, + }, + Canary: &v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + Tensorflow: &v1alpha2.TensorflowSpec{ + RuntimeVersion: v1alpha2.DefaultTensorflowRuntimeVersion, + ModelURI: "gs://testuri2", + }, + }, + }, + }, + }, + desiredDefault: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("mnist"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + "internal.serving.kubeflow.org/model-initializer-sourceuri": "gs://testuri", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.TensorflowServingImageName + ":" + v1alpha2.DefaultTensorflowRuntimeVersion, + Command: []string{v1alpha2.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + v1alpha2.TensorflowServingGRPCPort, + "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, + "--model_name=mnist", + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + desiredCanary: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.CanaryPredictorServiceName("mnist"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + "internal.serving.kubeflow.org/model-initializer-sourceuri": "gs://testuri2", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.TensorflowServingImageName + ":" + v1alpha2.DefaultTensorflowRuntimeVersion, + Command: []string{v1alpha2.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + v1alpha2.TensorflowServingGRPCPort, + "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, + "--model_name=mnist", + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "Reconcile ignores canary if unspecified": { + kfsvc: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mnist", + Namespace: "default", + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + Tensorflow: &v1alpha2.TensorflowSpec{ + RuntimeVersion: v1alpha2.DefaultTensorflowRuntimeVersion, + ModelURI: "gs://testuri", + }, + }, + }, + }, + }, + desiredDefault: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("mnist"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + "internal.serving.kubeflow.org/model-initializer-sourceuri": "gs://testuri", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.TensorflowServingImageName + ":" + v1alpha2.DefaultTensorflowRuntimeVersion, + Command: []string{v1alpha2.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + v1alpha2.TensorflowServingGRPCPort, + "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, + "--model_name=mnist", + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + desiredCanary: nil, + }, + } + for name, scenario := range scenarios { + t.Logf("Scenario: %s", name) + g.Expect(c.Create(context.TODO(), &scenario.kfsvc)).NotTo(gomega.HaveOccurred()) + + if err := serviceReconciler.Reconcile(&scenario.kfsvc); err != nil { + t.Errorf("Test %q failed: returned error: %v", name, err) + } + + g.Eventually(func() error { return awaitDesired(c, scenario.desiredDefault) }, timeout).Should(gomega.Succeed()) + g.Eventually(func() error { return awaitDesired(c, scenario.desiredCanary) }, timeout).Should(gomega.Succeed()) + + g.Expect(c.Delete(context.TODO(), &scenario.kfsvc)).NotTo(gomega.HaveOccurred()) + } +} + +func awaitDesired(c client.Client, desired *knservingv1alpha1.Service) error { + if desired == nil { + return nil + } + actual := knservingv1alpha1.Service{} + if err := c.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, &actual); err != nil { + return err + } + if diff := cmp.Diff(desired.Spec, actual.Spec); diff != "" { + return fmt.Errorf("Unexpected service spec (-want +got): %v", diff) + } + if diff := cmp.Diff(desired.ObjectMeta.Labels, actual.ObjectMeta.Labels); diff != "" { + return fmt.Errorf("Unexpected service labels (-want +got): %v", diff) + } + if diff := cmp.Diff(desired.ObjectMeta.Annotations, actual.ObjectMeta.Annotations); diff != "" { + return fmt.Errorf("Unexpected service annotations (-want +got): %v", diff) + } + return nil +} diff --git a/pkg/controller/kfservice/resources/knative/configuration_test.go b/pkg/controller/kfservice/resources/knative/configuration_test.go deleted file mode 100644 index bb5eb7ab20f..00000000000 --- a/pkg/controller/kfservice/resources/knative/configuration_test.go +++ /dev/null @@ -1,468 +0,0 @@ -/* -Copyright 2019 kubeflow.org. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package knative - -import ( - "knative.dev/serving/pkg/apis/serving/v1beta1" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" - "github.com/kubeflow/kfserving/pkg/constants" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" -) - -var kfsvc = v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", - Namespace: "default", - Annotations: map[string]string{ - constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", - }, - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - DeploymentSpec: v1alpha2.DeploymentSpec{ - MinReplicas: 1, - MaxReplicas: 3, - ServiceAccountName: "testsvcacc", - }, - Tensorflow: &v1alpha2.TensorflowSpec{ - ModelURI: "s3://test/mnist/export", - RuntimeVersion: "1.13.0", - }, - }, - }, - }, -} - -var configMapData = map[string]string{ - "frameworks": `{ - "tensorflow" : { - "image" : "tensorflow/tfserving" - }, - "sklearn" : { - "image" : "kfserving/sklearnserver" - }, - "xgboost" : { - "image" : "kfserving/xgbserver" - } - }`, -} - -var defaultConfiguration = &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.DefaultConfigurationName("mnist"), - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - "autoscaling.knative.dev/minScale": "1", - "autoscaling.knative.dev/maxScale": "3", - constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", - constants.ModelInitializerSourceUriInternalAnnotationKey: kfsvc.Spec.Default.Predictor.Tensorflow.ModelURI, - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - ServiceAccountName: "testsvcacc", - Containers: []v1.Container{ - { - Image: v1alpha2.TensorflowServingImageName + ":" + kfsvc.Spec.Default.Predictor.Tensorflow.RuntimeVersion, - Command: []string{v1alpha2.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + v1alpha2.TensorflowServingGRPCPort, - "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, - "--model_name=mnist", - "--model_base_path=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, -} - -var canaryConfiguration = &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.CanaryConfigurationName("mnist"), - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, - Annotations: map[string]string{ - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - "autoscaling.knative.dev/minScale": "1", - "autoscaling.knative.dev/maxScale": "3", - constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", - constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/mnist-2/export", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.TensorflowServingImageName + ":" + kfsvc.Spec.Default.Predictor.Tensorflow.RuntimeVersion, - Command: []string{v1alpha2.TensorflowEntrypointCommand}, - Args: []string{ - "--port=" + v1alpha2.TensorflowServingGRPCPort, - "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, - "--model_name=mnist", - "--model_base_path=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, -} - -func TestKnativeConfiguration(t *testing.T) { - scenarios := map[string]struct { - configMapData map[string]string - kfService v1alpha2.KFService - expectedDefault *knservingv1alpha1.Configuration - expectedCanary *knservingv1alpha1.Configuration - }{ - "RunLatestModel": { - kfService: kfsvc, - expectedDefault: defaultConfiguration, - expectedCanary: nil, - }, - "RunCanaryModel": { - kfService: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", - Namespace: "default", - Annotations: map[string]string{ - constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", - }, - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - DeploymentSpec: v1alpha2.DeploymentSpec{ - MinReplicas: 1, - MaxReplicas: 3, - ServiceAccountName: "testsvcacc", - }, - Tensorflow: &v1alpha2.TensorflowSpec{ - ModelURI: kfsvc.Spec.Default.Predictor.Tensorflow.ModelURI, - RuntimeVersion: "1.13.0", - }, - }, - }, - CanaryTrafficPercent: 20, - Canary: &v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - DeploymentSpec: v1alpha2.DeploymentSpec{ - MinReplicas: 1, - MaxReplicas: 3, - }, - Tensorflow: &v1alpha2.TensorflowSpec{ - ModelURI: "s3://test/mnist-2/export", - RuntimeVersion: "1.13.0", - }, - }, - }, - }, - Status: v1alpha2.KFServiceStatus{ - Default: v1alpha2.StatusConfigurationSpec{ - Name: "v1", - }, - }, - }, - expectedDefault: defaultConfiguration, - expectedCanary: canaryConfiguration, - }, - "RunSklearnModel": { - kfService: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "sklearn", - Namespace: "default", - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - SKLearn: &v1alpha2.SKLearnSpec{ - ModelURI: "s3://test/sklearn/export", - RuntimeVersion: "latest", - }, - }, - }, - }, - }, - expectedDefault: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.DefaultConfigurationName("sklearn"), - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "sklearn"}, - Annotations: map[string]string{ - constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/sklearn/export", - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.SKLearnServerImageName + ":" + v1alpha2.DefaultSKLearnRuntimeVersion, - Args: []string{ - "--model_name=sklearn", - "--model_dir=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - "RunXgboostModel": { - kfService: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "xgboost", - Namespace: "default", - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - XGBoost: &v1alpha2.XGBoostSpec{ - ModelURI: "s3://test/xgboost/export", - RuntimeVersion: "latest", - }, - }, - }, - }, - }, - expectedDefault: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.DefaultConfigurationName("xgboost"), - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "xgboost"}, - Annotations: map[string]string{ - constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/xgboost/export", - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.XGBoostServerImageName + ":" + v1alpha2.DefaultXGBoostRuntimeVersion, - Args: []string{ - "--model_name=xgboost", - "--model_dir=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - "TestConfigOverride": { - configMapData: configMapData, - kfService: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "xgboost", - Namespace: "default", - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - XGBoost: &v1alpha2.XGBoostSpec{ - ModelURI: "s3://test/xgboost/export", - RuntimeVersion: "latest", - }, - }, - }, - }, - }, - expectedDefault: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.DefaultConfigurationName("xgboost"), - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "xgboost"}, - Annotations: map[string]string{ - constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/xgboost/export", - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "1", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: "kfserving/xgbserver:" + v1alpha2.DefaultXGBoostRuntimeVersion, - Args: []string{ - "--model_name=xgboost", - "--model_dir=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - "TestAnnotation": { - kfService: v1alpha2.KFService{ - ObjectMeta: metav1.ObjectMeta{ - Name: "sklearn", - Namespace: "default", - Annotations: map[string]string{ - "sourceName": "srcName", - "prop1": "val1", - "autoscaling.knative.dev/minScale": "2", - "autoscaling.knative.dev/target": "2", - constants.ModelInitializerSourceUriInternalAnnotationKey: "test", - "kubectl.kubernetes.io/last-applied-configuration": "test2", - }, - }, - Spec: v1alpha2.KFServiceSpec{ - Default: v1alpha2.EndpointSpec{ - Predictor: v1alpha2.PredictorSpec{ - SKLearn: &v1alpha2.SKLearnSpec{ - ModelURI: "s3://test/sklearn/export", - RuntimeVersion: "latest", - }, - DeploymentSpec: v1alpha2.DeploymentSpec{ - MinReplicas: 1, - }, - }, - }, - }, - }, - expectedDefault: &knservingv1alpha1.Configuration{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.DefaultConfigurationName("sklearn"), - Namespace: "default", - }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"serving.kubeflow.org/kfservice": "sklearn"}, - Annotations: map[string]string{ - constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/sklearn/export", - "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", - "autoscaling.knative.dev/target": "2", - "autoscaling.knative.dev/minScale": "1", - "sourceName": "srcName", - "prop1": "val1", - }, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - Containers: []v1.Container{ - { - Image: v1alpha2.SKLearnServerImageName + ":" + v1alpha2.DefaultSKLearnRuntimeVersion, - Args: []string{ - "--model_name=sklearn", - "--model_dir=" + constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - for name, scenario := range scenarios { - configurationBuilder := NewConfigurationBuilder(c, &v1.ConfigMap{ - Data: scenario.configMapData, - }) - actualDefaultConfiguration, err := configurationBuilder.CreateKnativeConfiguration( - constants.DefaultConfigurationName(scenario.kfService.Name), - scenario.kfService.ObjectMeta, - &scenario.kfService.Spec.Default.Predictor, - ) - if err != nil { - t.Errorf("Test %q unexpected error %s", name, err.Error()) - } - - if diff := cmp.Diff(scenario.expectedDefault, actualDefaultConfiguration); diff != "" { - t.Errorf("Test %q unexpected default configuration (-want +got): %v", name, diff) - } - - if scenario.kfService.Spec.Canary != nil { - actualCanaryConfiguration, err := configurationBuilder.CreateKnativeConfiguration( - constants.CanaryConfigurationName(kfsvc.Name), - scenario.kfService.ObjectMeta, - &scenario.kfService.Spec.Canary.Predictor, - ) - if err != nil { - t.Errorf("Test %q unexpected error %s", name, err.Error()) - } - if diff := cmp.Diff(scenario.expectedCanary, actualCanaryConfiguration); diff != "" { - t.Errorf("Test %q unexpected canary configuration (-want +got): %v", name, diff) - } - } - - } -} diff --git a/pkg/controller/kfservice/resources/knative/route.go b/pkg/controller/kfservice/resources/knative/route.go index 2a02b9bc797..2af1b29c8c5 100644 --- a/pkg/controller/kfservice/resources/knative/route.go +++ b/pkg/controller/kfservice/resources/knative/route.go @@ -36,7 +36,8 @@ func NewRouteBuilder() *RouteBuilder { return &RouteBuilder{} } -func (r *RouteBuilder) CreateKnativeRoute(kfsvc *v1alpha2.KFService) *knservingv1alpha1.Route { +func (r *RouteBuilder) CreateKnativeRoute(kfsvc *v1alpha2.KFService, endpoint constants.KFServiceEndpoint, + verb constants.KFServiceVerb) *knservingv1alpha1.Route { defaultPercent := 100 canaryPercent := 0 if kfsvc.Spec.Canary != nil { @@ -46,7 +47,7 @@ func (r *RouteBuilder) CreateKnativeRoute(kfsvc *v1alpha2.KFService) *knservingv trafficTargets := []knservingv1alpha1.TrafficTarget{ { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: constants.DefaultConfigurationName(kfsvc.Name), + ConfigurationName: constants.DefaultServiceName(kfsvc.Name, endpoint), Percent: defaultPercent, }, }, @@ -54,7 +55,7 @@ func (r *RouteBuilder) CreateKnativeRoute(kfsvc *v1alpha2.KFService) *knservingv if kfsvc.Spec.Canary != nil { trafficTargets = append(trafficTargets, knservingv1alpha1.TrafficTarget{ TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: constants.CanaryConfigurationName(kfsvc.Name), + ConfigurationName: constants.CanaryServiceName(kfsvc.Name, endpoint), Percent: canaryPercent, }, }) @@ -64,7 +65,7 @@ func (r *RouteBuilder) CreateKnativeRoute(kfsvc *v1alpha2.KFService) *knservingv }) return &knservingv1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: kfsvc.Name, + Name: constants.RouteName(kfsvc.Name, verb), Namespace: kfsvc.Namespace, Labels: kfsvc.Labels, Annotations: kfsvcAnnotations, diff --git a/pkg/controller/kfservice/resources/knative/route_test.go b/pkg/controller/kfservice/resources/knative/route_test.go index 45ba524e29a..387b5facf53 100644 --- a/pkg/controller/kfservice/resources/knative/route_test.go +++ b/pkg/controller/kfservice/resources/knative/route_test.go @@ -17,6 +17,7 @@ limitations under the License. package knative import ( + "github.com/kubeflow/kfserving/pkg/constants" "testing" "github.com/google/go-cmp/cmp" @@ -51,7 +52,7 @@ func TestKnativeRoute(t *testing.T) { }, expectedRoute: &knservingv1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", + Name: constants.PredictRouteName("mnist"), Namespace: "default", Annotations: make(map[string]string), }, @@ -59,7 +60,7 @@ func TestKnativeRoute(t *testing.T) { Traffic: []knservingv1alpha1.TrafficTarget{ { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: "mnist-default", + ConfigurationName: constants.DefaultPredictorServiceName("mnist"), Percent: 100, }, }, @@ -100,7 +101,7 @@ func TestKnativeRoute(t *testing.T) { }, expectedRoute: &knservingv1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", + Name: constants.RouteName("mnist", constants.Predict), Namespace: "default", Annotations: make(map[string]string), }, @@ -108,13 +109,13 @@ func TestKnativeRoute(t *testing.T) { Traffic: []knservingv1alpha1.TrafficTarget{ { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: "mnist-default", + ConfigurationName: constants.DefaultPredictorServiceName("mnist"), Percent: 80, }, }, { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: "mnist-canary", + ConfigurationName: constants.CanaryPredictorServiceName("mnist"), Percent: 20, }, }, @@ -160,7 +161,7 @@ func TestKnativeRoute(t *testing.T) { }, expectedRoute: &knservingv1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ - Name: "mnist", + Name: constants.RouteName("mnist", constants.Predict), Namespace: "default", Annotations: map[string]string{ "sourceName": "srcName", @@ -171,13 +172,13 @@ func TestKnativeRoute(t *testing.T) { Traffic: []knservingv1alpha1.TrafficTarget{ { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: "mnist-default", + ConfigurationName: constants.DefaultPredictorServiceName("mnist"), Percent: 80, }, }, { TrafficTarget: v1beta1.TrafficTarget{ - ConfigurationName: "mnist-canary", + ConfigurationName: constants.CanaryPredictorServiceName("mnist"), Percent: 20, }, }, @@ -189,7 +190,7 @@ func TestKnativeRoute(t *testing.T) { for name, scenario := range scenarios { routeBuilder := NewRouteBuilder() - route := routeBuilder.CreateKnativeRoute(&scenario.kfService) + route := routeBuilder.CreateKnativeRoute(&scenario.kfService, constants.Predictor, constants.Predict) // Validate if scenario.shouldFail { t.Errorf("Test %q failed: returned success but expected error", name) diff --git a/pkg/controller/kfservice/resources/knative/configuration.go b/pkg/controller/kfservice/resources/knative/service.go similarity index 56% rename from pkg/controller/kfservice/resources/knative/configuration.go rename to pkg/controller/kfservice/resources/knative/service.go index 2e22b815610..41b37753149 100644 --- a/pkg/controller/kfservice/resources/knative/configuration.go +++ b/pkg/controller/kfservice/resources/knative/service.go @@ -37,19 +37,19 @@ const ( FrameworkConfigKeyName = "frameworks" ) -var configurationAnnotationDisallowedList = []string{ +var serviceAnnotationDisallowedList = []string{ autoscaling.MinScaleAnnotationKey, autoscaling.MaxScaleAnnotationKey, constants.ModelInitializerSourceUriInternalAnnotationKey, "kubectl.kubernetes.io/last-applied-configuration", } -type ConfigurationBuilder struct { +type ServiceBuilder struct { frameworksConfig *v1alpha2.FrameworksConfig credentialBuilder *credentials.CredentialBuilder } -func NewConfigurationBuilder(client client.Client, config *v1.ConfigMap) *ConfigurationBuilder { +func NewServiceBuilder(client client.Client, config *v1.ConfigMap) *ServiceBuilder { frameworkConfig := &v1alpha2.FrameworksConfig{} if fmks, ok := config.Data[FrameworkConfigKeyName]; ok { err := json.Unmarshal([]byte(fmks), &frameworkConfig) @@ -58,15 +58,51 @@ func NewConfigurationBuilder(client client.Client, config *v1.ConfigMap) *Config } } - return &ConfigurationBuilder{ + return &ServiceBuilder{ frameworksConfig: frameworkConfig, credentialBuilder: credentials.NewCredentialBulder(client, config), } } -func (c *ConfigurationBuilder) CreateKnativeConfiguration(name string, metadata metav1.ObjectMeta, predictorSpec *v1alpha2.PredictorSpec) (*knservingv1alpha1.Configuration, error) { +func (c *ServiceBuilder) CreateEndpointService(kfsvc *v1alpha2.KFService, endpoint constants.KFServiceEndpoint, isCanary bool) (*knservingv1alpha1.Service, error) { + serviceName := constants.DefaultServiceName(kfsvc.Name, endpoint) + if isCanary { + serviceName = constants.CanaryServiceName(kfsvc.Name, endpoint) + } + switch endpoint { + case constants.Predictor: + predictorSpec := &kfsvc.Spec.Default.Predictor + if isCanary { + predictorSpec = &kfsvc.Spec.Canary.Predictor + } + return c.CreatePredictorService(serviceName, kfsvc.ObjectMeta, predictorSpec) + case constants.Transformer: + transformerSpec := &kfsvc.Spec.Default.Transformer + if isCanary { + transformerSpec = &kfsvc.Spec.Canary.Transformer + } + if transformerSpec == nil { + return nil, nil + } + //TODO create transformer + return nil, nil + case constants.Explainer: + explainerSpec := &kfsvc.Spec.Default.Explainer + if isCanary { + explainerSpec = &kfsvc.Spec.Canary.Explainer + } + if explainerSpec == nil { + return nil, nil + } + //TODO create explainer + return nil, nil + } + return nil, fmt.Errorf("Invalid endpoint") +} + +func (c *ServiceBuilder) CreatePredictorService(name string, metadata metav1.ObjectMeta, predictorSpec *v1alpha2.PredictorSpec) (*knservingv1alpha1.Service, error) { annotations := utils.Filter(metadata.Annotations, func(key string) bool { - return !utils.Includes(configurationAnnotationDisallowedList, key) + return !utils.Includes(serviceAnnotationDisallowedList, key) }) if predictorSpec.MinReplicas != 0 { @@ -91,29 +127,31 @@ func (c *ConfigurationBuilder) CreateKnativeConfiguration(name string, metadata annotations[constants.ModelInitializerSourceUriInternalAnnotationKey] = sourceURI } - configuration := &knservingv1alpha1.Configuration{ + service := &knservingv1alpha1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: metadata.Namespace, Labels: metadata.Labels, }, - Spec: knservingv1alpha1.ConfigurationSpec{ - Template: &knservingv1alpha1.RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: utils.Union(metadata.Labels, map[string]string{ - constants.KFServicePodLabelKey: metadata.Name, - }), - Annotations: annotations, - }, - Spec: knservingv1alpha1.RevisionSpec{ - RevisionSpec: v1beta1.RevisionSpec{ - // Defaulting here since this always shows a diff with nil vs 300s(knative default) - // we may need to expose this field in future - TimeoutSeconds: &constants.DefaultTimeout, - PodSpec: v1.PodSpec{ - ServiceAccountName: predictorSpec.ServiceAccountName, - Containers: []v1.Container{ - *predictorSpec.CreateModelServingContainer(metadata.Name, c.frameworksConfig), + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: utils.Union(metadata.Labels, map[string]string{ + constants.KFServicePodLabelKey: metadata.Name, + }), + Annotations: annotations, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + // Defaulting here since this always shows a diff with nil vs 300s(knative default) + // we may need to expose this field in future + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + ServiceAccountName: predictorSpec.ServiceAccountName, + Containers: []v1.Container{ + *predictorSpec.CreateModelServingContainer(metadata.Name, c.frameworksConfig), + }, }, }, }, @@ -125,11 +163,11 @@ func (c *ConfigurationBuilder) CreateKnativeConfiguration(name string, metadata if err := c.credentialBuilder.CreateSecretVolumeAndEnv( metadata.Namespace, predictorSpec.ServiceAccountName, - &configuration.Spec.Template.Spec.Containers[0], - &configuration.Spec.Template.Spec.Volumes, + &service.Spec.Template.Spec.Containers[0], + &service.Spec.Template.Spec.Volumes, ); err != nil { return nil, err } - return configuration, nil + return service, nil } diff --git a/pkg/controller/kfservice/resources/knative/service_test.go b/pkg/controller/kfservice/resources/knative/service_test.go new file mode 100644 index 00000000000..1fc29758ca1 --- /dev/null +++ b/pkg/controller/kfservice/resources/knative/service_test.go @@ -0,0 +1,480 @@ +/* +Copyright 2019 kubeflow.org. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package knative + +import ( + "knative.dev/serving/pkg/apis/serving/v1beta1" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" + "github.com/kubeflow/kfserving/pkg/constants" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + knservingv1alpha1 "knative.dev/serving/pkg/apis/serving/v1alpha1" +) + +var kfsvc = v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mnist", + Namespace: "default", + Annotations: map[string]string{ + constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", + }, + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + DeploymentSpec: v1alpha2.DeploymentSpec{ + MinReplicas: 1, + MaxReplicas: 3, + ServiceAccountName: "testsvcacc", + }, + Tensorflow: &v1alpha2.TensorflowSpec{ + ModelURI: "s3://test/mnist/export", + RuntimeVersion: "1.13.0", + }, + }, + }, + }, +} + +var configMapData = map[string]string{ + "frameworks": `{ + "tensorflow" : { + "image" : "tensorflow/tfserving" + }, + "sklearn" : { + "image" : "kfserving/sklearnserver" + }, + "xgboost" : { + "image" : "kfserving/xgbserver" + } + }`, +} + +var defaultService = &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("mnist"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + "autoscaling.knative.dev/minScale": "1", + "autoscaling.knative.dev/maxScale": "3", + constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", + constants.ModelInitializerSourceUriInternalAnnotationKey: kfsvc.Spec.Default.Predictor.Tensorflow.ModelURI, + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + ServiceAccountName: "testsvcacc", + Containers: []v1.Container{ + { + Image: v1alpha2.TensorflowServingImageName + ":" + kfsvc.Spec.Default.Predictor.Tensorflow.RuntimeVersion, + Command: []string{v1alpha2.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + v1alpha2.TensorflowServingGRPCPort, + "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, + "--model_name=mnist", + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, +} + +var canaryService = &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.CanaryPredictorServiceName("mnist"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "mnist"}, + Annotations: map[string]string{ + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + "autoscaling.knative.dev/minScale": "1", + "autoscaling.knative.dev/maxScale": "3", + constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", + constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/mnist-2/export", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.TensorflowServingImageName + ":" + kfsvc.Spec.Default.Predictor.Tensorflow.RuntimeVersion, + Command: []string{v1alpha2.TensorflowEntrypointCommand}, + Args: []string{ + "--port=" + v1alpha2.TensorflowServingGRPCPort, + "--rest_api_port=" + v1alpha2.TensorflowServingRestPort, + "--model_name=mnist", + "--model_base_path=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, +} + +func TestKFServiceToKnativeService(t *testing.T) { + scenarios := map[string]struct { + configMapData map[string]string + kfService v1alpha2.KFService + expectedDefault *knservingv1alpha1.Service + expectedCanary *knservingv1alpha1.Service + }{ + "RunLatestModel": { + kfService: kfsvc, + expectedDefault: defaultService, + expectedCanary: nil, + }, + "RunCanaryModel": { + kfService: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mnist", + Namespace: "default", + Annotations: map[string]string{ + constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-t4", + }, + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + DeploymentSpec: v1alpha2.DeploymentSpec{ + MinReplicas: 1, + MaxReplicas: 3, + ServiceAccountName: "testsvcacc", + }, + Tensorflow: &v1alpha2.TensorflowSpec{ + ModelURI: kfsvc.Spec.Default.Predictor.Tensorflow.ModelURI, + RuntimeVersion: "1.13.0", + }, + }, + }, + CanaryTrafficPercent: 20, + Canary: &v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + DeploymentSpec: v1alpha2.DeploymentSpec{ + MinReplicas: 1, + MaxReplicas: 3, + }, + Tensorflow: &v1alpha2.TensorflowSpec{ + ModelURI: "s3://test/mnist-2/export", + RuntimeVersion: "1.13.0", + }, + }, + }, + }, + Status: v1alpha2.KFServiceStatus{ + Default: v1alpha2.StatusConfigurationSpec{ + Name: "v1", + }, + }, + }, + expectedDefault: defaultService, + expectedCanary: canaryService, + }, + "RunSklearnModel": { + kfService: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sklearn", + Namespace: "default", + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + SKLearn: &v1alpha2.SKLearnSpec{ + ModelURI: "s3://test/sklearn/export", + RuntimeVersion: "latest", + }, + }, + }, + }, + }, + expectedDefault: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("sklearn"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "sklearn"}, + Annotations: map[string]string{ + constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/sklearn/export", + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.SKLearnServerImageName + ":" + v1alpha2.DefaultSKLearnRuntimeVersion, + Args: []string{ + "--model_name=sklearn", + "--model_dir=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "RunXgboostModel": { + kfService: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "xgboost", + Namespace: "default", + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + XGBoost: &v1alpha2.XGBoostSpec{ + ModelURI: "s3://test/xgboost/export", + RuntimeVersion: "latest", + }, + }, + }, + }, + }, + expectedDefault: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("xgboost"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "xgboost"}, + Annotations: map[string]string{ + constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/xgboost/export", + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.XGBoostServerImageName + ":" + v1alpha2.DefaultXGBoostRuntimeVersion, + Args: []string{ + "--model_name=xgboost", + "--model_dir=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "TestConfigOverride": { + configMapData: configMapData, + kfService: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "xgboost", + Namespace: "default", + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + XGBoost: &v1alpha2.XGBoostSpec{ + ModelURI: "s3://test/xgboost/export", + RuntimeVersion: "latest", + }, + }, + }, + }, + }, + expectedDefault: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("xgboost"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "xgboost"}, + Annotations: map[string]string{ + constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/xgboost/export", + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "1", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: "kfserving/xgbserver:" + v1alpha2.DefaultXGBoostRuntimeVersion, + Args: []string{ + "--model_name=xgboost", + "--model_dir=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + "TestAnnotation": { + kfService: v1alpha2.KFService{ + ObjectMeta: metav1.ObjectMeta{ + Name: "sklearn", + Namespace: "default", + Annotations: map[string]string{ + "sourceName": "srcName", + "prop1": "val1", + "autoscaling.knative.dev/minScale": "2", + "autoscaling.knative.dev/target": "2", + constants.ModelInitializerSourceUriInternalAnnotationKey: "test", + "kubectl.kubernetes.io/last-applied-configuration": "test2", + }, + }, + Spec: v1alpha2.KFServiceSpec{ + Default: v1alpha2.EndpointSpec{ + Predictor: v1alpha2.PredictorSpec{ + SKLearn: &v1alpha2.SKLearnSpec{ + ModelURI: "s3://test/sklearn/export", + RuntimeVersion: "latest", + }, + DeploymentSpec: v1alpha2.DeploymentSpec{ + MinReplicas: 1, + }, + }, + }, + }, + }, + expectedDefault: &knservingv1alpha1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultPredictorServiceName("sklearn"), + Namespace: "default", + }, + Spec: knservingv1alpha1.ServiceSpec{ + ConfigurationSpec: knservingv1alpha1.ConfigurationSpec{ + Template: &knservingv1alpha1.RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"serving.kubeflow.org/kfservice": "sklearn"}, + Annotations: map[string]string{ + constants.ModelInitializerSourceUriInternalAnnotationKey: "s3://test/sklearn/export", + "autoscaling.knative.dev/class": "kpa.autoscaling.knative.dev", + "autoscaling.knative.dev/target": "2", + "autoscaling.knative.dev/minScale": "1", + "sourceName": "srcName", + "prop1": "val1", + }, + }, + Spec: knservingv1alpha1.RevisionSpec{ + RevisionSpec: v1beta1.RevisionSpec{ + TimeoutSeconds: &constants.DefaultTimeout, + PodSpec: v1.PodSpec{ + Containers: []v1.Container{ + { + Image: v1alpha2.SKLearnServerImageName + ":" + v1alpha2.DefaultSKLearnRuntimeVersion, + Args: []string{ + "--model_name=sklearn", + "--model_dir=" + constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + for name, scenario := range scenarios { + serviceBuilder := NewServiceBuilder(c, &v1.ConfigMap{ + Data: scenario.configMapData, + }) + actualDefaultService, err := serviceBuilder.CreatePredictorService( + constants.DefaultPredictorServiceName(scenario.kfService.Name), + scenario.kfService.ObjectMeta, + &scenario.kfService.Spec.Default.Predictor, + ) + if err != nil { + t.Errorf("Test %q unexpected error %s", name, err.Error()) + } + + if diff := cmp.Diff(scenario.expectedDefault, actualDefaultService); diff != "" { + t.Errorf("Test %q unexpected default service (-want +got): %v", name, diff) + } + + if scenario.kfService.Spec.Canary != nil { + actualCanaryService, err := serviceBuilder.CreatePredictorService( + constants.CanaryPredictorServiceName(kfsvc.Name), + scenario.kfService.ObjectMeta, + &scenario.kfService.Spec.Canary.Predictor, + ) + if err != nil { + t.Errorf("Test %q unexpected error %s", name, err.Error()) + } + if diff := cmp.Diff(scenario.expectedCanary, actualCanaryService); diff != "" { + t.Errorf("Test %q unexpected canary service (-want +got): %v", name, diff) + } + } + + } +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index b3d6a17fe29..96f27a409e6 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -57,22 +57,22 @@ func TestUnionUtil(t *testing.T) { "UnionTwoMaps": { input1: map[string]string{"serving.kubeflow.org/service": "mnist", "label1": "value1"}, - input2: map[string]string{"service.knative.dev/configuration": "mnist", + input2: map[string]string{"service.knative.dev/service": "mnist", "label2": "value2"}, expected: map[string]string{"serving.kubeflow.org/service": "mnist", - "label1": "value1", "service.knative.dev/configuration": "mnist", "label2": "value2"}, + "label1": "value1", "service.knative.dev/service": "mnist", "label2": "value2"}, }, "UnionWithEmptyMap": { input1: map[string]string{}, - input2: map[string]string{"service.knative.dev/configuration": "mnist", + input2: map[string]string{"service.knative.dev/service": "mnist", "label2": "value2"}, - expected: map[string]string{"service.knative.dev/configuration": "mnist", "label2": "value2"}, + expected: map[string]string{"service.knative.dev/service": "mnist", "label2": "value2"}, }, "UnionWithNilMap": { input1: nil, - input2: map[string]string{"service.knative.dev/configuration": "mnist", + input2: map[string]string{"service.knative.dev/service": "mnist", "label2": "value2"}, - expected: map[string]string{"service.knative.dev/configuration": "mnist", "label2": "value2"}, + expected: map[string]string{"service.knative.dev/service": "mnist", "label2": "value2"}, }, "UnionNilMaps": { input1: nil, diff --git a/test/crds/knative_service.yaml b/test/crds/knative_service.yaml new file mode 100644 index 00000000000..730fd691adb --- /dev/null +++ b/test/crds/knative_service.yaml @@ -0,0 +1,54 @@ +# Copyright 2019 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: services.serving.knative.dev + labels: + serving.knative.dev/release: devel + knative.dev/crd-install: "true" +spec: + group: serving.knative.dev + version: v1alpha1 + names: + kind: Service + plural: services + singular: service + categories: + - all + - knative + - serving + shortNames: + - kservice + - ksvc + scope: Namespaced + subresources: + status: {} + additionalPrinterColumns: + - name: URL + type: string + JSONPath: .status.url + - name: LatestCreated + type: string + JSONPath: .status.latestCreatedRevisionName + - name: LatestReady + type: string + JSONPath: .status.latestReadyRevisionName + - name: Ready + type: string + JSONPath: ".status.conditions[?(@.type=='Ready')].status" + - name: Reason + type: string + JSONPath: ".status.conditions[?(@.type=='Ready')].reason"