Skip to content

Conversation

@zstack-robot-1
Copy link
Collaborator

Resolves: ZSTAC-80998

Change-Id: I6868776c19a1b8f5da7446b2a72bfbe1b4a483b8

sync from gitlab !8960

Resolves: ZSTAC-80998

Change-Id: I6868776c19a1b8f5da7446b2a72bfbe1b4a483b8
@coderabbitai
Copy link

coderabbitai bot commented Dec 26, 2025

概览

在VirtualRouterLoadBalancerBackend中,强化VR-NIC过滤逻辑,要求在提供vr时vipInUsed必须非空;在VirtualRouterHa备份刷新回调路径中增加调试日志并清除VIP和IPv6 VIP以避免双主场景。

变更

内聚组 / 文件 变更总结
负载均衡后端逻辑优化
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
makeLbTOs方法中强化VR-NIC过滤:当提供vr参数时,现要求vipInUsed非空才能从匹配的VM NIC中选择publicNic;VirtualRouterHa备份刷新回调路径中新增调试日志记录备份VR上的LB刷新操作,并在刷新前清除LoadBalancerStruct中的VIP和IPv6 VIP,以防止双主场景发生

预估审查工作量

🎯 2 (简单) | ⏱️ ~10分钟

诗文

🐰✨ 虚拟路由添新装,
NIC过滤更周详,
备份清晰防双主,
日志闪烁亮堂堂,
均衡负载无忧扰~

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 标题遵循了规定的格式,包含类型前缀、作用域和描述,长度为55个字符,符合72字符以内的要求。
Description check ✅ Passed 描述信息与变更集相关,提及了具体的问题追踪号ZSTAC-80998和备份负载均衡VIP刷新的修复内容。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/boce.wang/feature-5.5.0-bond-sriov-final

📜 Recent review details

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54d76dc and e726ee1.

📒 Files selected for processing (1)
  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*

⚙️ CodeRabbit configuration file

**/*.*: - 代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写

Files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 1. API 设计要求

  • API 命名:
    • API 名称必须唯一,不能重复。
    • API 消息类需要继承 APIMessage;其返回类必须继承 APIReplyAPIEvent,并在注释中用 @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 类型类除外。
      • 抽象类采用 AbstractBase 前缀/后缀。
      • 异常类应以 Exception 结尾。
      • 测试类需要以 TestCase 结尾。
  • 方法名、参数名、成员变量和局部变量:

    • 使用 lowerCamelCase 风格。
  • 常量命名:

    • 全部大写,使用下划线分隔单词。
    • 要求表达清楚,避免使用含糊或不准确的名称。
  • 包名:

    • 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
  • 命名细节:

    • 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
    • 命名缩写:
      • 不允许使用不必要的缩写,如:AbsSchedulerJobcondiFu 等。应使用完整单词提升可读性。

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:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
🧠 Learnings (7)
📚 Learning: 2025-08-12T05:39:14.846Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3856-3865
Timestamp: 2025-08-12T05:39:14.846Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp 流程中,从数据库加载的 VmNicVO 实例其 getUsedIps() 为空时也不会为 null(返回空集合)。因此无需对 getUsedIps() 再做 Optional/空列表归一化的空指针保护;若找不到对应 NIC,使用 orElse(new VmNicVO()) 的约定允许后续逻辑通过 NPE 暴露问题,与项目既有约定一致。

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
📚 Learning: 2025-10-09T02:23:30.305Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2709
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:5142-5142
Timestamp: 2025-10-09T02:23:30.305Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 getAttachableL3Network() 逻辑中,项目约束是:同一 VmNic 可能同时存在两条 UsedIpVO,但它们只代表同一 L3 的 IPv4/IPv6 双栈,UsedIpVO.l3NetworkUuid 与 VmNicVO.l3NetworkUuid 保持一致;因此使用 VmNicVO_.l3NetworkUuid 列表判断 IPAM 关闭网络是否已附加是正确的,无需通过 UsedIpVO 或 VmNicHelper.getL3Uuids() 额外聚合。

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
📚 Learning: 2025-08-04T03:23:16.366Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3839-3840
Timestamp: 2025-08-04T03:23:16.366Z
Learning: 在 `VmInstanceBase.java` 中,查找 NIC 时使用 `orElse(new VmNicVO())` 是为了类型统一,后续操作如果未找到 NIC 会直接抛出空指针异常,无需在此处额外处理异常。

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
📚 Learning: 2025-07-22T02:30:46.123Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2311
File: plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java:52-53
Timestamp: 2025-07-22T02:30:46.123Z
Learning: In ZStack, backend realization code (such as KVMRealizeHardwareVxlanNetworkBackend) can safely assume the existence of the VO for the resource being operated on (e.g., HardwareL2VxlanNetworkVO for a given L2NetworkInventory), due to system lifecycle management and strong foreign key constraints in the database schema. Explicit null checks after findByUuid are not required in these contexts.

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
📚 Learning: 2025-07-22T02:30:46.123Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2311
File: plugin/sdnController/src/main/java/org/zstack/sdnController/hardwareVxlan/KVMRealizeHardwareVxlanNetworkBackend.java:52-53
Timestamp: 2025-07-22T02:30:46.123Z
Learning: In ZStack, DatabaseFacade.findByUuid() calls for VO objects in contexts like KVMRealizeHardwareVxlanNetworkBackend do not require null checks because the L2NetworkInventory passed to these methods is guaranteed to have a corresponding VO record in the database by the system's design and calling patterns.

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
📚 Learning: 2025-08-29T03:35:56.509Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2528
File: plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupNetworkServiceExtension.java:134-139
Timestamp: 2025-08-29T03:35:56.509Z
Learning: In ZStack's SecurityGroupNetworkServiceExtension.java applyNetworkService method, there is no need to add null/empty checks for destNics because NetworkServiceManagerImpl.java already performs this validation upstream with `if (spec.getDestNics().isEmpty()) { completion.success(); return; }` before calling applyNetworkService.

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
📚 Learning: 2025-08-29T03:35:56.509Z
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2528
File: plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupNetworkServiceExtension.java:134-139
Timestamp: 2025-08-29T03:35:56.509Z
Learning: In ZStack's SecurityGroupNetworkServiceExtension.java applyNetworkService method, there is no need to add null/empty checks for destNics because NetworkServiceManagerImpl.java already performs this validation upstream with `if (spec.getDestNics().isEmpty()) { completion.success(); return; }` before calling applyNetworkService, ensuring destNics will never be empty when reaching the extension methods.

Applied to files:

  • plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java
🔇 Additional comments (2)
plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lb/VirtualRouterLoadBalancerBackend.java (2)

664-671: 空指针防护逻辑正确。

新增的 vipInUsed != null 检查可以防止当 vipvip6 都为 null 时,在第 666 行访问 vipInUsed.getL3NetworkUuid() 导致的空指针异常。这是一个合理的防御性编程改进。


2766-2770: 清除 VIP 的 HA 设计正确,无需进一步修改。

HA 备用虚拟路由器在刷新负载均衡器配置前清除 VIP 和 IPv6 VIP 是正确的设计:

  1. VIP 清除语义正确:备用 VR 不应拥有 VIP,防止主备双活。已在 REFRESH_LB_TASK 回调中实施。

  2. 其他配置正常保留makeLbTOs() 方法通过 vipInUsed = vip == null ? vip6 : vip 逻辑安全处理 null VIP,后端服务器列表和监听器配置不受影响。

  3. 证书刷新无需 VIP 清除:证书绑定到 LoadBalancerListenerInventory(监听器),与 VIP 无关,REFRESH_CERTIFICATE_TASK 不需要类似处理。


Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants