Skip to content

<fix>[capacity]: non-KVM host capacity without KVM role#4324

Open
ZStack-Robot wants to merge 6 commits into
feature-unifi-hostfrom
sync/jin.ma/fix/ZSTAC-85170-it-host-fixes@@2
Open

<fix>[capacity]: non-KVM host capacity without KVM role#4324
ZStack-Robot wants to merge 6 commits into
feature-unifi-hostfrom
sync/jin.ma/fix/ZSTAC-85170-it-host-fixes@@2

Conversation

@ZStack-Robot

Copy link
Copy Markdown
Collaborator

根因: unified-hardware 容量改造让每个 host 的容量更新走 PhysicalServerCapacityVO(resolveServerUuidOrThrow),强制要 KVM_HOST PhysicalServerRoleVO。ESX/非-KVM host 无此 role → 容量更新抛错 → ESX host connect 在 sync-esx-host-capacity 失败(HOST.1003 / ORG_ZSTACK_VMWARE_10049),打挂所有非-KVM host。

修复:

  • resolveServerUuidOrThrow 对非-KVM host 返回 hostUuid(其 PhysicalServerCapacityVO 经 COALESCE(serverUuid, hostUuid) view 按 hostUuid 索引);KVM host 缺 role 仍 fail-loud(NB-24)。
  • ReportHostCapacityMessage handler 对非-unified host inline 上报(recalculate 需要 PhysicalServerVO)。

验证(本地 woven build): VmwareCreateVm/UpdateVm/AttachDetachVolume/StartVmWithoutEnoughCapacity 4 case + VmwareTest 全套 7 case 全绿;PhysicalServerOpsCase(KVM 回归)绿。

跨仓库 @@2 配对: premium fix/ZSTAC-85170-it-host-fixes@@2 (!14423)。Jira 暂填 feature epic ZSTAC-85170。

(重提: 替换错误命名的 !10275,分支统一为 @@2)

sync from gitlab !10284

MaJin1996 added 3 commits June 3, 2026 11:50
unified-hardware feature follow-ups, squashed:

- Backfill PhysicalServerVO serialNumber/manufacturer/model
  from KVM SystemTag via afterAddHost chain-tail hook (PRD
  2.5.1); null-only update preserves path-1 user values.
- Drop unmaintained count fields (cpuSockets/cpuCores/
  memoryModuleCount/diskCount/nicCount/gpuCount) on
  PhysicalServerHardwareInfoVO + SPI carrier + DTO + Flyway.
- Narrow ipmi test bypass to data-plane leaf: static
  overrides return ShellResult so prod PowerStatusParser
  and isAuthFailure run on mocked output.
- Drop redundant try-catch in backfill (rely on
  @ExceptionSafe uniform handling).
- Adapt merge tests to dropped fields.

Resolves: ZSTAC-84191

Change-Id: If38d8ffb786266f3e1c96a99e94715476df364ff
Annotate PhysicalServerAO.oobPassword with
@convert(PasswordConverter.class) so Hibernate encrypts on
write and decrypts on read transparently — same mechanism
KVMHostVO.password uses.

PasswordConverter + EncryptFacade are relocated from core to
header.core.convert so header-resident PhysicalServerAO can
apply the converter directly; EncryptFacade gains
isEncryptionDisabled() so the converter no longer imports
EncryptGlobalConfig. Mechanical FQN updates across existing
import sites (KVMHostVO, CephMonAO, SftpBackupStorageVO,
SpecialDataConverter, encrypt.xml).

No application-layer crypto helper: read/write sites use the
plain getter/setter, on par with the rest of the platform.

IT PhysicalServerOobPasswordEncryptCase mirrors
HostPasswordEncryptCase: asserts the field registers in
EncryptEntityMetadataVO, the toggle flips to Encrypted, and
the converter is a pass-through when encryption is disabled.

Resolves: ZSTAC-85182

Change-Id: I8d9ec342c5e70a5f940227c1c8d70906ab2ac2d2
PhysicalServerApiInterceptor surfaces named errors instead
of bubbling SYS.1000 constraint violations or SYS.1006 with
just a uuid. All via 3-arg argerr so globalErrorCode +
formatted details land in the response payload.

- CreatePhysicalServer: pre-check zone+serialNumber
  uniqueness; names the offending serialNumber and Zone.
- AttachPhysicalServerRole: pre-check (serverUuid,
  roleType); names the conflicting role, suggests detach.
- CreateProvisionNetwork GATEWAY_PXE: reject when DHCP
  wiring (dhcpInterface + dhcpRangeStartIp/EndIp/Netmask)
  is missing; lists the missing fields.

IT PhysicalServerInterceptorErrorsCase exercises all three
error paths and asserts the offending field appears.

Resolves: ZSTAC-85184
Resolves: ZSTAC-85190
Resolves: ZSTAC-85350

Change-Id: I1bf5d9bb386570b8d434cecf923f1bee35422221
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@MatheMatrix, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 52 minutes and 27 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b4b6a284-e6e5-44f0-b06a-66f703d1642e

📥 Commits

Reviewing files that changed from the base of the PR and between 658e302 and 71f8153.

⛔ Files ignored due to path filters (3)
  • conf/springConfigXml/PhysicalServerManager.xml is excluded by !**/*.xml
  • conf/springConfigXml/encrypt.xml is excluded by !**/*.xml
  • test/src/test/resources/springConfigXml/PhysicalServerTestProviders.xml is excluded by !**/*.xml
📒 Files selected for processing (31)
  • compute/src/main/java/org/zstack/compute/allocator/HostAllocatorManagerImpl.java
  • compute/src/main/java/org/zstack/compute/allocator/HostCapacityUpdater.java
  • conf/db/upgrade/V5.5.18__schema.sql
  • core/src/main/java/org/zstack/core/convert/SpecialDataConverter.java
  • core/src/main/java/org/zstack/core/encrypt/EncryptFacadeImpl.java
  • header/src/main/java/org/zstack/header/core/convert/EncryptFacade.java
  • header/src/main/java/org/zstack/header/core/convert/PasswordConverter.java
  • header/src/main/java/org/zstack/header/server/PhysicalServerAO.java
  • header/src/main/java/org/zstack/header/server/PhysicalServerHardwareDiscoveryExtensionPoint.java
  • header/src/main/java/org/zstack/header/server/PhysicalServerHardwareInfoVO.java
  • plugin/ceph/src/main/java/org/zstack/storage/ceph/CephMonAO.java
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMHostVO.java
  • plugin/physicalServer/src/main/java/org/zstack/server/PhysicalServerApiInterceptor.java
  • plugin/physicalServer/src/main/java/org/zstack/server/PhysicalServerKvmIdentityBackfillExtension.java
  • plugin/physicalServer/src/main/java/org/zstack/server/PhysicalServerManagerImpl.java
  • plugin/physicalServer/src/main/java/org/zstack/server/PhysicalServerPowerTracker.java
  • plugin/physicalServer/src/main/java/org/zstack/server/PhysicalServerScanner.java
  • plugin/physicalServer/src/main/java/org/zstack/server/hardware/PhysicalServerHardwareService.java
  • plugin/physicalServer/src/main/java/org/zstack/server/hardware/UnifiedHardwareInfo.java
  • plugin/physicalServer/src/test/java/org/zstack/server/hardware/UnifiedHardwareInfoMergeTest.java
  • plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorageVO.java
  • test/src/test/groovy/org/zstack/test/integration/kvm/KvmTest.groovy
  • test/src/test/groovy/org/zstack/test/integration/kvm/host/HostPasswordEncryptCase.groovy
  • test/src/test/groovy/org/zstack/test/integration/server/PhysicalServerInterceptorErrorsCase.groovy
  • test/src/test/groovy/org/zstack/test/integration/server/PhysicalServerKvmBackfillCase.groovy
  • test/src/test/groovy/org/zstack/test/integration/server/PhysicalServerOobPasswordEncryptCase.groovy
  • test/src/test/groovy/org/zstack/test/integration/server/PhysicalServerOpsCase.groovy
  • test/src/test/java/org/zstack/test/TestGatewayPxeProvisionProvider.java
  • testlib/src/main/java/org/zstack/testlib/ApiHelper.groovy
  • testlib/src/main/java/org/zstack/testlib/EnvSpec.groovy
  • utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Could not fetch remote config from http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml: TimeoutError: The operation was aborted due to timeout
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/jin.ma/fix/ZSTAC-85170-it-host-fixes@@2

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

Unified-hardware capacity rework routed every host through
PhysicalServerCapacityVO via resolveServerUuidOrThrow, which
required a KVM_HOST PhysicalServerRoleVO. ESX/non-KVM hosts have
no such role, so the capacity update threw and ESX host connect
failed at sync-esx-host-capacity.

resolveServerUuidOrThrow now returns hostUuid for non-KVM hosts
(their PhysicalServerCapacityVO is keyed by hostUuid via the
COALESCE(serverUuid, hostUuid) view); a KVM host missing its role
still fails loud. The ReportHostCapacityMessage handler reports
non-unified hosts inline since recalculate needs a PhysicalServerVO.

Resolves: ZSTAC-85170

Change-Id: I1396c45f9b4b412032ed6649b48f9b706d6b03a6
Re-add the model-mount testlib ApiHelper DSL methods
(mountModelToVmInstance / unmountModelFromVmInstance /
queryVmModelMount), lost in an earlier apihelper regen, so
VmModelMountCase resolves the API again.

Resolves: ZSTAC-85170

Change-Id: I47c6725ee4359ee8476144fb0aa48eead2baab6e
@MatheMatrix MatheMatrix force-pushed the sync/jin.ma/fix/ZSTAC-85170-it-host-fixes@@2 branch from 8ff495c to 541a6ae Compare June 24, 2026 11:35
EnvSpec.delete() left PhysicalServerProvisionNetwork cluster/pool refs
in place, so a cluster-attached provision network blocked deletion and
leaked across stability iterations; a later deleteServerPool then failed
with "cluster(s) still attached". Hard-delete the refs before cleanupEO
so provision networks are detached then deleted, in order — no per-case
manual cleanup needed.

Resolves: ZSTAC-85170

Change-Id: Ic7b0d504f468d6396610606dee49a74e08aca9af
@MatheMatrix MatheMatrix force-pushed the sync/jin.ma/fix/ZSTAC-85170-it-host-fixes@@2 branch from 6be71e5 to 71f8153 Compare June 25, 2026 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants