Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable setting the resource request/limits via annotations for queue-proxy side-car container #4151

Merged
4 changes: 4 additions & 0 deletions pkg/apis/serving/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,8 @@ const (
// UpdaterAnnotation is the annotation key to describe the user that
// last updated the resource.
UpdaterAnnotation = GroupName + "/lastModifier"

// QueueSideCarResourcePercentageAnnotation is the percentage of user container resources to be used for queue-proxy
// It has to be in [0.1,100]
QueueSideCarResourcePercentageAnnotation = "queue.sidecar." + GroupName + "/resourcePercentage"
)
28 changes: 28 additions & 0 deletions pkg/apis/serving/v1alpha1/revision_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1alpha1
import (
"context"
"fmt"
"strconv"
"strings"

"github.com/knative/pkg/apis"
Expand Down Expand Up @@ -81,6 +82,8 @@ func (rt *RevisionTemplateSpec) Validate(ctx context.Context) *apis.FieldError {
}
}

errs = errs.Also(validateAnnotations(rt.Annotations))

return errs
}

Expand Down Expand Up @@ -152,6 +155,31 @@ func (rs *RevisionSpec) Validate(ctx context.Context) *apis.FieldError {
return errs
}

func validateAnnotations(annotations map[string]string) *apis.FieldError {
return validatePercentageAnnotationKey(annotations, serving.QueueSideCarResourcePercentageAnnotation)
raushan2016 marked this conversation as resolved.
Show resolved Hide resolved
}

func validatePercentageAnnotationKey(annotations map[string]string, resourcePercentageAnnotationKey string) *apis.FieldError {
if len(annotations) == 0 {
return nil
}

v, ok := annotations[resourcePercentageAnnotationKey]
if !ok {
return nil
}
value, err := strconv.ParseFloat(v, 32)
raushan2016 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return apis.ErrInvalidValue(v, apis.CurrentField).ViaKey(resourcePercentageAnnotationKey)
}

if value <= float64(0.1) || value > float64(100) {
return apis.ErrOutOfBoundsValue(value, 0.1, 100.0, resourcePercentageAnnotationKey)
}
raushan2016 marked this conversation as resolved.
Show resolved Hide resolved

return nil
}

func validateTimeoutSeconds(timeoutSeconds int64) *apis.FieldError {
if timeoutSeconds != 0 {
if timeoutSeconds > int64(networking.DefaultTimeout.Seconds()) || timeoutSeconds < 0 {
Expand Down
37 changes: 37 additions & 0 deletions pkg/apis/serving/v1alpha1/revision_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/knative/pkg/ptr"
"github.com/knative/serving/pkg/apis/autoscaling"
net "github.com/knative/serving/pkg/apis/networking"
"github.com/knative/serving/pkg/apis/serving"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -298,6 +299,42 @@ func TestRevisionTemplateSpecValidation(t *testing.T) {
},
},
want: nil,
}, {
name: "Queue sidecar resource percentage annotation more than 100",
rts: &RevisionTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
serving.QueueSideCarResourcePercentageAnnotation: "200",
},
},
Spec: RevisionSpec{
DeprecatedContainer: &corev1.Container{
Image: "helloworld",
},
},
},
want: &apis.FieldError{
Message: "expected 0.1 <= 200 <= 100",
Paths: []string{serving.QueueSideCarResourcePercentageAnnotation},
},
}, {
name: "Invalid queue sidecar resource percentage annotation",
rts: &RevisionTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
serving.QueueSideCarResourcePercentageAnnotation: "50mx",
},
},
Spec: RevisionSpec{
DeprecatedContainer: &corev1.Container{
Image: "helloworld",
},
},
},
want: &apis.FieldError{
Message: "invalid value: 50mx",
Paths: []string{fmt.Sprintf("[%s]", serving.QueueSideCarResourcePercentageAnnotation)},
},
}}

for _, test := range tests {
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/revision/resources/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var (

defaultQueueContainer = &corev1.Container{
Name: QueueContainerName,
Resources: queueResources,
Resources: createQueueResources(make(map[string]string), &corev1.Container{}),
Ports: append(queueNonServingPorts, queueHTTPPort),
ReadinessProbe: queueReadinessProbe,
Env: []corev1.EnvVar{{
Expand Down
87 changes: 80 additions & 7 deletions pkg/reconciler/revision/resources/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ limitations under the License.
package resources

import (
"math"
"strconv"

"k8s.io/apimachinery/pkg/api/resource"

"k8s.io/apimachinery/pkg/util/intstr"

"github.com/knative/pkg/logging"
Expand All @@ -38,12 +41,6 @@ import (
const requestQueueHTTPPortName = "queue-port"

var (
queueResources = corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceName("cpu"): queueContainerCPU,
},
}

queueHTTPPort = corev1.ContainerPort{
Name: requestQueueHTTPPortName,
ContainerPort: int32(networking.BackendHTTPPort),
Expand Down Expand Up @@ -83,6 +80,82 @@ var (
}
)

func createQueueResources(annotations map[string]string, userContainer *corev1.Container) corev1.ResourceRequirements {
resources := corev1.ResourceRequirements{}
resourceRequests := corev1.ResourceList{corev1.ResourceCPU: queueContainerCPU}
resourceLimits := corev1.ResourceList{}
ok := false
var requestCPU, limitCPU, requestMemory, limitMemory resource.Quantity
var resourcePercentage float32

if ok, resourcePercentage = createResourcePercentageFromAnnotations(annotations, serving.QueueSideCarResourcePercentageAnnotation); ok {

if ok, requestCPU = computeResourceRequirements(userContainer.Resources.Requests.Cpu(), resourcePercentage, queueContainerRequestCPU); ok {
resourceRequests[corev1.ResourceCPU] = requestCPU
}

if ok, limitCPU = computeResourceRequirements(userContainer.Resources.Limits.Cpu(), resourcePercentage, queueContainerLimitCPU); ok {
resourceLimits[corev1.ResourceCPU] = limitCPU
}

if ok, requestMemory = computeResourceRequirements(userContainer.Resources.Requests.Memory(), resourcePercentage, queueContainerRequestMemory); ok {
resourceRequests[corev1.ResourceMemory] = requestMemory
}

if ok, limitMemory = computeResourceRequirements(userContainer.Resources.Limits.Memory(), resourcePercentage, queueContainerLimitMemory); ok {
resourceLimits[corev1.ResourceMemory] = limitMemory
}

}

resources.Requests = resourceRequests
mattmoor marked this conversation as resolved.
Show resolved Hide resolved

if len(resourceLimits) != 0 {
resources.Limits = resourceLimits
}

return resources
}

func computeResourceRequirements(resourceQuantity *resource.Quantity, percentage float32, boundary resourceBoundary) (bool, resource.Quantity) {
if resourceQuantity.IsZero() {
return false, resource.Quantity{}
}

// Incase the resourceQuantity MilliValue overflow in we use MaxInt64
// https://github.com/kubernetes/apimachinery/blob/master/pkg/api/resource/quantity.go
scaledValue := resourceQuantity.Value()
scaledMilliValue := int64(math.MaxInt64 - 1)
if scaledValue < (math.MaxInt64 / 1000) {
scaledMilliValue = resourceQuantity.MilliValue()
}

// float64(math.MaxInt64) > math.MaxInt64, to avoid overflow
percentageValue := float64(scaledMilliValue) * float64(percentage)
var newValue int64
if percentageValue >= math.MaxInt64 {
newValue = math.MaxInt64
} else {
newValue = int64(percentageValue)
}

newquantity := *resource.NewMilliQuantity(newValue, resource.BinarySI)
newquantity = boundary.applyBoundary(newquantity)
return true, newquantity
}

func createResourcePercentageFromAnnotations(m map[string]string, k string) (bool, float32) {
v, ok := m[k]
if !ok {
return false, 0
}
value, err := strconv.ParseFloat(v, 32)
if err != nil {
return false, 0
}
return true, float32(value / 100)
raushan2016 marked this conversation as resolved.
Show resolved Hide resolved
}

// makeQueueContainer creates the container spec for the queue sidecar.
func makeQueueContainer(rev *v1alpha1.Revision, loggingConfig *logging.Config, observabilityConfig *metrics.ObservabilityConfig,
autoscalerConfig *autoscaler.Config, deploymentConfig *deployment.Config) *corev1.Container {
Expand Down Expand Up @@ -116,7 +189,7 @@ func makeQueueContainer(rev *v1alpha1.Revision, loggingConfig *logging.Config, o
return &corev1.Container{
Name: QueueContainerName,
Image: deploymentConfig.QueueSidecarImage,
Resources: queueResources,
Resources: createQueueResources(rev.GetAnnotations(), rev.Spec.GetContainer()),
Ports: ports,
ReadinessProbe: queueReadinessProbe,
Env: []corev1.EnvVar{{
Expand Down
Loading