Merge pull request #2462 from tylerslaton/quota-ro

fix: add CPU field to resolvedOfferings and change QuotaRequests to use resolvedOfferings

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
 		}