<fix>[compute]: allow no-nic HA fallback across clusters#4210
Open
zstack-robot-1 wants to merge 1 commit into
Open
<fix>[compute]: allow no-nic HA fallback across clusters#4210zstack-robot-1 wants to merge 1 commit into
zstack-robot-1 wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughVmAllocateHostForStoppedVmFlow 改为先按 VM 规格计算并尝试分配到首选集群；若失败且满足无网卡/未指定 requiredHost/场景为 Auto 且配置允许，则重试不带首选集群。新增 allocateHost/handleReply/handleRetryReply 重构回包处理；新增集成测试 HaStartVmWithNoNicCase 覆盖三种 HA 启动场景。 变更VM 主机分配与 HA 启动跨集群回退
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
Comment from yaohua.wu: Review: MR !10169 — ZSTAC-72149Background (preserved across rounds)
P0 — Critical无。 🟡 Warning
🟢 Suggestion
Coverage
Verdict: APPROVED修复正确、完整，落点在正确的模块/层（ 🤖 Robot Reviewer |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@compute/src/main/java/org/zstack/compute/vm/VmAllocateHostForStoppedVmFlow.java`:
- Around line 39-40: Rename the autowired field rcf to the full, non-abbreviated
name resourceConfigFacade in class VmAllocateHostForStoppedVmFlow (type
ResourceConfigFacade) and update every use site in this class (replace rcf
references, e.g., the usage currently referenced later in the file) to
resourceConfigFacade so the code compiles and follows the
no-unnecessary-abbreviations guideline.
🪄 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 UI
Review profile: CHILL
Plan: Pro
Run ID: d6c90dd6-45c0-415b-aeea-5ac16a1dd8da
📒 Files selected for processing (2)
compute/src/main/java/org/zstack/compute/vm/VmAllocateHostForStoppedVmFlow.javatest/src/test/groovy/org/zstack/test/integration/kvm/vm/HaStartVmWithNoNicCase.groovy
Comment on lines
+39
to
+40
| @Autowired | ||
| private ResourceConfigFacade rcf; |
There was a problem hiding this comment.
字段命名使用了不必要的缩写。
字段 rcf 是 ResourceConfigFacade 的缩写。根据编码指南，"不允许使用不必要的缩写"，应使用完整单词提升可读性。建议重命名为 resourceConfigFacade。
🔧 建议的修复
`@Autowired`
-private ResourceConfigFacade rcf;
+private ResourceConfigFacade resourceConfigFacade;同时需要更新行90处的引用：
- && rcf.getResourceConfigValue(VmGlobalConfig.VM_HA_ACROSS_CLUSTERS,
+ && resourceConfigFacade.getResourceConfigValue(VmGlobalConfig.VM_HA_ACROSS_CLUSTERS,🤖 Prompt for 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.
In
`@compute/src/main/java/org/zstack/compute/vm/VmAllocateHostForStoppedVmFlow.java`
around lines 39 - 40, Rename the autowired field rcf to the full,
non-abbreviated name resourceConfigFacade in class
VmAllocateHostForStoppedVmFlow (type ResourceConfigFacade) and update every use
site in this class (replace rcf references, e.g., the usage currently referenced
later in the file) to resourceConfigFacade so the code compiles and follows the
no-unnecessary-abbreviations guideline.
Source: Coding guidelines
Collaborator
|
Comment from ya.wang: 已核对该 review。
mvn -Ppremium -pl test -am test -Dtest=TestCaseStabilityTest -Dcases=org.zstack.test.integration.kvm.vm.HaStartVmWithNoNicCase -Dtimes=2 -Dsurefire.failIfNoSpecifiedTests=false该命令已编译到 因此新增 case 仍无法在当前本机完成自动化执行。三节点真实 ZStack 环境已覆盖核心行为：
两条 suggestion 我先不改：当前两个 helper 的条件虽然有少量重复，但分别表达“首次 cluster 约束”和“失败后是否可去约束重试”，可读性更直接；重试失败返回最终分配错误符合当前 flow 行为，首次错误已可从真实 HA/allocator 日志排查，暂不扩大变更范围。 |
1. Why? No-nic VMs keep no L3 constraint during HA start. The stopped VM allocator still pinned them to their previous cluster, so HA could fail when that cluster had no available host. 2. How? - Prefer the VM's current cluster for no-nic HA allocation first. - Retry without the cluster limit only for auto HA starts. - Gate the retry by the VM HA across clusters resource config. - Add coverage for same-cluster, fallback, and disabled fallback. 3. Side effects? Manual starts and explicit host or cluster starts keep their existing allocation constraints. # Summary of changes (by module): - compute: retry no-nic HA allocation without cluster when allowed - test: cover no-nic HA cluster preference and fallback behavior Resolves: ZSTAC-72149 Change-Id: I4ede874e00f33b9b1fbb386520b3096f2d5c910f
Owner
|
Comment from ya.wang: 两个 suggestion 已处理并推送到当前 MR：
验证： git diff --check
mvn -pl compute -am -DskipTests package均通过。 |
MatheMatrix
force-pushed
the
sync/zstackio/fix/ZSTAC-72149
branch
from
b63bfa0 to
06e1535
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix no-NIC VM HA allocation on 4.8.38. HA first prefers the VM's current cluster, then retries without the cluster restriction only for Auto HA when vm.ha.across.clusters is enabled.
Tests
Jira
ZSTAC-72149
sync from gitlab !10169