Support set user-defined port to queue-proxy

cmd/queue/main.go

@@ -64,6 +64,7 @@ var (
 	servingRevisionKey    string
 	servingAutoscaler     string
 	servingAutoscalerPort string
+	userTargetPort        string
 	statChan              = make(chan *autoscaler.Stat, statReportingQueueLength)
 	reqChan               = make(chan queue.ReqEvent, requestCountingQueueLength)
 	statSink              *websocket.ManagedConnection
@@ -84,6 +85,7 @@ func initEnv() {
 	servingRevision = util.GetRequiredEnvOrFatal("SERVING_REVISION", logger)
 	servingAutoscaler = util.GetRequiredEnvOrFatal("SERVING_AUTOSCALER", logger)
 	servingAutoscalerPort = util.GetRequiredEnvOrFatal("SERVING_AUTOSCALER_PORT", logger)
+	userTargetPort = util.GetRequiredEnvOrFatal("USER_PORT", logger)
 
 	// TODO(mattmoor): Move this key to be in terms of the KPA.
 	servingRevisionKey = autoscaler.NewKpaKey(servingNamespace, servingRevision)
@@ -225,7 +227,7 @@ func main() {
 		zap.String(logkey.Key, servingRevisionKey),
 		zap.String(logkey.Pod, podName))
 
-	target, err := url.Parse("http://localhost:8080")
+	target, err := url.Parse(fmt.Sprintf("http://localhost:%s", userTargetPort))
 	if err != nil {
 		logger.Fatal("Failed to parse localhost url", zap.Error(err))
 	}

pkg/apis/serving/v1alpha1/revision_types.go

@@ -119,6 +119,11 @@ const (
 	RevisionContainerConcurrencyMax RevisionContainerConcurrencyType = 1000
 )
 
+const (
+	RevisionContainerUserPortName         = "user-port"
+	RevisionContainerUserPortDefaultValue = 8080
+)
+
 // RevisionSpec holds the desired state of the Revision (from the client).
 type RevisionSpec struct {
 	// TODO: Generation does not work correctly with CRD. They are scrubbed

pkg/apis/serving/v1alpha1/revision_validation.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"fmt"
 	"strconv"
 
 	"github.com/google/go-cmp/cmp"
@@ -109,9 +110,6 @@ func validateContainer(container corev1.Container) *apis.FieldError {
 	if !equality.Semantic.DeepEqual(container.Resources, corev1.ResourceRequirements{}) {
 		ignoredFields = append(ignoredFields, "resources")
 	}
-	if len(container.Ports) > 0 {
-		ignoredFields = append(ignoredFields, "ports")
-	}
 	if len(container.VolumeMounts) > 0 {
 		ignoredFields = append(ignoredFields, "volumeMounts")
 	}
@@ -123,6 +121,9 @@ func validateContainer(container corev1.Container) *apis.FieldError {
 		// Complain about all ignored fields so that user can remove them all at once.
 		errs = errs.Also(apis.ErrDisallowedFields(ignoredFields...))
 	}
+	if err := validateContainerPorts(container.Ports); err != nil {
+		errs = errs.Also(err.ViaField("ports"))
+	}
 	// Validate our probes
 	if err := validateProbe(container.ReadinessProbe).ViaField("readinessProbe"); err != nil {
 		errs = errs.Also(err)
@@ -133,6 +134,28 @@ func validateContainer(container corev1.Container) *apis.FieldError {
 	return errs
 }
 
+func validateContainerPorts(ports []corev1.ContainerPort) *apis.FieldError {
+	// user can set container port which names "user-port" to define application's port.
+	// Queue-proxy will use it to send requests to application
+	// if user didn't set any port, it will set default port user-port=8080.
+	if len(ports) > 1 {
+		return &apis.FieldError{
+			Message: "More than one container ports is set",
+			Paths:   []string{apis.CurrentField},
+			Details: "Only a single port named \"user-port\" is allowed",
+		}
+	}
+
+	if len(ports) == 1 && ports[0].Name != "user-port" {
+		return &apis.FieldError{
+			Message: fmt.Sprintf("Port name %v is not allowed", ports[0].Name),
+			Paths:   []string{apis.CurrentField},
+			Details: "Only a single port named \"user-port\" is allowed",
+		}
+	}
+	return nil
+}
+
 func validateBuildRef(buildRef *corev1.ObjectReference) *apis.FieldError {
 	if buildRef == nil {
 		return nil

pkg/apis/serving/v1alpha1/revision_validation_test.go

@@ -64,14 +64,34 @@ func TestContainerValidation(t *testing.T) {
 		},
 		want: apis.ErrDisallowedFields("resources"),
 	}, {
-		name: "has ports",
+		name: "has more than one port",
 		c: corev1.Container{
 			Ports: []corev1.ContainerPort{{
 				Name:          "http",
 				ContainerPort: 8080,
+			},{
+				Name:          "http2",
+				ContainerPort: 8181,
 			}},
 		},
-		want: apis.ErrDisallowedFields("ports"),
+		want: &apis.FieldError{
+			Message: "More than one container ports is set",
+			Paths:   []string{"ports"},
+			Details: "Only a single port named \"user-port\" is allowed",
+		},
+	}, {
+		name: "set wrong name port",
+		c: corev1.Container{
+			Ports: []corev1.ContainerPort{{
+				Name:          "http",
+				ContainerPort: 8080,
+			}},
+		},
+		want: &apis.FieldError{
+			Message: fmt.Sprintf("Port name %v is not allowed", "http"),
+			Paths:   []string{"ports"},
+			Details: "Only a single port named \"user-port\" is allowed",
+		},
 	}, {
 		name: "has volumeMounts",
 		c: corev1.Container{
@@ -151,7 +171,11 @@ func TestContainerValidation(t *testing.T) {
 			}},
 			Lifecycle: &corev1.Lifecycle{},
 		},
-		want: apis.ErrDisallowedFields("name", "resources", "ports", "volumeMounts", "lifecycle"),
+		want: apis.ErrDisallowedFields("name", "resources", "volumeMounts", "lifecycle").Also(&apis.FieldError{
+			Message: fmt.Sprintf("Port name %v is not allowed", "http"),
+			Paths:   []string{"ports"},
+			Details: "Only a single port named \"user-port\" is allowed",
+		}),
 	}}
 
 	for _, test := range tests {

pkg/reconciler/v1alpha1/revision/resources/constants.go

@@ -30,8 +30,6 @@ const (
 	// TODO(mattmoor): Make this private once we remove revision_test.go
 	IstioOutboundIPRangeAnnotation = "traffic.sidecar.istio.io/includeOutboundIPRanges"
 
-	userPortName    = "user-port"
-	userPort        = 8080
 	userPortEnvName = "PORT"
 
 	autoscalerPort = 8080

pkg/reconciler/v1alpha1/revision/resources/deploy.go

@@ -48,17 +48,6 @@ var (
 		MountPath: "/var/log",
 	}
 
-	userPorts = []corev1.ContainerPort{{
-		Name:          userPortName,
-		ContainerPort: int32(userPort),
-	}}
-
-	// Expose containerPort as env PORT.
-	userEnv = corev1.EnvVar{
-		Name:  userPortEnvName,
-		Value: strconv.Itoa(userPort),
-	}
-
 	userResources = corev1.ResourceRequirements{
 		Requests: corev1.ResourceList{
 			corev1.ResourceCPU: userContainerCPU,
@@ -81,7 +70,7 @@ var (
 	}
 )
 
-func rewriteUserProbe(p *corev1.Probe) {
+func rewriteUserProbe(p *corev1.Probe, userPort int) {
 	if p == nil {
 		return
 	}
@@ -101,19 +90,24 @@ func makePodSpec(rev *v1alpha1.Revision, loggingConfig *logging.Config, observab
 	// update the validations in pkg/webhook.validateContainer.
 	userContainer.Name = userContainerName
 	userContainer.Resources = userResources
-	userContainer.Ports = userPorts
 	userContainer.VolumeMounts = append(userContainer.VolumeMounts, varLogVolumeMount)
 	userContainer.Lifecycle = userLifecycle
-	userContainer.Env = append(userContainer.Env, userEnv)
+
+	userPort, found := getUserPort(rev)
+	if !found {
+		createAndSetDefaultUserPort(userContainer)
+	}
+	userContainer.Env = append(userContainer.Env, buildUserPortEnv(userPort))
 	userContainer.Env = append(userContainer.Env, getKnativeEnvVar(rev)...)
+
 	// Prefer imageDigest from revision if available
 	if rev.Status.ImageDigest != "" {
 		userContainer.Image = rev.Status.ImageDigest
 	}
 
 	// If the client provides probes, we should fill in the port for them.
-	rewriteUserProbe(userContainer.ReadinessProbe)
-	rewriteUserProbe(userContainer.LivenessProbe)
+	rewriteUserProbe(userContainer.ReadinessProbe, int(userPort))
+	rewriteUserProbe(userContainer.LivenessProbe, int(userPort))
 
 	podSpec := &corev1.PodSpec{
 		Containers: []corev1.Container{
@@ -133,6 +127,33 @@ func makePodSpec(rev *v1alpha1.Revision, loggingConfig *logging.Config, observab
 	return podSpec
 }
 
+func createAndSetDefaultUserPort(userContainer *corev1.Container) {
+	defaultUserPort := &corev1.ContainerPort{
+		Name:          v1alpha1.RevisionContainerUserPortName,
+		ContainerPort: int32(v1alpha1.RevisionContainerUserPortDefaultValue),
+	}
+	userContainer.Ports = append(userContainer.Ports, *defaultUserPort)
+}
+
+func getUserPort(rev *v1alpha1.Revision) (int32, bool) {
+	for _, port := range rev.Spec.Container.Ports {
+		if port.Name == v1alpha1.RevisionContainerUserPortName {
+			return port.ContainerPort, true
+		}
+	}
+
+	return v1alpha1.RevisionContainerUserPortDefaultValue, false
+}
+
+func buildUserPortEnv(userPort int32) corev1.EnvVar {
+	// Expose containerPort as env PORT.
+	userPortEnv := corev1.EnvVar{
+		Name:  userPortEnvName,
+		Value: strconv.Itoa(int(userPort)),
+	}
+	return userPortEnv
+}
+
 func MakeDeployment(rev *v1alpha1.Revision,
 	loggingConfig *logging.Config, networkConfig *config.Network, observabilityConfig *config.Observability,
 	autoscalerConfig *autoscaler.Config, controllerConfig *config.Controller) *appsv1.Deployment {