Skip to content
This repository was archived by the owner on Mar 16, 2024. It is now read-only.

Commit 7d17579

Browse files
committed
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 <[email protected]>
1 parent 92d24ac commit 7d17579

File tree

17 files changed

+101
-79
lines changed

17 files changed

+101
-79
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,9 @@ type VolumeResolvedOffering struct {
282282
}
283283

284284
type ContainerResolvedOffering struct {
285-
Class string `json:"class,omitempty"`
286-
Memory *int64 `json:"memory,omitempty"`
287-
CPUScaler *float64 `json:"cpuScaler,omitempty"`
285+
Class string `json:"class,omitempty"`
286+
Memory *int64 `json:"memory,omitempty"`
287+
CPU *int64 `json:"cpu,omitempty"`
288288
}
289289

290290
type Scheduling struct {

pkg/apis/internal.acorn.io/v1/zz_generated.deepcopy.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/computeclasses/computeclasses.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,13 @@ func Validate(cc apiv1.ComputeClass, memory resource.Quantity, memDefault *int64
123123
return nil
124124
}
125125

126-
func CalculateCPU(cc internaladminv1.ProjectComputeClassInstance, memory resource.Quantity) (resource.Quantity, error) {
126+
func CalculateCPU(cc internaladminv1.ProjectComputeClassInstance, memory resource.Quantity) resource.Quantity {
127127
// The CPU scaler calculates the CPUs per Gi of memory so get the memory in a ratio of Gi
128128
memoryInGi := memory.AsApproximateFloat64() / gi
129129
// Since we're putting this in to mili-cpu's, multiply memoryInGi by the scaler and by 1000
130130
value := cc.CPUScaler * memoryInGi * 1000
131131

132-
return *resource.NewMilliQuantity(int64(math.Ceil(value)), resource.DecimalSI), nil
132+
return *resource.NewMilliQuantity(int64(math.Ceil(value)), resource.DecimalSI)
133133
}
134134

135135
func GetComputeClassNameForWorkload(workload string, container internalv1.Container, computeClasses internalv1.ComputeClassMap) string {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ metadata:
66
description: Simple description for a simple ComputeClass
77
cpuScaler: 0.25
88
memory:
9-
min: 1Mi
10-
max: 2Mi
11-
default: 1Mi
9+
min: 50Mi
10+
max: 200Mi
11+
default: 100Mi
1212
affinity:
1313
nodeAffinity:
1414
requiredDuringSchedulingIgnoredDuringExecution:
@@ -17,4 +17,4 @@ affinity:
1717
- key: foo
1818
operator: In
1919
values:
20-
- bar
20+
- bar

pkg/controller/quota/quota.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
adminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1"
99
"github.com/acorn-io/runtime/pkg/controller/appdefinition"
1010
"github.com/acorn-io/runtime/pkg/labels"
11-
corev1 "k8s.io/api/core/v1"
11+
"github.com/acorn-io/z"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1313

1414
"github.com/acorn-io/runtime/pkg/condition"
@@ -124,26 +124,29 @@ func EnsureQuotaRequest(req router.Request, resp router.Response) error {
124124
// addContainers adds the number of containers and accounts for the scale of each container.
125125
func addContainers(containers map[string]v1.Container, quotaRequest *adminv1.QuotaRequestInstance) {
126126
for _, container := range containers {
127-
quotaRequest.Spec.Resources.Containers += replicas(container.Scale)
127+
quotaRequest.Spec.Resources.Containers += int(replicas(container.Scale))
128128
}
129129
}
130130

131131
// addCompute adds the compute resources of the containers passed to the quota request.
132132
func addCompute(containers map[string]v1.Container, appInstance *v1.AppInstance, quotaRequest *adminv1.QuotaRequestInstance) {
133133
// For each workload, add their memory/cpu requests to the quota request
134134
for name, container := range containers {
135-
var requirements corev1.ResourceRequirements
136-
if specific, ok := appInstance.Status.Scheduling[name]; ok {
137-
requirements = specific.Requirements
138-
} else if all, ok := appInstance.Status.Scheduling[""]; ok {
139-
requirements = all.Requirements
135+
var cpu, memory resource.Quantity
136+
if specific, ok := appInstance.Status.ResolvedOfferings.Containers[name]; ok {
137+
memory = *resource.NewQuantity(z.Dereference(specific.Memory), resource.BinarySI)
138+
cpu = *resource.NewMilliQuantity(z.Dereference(specific.CPU), resource.DecimalSI)
139+
} else if all, ok := appInstance.Status.ResolvedOfferings.Containers[""]; ok {
140+
cpu = *resource.NewMilliQuantity(z.Dereference(all.CPU), resource.DecimalSI)
141+
memory = *resource.NewQuantity(z.Dereference(all.Memory), resource.BinarySI)
140142
}
141143

142-
// Add the memory/cpu requests to the quota request for each container at the scale specified
143-
for i := 0; i < replicas(container.Scale); i++ {
144-
quotaRequest.Spec.Resources.CPU.Add(requirements.Requests["cpu"])
145-
quotaRequest.Spec.Resources.Memory.Add(requirements.Requests["memory"])
146-
}
144+
// Multiply the memory/cpu requests by the scale of the container
145+
cpu.Mul(replicas(container.Scale))
146+
memory.Mul(replicas(container.Scale))
147+
148+
quotaRequest.Spec.Resources.CPU.Add(cpu)
149+
quotaRequest.Spec.Resources.Memory.Add(memory)
147150

148151
// Recurse over any sidecars. Since sidecars can't have sidecars, this is safe.
149152
addCompute(container.Sidecars, appInstance, quotaRequest)
@@ -256,9 +259,9 @@ func isEnforced(req router.Request, namespace string) (bool, error) {
256259

257260
// replicas returns the number of replicas based on an int32 pointer. If the
258261
// pointer is nil, it is assumed to be 1.
259-
func replicas(s *int32) int {
262+
func replicas(s *int32) int64 {
260263
if s != nil {
261-
return int(*s)
264+
return int64(*s)
262265
}
263266
return 1
264267
}

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,32 @@ status:
5151
test:
5252
accessModes:
5353
- readWriteOnce
54+
resolvedOfferings:
55+
containers:
56+
"":
57+
class: sample-compute-class
58+
cpu: 125
59+
memory: 536870912
60+
container-name:
61+
class: sample-compute-class
62+
cpu: 125
63+
memory: 536870912
64+
sidecar-name:
65+
class: sample-compute-class
66+
cpu: 125
67+
memory: 536870912
5468
scheduling:
5569
container-name:
5670
requirements:
5771
limits:
5872
memory: 512Mi
5973
requests:
60-
cpu: 125m
61-
memory: 512Mi
74+
cpu: 75m
75+
memory: 128Mi # simulate requestScaler
6276
sidecar-name:
6377
requirements:
6478
limits:
6579
memory: 512Mi
6680
requests:
67-
cpu: 125m
68-
memory: 512Mi
81+
cpu: 75m
82+
memory: 128Mi # simulate requestScaler

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,11 @@ status:
5151
test:
5252
accessModes:
5353
- readWriteOnce
54-
scheduling:
55-
container-name:
56-
requirements:
57-
limits:
58-
memory: 512Mi
59-
requests:
60-
cpu: 125m
61-
memory: 512Mi
62-
sidecar-name:
63-
requirements:
64-
limits:
65-
memory: 512Mi
66-
requests:
67-
cpu: 125m
68-
memory: 512Mi
54+
resolvedOfferings:
55+
containers:
56+
container-name:
57+
cpu: 125
58+
memory: 536870912 # 512Mi
59+
sidecar-name:
60+
memory: 536870912 # 512Mi
61+
cpu: 125

pkg/controller/resolvedofferings/computeclass.go

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import (
66
v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
77
adminv1 "github.com/acorn-io/runtime/pkg/apis/internal.admin.acorn.io/v1"
88
"github.com/acorn-io/runtime/pkg/computeclasses"
9+
"github.com/acorn-io/z"
10+
911
apierrors "k8s.io/apimachinery/pkg/api/errors"
12+
"k8s.io/apimachinery/pkg/api/resource"
1013
"k8s.io/utils/strings/slices"
1114
)
1215

@@ -45,19 +48,24 @@ func resolveComputeClasses(req router.Request, cfg *apiv1.Config, appInstance *v
4548
return err
4649
}
4750
def := parsedMemory.Def.Value()
51+
cpuQuantity := computeclasses.CalculateCPU(*cc, *parsedMemory.Def)
4852
appInstance.Status.ResolvedOfferings.Containers[""] = v1.ContainerResolvedOffering{
49-
Memory: &def,
50-
CPUScaler: &cc.CPUScaler,
51-
Class: appInstance.Status.ResolvedOfferings.Containers[""].Class,
53+
Memory: &def,
54+
CPU: z.Pointer(cpuQuantity.MilliValue()),
55+
Class: appInstance.Status.ResolvedOfferings.Containers[""].Class,
5256
}
5357
}
5458

5559
// Check to see if the user overrode the memory for all containers
56-
if appInstance.Spec.Memory[""] != nil {
60+
if specificMemory := appInstance.Spec.Memory[""]; specificMemory != nil {
61+
var cpuQuantity resource.Quantity
62+
if cc != nil {
63+
cpuQuantity = computeclasses.CalculateCPU(*cc, *resource.NewQuantity(*specificMemory, resource.BinarySI))
64+
}
5765
appInstance.Status.ResolvedOfferings.Containers[""] = v1.ContainerResolvedOffering{
58-
Memory: appInstance.Spec.Memory[""],
59-
CPUScaler: appInstance.Status.ResolvedOfferings.Containers[""].CPUScaler,
60-
Class: appInstance.Status.ResolvedOfferings.Containers[""].Class,
66+
Memory: appInstance.Spec.Memory[""],
67+
CPU: z.Pointer(cpuQuantity.MilliValue()),
68+
Class: appInstance.Status.ResolvedOfferings.Containers[""].Class,
6169
}
6270
}
6371

@@ -105,10 +113,7 @@ func resolveComputeClass(req router.Request, appInstance *v1.AppInstance, config
105113
}
106114

107115
func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInstance, configDefault *int64, defaultCC *adminv1.ProjectComputeClassInstance, defaultCCName, containerName string, container v1.Container) (v1.ContainerResolvedOffering, error) {
108-
var (
109-
cpuScaler *float64
110-
ccName string
111-
)
116+
var ccName string
112117

113118
// First, get the compute class for the workload
114119
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
120125
}
121126
if cc != nil {
122127
ccName = cc.Name
123-
cpuScaler = &cc.CPUScaler
124128
} else {
125129
ccName = defaultCCName
126130
}
@@ -131,7 +135,10 @@ func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInst
131135
// 3. defaults from compute class
132136
// 4. global default
133137

134-
memory := configDefault // set to global default first, then check the higher priority values
138+
var (
139+
memory = configDefault // set to global default first, then check the higher priority values
140+
cpu *int64
141+
)
135142

136143
if appInstance.Spec.Memory[containerName] != nil { // runtime-level overrides from the user
137144
memory = appInstance.Spec.Memory[containerName]
@@ -148,9 +155,14 @@ func resolveComputeClassForContainer(req router.Request, appInstance *v1.AppInst
148155
memory = &def
149156
}
150157

158+
if cc != nil {
159+
cpuQuantity := computeclasses.CalculateCPU(*cc, *resource.NewQuantity(*memory, resource.BinarySI))
160+
cpu = z.Pointer(cpuQuantity.MilliValue())
161+
}
162+
151163
return v1.ContainerResolvedOffering{
152-
Class: ccName,
153-
Memory: memory,
154-
CPUScaler: cpuScaler,
164+
Class: ccName,
165+
Memory: memory,
166+
CPU: cpu,
155167
}, nil
156168
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ status:
5353
memory: 0
5454
left:
5555
class: sample-compute-class
56-
cpuScaler: 0.25
56+
cpu: 1
5757
memory: 1048576
5858
oneimage:
5959
class: sample-compute-class
60-
cpuScaler: 0.25
60+
cpu: 1
6161
memory: 2097152
6262
region: local
6363
staged:

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ status:
5151
resolvedOfferings:
5252
containers:
5353
"":
54+
cpu: 0
5455
memory: 1048576
5556
left:
5657
memory: 1048576

0 commit comments

Comments
 (0)