Allow the activator to deal with multiple service ports.

The activator today assumes that a service has exactly one port. An unnecessary restriction. This exposes a well-known port per service and allows the activator to deal with an arbitrary number of ports on the service side as long as the well-known port exists at least. Fixes knative#837
markusthoemmes · Sep 4, 2018 · bdaf5c9 · bdaf5c9
1 parent e5bbbad
commit bdaf5c9
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 9 deletions.
diff --git a/pkg/activator/revision.go b/pkg/activator/revision.go
@@ -23,6 +23,7 @@ import (
 	"github.com/knative/pkg/logging/logkey"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	clientset "github.com/knative/serving/pkg/client/clientset/versioned"
+	revisionresources "github.com/knative/serving/pkg/reconciler/v1alpha1/revision/resources"
 	revisionresourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/revision/resources/names"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
@@ -132,13 +133,19 @@ func (r *revisionActivator) getRevisionEndpoint(revision *v1alpha1.Revision) (en
 		return end, errors.Wrapf(err, "Unable to get service %s for revision", serviceName)
 	}
 
-	// TODO: in the future, the target service could have more than one port.
-	// https://github.com/knative/serving/issues/837
-	if len(svc.Spec.Ports) != 1 {
-		return end, fmt.Errorf("Revision needs one port. Found %v", len(svc.Spec.Ports))
-	}
 	fqdn := fmt.Sprintf("%s.%s.svc.cluster.local", serviceName, revision.Namespace)
-	port := svc.Spec.Ports[0].Port
+
+	// Search for the correct port in all the service ports.
+	port := int32(-1)
+	for _, p := range svc.Spec.Ports {
+		if p.Name == revisionresources.ServicePortName {
+			port = p.Port
+			break
+		}
+	}
+	if port == -1 {
+		return internalError("Revision needs external port. Found %v", len(svc.Spec.Ports))
+	}
 
 	return Endpoint{
 		FQDN: fqdn,

diff --git a/pkg/activator/revision_test.go b/pkg/activator/revision_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	clientset "github.com/knative/serving/pkg/client/clientset/versioned"
 	fakeKna "github.com/knative/serving/pkg/client/clientset/versioned/fake"
+	revisionresources "github.com/knative/serving/pkg/reconciler/v1alpha1/revision/resources"
 	. "github.com/knative/pkg/logging/testing"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -293,9 +294,13 @@ func newServiceBuilder() *serviceBuilder {
 			Spec: corev1.ServiceSpec{
 				Ports: []corev1.ServicePort{
 					{
-						Name: "http",
+						Name: revisionresources.ServicePortName,
 						Port: 8080,
 					},
+					{
+						Name: "anotherport",
+						Port: 9090,
+					},
 				},
 			},
 		},

diff --git a/pkg/reconciler/v1alpha1/revision/resources/constants.go b/pkg/reconciler/v1alpha1/revision/resources/constants.go
@@ -37,7 +37,11 @@ const (
 
 	// TODO(mattmoor): Make this private once we remove revision_test.go
 	AutoscalerPort       = 8080
-	ServicePort    int32 = 80
+
+	// ServicePortName is the name of the external port of the service
+	ServicePortName      = "http"
+	// ServicePort is the external port of the service
+	ServicePort          = int32(80)
 	AppLabelKey          = "app"
 )
 

diff --git a/pkg/reconciler/v1alpha1/revision/resources/service.go b/pkg/reconciler/v1alpha1/revision/resources/service.go
@@ -31,7 +31,7 @@ import (
 
 var (
 	servicePorts = []corev1.ServicePort{{
-		Name:       "http",
+		Name:       ServicePortName,
 		Protocol:   corev1.ProtocolTCP,
 		Port:       ServicePort,
 		TargetPort: intstr.IntOrString{Type: intstr.String, StrVal: queue.RequestQueuePortName},