changes and refactor to fix command deserialization issue

shwstppr · shwstppr · commit 3e8687491341 · 2025-07-02T06:31:07.000+05:30
Signed-off-by: Abhishek Kumar &lt;abhishek.mrt22@gmail.com&gt;
diff --git a/api/src/main/java/com/cloud/agent/api/Command.java b/api/src/main/java/com/cloud/agent/api/Command.java
@@ -19,9 +19,10 @@
 import java.util.HashMap;
 import java.util.Map;
 
-import com.cloud.agent.api.LogLevel.Log4jLevel;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import com.cloud.agent.api.LogLevel.Log4jLevel;
 
 /**
  * implemented by classes that extends the Command class. Command specifies
@@ -60,6 +61,7 @@ public enum State {
     private int wait;  //in second
     private boolean bypassHostMaintenance = false;
     private transient long requestSequence = 0L;
+    protected Map<String, Map<String, String>> externalDetails;
 
     protected Command() {
         this.wait = 0;
@@ -128,6 +130,14 @@ public void setRequestSequence(long requestSequence) {
         this.requestSequence = requestSequence;
     }
 
+    public void setExternalDetails(Map<String, Map<String, String>> externalDetails) {
+        this.externalDetails = externalDetails;
+    }
+
+    public Map<String, Map<String, String>> getExternalDetails() {
+        return externalDetails;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
diff --git a/core/src/main/java/com/cloud/agent/api/PrepareExternalProvisioningCommand.java b/core/src/main/java/com/cloud/agent/api/PrepareExternalProvisioningCommand.java
@@ -18,15 +18,11 @@
 //
 package com.cloud.agent.api;
 
-import java.util.Map;
-
 import com.cloud.agent.api.to.VirtualMachineTO;
 
 public class PrepareExternalProvisioningCommand extends Command {
 
     VirtualMachineTO virtualMachineTO;
-    Long clusterId;
-    Map<String, Object> externalDetails;
 
     public PrepareExternalProvisioningCommand(VirtualMachineTO vmTO) {
         this.virtualMachineTO = vmTO;
@@ -36,14 +32,6 @@ public VirtualMachineTO getVirtualMachineTO() {
         return virtualMachineTO;
     }
 
-    public Map<String, Object> getExternalDetails() {
-        return externalDetails;
-    }
-
-    public void setExternalDetails(Map<String, Object> externalDetails) {
-        this.externalDetails = externalDetails;
-    }
-
     @Override
     public boolean executeInSequence() {
         return false;
diff --git a/core/src/main/java/com/cloud/agent/api/RebootCommand.java b/core/src/main/java/com/cloud/agent/api/RebootCommand.java
@@ -19,14 +19,11 @@
 
 package com.cloud.agent.api;
 
-import java.util.Map;
-
 import com.cloud.agent.api.to.VirtualMachineTO;
 
 public class RebootCommand extends Command {
     String vmName;
     VirtualMachineTO vm;
-    private Map<String, Object> externalDetails;
     protected boolean executeInSequence = false;
 
     protected RebootCommand() {
@@ -49,14 +46,6 @@ public VirtualMachineTO getVirtualMachine() {
         return vm;
     }
 
-    public void setExternalDetails(Map<String, Object> externalDetails) {
-        this.externalDetails = externalDetails;
-    }
-
-    public Map<String, Object> getExternalDetails() {
-        return externalDetails;
-    }
-
     @Override
     public boolean executeInSequence() {
         return this.executeInSequence;
diff --git a/core/src/main/java/com/cloud/agent/api/RunCustomActionCommand.java b/core/src/main/java/com/cloud/agent/api/RunCustomActionCommand.java
@@ -26,7 +26,6 @@ public class RunCustomActionCommand extends Command {
     String actionName;
     Long vmId;
     Map<String, Object> parameters;
-    Map<String, Object> externalDetails;
 
     public RunCustomActionCommand(String actionName) {
         this.actionName = actionName;
@@ -53,14 +52,6 @@ public void setParameters(Map<String, Object> parameters) {
         this.parameters = parameters;
     }
 
-    public Map<String, Object> getExternalDetails() {
-        return externalDetails;
-    }
-
-    public void setExternalDetails(Map<String, Object> externalDetails) {
-        this.externalDetails = externalDetails;
-    }
-
     @Override
     public boolean executeInSequence() {
         return false;
diff --git a/core/src/main/java/com/cloud/agent/api/StartCommand.java b/core/src/main/java/com/cloud/agent/api/StartCommand.java
@@ -19,8 +19,6 @@
 
 package com.cloud.agent.api;
 
-import java.util.Map;
-
 import com.cloud.agent.api.to.VirtualMachineTO;
 import com.cloud.host.Host;
 
@@ -31,7 +29,6 @@ public class StartCommand extends Command {
     String hostIp;
     boolean executeInSequence = false;
     String secondaryStorage;
-    Map<String, Object> externalDetails;
 
     public VirtualMachineTO getVirtualMachine() {
         return vm;
@@ -67,12 +64,4 @@ public String getSecondaryStorage() {
     public void setSecondaryStorage(String secondary) {
         this.secondaryStorage = secondary;
     }
-
-    public Map<String, Object> getExternalDetails() {
-        return externalDetails;
-    }
-
-    public void setExternalDetails(Map<String, Object> externalDetails) {
-        this.externalDetails = externalDetails;
-    }
 }
diff --git a/core/src/main/java/com/cloud/agent/api/StopCommand.java b/core/src/main/java/com/cloud/agent/api/StopCommand.java
@@ -39,8 +39,6 @@ public class StopCommand extends RebootCommand {
     Map<String, Boolean> vlanToPersistenceMap;
     boolean expungeVM = false;
 
-    private Map<String, Object> externalDetails;
-
     public Map<String, DpdkTO> getDpdkInterfaceMapping() {
         return dpdkInterfaceMapping;
     }
@@ -149,12 +147,4 @@ public boolean isExpungeVM() {
     public void setExpungeVM(boolean expungeVM) {
         this.expungeVM = expungeVM;
     }
-
-    public void setExternalDetails(Map<String, Object> externalDetails) {
-        this.externalDetails = externalDetails;
-    }
-
-    public Map<String, Object> getExternalDetails() {
-        return externalDetails;
-    }
 }
diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -1272,7 +1272,7 @@ protected void processPrepareExternalProvisioning(boolean firstStart, Host host,
             virtualMachineTO.setNics(nicTOs);
         }
         Map<String, String> vmDetails = virtualMachineTO.getExternalDetails();
-        Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host,
+        Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host,
                 vmDetails);
         PrepareExternalProvisioningCommand cmd = new PrepareExternalProvisioningCommand(virtualMachineTO);
         cmd.setExternalDetails(externalDetails);
@@ -1677,7 +1677,7 @@ protected void updateStartCommandWithExternalDetails(Host host, VirtualMachineTO
                 vmExternalDetails.put(VmDetailConstants.CLOUDSTACK_VLAN, Networks.BroadcastDomainType.getValue(nic.getBroadcastUri()));
             }
         }
-        Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host, vmExternalDetails);
+        Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host, vmExternalDetails);
         command.setExternalDetails(externalDetails);
     }
 
@@ -1691,7 +1691,16 @@ protected void updateStopCommandForExternalHypervisorType(final HypervisorType h
             return;
         }
         VirtualMachineTO vmTO = ObjectUtils.defaultIfNull(stopCommand.getVirtualMachine(), toVmTO(vmProfile));
-        Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());
+        if (MapUtils.isEmpty(vmTO.getGuestOsDetails())) {
+            vmTO.setGuestOsDetails(null);
+        }
+        if (MapUtils.isEmpty(vmTO.getExtraConfig())) {
+            vmTO.setExtraConfig(null);
+        }
+        if (MapUtils.isEmpty(vmTO.getNetworkIdToNetworkNameMap())) {
+            vmTO.setNetworkIdToNetworkNameMap(null);
+        }
+        Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());
         stopCommand.setVirtualMachine(vmTO);
         stopCommand.setExternalDetails(externalDetails);
     }
@@ -1700,7 +1709,7 @@ protected void updateRebootCommandWithExternalDetails(Host host, VirtualMachineT
         if (!HypervisorType.External.equals(host.getHypervisorType())) {
             return;
         }
-        Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());
+        Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());
         rebootCmd.setExternalDetails(externalDetails);
     }
 
diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManager.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManager.java
@@ -84,7 +84,7 @@ public interface ExtensionsManager extends Manager {
 
     ExtensionCustomActionResponse createCustomActionResponse(ExtensionCustomAction customAction);
 
-    Map<String, Object> getExternalAccessDetails(Host host, Map<String, String> vmDetails);
+    Map<String, Map<String, String>> getExternalAccessDetails(Host host, Map<String, String> vmDetails);
 
     String handleExtensionServerCommands(ExtensionServerActionBaseCommand cmd);
 }
diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java
@@ -461,14 +461,16 @@ protected void updateAllExtensionHosts(Extension extension, Long clusterId, bool
         executorService.shutdown();
     }
 
-    protected Map<String, Object> getExternalAccessDetails(Map<String, String> actionDetails, long hostId,
+    protected Map<String, Map<String, String>> getExternalAccessDetails(Map<String, String> actionDetails, long hostId,
                            ExtensionResourceMap resourceMap) {
-        Map<String, Object> externalDetails = new HashMap<>();
+        Map<String, Map<String, String>> externalDetails = new HashMap<>();
         if (MapUtils.isNotEmpty(actionDetails)) {
             externalDetails.put(ApiConstants.ACTION, actionDetails);
         }
         Map<String, String> hostDetails = getFilteredExternalDetails(hostDetailsDao.findDetails(hostId));
-        externalDetails.put(ApiConstants.HOST, hostDetails);
+        if (MapUtils.isNotEmpty(hostDetails)) {
+            externalDetails.put(ApiConstants.HOST, hostDetails);
+        }
         if (resourceMap == null) {
             return externalDetails;
         }
@@ -477,7 +479,9 @@ protected Map<String, Object> getExternalAccessDetails(Map<String, String> actio
             externalDetails.put(ApiConstants.RESOURCE_MAP, resourceDetails);
         }
         Map<String, String> extensionDetails = extensionDetailsDao.listDetailsKeyPairs(resourceMap.getExtensionId(), true);
-        externalDetails.put(ApiConstants.EXTENSION, extensionDetails);
+        if (MapUtils.isNotEmpty(extensionDetails)) {
+            externalDetails.put(ApiConstants.EXTENSION, extensionDetails);
+        }
         return externalDetails;
     }
 
@@ -1326,10 +1330,12 @@ public CustomActionResultResponse runCustomAction(RunCustomActionCmd cmd) {
         response.setSuccess(false);
         result.put(ApiConstants.MESSAGE, getActionMessage(false, customActionVO, extensionVO,
                 actionResourceType, entity));
-        Map<String, Object> externalDetails = getExternalAccessDetails(allDetails.first(), hostId, extensionResource);
+        Map<String, Map<String, String>> externalDetails =
+                getExternalAccessDetails(allDetails.first(), hostId, extensionResource);
         runCustomActionCommand.setParameters(parameters);
         runCustomActionCommand.setExternalDetails(externalDetails);
         try {
+            logger.info("Running custom action: {}", GsonHelper.getGson().toJson(runCustomActionCommand));
             Answer answer = agentMgr.send(hostId, runCustomActionCommand);
             if (!(answer instanceof RunCustomActionAnswer)) {
                 logger.error("Unexpected answer [{}] received for {}", answer.getClass().getSimpleName(),
@@ -1393,11 +1399,11 @@ public ExtensionCustomActionResponse createCustomActionResponse(ExtensionCustomA
     }
 
     @Override
-    public Map<String, Object> getExternalAccessDetails(Host host, Map<String, String> vmDetails) {
+    public Map<String, Map<String, String>> getExternalAccessDetails(Host host, Map<String, String> vmDetails) {
         long clusterId = host.getClusterId();
         ExtensionResourceMapVO resourceMap = extensionResourceMapDao.findByResourceIdAndType(clusterId,
                 ExtensionResourceMap.ResourceType.Cluster);
-        Map<String, Object> details = getExternalAccessDetails(null, host.getId(), resourceMap);
+        Map<String, Map<String, String>> details = getExternalAccessDetails(null, host.getId(), resourceMap);
         if (MapUtils.isNotEmpty(vmDetails)) {
             details.put(ApiConstants.VIRTUAL_MACHINE, vmDetails);
         }
diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java
@@ -497,17 +497,19 @@ public void updateAllExtensionHostsHandlesNullClusterId() throws OperationTimedo
 
     @Test
     public void getExternalAccessDetailsReturnsMapWithHostAndExtension() {
-        Map<String, String> actionDetails = new HashMap<>();
-        actionDetails.put("external.detail.key", "value");
+        Map<String, String> map = new HashMap<>();
+        map.put("external.detail.key", "value");
         long hostId = 1L;
         ExtensionResourceMap resourceMap = mock(ExtensionResourceMap.class);
         when(resourceMap.getId()).thenReturn(2L);
         when(resourceMap.getExtensionId()).thenReturn(3L);
-        when(hostDetailsDao.findDetails(hostId)).thenReturn(actionDetails);
-        when(extensionResourceMapDetailsDao.listDetailsKeyPairs(2L, true)).thenReturn(actionDetails);
-        when(extensionDetailsDao.listDetailsKeyPairs(3L, true)).thenReturn(actionDetails);
-        Map<String, Object> result = extensionsManager.getExternalAccessDetails(actionDetails, hostId, resourceMap);
-        assertTrue(result.containsKey(ApiConstants.HOST));
+        when(hostDetailsDao.findDetails(hostId)).thenReturn(null);
+        when(extensionResourceMapDetailsDao.listDetailsKeyPairs(2L, true)).thenReturn(Collections.emptyMap());
+        when(extensionDetailsDao.listDetailsKeyPairs(3L, true)).thenReturn(map);
+        Map<String, Map<String, String>> result = extensionsManager.getExternalAccessDetails(map, hostId, resourceMap);
+        assertTrue(result.containsKey(ApiConstants.ACTION));
+        assertFalse(result.containsKey(ApiConstants.HOST));
+        assertFalse(result.containsKey(ApiConstants.RESOURCE_MAP));
         assertTrue(result.containsKey(ApiConstants.EXTENSION));
     }
 
diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java
@@ -130,7 +130,7 @@ public String getName() {
         return getClass().getSimpleName();
     }
 
-    protected Map<String, Object> loadAccessDetails(Map<String, Object> externalDetails,
+    protected Map<String, Object> loadAccessDetails(Map<String, Map<String, String>> externalDetails,
                             VirtualMachineTO virtualMachineTO) {
         Map<String, Object> modifiedDetails = new HashMap<>();
         if (MapUtils.isNotEmpty(externalDetails)) {
@@ -435,7 +435,8 @@ public Map<String, HostVmStateReportEntry> getHostVmStateReport(long hostId, Str
             logger.debug("No VMs found for the {}", host);
             return vmStates;
         }
-        Map<String, Object> accessDetails = extensionsManager.getExternalAccessDetails(host, null);
+        Map<String, Map<String, String>> accessDetails =
+                extensionsManager.getExternalAccessDetails(host, null);
         for (UserVmVO vm: allVms) {
             VirtualMachine.PowerState powerState = getVmPowerState(vm, accessDetails, extensionName, extensionPath);
             vmStates.put(vm.getInstanceName(), new HostVmStateReportEntry(powerState, "host-" + hostId));
@@ -452,7 +453,6 @@ public RunCustomActionAnswer runCustomAction(String hostGuid, String extensionNa
         }
         final String actionName = cmd.getActionName();
         final Map<String, Object> parameters = cmd.getParameters();
-        final Map<String, Object> externalDetails = cmd.getExternalDetails();
         logger.debug("Executing custom action '{}' in the external provisioner", actionName);
         VirtualMachineTO virtualMachineTO = null;
         if (cmd.getVmId() != null) {
@@ -462,7 +462,7 @@ public RunCustomActionAnswer runCustomAction(String hostGuid, String extensionNa
             virtualMachineTO = hvGuru.implement(profile);
         }
         logger.debug("Executing custom action '{}' in the external system", actionName);
-        Map<String, Object> accessDetails = loadAccessDetails(externalDetails, virtualMachineTO);
+        Map<String, Object> accessDetails = loadAccessDetails(cmd.getExternalDetails(), virtualMachineTO);
         accessDetails.put(ApiConstants.ACTION, actionName);
         if (MapUtils.isNotEmpty(parameters)) {
             accessDetails.put(ApiConstants.PARAMETERS, parameters);
@@ -614,7 +614,7 @@ public Pair<Boolean, String> runCustomActionOnExternalSystem(String extensionNam
                 String.format("Failed to execute custom action '%s' on external system", actionName), filename);
     }
 
-    private VirtualMachine.PowerState getVmPowerState(UserVmVO userVmVO, Map<String, Object> accessDetails,
+    private VirtualMachine.PowerState getVmPowerState(UserVmVO userVmVO, Map<String, Map<String, String>> accessDetails,
                   String extensionName, String extensionPath) {
         final HypervisorGuru hvGuru = hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.External);
         VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVmVO);
diff --git a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java
@@ -173,8 +173,8 @@ private void resetTestScript() {
 
     @Test
     public void testLoadAccessDetails() {
-        Map<String, Object> externalDetails = new HashMap<>();
-        externalDetails.put("key1", "value1");
+        Map<String, Map<String, String>> externalDetails = new HashMap<>();
+        externalDetails.put(ApiConstants.EXTENSION, Map.of("key1", "value1"));
 
         VirtualMachineTO vmTO = mock(VirtualMachineTO.class);
         when(vmTO.getUuid()).thenReturn("test-uuid");

Original file line number	Diff line number	Diff line change
`@@ -1272,7 +1272,7 @@ protected void processPrepareExternalProvisioning(boolean firstStart, Host host,`
`1272`	`1272`	`virtualMachineTO.setNics(nicTOs);`
`1273`	`1273`	`}`
`1274`	`1274`	`Map<String, String> vmDetails = virtualMachineTO.getExternalDetails();`
`1275`		`- Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host,`
	`1275`	`+ Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host,`
`1276`	`1276`	`vmDetails);`
`1277`	`1277`	`PrepareExternalProvisioningCommand cmd = new PrepareExternalProvisioningCommand(virtualMachineTO);`
`1278`	`1278`	`cmd.setExternalDetails(externalDetails);`
`@@ -1677,7 +1677,7 @@ protected void updateStartCommandWithExternalDetails(Host host, VirtualMachineTO`
`1677`	`1677`	`vmExternalDetails.put(VmDetailConstants.CLOUDSTACK_VLAN, Networks.BroadcastDomainType.getValue(nic.getBroadcastUri()));`
`1678`	`1678`	`}`
`1679`	`1679`	`}`
`1680`		`- Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host, vmExternalDetails);`
	`1680`	`+ Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host, vmExternalDetails);`
`1681`	`1681`	`command.setExternalDetails(externalDetails);`
`1682`	`1682`	`}`
`1683`	`1683`
`@@ -1691,7 +1691,16 @@ protected void updateStopCommandForExternalHypervisorType(final HypervisorType h`
`1691`	`1691`	`return;`
`1692`	`1692`	`}`
`1693`	`1693`	`VirtualMachineTO vmTO = ObjectUtils.defaultIfNull(stopCommand.getVirtualMachine(), toVmTO(vmProfile));`
`1694`		`- Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());`
	`1694`	`+ if (MapUtils.isEmpty(vmTO.getGuestOsDetails())) {`
	`1695`	`+ vmTO.setGuestOsDetails(null);`
	`1696`	`+ }`
	`1697`	`+ if (MapUtils.isEmpty(vmTO.getExtraConfig())) {`
	`1698`	`+ vmTO.setExtraConfig(null);`
	`1699`	`+ }`
	`1700`	`+ if (MapUtils.isEmpty(vmTO.getNetworkIdToNetworkNameMap())) {`
	`1701`	`+ vmTO.setNetworkIdToNetworkNameMap(null);`
	`1702`	`+ }`
	`1703`	`+ Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());`
`1695`	`1704`	`stopCommand.setVirtualMachine(vmTO);`
`1696`	`1705`	`stopCommand.setExternalDetails(externalDetails);`
`1697`	`1706`	`}`
`@@ -1700,7 +1709,7 @@ protected void updateRebootCommandWithExternalDetails(Host host, VirtualMachineT`
`1700`	`1709`	`if (!HypervisorType.External.equals(host.getHypervisorType())) {`
`1701`	`1710`	`return;`
`1702`	`1711`	`}`
`1703`		`- Map<String, Object> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());`
	`1712`	`+ Map<String, Map<String, String>> externalDetails = extensionsManager.getExternalAccessDetails(host, vmTO.getExternalDetails());`
`1704`	`1713`	`rebootCmd.setExternalDetails(externalDetails);`
`1705`	`1714`	`}`
`1706`	`1715`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ public interface ExtensionsManager extends Manager {`
`84`	`84`
`85`	`85`	`ExtensionCustomActionResponse createCustomActionResponse(ExtensionCustomAction customAction);`
`86`	`86`
`87`		`- Map<String, Object> getExternalAccessDetails(Host host, Map<String, String> vmDetails);`
	`87`	`+ Map<String, Map<String, String>> getExternalAccessDetails(Host host, Map<String, String> vmDetails);`
`88`	`88`
`89`	`89`	`String handleExtensionServerCommands(ExtensionServerActionBaseCommand cmd);`
`90`	`90`	`}`