From 7d1757901ecccf0f587ee5b4e46894a9c19ee693 Mon Sep 17 00:00:00 2001 From: tylerslaton Date: Fri, 26 Jan 2024 10:38:14 -0500 Subject: [PATCH] fix: add CPU field to resolvedOfferings and change QuotaRequests to use 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 --- pkg/apis/internal.acorn.io/v1/appinstance.go | 6 +-- .../v1/zz_generated.deepcopy.go | 6 +-- pkg/computeclasses/computeclasses.go | 4 +- .../computeclass/all-set/existing.yaml | 8 ++-- pkg/controller/quota/quota.go | 31 +++++++------ .../quota/testdata/basic/input.yaml | 22 ++++++++-- .../quota/testdata/not-enforced/input.yaml | 23 ++++------ .../resolvedofferings/computeclass.go | 44 ++++++++++++------- .../expected.golden | 4 +- .../all-set-overwrite/expected.golden | 1 + .../computeclass/all-set/expected.golden | 1 + .../compute-class-default/existing.yaml | 6 +-- .../compute-class-default/expected.golden | 8 ++-- .../expected.golden | 4 +- .../expected.golden | 1 + pkg/controller/scheduling/scheduling.go | 5 +-- pkg/openapi/generated/openapi_generated.go | 6 +-- 17 files changed, 101 insertions(+), 79 deletions(-) diff --git a/pkg/apis/internal.acorn.io/v1/appinstance.go b/pkg/apis/internal.acorn.io/v1/appinstance.go index 6481c877c..2c707f8c0 100644 --- a/pkg/apis/internal.acorn.io/v1/appinstance.go +++ b/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 { diff --git a/pkg/apis/internal.acorn.io/v1/zz_generated.deepcopy.go b/pkg/apis/internal.acorn.io/v1/zz_generated.deepcopy.go index e917be03d..0bec3f406 100644 --- a/pkg/apis/internal.acorn.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/internal.acorn.io/v1/zz_generated.deepcopy.go @@ -1408,9 +1408,9 @@ func (in *ContainerResolvedOffering) DeepCopyInto(out *ContainerResolvedOffering *out = new(int64) **out = **in } - if in.CPUScaler != nil { - in, out := &in.CPUScaler, &out.CPUScaler - *out = new(float64) + if in.CPU != nil { + in, out := &in.CPU, &out.CPU + *out = new(int64) **out = **in } } diff --git a/pkg/computeclasses/computeclasses.go b/pkg/computeclasses/computeclasses.go index b8720cccc..01cb4067f 100644 --- a/pkg/computeclasses/computeclasses.go +++ b/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 { diff --git a/pkg/controller/appdefinition/testdata/computeclass/all-set/existing.yaml b/pkg/controller/appdefinition/testdata/computeclass/all-set/existing.yaml index c4fa886d4..9d66b0fc2 100644 --- a/pkg/controller/appdefinition/testdata/computeclass/all-set/existing.yaml +++ b/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 \ No newline at end of file + - bar diff --git a/pkg/controller/quota/quota.go b/pkg/controller/quota/quota.go index 11834ed8a..e8dd93edc 100644 --- a/pkg/controller/quota/quota.go +++ b/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,7 +124,7 @@ 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)) } } @@ -132,18 +132,21 @@ func addContainers(containers map[string]v1.Container, quotaRequest *adminv1.Quo 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 } diff --git a/pkg/controller/quota/testdata/basic/input.yaml b/pkg/controller/quota/testdata/basic/input.yaml index 6e6680243..09be973b2 100644 --- a/pkg/controller/quota/testdata/basic/input.yaml +++ b/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 diff --git a/pkg/controller/quota/testdata/not-enforced/input.yaml b/pkg/controller/quota/testdata/not-enforced/input.yaml index 6e6680243..b569c6bae 100644 --- a/pkg/controller/quota/testdata/not-enforced/input.yaml +++ b/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 diff --git a/pkg/controller/resolvedofferings/computeclass.go b/pkg/controller/resolvedofferings/computeclass.go index 41be722c8..b03cc7eb2 100644 --- a/pkg/controller/resolvedofferings/computeclass.go +++ b/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 } diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/acornfile-override-compute-class/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/acornfile-override-compute-class/expected.golden index 71607ade0..40315968a 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/acornfile-override-compute-class/expected.golden +++ b/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: diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/all-set-overwrite/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/all-set-overwrite/expected.golden index c1809048e..9a07a8985 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/all-set-overwrite/expected.golden +++ b/pkg/controller/resolvedofferings/testdata/computeclass/all-set-overwrite/expected.golden @@ -51,6 +51,7 @@ status: resolvedOfferings: containers: "": + cpu: 0 memory: 1048576 left: memory: 1048576 diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/all-set/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/all-set/expected.golden index 2c74889cc..d83795872 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/all-set/expected.golden +++ b/pkg/controller/resolvedofferings/testdata/computeclass/all-set/expected.golden @@ -50,6 +50,7 @@ status: resolvedOfferings: containers: "": + cpu: 0 memory: 1048576 left: memory: 1048576 diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/existing.yaml b/pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/existing.yaml index 1f862ac4e..4f0e3b883 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/existing.yaml +++ b/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: diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/expected.golden index 04ffc7b29..3b46bd188 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/compute-class-default/expected.golden +++ b/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: diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/user-override-compute-class/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/user-override-compute-class/expected.golden index 47301eef7..bcb2ab7b6 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/user-override-compute-class/expected.golden +++ b/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: diff --git a/pkg/controller/resolvedofferings/testdata/computeclass/with-acornfile-memory-and-spec-override/expected.golden b/pkg/controller/resolvedofferings/testdata/computeclass/with-acornfile-memory-and-spec-override/expected.golden index 1c2465d91..9cc799a5c 100644 --- a/pkg/controller/resolvedofferings/testdata/computeclass/with-acornfile-memory-and-spec-override/expected.golden +++ b/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 diff --git a/pkg/controller/scheduling/scheduling.go b/pkg/controller/scheduling/scheduling.go index e5bbe4a4c..c3ede5914 100644 --- a/pkg/controller/scheduling/scheduling.go +++ b/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 } diff --git a/pkg/openapi/generated/openapi_generated.go b/pkg/openapi/generated/openapi_generated.go index 8894dd84b..f631defb9 100644 --- a/pkg/openapi/generated/openapi_generated.go +++ b/pkg/openapi/generated/openapi_generated.go @@ -8852,10 +8852,10 @@ func schema_pkg_apis_internalacornio_v1_ContainerResolvedOffering(ref common.Ref Format: "int64", }, }, - "cpuScaler": { + "cpu": { SchemaProps: spec.SchemaProps{ - Type: []string{"number"}, - Format: "double", + Type: []string{"integer"}, + Format: "int64", }, }, },