add flag and several fixes

config/core/configmaps/deployment.yaml

@@ -22,7 +22,7 @@ metadata:
     app.kubernetes.io/component: controller
     app.kubernetes.io/version: devel
   annotations:
-    knative.dev/example-checksum: "dffb11e5"
+    knative.dev/example-checksum: "ed77183a"
 data:
   # This is the Go import path for the binary that is containerized
   # and substituted here.
@@ -57,23 +57,26 @@ data:
     queue-sidecar-cpu-request: "25m"
 
     # Sets the queue proxy's CPU limit.
-    # If omitted, a default value (currently "1000m"), is used.
+    # If omitted, a default value (currently "1000m"), is used when
+    # `queueproxy.resource-defaults` is set to `Enabled`.
     queue-sidecar-cpu-limit: "1000m"
 
     # Sets the queue proxy's memory request.
-    # If omitted, a default value (currently "400Mi"), is used.
+    # If omitted, a default value (currently "400Mi"), is used when
+    # `queueproxy.resource-defaults` is set to `Enabled`.
     queue-sidecar-memory-request: "400Mi"
 
     # Sets the queue proxy's memory limit.
-    # If omitted, a default value (currently "800Mi"), is used.
+    # If omitted, a default value (currently "800Mi"), is used when
+    # `queueproxy.resource-defaults` is set to `Enabled`.
     queue-sidecar-memory-limit: "800Mi"
 
     # Sets the queue proxy's ephemeral storage request.
-    # If omitted, a default value (currently "512Mi"), is used.
+    # If omitted, no value is specified and the system default is used.
     queue-sidecar-ephemeral-storage-request: "512Mi"
 
     # Sets the queue proxy's ephemeral storage limit.
-    # If omitted, a default value (currently "1024Mi"), is used.
+    # If omitted, no value is specified and the system default is used.
     queue-sidecar-ephemeral-storage-limit: "1024Mi"
 
     # Sets tokens associated with specific audiences for queue proxy - used by QPOptions

config/core/configmaps/features.yaml

@@ -22,7 +22,7 @@ metadata:
     app.kubernetes.io/component: controller
     app.kubernetes.io/version: devel
   annotations:
-    knative.dev/example-checksum: "d3565159"
+    knative.dev/example-checksum: "eb70e734"
 data:
   _example: |-
     ################################
@@ -201,3 +201,6 @@ data:
     #
     # NOTE THAT THIS IS AN EXPERIMENTAL / ALPHA FEATURE
     queueproxy.mount-podinfo: "disabled"
+
+    # Default queue proxy resource requests and limits to good values for most cases if set.
+    queueproxy.resource-defaults: "disabled"

pkg/apis/config/features.go

@@ -67,6 +67,7 @@ func defaultFeaturesConfig() *Features {
 		PodSpecPersistentVolumeClaim:     Disabled,
 		PodSpecPersistentVolumeWrite:     Disabled,
 		QueueProxyMountPodInfo:           Disabled,
+		QueueProxyResourceDefaults:       Disabled,
 		PodSpecInitContainers:            Disabled,
 		PodSpecDNSPolicy:                 Disabled,
 		PodSpecDNSConfig:                 Disabled,
@@ -102,6 +103,7 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {
 		asFlag("kubernetes.podspec-dnsconfig", &nc.PodSpecDNSConfig),
 		asFlag("secure-pod-defaults", &nc.SecurePodDefaults),
 		asFlag("tag-header-based-routing", &nc.TagHeaderBasedRouting),
+		asFlag("queueproxy.resource-defaults", &nc.QueueProxyResourceDefaults),
 		asFlag("queueproxy.mount-podinfo", &nc.QueueProxyMountPodInfo),
 		asFlag("autodetect-http2", &nc.AutoDetectHTTP2)); err != nil {
 		return nil, err
@@ -134,6 +136,7 @@ type Features struct {
 	PodSpecPersistentVolumeClaim     Flag
 	PodSpecPersistentVolumeWrite     Flag
 	QueueProxyMountPodInfo           Flag
+	QueueProxyResourceDefaults       Flag
 	PodSpecDNSPolicy                 Flag
 	PodSpecDNSConfig                 Flag
 	SecurePodDefaults                Flag

pkg/apis/config/features_test.go

@@ -73,6 +73,7 @@ func TestFeaturesConfiguration(t *testing.T) {
 			PodSpecDNSPolicy:                 Enabled,
 			PodSpecDNSConfig:                 Enabled,
 			SecurePodDefaults:                Enabled,
+			QueueProxyResourceDefaults:       Enabled,
 			TagHeaderBasedRouting:            Enabled,
 		}),
 		data: map[string]string{
@@ -90,6 +91,7 @@ func TestFeaturesConfiguration(t *testing.T) {
 			"kubernetes.podspec-dnspolicy":                 "Enabled",
 			"kubernetes.podspec-dnsconfig":                 "Enabled",
 			"secure-pod-defaults":                          "Enabled",
+			"queueproxy.resource-defaults":                 "Enabled",
 			"tag-header-based-routing":                     "Enabled",
 		},
 	}, {

pkg/deployment/config.go

@@ -99,15 +99,10 @@ var (
 
 func defaultConfig() *Config {
 	cfg := &Config{
-		ProgressDeadline:                    ProgressDeadlineDefault,
-		DigestResolutionTimeout:             digestResolutionTimeoutDefault,
-		RegistriesSkippingTagResolving:      sets.NewString("kind.local", "ko.local", "dev.local"),
-		QueueSidecarCPURequest:              &QueueSidecarCPURequestDefault,
-		QueueSidecarCPULimit:                &QueueSidecarCPULimitDefault,
-		QueueSidecarMemoryRequest:           &QueueSidecarMemoryRequestDefault,
-		QueueSidecarMemoryLimit:             &QueueSidecarMemoryLimitDefault,
-		QueueSidecarEphemeralStorageRequest: &QueueSidecarEphemeralStorageRequestDefault,
-		QueueSidecarEphemeralStorageLimit:   &QueueSidecarEphemeralStorageLimitDefault,
+		ProgressDeadline:               ProgressDeadlineDefault,
+		DigestResolutionTimeout:        digestResolutionTimeoutDefault,
+		RegistriesSkippingTagResolving: sets.NewString("kind.local", "ko.local", "dev.local"),
+		QueueSidecarCPURequest:         &QueueSidecarCPURequestDefault,
 	}
 	// The following code is needed for ConfigMap testing.
 	// defaultConfig must match the example in deployment.yaml which includes: `queue-sidecar-token-audiences: ""`

pkg/deployment/config_test.go

@@ -62,6 +62,12 @@ func TestControllerConfigurationFromFile(t *testing.T) {
 		// We require QSI to be explicitly set. So do it here.
 		want.QueueSidecarImage = "ko://knative.dev/serving/cmd/queue"
 
+		// The following are in the example yaml, to show usage,
+		// but default is nil, i.e. inheriting k8s.
+		// So for this test we ignore those, but verify the other fields.
+		got.QueueSidecarCPULimit = nil
+		got.QueueSidecarMemoryRequest, got.QueueSidecarMemoryLimit = nil, nil
+		got.QueueSidecarEphemeralStorageRequest, got.QueueSidecarEphemeralStorageLimit = nil, nil
 		if !cmp.Equal(got, want) {
 			t.Error("Example stanza does not match default, diff(-want,+got):", cmp.Diff(want, got))
 		}
@@ -77,17 +83,12 @@ func TestControllerConfiguration(t *testing.T) {
 	}{{
 		name: "controller configuration with bad registries",
 		wantConfig: &Config{
-			RegistriesSkippingTagResolving:      sets.NewString("ko.local", ""),
-			DigestResolutionTimeout:             digestResolutionTimeoutDefault,
-			QueueSidecarImage:                   defaultSidecarImage,
-			QueueSidecarCPURequest:              &QueueSidecarCPURequestDefault,
-			QueueSidecarCPULimit:                &QueueSidecarCPULimitDefault,
-			QueueSidecarMemoryRequest:           &QueueSidecarMemoryRequestDefault,
-			QueueSidecarMemoryLimit:             &QueueSidecarMemoryLimitDefault,
-			QueueSidecarEphemeralStorageRequest: &QueueSidecarEphemeralStorageRequestDefault,
-			QueueSidecarEphemeralStorageLimit:   &QueueSidecarEphemeralStorageLimitDefault,
-			QueueSidecarTokenAudiences:          sets.NewString("foo", "bar", "boo-srv"),
-			ProgressDeadline:                    ProgressDeadlineDefault,
+			RegistriesSkippingTagResolving: sets.NewString("ko.local", ""),
+			DigestResolutionTimeout:        digestResolutionTimeoutDefault,
+			QueueSidecarImage:              defaultSidecarImage,
+			QueueSidecarCPURequest:         &QueueSidecarCPURequestDefault,
+			QueueSidecarTokenAudiences:     sets.NewString("foo", "bar", "boo-srv"),
+			ProgressDeadline:               ProgressDeadlineDefault,
 		},
 		data: map[string]string{
 			QueueSidecarImageKey:              defaultSidecarImage,
@@ -97,17 +98,12 @@ func TestControllerConfiguration(t *testing.T) {
 	}, {
 		name: "controller configuration good progress deadline",
 		wantConfig: &Config{
-			RegistriesSkippingTagResolving:      sets.NewString("kind.local", "ko.local", "dev.local"),
-			DigestResolutionTimeout:             digestResolutionTimeoutDefault,
-			QueueSidecarImage:                   defaultSidecarImage,
-			QueueSidecarCPURequest:              &QueueSidecarCPURequestDefault,
-			QueueSidecarCPULimit:                &QueueSidecarCPULimitDefault,
-			QueueSidecarMemoryRequest:           &QueueSidecarMemoryRequestDefault,
-			QueueSidecarMemoryLimit:             &QueueSidecarMemoryLimitDefault,
-			QueueSidecarEphemeralStorageRequest: &QueueSidecarEphemeralStorageRequestDefault,
-			QueueSidecarEphemeralStorageLimit:   &QueueSidecarEphemeralStorageLimitDefault,
-			QueueSidecarTokenAudiences:          sets.NewString(""),
-			ProgressDeadline:                    444 * time.Second,
+			RegistriesSkippingTagResolving: sets.NewString("kind.local", "ko.local", "dev.local"),
+			DigestResolutionTimeout:        digestResolutionTimeoutDefault,
+			QueueSidecarImage:              defaultSidecarImage,
+			QueueSidecarCPURequest:         &QueueSidecarCPURequestDefault,
+			QueueSidecarTokenAudiences:     sets.NewString(""),
+			ProgressDeadline:               444 * time.Second,
 		},
 		data: map[string]string{
 			QueueSidecarImageKey: defaultSidecarImage,
@@ -116,17 +112,12 @@ func TestControllerConfiguration(t *testing.T) {
 	}, {
 		name: "controller configuration good digest resolution timeout",
 		wantConfig: &Config{
-			RegistriesSkippingTagResolving:      sets.NewString("kind.local", "ko.local", "dev.local"),
-			DigestResolutionTimeout:             60 * time.Second,
-			QueueSidecarImage:                   defaultSidecarImage,
-			QueueSidecarCPURequest:              &QueueSidecarCPURequestDefault,
-			QueueSidecarCPULimit:                &QueueSidecarCPULimitDefault,
-			QueueSidecarMemoryRequest:           &QueueSidecarMemoryRequestDefault,
-			QueueSidecarMemoryLimit:             &QueueSidecarMemoryLimitDefault,
-			QueueSidecarEphemeralStorageRequest: &QueueSidecarEphemeralStorageRequestDefault,
-			QueueSidecarEphemeralStorageLimit:   &QueueSidecarEphemeralStorageLimitDefault,
-			QueueSidecarTokenAudiences:          sets.NewString(""),
-			ProgressDeadline:                    ProgressDeadlineDefault,
+			RegistriesSkippingTagResolving: sets.NewString("kind.local", "ko.local", "dev.local"),
+			DigestResolutionTimeout:        60 * time.Second,
+			QueueSidecarImage:              defaultSidecarImage,
+			QueueSidecarCPURequest:         &QueueSidecarCPURequestDefault,
+			QueueSidecarTokenAudiences:     sets.NewString(""),
+			ProgressDeadline:               ProgressDeadlineDefault,
 		},
 		data: map[string]string{
 			QueueSidecarImageKey:       defaultSidecarImage,
@@ -135,17 +126,12 @@ func TestControllerConfiguration(t *testing.T) {
 	}, {
 		name: "controller configuration with registries",
 		wantConfig: &Config{
-			RegistriesSkippingTagResolving:      sets.NewString("ko.local", "ko.dev"),
-			DigestResolutionTimeout:             digestResolutionTimeoutDefault,
-			QueueSidecarImage:                   defaultSidecarImage,
-			QueueSidecarCPURequest:              &QueueSidecarCPURequestDefault,
-			QueueSidecarCPULimit:                &QueueSidecarCPULimitDefault,
-			QueueSidecarMemoryRequest:           &QueueSidecarMemoryRequestDefault,
-			QueueSidecarMemoryLimit:             &QueueSidecarMemoryLimitDefault,
-			QueueSidecarEphemeralStorageRequest: &QueueSidecarEphemeralStorageRequestDefault,
-			QueueSidecarEphemeralStorageLimit:   &QueueSidecarEphemeralStorageLimitDefault,
-			QueueSidecarTokenAudiences:          sets.NewString(""),
-			ProgressDeadline:                    ProgressDeadlineDefault,
+			RegistriesSkippingTagResolving: sets.NewString("ko.local", "ko.dev"),
+			DigestResolutionTimeout:        digestResolutionTimeoutDefault,
+			QueueSidecarImage:              defaultSidecarImage,
+			QueueSidecarCPURequest:         &QueueSidecarCPURequestDefault,
+			QueueSidecarTokenAudiences:     sets.NewString(""),
+			ProgressDeadline:               ProgressDeadlineDefault,
 		},
 		data: map[string]string{
 			QueueSidecarImageKey:              defaultSidecarImage,

pkg/reconciler/revision/resources/deploy_test.go

@@ -75,7 +75,7 @@ var (
 
 	defaultQueueContainer = &corev1.Container{
 		Name:      QueueContainerName,
-		Resources: createQueueResources(&deploymentConfig, make(map[string]string), &corev1.Container{}),
+		Resources: createQueueResources(&deploymentConfig, make(map[string]string), &corev1.Container{}, false),
 		Ports:     append(queueNonServingPorts, queueHTTPPort, queueHTTPSPort),
 		ReadinessProbe: &corev1.Probe{
 			ProbeHandler: corev1.ProbeHandler{

pkg/reconciler/revision/resources/queue.go

@@ -89,32 +89,42 @@ var (
 	}
 )
 
-func createQueueResources(cfg *deployment.Config, annotations map[string]string, userContainer *corev1.Container) corev1.ResourceRequirements {
+func createQueueResources(cfg *deployment.Config, annotations map[string]string, userContainer *corev1.Container, useDefaults bool) corev1.ResourceRequirements {
 	resourceRequests := corev1.ResourceList{}
 	resourceLimits := corev1.ResourceList{}
 
 	for _, r := range []struct {
-		Name    corev1.ResourceName
-		Request *resource.Quantity
-		Limit   *resource.Quantity
+		Name           corev1.ResourceName
+		Request        *resource.Quantity
+		RequestDefault *resource.Quantity
+		Limit          *resource.Quantity
+		LimitDefault   *resource.Quantity
 	}{{
-		Name:    corev1.ResourceCPU,
-		Request: cfg.QueueSidecarCPURequest,
-		Limit:   cfg.QueueSidecarCPULimit,
+		Name:           corev1.ResourceCPU,
+		Request:        cfg.QueueSidecarCPURequest,
+		RequestDefault: &deployment.QueueSidecarCPURequestDefault,
+		Limit:          cfg.QueueSidecarCPULimit,
+		LimitDefault:   &deployment.QueueSidecarCPULimitDefault,
 	}, {
-		Name:    corev1.ResourceMemory,
-		Request: cfg.QueueSidecarMemoryRequest,
-		Limit:   cfg.QueueSidecarMemoryLimit,
+		Name:           corev1.ResourceMemory,
+		Request:        cfg.QueueSidecarMemoryRequest,
+		RequestDefault: &deployment.QueueSidecarMemoryRequestDefault,
+		Limit:          cfg.QueueSidecarMemoryLimit,
+		LimitDefault:   &deployment.QueueSidecarMemoryLimitDefault,
 	}, {
 		Name:    corev1.ResourceEphemeralStorage,
 		Request: cfg.QueueSidecarEphemeralStorageRequest,
 		Limit:   cfg.QueueSidecarEphemeralStorageLimit,
 	}} {
 		if r.Request != nil {
 			resourceRequests[r.Name] = *r.Request
+		} else if useDefaults && r.RequestDefault != nil {
+			resourceRequests[r.Name] = *r.RequestDefault
 		}
 		if r.Limit != nil {
 			resourceLimits[r.Name] = *r.Limit
+		} else if useDefaults && r.LimitDefault != nil {
+			resourceLimits[r.Name] = *r.LimitDefault
 		}
 	}
 
@@ -288,10 +298,11 @@ func makeQueueContainer(rev *v1.Revision, cfg *config.Config) (*corev1.Container
 		}
 	}
 
+	useQPResourceDefaults := cfg.Features.QueueProxyResourceDefaults == apicfg.Enabled
 	c := &corev1.Container{
 		Name:            QueueContainerName,
 		Image:           cfg.Deployment.QueueSidecarImage,
-		Resources:       createQueueResources(cfg.Deployment, rev.GetAnnotations(), container),
+		Resources:       createQueueResources(cfg.Deployment, rev.GetAnnotations(), container, useQPResourceDefaults),
 		Ports:           ports,
 		StartupProbe:    execProbe,
 		ReadinessProbe:  httpProbe,

pkg/reconciler/revision/resources/queue_test.go

@@ -312,28 +312,37 @@ func TestMakeQueueContainer(t *testing.T) {
 			})
 		}),
 	}, {
-		name: "default resource config",
+		name: "default resource config with feature qp defaults disabled",
 		rev: revision("bar", "foo",
 			withContainers(containers)),
 		dc: deployment.Config{
-			QueueSidecarCPURequest:              &deployment.QueueSidecarCPURequestDefault,
-			QueueSidecarCPULimit:                &deployment.QueueSidecarCPULimitDefault,
-			QueueSidecarMemoryRequest:           &deployment.QueueSidecarMemoryRequestDefault,
-			QueueSidecarMemoryLimit:             &deployment.QueueSidecarMemoryLimitDefault,
-			QueueSidecarEphemeralStorageRequest: &deployment.QueueSidecarEphemeralStorageRequestDefault,
-			QueueSidecarEphemeralStorageLimit:   &deployment.QueueSidecarEphemeralStorageLimitDefault,
+			QueueSidecarCPURequest: &deployment.QueueSidecarCPURequestDefault,
 		},
 		want: queueContainer(func(c *corev1.Container) {
 			c.Env = env(map[string]string{})
 			c.Resources.Requests = corev1.ResourceList{
-				corev1.ResourceCPU:              resource.MustParse("25m"),
-				corev1.ResourceMemory:           resource.MustParse("400Mi"),
-				corev1.ResourceEphemeralStorage: resource.MustParse("512Mi"),
+				corev1.ResourceCPU: deployment.QueueSidecarCPURequestDefault,
+			}
+		}),
+	}, {
+		name: "resource config with feature qp defaults enabled",
+		rev: revision("bar", "foo",
+			withContainers(containers)),
+		dc: deployment.Config{
+			QueueSidecarCPURequest: &deployment.QueueSidecarCPURequestDefault,
+		},
+		fc: apicfg.Features{
+			QueueProxyResourceDefaults: apicfg.Enabled,
+		},
+		want: queueContainer(func(c *corev1.Container) {
+			c.Env = env(map[string]string{})
+			c.Resources.Requests = corev1.ResourceList{
+				corev1.ResourceCPU:    deployment.QueueSidecarCPURequestDefault,
+				corev1.ResourceMemory: deployment.QueueSidecarMemoryRequestDefault,
 			}
 			c.Resources.Limits = corev1.ResourceList{
-				corev1.ResourceCPU:              resource.MustParse("1000m"),
-				corev1.ResourceMemory:           resource.MustParse("800Mi"),
-				corev1.ResourceEphemeralStorage: resource.MustParse("1024Mi"),
+				corev1.ResourceCPU:    deployment.QueueSidecarCPULimitDefault,
+				corev1.ResourceMemory: deployment.QueueSidecarMemoryLimitDefault,
 			}
 		}),
 	}, {