Skip to content

Commit

Permalink
Allow to set QP resources per service (#14038)
Browse files Browse the repository at this point in the history 
* allow to set qp resources per service

* add ephemeral storage

* deprecate resource percentage

* remove the old casing

* add proper validation

* fix loop
  • Loading branch information
@skonto
skonto committed Jun 28, 2023
1 parent 9310e4d commit 63fa389
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 20 deletions.
37 changes: 37 additions & 0 deletions pkg/apis/serving/register.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,27 @@ const (

// QueueSidecarResourcePercentageAnnotationKey is the percentage of user container resources to be used for queue-proxy
// It has to be in [0.1,100]
// Deprecated: Please consider setting resources explicitly for the QP per service, see `QueueSidecarCPUResourceRequestAnnotationKey` for example.
QueueSidecarResourcePercentageAnnotationKey = "queue.sidecar." + GroupName + "/resource-percentage"

// QueueSidecarCPUResourceRequestAnnotationKey is the explicit value of the cpu request for queue-proxy's request resources
QueueSidecarCPUResourceRequestAnnotationKey = "queue.sidecar." + GroupName + "/cpu-resource-request"

// QueueSidecarCPUResourceLimitAnnotationKey is the explicit value of the cpu limit for queue-proxy's limit resources
QueueSidecarCPUResourceLimitAnnotationKey = "queue.sidecar." + GroupName + "/cpu-resource-limit"

// QueueSidecarMemoryResourceRequestAnnotationKey is the explicit value of the memory request for queue-proxy's request resources
QueueSidecarMemoryResourceRequestAnnotationKey = "queue.sidecar." + GroupName + "/memory-resource-request"

// QueueSidecarMemoryResourceLimitAnnotationKey is the explicit value of the memory limit for queue-proxy's limit resources
QueueSidecarMemoryResourceLimitAnnotationKey = "queue.sidecar." + GroupName + "/memory-resource-limit"

// QueueSidecarEphemeralStorageResourceRequestAnnotationKey is the explicit value of the ephemeral storage request for queue-proxy's request resources
QueueSidecarEphemeralStorageResourceRequestAnnotationKey = "queue.sidecar." + GroupName + "/ephemeral-storage-resource-request"

// QueueSidecarEphemeralStorageResourceLimitAnnotationKey is the explicit value of the ephemeral storage limit for queue-proxy's limit resources
QueueSidecarEphemeralStorageResourceLimitAnnotationKey = "queue.sidecar." + GroupName + "/ephemeral-storage-resource-limit"

// VisibilityClusterLocal is the label value for VisibilityLabelKey
// that will result to the Route/KService getting a cluster local
// domain suffix.
Expand Down Expand Up @@ -162,6 +181,24 @@ var (
QueueSidecarResourcePercentageAnnotationKey,
"queue.sidecar." + GroupName + "/resourcePercentage",
}
QueueSidecarCPUResourceRequestAnnotation = kmap.KeyPriority{
QueueSidecarCPUResourceRequestAnnotationKey,
}
QueueSidecarCPUResourceLimitAnnotation = kmap.KeyPriority{
QueueSidecarCPUResourceLimitAnnotationKey,
}
QueueSidecarMemoryResourceRequestAnnotation = kmap.KeyPriority{
QueueSidecarMemoryResourceRequestAnnotationKey,
}
QueueSidecarMemoryResourceLimitAnnotation = kmap.KeyPriority{
QueueSidecarMemoryResourceLimitAnnotationKey,
}
QueueSidecarEphemeralStorageResourceRequestAnnotation = kmap.KeyPriority{
QueueSidecarEphemeralStorageResourceRequestAnnotationKey,
}
QueueSidecarEphemeralStorageResourceLimitAnnotation = kmap.KeyPriority{
QueueSidecarEphemeralStorageResourceLimitAnnotationKey,
}
ProgressDeadlineAnnotation = kmap.KeyPriority{
ProgressDeadlineAnnotationKey,
}
Expand Down
47 changes: 34 additions & 13 deletions pkg/apis/serving/v1/revision_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
"strings"
"time"

"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/api/validation"
"knative.dev/pkg/apis"
"knative.dev/pkg/kmap"
"knative.dev/pkg/kmp"
"knative.dev/serving/pkg/apis/autoscaling"
"knative.dev/serving/pkg/apis/config"
Expand Down Expand Up @@ -68,7 +70,7 @@ func (rts *RevisionTemplateSpec) Validate(ctx context.Context) *apis.FieldError
// If the RevisionTemplateSpec has a name specified, then check that
// it follows the requirements on the name.
errs = errs.Also(validateRevisionName(ctx, rts.Name, rts.GenerateName))
errs = errs.Also(validateQueueSidecarAnnotation(rts.Annotations).ViaField("metadata.annotations"))
errs = errs.Also(validateQueueSidecarResourceAnnotations(rts.Annotations).ViaField("metadata.annotations"))
errs = errs.Also(validateProgressDeadlineAnnotation(rts.Annotations).ViaField("metadata.annotations"))
return errs
}
Expand Down Expand Up @@ -179,23 +181,42 @@ func validateTimeoutSeconds(ctx context.Context, timeoutSeconds int64) *apis.Fie
return nil
}

// validateQueueSidecarAnnotation validates QueueSideCarResourcePercentageAnnotation
func validateQueueSidecarAnnotation(m map[string]string) *apis.FieldError {
// validateQueueSidecarResourceAnnotations validates QueueSideCarResourcePercentageAnnotation and other QP resource related annotations.
func validateQueueSidecarResourceAnnotations(m map[string]string) *apis.FieldError {
if len(m) == 0 {
return nil
}
k, v, ok := serving.QueueSidecarResourcePercentageAnnotation.Get(m)
if !ok {
return nil
}
value, err := strconv.ParseFloat(v, 64)
if err != nil {
return apis.ErrInvalidValue(v, apis.CurrentField).ViaKey(k)

var errs *apis.FieldError
if k, v, ok := serving.QueueSidecarResourcePercentageAnnotation.Get(m); ok {
errs = apis.ErrGeneric("Queue proxy resource percentage annotation is deprecated. Please use the available annotations to explicitly set resource values per service").ViaKey(k).At(apis.WarningLevel)
value, err := strconv.ParseFloat(v, 64)
if err != nil {
errs = errs.Also(apis.ErrInvalidValue(v, apis.CurrentField).ViaKey(k))
} else {
if value < 0.1 || value > 100 {
errs = errs.Also(apis.ErrOutOfBoundsValue(value, 0.1, 100.0, apis.CurrentField).ViaKey(k))
}
}
}
if value < 0.1 || value > 100 {
return apis.ErrOutOfBoundsValue(value, 0.1, 100.0, apis.CurrentField).ViaKey(k)
annoKeys := []kmap.KeyPriority{
serving.QueueSidecarCPUResourceRequestAnnotation,
serving.QueueSidecarCPUResourceLimitAnnotation,
serving.QueueSidecarMemoryResourceRequestAnnotation,
serving.QueueSidecarMemoryResourceLimitAnnotation,
serving.QueueSidecarEphemeralStorageResourceRequestAnnotation,
serving.QueueSidecarEphemeralStorageResourceLimitAnnotation,
}
for _, resAnno := range annoKeys {
k, v, ok := resAnno.Get(m)
if !ok {
continue
}
if _, err := resource.ParseQuantity(v); err != nil {
errs = errs.Also(apis.ErrInvalidValue(v, apis.CurrentField).ViaKey(k))
}
}
return nil
return errs
}

// ValidateProgressDeadlineAnnotation validates the revision progress deadline annotation.
Expand Down
47 changes: 40 additions & 7 deletions pkg/apis/serving/v1/revision_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,34 @@ func TestRevisionTemplateSpecValidation(t *testing.T) {
Message: "invalid value: 50mx",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSidecarResourcePercentageAnnotationKey)},
}).ViaField("metadata.annotations"),
}, {
name: "Invalid queue sidecar resource annotations",
rts: &RevisionTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
serving.QueueSidecarCPUResourceLimitAnnotationKey: "100mx",
serving.QueueSidecarCPUResourceRequestAnnotationKey: "50mx",
serving.QueueSidecarMemoryResourceLimitAnnotationKey: "1Gi",
serving.QueueSidecarMemoryResourceRequestAnnotationKey: "100Mi",
serving.QueueSidecarEphemeralStorageResourceLimitAnnotationKey: "5Gi",
serving.QueueSidecarEphemeralStorageResourceRequestAnnotationKey: "2Gi",
},
},
Spec: RevisionSpec{
PodSpec: corev1.PodSpec{
Containers: []corev1.Container{{
Image: "helloworld",
}},
},
},
},
want: ((&apis.FieldError{
Message: "invalid value: 100mx",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSidecarCPUResourceLimitAnnotationKey)},
}).Also(&apis.FieldError{
Message: "invalid value: 50mx",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSidecarCPUResourceRequestAnnotationKey)},
})).ViaField("metadata.annotations"),
}, {
name: "Invalid initial scale when cluster doesn't allow zero",
ctx: autoscalerConfigCtx(false, 1),
Expand Down Expand Up @@ -1093,6 +1121,9 @@ func autoscalerConfigCtx(allowInitialScaleZero bool, initialScale int) context.C
}

func TestValidateQueueSidecarAnnotation(t *testing.T) {
resourcePercentageDeprecationWarning := apis.ErrGeneric("Queue proxy resource percentage annotation is deprecated. Please use the available annotations to explicitly set resource values per service").
ViaKey(serving.QueueSidecarResourcePercentageAnnotationKey).At(apis.WarningLevel)

cases := []struct {
name string
annotation map[string]string
Expand All @@ -1102,28 +1133,28 @@ func TestValidateQueueSidecarAnnotation(t *testing.T) {
annotation: map[string]string{
serving.QueueSidecarResourcePercentageAnnotationKey: "0.01982",
},
expectErr: &apis.FieldError{
expectErr: (&apis.FieldError{
Message: "expected 0.1 <= 0.01982 <= 100",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSidecarResourcePercentageAnnotationKey)},
},
}).Also(resourcePercentageDeprecationWarning),
}, {
name: "too big for Queue sidecar resource percentage annotation",
annotation: map[string]string{
serving.QueueSidecarResourcePercentageAnnotationKey: "100.0001",
},
expectErr: &apis.FieldError{
expectErr: (&apis.FieldError{
Message: "expected 0.1 <= 100.0001 <= 100",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSidecarResourcePercentageAnnotationKey)},
},
}).Also(resourcePercentageDeprecationWarning),
}, {
name: "Invalid queue sidecar resource percentage annotation",
annotation: map[string]string{
serving.QueueSidecarResourcePercentageAnnotationKey: "",
},
expectErr: &apis.FieldError{
expectErr: (&apis.FieldError{
Message: "invalid value: ",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSidecarResourcePercentageAnnotationKey)},
},
}).Also(resourcePercentageDeprecationWarning),
}, {
name: "empty annotation",
annotation: map[string]string{},
Expand All @@ -1137,16 +1168,18 @@ func TestValidateQueueSidecarAnnotation(t *testing.T) {
annotation: map[string]string{
serving.QueueSidecarResourcePercentageAnnotationKey: "0.1",
},
expectErr: resourcePercentageDeprecationWarning,
}, {
name: "valid value for Queue sidecar resource percentage annotation",
annotation: map[string]string{
serving.QueueSidecarResourcePercentageAnnotationKey: "100",
},
expectErr: resourcePercentageDeprecationWarning,
}}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
err := validateQueueSidecarAnnotation(c.annotation)
err := validateQueueSidecarResourceAnnotations(c.annotation)
if got, want := err.Error(), c.expectErr.Error(); got != want {
t.Errorf("Got: %q want: %q", got, want)
}
Expand Down
30 changes: 30 additions & 0 deletions pkg/reconciler/revision/resources/queue.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,30 @@ func createQueueResources(cfg *deployment.Config, annotations map[string]string,
}
}

if requestCPU, ok := resourceFromAnnotation(annotations, serving.QueueSidecarCPUResourceRequestAnnotation); ok {
resourceRequests[corev1.ResourceCPU] = requestCPU
}

if limitCPU, ok := resourceFromAnnotation(annotations, serving.QueueSidecarCPUResourceLimitAnnotation); ok {
resourceLimits[corev1.ResourceCPU] = limitCPU
}

if requestMemory, ok := resourceFromAnnotation(annotations, serving.QueueSidecarMemoryResourceRequestAnnotation); ok {
resourceRequests[corev1.ResourceMemory] = requestMemory
}

if limitMemory, ok := resourceFromAnnotation(annotations, serving.QueueSidecarMemoryResourceLimitAnnotation); ok {
resourceLimits[corev1.ResourceMemory] = limitMemory
}

if requestEphmeralStorage, ok := resourceFromAnnotation(annotations, serving.QueueSidecarEphemeralStorageResourceRequestAnnotation); ok {
resourceRequests[corev1.ResourceEphemeralStorage] = requestEphmeralStorage
}

if limitEphemeralStorage, ok := resourceFromAnnotation(annotations, serving.QueueSidecarEphemeralStorageResourceLimitAnnotation); ok {
resourceLimits[corev1.ResourceEphemeralStorage] = limitEphemeralStorage
}

resources := corev1.ResourceRequirements{
Requests: resourceRequests,
}
Expand Down Expand Up @@ -172,6 +196,12 @@ func computeResourceRequirements(resourceQuantity *resource.Quantity, fraction f
return true, newquantity
}

func resourceFromAnnotation(m map[string]string, key kmap.KeyPriority) (resource.Quantity, bool) {
_, v, _ := key.Get(m)
q, err := resource.ParseQuantity(v)
return q, err == nil
}

func fractionFromPercentage(m map[string]string, key kmap.KeyPriority) (float64, bool) {
_, v, _ := key.Get(m)
value, err := strconv.ParseFloat(v, 64)
Expand Down
116 changes: 116 additions & 0 deletions pkg/reconciler/revision/resources/queue_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,122 @@ func TestMakeQueueContainerWithPercentageAnnotation(t *testing.T) {
}
}

func TestMakeQueueContainerWithResourceAnnotations(t *testing.T) {
tests := []struct {
name string
rev *v1.Revision
want corev1.Container
dc deployment.Config
}{{
name: "resources defined via annotations",
rev: revision("bar", "foo",
func(revision *v1.Revision) {
revision.Annotations = map[string]string{
serving.QueueSidecarCPUResourceRequestAnnotationKey: "1",
serving.QueueSidecarCPUResourceLimitAnnotationKey: "2",
serving.QueueSidecarMemoryResourceRequestAnnotationKey: "1Gi",
serving.QueueSidecarMemoryResourceLimitAnnotationKey: "2Gi",
serving.QueueSidecarEphemeralStorageResourceRequestAnnotationKey: "500Mi",
serving.QueueSidecarEphemeralStorageResourceLimitAnnotationKey: "600Mi",
}
revision.Spec.PodSpec.Containers = []corev1.Container{{
Name: servingContainerName,
ReadinessProbe: testProbe,
}}
}),
want: queueContainer(func(c *corev1.Container) {
c.Env = env(map[string]string{})
c.Resources.Requests = corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("1Gi"),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceEphemeralStorage: resource.MustParse("500Mi"),
}
c.Resources.Limits = corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("2Gi"),
corev1.ResourceCPU: resource.MustParse("2"),
corev1.ResourceEphemeralStorage: resource.MustParse("600Mi"),
}
}),
}, {
name: "resources defined via annotations with bad values ignored",
rev: revision("bar", "foo",
func(revision *v1.Revision) {
revision.Annotations = map[string]string{
serving.QueueSidecarCPUResourceRequestAnnotationKey: "zzz",
serving.QueueSidecarCPUResourceLimitAnnotationKey: "2",
serving.QueueSidecarMemoryResourceRequestAnnotationKey: "Gdx",
serving.QueueSidecarMemoryResourceLimitAnnotationKey: "2Gi",
}
revision.Spec.PodSpec.Containers = []corev1.Container{{
Name: servingContainerName,
ReadinessProbe: testProbe,
}}
}),
want: queueContainer(func(c *corev1.Container) {
c.Env = env(map[string]string{})
c.Resources.Limits = corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("2Gi"),
corev1.ResourceCPU: resource.MustParse("2"),
}
}),
}, {
name: "resources defined via annotations mixed with percentage annotation",
rev: revision("bar", "foo",
func(revision *v1.Revision) {
revision.Annotations = map[string]string{
serving.QueueSidecarCPUResourceLimitAnnotationKey: "1",
serving.QueueSidecarMemoryResourceLimitAnnotationKey: "4Gi",
serving.QueueSidecarResourcePercentageAnnotationKey: "50",
}
revision.Spec.PodSpec.Containers = []corev1.Container{{
Name: servingContainerName,
ReadinessProbe: testProbe,
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("1Gi"),
corev1.ResourceCPU: resource.MustParse("1"),
},
Limits: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("2Gi"),
corev1.ResourceCPU: resource.MustParse("2"),
},
}},
}
}),
want: queueContainer(func(c *corev1.Container) {
c.Env = env(map[string]string{})
c.Resources.Requests = corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("200Mi"), // hits the boundary for max value
corev1.ResourceCPU: resource.MustParse("100m"), // hits the boundary for max value
}
c.Resources.Limits = corev1.ResourceList{ // enforce the desired limits
corev1.ResourceMemory: resource.MustParse("4Gi"),
corev1.ResourceCPU: resource.MustParse("1"),
}
}),
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cfg := revConfig()
cfg.Deployment = &test.dc
got, err := makeQueueContainer(test.rev, cfg)
if err != nil {
t.Fatal("makeQueueContainer returned error:", err)
}
test.want.Env = append(test.want.Env, corev1.EnvVar{
Name: "SERVING_READINESS_PROBE",
Value: probeJSON(test.rev.Spec.GetContainer()),
})
sortEnv(got.Env)
sortEnv(test.want.Env)
if got, want := *got, test.want; !cmp.Equal(got, want, quantityComparer) {
t.Errorf("makeQueueContainer (-want, +got) =\n%s", cmp.Diff(want, got, quantityComparer))
}
})
}
}

func TestProbeGenerationHTTPDefaults(t *testing.T) {
rev := revision("bar", "foo",
func(revision *v1.Revision) {
Expand Down

0 comments on commit 63fa389

Please sign in to comment.