Copilot commented on code in PR #11601:
URL: https://github.com/apache/cloudstack/pull/11601#discussion_r2336264800
##########
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java:
##########
@@ -311,4 +330,411 @@ public void listConsoleSessionByIdTestShouldCallDbLayer()
{
consoleAccessManager.listConsoleSessionById(1L);
Mockito.verify(consoleSessionDaoMock).findByIdIncludingRemoved(1L);
}
+
+ @Test
+ public void returnsNullWhenAnswerIsNull() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(null);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
##########
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java:
##########
@@ -311,4 +330,411 @@ public void listConsoleSessionByIdTestShouldCallDbLayer()
{
consoleAccessManager.listConsoleSessionById(1L);
Mockito.verify(consoleSessionDaoMock).findByIdIncludingRemoved(1L);
}
+
+ @Test
+ public void returnsNullWhenAnswerIsNull() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(null);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerResultIsFalse() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(false);
+ Mockito.when(answer.getDetails()).thenReturn("Error details");
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
##########
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java:
##########
@@ -311,4 +330,411 @@ public void listConsoleSessionByIdTestShouldCallDbLayer()
{
consoleAccessManager.listConsoleSessionById(1L);
Mockito.verify(consoleSessionDaoMock).findByIdIncludingRemoved(1L);
}
+
+ @Test
+ public void returnsNullWhenAnswerIsNull() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(null);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerResultIsFalse() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(false);
+ Mockito.when(answer.getDetails()).thenReturn("Error details");
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void setsDetailsWhenAnswerIsValid() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ GetExternalConsoleAnswer answer =
Mockito.mock(GetExternalConsoleAnswer.class);
+
+ String expectedHost = "10.0.0.1";
+ int expectedPort = 5900;
+ String expectedPassword = "password";
+
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(answer.getHost()).thenReturn(expectedHost);
+ Mockito.when(answer.getPort()).thenReturn(expectedPort);
+ Mockito.when(answer.getPassword()).thenReturn(expectedPassword);
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNotNull(result);
+ Assert.assertEquals(expectedHost, result.getHost());
+ Assert.assertEquals(expectedPort, result.getPort());
+ Assert.assertEquals(expectedPassword, result.getSid());
+ }
+
+ @Test
+ public void doesNotSetSidWhenPasswordIsBlank() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ GetExternalConsoleAnswer answer =
Mockito.mock(GetExternalConsoleAnswer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(answer.getHost()).thenReturn("10.0.0.1");
+ Mockito.when(answer.getPort()).thenReturn(5900);
+ Mockito.when(answer.getPassword()).thenReturn("");
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -427,67 +425,109 @@ private ConsoleEndpoint generateAccessEndpoint(Long
vmId, String sessionUuid, St
return consoleEndpoint;
}
- private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl,
VirtualMachine vm, HostVO hostVo, String addr,
- String sessionUuid,
String extraSecurityToken) {
- String host = hostVo.getPrivateIpAddress();
-
- Pair<String, Integer> portInfo = null;
- if (hostVo.getHypervisorType() == Hypervisor.HypervisorType.KVM &&
-
(hostVo.getResourceState().equals(ResourceState.ErrorInMaintenance) ||
-
hostVo.getResourceState().equals(ResourceState.ErrorInPrepareForMaintenance))) {
- VMInstanceDetailVO detailAddress =
vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_ADDRESS);
- VMInstanceDetailVO detailPort =
vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_PORT);
- if (detailAddress != null && detailPort != null) {
- portInfo = new Pair<>(detailAddress.getValue(),
Integer.valueOf(detailPort.getValue()));
- } else {
- logger.warn("KVM Host in
ErrorInMaintenance/ErrorInPrepareForMaintenance but " +
- "no VNC Address/Port was available. Falling back to
default one from MS.");
- }
+ protected ConsoleConnectionDetails
getConsoleConnectionDetailsFoxExternalVm(ConsoleConnectionDetails details,
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
```suggestion
protected ConsoleConnectionDetails
getConsoleConnectionDetailsForExternalVm(ConsoleConnectionDetails details,
```
##########
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java:
##########
@@ -311,4 +330,411 @@ public void listConsoleSessionByIdTestShouldCallDbLayer()
{
consoleAccessManager.listConsoleSessionById(1L);
Mockito.verify(consoleSessionDaoMock).findByIdIncludingRemoved(1L);
}
+
+ @Test
+ public void returnsNullWhenAnswerIsNull() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(null);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerResultIsFalse() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(false);
+ Mockito.when(answer.getDetails()).thenReturn("Error details");
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void setsDetailsWhenAnswerIsValid() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ GetExternalConsoleAnswer answer =
Mockito.mock(GetExternalConsoleAnswer.class);
+
+ String expectedHost = "10.0.0.1";
+ int expectedPort = 5900;
+ String expectedPassword = "password";
+
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(answer.getHost()).thenReturn(expectedHost);
+ Mockito.when(answer.getPort()).thenReturn(expectedPort);
+ Mockito.when(answer.getPassword()).thenReturn(expectedPassword);
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
##########
server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java:
##########
@@ -311,4 +330,411 @@ public void listConsoleSessionByIdTestShouldCallDbLayer()
{
consoleAccessManager.listConsoleSessionById(1L);
Mockito.verify(consoleSessionDaoMock).findByIdIncludingRemoved(1L);
}
+
+ @Test
+ public void returnsNullWhenAnswerIsNull() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(null);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerResultIsFalse() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(false);
+ Mockito.when(answer.getDetails()).thenReturn("Error details");
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void returnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() {
+ VirtualMachine vm = Mockito.mock(VirtualMachine.class);
+ HostVO host = Mockito.mock(HostVO.class);
+ ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid",
"en", "tag", "displayName");
+ Answer answer = Mockito.mock(Answer.class);
+
+ Mockito.when(answer.getResult()).thenReturn(true);
+ Mockito.when(managementServer.getExternalVmConsole(vm,
host)).thenReturn(answer);
+
+ ConsoleConnectionDetails result =
consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm,
host);
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -427,67 +425,109 @@ private ConsoleEndpoint generateAccessEndpoint(Long
vmId, String sessionUuid, St
return consoleEndpoint;
}
- private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl,
VirtualMachine vm, HostVO hostVo, String addr,
- String sessionUuid,
String extraSecurityToken) {
- String host = hostVo.getPrivateIpAddress();
-
- Pair<String, Integer> portInfo = null;
- if (hostVo.getHypervisorType() == Hypervisor.HypervisorType.KVM &&
-
(hostVo.getResourceState().equals(ResourceState.ErrorInMaintenance) ||
-
hostVo.getResourceState().equals(ResourceState.ErrorInPrepareForMaintenance))) {
- VMInstanceDetailVO detailAddress =
vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_ADDRESS);
- VMInstanceDetailVO detailPort =
vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_PORT);
- if (detailAddress != null && detailPort != null) {
- portInfo = new Pair<>(detailAddress.getValue(),
Integer.valueOf(detailPort.getValue()));
- } else {
- logger.warn("KVM Host in
ErrorInMaintenance/ErrorInPrepareForMaintenance but " +
- "no VNC Address/Port was available. Falling back to
default one from MS.");
- }
+ protected ConsoleConnectionDetails
getConsoleConnectionDetailsFoxExternalVm(ConsoleConnectionDetails details,
+ VirtualMachine vm, HostVO host) {
+ Answer answer = managementServer.getExternalVmConsole(vm, host);
+ if (answer == null) {
+ logger.error("Unable to get console access details for external {}
on {}: answer is null.", vm, host);
+ return null;
}
-
- if (portInfo == null) {
- portInfo = managementServer.getVncPort(vm);
+ if (!answer.getResult()) {
+ logger.error("Unable to get console access details for external {}
on {}: answer result is false. Reason: {}", vm, host, answer.getDetails());
+ return null;
}
-
- if (logger.isDebugEnabled())
- logger.debug("Port info " + portInfo.first());
-
- Ternary<String, String, String> parsedHostInfo =
parseHostInfo(portInfo.first());
-
- int port = -1;
- if (portInfo.second() == -9) {
- //for hyperv
- port =
Integer.parseInt(managementServer.findDetail(hostVo.getId(),
"rdp.server.port").getValue());
- } else {
- port = portInfo.second();
+ if (!(answer instanceof GetExternalConsoleAnswer)) {
+ logger.error("Unable to get console access details for external {}
on {}: answer is not of type GetExternalConsoleAnswer.", vm, host);
+ return null;
}
+ GetExternalConsoleAnswer getExternalConsoleAnswer =
(GetExternalConsoleAnswer) answer;
+ details.setHost(getExternalConsoleAnswer.getHost());
+ details.setPort(getExternalConsoleAnswer.getPort());
+ if (StringUtils.isNotBlank(getExternalConsoleAnswer.getPassword())) {
+ details.setSid(getExternalConsoleAnswer.getPassword());
+ }
+ return details;
+ }
- String sid = vm.getVncPassword();
- VMInstanceDetailVO details =
vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KEYBOARD);
+ protected Pair<String, Integer>
getHostAndPortForKVMMaintenanceHostIfNeeded(HostVO host,
+ Map<String, String> vmDetails) {
+ if (!Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) {
+ return null;
+ }
+ if(!MAINTENANCE_RESOURCE_STATES.contains(host.getResourceState())) {
+ return null;
+ }
+ String address = vmDetails.get(VmDetailConstants.KVM_VNC_ADDRESS);
+ String port = vmDetails.get(VmDetailConstants.KVM_VNC_PORT);
+ if (ObjectUtils.allNotNull(address, port)) {
+ return new Pair<>(address, Integer.valueOf(port));
+ }
+ logger.warn("KVM Host in
ErrorInMaintenance/ErrorInPrepareForMaintenance but " +
+ "no VNC Address/Port was available. Falling back to default
one from MS.");
+ return null;
+ }
+ protected ConsoleConnectionDetails
getConsoleConnectionDetails(VirtualMachine vm, HostVO host) {
+ String locale = null;
String tag = vm.getUuid();
String displayName = vm.getHostName();
if (vm instanceof UserVm) {
displayName = ((UserVm) vm).getDisplayName();
}
+ Map<String, String> vmDetails =
vmInstanceDetailsDao.listDetailsKeyPairs(vm.getId(),
+ List.of(VmDetailConstants.KEYBOARD,
VmDetailConstants.KVM_VNC_ADDRESS, VmDetailConstants.KVM_VNC_PORT));
+ if (vmDetails.get(VmDetailConstants.KEYBOARD) != null) {
+ locale = vmDetails.get(VmDetailConstants.KEYBOARD);
+ }
+ ConsoleConnectionDetails details = new
ConsoleConnectionDetails(vm.getVncPassword(), locale, tag, displayName);
+ if
(Hypervisor.HypervisorType.External.equals(host.getHypervisorType())) {
+ return getConsoleConnectionDetailsFoxExternalVm(details, vm, host);
Review Comment:
The method name contains a typo: 'Fox' should be 'For'. The method should be
named 'getConsoleConnectionDetailsForExternalVm'.
--
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: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
