Skip to content

fix(sdnController): validate L2 network detach before SDN controller delete#3346

Open
ZStack-Robot wants to merge 2 commits into5.5.6from
sync/ye.zou/fix/ZSTAC-80186
Open

fix(sdnController): validate L2 network detach before SDN controller delete#3346
ZStack-Robot wants to merge 2 commits into5.5.6from
sync/ye.zou/fix/ZSTAC-80186

Conversation

@ZStack-Robot
Copy link
Collaborator

Problem

ZSTAC-80186: Deleting an SDN controller without first detaching its L2 vxlan network pools leaves orphaned L2 networks with broken SDN references.

Root Cause

APIRemoveSdnControllerMsg has no validation in the interceptor. The cascade extension handleDeletionCheck() also immediately returns success without checking for attached resources.

Fix

Add validation in SdnControllerApiInterceptor for APIRemoveSdnControllerMsg: check if any HardwareL2VxlanNetworkPoolVO still references this SDN controller UUID. If so, reject the delete with a clear error message asking to detach first.

Changes

  • SdnControllerApiInterceptor.java: Add validate method for APIRemoveSdnControllerMsg
  • CloudOperationsErrorCode.java: Add error code ORG_ZSTACK_SDNCONTROLLER_10031

Testing

  • Compile verified: BUILD SUCCESS

sync from gitlab !9175

…r delete

Resolves: ZSTAC-80186

Change-Id: I8dcb689b022ad907c12bc1b481fb9d0db1e98d06
@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Could not fetch remote config from http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml: TimeoutError: The operation was aborted due to timeout
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉


Walkthrough

在 SDN 控制器的 API 拦截器中新增对移除消息的验证:在移除控制器前查询是否存在关联的 HardwareL2VxlanNetworkPoolVO 池,若存在则抛出异常阻止移除;同时新增错误码常量,并在测试清理中先删除所有 L2 VXLAN 池以避免阻塞删除流程。

Changes

Cohort / File(s) Summary
SDN 控制器拦截器
plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerApiInterceptor.java
新增私有方法 validate(APIRemoveSdnControllerMsg msg) 并在拦截逻辑中处理 APIRemoveSdnControllerMsg,查询关联的 HardwareL2VxlanNetworkPoolVO 池并在存在时抛出 ApiMessageInterceptionException 以阻止移除。
错误码常量扩展
utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
新增错误码常量 ORG_ZSTACK_SDNCONTROLLER_10031,用于表示 SDN 控制器移除时的特定错误情形。
测试清理调整
test/src/test/groovy/org/zstack/test/integration/network/sdnController/HardwareVxlanCase.groovy
clean() 中添加预清理:先查询并删除所有 L2 VXLAN 网络池(按 UUID 删除),然后再执行环境删除,避免移除被关联池阻塞。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 分钟

Poem

🐰 我在草间轻嗅风,
移除前把池塘查清,
若有波纹不得走,
新码一枚护后程,
跳跃验证笑盈盈。

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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.
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (6 files):

⚔️ build/pom.xml (content)
⚔️ compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java (content)
⚔️ core/pom.xml (content)
⚔️ plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerApiInterceptor.java (content)
⚔️ test/src/test/groovy/org/zstack/test/integration/network/sdnController/HardwareVxlanCase.groovy (content)
⚔️ utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java (content)

These conflicts must be resolved before merging into 5.5.6.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 标题清晰准确地反映了主要改动:在删除SDN控制器前验证L2网络已分离。
Description check ✅ Passed 描述详细说明了问题、根本原因和修复方案,与代码变更完全相关。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/ye.zou/fix/ZSTAC-80186
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch sync/ye.zou/fix/ZSTAC-80186
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test/src/test/groovy/org/zstack/test/integration/network/sdnController/HardwareVxlanCase.groovy (1)

44-48: 建议限定清理范围,避免无条件删除所有池

queryL2VxlanNetworkPool {} 无条件查询会删除当前数据库中所有 VXLAN 池,若测试并发或共享 DB,可能影响其他用例。建议限定到本用例的 zone(或缓存创建的 pool UUID 列表)再删除。

🛠️ 参考改法(按 zone 限定)
 void clean() {
     // ZSTAC-80186: delete pools created in createEnv() before env.delete()
     // removes the SDN controller (which now validates no attached pools)
-    queryL2VxlanNetworkPool {}.each { pool ->
+    def zone = env.inventoryByName("zone") as ZoneInventory
+    queryL2VxlanNetworkPool { conditions = ["zoneUuid=${zone.uuid}"] }.each { pool ->
         deleteL2Network { uuid = pool.uuid }
     }
     env.delete()
 }

Resolves: ZSTAC-80186

Change-Id: Ief74f07715f75fe86ae932a8a0cffc91e4bc36c1
@MatheMatrix MatheMatrix force-pushed the sync/ye.zou/fix/ZSTAC-80186 branch from 72b873f to 3bd207c Compare February 17, 2026 02:03
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.

2 participants