Only add KPA label to a K8S service of a revision if KPA is used (#3498)

* add logs * add pa class annotation * change comment * change test name * revert * add back test
knative · Mar 25, 2019 · 17b9066 · 17b9066
1 parent de40f6e
commit 17b9066
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 7 deletions.
diff --git a/pkg/reconciler/v1alpha1/configuration/resources/revision.go b/pkg/reconciler/v1alpha1/configuration/resources/revision.go
@@ -38,7 +38,6 @@ func MakeRevision(config *v1alpha1.Configuration, buildRef *corev1.ObjectReferen
 
 	UpdateRevisionLabels(rev, config)
 
-	// Populate the Configuration Generation annotation.
 	if rev.Annotations == nil {
 		rev.Annotations = make(map[string]string)
 	}

diff --git a/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go b/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go
@@ -175,8 +175,8 @@ func TestMakeRevisions(t *testing.T) {
 					serving.ConfigurationGenerationLabelKey:                   "100",
 					serving.DeprecatedConfigurationMetadataGenerationLabelKey: "100",
 					serving.ServiceLabelKey:                                   "",
-					"foo":                                                     "bar",
-					"baz":                                                     "blah",
+					"foo": "bar",
+					"baz": "blah",
 				},
 			},
 			Spec: v1alpha1.RevisionSpec{

diff --git a/pkg/reconciler/v1alpha1/revision/resources/service.go b/pkg/reconciler/v1alpha1/revision/resources/service.go
@@ -32,7 +32,12 @@ import (
 // serving.RevisionLabelKey label. Traffic is routed to queue-proxy port.
 func MakeK8sService(rev *v1alpha1.Revision) *corev1.Service {
 	labels := makeLabels(rev)
-	labels[autoscaling.KPALabelKey] = names.KPA(rev)
+	// Set KPALabelKey label if KPA is used for this Revision. If ClassAnnotationKey
+	// is empty, default to KPA class for backward compatibility.
+	if pa, ok := rev.Annotations[autoscaling.ClassAnnotationKey]; !ok || pa == autoscaling.KPA {
+		labels[autoscaling.KPALabelKey] = names.KPA(rev)
+	}
+
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            names.K8sService(rev),

diff --git a/pkg/reconciler/v1alpha1/revision/resources/service_test.go b/pkg/reconciler/v1alpha1/revision/resources/service_test.go
@@ -35,7 +35,7 @@ func TestMakeK8sService(t *testing.T) {
 		rev  *v1alpha1.Revision
 		want *corev1.Service
 	}{{
-		name: "name is bar",
+		name: "name is bar and use KPA by default",
 		rev: &v1alpha1.Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: "foo",
@@ -81,12 +81,15 @@ func TestMakeK8sService(t *testing.T) {
 			},
 		},
 	}, {
-		name: "name is baz",
+		name: "name is baz and use KPA explicitly",
 		rev: &v1alpha1.Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: "blah",
 				Name:      "baz",
 				UID:       "1234",
+				Annotations: map[string]string{
+					autoscaling.ClassAnnotationKey: autoscaling.KPA,
+				},
 			},
 			Spec: v1alpha1.RevisionSpec{
 				Container: corev1.Container{
@@ -106,7 +109,9 @@ func TestMakeK8sService(t *testing.T) {
 					serving.RevisionUID:      "1234",
 					AppLabelKey:              "baz",
 				},
-				Annotations: map[string]string{},
+				Annotations: map[string]string{
+					autoscaling.ClassAnnotationKey: autoscaling.KPA,
+				},
 				OwnerReferences: []metav1.OwnerReference{{
 					APIVersion:         v1alpha1.SchemeGroupVersion.String(),
 					Kind:               "Revision",
@@ -133,6 +138,56 @@ func TestMakeK8sService(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "use HPA explicitly",
+		rev: &v1alpha1.Revision{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "foo",
+				Name:      "bar",
+				UID:       "1234",
+				Annotations: map[string]string{
+					autoscaling.ClassAnnotationKey: autoscaling.HPA,
+				},
+			},
+		},
+		want: &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "foo",
+				Name:      "bar-service",
+				Labels: map[string]string{
+					serving.RevisionLabelKey: "bar",
+					serving.RevisionUID:      "1234",
+					AppLabelKey:              "bar",
+				},
+				Annotations: map[string]string{
+					autoscaling.ClassAnnotationKey: autoscaling.HPA,
+				},
+				OwnerReferences: []metav1.OwnerReference{{
+					APIVersion:         v1alpha1.SchemeGroupVersion.String(),
+					Kind:               "Revision",
+					Name:               "bar",
+					UID:                "1234",
+					Controller:         &boolTrue,
+					BlockOwnerDeletion: &boolTrue,
+				}},
+			},
+			Spec: corev1.ServiceSpec{
+				Ports: []corev1.ServicePort{{
+					Name:       ServicePortNameHTTP1,
+					Protocol:   corev1.ProtocolTCP,
+					Port:       ServicePort,
+					TargetPort: intstr.FromString(v1alpha1.RequestQueuePortName),
+				}, {
+					Name:       MetricsPortName,
+					Protocol:   corev1.ProtocolTCP,
+					Port:       MetricsPort,
+					TargetPort: intstr.FromString(v1alpha1.RequestQueueMetricsPortName),
+				}},
+				Selector: map[string]string{
+					serving.RevisionLabelKey: "bar",
+				},
+			},
+		},
 	}}
 
 	for _, test := range tests {