Skip to content

Commit

Permalink
enforce qp resource defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
@skonto
skonto committed Jul 3, 2023
1 parent 32ec382 commit d8847d3
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 43 deletions.
12 changes: 6 additions & 6 deletions config/core/configmaps/deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
33 changes: 29 additions & 4 deletions pkg/deployment/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""`
Expand Down
74 changes: 44 additions & 30 deletions pkg/deployment/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
17 changes: 14 additions & 3 deletions pkg/reconciler/revision/resources/queue_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit d8847d3

Please sign in to comment.