SUG-2795 integrate T2/T6 LB IPVS cloud changes#4346
Conversation
Persist listener dataPlane and forwardMode for TCP IPVS full_nat. Expose the fields through inventory, SDK, CLI binding, and lb.xml. Keep generated SDK forwardMode valid values aligned with P0. Validate TCP IPVS creation and cover schema upgrade behavior. Test: ./runMavenProfile sdk. Resolves: ZSTAC-86152 Change-Id: I85298aceccc2b3bba6260b093864bf5158935244
Allow the SDK to submit all declared TCP IPVS forward modes. This lets the API interceptor return the version validation error. Unsupported modes are still rejected by the backend interceptor. Keep the generated SDK and API Groovy template aligned. Related: ZSTAC-86152 Change-Id: I1d72e07c4343d1f6420033ab130543981d8be43b
…5.5.28-lb-ipvs' <feature>[lb]: support tcp ipvs listener See merge request zstackio/zstack!10287
Add TCP IPVS listener health check API validation. Update listener tags for timeout changes and SDK actions. Test: lb and vr provider package builds; sdk; docpremium. Resolves: ZSTAC-86152 Change-Id: I83f933cc780329ba8fb63ad6f3f3958693812de1
…-ipvs' <fix>[lb]: validate tcp ipvs health check See merge request zstackio/zstack!10297
Complete the SUG-2795 dedicated TCP IPVS T2 path. Allow separate virtual router listeners to use server IP backends, preserve server group metadata in refresh payloads, and fix listener removal cleanup semantics. Test: TcpIpvsLoadBalancerListenerApiCase and live 172.24.241.166 Resolves: ZSTAC-2795 Change-Id: I328135e3a1fb4dcfd7eb647a02151743c7157098
|
Warning Review limit reached
More reviews will be available in 57 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 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 configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Warning
|
| Layer / File(s) | Summary |
|---|---|
常量、VO 与 Inventory 数据契约 plugin/loadBalancer/.../LoadBalancerConstants.java, LoadBalancerListenerVO.java, LoadBalancerListenerVO_.java, LoadBalancerListenerInventory.java, conf/db/upgrade/V5.5.28__schema.sql |
新增 DATA_PLANE_*/FORWARD_MODE_*/HEALTH_CHECK_TIMEOUT_* 常量;LoadBalancerListenerVO 新增 data_plane/forward_mode JPA 列;Inventory 的 valueOf 仅在 TCP+IPVS 时填充两字段;SQL 升级脚本补齐存量数据默认值。 |
API 消息与 SDK 参数扩展 APICreateLoadBalancerListenerMsg.java, APIChangeLoadBalancerListenerMsg.java, APICreate...MsgDoc_zh_cn.groovy, APIChange...MsgDoc_zh_cn.groovy, sdk/.../CreateLoadBalancerListenerAction.java, sdk/.../ChangeLoadBalancerListenerAction.java, sdk/.../LoadBalancerListenerInventory.java |
Create 消息新增 dataPlane/forwardMode 入参,Change 消息新增 healthCheckTimeout 入参;SDK Action 类与中文接口文档同步更新。 |
ApiInterceptor 校验逻辑重构 LoadBalancerApiInterceptor.java, LoadBalancerManagerImpl.java |
创建时补全 dataPlane 默认值(haproxy),IPVS 强制 TCP+full_nat;变更时前置 healthCheckTarget 归一化;两处均引入 dataPlane 维度的健康检查协议兼容性判断;共享 LB 判断替换为考虑 SEPARATE_VR 的 helper。 |
LoadBalancerBase 持久化与回滚 LoadBalancerBase.java |
createListener 写入 dataPlane/forwardMode;Change 消息处理新增 HEALTH_TIMEOUT 系统标签更新与失败回滚;addVmNicToListener 引入 finalProviderType 兜底。 |
LoadBalancerFactory 默认 ProviderType LoadBalancerFactory.java, SharedLoadBalancerFactory.java |
接口新增默认方法 getDefaultProviderType(),SharedLoadBalancerFactory 覆盖返回 VyosConstants.VYOS_ROUTER_PROVIDER_TYPE。 |
VirtualRouter 后端重构 VirtualRouterLoadBalancerBackend.java |
新增 findDedicatedLoadBalancerVirtualRouter、findBackendL3Network、getBackendL3NetworkUuids、hasBackendServers;移除 removeVmNics/removeListener 中 SEPARATE_VR 早退限制;detach 条件改为 hasBackendServers;LbTO 新增 forwardMode 并在 makeLbTOs 中填充。 |
集成测试与 Schema 升级测试 TcpIpvsLoadBalancerListenerApiCase.groovy, CheckSchemaUpgradeCase.groovy, TestVirtualRouterLb13.java |
新增 IPVS 监听器全链路集成测试;校验 V5.5.28 SQL 升级内容;TestVirtualRouterLb13 新增 healthCheckTimeout 变更与 tag 断言。 |
时序图
sequenceDiagram
participant Client
participant LoadBalancerApiInterceptor
participant LoadBalancerBase
participant LoadBalancerListenerVO
participant VirtualRouterLoadBalancerBackend
Client->>LoadBalancerApiInterceptor: APICreateLoadBalancerListenerMsg(dataPlane, forwardMode)
LoadBalancerApiInterceptor->>LoadBalancerApiInterceptor: 补全 dataPlane 默认值(haproxy)<br>校验 IPVS 仅支持 TCP+full_nat<br>校验健康检查协议兼容性
LoadBalancerApiInterceptor-->>LoadBalancerBase: 通过校验
LoadBalancerBase->>LoadBalancerListenerVO: 写入 dataPlane / forwardMode
LoadBalancerBase->>VirtualRouterLoadBalancerBackend: refresh LB
VirtualRouterLoadBalancerBackend->>VirtualRouterLoadBalancerBackend: findDedicatedLoadBalancerVirtualRouter<br>getBackendL3NetworkUuids
VirtualRouterLoadBalancerBackend->>VirtualRouterLoadBalancerBackend: makeLbTOs(dataPlane, forwardMode 写入 LbTO)
VirtualRouterLoadBalancerBackend-->>Client: RefreshLbCmd 下发
评估代码审查工作量
🎯 5 (Critical) | ⏱️ ~120 分钟
可能相关的 PR
- MatheMatrix/zstack#4336: 同样涉及
healthCheckTimeout在APIChangeLoadBalancerListenerMsg、LoadBalancerApiInterceptor校验以及LoadBalancerBase中HEALTH_TIMEOUT系统标签的读写与回滚,代码层面直接重叠。
小诗
🐇 小兔跑进机房里,
IPVS 平面新开启,
full_nat 路由定乾坤,
超时检查不忘记,
专用路由找得到,
后端 L3 推得齐!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 2.08% 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 | 标题准确概括了本次 TCP/IPVS 负载均衡云端集成变更。 |
| Description check | ✅ Passed | 描述与变更内容一致,说明了集成范围、验证方式和部署背景。 |
| 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/zstackio/codex/sug-2795-integration-5.5.28
Comment @coderabbitai help to get the list of available commands.
5069d12 to
0558606
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerFactory.java (1)
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win为新增接口方法补充 Javadoc。
这是新的扩展点,默认返回
null的语义需要写清楚,方便实现方判断是否必须覆盖。建议修改
+ /** + * Returns the fallback provider type used when no backend NIC is available. + * + * `@return` the default provider type, or {`@code` null} if this factory has no fallback provider + */ default String getDefaultProviderType() { return null; }As per path instructions, 接口方法不应有多余的修饰符(例如 public),且必须配有有效的 Javadoc 注释。
🤖 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/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerFactory.java` around lines 21 - 23, The new extension method getDefaultProviderType in LoadBalancerFactory needs a proper Javadoc explaining that the default null return means no default provider is specified and implementers should override it when required. Add the Javadoc directly on this interface method, keep the method declaration as a plain interface method without extra modifiers, and make the null semantics explicit so implementers can decide whether overriding is mandatory.Source: Path instructions
🤖 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/loadBalancer/src/main/java/org/zstack/network/service/lb/APICreateLoadBalancerListenerMsgDoc_zh_cn.groovy`:
- Around line 228-247: 补全 APICreateLoadBalancerListenerMsgDoc_zh_cn.groovy
中参数定义的说明,当前 dataPlane 和 forwardMode 的 desc 为空会导致文档无法表达用途。请在生成文档的 column 配置里为
dataPlane、forwardMode 填写清晰的中文描述,并在 APICreateLoadBalancerListenerMsgDoc_zh_cn
相关参数说明中明确 dataPlane 表示的数据平面含义、forwardMode 表示的转发模式及其适用场景,确保与现有 values 枚举一致。
In
`@plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java`:
- Around line 233-249: After finding a dedicated VR via
findDedicatedLoadBalancerVirtualRouter(), return that VR immediately instead of
only validating it and falling through, because vrs may still be empty and later
addVmNics() can mis-handle it as missing. Update the logic in
VirtualRouterLoadBalancerBackend around the vr Optional handling so the
dedicated VR becomes the method result before the empty-vrs path or the
subsequent assertion is reached.
- Around line 2107-2149: The backend L3 discovery logic only returns the first
matching L3 for server IPs, which can miss additional peer L3 networks or pick
the wrong one when IPs overlap. Update findBackendL3Network() and
getBackendL3NetworkUuids() in VirtualRouterLoadBalancerBackend so they collect
all matching L3NetworkUuid values from UsedIpVO lookups instead of stopping at
the first hit. If the dedicated VR path must remain single-L3, add an explicit
validation at the VR चयन/entry point and fail fast with a clear error when
multiple backend L3s are found.
---
Nitpick comments:
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerFactory.java`:
- Around line 21-23: The new extension method getDefaultProviderType in
LoadBalancerFactory needs a proper Javadoc explaining that the default null
return means no default provider is specified and implementers should override
it when required. Add the Javadoc directly on this interface method, keep the
method declaration as a plain interface method without extra modifiers, and make
the null semantics explicit so implementers can decide whether overriding is
mandatory.
🪄 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: 8b62a1b1-ece5-4649-9168-a17533479bf0
📒 Files selected for processing (21)
conf/db/upgrade/V5.5.28__schema.sqlplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APIChangeLoadBalancerListenerMsg.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APIChangeLoadBalancerListenerMsgDoc_zh_cn.groovyplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APICreateLoadBalancerListenerMsg.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/APICreateLoadBalancerListenerMsgDoc_zh_cn.groovyplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerBase.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerFactory.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerListenerInventory.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerListenerVO.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerListenerVO_.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerManagerImpl.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/SharedLoadBalancerFactory.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javasdk/src/main/java/org/zstack/sdk/ChangeLoadBalancerListenerAction.javasdk/src/main/java/org/zstack/sdk/CreateLoadBalancerListenerAction.javasdk/src/main/java/org/zstack/sdk/LoadBalancerListenerInventory.javatest/src/test/groovy/org/zstack/test/integration/db/schema/CheckSchemaUpgradeCase.groovytest/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/loadbalancer/TcpIpvsLoadBalancerListenerApiCase.groovytest/src/test/java/org/zstack/test/lb/TestVirtualRouterLb13.java
0558606 to
ce4344c
Compare
|
Comment from 刘德靖: SUG-2795 integration update: 已修复并推送 zstack MR !10307 的 3 条 CodeRabbit blocker:
验证:
当前 zstack HEAD: 剩余 completion gate:DevOps task/build 聚合结果还没找到,待补 DevOps PASS/success 证据。 |
Fix the SUG-2795 T2 Cloud review findings on top of the feature LB IPVS branch. Add API docs for IPVS listener fields, return the dedicated VR found through peer-L3 fallback, and collect all matching backend L3 networks for server IP backends. Test: git diff --check Resolves: ZSTAC-86152 Change-Id: Id0b8e8c1b972c2683e82a5ddb0d77888774c84ba
ce4344c to
3fe5c25
Compare
|
Comment from 刘德靖: SUG-2795 integration update: 已按要求把 zstack MR !10307 的 base/target 从 当前确认:
剩余 completion gate:DevOps 聚合结果待补。 |
Match the SUG-2795 listener doc template output generated by the UTBuild docpremium check so the working tree stays clean after template generation. Test: git diff --check Resolves: ZSTAC-86152 Change-Id: I337645c6290c9fc730057439ec4d38633c2c7336
Restore the legacy VM nic backend fallback when selecting the backend L3 used to create a virtual router. Keep the stricter multi-L3 validation for server IP backend discovery, where there is no VM nic ordering to preserve. Test: git diff --check Test: mvn -pl plugin/loadBalancer,plugin/virtualRouterProvider -am -DskipTests compile Resolves: ZSTAC-86152 Change-Id: I0a5774b8e4fdef15fb9c5d0a96459dd23b6b669c
SUG-2795 integration MR for TCP IPVS LB backend.\n\nScope:\n- Integrate T2 Cloud management-plane patch on 5.5.28.\n- T6 has no Cloud-side code per integration confirmation.\n\nValidation:\n- git diff --check passed during integration.\n- Local package build produced zstack.war.\n- Live post-deploy E2E on MN 172.24.241.166 covered TCP IPVS full_nat listener create, multi backend, multi server group, weight, scheduler, metrics/status observation, backend delete and listener cleanup.\n\nNote:\n- Live environment is premium; full community zstack.war was not used for final deployment. Cloud validation used minimal loadBalancer/sdk jar replacement to avoid premium dependency mismatch.
sync from gitlab !10307