fix: add CPU field to resolvedOfferings and change QuotaRequests to u…

…se resolvedOfferings

This change makes it so that we always quota against the resolvedOfferings instead
of the scheduling for the app. This is because the scheduling for the app is always
accurate to what is actually running, but the resolvedOfferings is what the user
requested.

This change also adds a CPU field to the resolvedOfferings so that we can quota
against the CPU that the user requested from one place instead of having to calculate
it from the cpuScaler.

Signed-off-by: tylerslaton <[email protected]>

pkg/apis/internal.acorn.io/v1/appinstance.go

@@ -282,9 +282,9 @@ type VolumeResolvedOffering struct {
 }
 
 type ContainerResolvedOffering struct {
-	Class     string   `json:"class,omitempty"`
-	Memory    *int64   `json:"memory,omitempty"`
-	CPUScaler *float64 `json:"cpuScaler,omitempty"`
+	Class  string `json:"class,omitempty"`
+	Memory *int64 `json:"memory,omitempty"`
+	CPU    *int64 `json:"cpu,omitempty"`
 }
 
 type Scheduling struct {

pkg/computeclasses/computeclasses.go

@@ -123,13 +123,13 @@ func Validate(cc apiv1.ComputeClass, memory resource.Quantity, memDefault *int64
 	return nil
 }
 
-func CalculateCPU(cc internaladminv1.ProjectComputeClassInstance, memory resource.Quantity) (resource.Quantity, error) {
+func CalculateCPU(cc internaladminv1.ProjectComputeClassInstance, memory resource.Quantity) resource.Quantity {
 	// The CPU scaler calculates the CPUs per Gi of memory so get the memory in a ratio of Gi
 	memoryInGi := memory.AsApproximateFloat64() / gi
 	// Since we're putting this in to mili-cpu's, multiply memoryInGi by the scaler and by 1000
 	value := cc.CPUScaler * memoryInGi * 1000
 
-	return *resource.NewMilliQuantity(int64(math.Ceil(value)), resource.DecimalSI), nil
+	return *resource.NewMilliQuantity(int64(math.Ceil(value)), resource.DecimalSI)
 }
 
 func GetComputeClassNameForWorkload(workload string, container internalv1.Container, computeClasses internalv1.ComputeClassMap) string {

pkg/controller/appdefinition/testdata/computeclass/all-set/existing.yaml

@@ -6,9 +6,9 @@ metadata:
 description: Simple description for a simple ComputeClass
 cpuScaler: 0.25
 memory:
-  min: 1Mi
-  max: 2Mi
-  default: 1Mi
+  min: 50Mi
+  max: 200Mi
+  default: 100Mi
 affinity:
   nodeAffinity:
     requiredDuringSchedulingIgnoredDuringExecution:
@@ -17,4 +17,4 @@ affinity:
         - key: foo
           operator: In
           values:
-          - bar
+          - bar

pkg/controller/quota/quota.go

@@ -8,7 +8,7 @@ import (
 	adminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1"
 	"github.com/acorn-io/runtime/pkg/controller/appdefinition"
 	"github.com/acorn-io/runtime/pkg/labels"
-	corev1 "k8s.io/api/core/v1"
+	"github.com/acorn-io/z"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/acorn-io/runtime/pkg/condition"
@@ -124,26 +124,29 @@ func EnsureQuotaRequest(req router.Request, resp router.Response) error {
 // addContainers adds the number of containers and accounts for the scale of each container.
 func addContainers(containers map[string]v1.Container, quotaRequest *adminv1.QuotaRequestInstance) {
 	for _, container := range containers {
-		quotaRequest.Spec.Resources.Containers += replicas(container.Scale)
+		quotaRequest.Spec.Resources.Containers += int(replicas(container.Scale))
 	}
 }
 
 // addCompute adds the compute resources of the containers passed to the quota request.
 func addCompute(containers map[string]v1.Container, appInstance *v1.AppInstance, quotaRequest *adminv1.QuotaRequestInstance) {
 	// For each workload, add their memory/cpu requests to the quota request
 	for name, container := range containers {
-		var requirements corev1.ResourceRequirements
-		if specific, ok := appInstance.Status.Scheduling[name]; ok {
-			requirements = specific.Requirements
-		} else if all, ok := appInstance.Status.Scheduling[""]; ok {
-			requirements = all.Requirements
+		var cpu, memory resource.Quantity
+		if specific, ok := appInstance.Status.ResolvedOfferings.Containers[name]; ok {
+			memory = *resource.NewQuantity(z.Dereference(specific.Memory), resource.BinarySI)
+			cpu = *resource.NewMilliQuantity(z.Dereference(specific.CPU), resource.DecimalSI)
+		} else if all, ok := appInstance.Status.ResolvedOfferings.Containers[""]; ok {
+			cpu = *resource.NewMilliQuantity(z.Dereference(all.CPU), resource.DecimalSI)
+			memory = *resource.NewQuantity(z.Dereference(all.Memory), resource.BinarySI)
 		}
 
-		// Add the memory/cpu requests to the quota request for each container at the scale specified
-		for i := 0; i < replicas(container.Scale); i++ {
-			quotaRequest.Spec.Resources.CPU.Add(requirements.Requests["cpu"])
-			quotaRequest.Spec.Resources.Memory.Add(requirements.Requests["memory"])
-		}
+		// Multiply the memory/cpu requests by the scale of the container
+		cpu.Mul(replicas(container.Scale))
+		memory.Mul(replicas(container.Scale))
+
+		quotaRequest.Spec.Resources.CPU.Add(cpu)
+		quotaRequest.Spec.Resources.Memory.Add(memory)
 
 		// Recurse over any sidecars. Since sidecars can't have sidecars, this is safe.
 		addCompute(container.Sidecars, appInstance, quotaRequest)
@@ -256,9 +259,9 @@ func isEnforced(req router.Request, namespace string) (bool, error) {
 
 // replicas returns the number of replicas based on an int32 pointer. If the
 // pointer is nil, it is assumed to be 1.
-func replicas(s *int32) int {
+func replicas(s *int32) int64 {
 	if s != nil {
-		return int(*s)
+		return int64(*s)
 	}
 	return 1
 }

pkg/controller/quota/testdata/basic/input.yaml

@@ -51,18 +51,32 @@ status:
       test:
         accessModes:
         - readWriteOnce
+  resolvedOfferings:
+    containers:
+      "":
+        class: sample-compute-class
+        cpu: 125
+        memory: 536870912
+      container-name:
+        class: sample-compute-class
+        cpu: 125
+        memory: 536870912
+      sidecar-name:
+        class: sample-compute-class
+        cpu: 125
+        memory: 536870912
   scheduling:
     container-name:
       requirements:
         limits:
           memory: 512Mi
         requests:
-          cpu: 125m
-          memory: 512Mi
+          cpu: 75m
+          memory: 128Mi # simulate requestScaler
     sidecar-name:
       requirements:
         limits:
           memory: 512Mi
         requests:
-          cpu: 125m
-          memory: 512Mi
+          cpu: 75m
+          memory: 128Mi # simulate requestScaler

pkg/controller/quota/testdata/not-enforced/input.yaml

@@ -51,18 +51,11 @@ status:
       test:
         accessModes:
         - readWriteOnce
-  scheduling:
-    container-name:
-      requirements:
-        limits:
-          memory: 512Mi
-        requests:
-          cpu: 125m
-          memory: 512Mi
-    sidecar-name:
-      requirements:
-        limits:
-          memory: 512Mi
-        requests:
-          cpu: 125m
-          memory: 512Mi
+  resolvedOfferings:
+    containers:
+      container-name:
+        cpu: 125
+        memory: 536870912 # 512Mi
+      sidecar-name:
+        memory: 536870912 # 512Mi
+        cpu: 125

pkg/controller/resolvedofferings/computeclass.go

@@ -6,7 +6,10 @@ import (
 	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
 	adminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1"
 	"github.com/acorn-io/runtime/pkg/computeclasses"
+	"github.com/acorn-io/z"
+
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/utils/strings/slices"
 )
 
@@ -45,19 +48,24 @@ func resolveComputeClasses(req router.Request, cfg *apiv1.Config, appInstance *v
 			return err
 		}
 		def := parsedMemory.Def.Value()
+		cpuQuantity := computeclasses.CalculateCPU(*cc, *parsedMemory.Def)
 		appInstance.Status.ResolvedOfferings.Containers[""] = v1.ContainerResolvedOffering{
-			Memory:    &def,
-			CPUScaler: &cc.CPUScaler,
-			Class:     appInstance.Status.ResolvedOfferings.Containers[""].Class,
+			Memory: &def,
+			CPU:    z.Pointer(cpuQuantity.MilliValue()),
+			Class:  appInstance.Status.ResolvedOfferings.Containers[""].Class,
 		}
 	}
 
 	// Check to see if the user overrode the memory for all containers
-	if appInstance.Spec.Memory[""] != nil {
+	if specificMemory := appInstance.Spec.Memory[""]; specificMemory != nil {
+		var cpuQuantity resource.Quantity
+		if cc != nil {
+			cpuQuantity = computeclasses.CalculateCPU(*cc, *resource.NewQuantity(*specificMemory, resource.BinarySI))
+		}
 		appInstance.Status.ResolvedOfferings.Containers[""] = v1.ContainerResolvedOffering{
-			Memory:    appInstance.Spec.Memory[""],
-			CPUScaler: appInstance.Status.ResolvedOfferings.Containers[""].CPUScaler,
-			Class:     appInstance.Status.ResolvedOfferings.Containers[""].Class,
+			Memory: appInstance.Spec.Memory[""],
+			CPU:    z.Pointer(cpuQuantity.MilliValue()),
+			Class:  appInstance.Status.ResolvedOfferings.Containers[""].Class,
 		}
 	}
 
@@ -105,10 +113,7 @@ func resolveComputeClass(req router.Request, appInstance *v1.AppInstance, config
 }
 
 func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInstance, configDefault *int64, defaultCC *adminv1.ProjectComputeClassInstance, defaultCCName, containerName string, container v1.Container) (v1.ContainerResolvedOffering, error) {
-	var (
-		cpuScaler *float64
-		ccName    string
-	)
+	var ccName string
 
 	// First, get the compute class for the workload
 	cc, err := computeclasses.GetClassForWorkload(req.Ctx, req.Client, appInstance.Spec.ComputeClasses, container, containerName, appInstance.Namespace)
@@ -120,7 +125,6 @@ func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInst
 	}
 	if cc != nil {
 		ccName = cc.Name
-		cpuScaler = &cc.CPUScaler
 	} else {
 		ccName = defaultCCName
 	}
@@ -131,7 +135,10 @@ func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInst
 	// 3. defaults from compute class
 	// 4. global default
 
-	memory := configDefault // set to global default first, then check the higher priority values
+	var (
+		memory = configDefault // set to global default first, then check the higher priority values
+		cpu    *int64
+	)
 
 	if appInstance.Spec.Memory[containerName] != nil { // runtime-level overrides from the user
 		memory = appInstance.Spec.Memory[containerName]
@@ -148,9 +155,14 @@ func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInst
 		memory = &def
 	}
 
+	if cc != nil {
+		cpuQuantity := computeclasses.CalculateCPU(*cc, *resource.NewQuantity(*memory, resource.BinarySI))
+		cpu = z.Pointer(cpuQuantity.MilliValue())
+	}
+
 	return v1.ContainerResolvedOffering{
-		Class:     ccName,
-		Memory:    memory,
-		CPUScaler: cpuScaler,
+		Class:  ccName,
+		Memory: memory,
+		CPU:    cpu,
 	}, nil
 }

pkg/controller/resolvedofferings/testdata/computeclass/acornfile-override-compute-class/expected.golden

@@ -53,11 +53,11 @@ status:
         memory: 0
       left:
         class: sample-compute-class
-        cpuScaler: 0.25
+        cpu: 1
         memory: 1048576
       oneimage:
         class: sample-compute-class
-        cpuScaler: 0.25
+        cpu: 1
         memory: 2097152
     region: local
   staged:

pkg/controller/resolvedofferings/testdata/computeclass/all-set-overwrite/expected.golden

@@ -51,6 +51,7 @@ status:
   resolvedOfferings:
     containers:
       "":
+        cpu: 0
         memory: 1048576
       left:
         memory: 1048576

pkg/controller/resolvedofferings/testdata/computeclass/all-set/expected.golden

@@ -50,6 +50,7 @@ status:
   resolvedOfferings:
     containers:
       "":
+        cpu: 0
         memory: 1048576
       left:
         memory: 1048576

pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/existing.yaml

@@ -17,9 +17,9 @@ description: Simple description for a simple ComputeClass
 cpuScaler: 0.25
 default: true
 memory:
-  min: 1Mi # 1Mi
-  max: 2Mi # 2Mi
-  default: 1Mi # 1Mi
+  min: 100Mi # 100Mi
+  max: 200Mi # 200Mi
+  default: 100Mi # 100Mi
 affinity:
   nodeAffinity:
     requiredDuringSchedulingIgnoredDuringExecution:

pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/expected.golden

@@ -52,12 +52,12 @@ status:
         memory: 0
       left:
         class: sample-compute-class
-        cpuScaler: 0.25
-        memory: 1048576
+        cpu: 25
+        memory: 104857600
       oneimage:
         class: sample-compute-class
-        cpuScaler: 0.25
-        memory: 1048576
+        cpu: 25
+        memory: 104857600
     region: local
   staged:
     appImage:

pkg/controller/resolvedofferings/testdata/computeclass/user-override-compute-class/expected.golden

@@ -55,11 +55,11 @@ status:
         memory: 0
       left:
         class: sample-compute-class
-        cpuScaler: 0.25
+        cpu: 1
         memory: 1048576
       oneimage:
         class: sample-compute-class
-        cpuScaler: 0.25
+        cpu: 1
         memory: 3145728
     region: local
   staged:

pkg/controller/resolvedofferings/testdata/computeclass/with-acornfile-memory-and-spec-override/expected.golden

@@ -51,6 +51,7 @@ status:
   resolvedOfferings:
     containers:
       "":
+        cpu: 0
         memory: 3145728
       left:
         memory: 3145728

pkg/controller/scheduling/scheduling.go

@@ -219,10 +219,7 @@ func ResourceRequirements(req router.Request, app *v1.AppInstance, containerName
 	}
 
 	if computeClass != nil {
-		cpuQuantity, err := computeclasses.CalculateCPU(*computeClass, memoryRequest)
-		if err != nil {
-			return nil, err
-		}
+		cpuQuantity := computeclasses.CalculateCPU(*computeClass, memoryRequest)
 		if cpuQuantity.Value() != 0 {
 			requirements.Requests[corev1.ResourceCPU] = cpuQuantity
 		}