diff --git a/docs/scaling/README.md b/docs/scaling/README.md index 59e69f3c4b6a..64aa38c95db2 100644 --- a/docs/scaling/README.md +++ b/docs/scaling/README.md @@ -4,8 +4,8 @@ There are three main components to the autoscaling system: autoscaler, activator ## Queue-proxy Whenever a user deploys an application, it is injected with a queue-proxy sidecar container. This queue-proxy does the following: - - Any request coming to the application user-container will go to the queue-proxy and then be routed to the user-container - - Any readiness and liveness probes defined by the user will be replaced with queue-proxy probes externally to the pod while the user-containers probe endpoints will only be accessed by the proxy. + - Any request coming to the application serving container will go to the queue-proxy and then be routed to the serving container. + - Any readiness and liveness probes defined by the user will be replaced with queue-proxy probes externally to the pod while the serving container probe endpoints will only be accessed by the proxy. - Makes sure that no more than the ‘defined container concurrency’ requests reach the application's instance at once by queueing other requests. For example if a revision defines a concurrency limit of 5, the queue-proxy makes sure that no more than 5 requests reach the application's instance at once. If there are more requests being sent to it than that, it will queue them locally. - Collects metrics about the load of requests the application container receives and reports the `average concurrency` and `requests per second` on a separate port. diff --git a/pkg/apis/config/defaults.go b/pkg/apis/config/defaults.go index df45e8ee0b8c..26fce6ca6820 100644 --- a/pkg/apis/config/defaults.go +++ b/pkg/apis/config/defaults.go @@ -56,9 +56,15 @@ const ( // specified by the user, if `name:` is omitted. DefaultInitContainerName = "init-container" + // DefaultServingContainerName is the default name we give to the container + // that serves traffic, if `name:` is omitted. + DefaultServingContainerName = "user-container" + // DefaultUserContainerName is the default name we give to the container // specified by the user, if `name:` is omitted. - DefaultUserContainerName = "user-container" + // Kept for compatibility with existing callers; new code should use + // DefaultServingContainerName. + DefaultUserContainerName = DefaultServingContainerName // DefaultContainerConcurrency is the default container concurrency. It will be set if ContainerConcurrency is not specified. DefaultContainerConcurrency = 0 @@ -73,8 +79,11 @@ const ( ) var ( - DefaultInitContainerNameTemplate = mustParseTemplate(DefaultInitContainerName) - DefaultUserContainerNameTemplate = mustParseTemplate(DefaultUserContainerName) + DefaultInitContainerNameTemplate = mustParseTemplate(DefaultInitContainerName) + DefaultServingContainerNameTemplate = mustParseTemplate(DefaultServingContainerName) + // Kept for compatibility with existing callers; new code should use + // DefaultServingContainerNameTemplate. + DefaultUserContainerNameTemplate = DefaultServingContainerNameTemplate ) func defaultDefaultsConfig() *Defaults { @@ -84,7 +93,7 @@ func defaultDefaultsConfig() *Defaults { RevisionResponseStartTimeoutSeconds: DefaultRevisionResponseStartTimeoutSeconds, RevisionIdleTimeoutSeconds: DefaultRevisionIdleTimeoutSeconds, InitContainerNameTemplate: DefaultInitContainerNameTemplate, - UserContainerNameTemplate: DefaultUserContainerNameTemplate, + UserContainerNameTemplate: DefaultServingContainerNameTemplate, ContainerConcurrency: DefaultContainerConcurrency, ContainerConcurrencyMaxLimit: DefaultMaxRevisionContainerConcurrency, AllowContainerConcurrencyZero: DefaultAllowContainerConcurrencyZero, @@ -226,9 +235,16 @@ func containerNameFromTemplate(ctx context.Context, tmpl *ObjectMetaTemplate) st return buf.String() } +// ServingContainerName returns the name of the serving container based on the context. +func (d Defaults) ServingContainerName(ctx context.Context) string { + return containerNameFromTemplate(ctx, d.UserContainerNameTemplate) +} + // UserContainerName returns the name of the user container based on the context. +// Kept for compatibility with existing callers; new code should use +// ServingContainerName. func (d Defaults) UserContainerName(ctx context.Context) string { - return containerNameFromTemplate(ctx, d.UserContainerNameTemplate) + return d.ServingContainerName(ctx) } // InitContainerName returns the name of the init container based on the context. diff --git a/pkg/apis/config/defaults_test.go b/pkg/apis/config/defaults_test.go index 5fb94099a86f..3b450e99b331 100644 --- a/pkg/apis/config/defaults_test.go +++ b/pkg/apis/config/defaults_test.go @@ -308,8 +308,8 @@ func TestTemplating(t *testing.T) { Namespace: "guardians", }) - if got, want := def.UserContainerName(ctx), test.want; got != want { - t.Errorf("UserContainerName() = %v, wanted %v", got, want) + if got, want := def.ServingContainerName(ctx), test.want; got != want { + t.Errorf("ServingContainerName() = %v, wanted %v", got, want) } if got, want := def.InitContainerName(ctx), test.want; got != want { diff --git a/pkg/apis/serving/k8s_validation.go b/pkg/apis/serving/k8s_validation.go index fd90e0f1583f..e291f7ed1af1 100644 --- a/pkg/apis/serving/k8s_validation.go +++ b/pkg/apis/serving/k8s_validation.go @@ -422,7 +422,7 @@ func ValidatePodSpec(ctx context.Context, ps corev1.PodSpec) *apis.FieldError { case 0: errs = errs.Also(apis.ErrMissingField("containers")) case 1: - errs = errs.Also(ValidateUserContainer(ctx, ps.Containers[0], volumes, port). + errs = errs.Also(ValidateServingContainer(ctx, ps.Containers[0], volumes, port). ViaFieldIndex("containers", 0)) default: errs = errs.Also(validateContainers(ctx, ps.Containers, volumes, port)) @@ -479,7 +479,7 @@ func validateContainers(ctx context.Context, containers []corev1.Container, volu if len(containers[i].Ports) == 0 { errs = errs.Also(validateSidecarContainer(WithinSidecarContainer(ctx), containers[i], volumes).ViaFieldIndex("containers", i)) } else { - errs = errs.Also(ValidateUserContainer(WithinUserContainer(ctx), containers[i], volumes, port).ViaFieldIndex("containers", i)) + errs = errs.Also(ValidateServingContainer(WithinServingContainer(ctx), containers[i], volumes, port).ViaFieldIndex("containers", i)) } } return errs @@ -585,8 +585,8 @@ func validateInitContainer(ctx context.Context, container corev1.Container, volu return errs.Also(validate(WithinInitContainer(ctx), container, volumes)) } -// ValidateUserContainer validate fields for serving containers -func ValidateUserContainer(ctx context.Context, container corev1.Container, volumes map[string]corev1.Volume, port corev1.ContainerPort) (errs *apis.FieldError) { +// ValidateServingContainer validates fields for containers that serve traffic. +func ValidateServingContainer(ctx context.Context, container corev1.Container, volumes map[string]corev1.Volume, port corev1.ContainerPort) (errs *apis.FieldError) { // Liveness Probes errs = errs.Also(validateProbe(container.LivenessProbe, &port, true).ViaField("livenessProbe")) // Readiness Probes @@ -594,6 +594,13 @@ func ValidateUserContainer(ctx context.Context, container corev1.Container, volu return errs.Also(validate(ctx, container, volumes)) } +// ValidateUserContainer validate fields for serving containers +// Kept for compatibility with existing callers; new code should use +// ValidateServingContainer. +func ValidateUserContainer(ctx context.Context, container corev1.Container, volumes map[string]corev1.Volume, port corev1.ContainerPort) (errs *apis.FieldError) { + return ValidateServingContainer(ctx, container, volumes, port) +} + func validate(ctx context.Context, container corev1.Container, volumes map[string]corev1.Volume) *apis.FieldError { if equality.Semantic.DeepEqual(container, corev1.Container{}) { return apis.ErrMissingField(apis.CurrentField) @@ -1035,14 +1042,22 @@ func warnDefaultContainerSecurityContext(ctx context.Context, psc *corev1.PodSec return errs } -// This is attached to contexts as they are passed down through a user container +// This is attached to contexts as they are passed down through a serving container // being validated. -type userContainer struct{} +type servingContainer struct{} + +// WithinServingContainer notes on the context that further validation or defaulting +// is within the context of a serving container in the revision. +func WithinServingContainer(ctx context.Context) context.Context { + return context.WithValue(ctx, servingContainer{}, struct{}{}) +} // WithinUserContainer notes on the context that further validation or defaulting // is within the context of a user container in the revision. +// Kept for compatibility with existing callers; new code should use +// WithinServingContainer. func WithinUserContainer(ctx context.Context) context.Context { - return context.WithValue(ctx, userContainer{}, struct{}{}) + return WithinServingContainer(ctx) } // This is attached to contexts as they are passed down through a sidecar container diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go index 711f7a565dea..18b81839ce02 100644 --- a/pkg/apis/serving/k8s_validation_test.go +++ b/pkg/apis/serving/k8s_validation_test.go @@ -2146,10 +2146,10 @@ func TestUserContainerValidation(t *testing.T) { } port, err := validateContainersPorts([]corev1.Container{test.c}) - got := err.Also(ValidateUserContainer(ctx, test.c, test.volumes, port)) + got := err.Also(ValidateServingContainer(ctx, test.c, test.volumes, port)) got = got.Filter(apis.ErrorLevel) if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { - t.Errorf("ValidateUserContainer (-want, +got): \n%s", diff) + t.Errorf("ValidateServingContainer (-want, +got): \n%s", diff) } }) } diff --git a/pkg/apis/serving/v1/revision_defaults.go b/pkg/apis/serving/v1/revision_defaults.go index be8c81238032..a81e2d1b4bb3 100644 --- a/pkg/apis/serving/v1/revision_defaults.go +++ b/pkg/apis/serving/v1/revision_defaults.go @@ -84,8 +84,8 @@ func (rs *RevisionSpec) SetDefaults(ctx context.Context) { for idx := range rs.PodSpec.InitContainers { containerNames.Insert(rs.PodSpec.InitContainers[idx].Name) } - defaultUserContainerName := cfg.Defaults.UserContainerName(ctx) - applyDefaultContainerNames(rs.PodSpec.Containers, containerNames, defaultUserContainerName) + defaultServingContainerName := cfg.Defaults.ServingContainerName(ctx) + applyDefaultContainerNames(rs.PodSpec.Containers, containerNames, defaultServingContainerName) defaultInitContainerName := cfg.Defaults.InitContainerName(ctx) applyDefaultContainerNames(rs.PodSpec.InitContainers, containerNames, defaultInitContainerName) for idx := range rs.PodSpec.Containers { @@ -134,7 +134,7 @@ func (rs *RevisionSpec) applyDefault(ctx context.Context, container *corev1.Cont // If there are multiple containers then default probes will be applied to the container where user specified PORT // default probes will not be applied for non serving containers if len(rs.PodSpec.Containers) == 1 || len(container.Ports) != 0 { - rs.applyUserContainerDefaultReadinessProbe(container) + rs.applyServingContainerDefaultReadinessProbe(container) } rs.applyReadinessProbeDefaults(container) rs.applyGRPCProbeDefaults(container) @@ -157,7 +157,7 @@ func (rs *RevisionSpec) applyDefault(ctx context.Context, container *corev1.Cont } } -func (*RevisionSpec) applyUserContainerDefaultReadinessProbe(container *corev1.Container) { +func (*RevisionSpec) applyServingContainerDefaultReadinessProbe(container *corev1.Container) { if container.ReadinessProbe == nil { container.ReadinessProbe = &corev1.Probe{} } diff --git a/pkg/reconciler/revision/resources/deploy.go b/pkg/reconciler/revision/resources/deploy.go index 63d398a6dc2c..c70037e82055 100644 --- a/pkg/reconciler/revision/resources/deploy.go +++ b/pkg/reconciler/revision/resources/deploy.go @@ -100,9 +100,9 @@ var ( // This PreStop hook is actually calling an endpoint on the queue-proxy // because of the way PreStop hooks are called by kubelet. We use this - // to block the user-container from exiting before the queue-proxy is ready + // to block the serving container from exiting before the queue-proxy is ready // to exit so we can guarantee that there are no more requests in flight. - userLifecycle = &corev1.Lifecycle{ + servingLifecycle = &corev1.Lifecycle{ PreStop: &corev1.LifecycleHandler{ HTTPGet: &corev1.HTTPGetAction{ Port: intstr.FromInt(networking.QueueAdminPort), @@ -110,6 +110,9 @@ var ( }, }, } + // Kept for compatibility with existing callers; new code should use + // servingLifecycle. + userLifecycle = servingLifecycle ) func addToken(tokenVolume *corev1.Volume, filename string, audience string, expiry *int64) { @@ -205,7 +208,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(BuildServingContainers(rev), *queueContainer), cfg) podSpec.Volumes = append(podSpec.Volumes, extraVolumes...) if val := cfg.Deployment.PodRuntimeClassName(rev.ObjectMeta.Labels); podSpec.RuntimeClassName == nil { @@ -235,8 +238,8 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error) return podSpec, nil } -// BuildUserContainers makes an array of containers from the Revision template. -func BuildUserContainers(rev *v1.Revision) []corev1.Container { +// BuildServingContainers makes an array of containers from the Revision template. +func BuildServingContainers(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 @@ -258,10 +261,17 @@ func BuildUserContainers(rev *v1.Revision) []corev1.Container { return containers } +// BuildUserContainers makes an array of containers from the Revision template. +// Kept for compatibility with existing callers; new code should use +// BuildServingContainers. +func BuildUserContainers(rev *v1.Revision) []corev1.Container { + return BuildServingContainers(rev) +} + 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 - container.Lifecycle = userLifecycle + container.Lifecycle = servingLifecycle container.Env = append(container.Env, getKnativeEnvVar(rev)...) // Explicitly disable stdin and tty allocation @@ -289,7 +299,7 @@ func makeServingContainer(servingContainer corev1.Container, rev *v1.Revision) c servingContainer.Ports = buildContainerPorts(userPort) 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. + // If the user provides a liveness probe, we should rewrite in the port on the serving container for them. rewriteUserLivenessProbe(container.LivenessProbe, int(userPort)) return container } diff --git a/pkg/webhook/podspec_dryrun.go b/pkg/webhook/podspec_dryrun.go index 852efc101b61..34c1139ddfd3 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.BuildServingContainers(rev), nil /*configs*/) // Make a sample pod with the template Revisions & PodSpec and dryrun call to API-server pod := &corev1.Pod{ diff --git a/test/conformance/runtime/liveness_probe_test.go b/test/conformance/runtime/liveness_probe_test.go index 74cd379decd2..60dd7aff6291 100644 --- a/test/conformance/runtime/liveness_probe_test.go +++ b/test/conformance/runtime/liveness_probe_test.go @@ -89,7 +89,7 @@ func TestLivenessWithFail(t *testing.T) { t.Fatalf("The endpoint for Route %s at %s didn't return success: %v", names.Route, url, err) } - // Check that user-container hasn't been restarted yet. + // Check that the serving container hasn't been restarted yet. deploymentName := resourcenames.Deployment(resources.Revision) podList, err := clients.KubeClient.CoreV1().Pods(test.ServingFlags.TestNamespace).List(context.Background(), metav1.ListOptions{}) if err != nil { @@ -97,8 +97,8 @@ func TestLivenessWithFail(t *testing.T) { } for i := range podList.Items { pod := &podList.Items[i] - if strings.Contains(pod.Name, deploymentName) && test.UserContainerRestarted(pod) { - t.Fatal("User container unexpectedly restarted") + if strings.Contains(pod.Name, deploymentName) && test.ServingContainerRestarted(pod) { + t.Fatal("Serving container unexpectedly restarted") } } @@ -122,21 +122,21 @@ func TestLivenessWithFail(t *testing.T) { t.Fatalf("POST to /start-failing failed: %v", err) } - // Wait for the user-container to be restarted. + // Wait for the serving container to be restarted. if err := pkgtest.WaitForPodListState( context.Background(), clients.KubeClient, func(p *corev1.PodList) (bool, error) { for i := range p.Items { pod := &p.Items[i] - if strings.Contains(pod.Name, deploymentName) && test.UserContainerRestarted(pod) { + if strings.Contains(pod.Name, deploymentName) && test.ServingContainerRestarted(pod) { return true, nil } } return false, nil }, "WaitForContainerRestart", test.ServingFlags.TestNamespace); err != nil { - t.Fatalf("Failed waiting for user-container to be restarted: %v", err) + t.Fatalf("Failed waiting for serving container to be restarted: %v", err) } // After restart, verify the liveness probe passes a few times again. diff --git a/test/e2e/readiness_test.go b/test/e2e/readiness_test.go index f2e85639eb8c..03dd01751f84 100644 --- a/test/e2e/readiness_test.go +++ b/test/e2e/readiness_test.go @@ -238,7 +238,7 @@ func TestLivenessProbeAwareOfStartupProbe(t *testing.T) { t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err) } - // Check that user-container hasn't been restarted. + // Check that the serving container hasn't been restarted. deploymentName := resourcenames.Deployment(resources.Revision) podList, err := clients.KubeClient.CoreV1().Pods(test.ServingFlags.TestNamespace).List(context.Background(), metav1.ListOptions{}) if err != nil { @@ -246,8 +246,8 @@ func TestLivenessProbeAwareOfStartupProbe(t *testing.T) { } for i := range podList.Items { pod := &podList.Items[i] - if strings.Contains(pod.Name, deploymentName) && test.UserContainerRestarted(pod) { - t.Fatal("User container unexpectedly restarted") + if strings.Contains(pod.Name, deploymentName) && test.ServingContainerRestarted(pod) { + t.Fatal("Serving container unexpectedly restarted") } } } diff --git a/test/util.go b/test/util.go index 2d72b9ad3937..215c9d83d464 100644 --- a/test/util.go +++ b/test/util.go @@ -108,12 +108,19 @@ func AddTestAnnotation(t testing.TB, m metav1.ObjectMeta) { }) } -// UserContainerRestarted checks if the container was restarted. -func UserContainerRestarted(pod *corev1.Pod) bool { +// ServingContainerRestarted checks if the serving container was restarted. +func ServingContainerRestarted(pod *corev1.Pod) bool { for _, status := range pod.Status.ContainerStatuses { - if status.Name == config.DefaultUserContainerName && status.RestartCount > 0 { + if status.Name == config.DefaultServingContainerName && status.RestartCount > 0 { return true } } return false } + +// UserContainerRestarted checks if the container was restarted. +// Kept for compatibility with existing callers; new code should use +// ServingContainerRestarted. +func UserContainerRestarted(pod *corev1.Pod) bool { + return ServingContainerRestarted(pod) +}