fix(quota): enforce explicit zero limit in ResourceQuota

[iasthc]

What type of PR is this?

/kind bug

What this PR does / why we need it:

QuotaManager used Limit != 0 to decide whether a quota was configured. As a result, setting limits.nvidia.com/gpumem: "0" (or gpucores: "0") was silently treated as "no limit" and every pod was allowed through, instead of blocking GPU usage in the namespace as the docs describe.

This PR adds a LimitSet flag on Quota to distinguish an explicitly-set zero from an auto-initialized entry. FitQuota now gates on LimitSet instead of Limit != 0, so an explicit zero is honored as a hard block.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

• AddQuota sets LimitSet = true; DelQuota clears it.
• Entries auto-created by AddUsage keep LimitSet = false, so behavior when no quota is configured is unchanged.
• Existing NVIDIA-only / nvidia.com/gpu == 1 limitations in the webhook are untouched.
• New tests: TestFitQuotaZeroLimitEnforced, TestAddQuotaExplicitZero. Existing fixtures updated.

Does this PR introduce a user-facing change?:

Honor an explicit zero in limits.nvidia.com/gpumem / limits.nvidia.com/gpucores ResourceQuota as a hard block instead of silently treating it as "no limit".

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iasthc
Once this PR has been reviewed and has the lgtm label, please assign archlitchi for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[hami-robot]

Welcome @iasthc! It looks like this is your first PR to Project-HAMi/HAMi 🎉

[gemini-code-assist]

Code Review

This pull request introduces a LimitSet boolean to the Quota struct to distinguish between an unconfigured limit and an explicit zero limit, ensuring that zero-value quotas are correctly enforced. The logic in FitQuota, AddQuota, and DelQuota has been updated to utilize this flag, and new test cases verify the enforcement of zero limits. Feedback indicates that the GetResourceQuota function needs to be updated to include the LimitSet field when copying objects to prevent data loss for API consumers.

[iasthc]

Hi @DSFans2014, this fixes a bug where limits.nvidia.com/gpumem: "0" in ResourceQuota was silently ignored. Would appreciate your review when you have time, thanks!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
unittests	58.36% <100.00%> (+0.21%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/device/quota.go	75.16% <100.00%> (+12.49%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DSFans2014]

I think we can set the default value of limit to MaxInt64 to avoid introducing a new variable. The information from ResourceQuota will be used in metrics as below.

HAMi/cmd/scheduler/metrics.go

Lines 299 to 315 in f852784

	for ns, val := range sher.GetQuotaManager().GetResourceQuota() {
	for quotaname, q := range *val {
	ch <- prometheus.MustNewConstMetric(
	quotaUsedDesc,
	prometheus.GaugeValue,
	float64(q.Used),
	ns, quotaname, fmt.Sprint(q.Limit),
	)
	if legacy {
	ch <- prometheus.MustNewConstMetric(
	legacyQuotaUsed,
	prometheus.GaugeValue,
	float64(q.Used),
	ns, quotaname, fmt.Sprint(q.Limit),
	)
	}
	}

[iasthc]

Thanks for the suggestion. I think LimitSet is still the safer choice here:

1. FitQuota would overflow. pkg/device/quota.go#L75-L78 multiplies Limit by memoryFactor. With Limit == MaxInt64 and memoryFactor > 1 this overflows to a negative number, and legitimate requests get rejected. Guarding against it requires an if Limit == MaxInt64 check — another sentinel flag in disguise.

2. Metric label. Fair point that the rendering needs work, but limit="9223372036854775807" isn't friendlier than "0". I've pushed a follow-up that uses LimitSet to render "unlimited". This case is real today: AddUsage creates LimitSet=false entries for namespaces with usage but no ResourceQuota, and fix: rollback quota usage when filter annotation patch fails #1898 / fix(quota): roll back usage when Filter evicts a stale pod entry #1905 exercise those paths more often now.

3. Sentinel semantics would spread. AddUsage and DelQuota would both need to use MaxInt64, and every reader has to remember 0 and MaxInt64 carry special meaning. A bool is one byte and explicit.

The metrics commit is on top of the quota fixes, rebased onto current master — PTAL.

[mesutoezdil]

1 more thing worth flagging: the metrics label changes from a numeric string to "unlimited" for entries where no quota is configured (LimitSet=false). before this pr those entries showed limit="0" in prometheus. after this pr they show limit="unlimited". any existing dashboard or alert filtering on limit="0" for the unset case will silently break. this is technically a more correct label, but it is a label value change and it affects cardinality. worth adding a note in the pr description or release notes so operators know to update their dashboards.

[mesutoezdil]

this changes the prometheus label value from "0" to "unlimited" for namespaces that have usage but no configured quota (LimitSet=false). existing dashboards or alerts that filter on limit="0" for the no-quota case will break silently. the new label is semantically correct, but the change should be called out explicitly for operators.