fix: improve deletion reliability in tests, harden GPU quota calculation against overflow, and expand error rate parsing test cases

Author: gitcommitankit

api/v1alpha1/error_rate_test.go

@@ -40,8 +40,10 @@ func TestParseErrorRate(t *testing.T) {
 		{"abc%", 0, true},
 		// trailing garbage — strconv.ParseFloat must reject these
 		{"5x%", 0, true},
-		// non-finite numeric input
+		// non-finite numeric input — caught by math.IsNaN / math.IsInf guard
 		{"NaN%", 0, true},
+		{"Inf%", 0, true},
+		{"-Inf%", 0, true},
 		// leading whitespace — strconv.ParseFloat must reject " 5"
 		{" 5%", 0, true},
 	}

internal/controller/tenantquota_controller_test.go

@@ -53,19 +53,30 @@ var _ = Describe("TenantQuota Controller", func() {
 		adList := &agentraxv1alpha1.AgentDeploymentList{}
 		Expect(k8sClient.List(ctx, adList, inNamespace(tqNS))).To(Succeed())
 		for i := range adList.Items {
-			// Remove finalizer so deletion doesn't block.
 			ad := &adList.Items[i]
+			// Remove finalizer so deletion is not blocked by the controller.
 			ad.Finalizers = nil
-			_ = k8sClient.Update(ctx, ad)
-			_ = k8sClient.Delete(ctx, ad)
+			if err := k8sClient.Update(ctx, ad); err != nil && !apierrors.IsNotFound(err) {
+				Expect(err).NotTo(HaveOccurred(), "removing finalizer from AD %s", ad.Name)
+			}
+			if err := k8sClient.Delete(ctx, ad); err != nil && !apierrors.IsNotFound(err) {
+				Expect(err).NotTo(HaveOccurred(), "deleting AD %s", ad.Name)
+			}
+			// Wait until the API server confirms the object is gone.
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, namespacedName(ad.Name, tqNS), &agentraxv1alpha1.AgentDeployment{})
+				return apierrors.IsNotFound(err)
+			}, timeout, interval).Should(BeTrue(), "AD %s should be fully deleted", ad.Name)
 		}
 
 		By("deleting all TenantQuotas in the test namespace")
 		tqList := &agentraxv1alpha1.TenantQuotaList{}
 		Expect(k8sClient.List(ctx, tqList, inNamespace(tqNS))).To(Succeed())
 		for i := range tqList.Items {
 			tq := &tqList.Items[i]
-			_ = k8sClient.Delete(ctx, tq)
+			if err := k8sClient.Delete(ctx, tq); err != nil && !apierrors.IsNotFound(err) {
+				Expect(err).NotTo(HaveOccurred(), "deleting TQ %s", tq.Name)
+			}
 		}
 	})
 
@@ -213,6 +224,9 @@ var _ = Describe("TenantQuota Controller", func() {
 				// The reconciler calls RemoveStatusCondition, so the condition
 				// must be fully absent once usage normalises.
 				g.Expect(cond).To(BeNil())
+				// Usage counters must reflect the one remaining AD (maxReplicas=2).
+				g.Expect(f.Status.UsedAgents).To(BeNumerically("==", 1))
+				g.Expect(f.Status.UsedTotalReplicas).To(BeNumerically("==", 2))
 			}, timeout, interval).Should(Succeed())
 		})
 

internal/quota/enforcer.go

@@ -129,8 +129,21 @@ func (e *Enforcer) extractGPUs(resources corev1.ResourceRequirements) int64 {
 // gpusForAD returns the total GPU units for one AgentDeployment:
 // gpuPerReplica × spec.replicas.max. The multiplication is performed in
 // int64 to prevent overflow, then clamped to the int32 range.
+// Fails closed (returns MaxInt32) on unexpected negative inputs or int64
+// overflow so the quota check rejects rather than under-counts.
 func (e *Enforcer) gpusForAD(ad agentraxv1alpha1.AgentDeploymentSpec) int32 {
 	perReplica := e.extractGPUs(ad.Resources)
+	// GPU quantities and replica counts must be non-negative. If either is
+	// negative (should never happen given CRD validation), fail closed.
+	if perReplica < 0 || ad.Replicas.Max < 0 {
+		return math.MaxInt32
+	}
+	// Detect int64 multiplication overflow before computing total.
+	// perReplica and Replicas.Max are both non-negative at this point, so
+	// overflow can only occur in the positive direction.
+	if perReplica > 0 && int64(ad.Replicas.Max) > math.MaxInt64/perReplica {
+		return math.MaxInt32
+	}
 	total := perReplica * int64(ad.Replicas.Max)
 	const maxInt32 = int64(math.MaxInt32)
 	if total > maxInt32 {