From d8847d3ab7147a283edec99c8b867eb252d574be Mon Sep 17 00:00:00 2001 From: Stavros Kontopoulos Date: Wed, 31 May 2023 13:38:32 +0300 Subject: [PATCH] enforce qp resource defaults --- config/core/configmaps/deployment.yaml | 12 +-- pkg/deployment/config.go | 33 ++++++++- pkg/deployment/config_test.go | 74 +++++++++++-------- .../revision/resources/queue_test.go | 17 ++++- 4 files changed, 93 insertions(+), 43 deletions(-) diff --git a/config/core/configmaps/deployment.yaml b/config/core/configmaps/deployment.yaml index 37472b983f7d..89c57fa526b9 100644 --- a/config/core/configmaps/deployment.yaml +++ b/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: "410041a0" + knative.dev/example-checksum: "dffb11e5" data: # This is the Go import path for the binary that is containerized # and substituted here. @@ -57,23 +57,23 @@ data: queue-sidecar-cpu-request: "25m" # Sets the queue proxy's CPU limit. - # If omitted, no value is specified and the system default is used. + # If omitted, a default value (currently "1000m"), is used. queue-sidecar-cpu-limit: "1000m" # Sets the queue proxy's memory request. - # If omitted, no value is specified and the system default is used. + # If omitted, a default value (currently "400Mi"), is used. queue-sidecar-memory-request: "400Mi" # Sets the queue proxy's memory limit. - # If omitted, no value is specified and the system default is used. + # If omitted, a default value (currently "800Mi"), is used. queue-sidecar-memory-limit: "800Mi" # Sets the queue proxy's ephemeral storage request. - # If omitted, no value is specified and the system default is used. + # If omitted, a default value (currently "512Mi"), is used. queue-sidecar-ephemeral-storage-request: "512Mi" # Sets the queue proxy's ephemeral storage limit. - # If omitted, no value is specified and the system default is used. + # If omitted, a default value (currently "1024Mi"), is used. queue-sidecar-ephemeral-storage-limit: "1024Mi" # Sets tokens associated with specific audiences for queue proxy - used by QPOptions diff --git a/pkg/deployment/config.go b/pkg/deployment/config.go index c2014195ad09..5f3d9bc4ec1a 100644 --- a/pkg/deployment/config.go +++ b/pkg/deployment/config.go @@ -75,14 +75,39 @@ var ( // queue sidecar. It is set at 25m for backwards-compatibility since this was // the historic default before the field was operator-settable. QueueSidecarCPURequestDefault = resource.MustParse("25m") + + // QueueSidecarCPULimitDefault is the default limit.cpu to set for the + // queue sidecar. + QueueSidecarCPULimitDefault = resource.MustParse("1000m") + + // QueueSidecarMemoryRequestDefault is the default request.memory to set for the + // queue sidecar. + QueueSidecarMemoryRequestDefault = resource.MustParse("400Mi") + + // QueueSidecarMemoryLimitDefault is the default limit.memory to set for the + // queue sidecar. + QueueSidecarMemoryLimitDefault = resource.MustParse("800Mi") + + // QueueSidecarEphemeralStorageRequestDefault is the default request.ephemeral-storage set for the + // queue sidecar. + QueueSidecarEphemeralStorageRequestDefault = resource.MustParse("512Mi") + + // QueueSidecarEphemeralStorageLimitDefault is the default limit.ephemeral-storage to set for the + // queue sidecar. + QueueSidecarEphemeralStorageLimitDefault = resource.MustParse("1024Mi") ) func defaultConfig() *Config { cfg := &Config{ - ProgressDeadline: ProgressDeadlineDefault, - DigestResolutionTimeout: digestResolutionTimeoutDefault, - RegistriesSkippingTagResolving: sets.NewString("kind.local", "ko.local", "dev.local"), - QueueSidecarCPURequest: &QueueSidecarCPURequestDefault, + 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, } // The following code is needed for ConfigMap testing. // defaultConfig must match the example in deployment.yaml which includes: `queue-sidecar-token-audiences: ""` diff --git a/pkg/deployment/config_test.go b/pkg/deployment/config_test.go index ed2108422ded..183f69644a2d 100644 --- a/pkg/deployment/config_test.go +++ b/pkg/deployment/config_test.go @@ -62,12 +62,6 @@ 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)) } @@ -83,12 +77,17 @@ func TestControllerConfiguration(t *testing.T) { }{{ name: "controller configuration with bad registries", wantConfig: &Config{ - RegistriesSkippingTagResolving: sets.NewString("ko.local", ""), - DigestResolutionTimeout: digestResolutionTimeoutDefault, - QueueSidecarImage: defaultSidecarImage, - QueueSidecarCPURequest: &QueueSidecarCPURequestDefault, - QueueSidecarTokenAudiences: sets.NewString("foo", "bar", "boo-srv"), - ProgressDeadline: ProgressDeadlineDefault, + 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, }, data: map[string]string{ QueueSidecarImageKey: defaultSidecarImage, @@ -98,12 +97,17 @@ 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, - QueueSidecarTokenAudiences: sets.NewString(""), - ProgressDeadline: 444 * time.Second, + 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, }, data: map[string]string{ QueueSidecarImageKey: defaultSidecarImage, @@ -112,12 +116,17 @@ 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, - QueueSidecarTokenAudiences: sets.NewString(""), - ProgressDeadline: ProgressDeadlineDefault, + 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, }, data: map[string]string{ QueueSidecarImageKey: defaultSidecarImage, @@ -126,12 +135,17 @@ 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, - QueueSidecarTokenAudiences: sets.NewString(""), - ProgressDeadline: ProgressDeadlineDefault, + 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, }, data: map[string]string{ QueueSidecarImageKey: defaultSidecarImage, diff --git a/pkg/reconciler/revision/resources/queue_test.go b/pkg/reconciler/revision/resources/queue_test.go index 39a3c2bd8ad9..346339719653 100644 --- a/pkg/reconciler/revision/resources/queue_test.go +++ b/pkg/reconciler/revision/resources/queue_test.go @@ -316,14 +316,25 @@ func TestMakeQueueContainer(t *testing.T) { rev: revision("bar", "foo", withContainers(containers)), dc: deployment.Config{ - QueueSidecarCPURequest: &deployment.QueueSidecarCPURequestDefault, + QueueSidecarCPURequest: &deployment.QueueSidecarCPURequestDefault, + QueueSidecarCPULimit: &deployment.QueueSidecarCPULimitDefault, + QueueSidecarMemoryRequest: &deployment.QueueSidecarMemoryRequestDefault, + QueueSidecarMemoryLimit: &deployment.QueueSidecarMemoryLimitDefault, + QueueSidecarEphemeralStorageRequest: &deployment.QueueSidecarEphemeralStorageRequestDefault, + QueueSidecarEphemeralStorageLimit: &deployment.QueueSidecarEphemeralStorageLimitDefault, }, want: queueContainer(func(c *corev1.Container) { c.Env = env(map[string]string{}) c.Resources.Requests = corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("25m"), + corev1.ResourceCPU: resource.MustParse("25m"), + corev1.ResourceMemory: resource.MustParse("400Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("512Mi"), + } + c.Resources.Limits = corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1000m"), + corev1.ResourceMemory: resource.MustParse("800Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse("1024Mi"), } - c.Resources.Limits = nil }), }, { name: "overridden resources",