Fix cluster-local annonation, set knative svc defaults (#211)

* Fix cluster-local annonation, set knative svc defaults

* Revert cluster-local annotation for testing

* Revert "Revert cluster-local annotation for testing"

This reverts commit df0f4ce.

* Update comments

Co-authored-by: Krithika Sundararajan <krithika.sundararajan@go-jek.com>

Author: krithika369

api/turing/cluster/knative_service.go

@@ -1,6 +1,7 @@
 package cluster
 
 import (
+	"context"
 	"fmt"
 	"strconv"
 
@@ -59,19 +60,23 @@ func (cfg *KnativeService) BuildKnativeServiceConfig() *knservingv1.Service {
 	kserviceLabels := clone(cfg.Labels)
 	if cfg.IsClusterLocal {
 		// Kservice should only be accessible from within the cluster
-		// https://knative.dev/docs/serving/cluster-local-route/
-		kserviceLabels["serving.knative.dev/visibility"] = "cluster-local"
+		// https://knative.dev/v1.2-docs/serving/services/private-services/
+		kserviceLabels["networking.knative.dev/visibility"] = "cluster-local"
 	}
 	kserviceObjectMeta := cfg.buildSvcObjectMeta(kserviceLabels)
 
 	revisionLabels := clone(cfg.Labels)
 	kserviceSpec := cfg.buildSvcSpec(revisionLabels)
 
-	return &knservingv1.Service{
+	svc := &knservingv1.Service{
 		ObjectMeta: *kserviceObjectMeta,
 		Spec:       *kserviceSpec,
 	}
-
+	// Call setDefaults on desired knative service here to avoid diffs generated because knative defaulter webhook
+	// is called when creating or updating the knative service. Ref: https://github.com/kserve/kserve/blob/v0.8.0
+	// /pkg/controller/v1beta1/inferenceservice/reconcilers/knative/ksvc_reconciler.go#L159
+	svc.SetDefaults(context.TODO())
+	return svc
 }
 
 func (cfg *KnativeService) buildSvcObjectMeta(labels map[string]string) *metav1.ObjectMeta {

api/turing/cluster/knative_service_test.go

@@ -85,7 +85,17 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 	}
 
 	// Expected specs
+	var defaultConcurrency, defaultTrafficPercent int64 = 0, 100
+	var defaultLatestRevision bool = true
 	var timeout int64 = 30
+	defaultRouteSpec := knservingv1.RouteSpec{
+		Traffic: []knservingv1.TrafficTarget{
+			{
+				LatestRevision: &defaultLatestRevision,
+				Percent:        &defaultTrafficPercent,
+			},
+		},
+	}
 	resources := corev1.ResourceRequirements{
 		Limits: map[corev1.ResourceName]resource.Quantity{
 			corev1.ResourceCPU:    resource.MustParse("600m"),
@@ -113,6 +123,7 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 	podSpec := corev1.PodSpec{
 		Containers: []corev1.Container{
 			{
+				Name:  "user-container",
 				Image: "asia.gcr.io/gcp-project-id/turing-router:latest",
 				Ports: []corev1.ContainerPort{
 					{
@@ -141,6 +152,7 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 					},
 					InitialDelaySeconds: 20,
 					PeriodSeconds:       10,
+					SuccessThreshold:    1,
 					TimeoutSeconds:      5,
 					FailureThreshold:    5,
 				},
@@ -171,8 +183,8 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 					Name:      "test-svc",
 					Namespace: "test-namespace",
 					Labels: map[string]string{
-						"labelKey":                       "labelVal",
-						"serving.knative.dev/visibility": "cluster-local",
+						"labelKey":                          "labelVal",
+						"networking.knative.dev/visibility": "cluster-local",
 					},
 				},
 				Spec: knservingv1.ServiceSpec{
@@ -192,11 +204,13 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 								},
 							},
 							Spec: knservingv1.RevisionSpec{
-								PodSpec:        podSpec,
-								TimeoutSeconds: &timeout,
+								PodSpec:              podSpec,
+								TimeoutSeconds:       &timeout,
+								ContainerConcurrency: &defaultConcurrency,
 							},
 						},
 					},
+					RouteSpec: defaultRouteSpec,
 				},
 			},
 		},
@@ -234,11 +248,13 @@ func TestBuildKnativeServiceConfig(t *testing.T) {
 								},
 							},
 							Spec: knservingv1.RevisionSpec{
-								PodSpec:        podSpec,
-								TimeoutSeconds: &timeout,
+								PodSpec:              podSpec,
+								TimeoutSeconds:       &timeout,
+								ContainerConcurrency: &defaultConcurrency,
 							},
 						},
 					},
+					RouteSpec: defaultRouteSpec,
 				},
 			},
 		},