DaanHoogland commented on code in PR #9752:
URL: https://github.com/apache/cloudstack/pull/9752#discussion_r1782344746
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java:
##########
@@ -67,6 +70,9 @@ public class UpdateHostCmd extends BaseCmd {
@Parameter(name = ApiConstants.ANNOTATION, type = CommandType.STRING,
description = "Add an annotation to this host", since = "4.11", authorized =
{RoleType.Admin})
private String annotation;
+ @Parameter(name = ApiConstants.EXTERNAL_DETAILS, type = CommandType.MAP,
description = "Details in key/value pairs using format
externaldetails[i].keyname=keyvalue. Example:
externaldetails[0].endpoint.url=urlvalue", since = "4.21.0")
Review Comment:
`details` instead of external...
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java:
##########
@@ -359,9 +362,21 @@ public Map<String, String> getDetails() {
}
}
}
+
+ detailsMap.putAll(getExternalDetails());
Review Comment:
see? these are just `details`.
##########
core/src/main/java/com/cloud/agent/api/RebootCommand.java:
##########
@@ -46,6 +49,14 @@ public VirtualMachineTO getVirtualMachine() {
return vm;
}
+ public void setDetails(Map<String, String> details) {
+ _details = details;
+ }
+
+ public Map<String, String> getDetails() {
+ return _details;
Review Comment:
```suggestion
return details;
```
##########
core/src/main/java/com/cloud/agent/api/StopCommand.java:
##########
@@ -37,6 +37,9 @@ public class StopCommand extends RebootCommand {
boolean forceStop = false;
private Map<String, DpdkTO> dpdkInterfaceMapping;
Map<String, Boolean> vlanToPersistenceMap;
+ boolean expungeVM = false;
+
+ private Map<String, String> _details;
Review Comment:
```suggestion
private Map<String, String> details;
```
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -1845,7 +1905,19 @@ private List<Map<String, String>>
getVolumesToDisconnect(VirtualMachine vm) {
protected boolean sendStop(final VirtualMachineGuru guru, final
VirtualMachineProfile profile, final boolean force, final boolean
checkBeforeCleanup) {
final VirtualMachine vm = profile.getVirtualMachine();
Map<String, Boolean> vlanToPersistenceMap =
getVlanToPersistenceMapForVM(vm.getId());
+
StopCommand stpCmd = new StopCommand(vm,
getExecuteInSequence(vm.getHypervisorType()), checkBeforeCleanup);
+ if (HypervisorType.External.equals(vm.getHypervisorType())) {
+ Long hostID = profile.getHostId();
+ if (hostID != null) {
+ HostVO host = _hostDao.findById(hostID);
+ HashMap<String, String> accessDetails = new HashMap<>();
+ loadExternalHostAccessDetails(host, accessDetails);
+ loadExternalInstanceDetails(vm.getId(), accessDetails);
+
+ stpCmd.setDetails(accessDetails);
+ }
+ }
Review Comment:
can this be a method too?
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java:
##########
@@ -251,6 +252,8 @@ public class CreateServiceOfferingCmd extends BaseCmd {
since="4.20")
private Boolean purgeResources;
+ @Parameter(name = ApiConstants.EXTERNAL_DETAILS, type = CommandType.MAP,
description = "Details in key/value pairs using format
externaldetails[i].keyname=keyvalue. Example:
externaldetails[0].endpoint.url=urlvalue", since = "4.21.0")
Review Comment:
why do we need those extra external-`details`? aren't these just `details`?
##########
core/src/main/java/com/cloud/agent/api/RebootCommand.java:
##########
@@ -21,9 +21,12 @@
import com.cloud.agent.api.to.VirtualMachineTO;
+import java.util.Map;
+
public class RebootCommand extends Command {
String vmName;
VirtualMachineTO vm;
+ private Map<String, String> _details;
Review Comment:
```suggestion
private Map<String, String> details;
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java:
##########
@@ -75,6 +78,12 @@ public class AddHostCmd extends BaseCmd {
@Parameter(name = ApiConstants.HOST_TAGS, type = CommandType.LIST,
collectionType = CommandType.STRING, description = "list of tags to be added to
the host")
private List<String> hostTags;
+ @Parameter(name = ApiConstants.EXTERNAL_PROVISIONER, type =
CommandType.STRING, description = "Name of the provisioner for the external
host, this is mandatory input in case of hypervisor type external", since =
"4.21.0")
+ private String provisioner;
+
+ @Parameter(name = ApiConstants.EXTERNAL_DETAILS, type = CommandType.MAP,
description = "Details in key/value pairs using format
externaldetails[i].keyname=keyvalue. Example:
externaldetails[0].endpoint.url=urlvalue", since = "4.21.0")
Review Comment:
why are these `externaldetails` and not just `details`
##########
core/src/main/java/com/cloud/agent/api/StopCommand.java:
##########
@@ -138,4 +141,20 @@ public Map<String, Boolean> getVlanToPersistenceMap() {
public void setVlanToPersistenceMap(Map<String, Boolean>
vlanToPersistenceMap) {
this.vlanToPersistenceMap = vlanToPersistenceMap;
}
+
+ public boolean isExpungeVM() {
+ return expungeVM;
+ }
+
+ public void setExpungeVM(boolean expungeVM) {
+ this.expungeVM = expungeVM;
+ }
+
+ public void setDetails(Map<String, String> details) {
+ _details = details;
Review Comment:
```suggestion
this.details = details;
```
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -564,8 +571,8 @@ private void allocateRootVolume(VMInstanceVO vm,
VirtualMachineTemplate template
if (template.getFormat() == ImageFormat.ISO) {
volumeMgr.allocateRawVolume(Type.ROOT, rootVolumeName,
rootDiskOfferingInfo.getDiskOffering(), rootDiskOfferingInfo.getSize(),
rootDiskOfferingInfo.getMinIops(),
rootDiskOfferingInfo.getMaxIops(), vm, template, owner, null);
- } else if (template.getFormat() == ImageFormat.BAREMETAL) {
- logger.debug("%s has format [{}]. Skipping ROOT volume [{}]
allocation.", template.toString(), ImageFormat.BAREMETAL, rootVolumeName);
+ } else if (template.getFormat() == ImageFormat.BAREMETAL ||
template.getFormat() == ImageFormat.EXTERNAL) {
+ logger.debug(String.format("%s has format [%s]. Skipping ROOT
volume [%s] allocation.", template.toString(), template.getFormat(),
rootVolumeName));
Review Comment:
```suggestion
logger.debug("{} has format [{}]. Skipping ROOT volume [{}]
allocation.", template.toString(), template.getFormat(), rootVolumeName);
```
##########
core/src/main/java/com/cloud/agent/api/RebootCommand.java:
##########
@@ -46,6 +49,14 @@ public VirtualMachineTO getVirtualMachine() {
return vm;
}
+ public void setDetails(Map<String, String> details) {
+ _details = details;
Review Comment:
```suggestion
this.details = details;
```
##########
core/src/main/java/com/cloud/agent/api/StopCommand.java:
##########
@@ -138,4 +141,20 @@ public Map<String, Boolean> getVlanToPersistenceMap() {
public void setVlanToPersistenceMap(Map<String, Boolean>
vlanToPersistenceMap) {
this.vlanToPersistenceMap = vlanToPersistenceMap;
}
+
+ public boolean isExpungeVM() {
+ return expungeVM;
+ }
+
+ public void setExpungeVM(boolean expungeVM) {
+ this.expungeVM = expungeVM;
+ }
+
+ public void setDetails(Map<String, String> details) {
+ _details = details;
+ }
+
+ public Map<String, String> getDetails() {
+ return _details;
Review Comment:
```suggestion
return details;
```
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -2173,6 +2245,18 @@ private void advanceStop(final VMInstanceVO vm, final
boolean cleanUpEvenIfUnabl
Map<String, Boolean> vlanToPersistenceMap =
getVlanToPersistenceMapForVM(vm.getId());
final StopCommand stop = new StopCommand(vm,
getExecuteInSequence(vm.getHypervisorType()), false, cleanUpEvenIfUnableToStop);
stop.setControlIp(getControlNicIpForVM(vm));
+
+ if (HypervisorType.External.equals(vm.getHypervisorType())) {
+ Long hostID = profile.getHostId();
+ HostVO host = _hostDao.findById(hostID);
+
+ HashMap<String, String> accessDetails = new HashMap<>();
+ loadExternalHostAccessDetails(host, accessDetails);
+ loadExternalInstanceDetails(vm.getId(), accessDetails);
+
+ stop.setDetails(accessDetails);
+ }
Review Comment:
another method? maybe even unify with the one at line 1910-1920.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
