Add multi container changes to deploy PodSpec

pkg/apis/serving/v1/revision_helpers.go

@@ -69,11 +69,19 @@ func (e LastPinnedParseError) Error() string {
 }
 
 // GetContainer returns a pointer to the relevant corev1.Container field.
-// It is never nil and should be exactly the specified container as guaranteed
-// by validation.
+// It is never nil and should be exactly the specified container if len(containers) == 1 or
+// if there are multiple containers it returns the container which has Ports
+// as guaranteed by validation.
 func (rs *RevisionSpec) GetContainer() *corev1.Container {
-	if len(rs.Containers) > 0 {
+	switch {
+	case len(rs.Containers) == 1:
 		return &rs.Containers[0]
+	case len(rs.Containers) > 1:
+		for i := range rs.Containers {
+			if len(rs.Containers[i].Ports) != 0 {
+				return &rs.Containers[i]
+			}
+		}
 	}
 	// Should be unreachable post-validation, but here to ease testing.
 	return &corev1.Container{}

pkg/apis/serving/v1/revision_lifecycle_test.go

@@ -765,12 +765,15 @@ func TestGetContainer(t *testing.T) {
 			Image: "foo",
 		},
 	}, {
-		name: "get first container info even after passing multiple",
+		name: "get serving container info even if there are multiple containers",
 		status: RevisionSpec{
 			PodSpec: corev1.PodSpec{
 				Containers: []corev1.Container{{
 					Name:  "firstContainer",
 					Image: "firstImage",
+					Ports: []corev1.ContainerPort{{
+						ContainerPort: 8888,
+					}},
 				}, {
 					Name:  "secondContainer",
 					Image: "secondImage",
@@ -780,7 +783,24 @@ func TestGetContainer(t *testing.T) {
 		want: &corev1.Container{
 			Name:  "firstContainer",
 			Image: "firstImage",
+			Ports: []corev1.ContainerPort{{
+				ContainerPort: 8888,
+			}},
+		},
+	}, {
+		name: "get empty container when passed multiple containers without the container port",
+		status: RevisionSpec{
+			PodSpec: corev1.PodSpec{
+				Containers: []corev1.Container{{
+					Name:  "firstContainer",
+					Image: "firstImage",
+				}, {
+					Name:  "secondContainer",
+					Image: "secondImage",
+				}},
+			},
 		},
+		want: &corev1.Container{},
 	}}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {

pkg/apis/serving/v1alpha1/revision_lifecycle.go

@@ -70,14 +70,22 @@ func (r *Revision) GetGroupVersionKind() schema.GroupVersionKind {
 }
 
 // GetContainer returns a pointer to the relevant corev1.Container field.
-// It is never nil and should be exactly the specified container as guaranteed
-// by validation.
+// It is never nil and should be exactly the specified container if len(containers) == 1 or
+// if there are multiple containers it returns the container which has Ports
+// as guaranteed by validation.
 func (rs *RevisionSpec) GetContainer() *corev1.Container {
 	if rs.DeprecatedContainer != nil {
 		return rs.DeprecatedContainer
 	}
-	if len(rs.Containers) > 0 {
+	switch {
+	case len(rs.Containers) == 1:
 		return &rs.Containers[0]
+	case len(rs.Containers) > 1:
+		for i := range rs.Containers {
+			if len(rs.Containers[i].Ports) != 0 {
+				return &rs.Containers[i]
+			}
+		}
 	}
 	// Should be unreachable post-validation, but is here to ease testing.
 	return &corev1.Container{}

pkg/apis/serving/v1alpha1/revision_lifecycle_test.go

@@ -759,13 +759,32 @@ func TestGetContainer(t *testing.T) {
 			Image: "foo",
 		},
 	}, {
-		name: "get first container info even after passing multiple",
+		name: "get container info",
+		status: RevisionSpec{
+			RevisionSpec: v1.RevisionSpec{
+				PodSpec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:  "servingContainer",
+						Image: "firstImage",
+					}},
+				},
+			},
+		},
+		want: &corev1.Container{
+			Name:  "servingContainer",
+			Image: "firstImage",
+		},
+	}, {
+		name: "get serving container info even if there are multiple containers",
 		status: RevisionSpec{
 			RevisionSpec: v1.RevisionSpec{
 				PodSpec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Name:  "firstContainer",
 						Image: "firstImage",
+						Ports: []corev1.ContainerPort{{
+							ContainerPort: 8888,
+						}},
 					}, {
 						Name:  "secondContainer",
 						Image: "secondImage",
@@ -776,7 +795,26 @@ func TestGetContainer(t *testing.T) {
 		want: &corev1.Container{
 			Name:  "firstContainer",
 			Image: "firstImage",
+			Ports: []corev1.ContainerPort{{
+				ContainerPort: 8888,
+			}},
+		},
+	}, {
+		name: "get empty container when passed multiple containers without the container port",
+		status: RevisionSpec{
+			RevisionSpec: v1.RevisionSpec{
+				PodSpec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:  "firstContainer",
+						Image: "firstImage",
+					}, {
+						Name:  "secondContainer",
+						Image: "secondImage",
+					}},
+				},
+			},
 		},
+		want: &corev1.Container{},
 	}}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {

pkg/reconciler/revision/resources/deploy.go

@@ -126,8 +126,7 @@ func makePodSpec(rev *v1.Revision, loggingConfig *logging.Config, tracingConfig
 		return nil, fmt.Errorf("failed to create queue-proxy container: %w", err)
 	}
 
-	userContainer := BuildUserContainer(rev)
-	podSpec := BuildPodSpec(rev, []corev1.Container{*userContainer, *queueContainer})
+	podSpec := BuildPodSpec(rev, append(BuildUserContainers(rev), *queueContainer))
 
 	if autoscalerConfig.EnableGracefulScaledown {
 		podSpec.Volumes = append(podSpec.Volumes, labelVolume)
@@ -136,49 +135,65 @@ func makePodSpec(rev *v1.Revision, loggingConfig *logging.Config, tracingConfig
 	return podSpec, nil
 }
 
-// BuildUserContainer makes a container from the Revision template.
-func BuildUserContainer(rev *v1.Revision) *corev1.Container {
-	userContainer := rev.Spec.GetContainer().DeepCopy()
+// BuildUserContainers makes an array of containers from the Revision template.
+func BuildUserContainers(rev *v1.Revision) []corev1.Container {
+	containers := make([]corev1.Container, 0, len(rev.Spec.PodSpec.Containers))
+	for i := range rev.Spec.PodSpec.Containers {
+		var container corev1.Container
+		if len(rev.Spec.PodSpec.Containers[i].Ports) != 0 || len(rev.Spec.PodSpec.Containers) == 1 {
+			container = makeServingContainer(*rev.Spec.PodSpec.Containers[i].DeepCopy(), rev)
+		} else {
+			container = makeContainer(*rev.Spec.PodSpec.Containers[i].DeepCopy(), rev)
+		}
+		// The below logic is safe because the image digests in Status.ContainerStatus will have been resolved
+		// before this method is called. We check for an empty array here because the method can also be
+		// called during DryRun, where ContainerStatuses will not yet have been resolved.
+		if len(rev.Status.ContainerStatuses) != 0 {
+			if rev.Status.ContainerStatuses[i].ImageDigest != "" {
+				container.Image = rev.Status.ContainerStatuses[i].ImageDigest
+			}
+		}
+		containers = append(containers, container)
+	}
+	return containers
+}
+
+func makeContainer(container corev1.Container, rev *v1.Revision) corev1.Container {
 	// Adding or removing an overwritten corev1.Container field here? Don't forget to
 	// update the fieldmasks / validations in pkg/apis/serving
 	varLogMount := varLogVolumeMount.DeepCopy()
-	varLogMount.SubPathExpr += userContainer.Name
-
-	userContainer.VolumeMounts = append(userContainer.VolumeMounts, *varLogMount)
-	userContainer.Lifecycle = userLifecycle
-	userPort := getUserPort(rev)
-	userPortInt := int(userPort)
-	userPortStr := strconv.Itoa(userPortInt)
-	// Replacement is safe as only up to a single port is allowed on the Revision
-	userContainer.Ports = buildContainerPorts(userPort)
-	userContainer.Env = append(userContainer.Env, buildUserPortEnv(userPortStr))
-	userContainer.Env = append(userContainer.Env, getKnativeEnvVar(rev)...)
-	userContainer.Env = append(userContainer.Env, buildVarLogSubpathEnvs()...)
+	varLogMount.SubPathExpr += container.Name
 
+	container.VolumeMounts = append(container.VolumeMounts, *varLogMount)
+	container.Lifecycle = userLifecycle
+	container.Env = append(container.Env, getKnativeEnvVar(rev)...)
+	container.Env = append(container.Env, buildVarLogSubpathEnvs()...)
 	// Explicitly disable stdin and tty allocation
-	userContainer.Stdin = false
-	userContainer.TTY = false
-
-	// Prefer imageDigest from revision if available
-	if rev.Status.DeprecatedImageDigest != "" {
-		userContainer.Image = rev.Status.DeprecatedImageDigest
-	}
-
-	if userContainer.TerminationMessagePolicy == "" {
-		userContainer.TerminationMessagePolicy = corev1.TerminationMessageFallbackToLogsOnError
+	container.Stdin = false
+	container.TTY = false
+	if container.TerminationMessagePolicy == "" {
+		container.TerminationMessagePolicy = corev1.TerminationMessageFallbackToLogsOnError
 	}
+	return container
+}
 
-	if userContainer.ReadinessProbe != nil {
-		if userContainer.ReadinessProbe.HTTPGet != nil || userContainer.ReadinessProbe.TCPSocket != nil {
+func makeServingContainer(servingContainer corev1.Container, rev *v1.Revision) corev1.Container {
+	userPort := getUserPort(rev)
+	userPortStr := strconv.Itoa(int(userPort))
+	// Replacement is safe as only up to a single port is allowed on the Revision
+	servingContainer.Ports = buildContainerPorts(userPort)
+	servingContainer.Env = append(servingContainer.Env, buildUserPortEnv(userPortStr))
+	container := makeContainer(servingContainer, rev)
+	if container.ReadinessProbe != nil {
+		if container.ReadinessProbe.HTTPGet != nil || container.ReadinessProbe.TCPSocket != nil {
 			// HTTP and TCP ReadinessProbes are executed by the queue-proxy directly against the
 			// user-container instead of via kubelet.
-			userContainer.ReadinessProbe = nil
+			container.ReadinessProbe = nil
 		}
 	}
-
 	// If the client provides probes, we should fill in the port for them.
-	rewriteUserProbe(userContainer.LivenessProbe, userPortInt)
-	return userContainer
+	rewriteUserProbe(container.LivenessProbe, int(userPort))
+	return container
 }
 
 // BuildPodSpec creates a PodSpec from the given revision and containers.