Skip to content

Commit 5ef8d02

Browse files
coderabbitai[bot]CodeRabbit
andauthored
fix: apply CodeRabbit auto-fixes
Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 parent 31a8c15 commit 5ef8d02

4 files changed

Lines changed: 43 additions & 17 deletions

File tree

config/prometheus-adapter/custom-metrics-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
# kubectl rollout restart deployment/prometheus-adapter -n monitoring
1010
#
1111
# Verify metrics are registered:
12-
# kubectl get --raw /apis/custom.metrics.k8s.io/v1beta1 | jq .
12+
# kubectl get --raw /apis/external.metrics.k8s.io/v1beta1 | jq .
1313
#
1414
apiVersion: v1
1515
kind: ConfigMap

config/prometheus-adapter/kustomization.yaml

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,28 @@ namespace: monitoring
1414
resources:
1515
- custom-metrics-config.yaml
1616

17-
# Patch removed: The prometheus-adapter Deployment is not included in this
18-
# kustomization's resources, so the patch has no valid target. Users should
19-
# apply this ConfigMap to their cluster and manually configure their existing
20-
# prometheus-adapter Deployment to mount it, or include the prometheus-adapter
21-
# base resources in this kustomization before re-adding the patch.
17+
# To apply this configuration, either:
18+
# 1. Include the prometheus-adapter Deployment base in resources above, then
19+
# uncomment the patches section below, OR
20+
# 2. Manually configure your existing prometheus-adapter Deployment to mount
21+
# this ConfigMap at /etc/adapter/config.yaml and pass --config=/etc/adapter/config.yaml
22+
#
23+
# patches:
24+
# - target:
25+
# kind: Deployment
26+
# name: prometheus-adapter
27+
# patch: |-
28+
# - op: add
29+
# path: /spec/template/spec/volumes/-
30+
# value:
31+
# name: adapter-config
32+
# configMap:
33+
# name: agentrax-custom-metrics
34+
# - op: add
35+
# path: /spec/template/spec/containers/0/volumeMounts/-
36+
# value:
37+
# name: adapter-config
38+
# mountPath: /etc/adapter
39+
# - op: add
40+
# path: /spec/template/spec/containers/0/args/-
41+
# value: --config=/etc/adapter/config.yaml

internal/controller/agentdeployment_controller_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package controller
1818

1919
import (
2020
"context"
21-
"fmt"
2221
"time"
2322

2423
. "github.com/onsi/ginkgo/v2"
@@ -142,12 +141,10 @@ func deleteChildResources(key types.NamespacedName) {
142141
hpa := &autoscalingv2.HorizontalPodAutoscaler{}
143142
err := k8sClient.Get(ctx, key, hpa)
144143
if err != nil && !apierrors.IsNotFound(err) {
145-
panic(fmt.Sprintf("unexpected error reading HPA during cleanup: %v", err))
144+
Expect(err).NotTo(HaveOccurred(), "unexpected error reading HPA during cleanup")
146145
}
147146
if err == nil {
148-
if err := k8sClient.Delete(ctx, hpa); err != nil {
149-
panic(fmt.Sprintf("failed to delete HPA during cleanup: %v", err))
150-
}
147+
Expect(k8sClient.Delete(ctx, hpa)).To(Succeed(), "HPA cleanup delete should succeed")
151148
Eventually(func() bool {
152149
return apierrors.IsNotFound(k8sClient.Get(ctx, key, &autoscalingv2.HorizontalPodAutoscaler{}))
153150
}, testTimeout, testInterval).Should(BeTrue(), "child HPA should be deleted")

internal/quota/enforcer.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,28 +254,37 @@ func evalQuotaRules(
254254
isUpdate bool,
255255
prevMaxReplicas int32, // 0 for creates; used by per-agent ceiling check
256256
) (bool, string) {
257-
projAgents := committedUsage.UsedAgents + inFlight.agents + delta.agents
258-
projGPUs := committedUsage.UsedGPUs + inFlight.gpus + delta.gpus
259-
projReplicas := committedUsage.UsedTotalReplicas + inFlight.replicas + delta.replicas
257+
// Reject the math.MaxInt32 sentinel used by gpusForAD to signal overflow.
258+
// If delta.gpus is MaxInt32, the GPU calculation overflowed and we must
259+
// fail closed to prevent undercount-based admission.
260+
if delta.gpus == math.MaxInt32 {
261+
return false, "GPU resource calculation overflowed; request denied"
262+
}
263+
264+
// Perform projection calculations in int64 to prevent overflow, then
265+
// compare against int64-converted quota limits.
266+
projAgents := int64(committedUsage.UsedAgents) + int64(inFlight.agents) + int64(delta.agents)
267+
projGPUs := int64(committedUsage.UsedGPUs) + int64(inFlight.gpus) + int64(delta.gpus)
268+
projReplicas := int64(committedUsage.UsedTotalReplicas) + int64(inFlight.replicas) + int64(delta.replicas)
260269

261270
// For UPDATE requests, only reject when the delta increases a dimension that
262271
// is already at or over quota. If quota was lowered below current usage, the
263272
// existing ADs are already OverQuota (indicated by the TQ condition) — we
264273
// must not block updates that don't make things worse, otherwise finalizer
265274
// removal and spec corrections are deadlocked.
266-
if projAgents > quota.MaxAgents && (!isUpdate || delta.agents > 0) {
275+
if projAgents > int64(quota.MaxAgents) && (!isUpdate || delta.agents > 0) {
267276
return false, fmt.Sprintf(
268277
"would exceed maxAgents (%d): current=%d in-flight=%d delta=%d",
269278
quota.MaxAgents, committedUsage.UsedAgents, inFlight.agents, delta.agents,
270279
)
271280
}
272-
if projGPUs > quota.MaxGPUs && (!isUpdate || delta.gpus > 0) {
281+
if projGPUs > int64(quota.MaxGPUs) && (!isUpdate || delta.gpus > 0) {
273282
return false, fmt.Sprintf(
274283
"would exceed maxGPUs (%d): current=%d in-flight=%d delta=%d",
275284
quota.MaxGPUs, committedUsage.UsedGPUs, inFlight.gpus, delta.gpus,
276285
)
277286
}
278-
if projReplicas > quota.MaxTotalReplicas && (!isUpdate || delta.replicas > 0) {
287+
if projReplicas > int64(quota.MaxTotalReplicas) && (!isUpdate || delta.replicas > 0) {
279288
return false, fmt.Sprintf(
280289
"would exceed maxTotalReplicas (%d): current=%d in-flight=%d delta=%d",
281290
quota.MaxTotalReplicas, committedUsage.UsedTotalReplicas, inFlight.replicas, delta.replicas,

0 commit comments

Comments
 (0)