Skip to content

<fix>[rest]: fix bug<description> #3611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zstack-robot-2 wants to merge 2 commits into
base: feature-5.5.12-zns
Choose a base branch
from
+691 −352
Open

<fix>[rest]: fix bug<description> #3611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fec131c
<fix>[rest]: fix bug<description>
ruansteve Mar 26, 2026
bfc85ab
<fix>[sdnController]: ususu <description>
ruansteve Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmAllocateNicIpFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
import org.zstack.core.cloudbus.CloudBusListCallBack;
import org.zstack.core.db.DatabaseFacade;
import org.zstack.core.db.Q;
import org.zstack.core.componentloader.PluginRegistry;
import org.zstack.core.errorcode.ErrorFacade;
import org.zstack.header.core.Completion;
import org.zstack.header.core.WhileDoneCompletion;
import org.zstack.header.core.workflow.Flow;
import org.zstack.header.core.workflow.FlowRollback;
Expand Down Expand Up @@ -47,6 +49,8 @@ public class VmAllocateNicIpFlow implements Flow {
@Autowired
private VmNicManager nicManager;
@Autowired
private PluginRegistry pluginRgty;
@Autowired
protected VmInstanceManager vmMgr;

@Override
Expand Down Expand Up @@ -186,6 +190,39 @@ public void done(ErrorCodeList errorCodeList) {
} else {
dbf.updateCollection(nicsWithIp);
dbf.updateCollection(ipVOS);
callAfterAllocateVmNicIpExtensions(spec, trigger);
}
Comment on lines +193 to +194
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

不要把全量 spec 直接交给 AfterAllocateVmNicIpExtensionPoint。

当前 flow 只会给进入 firstL3s 的 NIC 回填 ip/UsedIpVO；enableIpAddressAllocation() == false 的 L3 会把未更新的 NIC 留在 spec.getDestNics() 里。这里把整个 spec 传给 after-hook 后，SdnControllerManagerImpl.afterAllocateVmNicIp() 这类实现会同时看到“已分配 IP”和“未分配 IP”的 NIC，混合/no-IPAM 场景很容易把未就绪 NIC 一起拿去建 port。建议改为传递本次真正完成 IP 准备的 NIC 集合，或至少先按 nicsWithIp 过滤后再触发扩展。

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateNicIpFlow.java` around
lines 193 - 194, The flow currently passes the entire spec into
callAfterAllocateVmNicIpExtensions(spec, trigger), which exposes both
IP-assigned and non-assigned NICs to AfterAllocateVmNicIpExtensionPoint
implementations (e.g., SdnControllerManagerImpl.afterAllocateVmNicIp); change
the call to only include the NICs that actually received IPs (the nicsWithIp /
firstL3s result) by constructing a filtered spec (or a minimal wrapper) that
contains only those destNics and their ip/UsedIpVO entries, then invoke
callAfterAllocateVmNicIpExtensions(filteredSpec, trigger) so extensions only see
NICs with prepared IPs.

}
});
}

private void callAfterAllocateVmNicIpExtensions(VmInstanceSpec spec, FlowTrigger trigger) {
List<AfterAllocateVmNicIpExtensionPoint> exts =
pluginRgty.getExtensionList(AfterAllocateVmNicIpExtensionPoint.class);
if (exts.isEmpty()) {
trigger.next();
return;
}

new While<>(exts).each((ext, wcomp) -> {
ext.afterAllocateVmNicIp(spec, new Completion(wcomp) {
@Override
public void success() {
wcomp.done();
}

@Override
public void fail(ErrorCode errorCode) {
wcomp.addError(errorCode);
wcomp.allDone();
}
});
}).run(new WhileDoneCompletion(trigger) {
@Override
public void done(ErrorCodeList errorCodeList) {
if (!errorCodeList.getCauses().isEmpty()) {
trigger.fail(errorCodeList.getCauses().get(0));
} else {
trigger.next();
}
}
Expand Down
5 changes: 5 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.zstack.header.tag.AdminOnlyTag;
import org.zstack.header.tag.TagDefinition;
import org.zstack.header.vm.VmInstanceVO;
import org.zstack.header.vm.VmNicVO;
import org.zstack.tag.PatternedSystemTag;
import org.zstack.tag.SensitiveTagOutputHandler;
import org.zstack.tag.SensitiveTag;
Expand Down Expand Up @@ -314,4 +315,8 @@ public String desensitizeTag(SystemTag systemTag, String tag) {
public static PatternedSystemTag VM_STATE_PAUSED_AFTER_MIGRATE = new PatternedSystemTag(("vmPausedAfterMigrate"), VmInstanceVO.class);

public static PatternedSystemTag VM_MEMORY_ACCESS_MODE_SHARED = new PatternedSystemTag(("vmMemoryAccessModeShared"), VmInstanceVO.class);

public static String IFACE_ID_TOKEN = "ifaceId";
public static PatternedSystemTag IFACE_ID = new PatternedSystemTag(
String.format("ifaceId::{%s}", IFACE_ID_TOKEN), VmNicVO.class);
}
1 change: 0 additions & 1 deletion conf/springConfigXml/sdnController.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
<zstack:extension interface="org.zstack.header.network.l3.AfterAddIpRangeExtensionPoint"/>
<zstack:extension interface="org.zstack.header.network.l3.IpRangeDeletionExtensionPoint"/>
<zstack:extension interface="org.zstack.header.network.service.GetSdnControllerExtensionPoint"/>
<zstack:extension interface="org.zstack.header.vm.BeforeAllocateVmNicExtensionPoint"/>
<zstack:extension interface="org.zstack.header.vm.AfterReleaseVmNicExtensionPoint"/>
</zstack:plugin>
</bean>
Expand Down
17 changes: 17 additions & 0 deletions core/src/main/java/org/zstack/core/rest/webhook/WebhookCallbackClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,30 @@ public String getCallbackUrl() {
return callbackUrl;
}

/**
* Override the callback URL. Use this when the callback is handled by a
* dedicated HTTP endpoint (e.g. a Spring Controller) rather than the
* sendCommand channel.
*/
public void setCallbackUrl(String callbackUrl) {
this.callbackUrl = callbackUrl;
}

/**
* @return the protocol adapter
*/
public WebhookProtocol<T> getProtocol() {
return protocol;
}

/**
* Deliver a callback that was received outside the sendCommand channel
* (e.g. from a dedicated Spring Controller endpoint for external systems).
*/
public void deliverCallback(T cmd) {
onCallback(cmd);
}

/**
* Callback handler invoked by the RESTFacade sendCommand channel.
*/
Expand Down
Loading