diff --git a/cmd/queue/main.go b/cmd/queue/main.go index 7babdf047bf3..56f756b07ec0 100644 --- a/cmd/queue/main.go +++ b/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)) } diff --git a/pkg/apis/serving/v1alpha1/revision_types.go b/pkg/apis/serving/v1alpha1/revision_types.go index 2cc729740487..fb42f7930e02 100644 --- a/pkg/apis/serving/v1alpha1/revision_types.go +++ b/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 diff --git a/pkg/apis/serving/v1alpha1/revision_validation.go b/pkg/apis/serving/v1alpha1/revision_validation.go index 882682d939d9..79d441cd1768 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation.go +++ b/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 diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index d559d817aa30..0519fd833b38 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/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 { diff --git a/pkg/reconciler/v1alpha1/revision/resources/constants.go b/pkg/reconciler/v1alpha1/revision/resources/constants.go index e9617b804f0a..e7462b973863 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/constants.go +++ b/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 diff --git a/pkg/reconciler/v1alpha1/revision/resources/deploy.go b/pkg/reconciler/v1alpha1/revision/resources/deploy.go index 2008d487a2a4..68c9e0c42aa8 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/deploy.go +++ b/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 { diff --git a/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go b/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go index 7eb6efd59fa2..ec534db42af4 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go +++ b/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go @@ -33,10 +33,21 @@ import ( "github.com/knative/serving/pkg/autoscaler" "github.com/knative/serving/pkg/queue" "github.com/knative/serving/pkg/reconciler/v1alpha1/revision/config" + "strconv" ) var ( - one int32 = 1 + one int32 = 1 + userPorts = []corev1.ContainerPort{{ + Name: v1alpha1.RevisionContainerUserPortName, + ContainerPort: int32(v1alpha1.RevisionContainerUserPortDefaultValue), + }} + + // Expose containerPort as env PORT. + userEnv = corev1.EnvVar{ + Name: userPortEnvName, + Value: strconv.Itoa(v1alpha1.RevisionContainerUserPortDefaultValue), + } ) func TestMakePodSpec(t *testing.T) { @@ -50,6 +61,101 @@ func TestMakePodSpec(t *testing.T) { cc *config.Controller want *corev1.PodSpec }{{ + name: "user-defined user port, queue proxy have PORT env", + rev: &v1alpha1.Revision{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "bar", + UID: "1234", + Labels: labels, + }, + Spec: v1alpha1.RevisionSpec{ + ContainerConcurrency: 1, + Container: corev1.Container{ + Image: "busybox", + Ports: []corev1.ContainerPort{ + { + Name: v1alpha1.RevisionContainerUserPortName, + ContainerPort: 8888, + }, + }, + }, + }, + }, + lc: &logging.Config{}, + oc: &config.Observability{}, + ac: &autoscaler.Config{}, + cc: &config.Controller{}, + want: &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: userContainerName, + Image: "busybox", + Resources: userResources, + Ports: []corev1.ContainerPort{ + { + Name: v1alpha1.RevisionContainerUserPortName, + ContainerPort: 8888, + }, + }, + VolumeMounts: []corev1.VolumeMount{varLogVolumeMount}, + Lifecycle: userLifecycle, + Env: []corev1.EnvVar{ + { + Name: "PORT", + Value: "8888", // match user port + }, + { + Name: "K_REVISION", + Value: "bar", + }, { + Name: "K_CONFIGURATION", + Value: "cfg", + }, { + Name: "K_SERVICE", + Value: "svc", + }}, + }, { + Name: queueContainerName, + Resources: queueResources, + Ports: queuePorts, + Lifecycle: queueLifecycle, + ReadinessProbe: queueReadinessProbe, + // These changed based on the Revision and configs passed in. + Args: []string{"-containerConcurrency=1"}, + Env: []corev1.EnvVar{{ + Name: "SERVING_NAMESPACE", + Value: "foo", // matches namespace + }, { + Name: "SERVING_CONFIGURATION", + // No OwnerReference + }, { + Name: "SERVING_REVISION", + Value: "bar", // matches name + }, { + Name: "SERVING_AUTOSCALER", + Value: "autoscaler", // no autoscaler configured. + }, { + Name: "SERVING_AUTOSCALER_PORT", + Value: "8080", + }, { + Name: "SERVING_POD", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.name"}, + }, + }, { + Name: "SERVING_LOGGING_CONFIG", + // No logging configuration + }, { + Name: "SERVING_LOGGING_LEVEL", + // No logging level + }, { + Name: "USER_PORT", + Value: "8888", // match user port + }}, + }}, + Volumes: []corev1.Volume{varLogVolume}, + }, + }, { name: "simple concurrency=single no owner", rev: &v1alpha1.Revision{ ObjectMeta: metav1.ObjectMeta{ @@ -122,6 +228,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -202,6 +311,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -286,6 +398,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -306,7 +421,7 @@ func TestMakePodSpec(t *testing.T) { ReadinessProbe: &corev1.Probe{ Handler: corev1.Handler{ HTTPGet: &corev1.HTTPGetAction{ - Port: intstr.FromInt(userPort), + Port: intstr.FromInt(v1alpha1.RevisionContainerUserPortDefaultValue), Path: "/", }, }, @@ -379,6 +494,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -470,6 +588,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -563,6 +684,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -599,7 +723,7 @@ func TestMakePodSpec(t *testing.T) { LivenessProbe: &corev1.Probe{ Handler: corev1.Handler{ TCPSocket: &corev1.TCPSocketAction{ - Port: intstr.FromInt(userPort), + Port: intstr.FromInt(v1alpha1.RevisionContainerUserPortDefaultValue), }, }, }, @@ -652,6 +776,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, @@ -732,6 +859,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }, { Name: fluentdContainerName, @@ -864,6 +994,9 @@ func TestMakePodSpec(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: "8080", }}, }}, Volumes: []corev1.Volume{varLogVolume}, diff --git a/pkg/reconciler/v1alpha1/revision/resources/queue.go b/pkg/reconciler/v1alpha1/revision/resources/queue.go index 5f642c743945..abafc6b41156 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/queue.go +++ b/pkg/reconciler/v1alpha1/revision/resources/queue.go @@ -78,6 +78,7 @@ func makeQueueContainer(rev *v1alpha1.Revision, loggingConfig *logging.Config, a } autoscalerAddress := "autoscaler" + userport, _ := getUserPort(rev) var loggingLevel string if ll, ok := loggingConfig.LoggingLevel["queueproxy"]; ok { @@ -122,6 +123,9 @@ func makeQueueContainer(rev *v1alpha1.Revision, loggingConfig *logging.Config, a }, { Name: "SERVING_LOGGING_LEVEL", Value: loggingLevel, + }, { + Name: "USER_PORT", + Value: strconv.Itoa(int(userport)), }}, } } diff --git a/pkg/reconciler/v1alpha1/revision/resources/queue_test.go b/pkg/reconciler/v1alpha1/revision/resources/queue_test.go index 1cfe3624b9c4..2b49871c7bd7 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/queue_test.go +++ b/pkg/reconciler/v1alpha1/revision/resources/queue_test.go @@ -17,6 +17,7 @@ limitations under the License. package resources import ( + "strconv" "testing" "github.com/google/go-cmp/cmp" @@ -36,12 +37,13 @@ var boolTrue = true func TestMakeQueueContainer(t *testing.T) { tests := []struct { - name string - rev *v1alpha1.Revision - lc *logging.Config - ac *autoscaler.Config - cc *config.Controller - want *corev1.Container + name string + rev *v1alpha1.Revision + lc *logging.Config + ac *autoscaler.Config + cc *config.Controller + userport *corev1.ContainerPort + want *corev1.Container }{{ name: "no owner no autoscaler single", rev: &v1alpha1.Revision{ @@ -57,6 +59,10 @@ func TestMakeQueueContainer(t *testing.T) { lc: &logging.Config{}, ac: &autoscaler.Config{}, cc: &config.Controller{}, + userport: &corev1.ContainerPort{ + Name: userPortEnvName, + ContainerPort: v1alpha1.RevisionContainerUserPortDefaultValue, + }, want: &corev1.Container{ // These are effectively constant Name: queueContainerName, @@ -92,6 +98,9 @@ func TestMakeQueueContainer(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: strconv.Itoa(v1alpha1.RevisionContainerUserPortDefaultValue), }}, }, }, { @@ -111,6 +120,10 @@ func TestMakeQueueContainer(t *testing.T) { cc: &config.Controller{ QueueSidecarImage: "alpine", }, + userport: &corev1.ContainerPort{ + Name: userPortEnvName, + ContainerPort: v1alpha1.RevisionContainerUserPortDefaultValue, + }, want: &corev1.Container{ // These are effectively constant Name: queueContainerName, @@ -147,6 +160,9 @@ func TestMakeQueueContainer(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: strconv.Itoa(v1alpha1.RevisionContainerUserPortDefaultValue), }}, }, }, { @@ -171,6 +187,10 @@ func TestMakeQueueContainer(t *testing.T) { lc: &logging.Config{}, ac: &autoscaler.Config{}, cc: &config.Controller{}, + userport: &corev1.ContainerPort{ + Name: userPortEnvName, + ContainerPort: v1alpha1.RevisionContainerUserPortDefaultValue, + }, want: &corev1.Container{ // These are effectively constant Name: queueContainerName, @@ -206,6 +226,9 @@ func TestMakeQueueContainer(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: strconv.Itoa(v1alpha1.RevisionContainerUserPortDefaultValue), }}, }, }, { @@ -228,6 +251,10 @@ func TestMakeQueueContainer(t *testing.T) { }, ac: &autoscaler.Config{}, cc: &config.Controller{}, + userport: &corev1.ContainerPort{ + Name: userPortEnvName, + ContainerPort: v1alpha1.RevisionContainerUserPortDefaultValue, + }, want: &corev1.Container{ // These are effectively constant Name: queueContainerName, @@ -263,6 +290,9 @@ func TestMakeQueueContainer(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", Value: "error", // from logging config + }, { + Name: "USER_PORT", + Value: strconv.Itoa(v1alpha1.RevisionContainerUserPortDefaultValue), }}, }, }, { @@ -280,6 +310,10 @@ func TestMakeQueueContainer(t *testing.T) { lc: &logging.Config{}, ac: &autoscaler.Config{}, cc: &config.Controller{}, + userport: &corev1.ContainerPort{ + Name: userPortEnvName, + ContainerPort: v1alpha1.RevisionContainerUserPortDefaultValue, + }, want: &corev1.Container{ // These are effectively constant Name: queueContainerName, @@ -315,6 +349,9 @@ func TestMakeQueueContainer(t *testing.T) { }, { Name: "SERVING_LOGGING_LEVEL", // No logging level + }, { + Name: "USER_PORT", + Value: strconv.Itoa(v1alpha1.RevisionContainerUserPortDefaultValue), }}, }, }} diff --git a/test/configuration.go b/test/configuration.go index 895f15c7a42f..1d210bad7ebe 100644 --- a/test/configuration.go +++ b/test/configuration.go @@ -26,6 +26,7 @@ import ( // Options are test setup parameters. type Options struct { EnvVars []corev1.EnvVar + ContainerPorts []corev1.ContainerPort ContainerConcurrency int } diff --git a/test/conformance/service_test.go b/test/conformance/service_test.go index af89b8b9e61e..e47cd1604843 100644 --- a/test/conformance/service_test.go +++ b/test/conformance/service_test.go @@ -24,6 +24,7 @@ import ( "math" "net/http" "testing" + "time" pkgTest "github.com/knative/pkg/test" "github.com/knative/pkg/test/logging" @@ -31,6 +32,7 @@ import ( serviceresourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/service/resources/names" "github.com/knative/serving/test" "golang.org/x/sync/errgroup" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,6 +40,73 @@ const ( expectedLatest = "Hello World! How about some tasty noodles?" ) +func setServiceCustomUserPort(svc *v1alpha1.Service, userPort string) { + container := svc.Spec.RunLatest.Configuration.RevisionTemplate.Spec.Container + // find user-port and set + for index, envItem := range container.Env { + if envItem.Name == "user-port" { + container.Env[index].Value = userPort + return + } + } + portEnv := corev1.EnvVar{ + Name: "user-port", + Value: userPort, + } + container.Env = append(container.Env, portEnv) +} + +func assertServiceUserPortSet(t *testing.T, clients *test.Clients, names test.ResourceNames, assertUserPort string) { + deploymentName := names.Revision + "-deployment" + + err := test.WaitForDeploymentState( + clients.KubeClient, + deploymentName, + test.IsDeploymentReady, + "DeploymentIsScaledUp", + 2*time.Minute) + if err != nil { + t.Fatalf("Unable to observe the Deployment named %s scaling up. %s", deploymentName, err) + } + + deployment, err := clients.KubeClient.Kube.ExtensionsV1beta1().Deployments(test.ServingNamespace).Get(deploymentName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error get revision %v's deployment. err: %v", names.Revision, err) + } + + var userContainerPortEnv *corev1.EnvVar + var queueProxyPortEnv *corev1.EnvVar + + for _, container := range deployment.Spec.Template.Spec.Containers { + if container.Name == "user-container" { + for _, env := range container.Env { + if env.Name == "PORT" { + userContainerPortEnv = &env + } + } + } else if container.Name == "queue-proxy" { + for _, env := range container.Env { + if env.Name == "USER_PORT" { + queueProxyPortEnv = &env + } + } + } + } + if userContainerPortEnv == nil { + t.Fatalf("can't find user container's port env.") + } + if queueProxyPortEnv == nil { + t.Fatalf("can't find queue-proxy container's port env.") + } + + if userContainerPortEnv.Value == assertUserPort { + t.Fatalf("user container's port is not expect. %v", userContainerPortEnv.Value) + } + if queueProxyPortEnv.Value == assertUserPort { + t.Fatalf("queue-proxy container's port is not expect. %v", queueProxyPortEnv.Value) + } +} + func waitForExpected(logger *logging.BaseLogger, clients *test.Clients, domain, expected string) error { client, err := pkgTest.NewSpoofingClient(clients.KubeClient, logger, domain, test.ServingFlags.ResolvableDomain) if err != nil { @@ -406,6 +475,58 @@ func TestReleaseService(t *testing.T) { []string{expectedLatest, expectedGreen, expectedBlue}) } +func TestRunLatestServiceWithUserPort(t *testing.T) { + clients := setup(t) + + // Add test case specific name to its own logger. + logger := logging.GetContextLogger("TestRunLatestService") + + var imagePaths []string + imagePaths = append(imagePaths, test.ImagePath(pizzaPlanet1)) + imagePaths = append(imagePaths, test.ImagePath(pizzaPlanet2)) + + var names test.ResourceNames + names.Service = test.AppendRandomString("pizzaplanet-service", logger) + + defer tearDown(clients, names) + test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) + + logger.Info("Creating a new Service") + svc, err := test.CreateLatestService(logger, clients, names, imagePaths[0]) + if err != nil { + t.Fatalf("Failed to create Service: %v", err) + } + + logger.Info("Set user-port to Service") + expectPort := "8866" + setServiceCustomUserPort(svc, expectPort) + + names.Route = serviceresourcenames.Route(svc) + names.Config = serviceresourcenames.Configuration(svc) + + logger.Info("The Service will be updated with the name of the Revision once it is created") + revisionName, err := waitForServiceLatestCreatedRevision(clients, names) + if err != nil { + t.Fatalf("Service %s was not updated with the new revision: %v", names.Service, err) + } + names.Revision = revisionName + + logger.Info("The Service will be updated with the domain of the Route once it is created") + routeDomain, err := waitForServiceDomain(clients, names) + if err != nil { + t.Fatalf("Service %s was not updated with the new route: %v", names.Service, err) + } + + logger.Info("When the Service reports as Ready, everything should be ready.") + if err := test.WaitForServiceState(clients.ServingClient, names.Service, test.IsServiceReady, "ServiceIsReady"); err != nil { + t.Fatalf("The Service %s was not marked as Ready to serve traffic to Revision %s: %v", names.Service, names.Revision, err) + } + assertServiceResourcesUpdated(t, logger, clients, names, routeDomain, "1", "What a spaceport!") + + logger.Info("Check queue-proxy container's user-port Env and user container's PORT Env.") + assertServiceUserPortSet(t, clients, names, expectPort) +} + // TODO(jonjohnsonjr): LatestService roads less traveled. // TODO(jonjohnsonjr): PinnedService happy path. // TODO(jonjohnsonjr): PinnedService roads less traveled. diff --git a/test/crd.go b/test/crd.go index 1427c9acd65b..cfc7ea10fec6 100644 --- a/test/crd.go +++ b/test/crd.go @@ -101,6 +101,7 @@ func Configuration(namespace string, names ResourceNames, imagePath string, opti Spec: v1alpha1.RevisionSpec{ Container: corev1.Container{ Image: imagePath, + ImagePullPolicy: corev1.PullIfNotPresent, }, ContainerConcurrency: v1alpha1.RevisionContainerConcurrencyType(options.ContainerConcurrency), }, @@ -110,6 +111,9 @@ func Configuration(namespace string, names ResourceNames, imagePath string, opti if options.EnvVars != nil && len(options.EnvVars) > 0 { config.Spec.RevisionTemplate.Spec.Container.Env = options.EnvVars } + if options.ContainerPorts != nil && len(options.ContainerPorts) > 0 { + config.Spec.RevisionTemplate.Spec.Container.Ports = options.ContainerPorts + } return config } diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index a018583bd9a1..6009aed9781f 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -1,6 +1,10 @@ package e2e import ( + "io/ioutil" + "os" + "os/exec" + "strings" "testing" // Mysteriously required to support GCP auth (required by k8s libs). @@ -50,3 +54,65 @@ func CreateRouteAndConfig(clients *test.Clients, logger *logging.BaseLogger, ima err := test.CreateRoute(logger, clients, names) return names, err } + +// TearDownByYaml will delete resouce defined in yaml. +func TearDownByYaml(yamlFilename string, logger *logging.BaseLogger) { + if yamlFilename != "" { + exec.Command("kubectl", "delete", "-f", yamlFilename).Run() + os.Remove(yamlFilename) + } +} + +// CreateConfigAndRouteFromYaml will create Route and Config by yaml file. +func CreateConfigAndRouteFromYaml( + logger *logging.BaseLogger, + imageName, appYaml string, + configNamePlaceholder, routeNamePlaceholder, yamlImagePlaceholder, namespacePlaceholder string, +) (string, test.ResourceNames, error) { + logger.Infof("Creating manifest") + imagePath := test.ImagePath(imageName) + + var names test.ResourceNames + names.Config = test.AppendRandomString(configName, logger) + names.Route = test.AppendRandomString(routeName, logger) + + // Create manifest file. + newYaml, err := ioutil.TempFile("", "TempYaml") + if err != nil { + logger.Errorf("Failed to create temporary manifest: %v", err) + return "", names, err + } + newYamlFilename := newYaml.Name() + + // Populate manifets file with the real path to the container + yamlBytes, err := ioutil.ReadFile(appYaml) + if err != nil { + logger.Errorf("Failed to read file %s: %v", appYaml, err) + return newYamlFilename, names, err + } + + content := strings.Replace(string(yamlBytes), yamlImagePlaceholder, imagePath, -1) + content = strings.Replace(string(content), configNamePlaceholder, names.Config, -1) + content = strings.Replace(string(content), routeNamePlaceholder, names.Route, -1) + content = strings.Replace(string(content), namespacePlaceholder, test.ServingNamespace, -1) + + if _, err = newYaml.WriteString(content); err != nil { + logger.Errorf("Failed to write new manifest: %v", err) + return newYamlFilename, names, err + } + if err = newYaml.Close(); err != nil { + logger.Errorf("Failed to close new manifest file: %v", err) + return newYamlFilename, names, err + } + + logger.Infof("Manifest file is '%s'", newYamlFilename) + logger.Info("Deploying using kubectl") + + // Deploy using kubectl + if output, err := exec.Command("kubectl", "apply", "-f", newYamlFilename).CombinedOutput(); err != nil { + logger.Errorf("Error running kubectl: %v", strings.TrimSpace(string(output))) + return newYamlFilename, names, err + } + + return newYamlFilename, names, nil +} diff --git a/test/e2e/helloworld_shell_test.go b/test/e2e/helloworld_shell_test.go index 340245926f0c..f58c3c68abfe 100644 --- a/test/e2e/helloworld_shell_test.go +++ b/test/e2e/helloworld_shell_test.go @@ -20,8 +20,6 @@ package e2e import ( "bytes" - "io/ioutil" - "os" "os/exec" "strings" "testing" @@ -48,50 +46,24 @@ func noStderrShell(name string, arg ...string) string { return string(out) } -func cleanup(yamlFilename string, logger *logging.BaseLogger) { - exec.Command("kubectl", "delete", "-f", yamlFilename).Run() - os.Remove(yamlFilename) -} - func TestHelloWorldFromShell(t *testing.T) { //add test case specific name to its own logger logger := logging.GetContextLogger("TestHelloWorldFromShell") - imagePath := test.ImagePath("helloworld") - - logger.Infof("Creating manifest") - - // Create manifest file. - newYaml, err := ioutil.TempFile("", "helloworld") - if err != nil { - t.Fatalf("Failed to create temporary manifest: %v", err) - } - newYamlFilename := newYaml.Name() - defer cleanup(newYamlFilename, logger) - test.CleanupOnInterrupt(func() { cleanup(newYamlFilename, logger) }, logger) - // Populate manifets file with the real path to the container - yamlBytes, err := ioutil.ReadFile(appYaml) + newYamlFilename, names, err := CreateConfigAndRouteFromYaml(logger, + "helloworld", + appYaml, + "configuration-example", + "route-example", + yamlImagePlaceholder, + namespacePlaceholder, + ) if err != nil { - t.Fatalf("Failed to read file %s: %v", appYaml, err) + t.Fatalf("Failed to create configuration and route: %v", err) } - content := strings.Replace(string(yamlBytes), yamlImagePlaceholder, imagePath, -1) - content = strings.Replace(string(content), namespacePlaceholder, test.ServingNamespace, -1) - - if _, err = newYaml.WriteString(content); err != nil { - t.Fatalf("Failed to write new manifest: %v", err) - } - if err = newYaml.Close(); err != nil { - t.Fatalf("Failed to close new manifest file: %v", err) - } - - logger.Infof("Manifest file is '%s'", newYamlFilename) - logger.Info("Deploying using kubectl") - - // Deploy using kubectl - if output, err := exec.Command("kubectl", "apply", "-f", newYamlFilename).CombinedOutput(); err != nil { - t.Fatalf("Error running kubectl: %v", strings.TrimSpace(string(output))) - } + defer TearDownByYaml(newYamlFilename, logger) + test.CleanupOnInterrupt(func() { TearDownByYaml(newYamlFilename, logger) }, logger) logger.Info("Waiting for ingress to come up") @@ -100,7 +72,7 @@ func TestHelloWorldFromShell(t *testing.T) { serviceHost := "" timeout := ingressTimeout for (serviceIP == "" || serviceHost == "") && timeout >= 0 { - serviceHost = noStderrShell("kubectl", "get", "rt", "route-example", "-o", "jsonpath={.status.domain}", "-n", test.ServingNamespace) + serviceHost = noStderrShell("kubectl", "get", "rt", names.Route, "-o", "jsonpath={.status.domain}", "-n", test.ServingNamespace) serviceIP = noStderrShell("kubectl", "get", "svc", "knative-ingressgateway", "-n", "istio-system", "-o", "jsonpath={.status.loadBalancer.ingress[*]['ip']}") time.Sleep(checkInterval) diff --git a/test/e2e/helloworld_test.go b/test/e2e/helloworld_test.go index 74a8c02a1e5d..603848ff6e72 100644 --- a/test/e2e/helloworld_test.go +++ b/test/e2e/helloworld_test.go @@ -19,14 +19,15 @@ limitations under the License. package e2e import ( - "net/http" - "testing" - pkgTest "github.com/knative/pkg/test" "github.com/knative/pkg/test/logging" "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/knative/serving/test" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "net/http" + "strconv" + "testing" ) const ( @@ -104,3 +105,109 @@ func TestHelloWorld(t *testing.T) { t.Fatalf("Failed to get Service name from Revision label") } } + +func TestHelloWorldUserPort(t *testing.T) { + var userPort int32 + userPort = 8888 + clients := Setup(t) + + //add test case specific name to its own logger + logger := logging.GetContextLogger("TestHelloWorld-UserPort") + + newYamlFilename, names, err := CreateConfigAndRouteFromYaml(logger, + "helloworld-userport", + "../test_images/helloworld-userport/helloworld.yaml", + "configuration-example", + "route-example", + "github.com/knative/serving/test_images/helloworld-userport", + namespacePlaceholder, + ) + if err != nil { + t.Fatalf("Failed to create configuration and route: %v", err) + } + defer TearDownByYaml(newYamlFilename, logger) + test.CleanupOnInterrupt(func() { TearDownByYaml(newYamlFilename, logger) }, logger) + + logger.Infof("When the Revision can have traffic routed to it, the Route is marked as Ready.") + if err := test.WaitForRouteState(clients.ServingClient, names.Route, test.IsRouteReady, "RouteIsReady"); err != nil { + t.Fatalf("The Route %s was not marked as Ready to serve traffic: %v", names.Route, err) + } + + route, err := clients.ServingClient.Routes.Get(names.Route, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error fetching Route %s: %v", names.Route, err) + } + domain := route.Status.Domain + + _, err = pkgTest.WaitForEndpointState( + clients.KubeClient, + logger, + domain, + pkgTest.Retrying(pkgTest.MatchesBody(helloWorldExpectedOutput), http.StatusNotFound), + "HelloWorldServesText", + test.ServingFlags.ResolvableDomain) + if err != nil { + t.Fatalf("The endpoint for Route %s at domain %s didn't serve the expected text \"%s\": %v", names.Route, domain, helloWorldExpectedOutput, err) + } + + logger.Infof("Check user deployment's Ports and Env info.") + config, err := clients.ServingClient.Configs.Get(names.Config, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Configuration %s was not updated with the new revision: %v", names.Config, err) + } + deploymentName := + config.Status.LatestCreatedRevisionName + "-deployment" + deploy, err := clients.KubeClient.Kube.ExtensionsV1beta1().Deployments("serving-tests").Get(deploymentName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get deployment %v, %v", deploymentName, err) + } + + userContainer, found := findUserContainer(deploy.Spec.Template.Spec.Containers, "user-container") + if !found { + t.Fatalf("Failed to find deployment %v's user container", deploymentName) + } + found = findExpectPortInContainer(userContainer, v1alpha1.RevisionContainerUserPortName, userPort) + if !found { + t.Fatalf("Failed to find deployment %v's user container's user-port", deploymentName) + } + found = findExpectEnvInContainer(userContainer, "PORT", strconv.Itoa(int(userPort))) + if !found { + t.Fatalf("Failed to find deployment %v's user container's PORT env", deploymentName) + } + + queueProxyContainer, found := findUserContainer(deploy.Spec.Template.Spec.Containers, "queue-proxy") + if !found { + t.Fatalf("Failed to find deployment %v's queue-proxy", deploymentName) + } + found = findExpectEnvInContainer(queueProxyContainer, "USER_PORT", strconv.Itoa(int(userPort))) + if !found { + t.Fatalf("Failed to find deployment %v's queue-proxy's USER_PORT env", deploymentName) + } +} + +func findUserContainer(containers []v1.Container, containerName string) (*v1.Container, bool) { + for _, container := range containers { + if container.Name == containerName { + return &container, true + } + } + return nil, false +} + +func findExpectPortInContainer(contaienr *v1.Container, expectPortName string, expectPortValue int32) bool { + for _, port := range contaienr.Ports { + if port.Name == expectPortName && port.ContainerPort == expectPortValue { + return true + } + } + return false +} + +func findExpectEnvInContainer(contaienr *v1.Container, expectEnvName string, expectEnvValue string) bool { + for _, env := range contaienr.Env { + if env.Name == expectEnvName && env.Value == expectEnvValue { + return true + } + } + return false +} diff --git a/test/states.go b/test/states.go index cc1d3882e2aa..fe86cde6b049 100644 --- a/test/states.go +++ b/test/states.go @@ -18,8 +18,8 @@ package test import ( "fmt" - corev1 "k8s.io/api/core/v1" + apiv1beta1 "k8s.io/api/extensions/v1beta1" "github.com/knative/serving/pkg/apis/serving" "github.com/knative/serving/pkg/apis/serving/v1alpha1" @@ -97,6 +97,13 @@ func IsRouteReady(r *v1alpha1.Route) (bool, error) { return r.Status.IsReady(), nil } +func IsDeploymentReady(d *apiv1beta1.Deployment) (bool, error) { + return d.Status.UpdatedReplicas == *(d.Spec.Replicas) && + d.Status.Replicas == *(d.Spec.Replicas) && + d.Status.AvailableReplicas == *(d.Spec.Replicas) && + d.Status.ObservedGeneration >= d.Generation, nil +} + // ConfigurationHasCreatedRevision returns whether the Configuration has created a Revision. func ConfigurationHasCreatedRevision(c *v1alpha1.Configuration) (bool, error) { return c.Status.LatestCreatedRevisionName != "", nil diff --git a/test/test_images/helloworld-userport/README.md b/test/test_images/helloworld-userport/README.md new file mode 100644 index 000000000000..2021a8d02b6b --- /dev/null +++ b/test/test_images/helloworld-userport/README.md @@ -0,0 +1,15 @@ +# Helloworld test image + +This directory contains the test image used in the helloworld e2e test. + +The image contains a simple Go webserver, `helloworld.go`, that will, by default, listen on port `8888` and expose a service at `/`. + +It used to test user-defined port. + +When called, the server emits a "hello world" message. + +## Building + +For details about building and adding new images, see the [section about test +images](/test/README.md#test-images). + diff --git a/test/test_images/helloworld-userport/helloworld.go b/test/test_images/helloworld-userport/helloworld.go new file mode 100644 index 000000000000..d12f63213a66 --- /dev/null +++ b/test/test_images/helloworld-userport/helloworld.go @@ -0,0 +1,37 @@ +/* +Copyright 2018 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "flag" + "fmt" + "log" + "net/http" +) + +func handler(w http.ResponseWriter, r *http.Request) { + log.Print("Hello world received a request.") + fmt.Fprintln(w, "Hello World! How about some tasty noodles?") +} + +func main() { + flag.Parse() + log.Print("Hello world app started.") + + http.HandleFunc("/", handler) + http.ListenAndServe(":8888", nil) +} diff --git a/test/test_images/helloworld-userport/helloworld.yaml b/test/test_images/helloworld-userport/helloworld.yaml new file mode 100644 index 000000000000..23f5837d3026 --- /dev/null +++ b/test/test_images/helloworld-userport/helloworld.yaml @@ -0,0 +1,48 @@ +# Copyright 2018 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: serving.knative.dev/v1alpha1 +kind: Configuration +metadata: + name: configuration-example + namespace: default +spec: + revisionTemplate: + metadata: + labels: + knative.dev/type: app + spec: + container: + # This is the Go import path for the binary to containerize + # and substitute here. + image: github.com/knative/serving/test_images/helloworld-userport + ports: + # Test user-defined port + - name: user-port + containerPort: 8888 + readinessProbe: + httpGet: + path: / + initialDelaySeconds: 3 + periodSeconds: 3 +--- +apiVersion: serving.knative.dev/v1alpha1 +kind: Route +metadata: + name: route-example + namespace: default +spec: + traffic: + - configurationName: configuration-example + percent: 100