feat(balancing): remap target storage on node-local storage clusters#114
feat(balancing): remap target storage on node-local storage clusters#114arturod67 wants to merge 3 commits into
Conversation
ProxLB always migrates keeping the source storage id. On clusters built on
node-local (non-shared) storage, the source storage id often does not exist on
the target node, so the migration is rejected by Proxmox at validation time
("storage 'X' is not available on node 'Y'") before any task is created.
Add optional target storage resolution:
- balancing.target_storage_auto (default False): pick the active/enabled
storage on the target node accepting the guest content type ('images' for
VMs, 'rootdir' for CTs) with the most free space.
- balancing.target_storage_map (optional): pin a storage per target node,
taking precedence over auto selection.
VMs receive 'targetstorage', CTs receive 'target-storage'. Default behaviour is
unchanged (both options off => no remap), preserving correctness on
shared-storage clusters.
Adds unit tests for _resolve_target_storage and documents the options in the
README and example config.
|
While testing I realized that the In ProxLB this creates an infinite loop:
|
There was a problem hiding this comment.
I would like to postpone the commit regarding the capacity guard and fix #115 first. The storage api calls you make for this guard would be part of the fix.
- gather node storage information from
nodes(*).storage.get() - gather guest storage information from
nodes(*).storage(*).content.get()
Also in the long run, I would like to make the guard obsolete and instead let proxlb-solver handle it.
- Replace the copy-pasted author header in the resolve-target-storage tests. - Add the missing type annotations in the test helpers and type the storage fixtures with the proxmoxer stubs' Storage TypedDict (via a 'Storages' alias in the TYPE_CHECKING block, as suggested in review). - Annotate the storage listing call in _resolve_target_storage with the same 'Storages' alias. - Drop the unnecessary int() coercions on the 'active'/'enabled' storage flags and on 'avail'. - Match the requested content type against the comma-separated content tokens instead of a substring check, with a regression test.
Migrations of HA-managed guests are routed through the Proxmox HA stack (hamigrate), which does not forward the 'targetstorage' / 'target-storage' parameter to the migration task it spawns. On node-local storage clusters that task then fails with "storage 'X' is not available on node 'Y'", leaving ProxLB waiting for a qmigrate task that never appears. Detect HA-managed guests via /cluster/ha/resources (fetched lazily, once per balancing pass) and skip their migration with an explanatory warning whenever a target storage remap would be applied. Guests without a remap keep the existing behaviour, and an API error while listing HA resources safely degrades to 'not HA-managed'. Documents the limitation in the README and the example config.
arturod67
force-pushed
the
feat/target-storage-remap
branch
from
04992c7 to
fbaa15a
Compare
|
Done I've dropped the capacity guard from this PR so it can be revisited on top of the #115 fix, where the nodes().storage.get() / storage().content.get() calls will live. The branch now only contains the original feature, the review fixes, and the HA-managed guest skip (the infinite-loop issue you hit while testing: since the HA stack doesn't forward targetstorage, ProxLB now checks /cluster/ha/resources once per pass and skips those guests with an explanatory warning instead). |
3 participants
ProxLB always migrates keeping the source storage id. On clusters built on node-local (non-shared) storage, the source storage id often does not exist on the target node, so the migration is rejected by Proxmox at validation time ("storage 'X' is not available on node 'Y'") before any task is created.
Add optional target storage resolution:
VMs receive 'targetstorage', CTs receive 'target-storage'. Default behaviour is unchanged (both options off => no remap), preserving correctness on shared-storage clusters.
Adds unit tests for _resolve_target_storage and documents the options in the README and example config.