diff --git a/pkg/reconciler/revision/resources/deploy.go b/pkg/reconciler/revision/resources/deploy.go index 117c5bd21ef0..13880c270033 100644 --- a/pkg/reconciler/revision/resources/deploy.go +++ b/pkg/reconciler/revision/resources/deploy.go @@ -138,15 +138,19 @@ func certVolume(secret string) corev1.Volume { } } -func rewriteUserLivenessProbe(p *corev1.Probe, userPort int) { +func rewriteUserLivenessProbe(p *corev1.Probe, userPort int, allowCustomPort bool) { if p == nil { return } switch { case p.HTTPGet != nil: - p.HTTPGet.Port = intstr.FromInt(userPort) + if !allowCustomPort || p.HTTPGet.Port.IntValue() == 0 { + p.HTTPGet.Port = intstr.FromInt(userPort) + } case p.TCPSocket != nil: - p.TCPSocket.Port = intstr.FromInt(userPort) + if !allowCustomPort || p.TCPSocket.Port.IntValue() == 0 { + p.TCPSocket.Port = intstr.FromInt(userPort) + } } } @@ -206,7 +210,7 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error) extraVolumes = append(extraVolumes, certVolume(networking.ServingCertName)) } - podSpec := BuildPodSpec(rev, append(BuildUserContainers(rev), *queueContainer), cfg) + podSpec := BuildPodSpec(rev, append(BuildUserContainers(rev, cfg.Features), *queueContainer), cfg) podSpec.Volumes = append(podSpec.Volumes, extraVolumes...) if val := cfg.Deployment.PodRuntimeClassName(rev.ObjectMeta.Labels); podSpec.RuntimeClassName == nil { @@ -237,12 +241,14 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error) } // BuildUserContainers makes an array of containers from the Revision template. -func BuildUserContainers(rev *v1.Revision) []corev1.Container { +// features may be nil, in which case default behavior (strict port rewriting) is used. +func BuildUserContainers(rev *v1.Revision, features *apiconfig.Features) []corev1.Container { + allowCustomPort := features != nil && features.MultiContainerProbing == apiconfig.Enabled 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) + container = makeServingContainer(*rev.Spec.PodSpec.Containers[i].DeepCopy(), rev, allowCustomPort) } else { container = makeContainer(*rev.Spec.PodSpec.Containers[i].DeepCopy(), rev) } @@ -283,7 +289,7 @@ func makeContainer(container corev1.Container, rev *v1.Revision) corev1.Containe return container } -func makeServingContainer(servingContainer corev1.Container, rev *v1.Revision) corev1.Container { +func makeServingContainer(servingContainer corev1.Container, rev *v1.Revision, allowCustomPort bool) 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 @@ -291,7 +297,7 @@ func makeServingContainer(servingContainer corev1.Container, rev *v1.Revision) c servingContainer.Env = append(servingContainer.Env, buildUserPortEnv(userPortStr)) container := makeContainer(servingContainer, rev) // If the user provides a liveness probe, we should rewrite in the port on the user-container for them. - rewriteUserLivenessProbe(container.LivenessProbe, int(userPort)) + rewriteUserLivenessProbe(container.LivenessProbe, int(userPort), allowCustomPort) return container } diff --git a/pkg/reconciler/revision/resources/deploy_test.go b/pkg/reconciler/revision/resources/deploy_test.go index 73c16c0181a5..55a61165efcc 100644 --- a/pkg/reconciler/revision/resources/deploy_test.go +++ b/pkg/reconciler/revision/resources/deploy_test.go @@ -1077,6 +1077,161 @@ func TestMakePodSpec(t *testing.T) { ), queueContainer(), }), + }, { + name: "with HTTP liveness probe on custom port and multi-container-probing enabled", + fc: apicfg.Features{ + MultiContainerProbing: apicfg.Enabled, + }, + rev: revision("bar", "foo", + withContainers([]corev1.Container{{ + Name: servingContainerName, + Image: "busybox", + ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort), + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8081), + }, + }, + }, + }}), + WithContainerStatuses([]v1.ContainerStatus{{ + ImageDigest: "busybox@sha256:deadbeef", + }}), + ), + want: podSpec( + []corev1.Container{ + servingContainer( + func(container *corev1.Container) { + container.Image = "busybox@sha256:deadbeef" + }, + withLivenessProbe(corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8081), + }, + }), + ), + queueContainer( + withEnvVar("ENABLE_MULTI_CONTAINER_PROBES", "true"), + withEnvVar("SERVING_READINESS_PROBE", `[{"tcpSocket":{"port":8080,"host":"127.0.0.1"}}]`), + ), + }), + }, { + name: "with TCP liveness probe on custom port and multi-container-probing enabled", + fc: apicfg.Features{ + MultiContainerProbing: apicfg.Enabled, + }, + rev: revision("bar", "foo", + withContainers([]corev1.Container{{ + Name: servingContainerName, + Image: "busybox", + ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort), + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt32(8081), + }, + }, + }, + }}), + WithContainerStatuses([]v1.ContainerStatus{{ + ImageDigest: "busybox@sha256:deadbeef", + }}), + ), + want: podSpec( + []corev1.Container{ + servingContainer( + func(container *corev1.Container) { + container.Image = "busybox@sha256:deadbeef" + }, + withLivenessProbe(corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt32(8081), + }, + }), + ), + queueContainer( + withEnvVar("ENABLE_MULTI_CONTAINER_PROBES", "true"), + withEnvVar("SERVING_READINESS_PROBE", `[{"tcpSocket":{"port":8080,"host":"127.0.0.1"}}]`), + ), + }), + }, { + name: "with HTTP liveness probe without port and multi-container-probing enabled", + fc: apicfg.Features{ + MultiContainerProbing: apicfg.Enabled, + }, + rev: revision("bar", "foo", + withContainers([]corev1.Container{{ + Name: servingContainerName, + Image: "busybox", + ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort), + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + }, + }, + }, + }}), + WithContainerStatuses([]v1.ContainerStatus{{ + ImageDigest: "busybox@sha256:deadbeef", + }}), + ), + want: podSpec( + []corev1.Container{ + servingContainer( + func(container *corev1.Container) { + container.Image = "busybox@sha256:deadbeef" + }, + withLivenessProbe(corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(v1.DefaultUserPort), + }, + }), + ), + queueContainer( + withEnvVar("ENABLE_MULTI_CONTAINER_PROBES", "true"), + withEnvVar("SERVING_READINESS_PROBE", `[{"tcpSocket":{"port":8080,"host":"127.0.0.1"}}]`), + ), + }), + }, { + name: "with HTTP liveness probe on custom port and multi-container-probing disabled", + rev: revision("bar", "foo", + withContainers([]corev1.Container{{ + Name: servingContainerName, + Image: "busybox", + ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort), + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(8081), + }, + }, + }, + }}), + WithContainerStatuses([]v1.ContainerStatus{{ + ImageDigest: "busybox@sha256:deadbeef", + }}), + ), + want: podSpec( + []corev1.Container{ + servingContainer( + func(container *corev1.Container) { + container.Image = "busybox@sha256:deadbeef" + }, + withLivenessProbe(corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt32(v1.DefaultUserPort), + }, + }), + ), + queueContainer(), + }), }, { name: "with HTTP startup probe", rev: revision("bar", "foo", diff --git a/pkg/webhook/podspec_dryrun.go b/pkg/webhook/podspec_dryrun.go index 852efc101b61..07081fc1455d 100644 --- a/pkg/webhook/podspec_dryrun.go +++ b/pkg/webhook/podspec_dryrun.go @@ -59,7 +59,7 @@ func validatePodSpec(ctx context.Context, ps v1.RevisionSpec, namespace string) Spec: ps, } rev.SetDefaults(ctx) - podSpec := resources.BuildPodSpec(rev, resources.BuildUserContainers(rev), nil /*configs*/) + podSpec := resources.BuildPodSpec(rev, resources.BuildUserContainers(rev, nil /*features*/), nil /*configs*/) // Make a sample pod with the template Revisions & PodSpec and dryrun call to API-server pod := &corev1.Pod{