Remove outdated code from revision, etc dealing with services (#3888)

* Remove outdated code from revision, etc dealing with services

* remove the redundant code

pkg/activator/handler/handler.go

@@ -27,12 +27,12 @@ import (
 	"github.com/knative/pkg/logging/logkey"
 	"github.com/knative/serving/pkg/activator"
 	"github.com/knative/serving/pkg/activator/util"
+	"github.com/knative/serving/pkg/apis/networking"
 	"github.com/knative/serving/pkg/apis/serving"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	pkghttp "github.com/knative/serving/pkg/http"
 	"github.com/knative/serving/pkg/network"
 	"github.com/knative/serving/pkg/queue"
-	"github.com/knative/serving/pkg/reconciler/revision/resources"
 
 	"go.opencensus.io/plugin/ochttp"
 	"go.opencensus.io/trace"
@@ -227,7 +227,7 @@ func (a *ActivationHandler) serviceHostName(rev *v1alpha1.Revision, serviceName
 	// Search for the appropriate port
 	port := int32(-1)
 	for _, p := range svc.Spec.Ports {
-		if p.Name == resources.ServicePortName(rev) {
+		if p.Name == networking.ServicePortName(rev.GetProtocol()) {
 			port = p.Port
 			break
 		}

pkg/apis/autoscaling/v1alpha1/pa_types.go

@@ -87,10 +87,9 @@ type PodAutoscalerSpec struct {
 	// is responsible for quickly right-sizing.
 	ScaleTargetRef autoscalingv1.CrossVersionObjectReference `json:"scaleTargetRef"`
 
-	// ServiceName holds the name of a core Kubernetes Service resource that
+	// DeprecatedServiceName holds the name of a core Kubernetes Service resource that
 	// load balances over the pods referenced by the ScaleTargetRef.
-	// TODO(vagababov): deprecate.
-	ServiceName string `json:"serviceName"`
+	DeprecatedServiceName string `json:"serviceName"`
 
 	// The application-layer protocol. Matches `ProtocolType` inferred from the revision spec.
 	ProtocolType net.ProtocolType

pkg/apis/autoscaling/v1alpha1/pa_validation.go

@@ -71,9 +71,6 @@ func (rs *PodAutoscalerSpec) Validate(ctx context.Context) *apis.FieldError {
 		return apis.ErrMissingField(apis.CurrentField)
 	}
 	errs := validateReference(rs.ScaleTargetRef).ViaField("scaleTargetRef")
-	if rs.ServiceName == "" {
-		errs = errs.Also(apis.ErrMissingField("serviceName"))
-	}
 	errs = errs.Also(rs.ContainerConcurrency.Validate(ctx).
 		ViaField("containerConcurrency"))
 	return errs.Also(validateSKSFields(ctx, rs))

pkg/apis/autoscaling/v1alpha1/pa_validation_test.go

@@ -40,7 +40,6 @@ func TestPodAutoscalerSpecValidation(t *testing.T) {
 		name: "valid",
 		rs: &PodAutoscalerSpec{
 			ContainerConcurrency: 0,
-			ServiceName:          "foo",
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
@@ -51,15 +50,13 @@ func TestPodAutoscalerSpecValidation(t *testing.T) {
 	}, {
 		name: "has missing scaleTargetRef",
 		rs: &PodAutoscalerSpec{
-			ContainerConcurrency: 0,
-			ServiceName:          "foo",
+			ContainerConcurrency: 1,
 		},
 		want: apis.ErrMissingField("scaleTargetRef"),
 	}, {
 		name: "has missing scaleTargetRef kind",
 		rs: &PodAutoscalerSpec{
-			ContainerConcurrency: 0,
-			ServiceName:          "foo",
+			ContainerConcurrency: 1,
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 				APIVersion: "apps/v1",
 				Name:       "bar",
@@ -70,7 +67,6 @@ func TestPodAutoscalerSpecValidation(t *testing.T) {
 		name: "has missing scaleTargetRef apiVersion",
 		rs: &PodAutoscalerSpec{
 			ContainerConcurrency: 0,
-			ServiceName:          "foo",
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 				Kind: "Deployment",
 				Name: "bar",
@@ -81,29 +77,16 @@ func TestPodAutoscalerSpecValidation(t *testing.T) {
 		name: "has missing scaleTargetRef name",
 		rs: &PodAutoscalerSpec{
 			ContainerConcurrency: 0,
-			ServiceName:          "foo",
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
 			},
 		},
 		want: apis.ErrMissingField("scaleTargetRef.name"),
-	}, {
-		name: "has missing serviceName",
-		rs: &PodAutoscalerSpec{
-			ContainerConcurrency: 0,
-			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
-				APIVersion: "apps/v1",
-				Kind:       "Deployment",
-				Name:       "bar",
-			},
-		},
-		want: apis.ErrMissingField("serviceName"),
 	}, {
 		name: "bad container concurrency",
 		rs: &PodAutoscalerSpec{
 			ContainerConcurrency: -1,
-			ServiceName:          "foo",
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 				APIVersion: "apps/v1",
 				Kind:       "Deployment",
@@ -116,7 +99,6 @@ func TestPodAutoscalerSpecValidation(t *testing.T) {
 		name: "multi invalid, bad concurrency and missing ref kind",
 		rs: &PodAutoscalerSpec{
 			ContainerConcurrency: -2,
-			ServiceName:          "foo",
 			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 				APIVersion: "apps/v1",
 				Name:       "bar",
@@ -131,7 +113,7 @@ func TestPodAutoscalerSpecValidation(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			got := test.rs.Validate(context.Background())
 			if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
-				t.Errorf("Validate (-want, +got) = %v", diff)
+				t.Errorf("Validate (-want, +got) = %s", diff)
 			}
 		})
 	}
@@ -152,7 +134,6 @@ func TestPodAutoscalerValidation(t *testing.T) {
 				},
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -172,7 +153,6 @@ func TestPodAutoscalerValidation(t *testing.T) {
 				},
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -191,7 +171,6 @@ func TestPodAutoscalerValidation(t *testing.T) {
 				},
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -211,7 +190,6 @@ func TestPodAutoscalerValidation(t *testing.T) {
 				},
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -239,7 +217,6 @@ func TestPodAutoscalerValidation(t *testing.T) {
 			},
 			Spec: PodAutoscalerSpec{
 				ContainerConcurrency: -1,
-				ServiceName:          "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -274,7 +251,6 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -287,7 +263,6 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -303,7 +278,6 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -317,7 +291,6 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -333,7 +306,6 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -347,7 +319,6 @@ func TestImmutableFields(t *testing.T) {
 			},
 			Spec: PodAutoscalerSpec{
 				ContainerConcurrency: 1,
-				ServiceName:          "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -371,7 +342,6 @@ func TestImmutableFields(t *testing.T) {
 			},
 			Spec: PodAutoscalerSpec{
 				ContainerConcurrency: 0,
-				ServiceName:          "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -385,7 +355,6 @@ func TestImmutableFields(t *testing.T) {
 			},
 			Spec: PodAutoscalerSpec{
 				ContainerConcurrency: 1,
-				ServiceName:          "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -408,7 +377,7 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ServiceName: "foo",
+				DeprecatedServiceName: "foo",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -421,8 +390,8 @@ func TestImmutableFields(t *testing.T) {
 				Name: "valid",
 			},
 			Spec: PodAutoscalerSpec{
-				ContainerConcurrency: 1,
-				ServiceName:          "food",
+				ContainerConcurrency:  1,
+				DeprecatedServiceName: "food",
 				ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
 					APIVersion: "apps/v1",
 					Kind:       "Deployment",
@@ -439,7 +408,7 @@ func TestImmutableFields(t *testing.T) {
 {v1alpha1.PodAutoscalerSpec}.ScaleTargetRef.Name:
 	-: "baz"
 	+: "bar"
-{v1alpha1.PodAutoscalerSpec}.ServiceName:
+{v1alpha1.PodAutoscalerSpec}.DeprecatedServiceName:
 	-: "food"
 	+: "foo"
 `,

pkg/apis/networking/register.go

@@ -16,7 +16,9 @@ limitations under the License.
 
 package networking
 
-import "time"
+import (
+	"time"
+)
 
 const (
 	// GroupName is the name for the networking API group.
@@ -48,6 +50,11 @@ const (
 	// ServiceTypeKey is the label key attached to a service specifying the type of service.
 	// e.g. Public, Metrics
 	ServiceTypeKey = GroupName + "/serviceType"
+
+	// ServicePortNameHTTP1 is the name of the external port of the service for HTTP/1.1
+	ServicePortNameHTTP1 = "http"
+	// ServicePortNameH2C is the name of the external port of the service for HTTP/2
+	ServicePortNameH2C = "http2"
 )
 
 // ServiceType is the enumeration type for the Kubernetes services
@@ -74,3 +81,11 @@ var (
 	// DefaultRetryCount will be set if Attempts not specified.
 	DefaultRetryCount = 3
 )
+
+// ServicePortName returns the port for the app level protocol.
+func ServicePortName(proto ProtocolType) string {
+	if proto == ProtocolH2C {
+		return ServicePortNameH2C
+	}
+	return ServicePortNameHTTP1
+}

pkg/reconciler/autoscaling/hpa/hpa_test.go

@@ -439,7 +439,6 @@ func pa(name, namespace string, options ...PodAutoscalerOption) *autoscalingv1al
 				Kind:       "Deployment",
 				Name:       name + "-deployment",
 			},
-			ServiceName:  name + "-service",
 			ProtocolType: networking.ProtocolHTTP1,
 		},
 	}

pkg/reconciler/autoscaling/kpa/kpa_test.go

@@ -104,7 +104,6 @@ func TestMetricsSvcIsReconciled(t *testing.T) {
 	defer logtesting.ClearAll()
 
 	rev := newTestRevision(testNamespace, testRevision)
-	ep := addEndpoint(makeEndpoints(rev))
 	kpa := revisionresources.MakeKPA(rev)
 	tests := []struct {
 		name               string
@@ -283,8 +282,6 @@ func TestMetricsSvcIsReconciled(t *testing.T) {
 
 			servingClient.ServingV1alpha1().Revisions(testNamespace).Create(rev)
 			servingInformer.Serving().V1alpha1().Revisions().Informer().GetIndexer().Add(rev)
-			kubeClient.CoreV1().Endpoints(testNamespace).Create(ep)
-			kubeInformer.Core().V1().Endpoints().Informer().GetIndexer().Add(ep)
 			servingClient.AutoscalingV1alpha1().PodAutoscalers(testNamespace).Create(kpa)
 			servingInformer.Autoscaling().V1alpha1().PodAutoscalers().Informer().GetIndexer().Add(kpa)
 
@@ -386,15 +383,6 @@ func markResourceNotOwned(rType, name string) PodAutoscalerOption {
 	}
 }
 
-func makeTestEndpoints(num int, ns, n string) *corev1.Endpoints {
-	rev := newTestRevision(ns, n)
-	eps := makeEndpoints(rev)
-	for i := 0; i < num; i++ {
-		eps = addEndpoint(eps)
-	}
-	return eps
-}
-
 func TestReconcileAndScaleToZero(t *testing.T) {
 	// This test suite uses special decider that will
 	// force KPA to scale to 0.
@@ -1116,10 +1104,6 @@ func TestEmptyEndpoints(t *testing.T) {
 	rev := newTestRevision(testNamespace, testRevision)
 	servingClient.ServingV1alpha1().Revisions(testNamespace).Create(rev)
 	servingInformer.Serving().V1alpha1().Revisions().Informer().GetIndexer().Add(rev)
-	// This is empty still.
-	ep := makeEndpoints(rev)
-	kubeClient.CoreV1().Endpoints(testNamespace).Create(ep)
-	kubeInformer.Core().V1().Endpoints().Informer().GetIndexer().Add(ep)
 	kpa := revisionresources.MakeKPA(rev)
 	servingClient.AutoscalingV1alpha1().PodAutoscalers(testNamespace).Create(kpa)
 	servingInformer.Autoscaling().V1alpha1().PodAutoscalers().Informer().GetIndexer().Add(kpa)
@@ -1503,16 +1487,6 @@ func makeSKSPrivateEndpoints(num int, ns, n string) *corev1.Endpoints {
 	return eps
 }
 
-func makeEndpoints(rev *v1alpha1.Revision) *corev1.Endpoints {
-	service := revisionresources.MakeK8sService(rev)
-	return &corev1.Endpoints{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: service.Namespace,
-			Name:      service.Name,
-		},
-	}
-}
-
 func addEndpoint(ep *corev1.Endpoints) *corev1.Endpoints {
 	ep.Subsets = []corev1.EndpointSubset{{
 		Addresses: []corev1.EndpointAddress{{IP: "127.0.0.1"}},

pkg/reconciler/autoscaling/kpa/resources/service_test.go

@@ -49,7 +49,6 @@ func TestMakeService(t *testing.T) {
 				Kind:       "Deployment",
 				Name:       "with-you",
 			},
-			ServiceName: "with-you-service",
 		},
 	}
 	selector := map[string]string{"cant": "stop"}

pkg/reconciler/revision/resources/constants.go

@@ -38,11 +38,6 @@ const (
 
 	autoscalerPort = 8080
 
-	// ServicePortNameHTTP1 is the name of the external port of the service for HTTP/1.1
-	ServicePortNameHTTP1 = "http"
-	// ServicePortNameH2C is the name of the external port of the service for HTTP/2
-	ServicePortNameH2C = "http2"
-
 	// ServicePort is the external port of the service
 	ServicePort = int32(80)
 	AppLabelKey = "app"

pkg/reconciler/revision/resources/kpa.go

@@ -48,8 +48,7 @@ func MakeKPA(rev *v1alpha1.Revision) *kpa.PodAutoscaler {
 				Kind:       "Deployment",
 				Name:       names.Deployment(rev),
 			},
-			ServiceName:  names.K8sService(rev),
-			ProtocolType: rev.GetProtocol(),
+			ProtocolType:          rev.GetProtocol(),
 		},
 	}
 }