<fix>[sdnController]: reinstall znsproxy on 5.5.28#4207
Open
zstack-robot-2 wants to merge 1 commit into
Open
Conversation
Track prepared znsproxy hosts with a Host system tag and reuse the Ansible installer from prepare-service during KVM reconnect. Keep first installation driven by ZNS prepare-service while reconnect repairs only hosts already marked as prepared. JIRA: ZCF-2876 Change-Id: Ie7bdfa8348014228136cc59e739a9f4752510658
|
Warning
|
| 层次 / 文件 | 摘要 |
|---|---|
全局配置与数据结构定义 ZnsProxyGlobalProperty.java, SdnControllerSystemTags.java, ZnsProxyPrepareServiceCmd.java, ZnsProxyDeployArguments.java |
声明7个全局配置项（包路径、端口、Ansible脚本等）；新增ZNS_PROXY_PREPARED系统标签；定义请求命令与部署参数数据契约。 |
ZnsProxyInstaller核心实现 ZnsProxyInstaller.java |
实现完整的安装编排流程：校验参数、解析包文件（支持仓库和类路径两种来源）、配置SSH文件校验、执行Ansible部署、标记主机已准备；包含包名安全校验、端口规范化、shell参数安全引用等工具方法。 |
KVM主机重连自动重装 ZnsProxyKvmReconnectExtension.java |
实现KVM主机连接重建与流程创建扩展点，在重连时检查ZNS_PROXY_PREPARED标记并自动重装；支持新加入主机跳过、失败异常上报。 |
管理器启动与HTTP路由 SdnControllerManagerImpl.java |
启动时有条件地部署Ansible playbook模块（非单元测试模式）；实例化安装器并向REST框架注册ZnsProxyPrepareServiceCmd同步处理器。 |
单元测试全覆盖 ZnsProxyInstallerTest.java |
16个测试用例覆盖命令构建、包解析（按包名/版本/默认）、系统标签格式、命令反序列化、重连行为（prepared/非KVM/流程分支）、异常路径（路径穿越/文件缺失）；含测试辅助类与模拟扩展实现。 |
交互流程
该实现的典型执行流程为：
- 请求接收：HTTP请求携带
ZnsProxyPrepareServiceCmd（管理器UUID、主机列表、代理版本、包名） - 包定位：根据优先级尝试仓库目录→类路径模块查找对应版本的包
- 安装部署：针对每个主机执行SSH验证→Ansible playbook运行→状态等待→标签标记
- 重连自动化：主机连接重建时自动检测并重装已标记的ZnsProxy
代码审查工作量估计
🎯 3 (中等复杂度) | ⏱️ ~25 分钟
兔子的诗
🐰 代理准备装上去，重连时再装一遍，
咔咔咔敲键盘，SSH通，Ansible跑，
主机标签闪闪发光，零信任网络我来护航！✨
🚥 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 | 标题清晰准确地反映了主要变更：在5.5.28版本中实现znsproxy的重新安装功能。 |
| Description check | ✅ Passed | 描述说明了PR来源（从GitLab !10163同步），与变更集相关联且有明确的同步来源指向。 |
| 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. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
sync/dejing.liu/codex/ZCF-3881-znsproxy-5.5.28
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyInstaller.java (2)
106-132: 💤 Low value建议将版本号对应的包名格式提取为常量。
Line 115 中硬编码了包名格式
"zns-proxy-" + cmd.proxyVersion.trim() + ".bin"，建议将此格式字符串提取为类常量（如PROXY_PACKAGE_NAME_PATTERN = "zns-proxy-%s.bin"）并使用String.format构造，以提高代码的可维护性和一致性。🤖 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 `@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyInstaller.java` around lines 106 - 132, Extract the hardcoded package name pattern in ZnsProxyInstaller.resolvePackage into a class-level constant (e.g. PROXY_PACKAGE_NAME_PATTERN = "zns-proxy-%s.bin") and replace the inline concatenation ("zns-proxy-" + cmd.proxyVersion.trim() + ".bin") with String.format(PROXY_PACKAGE_NAME_PATTERN, cmd.proxyVersion.trim()) when building packageName; keep existing fallbacks using PROXY_PACKAGE_NAME and ensure references to packageInRepository and packageInAnsibleModule remain unchanged.Source: Coding guidelines
36-47: 💤 Low valueinstall 方法中的 cmd 空值校验存在冗余。
在 Line 37-39 对
cmd进行了空值校验，但在 Line 42 调用resolvePackage(cmd)时，该方法内部（Line 107-109）又进行了相同的空值校验。虽然防御性编程有其价值，但此处的重复校验属于冗余，建议在resolvePackage方法中移除该校验，或在方法注释中说明调用方需保证 cmd 非空。🤖 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 `@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyInstaller.java` around lines 36 - 47, The null-check for ZnsProxyPrepareServiceCmd is redundant because install(ZnsProxyPrepareServiceCmd cmd) already validates cmd; remove the duplicate null validation from resolvePackage(...) (the method that currently re-checks cmd) and instead add a short Javadoc on resolvePackage stating the caller must pass a non-null ZnsProxyPrepareServiceCmd; keep the existing null check in install and leave normalizeHostUuids/installOnHost/markHostPrepared unchanged.plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyPrepareServiceCmd.java (1)
5-12: 💤 Low value建议添加类与字段的 JavaDoc 注释。
虽然该类是用于 HTTP 同步调用的数据传输对象，建议为类及其公开字段添加 JavaDoc 注释，以提高代码的可读性和可维护性，说明各字段的用途（如
computeManagerUuid是计算管理器 UUID，hostUuids是待安装的主机 UUID 列表等）。🤖 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 `@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyPrepareServiceCmd.java` around lines 5 - 12, 为 ZnsProxyPrepareServiceCmd 类添加 JavaDoc 注释，说明该类作为 HTTP 同步调用的 DTO 作用，并为每个公开成员（COMMAND_PATH 常量、computeManagerUuid、hostUuids、proxyVersion、packageName）添加字段级 JavaDoc，简短描述用途和格式/示例（例如 computeManagerUuid 为计算管理器的 UUID，hostUuids 为待安装主机的 UUID 列表，proxyVersion 为代理版本号，packageName 为安装包名）；在注释中注明请求路径常量 COMMAND_PATH 的用途以便调用方识别。Source: Coding guidelines
plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyDeployArguments.java (1)
18-22: 💤 Low value考虑构造函数直接接收 String 路径参数。
构造函数接收
File sourcePackage参数，但仅调用getAbsolutePath()转换为字符串。若调用方已经持有文件路径字符串，或者仅需要路径而非 File 对象的其他功能，可考虑直接接收 String 参数以简化调用。不过当前设计能确保调用方传入的是有效的 File 对象，具有一定的类型安全性。🤖 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 `@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyDeployArguments.java` around lines 18 - 22, 构造器 ZnsProxyDeployArguments(File sourcePackage, String destinationPackagePath, String healthUrl) 仅使用 sourcePackage.getAbsolutePath()，建议增加一个接收 String 的重载或将参数类型改为 String：新增一个 ZnsProxyDeployArguments(String sourcePackagePath, String destinationPackagePath, String healthUrl) 构造器并直接赋值到字段 sourcePackagePath，同时保留现有 File 版本（或用单一 String 版本替换并更新所有调用点）；确保 destinationPackagePath 和 healthUrl 字段赋值保持不变，并在变更后更新所有调用处以传入路径字符串或使用 File 版构造器以兼容旧代码。
🤖 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.
Nitpick comments:
In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyDeployArguments.java`:
- Around line 18-22: 构造器 ZnsProxyDeployArguments(File sourcePackage, String
destinationPackagePath, String healthUrl) 仅使用
sourcePackage.getAbsolutePath()，建议增加一个接收 String 的重载或将参数类型改为 String：新增一个
ZnsProxyDeployArguments(String sourcePackagePath, String destinationPackagePath,
String healthUrl) 构造器并直接赋值到字段 sourcePackagePath，同时保留现有 File 版本（或用单一 String
版本替换并更新所有调用点）；确保 destinationPackagePath 和 healthUrl
字段赋值保持不变，并在变更后更新所有调用处以传入路径字符串或使用 File 版构造器以兼容旧代码。
In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyInstaller.java`:
- Around line 106-132: Extract the hardcoded package name pattern in
ZnsProxyInstaller.resolvePackage into a class-level constant (e.g.
PROXY_PACKAGE_NAME_PATTERN = "zns-proxy-%s.bin") and replace the inline
concatenation ("zns-proxy-" + cmd.proxyVersion.trim() + ".bin") with
String.format(PROXY_PACKAGE_NAME_PATTERN, cmd.proxyVersion.trim()) when building
packageName; keep existing fallbacks using PROXY_PACKAGE_NAME and ensure
references to packageInRepository and packageInAnsibleModule remain unchanged.
- Around line 36-47: The null-check for ZnsProxyPrepareServiceCmd is redundant
because install(ZnsProxyPrepareServiceCmd cmd) already validates cmd; remove the
duplicate null validation from resolvePackage(...) (the method that currently
re-checks cmd) and instead add a short Javadoc on resolvePackage stating the
caller must pass a non-null ZnsProxyPrepareServiceCmd; keep the existing null
check in install and leave normalizeHostUuids/installOnHost/markHostPrepared
unchanged.
In
`@plugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyPrepareServiceCmd.java`:
- Around line 5-12: 为 ZnsProxyPrepareServiceCmd 类添加 JavaDoc 注释，说明该类作为 HTTP 同步调用的
DTO 作用，并为每个公开成员（COMMAND_PATH
常量、computeManagerUuid、hostUuids、proxyVersion、packageName）添加字段级
JavaDoc，简短描述用途和格式/示例（例如 computeManagerUuid 为计算管理器的 UUID，hostUuids 为待安装主机的 UUID
列表，proxyVersion 为代理版本号，packageName 为安装包名）；在注释中注明请求路径常量 COMMAND_PATH 的用途以便调用方识别。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f2bcce9a-7b72-467b-b48e-72257bc78329
⛔ Files ignored due to path filters (1)
conf/springConfigXml/sdnController.xmlis excluded by!**/*.xml
📒 Files selected for processing (8)
plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerManagerImpl.javaplugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerSystemTags.javaplugin/sdnController/src/main/java/org/zstack/sdnController/ZnsProxyGlobalProperty.javaplugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyDeployArguments.javaplugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyInstaller.javaplugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyKvmReconnectExtension.javaplugin/sdnController/src/main/java/org/zstack/sdnController/znsproxy/ZnsProxyPrepareServiceCmd.javatest/src/test/java/org/zstack/test/unittest/sdnController/ZnsProxyInstallerTest.java
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
1 participant
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.
sync from gitlab !10163