Allow to set QP resources per service

pkg/apis/serving/register.go

@@ -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.
@@ -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,
 	}

pkg/apis/serving/v1/revision_validation.go

@@ -188,14 +188,15 @@ func validateQueueSidecarAnnotation(m map[string]string) *apis.FieldError {
 	if !ok {
 		return nil
 	}
+	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 {
-		return apis.ErrInvalidValue(v, apis.CurrentField).ViaKey(k)
+		return errs.Also(apis.ErrInvalidValue(v, apis.CurrentField).ViaKey(k))
 	}
 	if value < 0.1 || value > 100 {
-		return apis.ErrOutOfBoundsValue(value, 0.1, 100.0, apis.CurrentField).ViaKey(k)
+		return errs.Also(apis.ErrOutOfBoundsValue(value, 0.1, 100.0, apis.CurrentField).ViaKey(k))
 	}
-	return nil
+	return errs
 }
 
 // ValidateProgressDeadlineAnnotation validates the revision progress deadline annotation.

pkg/apis/serving/v1/revision_validation_test.go

@@ -1093,6 +1093,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
@@ -1102,28 +1105,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{},
@@ -1137,11 +1140,13 @@ 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 {

pkg/reconciler/revision/resources/queue.go

@@ -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,
 	}
@@ -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)

pkg/reconciler/revision/resources/queue_test.go

@@ -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) {