nvazquez commented on code in PR #12571:
URL: https://github.com/apache/cloudstack/pull/12571#discussion_r2791047730
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java:
##########
@@ -60,7 +60,8 @@ public class AddHostCmd extends BaseCmd {
@Parameter(name = ApiConstants.POD_ID, type = CommandType.UUID, entityType
= PodResponse.class, required = true, description = "The Pod ID for the host")
private Long podId;
- @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required =
true, description = "The host URL")
+ @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required =
true, description = "The host URL, optionally add ssh port for KVM hosts," +
Review Comment:
```suggestion
@Parameter(name = ApiConstants.URL, type = CommandType.STRING, required
= true, description = "The host URL, optionally add ssh port (format:
'host:port') for KVM hosts," +
```
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -2093,6 +2093,24 @@ public void propagateChangeToAgents(Map<String, String>
params) {
}
}
+ @Override
+ public int getHostSshPort(HostVO host) {
+ if (host == null) {
+ return KVMHostDiscoverySshPort.value();
+ }
+
+ _hostDao.loadDetails(host);
+ String hostPort = host.getDetail(Host.HOST_SSH_POST);
+ int sshPort;
+ if (StringUtils.isBlank(hostPort)) {
+ sshPort = KVMHostDiscoverySshPort.valueIn(host.getClusterId());
Review Comment:
Just to double check, in case the setting does not have a value on the
cluster, will this method return the global set value, or the default value? It
should honor the global value, only in case it is not set either, then use
default value (22)
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -2093,6 +2093,24 @@ public void propagateChangeToAgents(Map<String, String>
params) {
}
}
+ @Override
+ public int getHostSshPort(HostVO host) {
+ if (host == null) {
+ return KVMHostDiscoverySshPort.value();
+ }
+
+ _hostDao.loadDetails(host);
Review Comment:
Should there be any check for the host hypervisor type? I think this should
be only for KVM
--
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]
