<fix>[console]: change port range for console proxy #2703
Conversation
APIImpact GlobalPropertyImpact Resolves: ZSTAC-77943 Change-Id: I70667073717179646368647a6d646b7979646375
APIImpact Resolves: ZSTAC-77943 Change-Id: I696a776e6d6f68667071656e756d7376667a6471
|
Warning Rate limit exceeded@MatheMatrix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough在更新 ConsoleProxyAgent 的流程中新增端口占用预检（基于 shell/netstat），仅当提供非空端口时执行；若端口被占用则失败。API 层将端口类型由 int 改为可空 Integer，并增加端口取值校验范围。全局属性的端口范围扩大。测试用例加入一次中间更新与断言。 Changes
Sequence Diagram(s)sequenceDiagram
actor Client as Client
participant API as API Server
participant Backend as ManagementServerConsoleProxyBackend
participant Shell as Shell (netstat)
participant Store as VO/Global Properties
participant Agent as Console Proxy Agent
Client->>API: APIUpdateConsoleProxyAgentMsg (consoleProxyPort: Integer?)
API->>Backend: Dispatch update request
alt consoleProxyPort 非空
Backend->>Shell: 检查端口占用 (netstat)
alt 端口被占用
Shell-->>Backend: 占用
Backend-->>API: 失败（端口已被占用）
API-->>Client: 错误响应
else 端口空闲
Shell-->>Backend: 空闲
Backend->>Store: 更新 VO / 全局属性
Backend->>Agent: 下发配置/重载
Agent-->>Backend: 完成
Backend-->>API: 成功
API-->>Client: 成功响应
end
else consoleProxyPort 为空
Backend->>Store: 使用默认/现有端口，更新其他字段
Backend->>Agent: 下发配置/重载
Agent-->>Backend: 完成
Backend-->>API: 成功
API-->>Client: 成功响应
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![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
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
sdk/src/main/java/org/zstack/sdk/UpdateConsoleProxyAgentAction.javais excluded by!sdk/**
📒 Files selected for processing (5)
console/src/main/java/org/zstack/console/ManagementServerConsoleProxyBackend.java(1 hunks)core/src/main/java/org/zstack/core/CoreGlobalProperty.java(1 hunks)header/src/main/java/org/zstack/header/console/APIUpdateConsoleProxyAgentMsg.java(1 hunks)header/src/main/java/org/zstack/header/console/APIUpdateConsoleProxyAgentMsgDoc_zh_cn.groovy(1 hunks)test/src/test/groovy/org/zstack/test/integration/console/ConsoleProxyCase.groovy(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
console/src/main/java/org/zstack/console/ManagementServerConsoleProxyBackend.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.javaheader/src/main/java/org/zstack/header/console/APIUpdateConsoleProxyAgentMsg.javatest/src/test/groovy/org/zstack/test/integration/console/ConsoleProxyCase.groovyheader/src/main/java/org/zstack/header/console/APIUpdateConsoleProxyAgentMsgDoc_zh_cn.groovy
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名：
- API 名称必须唯一，不能重复。
- API 消息类需要继承
APIMessage；其返回类必须继承APIReply或APIEvent，并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest，并满足如下规范：
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时，使用
{variableName}格式。- HTTP 方法对应：
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档，并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名：
- 使用 UpperCamelCase 风格。
- 特殊情况：
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量：
- 使用 lowerCamelCase 风格。
常量命名：
- 全部大写，使用下划线分隔单词。
- 要求表达清楚，避免使用含糊或不准确的名称。
包名：
- 统一使用小写，使用点分隔符，每个部分应是一个具有自然语义的英文单词（参考 Spring 框架的结构）。
命名细节：
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量，防止混淆。
- 命名缩写：
- 不允许使用不必要的缩写，如：
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达：
- 避免使用布尔型参数造成含义不明确。例如：
- 对于
stopAgent(boolean ignoreError)，建议拆分为不同函数（如stopAgentIgnoreError()），或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图，并在名称中体现数据类型或用途（例如在常量与变量名称中，将类型词放在末尾）。
- 避免使用魔法值（Magic Value）：
直接使用未经定义的数值或字符串（如 if (status == 5)）应替换为枚举或常量。
示例：
// 错误示例：魔法值
if (user.getStatus() == 5) { ... }
// 正确示例：常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释：
- 代码应尽量做到自解释，对少于两行的说明可以直接写在代码中。
- 对于较长的注释，需要仔细校对并随代码更新，确保内容正确。
- 接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用：
- 应尽量减少 if...else 结构的使用，建议：
- 限制嵌套层级最多为两层，且内层不应再出现
else分支。- 尽早返回（Early Return），将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断：
- if 条件表达不宜过长或过于复杂，必要时可以将条件抽成 boolean 变量描述。
代码块长度：
...
Files:
console/src/main/java/org/zstack/console/ManagementServerConsoleProxyBackend.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.javaheader/src/main/java/org/zstack/header/console/APIUpdateConsoleProxyAgentMsg.java
| runner.setCommand(String.format("netstat -nltp4 | grep :%d\b", msg.getConsoleProxyPort())); | ||
| runner.setVerbose(false); | ||
| runner.setWithSudo(true); | ||
| runner.setSuppressTraceLog(true); | ||
| ShellResult res = runner.run(); | ||
| String stdout = res.getStdout(); | ||
| if (res.getRetCode() == 0) { | ||
| trigger.fail(argerr("there is other process using the port: %s", stdout)); | ||
| } else { |
There was a problem hiding this comment.
端口占用预检无法生效
这里的 grep :%d\b 有两个明显错误：其一，前置空格导致模式永远匹配不到 netstat 输出中的 0.0.0.0:%d 形式；其二，\b 没有转义，Java 会生成退格控制字符而非单词边界，同样使匹配失效。结果是即便端口已被其他进程占用，检测仍返回未占用，直接放行后续更新，端口冲突就会落到生产环境，属于阻断级问题。请修正匹配模式，确保能精准命中正在监听的端口。
建议修改如下：
- runner.setCommand(String.format("netstat -nltp4 | grep :%d\b", msg.getConsoleProxyPort()));
+ runner.setCommand(String.format("netstat -nltp4 | grep -E ':%d([[:space:]]|$)'", msg.getConsoleProxyPort()));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runner.setCommand(String.format("netstat -nltp4 | grep :%d\b", msg.getConsoleProxyPort())); | |
| runner.setVerbose(false); | |
| runner.setWithSudo(true); | |
| runner.setSuppressTraceLog(true); | |
| ShellResult res = runner.run(); | |
| String stdout = res.getStdout(); | |
| if (res.getRetCode() == 0) { | |
| trigger.fail(argerr("there is other process using the port: %s", stdout)); | |
| } else { | |
| runner.setCommand(String.format( | |
| "netstat -nltp4 | grep -E ':%d([[:space:]]|$)'", | |
| msg.getConsoleProxyPort() | |
| )); | |
| runner.setVerbose(false); | |
| runner.setWithSudo(true); | |
| runner.setSuppressTraceLog(true); | |
| ShellResult res = runner.run(); |
🤖 Prompt for AI Agents
In
console/src/main/java/org/zstack/console/ManagementServerConsoleProxyBackend.java
around lines 498 to 506, the grep pattern " :%d\b" is wrong: the leading space
prevents matching addresses like "0.0.0.0:PORT" and the "\b" is not escaped so
Java turns it into a backspace; change the command to remove the leading space,
use a proper word-boundary or regex flag and properly escape backslashes in the
Java string and quote the pattern for the shell — for example build the command
with String.format("netstat -nltp4 | grep -E ':%d\\\\b'",
msg.getConsoleProxyPort()) or an equivalent that uses grep -E/grep -P and
"\\\\b" (or a numeric-port regex) so the pattern correctly matches listeners on
the target port.
There was a problem hiding this comment.
Comment from shixin.ruan:
“netstat -nltp4 | grep -E ':%d([[:space:]]|$)'”可以起到检查%d代表的port是否被占用吗？
There was a problem hiding this comment.
是的,这个命令可以有效检查端口是否被占用。让我解释一下它的工作原理:
命令分析:
netstat -nltp4 | grep -E ':%d([[:space:]]|$)'-
netstat -nltp4会列出所有 TCP 监听端口,输出格式类似:tcp 0 0 0.0.0.0:7758 0.0.0.0:* LISTEN 12345/python tcp 0 0 127.0.0.1:3306 0.0.0.0:* LISTEN 67890/mysqld -
grep -E ':%d([[:space:]]|$)'使用扩展正则表达式匹配::7758后面跟着空格字符[[:space:]], 或者:7758在行尾$
为什么有效:
在 netstat 的输出中,端口号后面通常跟着空格,然后是远程地址。这个模式能准确匹配到 0.0.0.0:7758 或 127.0.0.1:7758 这样的字符串,而不会误匹配 77580 这样的数字。
检测逻辑:
- 如果
grep返回码是 0 (找到匹配) → 端口已被占用 → 触发失败 - 如果
grep返回码非 0 (未找到匹配) → 端口未被占用 → 继续执行
这个方案比原来的 grep :%d\b 要可靠得多,可以正确检测端口占用情况。
There was a problem hiding this comment.
Comment from shixin.ruan:
监听端口在第三列，表达式不应该匹配以7758结尾的行，请修改匹配表达式
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Resolves: ZSTAC-77943 Change-Id: I636d7669616b777075696c656968706869686d62
MatheMatrix
force-pushed
the
sync/shixin.ruan/shixin-ZSTAC-77943
branch
from
cb5a52f to
e32de8e
Compare
APIImpact
GlobalPropertyImpact
Resolves: ZSTAC-77943
Change-Id: I70667073717179646368647a6d646b7979646375
sync from gitlab !8492