SPLAT-2238: Redesign logic to handle 4 pools with 3 max vcenters

[vr4manta]

SPLAT-2238

Changes

• Redesign logic to handle 4 pools with 3 max vcenters

Summary by CodeRabbit

• Bug Fixes
  • Improved vCenter pool availability filtering when a lease has a vCenter limit, so candidates are excluded more consistently based on available pool counts.
  • Added a new scenario covering uneven pool distribution to better validate which vCenters stay eligible.
  • Updated logging to show clearer exclusion details while avoiding exposure of full hostnames.

[openshift-ci-robot]

@vr4manta: This pull request references SPLAT-2238 which is a valid jira issue.

Details

In response to this:

SPLAT-2238

Changes

• Redesign logic to handle 4 pools with 3 max vcenters

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vr4manta

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [vr4manta]

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

[coderabbitai]

Warning

Review limit reached

@vr4manta, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 31 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1ec382c3-cacc-4c86-beab-ba400e8dec2f

📥 Commits

Reviewing files that changed from the base of the PR and between c0a6896 and a373b2e.

📒 Files selected for processing (2)

• pkg/controller/leases.go
• pkg/controller/leases_test.go

Walkthrough

The vCenter exclusion algorithm in lease reconciliation is replaced with a hybrid strategy: it determines the minimum number of top-ranked vCenters needed by cumulative pool count to satisfy required pools, keeps those, and excludes remaining vCenters below a computed ceiling threshold. Tests are updated to match, adding a new scenario.

Changes

Hybrid vCenter exclusion algorithm

Layer / File(s)	Summary
Hybrid exclusion computation pkg/controller/leases.go	Replaces the prior combination-based exclusion check with a hybrid strategy computing minVCentersNeeded via cumulative pool counts and a ceiling threshold, keeping top vCenters and excluding under-capacity remaining ones; updates the exclusion log message.
Test coverage and simulated algorithm update pkg/controller/leases_test.go	Adds a new test case (vcentersLimit=3, requiredPools=4) with mixed pool counts, and replaces the simulated exclusion algorithm in the test to mirror the new min-needed/ceiling hybrid approach.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• openshift-eng/vsphere-capacity-manager#66: Both PRs implement/adjust the excludedVCenters filtering logic in pkg/controller/leases.go and corresponding tests.
• openshift-eng/vsphere-capacity-manager#67: Both PRs modify the same vCenter-cap pre-filtering/exclusion logic and TestLeaseVCenterPoolAvailability.

Suggested reviewers: mtulio

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main change and matches the capacity-manager logic update for 4 pools with 3 vCenters.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto	✅ Passed	Touched files only change vCenter selection tests/logic; no crypto imports, weak ciphers, custom crypto, or secret comparisons appear.
Container-Privileges	✅ Passed	This PR only changes pkg/controller Go files; no K8s/container manifests were modified and no privileged settings appear in the patch.
No-Sensitive-Data-In-Logs	✅ Passed	The new exclusion logs are count-only and explicitly avoid printing vCenter FQDNs; no new sensitive data exposure was introduced.
No-Hardcoded-Secrets	✅ Passed	Touched files only change vCenter filtering logic and tests; no API keys, tokens, passwords, private keys, or embedded-credential URLs found.
No-Injection-Vectors	✅ Passed	Changed Go code only reworks vCenter filtering and tests; no SQL/shell/eval/pickle/YAML/dangerous HTML sinks or user-controlled execution found.
Ai-Attribution	✅ Passed	PR commit and description contain no AI-tool attribution; no Assisted-by/Generated-by or Co-Authored-By appears in the PR range.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/controller/leases_test.go`:
- Around line 1677-1683: Update the stale-table expectation in leases_test to
match the new threshold behavior: for the scenario with requiredPools=3,
vcentersLimit=2, and counts 2,2,1, the logic in the vcenter exclusion branch now
marks the 1-pool vCenter as excluded. Adjust the affected test case near the
existing table-driven expectations so it asserts that the single-pool server is
excluded instead of expecting no exclusions, using the same symbols and setup
used by the lease simulator test helpers.

In `@pkg/controller/leases.go`:
- Line 859: The ceiling calculation in the leases controller can overflow when
adding large user-supplied values; update the logic in the lease handling path
that computes ceiling from requiredPools and lease.Spec.VCenters to use the
safer equivalent form `(requiredPools-1)/lease.Spec.VCenters + 1`, and mirror
the same change in the corresponding expectation in leases tests so the
controller behavior and test remain aligned.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c1a6b197-126b-4e34-85af-bc8693609d7f

📥 Commits

Reviewing files that changed from the base of the PR and between e5e2dc6 and c0a6896.

📒 Files selected for processing (2)

• pkg/controller/leases.go
• pkg/controller/leases_test.go

[jcpowermac]

/lgtm