Incorrect quantity calculations in CEL expressions

[MondayCha]

How to use it?

• kwok
• kwokctl --runtime=docker (default runtime)
• kwokctl --runtime=binary
• kwokctl --runtime=nerdctl
• kwokctl --runtime=kind

What happened?

I am currently simulating a large-scale cluster using Kwok to test Horizontal Pod Autoscaling (HPA). My setup involves simulating a lots of Pods with resource usage fluctuating randomly within a defined range.

While configuring the ClusterResourceUsage expression, I noticed an issue where performing calculations on Quantity leads to incorrect results.

To address this, I have submitted a simple PR to address this issue.

What did you expect to happen?

The result of Quantity in the CEL expression should be correct.

How can we reproduce it (as minimally and precisely as possible)?

Normal Case

Directly specifying Quantity values in ClusterResourceUsage CRD for easy reproduction:

apiVersion: kwok.x-k8s.io/v1alpha1
kind: ClusterResourceUsage
metadata:
  name: usage-from-annotation
spec:
  usages:
  - usage:
      cpu:
        expression: |
          Quantity("1")
      memory:
        expression: |
          Quantity("2Gi")

Printing the resource usage of the Mock Pod:

$ kubectl top pod                   
NAME                        CPU(cores)   MEMORY(bytes)   
fake-pod-5c449fc8f7-2rt7z   1000m        2048Mi

When the expression in ClusterResourceUsage does not involve any operations on Quantity, the results are as expected.

Abnormal Case

Editing ClusterResourceUsage to use multiplication in the expressions, multiplying Quantity by 1. The expected result should remain consistent with the normal case.

apiVersion: kwok.x-k8s.io/v1alpha1
kind: ClusterResourceUsage
metadata:
  name: usage-from-annotation
spec:
  usages:
  - usage:
      cpu:
        expression: |
          Quantity("1") * 1.0
      memory:
        expression: |
          Quantity("2Gi") * 1.0

Printing the resource usage of the Mock Pod:

$ kubectl top pod
NAME                        CPU(cores)   MEMORY(bytes)   
fake-pod-5c449fc8f7-2rt7z   10001m       -8796Mi

Observed that the CPU usage is multiplied by a factor of ten, and the Memory value appears to have overflowed.

Anything else we need to know?

After consistently reproducing the problem, I read the Kwok source code and identified the issue in the newQuantityFromFloat64 function of quantity.go:

func newQuantityFromFloat64(v float64) Quantity {
    r := resource.NewScaledQuantity(int64(v * 10e9), resource.Nano)
    return NewQuantity(r)
}

The problem lies in the multiplication of the input float64 value v by $10 \times 10^{9}$. Since resource.Nano represents $1 \times 10^{-9}$, this results in the function returning a value that is 10 times greater than expected.

Unfortunately, the unit tests did not catch this problem, as they also performed the incorrect calculation. In the TestResourceEvaluation function of evaluator_test.go, the correct calculation should be:

$$\begin{align*} & ( \text{Quantity}("10m") + \text{Quantity}("90m") + \text{Quantity}("10m") + \text{Quantity}("10m") ) \times 1.5 \times 10 \\\ & = \text{Quantity}("120m") \times 15 \\\ & = \text{Quantity}("1800m") \\\ & = 1.8 \end{align*}$$

However, the unit tests compared the CEL expression's result to 18.

func TestResourceEvaluation(t *testing.T) {
	n := &corev1.Node{
		Status: corev1.NodeStatus{
			Allocatable: map[corev1.ResourceName]resource.Quantity{
				corev1.ResourceCPU: resource.MustParse("90m"),
			},
		},
	}
	p := &corev1.Pod{
		ObjectMeta: metav1.ObjectMeta{
			Annotations: map[string]string{
				"cpu_usage": "10m",
			},
		},
		Spec: corev1.PodSpec{
			Containers: []corev1.Container{
				{
					Resources: corev1.ResourceRequirements{
						Requests: map[corev1.ResourceName]resource.Quantity{
							corev1.ResourceCPU: resource.MustParse("10m"),
						},
					},
				},
			},
		},
	}

	exp := `( Quantity("10m") + Quantity(pod.metadata.annotations["cpu_usage"]) + node.status.allocatable["cpu"] + pod.spec.containers[0].resources.requests["cpu"] ) * 1.5 * 10`

	env, err := NewEnvironment(EnvironmentConfig{})
	if err != nil {
		t.Fatalf("failed to instantiate node Evaluator: %v", err)
	}

	eval, err := env.Compile(exp)
	if err != nil {
		t.Fatalf("failed to compile expression: %v", err)
	}

	actual, err := eval.EvaluateFloat64(context.Background(), Data{
		Node: n,
		Pod:  p,
	})
	if err != nil {
		t.Fatalf("evaluation failed: %v", err)
	}

	if actual != 18 {
		t.Errorf("expected %v, got %v", 18, actual)
	}
}

I have submitted a simple PR to address this issue.

Kwok version

$ kwok --version
0.6.1

OS version

$ cat /etc/os-release
PRETTY_NAME="Ubuntu 22.04.4 LTS"
NAME="Ubuntu"
VERSION_ID="22.04"
VERSION="22.04.4 LTS (Jammy Jellyfish)"
VERSION_CODENAME=jammy
ID=ubuntu
ID_LIKE=debian
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
UBUNTU_CODENAME=jammy
$ uname -a
Linux act-1 6.8.0-47-generic #47~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Oct  2 16:16:55 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

[MondayCha]

/assign @MondayCha