fix(StockControl): match storageType code instead of pool id
The package field exposed by VirtFusion as `primaryStorageProfile` is a storage *type code* (mirrors `server_packages.storage_type` in the VF database), not a profile id. It's meant to filter to any pool whose `storageType` matches — multiple pools across the fleet can carry the same code, which is exactly how multi-hypervisor placement works for mountpoint/datastore storage. `capForStorage()` was checking `pool.id` against this code. Pool ids are unique per hypervisor (e.g. for the same logical mountpoint on three hypervisors, ids 23/28/30) and almost never match the type-code domain (0=local default, 4=mountpoint, etc.). The mismatch silently returned 0 for every hypervisor, zeroing qty fleet-wide whenever the package's type code didn't accidentally collide with some pool id. Symptoms in the wild: every stock-controlled VPS product showed qty=0 in WHMCS even with abundant memory/CPU/IPv4 capacity. Disabling `stockcontrol` on the product or removing `primaryStorageProfile` from the package were the only known workarounds; both lose the actual stock gating this module is meant to provide. Fix: - Match `pool.storageType` instead of `pool.id`. - Walk all pools that match (a hypervisor may have multiple pools of the same type) and use the one that fits the most VMs, instead of short-circuiting on the first match. A disabled pool no longer kills the whole hypervisor's capacity for that type — we just skip it and keep looking for an enabled peer. - Rename the parameter from `$profileId` to `$storageTypeId` so future readers don't fall into the same naming trap. Updated the docblock with a NOTE explaining the VirtFusion-side naming inconsistency. Verified on a 3-hypervisor cluster with `storageType=4` (mountpoint) packages: qty went from 0/0/0/0/0/0/0/0 to 66/32/15/7/3/1/32/15 across the VPS-1 through VPS-32 + storage products without any other config change.
This commit is contained in:
@@ -365,36 +365,56 @@ class StockControl
|
||||
/**
|
||||
* Storage variant of capFor() that respects the package's primaryStorageProfile.
|
||||
*
|
||||
* NOTE on naming: VirtFusion exposes two confusingly-named fields with the
|
||||
* same numeric domain. `package.primaryStorageProfile` (mirrors the DB column
|
||||
* `server_packages.storage_type`) is a **storage type code** — a filter,
|
||||
* not an ID — and matches `otherStorage[].storageType` on each hypervisor.
|
||||
* The pool's own `id` is unique per hypervisor and is never what the package
|
||||
* targets. Treating $storageTypeId as `pool.id` (as this method previously
|
||||
* did) returned 0 for every package whose type code didn't happen to also
|
||||
* exist as a pool id, silently zeroing qty fleet-wide.
|
||||
*
|
||||
* Rules:
|
||||
* - profileId > 0 → must match an otherStorage[].id on the hypervisor; if the
|
||||
* matched pool is disabled or missing, this hypervisor has
|
||||
* zero storage capacity for this product (can't place there).
|
||||
* - profileId <= 0 → fall back to localStorage. If local is disabled, 0.
|
||||
* - storageTypeId > 0 → match any enabled otherStorage[] whose storageType
|
||||
* equals this code. If multiple match (e.g. several
|
||||
* mountpoint pools on one hypervisor), pick the one
|
||||
* that fits the most VMs.
|
||||
* - storageTypeId <= 0 → fall back to localStorage. If local is disabled, 0.
|
||||
*/
|
||||
private static function capForStorage(array $res, int $profileId, int $needGb, float $bufferPct): int
|
||||
private static function capForStorage(array $res, int $storageTypeId, int $needGb, float $bufferPct): int
|
||||
{
|
||||
if ($needGb <= 0) {
|
||||
return PHP_INT_MAX;
|
||||
}
|
||||
|
||||
if ($profileId > 0) {
|
||||
if ($storageTypeId > 0) {
|
||||
$best = 0;
|
||||
$matched = false;
|
||||
foreach ($res['otherStorage'] ?? [] as $pool) {
|
||||
if ((int) ($pool['id'] ?? 0) !== $profileId) {
|
||||
if ((int) ($pool['storageType'] ?? 0) !== $storageTypeId) {
|
||||
continue;
|
||||
}
|
||||
$matched = true;
|
||||
if (empty($pool['enabled'])) {
|
||||
return 0;
|
||||
continue;
|
||||
}
|
||||
|
||||
return self::capFor(
|
||||
$cap = self::capFor(
|
||||
['max' => (int) ($pool['max'] ?? 0), 'free' => (int) ($pool['free'] ?? 0)],
|
||||
$needGb,
|
||||
$bufferPct,
|
||||
);
|
||||
if ($cap > $best) {
|
||||
$best = $cap;
|
||||
}
|
||||
}
|
||||
|
||||
// Storage profile not present on this hypervisor — cannot place the VM.
|
||||
return 0;
|
||||
if (! $matched) {
|
||||
// No pool of this storage type on this hypervisor — cannot place the VM.
|
||||
return 0;
|
||||
}
|
||||
|
||||
return $best;
|
||||
}
|
||||
|
||||
$local = $res['localStorage'] ?? null;
|
||||
|
||||
Reference in New Issue
Block a user