Skip to content

Commit b83358e

Browse files
committed
Include initContainers when calculating pod overhead
2i2c-org#3569 changed the cryptnono daemonset to have different resource requests for the init containers as well as the container. While working on 2i2c-org#3566, I noticed this was generating wrong choices - the overhead was calculated wrong (too small). We were intentionally ignoring init containers while calculating overhead, and turns out the scheduler and the autoscaler both do take it into consideration. The effective resource request for a pod is the higher of the resource requests for the containers *or* the init containers - this ensures that a pod with higher requests for init containers than containers (like our cryptnono pod!) will actually run. This is documented at https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resource-sharing-within-containers, and implemented in Kubernetes itself at https://github.com/kubernetes/kubernetes/blob/9bd0ef5f173de3cc2d1d629a4aee499d53690aee/pkg/api/v1/resource/helpers.go#L50 (this is the library code that the cluster autoscaler uses). This PR updates the two places we currently have that calculate effective resource requests (I assume eventually these will be merged into one - I haven't kept up with the team's work last quarter here). I've updated the node-capacity-info.json file, which is what seems to be used by the generator script right now.
1 parent e6ed45a commit b83358e

File tree

4 files changed

+72
-23
lines changed

4 files changed

+72
-23
lines changed

deployer/commands/generate/resource_allocation/daemonset_requests.py

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,23 +64,50 @@ def get_daemon_sets_requests():
6464
info = []
6565
for ds in daemon_sets:
6666
name = ds["metadata"]["name"]
67-
req_mem = req_cpu = lim_mem = lim_cpu = 0
67+
# From https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resource-sharing-within-containers
68+
# > - The highest of any particular resource request or limit defined on
69+
# > all init containers is the effective init request/limit. If any
70+
# > resource has no resource limit specified this is considered as the
71+
# > highest limit.
72+
# > - The Pod's effective request/limit for a resource is the higher of:
73+
# > - the sum of all app containers request/limit for a resource
74+
# > - the effective init request/limit for a resource
75+
#
76+
# So we have to calculate the requests of the init containers and containers separately,
77+
# and take the max as the effective request / limit
78+
79+
container_req_mem = (
80+
container_req_cpu
81+
) = container_lim_mem = container_lim_cpu = 0
82+
init_container_req_mem = (
83+
init_container_req_cpu
84+
) = init_container_lim_mem = init_container_lim_cpu = 0
85+
6886
for c in ds["spec"]["template"]["spec"]["containers"]:
6987
resources = c.get("resources", {})
7088
requests = resources.get("requests", {})
7189
limits = resources.get("limits", {})
72-
req_mem += parse_quantity(requests.get("memory", 0))
73-
lim_mem += parse_quantity(limits.get("memory", 0))
74-
req_cpu += parse_quantity(requests.get("cpu", 0))
75-
lim_cpu += parse_quantity(limits.get("cpu", 0))
90+
container_req_mem += parse_quantity(requests.get("memory", 0))
91+
container_lim_mem += parse_quantity(limits.get("memory", 0))
92+
container_req_cpu += parse_quantity(requests.get("cpu", 0))
93+
container_lim_cpu += parse_quantity(limits.get("cpu", 0))
94+
95+
for c in ds["spec"]["template"]["spec"].get("initContainers", []):
96+
resources = c.get("resources", {})
97+
requests = resources.get("requests", {})
98+
limits = resources.get("limits", {})
99+
init_container_req_mem += parse_quantity(requests.get("memory", 0))
100+
init_container_lim_mem += parse_quantity(limits.get("memory", 0))
101+
init_container_req_cpu += parse_quantity(requests.get("cpu", 0))
102+
init_container_lim_cpu += parse_quantity(limits.get("cpu", 0))
76103

77104
info.append(
78105
{
79106
"name": name,
80-
"cpu_request": float(req_cpu),
81-
"cpu_limit": float(lim_cpu),
82-
"memory_request": int(req_mem),
83-
"memory_limit": int(lim_mem),
107+
"cpu_request": float(max(container_req_cpu, init_container_req_cpu)),
108+
"cpu_limit": float(max(container_lim_cpu, init_container_lim_cpu)),
109+
"memory_request": int(max(container_req_mem, init_container_req_mem)),
110+
"memory_limit": int(max(container_lim_mem, init_container_lim_mem)),
84111
}
85112
)
86113

deployer/commands/generate/resource_allocation/daemonset_requests.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ eks:
134134
other_daemon_sets: ""
135135
cpu_requests: 170m
136136
memory_requests: 250Mi
137-
k8s_version: v1.25.12-eks-2d98532
137+
k8s_version: v1.27.8-eks-8cb36c9
138138
openscapes:
139139
requesting_daemon_sets: aws-node,ebs-csi-node,kube-proxy,support-cryptnono,support-prometheus-node-exporter
140140
other_daemon_sets: ""

deployer/commands/generate/resource_allocation/node-capacity-info.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@
5555
"memory": 130451771392
5656
},
5757
"measured_overhead": {
58-
"cpu": 0.165,
59-
"memory": 157286400
58+
"cpu": 0.17,
59+
"memory": 262144000
6060
},
6161
"available": {
62-
"cpu": 15.725,
63-
"memory": 130294484992
62+
"cpu": 15.72,
63+
"memory": 130189627392
6464
}
6565
},
6666
"n2-highmem-32": {

deployer/commands/generate/resource_allocation/update_nodeinfo.py

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,20 +106,42 @@ def get_node_capacity_info(instance_type: str):
106106
mem_available = mem_allocatable
107107

108108
for p in pods:
109-
mem_request = 0
110-
cpu_request = 0
111-
# Iterate through all the containers in the pod, and count the memory & cpu requests
112-
# they make. We don't count initContainers' requests as they don't overlap with the
113-
# container requests at any point.
109+
# From https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#resource-sharing-within-containers
110+
# > - The highest of any particular resource request or limit defined on
111+
# > all init containers is the effective init request/limit. If any
112+
# > resource has no resource limit specified this is considered as the
113+
# > highest limit.
114+
# > - The Pod's effective request/limit for a resource is the higher of:
115+
# > - the sum of all app containers request/limit for a resource
116+
# > - the effective init request/limit for a resource
117+
#
118+
# So we have to calculate the requests of the init containers and containers separately,
119+
# and take the max as the effective request / limit
120+
container_cpu_request = container_mem_request = 0
121+
init_container_cpu_request = init_container_mem_request = 0
122+
114123
for c in p["spec"]["containers"]:
115-
mem_request += parse_quantity(
124+
container_mem_request += parse_quantity(
125+
c.get("resources", {}).get("requests", {}).get("memory", "0")
126+
)
127+
container_cpu_request += parse_quantity(
128+
c.get("resources", {}).get("requests", {}).get("cpu", "0")
129+
)
130+
131+
for c in p["spec"].get("initContainers", []):
132+
init_container_mem_request += parse_quantity(
116133
c.get("resources", {}).get("requests", {}).get("memory", "0")
117134
)
118-
cpu_request += parse_quantity(
135+
init_container_cpu_request += parse_quantity(
119136
c.get("resources", {}).get("requests", {}).get("cpu", "0")
120137
)
121-
cpu_available -= cpu_request
122-
mem_available -= mem_request
138+
139+
print(
140+
p["metadata"]["name"],
141+
max(init_container_mem_request, container_mem_request),
142+
)
143+
cpu_available -= max(container_cpu_request, init_container_cpu_request)
144+
mem_available -= max(container_mem_request, init_container_mem_request)
123145

124146
return {
125147
# CPU units are in fractions, while memory units are bytes

0 commit comments

Comments
 (0)