<feat>[lb]: add IPVS DR/FullNAT load balancer support#3847
Open
MatheMatrix wants to merge 1 commit into5.5.16from
Open
<feat>[lb]: add IPVS DR/FullNAT load balancer support#3847MatheMatrix wants to merge 1 commit into5.5.16from
MatheMatrix wants to merge 1 commit into5.5.16from
Conversation
A1-A3: Add IPVS_MODE constants, IPVS_MODE PatternedSystemTag on
LoadBalancerListenerVO, and IPVS_DEFAULT_MODE/SCHEDULER GlobalConfig
A4: ValidateIpvsMode() in LoadBalancerApiInterceptor — enforces LB-wide
mode consistency, rejects L7 tags, whitelists scheduler algorithms,
skips MAX_CONNECTION/CONNECTION_IDLE_TIMEOUT auto-inserts for IPVS
A5-A6: Extend LbTO with ipvsMode/scheduler/connectionType fields;
populate from IPVS_MODE systemTag in makeLbTOs()
C1: Add bindToLo field to VyosKeepalivedCommands.VyosHaVip (@GrayVersion 5.5.16)
Resolves: ZSTAC-84610
Change-Id: I15d1ec49aacb2de818d71a50900c9e7440fb77a2
总览本次变更为负载均衡器添加IPVS（IP虚拟服务器）模式配置支持，包括模式验证、全局配置、系统标签定义、以及虚拟路由转移对象的IPVS字段扩展。 变更详情
代码审查难度估计🎯 3 (中等) | ⏱️ ~25 分钟 诗歌
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java`:
- Around line 1312-1325: The update currently calls validateIpvsMode() only when
an IPVS_MODE tag is present in msg.getSystemTags(), which misses cases where a
listener already using IPVS changes other tags or when switching a non-IPVS
listener to IPVS because persisted listener tags (maxConnection,
connectionIdleTimeout, session*, etc.) aren’t considered; fix by loading the
existing listener’s persisted system tags and current ipvsMode (via the
LoadBalancerListenerVO query used to get protocol), merge those persisted tags
with msg.getSystemTags() to produce an effective tag set and determine the
effective ipvsMode, then call validateIpvsMode(loadBalancerUuid,
effectiveIpvsMode, protocol, mergedTags, null,
msg.getLoadBalancerListenerUuid()) so all historical and new tags are validated
together before applying the update.
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalConfig.java`:
- Around line 39-43: 限制全局配置 IPVS 默认值的取值范围：为 GlobalConfig IPVS_DEFAULT_MODE 和
IPVS_DEFAULT_SCHEDULER 增加严格校验，确保 IPVS_DEFAULT_MODE 只接受
LoadBalancerConstants.IPVS_MODES 中定义的值，IPVS_DEFAULT_SCHEDULER 只接受集合
{"rr","wrr","lc","sh"}；在其 GlobalConfigValidation 的实现或注册点（与 IPVS_DEFAULT_MODE /
IPVS_DEFAULT_SCHEDULER 声明相近的校验器）添加检查逻辑并在不符合时拒绝/报错，避免非法字符串被写入后由
VirtualRouterLoadBalancerBackend.mapBalancerAlgorithmToIpvsScheduler 或 listener
创建时才失败。
🪄 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: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 5f6afb7c-76ba-4f59-9acc-e71d1b4da8a8
⛔ Files ignored due to path filters (1)
conf/globalConfig/lb.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerConstants.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalConfig.javaplugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerSystemTags.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/vyos/VyosKeepalivedCommands.java
Comment on lines
+1312
to
+1325
| // Validate ipvsMode consistency if the update carries a new ipvsMode tag | ||
| String newIpvsMode = null; | ||
| for (String tag : msg.getSystemTags()) { | ||
| if (LoadBalancerSystemTags.IPVS_MODE.isMatch(tag)) { | ||
| newIpvsMode = LoadBalancerSystemTags.IPVS_MODE.getTokenByTag(tag, LoadBalancerSystemTags.IPVS_MODE_TOKEN); | ||
| } | ||
| } | ||
| if (newIpvsMode != null) { | ||
| String protocol = Q.New(LoadBalancerListenerVO.class) | ||
| .select(LoadBalancerListenerVO_.protocol) | ||
| .eq(LoadBalancerListenerVO_.uuid, msg.getLoadBalancerListenerUuid()) | ||
| .findValue(); | ||
| validateIpvsMode(loadBalancerUuid, newIpvsMode, protocol, msg.getSystemTags(), null, msg.getLoadBalancerListenerUuid()); | ||
| } |
There was a problem hiding this comment.
更新 listener 时的 IPVS 校验还不完整。
这里仅在请求里显式带了 IPVS_MODE tag 时才调用 validateIpvsMode()，而且传进去的是 msg.getSystemTags()。这会漏掉两类场景：1）一个已经是 IPVS 的 listener 只改 balancerAlgorithm，不会再走 IPVS 白名单校验；2）把现有非 IPVS listener 切到 IPVS 时，库里已经存在的 maxConnection / connectionIdleTimeout / session* 等历史 tag 不会被检查。后面 plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java 的 makeLbTOs() 仍会把这些旧 tag 放进 parameters 下发，最终形成非法组合。建议这里先读取当前 listener 的持久化 tags 和当前 ipvsMode，合并出一份 effective 配置后再统一做校验。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerApiInterceptor.java`
around lines 1312 - 1325, The update currently calls validateIpvsMode() only
when an IPVS_MODE tag is present in msg.getSystemTags(), which misses cases
where a listener already using IPVS changes other tags or when switching a
non-IPVS listener to IPVS because persisted listener tags (maxConnection,
connectionIdleTimeout, session*, etc.) aren’t considered; fix by loading the
existing listener’s persisted system tags and current ipvsMode (via the
LoadBalancerListenerVO query used to get protocol), merge those persisted tags
with msg.getSystemTags() to produce an effective tag set and determine the
effective ipvsMode, then call validateIpvsMode(loadBalancerUuid,
effectiveIpvsMode, protocol, mergedTags, null,
msg.getLoadBalancerListenerUuid()) so all historical and new tags are validated
together before applying the update.
Comment on lines
+39
to
+43
| @GlobalConfigValidation | ||
| public static GlobalConfig IPVS_DEFAULT_MODE = new GlobalConfig(CATEGORY, "ipvs.defaultMode"); | ||
|
|
||
| @GlobalConfigValidation | ||
| public static GlobalConfig IPVS_DEFAULT_SCHEDULER = new GlobalConfig(CATEGORY, "ipvs.defaultScheduler"); |
There was a problem hiding this comment.
请在全局配置层限制 IPVS 默认值的取值范围。
这里的校验还不够强：ipvs.defaultScheduler 现在可以被写成任意字符串，而 plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java 的 mapBalancerAlgorithmToIpvsScheduler() 会直接把它当 fallback 下发给 VR；ipvs.defaultMode 也会在创建 listener 时才迟到失败。建议把这两个配置收敛到明确的允许集合（LoadBalancerConstants.IPVS_MODES 和 rr/wrr/lc/sh），避免坏配置先落库、再在运行时爆出来。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/loadBalancer/src/main/java/org/zstack/network/service/lb/LoadBalancerGlobalConfig.java`
around lines 39 - 43, 限制全局配置 IPVS 默认值的取值范围：为 GlobalConfig IPVS_DEFAULT_MODE 和
IPVS_DEFAULT_SCHEDULER 增加严格校验，确保 IPVS_DEFAULT_MODE 只接受
LoadBalancerConstants.IPVS_MODES 中定义的值，IPVS_DEFAULT_SCHEDULER 只接受集合
{"rr","wrr","lc","sh"}；在其 GlobalConfigValidation 的实现或注册点（与 IPVS_DEFAULT_MODE /
IPVS_DEFAULT_SCHEDULER 声明相近的校验器）添加检查逻辑并在不符合时拒绝/报错，避免非法字符串被写入后由
VirtualRouterLoadBalancerBackend.mapBalancerAlgorithmToIpvsScheduler 或 listener
创建时才失败。
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
2 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.
A1-A3: Add IPVS_MODE constants, IPVS_MODE PatternedSystemTag on
LoadBalancerListenerVO, and IPVS_DEFAULT_MODE/SCHEDULER GlobalConfig
A4: ValidateIpvsMode() in LoadBalancerApiInterceptor — enforces LB-wide
mode consistency, rejects L7 tags, whitelists scheduler algorithms,
skips MAX_CONNECTION/CONNECTION_IDLE_TIMEOUT auto-inserts for IPVS
A5-A6: Extend LbTO with ipvsMode/scheduler/connectionType fields;
populate from IPVS_MODE systemTag in makeLbTOs()
C1: Add bindToLo field to VyosKeepalivedCommands.VyosHaVip (@GrayVersion 5.5.16)
Resolves: ZSTAC-84610
Change-Id: I15d1ec49aacb2de818d71a50900c9e7440fb77a2
sync from gitlab !9718