<fix>[eip]: expose attaching EIP to release#4127
Open
ZStack-Robot wants to merge 1 commit into
Open
Conversation
Persist vmNicUuid and guestIp before applying an EIP on the backend. This lets VM stop and migration release paths see the in-flight EIP and enqueue cleanup while the backend apply is still running. Roll back the binding only when the same EIP, NIC, and guest IP are still present if backend apply fails. Related: ZSTAC-83309 Change-Id: Ia9b75acc82004755b905a02acca1a6a82c207332
走查在 EIP 附加请求处理中,改进了成功和失败两条路径的状态管理逻辑。成功时缓存目标值并使用刷新后的数据生成回调,失败时通过原值精确定位并清空绑定。 变更EIP 附加状态管理
评估代码审查工作量🎯 2 (简单) | ⏱️ ~10 分钟 诗
🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/eip/src/main/java/org/zstack/network/service/eip/EipManagerImpl.java (1)
610-621: ⚡ Quick win在回滚条件更新后检查受影响行数并告警,防止与
vmIpChanged竞态造成状态不一致。
vmIpChanged(...)会在找到EipVO_.vmNicUuid == nic.uuid且EipVO_.guestIp == oldIp时,直接eip.setGuestIp(newIp)并dbf.update(eip)。- 回滚里使用条件更新同时匹配
EipVO_.vmNicUuid == attachedVmNicUuid且EipVO_.guestIp == attachedGuestIp;若回滚前已发生vmIpChanged改写了guestIp,则该回滚可能更新 0 行,绑定不会被清空。UpdateQuery.update()返回int,建议在回滚处接收返回值:affectedRows == 0时记录 warning 日志(带eipUuid/attachedVmNicUuid/attachedGuestIp),便于排查该边界竞态。🤖 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/eip/src/main/java/org/zstack/network/service/eip/EipManagerImpl.java` around lines 610 - 621, The rollback in the anonymous fail(...) inside EipManagerImpl uses a conditional UpdateQuery but ignores the returned affected row count; modify the fail handler to capture the int returned by SQL.New(...).set(...).update() and when affectedRows == 0 emit a warning log (include msg.getEipUuid(), attachedVmNicUuid and attachedGuestIp) to surface the vmIpChanged race where guestIp was changed and the conditional update cleared 0 rows; keep existing evt.setError(...) and bus.publish(evt) behavior after logging.
🤖 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.
Nitpick comments:
In `@plugin/eip/src/main/java/org/zstack/network/service/eip/EipManagerImpl.java`:
- Around line 610-621: The rollback in the anonymous fail(...) inside
EipManagerImpl uses a conditional UpdateQuery but ignores the returned affected
row count; modify the fail handler to capture the int returned by
SQL.New(...).set(...).update() and when affectedRows == 0 emit a warning log
(include msg.getEipUuid(), attachedVmNicUuid and attachedGuestIp) to surface the
vmIpChanged race where guestIp was changed and the conditional update cleared 0
rows; keep existing evt.setError(...) and bus.publish(evt) behavior after
logging.
ℹ️ 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: 7442b00d-6e9c-4be2-9da2-4d7be0a506d2
📒 Files selected for processing (1)
plugin/eip/src/main/java/org/zstack/network/service/eip/EipManagerImpl.java
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
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.
Persist vmNicUuid and guestIp before applying an EIP on the backend. This lets VM stop and migration release paths see the in-flight EIP and enqueue cleanup while the backend apply is still running.
Roll back the binding only when the same EIP, NIC, and guest IP are still present if backend apply fails.
Related: ZSTAC-83309
Change-Id: Ia9b75acc82004755b905a02acca1a6a82c207332
sync from gitlab !10029