diff --git a/README.md b/README.md index 0b44e69..a16a924 100644 --- a/README.md +++ b/README.md @@ -14,14 +14,16 @@ Helps you resize pods created by a DaemonSet depending on the amount of allocata - `node-specific-sizing.manomano.tech/request-memory-fraction: 0.1` - `node-specific-sizing.manomano.tech/limit-memory-fraction: 0.1` -3. Optionally set absolute minimums, maximums and exclusions - NB: Only pods with the `node-specific-sizing.manomano.tech/enabled: "true"` label will see their resource modified. - - `node-specific-sizing.manomano.tech/minimum-cpu-request: 0.5` - - `node-specific-sizing.manomano.tech/minimum-cpu-limit: 0.5` - - `node-specific-sizing.manomano.tech/maximum-memory-request: 0.5` - - `node-specific-sizing.manomano.tech/maximum-memory-limit: 0.5` - -4. Optionally exclude some containers from dynamic-sizing +3. *Optionally*, set up appropriate minimums and maximums. + - `node-specific-sizing.manomano.tech/minimum-cpu: 50m` + - `node-specific-sizing.manomano.tech/maximum-cpu: 4` + - `node-specific-sizing.manomano.tech/minimum-memory: 50M` + - `node-specific-sizing.manomano.tech/maximum-memory: 4G` + - NOTE: Minimums and maximums are applied to both resource and limits. + We don't see the need to add different minimums for requests in limits in practice. You may challenge that choice by opening an issue. + - NOTE: Minimums and maximums are to be understood per-pod and not per-container. See resource-sizing algorithm for details. + +4. *Optionally*, exclude some containers from dynamic sizing. - `node-specific-sizing.manomano.tech/exclude-containers: istio-init,istio-proxy` - NOT IMPLEMENTED @@ -43,7 +45,7 @@ follows: For any given container, `relative_tunable = container_tunable / (sum(container_tunables) - sum(excluded_container_tunables))` - Derive a `pod_tunable_budget = allocatable_tunable_on_node * configured_pod_proportion - sum(excluded_container_tunables)`. This represents the resources that will be given to the pod. - Clamp `pod_tunable_budget` if minimums and/or maximums are set for that tunable. -- Finally, `new_absolute_tunable = pod_tunable_budget * relative_tunable` spreads the budget around. +- Finally, `new_absolute_tunable = pod_tunable_budget * relative_tunable` spreads the budget between containers. Exclusions and clamping notwithstanding, the requests/limits proportions between the different containers do not vary with node specific sizing. diff --git a/bin/playground.sh b/bin/playground.sh index be9097b..e8166c5 100755 --- a/bin/playground.sh +++ b/bin/playground.sh @@ -100,6 +100,8 @@ if [[ "$OPT_IN_MODE" == 0 || "$WITH_WORKLOAD" == 1 ]]; then annotations: node-specific-sizing.manomano.tech/request-memory-fraction: "0.05" node-specific-sizing.manomano.tech/limit-memory-fraction: "0.09" + node-specific-sizing.manomano.tech/minimum-memory: "40M" + node-specific-sizing.manomano.tech/maximum-memory: "742M" spec: terminationGracePeriodSeconds: 0 containers: diff --git a/bin/print_resources.sh b/bin/print_resources.sh index 863f124..0b6c9ac 100755 --- a/bin/print_resources.sh +++ b/bin/print_resources.sh @@ -2,5 +2,24 @@ set -eo pipefail -kubectl -n default get -o json pods | \ - jq -r '.items[] | .status.hostIP as $hip | .spec.containers[] | .name as $n | .resources | "\($hip) - \($n) - REQ \(.requests.memory) - LIM \(.limits.memory)"' +_pod_json=$(kubectl -n default get -o json pods) +_len=$(echo "$_pod_json" | jq -r '.items | length - 1') + +# jq -r '.spec.nodeName as $hip | .spec.containers[] | .name as $n | .resources | "\($hip) - \($n) - REQ \(.requests.memory) - LIM \(.limits.memory)"' + +for i in `seq 0 "$_len"`; do + _node=$(echo "$_pod_json" | jq -r ".items[$i] | .spec.nodeName") + _containers_count=$(echo "$_pod_json" | jq ".items[$i] | .spec.containers | length - 1") + + for j in `seq 0 "$_containers_count"`; do + _name=$(echo "$_pod_json" | jq -r ".items[$i] | .spec.containers[$j] | .name") + _cpu_req=$(echo "$_pod_json" | jq -r ".items[$i] | .spec.containers[$j] | .resources.requests.cpu") + _mem_req=$(echo "$_pod_json" | jq -r ".items[$i] | .spec.containers[$j] | .resources.requests.memory") + _cpu_lim=$(echo "$_pod_json" | jq -r ".items[$i] | .spec.containers[$j] | .resources.limits.cpu") + _mem_lim=$(echo "$_pod_json" | jq -r ".items[$i] | .spec.containers[$j] | .resources.limits.memory") + + # shellcheck disable=SC2182 + printf "%-20s cpu: req=%-5s lim=%-5s\n" "$_node" "$_cpu_req" "$_cpu_lim" + printf "%-20s mem: req=%-5s lim=%-5s\n" "--- $_name" "$_mem_req" "$_mem_lim" + done +done diff --git a/cmd/pod_patcher.go b/cmd/pod_patcher.go index f2434eb..3b2166e 100644 --- a/cmd/pod_patcher.go +++ b/cmd/pod_patcher.go @@ -34,14 +34,15 @@ func computeProportionalResourceRequirements(pod *corev1.Pod) map[string]*rps.Re return containerRequirements } -func computePodResourceBudget(fractions *rps.ResourceProperties, node *corev1.Node) *rps.ResourceProperties { +func computePodResourceBudget(userSettings *rps.ResourceProperties, node *corev1.Node) *rps.ResourceProperties { podResourceBudget := rps.New() - for prop := range fractions.All() { + for prop := range userSettings.All() { if nodeCapacity, ok := node.Status.Capacity[prop.ResourceName()]; ok { qty := nodeCapacity.AsApproximateFloat64() - podResourceBudget.BindPropertyFloat(prop.Property(), prop.ResourceName(), qty*prop.Value()) + podResourceBudget.BindPropertyFloat(rps.ResourceQuantity, prop.Property(), prop.ResourceName(), qty*prop.Value()) } } + podResourceBudget.ClampRequestsAndLimits(userSettings) return podResourceBudget } @@ -61,7 +62,10 @@ func multiplyQuantity(quantity resource.Quantity, multiplier float64) *resource. } } -func computePodContainerResourceBudget(containersProportionalResourceRequirements map[string]*rps.ResourceProperties, podResourceBudget *rps.ResourceProperties) map[string]*rps.ResourceProperties { +func computePodContainerResourceBudget( + containersProportionalResourceRequirements map[string]*rps.ResourceProperties, + podResourceBudget *rps.ResourceProperties, +) map[string]*rps.ResourceProperties { result := make(map[string]*rps.ResourceProperties) for containerName, proportionalResourceRequirements := range containersProportionalResourceRequirements { result[containerName] = proportionalResourceRequirements.Mul(podResourceBudget) @@ -120,7 +124,7 @@ func createPatch(ctx context.Context, pod *corev1.Pod) ([]byte, error) { zap.L().Debug("Starting patch process") - err, fractions := rps.NewFromAnnotations(pod.Annotations) + err, userSettings := rps.NewFromAnnotations(pod.Annotations) if err != nil { return nil, fmt.Errorf("problem parsing annotations: %w", err) } @@ -151,7 +155,7 @@ func createPatch(ctx context.Context, pod *corev1.Pod) ([]byte, error) { // We need pod budget = node resources * nssConfig.nodeResourcesFractions // When we have pod budget we want pod container budget = podBudget * containersProportionalRequirements // Then set values - podResourceBudget := computePodResourceBudget(fractions, &node) + podResourceBudget := computePodResourceBudget(userSettings, &node) zap.L().Debug("podResourceBudget", zap.Any("pRB", *podResourceBudget)) diff --git a/go.mod b/go.mod index 1885dd1..e07889d 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,11 @@ module github.com/ManoManoTech/kubernetes-node-specific-sizing -go 1.23 +go 1.23.0 require ( + github.com/deckarep/golang-set/v2 v2.6.0 + github.com/onsi/ginkgo/v2 v2.19.0 + github.com/onsi/gomega v1.33.1 go.uber.org/zap v1.27.0 k8s.io/api v0.31.0 k8s.io/apimachinery v0.31.0 @@ -10,24 +13,21 @@ require ( ) require ( - github.com/beorn7/perks v1.0.1 // indirect - github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect - github.com/deckarep/golang-set/v2 v2.6.0 // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect github.com/evanphx/json-patch/v5 v5.9.0 // indirect - github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect github.com/go-openapi/swag v0.23.0 // indirect + github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect - github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/gnostic-models v0.6.8 // indirect github.com/google/go-cmp v0.6.0 // indirect github.com/google/gofuzz v1.2.0 // indirect + github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 // indirect github.com/google/uuid v1.6.0 // indirect github.com/imdario/mergo v0.3.16 // indirect github.com/josharian/intern v1.0.0 // indirect @@ -37,10 +37,6 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/prometheus/client_golang v1.19.1 // indirect - github.com/prometheus/client_model v0.6.1 // indirect - github.com/prometheus/common v0.55.0 // indirect - github.com/prometheus/procfs v0.15.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/x448/float16 v0.8.4 // indirect go.uber.org/multierr v1.11.0 // indirect @@ -51,12 +47,11 @@ require ( golang.org/x/term v0.23.0 // indirect golang.org/x/text v0.17.0 // indirect golang.org/x/time v0.6.0 // indirect - gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect + golang.org/x/tools v0.24.0 // indirect google.golang.org/protobuf v1.34.2 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/apiextensions-apiserver v0.31.0 // indirect k8s.io/client-go v0.31.0 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 // indirect diff --git a/go.sum b/go.sum index c0d906b..9e7712b 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,6 @@ github.com/deckarep/golang-set/v2 v2.6.0 h1:XfcQbWM1LlMB8BsJ8N9vW5ehnnPVIw0je80N github.com/deckarep/golang-set/v2 v2.6.0/go.mod h1:VAky9rY/yGXJOLEDv3OMci+7wtDpOF4IN+y82NBOac4= github.com/emicklei/go-restful/v3 v3.12.1 h1:PJMDIM/ak7btuL8Ex0iYET9hxM3CI2sjZtzpL63nKAU= github.com/emicklei/go-restful/v3 v3.12.1/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= -github.com/evanphx/json-patch v0.5.2 h1:xVCHIVMUu1wtM/VkR9jVZ45N3FhZfYMMYGorLCR8P3k= -github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ= github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= @@ -102,8 +100,6 @@ go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= -go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= diff --git a/pkg/resource_properties/resource_properties.go b/pkg/resource_properties/resource_properties.go index 5ca78b9..a20f7ce 100644 --- a/pkg/resource_properties/resource_properties.go +++ b/pkg/resource_properties/resource_properties.go @@ -16,9 +16,11 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "math" "strconv" + "strings" ) type ResourceProperty string +type ResourceKind string const ( ResourceInvalid ResourceProperty = "invalid" @@ -26,16 +28,29 @@ const ( ResourceLimits ResourceProperty = "limits" ResourcePodMinimum ResourceProperty = "pod-minimum" ResourcePodMaximum ResourceProperty = "pod-maximum" + + ResourceFraction ResourceKind = "fraction" + ResourceQuantity ResourceKind = "quantity" ) var allValidResourceProperties = []ResourceProperty{ResourceRequests, ResourceLimits, ResourcePodMinimum, ResourcePodMaximum} type ResourcePropertyBinding struct { + resourceKind ResourceKind resourceProp ResourceProperty resourceName corev1.ResourceName value float64 } +func NewBinding(resourceKind ResourceKind, resourceProp ResourceProperty, resourceName corev1.ResourceName, value float64) *ResourcePropertyBinding { + return &ResourcePropertyBinding{ + resourceKind: resourceKind, + resourceProp: resourceProp, + resourceName: resourceName, + value: value, + } +} + func (rpb *ResourcePropertyBinding) ResourceName() corev1.ResourceName { return rpb.resourceName } @@ -48,16 +63,37 @@ func (rpb *ResourcePropertyBinding) Value() float64 { return rpb.value } +func (rpb *ResourcePropertyBinding) SetValue(v float64) { + rpb.value = v +} + +func (rpb *ResourcePropertyBinding) String() string { + return fmt.Sprintf("%s.%s=%f=%s (%s)", rpb.resourceProp, rpb.resourceName, rpb.value, rpb.HumanValue(), rpb.resourceKind) +} + +func appropriateIntegerExponent(n float64, base float64) int { + if n == 0 { + return 0 + } + log := math.Log(n) / math.Log(base) + truncatedLog := int(math.Trunc(log)) + return (truncatedLog / 3) * 3 // (8 / 3) * 3 = 6, as everybody knows +} + // HumanValue converts from the internal float to a string that looks like // the usual suffixed representation, i.e. 2G or 200m func (rpb *ResourcePropertyBinding) HumanValue() string { + if rpb.resourceKind == ResourceFraction { + return strconv.FormatFloat(rpb.value, 'f', -1, 64) + } + milliQty := rpb.value * 1000 if milliQty > 10_000 { - scale := math.Log10(rpb.value) + scale := appropriateIntegerExponent(rpb.value, 10.0) // we should be aware if we're not a power of 10 but a power of 2 instead, to preserve Mi/Gi suffixes exp := math.Pow10(int(scale)) return resource.NewScaledQuantity(int64(math.Floor(rpb.value/exp)), resource.Scale(scale)).String() } else { - return resource.NewMilliQuantity(int64(milliQty), resource.BinarySI).String() + return resource.NewMilliQuantity(int64(milliQty), resource.DecimalSI).String() } } @@ -67,14 +103,14 @@ func (rpb *ResourcePropertyBinding) PropertyJsonPath(containerIndex int) string // We could technically allow other packages to register or modify the supported annotations. Should we? File an issue! var supportedAnnotations = map[string]ResourcePropertyBinding{ - "node-specific-sizing.manomano.tech/request-cpu-fraction": {resourceProp: ResourceRequests, resourceName: corev1.ResourceCPU}, - "node-specific-sizing.manomano.tech/request-memory-fraction": {resourceProp: ResourceRequests, resourceName: corev1.ResourceMemory}, - "node-specific-sizing.manomano.tech/limit-cpu-fraction": {resourceProp: ResourceLimits, resourceName: corev1.ResourceCPU}, - "node-specific-sizing.manomano.tech/limit-memory-fraction": {resourceProp: ResourceLimits, resourceName: corev1.ResourceMemory}, - "node-specific-sizing.manomano.tech/minimum-cpu-request": {resourceProp: ResourcePodMinimum, resourceName: corev1.ResourceCPU}, - "node-specific-sizing.manomano.tech/maximum-cpu-request": {resourceProp: ResourcePodMaximum, resourceName: corev1.ResourceCPU}, - "node-specific-sizing.manomano.tech/minimum-memory-request": {resourceProp: ResourcePodMinimum, resourceName: corev1.ResourceMemory}, - "node-specific-sizing.manomano.tech/maximum-memory-request": {resourceProp: ResourcePodMaximum, resourceName: corev1.ResourceMemory}, + "node-specific-sizing.manomano.tech/request-cpu-fraction": {resourceKind: ResourceFraction, resourceProp: ResourceRequests, resourceName: corev1.ResourceCPU}, + "node-specific-sizing.manomano.tech/request-memory-fraction": {resourceKind: ResourceFraction, resourceProp: ResourceRequests, resourceName: corev1.ResourceMemory}, + "node-specific-sizing.manomano.tech/limit-cpu-fraction": {resourceKind: ResourceFraction, resourceProp: ResourceLimits, resourceName: corev1.ResourceCPU}, + "node-specific-sizing.manomano.tech/limit-memory-fraction": {resourceKind: ResourceFraction, resourceProp: ResourceLimits, resourceName: corev1.ResourceMemory}, + "node-specific-sizing.manomano.tech/minimum-cpu": {resourceKind: ResourceQuantity, resourceProp: ResourcePodMinimum, resourceName: corev1.ResourceCPU}, + "node-specific-sizing.manomano.tech/minimum-memory": {resourceKind: ResourceQuantity, resourceProp: ResourcePodMinimum, resourceName: corev1.ResourceMemory}, + "node-specific-sizing.manomano.tech/maximum-cpu": {resourceKind: ResourceQuantity, resourceProp: ResourcePodMaximum, resourceName: corev1.ResourceCPU}, + "node-specific-sizing.manomano.tech/maximum-memory": {resourceKind: ResourceQuantity, resourceProp: ResourcePodMaximum, resourceName: corev1.ResourceMemory}, } type ResourceProperties struct { @@ -98,7 +134,7 @@ func NewFromAnnotations(annotations map[string]string) (error, *ResourceProperti for supportedAnnotation, supportedBinding := range supportedAnnotations { if value, ok := annotations[supportedAnnotation]; ok { - err := result.BindPropertyString(supportedBinding.resourceProp, supportedBinding.resourceName, value) + err := result.BindPropertyString(supportedBinding.resourceKind, supportedBinding.resourceProp, supportedBinding.resourceName, value) if err != nil { return err, nil } @@ -108,6 +144,15 @@ func NewFromAnnotations(annotations map[string]string) (error, *ResourceProperti return nil, result } +func (rp *ResourceProperties) String() string { + sb := strings.Builder{} + for rp := range rp.All() { + sb.WriteString(rp.String()) + sb.WriteByte('\n') + } + return sb.String() +} + // GetValue returns (value, true) of an existing binding, or (0, false) for an unbound prop func (rp *ResourceProperties) GetValue(prop ResourceProperty, res corev1.ResourceName) (float64, bool) { if ourBinding, ok := rp.props[prop][res]; ok { @@ -137,51 +182,62 @@ func (rp *ResourceProperties) Bind(bind ResourcePropertyBinding) { } // BindPropertyFloat binds a given resource property to a float value -func (rp *ResourceProperties) BindPropertyFloat(prop ResourceProperty, res corev1.ResourceName, value float64) { +func (rp *ResourceProperties) BindPropertyFloat(kind ResourceKind, prop ResourceProperty, res corev1.ResourceName, value float64) { if existing, ok := rp.props[prop][res]; ok { existing.value = value } else { - rp.props[prop][res] = &ResourcePropertyBinding{prop, res, value} + rp.props[prop][res] = &ResourcePropertyBinding{kind, prop, res, value} } } -// BindPropertyString binds a given resource property to a float value by parsing it from a string. -func (rp *ResourceProperties) BindPropertyString(prop ResourceProperty, res corev1.ResourceName, value string) error { +func parseFraction(value string) (float64, error) { result, err := strconv.ParseFloat(value, 64) + if err != nil { - return err + return 0, err } if result <= 0 { // We forbid 0 included because it makes no sense as a request or limit - return fmt.Errorf("%s is not a valid fraction: cannot be <= 0", value) + return 0, fmt.Errorf("%s is not a valid fraction: cannot be <= 0", value) } if result > 1 { - return fmt.Errorf("%s is not a valid fraction: cannot be > 1", value) + return 0, fmt.Errorf("%s is not a valid fraction: cannot be > 1", value) } - rp.BindPropertyFloat(prop, res, result) + return result, nil +} - return nil +func parseQuantity(value string) (float64, error) { + qty, err := resource.ParseQuantity(value) + if err != nil { + return 0, err + } + return qty.AsApproximateFloat64(), nil } -// IncreaseBoundProperty mutates a bound value by adding an increment to it. Calling IncreaseBoundProperty() for an unbound -// property will bind it to the increment, similarly to BindPropertyFloat. -func (rp *ResourceProperties) IncreaseBoundProperty(prop ResourceProperty, res corev1.ResourceName, increment float64) { - if bind, ok := rp.props[prop][res]; ok { - bind.value += increment +// BindPropertyString binds a given resource property to a float value by parsing it from a string. +// The parsing is different whether the kind is a fraction or a quantity: +// - For fractions, a floating point number between 0 and 1 (excluded) is expected. +// I'm ~into the idea of support N/M rationals, but that might be purely a curiosity thing. +// - For quantities, any number that Kubernetes would accept will do. That includes many quantities with SI suffixes, like 100m or 2G +func (rp *ResourceProperties) BindPropertyString(kind ResourceKind, prop ResourceProperty, res corev1.ResourceName, value string) error { + var err error + var parsedValue float64 + + if kind == ResourceFraction { + parsedValue, err = parseFraction(value) } else { - rp.props[prop][res] = &ResourcePropertyBinding{prop, res, increment} + parsedValue, err = parseQuantity(value) } -} -// ScaleBoundProperty mutates a bound value by multiplying it by a scale factor. Calling ScaleBoundProperty() for an unbound -// property is a no-op. -func (rp *ResourceProperties) ScaleBoundProperty(prop ResourceProperty, res corev1.ResourceName, factor float64) { - if bind, ok := rp.props[prop][res]; ok { - bind.value *= factor + if err != nil { + return fmt.Errorf("%s cannot be parsed as a %s: %s", value, kind, err) } + + rp.BindPropertyFloat(kind, prop, res, parsedValue) + return nil } // Add merges two sets of properties into the receiver, by adding properties values from the added props, creating new @@ -201,21 +257,28 @@ func (rp *ResourceProperties) Add(operand *ResourceProperties) { // AddResourceRequirements merge a Kubernetes ResourceRequirements to the props func (rp *ResourceProperties) AddResourceRequirements(reqs *corev1.ResourceRequirements) { for name, quantity := range reqs.Requests { - rp.BindPropertyFloat(ResourceRequests, name, quantity.AsApproximateFloat64()) + rp.BindPropertyFloat(ResourceQuantity, ResourceRequests, name, quantity.AsApproximateFloat64()) } for name, quantity := range reqs.Limits { - rp.BindPropertyFloat(ResourceLimits, name, quantity.AsApproximateFloat64()) + rp.BindPropertyFloat(ResourceQuantity, ResourceLimits, name, quantity.AsApproximateFloat64()) } } // Mul produces new resource properties by multiplying the receiver values by the operand values // Props unset on either side of the operation are unset on the result rather than set to zero. +// +// The output kind depends on the input kind : multiplying two fractions produces another fraction, +// while any other combination produces a quantity. func (rp *ResourceProperties) Mul(operand *ResourceProperties) *ResourceProperties { result := New() for ourBinding := range rp.All() { if otherBinding, ok := operand.props[ourBinding.resourceProp][ourBinding.resourceName]; ok { - result.BindPropertyFloat(ourBinding.resourceProp, ourBinding.resourceName, ourBinding.value*otherBinding.value) + kind := ResourceQuantity + if ourBinding.resourceKind == ResourceFraction && otherBinding.resourceKind == ResourceFraction { + kind = ResourceFraction + } + result.BindPropertyFloat(kind, ourBinding.resourceProp, ourBinding.resourceName, ourBinding.value*otherBinding.value) } } return result @@ -228,11 +291,21 @@ func (rp *ResourceProperties) Mul(operand *ResourceProperties) *ResourceProperti // // If some props are defined on the operand but not on the receiver, then these props will be absent // from the result. +// +// The ResourceKind algebra is as follows: +// - quantity / quantity => fraction +// - fraction / quantity => quantity (weird way to put things, consider using Mul instead) +// - quantity / fraction => quantity (weird way to put things, consider using Mul instead) +// - fraction / fraction => fraction func (rp *ResourceProperties) Div(operand *ResourceProperties) *ResourceProperties { result := New() for ourBinding := range rp.All() { otherBinding := operand.props[ourBinding.resourceProp][ourBinding.resourceName] - result.BindPropertyFloat(ourBinding.resourceProp, ourBinding.resourceName, ourBinding.value/otherBinding.value) + kind := ResourceQuantity + if ourBinding.resourceKind == otherBinding.resourceKind { + kind = ResourceFraction + } + result.BindPropertyFloat(kind, ourBinding.resourceProp, ourBinding.resourceName, ourBinding.value/otherBinding.value) } return result } @@ -264,7 +337,28 @@ func (rp *ResourceProperties) ForceLimitAboveRequest() { if hasRequest && hasLimit && (request.Value() > limit.Value()) { // XXX log warning, we shouldn't have to do this but because of float imprecision, we sometimes do - rp.BindPropertyFloat(ResourceRequests, resourceName, limit.Value()) + rp.BindPropertyFloat(request.resourceKind, ResourceRequests, resourceName, limit.Value()) + } + } +} + +// ClampRequestsAndLimits goes over every bound property. If, for any given resourceName, a limit or a requests needs +// to be clamped according to the matching minimum or maximum from userSettings, it will be. +func (rp *ResourceProperties) ClampRequestsAndLimits(userSettings *ResourceProperties) { + // It could be asserted that the receiver is only made of + for resourceName := range rp.allResourceNames() { + minimum, hasMinimum := userSettings.props[ResourcePodMinimum][resourceName] + maximum, hasMaximum := userSettings.props[ResourcePodMaximum][resourceName] + + for _, prop := range []ResourceProperty{ResourceLimits, ResourceRequests} { + if bind, isBound := rp.props[prop][resourceName]; isBound { + if hasMinimum && bind.Value() < minimum.Value() { + bind.SetValue(minimum.Value()) + } + if hasMaximum && bind.Value() > maximum.Value() { + bind.SetValue(maximum.Value()) + } + } } } } diff --git a/pkg/resource_properties/resource_properties_suite_test.go b/pkg/resource_properties/resource_properties_suite_test.go new file mode 100644 index 0000000..789c307 --- /dev/null +++ b/pkg/resource_properties/resource_properties_suite_test.go @@ -0,0 +1,13 @@ +package resource_properties_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestResourceProperties(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "ResourceProperties Suite") +} diff --git a/pkg/resource_properties/resource_properties_test.go b/pkg/resource_properties/resource_properties_test.go new file mode 100644 index 0000000..a4116b9 --- /dev/null +++ b/pkg/resource_properties/resource_properties_test.go @@ -0,0 +1,19 @@ +package resource_properties_test + +import ( + rps "github.com/ManoManoTech/kubernetes-node-specific-sizing/pkg/resource_properties" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +var _ = Describe("Manipulating resource property bindings", Label("ResourcePropertyBinding"), func() { + When("the quantity is reasonably large", func() { + rpa := rps.NewBinding(rps.ResourceQuantity, rps.ResourceRequests, corev1.ResourceCPU, 840_000_000) + rpb := rps.NewBinding(rps.ResourceQuantity, rps.ResourceRequests, corev1.ResourceCPU, 342_000_000) + It("prints with the proper suffix without loss of precision", func(ctx SpecContext) { + Expect(rpa.HumanValue()).To(Equal("840M")) + Expect(rpb.HumanValue()).To(Equal("342M")) + }) + }) +})