fix(management-network): support target-aware endpoints [ZSTAC-86567]#4543
fix(management-network): support target-aware endpoints [ZSTAC-86567]#4543MatheMatrix wants to merge 1 commit into
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 5 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 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 configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
Warning
|
| Layer / File(s) | Summary |
|---|---|
严格地址选择与错误契约 core/src/main/java/org/zstack/core/ManagementServerIpSelection.java, core/src/main/java/org/zstack/core/Platform.java, utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java |
新增不可变选择结果对象、严格地址族与路由源校验，以及失败原因到错误码的映射。 |
REST 地址族与防火墙支持 header/src/main/java/org/zstack/header/rest/RESTFacade.java, core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java, utils/src/main/java/org/zstack/utils/IptablesUtils.java |
新增目标感知的发送命令 URL，并支持 IPv4/IPv6 REST 防火墙规则。 |
Ansible 与 Salt 地址传递 core/src/main/java/org/zstack/core/ansible/*, core/src/main/java/org/zstack/core/salt/*, conf/salt/* |
部署流程根据目标地址族选择管理地址，并将其传递给检查器、部署参数及 Salt 配置模板。 |
KVM 主机连接与重连 plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java |
KVM 连接、回调校验、Ansible 部署及重连配置更新复用严格管理地址选择结果。 |
双栈行为与回归验证 test/src/test/groovy/org/zstack/test/integration/**/*, test/src/test/groovy/org/zstack/core/ansible/* |
新增严格选择、REST、Salt、Ansible、yum 仓库及 KVM 双栈场景测试。 |
Estimated code review effort: 4 (Complex) | ~60 minutes
Sequence Diagram(s)
sequenceDiagram
participant KVMHost
participant Platform
participant RESTFacade
participant AnsibleRunner
KVMHost->>Platform: resolveManagementIpForRemote(targetIp)
Platform-->>KVMHost: 管理服务器 IP 选择结果
KVMHost->>RESTFacade: buildSendCommandUrlForManagementHost(selectedIp)
RESTFacade-->>KVMHost: send command URL
KVMHost->>AnsibleRunner: setManagementServerIpSelection(selection)
AnsibleRunner-->>KVMHost: 执行部署
Poem
我是小兔，蹦过双栈网，
IPv4、IPv6 都点亮。
路由寻源选好方向，
Salt 与 Ansible 携手忙。
KVM 回调不迷航，
新配置在月光下闪亮。
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | 标题简洁且准确概括了按目标地址族选择端点这一主要变更。 |
| Description check | ✅ Passed | 描述与本次变更一致，涵盖了端点选择、Salt 双栈配置和测试覆盖。 |
| 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. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
sync/shixin.ruan/shixin-ZSTAC-86567-risk-sweep@@3
Comment @coderabbitai help to get the list of available commands.
![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
🧹 Nitpick comments (5)
core/src/main/java/org/zstack/core/ansible/ManagementServerIpAwareChecker.java (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win缺少接口方法 Javadoc 注释。
setManagementServerIp方法未提供 Javadoc 说明其用途、参数含义（如该 IP 的地址族约束、何时被调用等）。根据AnsibleRunner.run的调用方式，该方法用于在 Ansible 部署前注入按目标地址族选定的管理服务器地址，建议补充说明。As per path instructions for
**/*.java: "接口方法不应有多余的修饰符（例如public），且必须配有有效的 Javadoc 注释。"📝 建议补充 Javadoc
package org.zstack.core.ansible; +/** + * Checkers implementing this interface will receive the selected management + * server IP (address-family-aware) before Ansible deployment runs. + */ public interface ManagementServerIpAwareChecker { + /** + * `@param` managementServerIp the management server IP selected for the target, + * matching the target's address family + */ void setManagementServerIp(String managementServerIp); }🤖 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 `@core/src/main/java/org/zstack/core/ansible/ManagementServerIpAwareChecker.java` around lines 3 - 5, 为接口方法 ManagementServerIpAwareChecker.setManagementServerIp 补充有效的 Javadoc，说明其在 Ansible 部署前注入管理服务器地址、地址族选择约束及调用时机，并使用 `@param` 解释 managementServerIp；保持接口方法不添加多余的 public 等修饰符。Source: Path instructions
header/src/main/java/org/zstack/header/rest/RESTFacade.java (1)
92-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win两个新接口方法语义相近，建议补充 Javadoc 区分职责。
buildSendCommandUrl(String remoteHost)与buildSendCommandUrlForManagementHost(String managementHost)命名相似但语义不同：根据KVMHost中的调用（restf.buildSendCommandUrl(self.getManagementIp())vsrestf.buildSendCommandUrlForManagementHost(selection.getSelectedIp())），前者接收远端主机地址（内部会解析对应的管理地址），后者直接接收已经过严格选择的管理服务器地址。这种细微差异容易被误用，建议补充 Javadoc 说明各自的参数含义与适用场景（类似本文件中buildCallbackUrl的 Javadoc 风格）。As per path instructions for
**/*.java: "接口方法...必须配有有效的 Javadoc 注释。"📝 建议补充 Javadoc
+ /** + * Builds the REST command URL for a remote host, resolving the + * management-server address by the remote host's address family. + * + * `@param` remoteHost the remote target host address + */ String buildSendCommandUrl(String remoteHost); + /** + * Builds the REST command URL using an already-selected management + * server address (see {`@code` Platform#resolveManagementServerIpForRemoteStrict}). + * + * `@param` managementHost the strictly-selected management server IP + */ String buildSendCommandUrlForManagementHost(String managementHost);🤖 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 `@header/src/main/java/org/zstack/header/rest/RESTFacade.java` around lines 92 - 95, 为 RESTFacade 中的 buildSendCommandUrl 与 buildSendCommandUrlForManagementHost 补充有效 Javadoc，明确前者接收远端主机地址并解析对应管理地址，后者接收已严格选择的管理服务器地址；同时说明各自适用场景和参数含义，参照 buildCallbackUrl 的注释风格。Source: Path instructions
test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy (1)
282-296: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议在第二次调用前清空
ipv6Rules。
ipv4Rules.clear()之后没有对应地ipv6Rules.clear()，当前能通过仅因为上一步已断言ipv6Rules.isEmpty()。为使测试独立于执行顺序、更健壮，建议同时清空两个列表。♻️ 建议修改
ipv4Rules.clear() + ipv6Rules.clear() RESTFacadeImpl.installRestPortFirewallRules(REST_PORT, true, { ipv4Rules.add(it) }, { ipv6Rules.add(it) })🤖 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 `@test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy` around lines 282 - 296, 在 testRestPortFirewallRulesFollowManagementAddressFamilies 的第二次 installRestPortFirewallRules 调用前，同时清空 ipv4Rules 和 ipv6Rules；保留现有断言，以确保第二次调用只验证本次生成的规则。test/src/test/groovy/org/zstack/test/integration/core/salt/SaltDualStackConfigCase.groovy (1)
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win测试名称与实际验证内容不符。
testMinionSelectsReachableSaltMasterFamily的命名暗示验证的是"根据可达性选择 Salt Master 地址族"，但方法体仅直接把硬编码的 IP 字符串传给SaltSetupMinionJob.renderMinionConfig，并未消费withManagementServerIpProperties设置的系统属性，也没有调用任何"选择"逻辑（如基于路由/可达性挑选地址）。这实际只覆盖了模板渲染，建议重命名为更准确的名称（如testMinionConfigRendersIpVersionFlag），或者补充对实际选择函数的调用来匹配测试名。🤖 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 `@test/src/test/groovy/org/zstack/test/integration/core/salt/SaltDualStackConfigCase.groovy` around lines 44 - 56, 测试名称与验证内容不一致：将 testMinionSelectsReachableSaltMasterFamily 重命名为准确描述模板渲染和 IP 版本标志的名称（如 testMinionConfigRendersIpVersionFlag）；或者改为调用实际的 Salt Master 地址族选择逻辑并断言其基于可达性的选择结果。test/src/test/groovy/org/zstack/test/integration/core/ansible/AnsibleRunnerTargetAwareArgumentsCase.groovy (1)
20-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
IPV6_TARGET与IPV6取值相同，弱化了地址族选择测试的意义。
IPV4_TARGET（127.0.0.2）特意与主地址IPV4（127.0.0.1）区分，但IPV6_TARGET却与IPV6完全相同（均为 "::1"）。这使得 IPv6 场景下"目标地址"与"被选中的管理地址"恰好相等，无法有效验证选择逻辑是否真正基于地址族匹配而非偶然相等。既然测试处于UNIT_TEST_ON = true模式，不依赖真实网络可达性，建议将IPV6_TARGET改为与IPV6不同的值（如"::2"），与 IPv4 场景保持一致的测试严谨度。♻️ 建议修改
- private static final String IPV6_TARGET = "::1" + private static final String IPV6_TARGET = "::2"🤖 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 `@test/src/test/groovy/org/zstack/test/integration/core/ansible/AnsibleRunnerTargetAwareArgumentsCase.groovy` around lines 20 - 23, 将 AnsibleRunnerTargetAwareArgumentsCase 中的 IPV6_TARGET 修改为与 IPV6 不同的有效 IPv6 地址（例如 "::2"），保持与 IPV4/IPV4_TARGET 的测试设计一致，确保目标地址选择逻辑不会因地址值恰好相同而通过。
🤖 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 `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 5360-5367: 在 KVMHost 的 pingHook 路径中，避免每个周期直接调用
Platform.resolveManagementServerIpForRemoteStrict；为路由解析增加缓存与失败退避，并区分瞬时解析失败和确实不存在同族管理
IP。仅在确认无同族管理 IP 时继续通过 needUpdateHostConfiguration 和 doUpdateHostConfiguration
触发重连，瞬时失败应复用缓存或跳过本次更新。
---
Nitpick comments:
In
`@core/src/main/java/org/zstack/core/ansible/ManagementServerIpAwareChecker.java`:
- Around line 3-5: 为接口方法 ManagementServerIpAwareChecker.setManagementServerIp
补充有效的 Javadoc，说明其在 Ansible 部署前注入管理服务器地址、地址族选择约束及调用时机，并使用 `@param` 解释
managementServerIp；保持接口方法不添加多余的 public 等修饰符。
In `@header/src/main/java/org/zstack/header/rest/RESTFacade.java`:
- Around line 92-95: 为 RESTFacade 中的 buildSendCommandUrl 与
buildSendCommandUrlForManagementHost 补充有效
Javadoc，明确前者接收远端主机地址并解析对应管理地址，后者接收已严格选择的管理服务器地址；同时说明各自适用场景和参数含义，参照
buildCallbackUrl 的注释风格。
In
`@test/src/test/groovy/org/zstack/test/integration/core/ansible/AnsibleRunnerTargetAwareArgumentsCase.groovy`:
- Around line 20-23: 将 AnsibleRunnerTargetAwareArgumentsCase 中的 IPV6_TARGET 修改为与
IPV6 不同的有效 IPv6 地址（例如 "::2"），保持与 IPV4/IPV4_TARGET
的测试设计一致，确保目标地址选择逻辑不会因地址值恰好相同而通过。
In
`@test/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovy`:
- Around line 282-296: 在
testRestPortFirewallRulesFollowManagementAddressFamilies 的第二次
installRestPortFirewallRules 调用前，同时清空 ipv4Rules 和
ipv6Rules；保留现有断言，以确保第二次调用只验证本次生成的规则。
In
`@test/src/test/groovy/org/zstack/test/integration/core/salt/SaltDualStackConfigCase.groovy`:
- Around line 44-56: 测试名称与验证内容不一致：将 testMinionSelectsReachableSaltMasterFamily
重命名为准确描述模板渲染和 IP 版本标志的名称（如 testMinionConfigRendersIpVersionFlag）；或者改为调用实际的 Salt
Master 地址族选择逻辑并断言其基于可达性的选择结果。
🪄 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: f5973bf4-8cf4-4670-97ba-bb40bff28836
📒 Files selected for processing (20)
conf/salt/masterconf/salt/minioncore/src/main/java/org/zstack/core/ManagementServerIpSelection.javacore/src/main/java/org/zstack/core/Platform.javacore/src/main/java/org/zstack/core/ansible/AnsibleRunner.javacore/src/main/java/org/zstack/core/ansible/CallBackNetworkChecker.javacore/src/main/java/org/zstack/core/ansible/ManagementServerIpAwareChecker.javacore/src/main/java/org/zstack/core/ansible/SshYumRepoChecker.javacore/src/main/java/org/zstack/core/rest/RESTFacadeImpl.javacore/src/main/java/org/zstack/core/salt/SaltFacadeImpl.javacore/src/main/java/org/zstack/core/salt/SaltSetupMinionJob.javaheader/src/main/java/org/zstack/header/rest/RESTFacade.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javatest/src/test/groovy/org/zstack/core/ansible/SshYumRepoCheckerCase.groovytest/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/core/ansible/AnsibleRunnerTargetAwareArgumentsCase.groovytest/src/test/groovy/org/zstack/test/integration/core/salt/SaltDualStackConfigCase.groovytest/src/test/groovy/org/zstack/test/integration/kvm/host/DualStackAddKvmHostCase.groovyutils/src/main/java/org/zstack/utils/IptablesUtils.javautils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
MatheMatrix
force-pushed
the
sync/shixin.ruan/shixin-ZSTAC-86567-risk-sweep@@3
branch
2 times, most recently
from
1bf2ad3 to
d34bfdc
Compare
Select the exact configured management IP chosen by the kernel route for each literal agent target. Return structured errors for missing family, route lookup failure, and unconfigured route source. Keep hostname compatibility and avoid host flapping on transient ping route lookup failures. Resolves: ZSTAC-86567 Change-Id: I9d3f0ea84d6a4eeb99c5a055c4fbd9003eefbe18
MatheMatrix
force-pushed
the
sync/shixin.ruan/shixin-ZSTAC-86567-risk-sweep@@3
branch
from
d34bfdc to
ad7511b
Compare
1 participant
Scope
Verification
./runMavenProfile premium: 143 modules passed.Dependency
This branch contains Batch A commit 276954a from MR !10504. The diff will shrink after !10504 merges.
Jira: ZSTAC-86567
sync from gitlab !10506