Skip to content

Commit d5c642a

Browse files
shwstpprshwstppr
committed
improve log, error
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent 0617ed4 commit d5c642a

File tree

3 files changed

+67
-37
lines changed

3 files changed

+67
-37
lines changed

‎framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1535,7 +1535,10 @@ public Answer getInstanceConsole(VirtualMachine vm, Host host) {
15351535
Extension extension = getExtensionForCluster(host.getClusterId());
15361536
if (extension == null || !Extension.Type.Orchestrator.equals(extension.getType()) ||
15371537
!Extension.State.Enabled.equals(extension.getState())) {
1538-
return new Answer(null, false, "No enabled orchestrator extension found for the host");
1538+
logger.error("No enabled orchestrator {} found for the {} while trying to get console for {}",
1539+
extension == null ? "extension" : extension, host, vm);
1540+
return new Answer(null, false,
1541+
String.format("No enabled orchestrator extension found for the host: %s", host.getName()));
15391542
}
15401543
VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm);
15411544
VirtualMachineTO virtualMachineTO = virtualMachineManager.toVmTO(vmProfile);

‎plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@
9393
import com.cloud.vm.VirtualMachineProfileImpl;
9494
import com.cloud.vm.VmDetailConstants;
9595
import com.cloud.vm.dao.UserVmDao;
96-
import com.cloud.vm.dao.VMInstanceDao;
9796
import com.google.gson.JsonObject;
9897
import com.google.gson.JsonParser;
9998

@@ -116,9 +115,6 @@ public class ExternalPathPayloadProvisioner extends ManagerBase implements Exter
116115
@Inject
117116
HostDao hostDao;
118117

119-
@Inject
120-
VMInstanceDao vmInstanceDao;
121-
122118
@Inject
123119
HypervisorGuruManager hypervisorGuruManager;
124120

@@ -307,12 +303,20 @@ public String getChecksumForExtensionPath(String extensionName, String relativeP
307303
}
308304
}
309305

306+
protected String getExtensionConfigureError(String extensionName, String hostName) {
307+
StringBuilder sb = new StringBuilder("Extension: ").append(extensionName).append(" not configured");
308+
if (StringUtils.isNotBlank(hostName)) {
309+
sb.append(" for host: ").append(hostName);
310+
}
311+
return sb.toString();
312+
}
313+
310314
@Override
311-
public PrepareExternalProvisioningAnswer prepareExternalProvisioning(String hostGuid,
315+
public PrepareExternalProvisioningAnswer prepareExternalProvisioning(String hostName,
312316
String extensionName, String extensionRelativePath, PrepareExternalProvisioningCommand cmd) {
313317
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
314318
if (StringUtils.isEmpty(extensionPath)) {
315-
return new PrepareExternalProvisioningAnswer(cmd, false, "Extension not configured");
319+
return new PrepareExternalProvisioningAnswer(cmd, false, getExtensionConfigureError(extensionName, hostName));
316320
}
317321
VirtualMachineTO vmTO = cmd.getVirtualMachineTO();
318322
String vmUUID = vmTO.getUuid();
@@ -340,11 +344,11 @@ public PrepareExternalProvisioningAnswer prepareExternalProvisioning(String host
340344
}
341345

342346
@Override
343-
public StartAnswer startInstance(String hostGuid, String extensionName, String extensionRelativePath,
347+
public StartAnswer startInstance(String hostName, String extensionName, String extensionRelativePath,
344348
StartCommand cmd) {
345349
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
346350
if (StringUtils.isEmpty(extensionPath)) {
347-
return new StartAnswer(cmd, "Extension not configured");
351+
return new StartAnswer(cmd, getExtensionConfigureError(extensionName, hostName));
348352
}
349353
VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine();
350354
Map<String, Object> accessDetails = loadAccessDetails(cmd.getExternalDetails(), virtualMachineTO);
@@ -384,11 +388,11 @@ private Pair<Boolean, String> executeStartCommandOnExternalSystem(String extensi
384388
}
385389

386390
@Override
387-
public StopAnswer stopInstance(String hostGuid, String extensionName, String extensionRelativePath,
391+
public StopAnswer stopInstance(String hostName, String extensionName, String extensionRelativePath,
388392
StopCommand cmd) {
389393
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
390394
if (StringUtils.isEmpty(extensionPath)) {
391-
return new StopAnswer(cmd, "Extension not configured", false);
395+
return new StopAnswer(cmd, getExtensionConfigureError(extensionName, hostName), false);
392396
}
393397
logger.debug("Executing stop command on the external provisioner");
394398
VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine();
@@ -405,11 +409,11 @@ public StopAnswer stopInstance(String hostGuid, String extensionName, String ext
405409
}
406410

407411
@Override
408-
public RebootAnswer rebootInstance(String hostGuid, String extensionName, String extensionRelativePath,
412+
public RebootAnswer rebootInstance(String hostName, String extensionName, String extensionRelativePath,
409413
RebootCommand cmd) {
410414
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
411415
if (StringUtils.isEmpty(extensionPath)) {
412-
return new RebootAnswer(cmd, "Extension not configured", false);
416+
return new RebootAnswer(cmd, getExtensionConfigureError(extensionName, hostName), false);
413417
}
414418
VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine();
415419
String vmUUID = virtualMachineTO.getUuid();
@@ -425,11 +429,11 @@ public RebootAnswer rebootInstance(String hostGuid, String extensionName, String
425429
}
426430

427431
@Override
428-
public StopAnswer expungeInstance(String hostGuid, String extensionName, String extensionRelativePath,
432+
public StopAnswer expungeInstance(String hostName, String extensionName, String extensionRelativePath,
429433
StopCommand cmd) {
430434
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
431435
if (StringUtils.isEmpty(extensionPath)) {
432-
return new StopAnswer(cmd, "Extension not configured", false);
436+
return new StopAnswer(cmd, getExtensionConfigureError(extensionName, hostName), false);
433437
}
434438
VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine();
435439
String vmUUID = virtualMachineTO.getUuid();
@@ -473,11 +477,11 @@ public Map<String, HostVmStateReportEntry> getHostVmStateReport(long hostId, Str
473477
}
474478

475479
@Override
476-
public GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String extensionName,
480+
public GetExternalConsoleAnswer getInstanceConsole(String hostName, String extensionName,
477481
String extensionRelativePath, GetExternalConsoleCommand cmd) {
478482
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
479483
if (StringUtils.isEmpty(extensionPath)) {
480-
return new GetExternalConsoleAnswer(cmd, "Extension not configured");
484+
return new GetExternalConsoleAnswer(cmd, getExtensionConfigureError(extensionName, hostName));
481485
}
482486
VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine();
483487
String vmUUID = virtualMachineTO.getUuid();
@@ -519,11 +523,11 @@ public GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String exten
519523
}
520524

521525
@Override
522-
public RunCustomActionAnswer runCustomAction(String hostGuid, String extensionName,
526+
public RunCustomActionAnswer runCustomAction(String hostName, String extensionName,
523527
String extensionRelativePath, RunCustomActionCommand cmd) {
524528
String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath);
525529
if (StringUtils.isEmpty(extensionPath)) {
526-
return new RunCustomActionAnswer(cmd, false, "Extension not configured");
530+
return new RunCustomActionAnswer(cmd, false, getExtensionConfigureError(extensionName, hostName));
527531
}
528532
final String actionName = cmd.getActionName();
529533
final Map<String, Object> parameters = cmd.getParameters();

‎plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ public void testPrepareExternalProvisioning() {
314314
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
315315

316316
PrepareExternalProvisioningAnswer answer = provisioner.prepareExternalProvisioning(
317-
"host-guid", "test-extension", "test-extension.sh", cmd);
317+
"host-name", "test-extension", "test-extension.sh", cmd);
318318

319319
assertTrue(answer.getResult());
320320
assertEquals("test-net-uuid", answer.getVirtualMachineTO().getNics()[0].getNetworkUuid());
@@ -326,11 +326,14 @@ public void testPrepareExternalProvisioning() {
326326
public void testPrepareExternalProvisioning_ExtensionNotConfigured() {
327327
PrepareExternalProvisioningCommand cmd = mock(PrepareExternalProvisioningCommand.class);
328328

329+
String extensionName = "test-extension";
330+
String hostName = "host-name";
329331
PrepareExternalProvisioningAnswer answer = provisioner.prepareExternalProvisioning(
330-
"host-guid", "test-extension", "nonexistent.sh", cmd);
332+
hostName, extensionName, "nonexistent.sh", cmd);
331333

332334
assertFalse(answer.getResult());
333-
assertEquals("Extension not configured", answer.getDetails());
335+
assertNotNull(answer);
336+
assertEquals(String.format("Extension: %s not configured for host: %s", extensionName, hostName), answer.getDetails());
334337
}
335338

336339
@Test
@@ -345,7 +348,7 @@ public void testStartInstance() {
345348
doReturn(new Pair<>(true, "{\"status\": \"success\", \"message\": \"Instance started\"}")).when(provisioner)
346349
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
347350

348-
StartAnswer answer = provisioner.startInstance("host-guid", "test-extension", "test-extension.sh", cmd);
351+
StartAnswer answer = provisioner.startInstance("host-name", "test-extension", "test-extension.sh", cmd);
349352

350353
assertTrue(answer.getResult());
351354
Mockito.verify(logger).debug("Starting VM test-uuid on the external system");
@@ -366,7 +369,7 @@ public void testStartInstanceDeploy() {
366369
doReturn(new Pair<>(true, "{\"status\": \"success\", \"message\": \"Instance started\"}")).when(provisioner)
367370
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
368371

369-
StartAnswer answer = provisioner.startInstance("host-guid", "test-extension", "test-extension.sh", cmd);
372+
StartAnswer answer = provisioner.startInstance("host-name", "test-extension", "test-extension.sh", cmd);
370373

371374
assertTrue(answer.getResult());
372375
Mockito.verify(logger).debug("Deploying VM test-uuid on the external system");
@@ -384,7 +387,7 @@ public void testStartInstanceError() {
384387
doReturn(new Pair<>(false, "{\"error\": \"Instance failed to start\"}")).when(provisioner)
385388
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
386389

387-
StartAnswer answer = provisioner.startInstance("host-guid", "test-extension", "test-extension.sh", cmd);
390+
StartAnswer answer = provisioner.startInstance("host-name", "test-extension", "test-extension.sh", cmd);
388391

389392
assertFalse(answer.getResult());
390393
assertEquals("{\"error\": \"Instance failed to start\"}", answer.getDetails());
@@ -403,7 +406,7 @@ public void testStopInstance() {
403406
doReturn(new Pair<>(true, "success")).when(provisioner)
404407
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
405408

406-
StopAnswer answer = provisioner.stopInstance("host-guid", "test-extension", "test-extension.sh", cmd);
409+
StopAnswer answer = provisioner.stopInstance("host-name", "test-extension", "test-extension.sh", cmd);
407410

408411
assertTrue(answer.getResult());
409412
}
@@ -420,7 +423,7 @@ public void testRebootInstance() {
420423
doReturn(new Pair<>(true, "success")).when(provisioner)
421424
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
422425

423-
RebootAnswer answer = provisioner.rebootInstance("host-guid", "test-extension", "test-extension.sh", cmd);
426+
RebootAnswer answer = provisioner.rebootInstance("host-name", "test-extension", "test-extension.sh", cmd);
424427

425428
assertTrue(answer.getResult());
426429
}
@@ -437,7 +440,7 @@ public void testExpungeInstance() {
437440
doReturn(new Pair<>(true, "success")).when(provisioner)
438441
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
439442

440-
StopAnswer answer = provisioner.expungeInstance("host-guid", "test-extension", "test-extension.sh", cmd);
443+
StopAnswer answer = provisioner.expungeInstance("host-name", "test-extension", "test-extension.sh", cmd);
441444

442445
assertTrue(answer.getResult());
443446
}
@@ -490,7 +493,7 @@ public void testRunCustomAction() {
490493
doReturn(new Pair<>(true, "success")).when(provisioner)
491494
.executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString());
492495

493-
RunCustomActionAnswer answer = provisioner.runCustomAction("host-guid", "test-extension", "test-extension.sh", cmd);
496+
RunCustomActionAnswer answer = provisioner.runCustomAction("host-name", "test-extension", "test-extension.sh", cmd);
494497

495498
assertTrue(answer.getResult());
496499
Mockito.verify(logger).debug("Executing custom action '{}' in the external system", "test-action");
@@ -770,7 +773,7 @@ public void getInstanceConsoleReturnsAnswerWhenConsoleDetailsAreValid() {
770773
doReturn(new Pair<>(true, validOutput)).when(provisioner)
771774
.getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt());
772775

773-
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd);
776+
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd);
774777

775778
assertNotNull(result);
776779
assertEquals("127.0.0.1", result.getHost());
@@ -784,11 +787,13 @@ public void getInstanceConsoleReturnsErrorWhenExtensionNotConfigured() {
784787
GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class);
785788
when(provisioner.getExtensionCheckedPath(anyString(), anyString())).thenReturn(null);
786789

787-
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid",
788-
"test-extension", "test-extension.sh", cmd);
790+
String extensionName = "test-extension";
791+
String hostName = "host-name";
792+
GetExternalConsoleAnswer result = provisioner.getInstanceConsole(hostName,
793+
extensionName, "test-extension.sh", cmd);
789794

790795
assertNotNull(result);
791-
assertEquals("Extension not configured", result.getDetails());
796+
assertEquals(String.format("Extension: %s not configured for host: %s", extensionName, hostName), result.getDetails());
792797
}
793798

794799
@Test
@@ -801,7 +806,7 @@ public void getInstanceConsoleReturnsErrorWhenExternalSystemFails() {
801806
doReturn(new Pair<>(false, "External system error")).when(provisioner)
802807
.getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt());
803808

804-
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd);
809+
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd);
805810

806811
assertNotNull(result);
807812
assertEquals("External system error", result.getDetails());
@@ -818,7 +823,7 @@ public void getInstanceConsoleReturnsErrorWhenConsoleObjectIsMissing() {
818823
doReturn(new Pair<>(true, invalidOutput)).when(provisioner)
819824
.getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt());
820825

821-
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd);
826+
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd);
822827

823828
assertNotNull(result);
824829
assertEquals("Missing console object in output", result.getDetails());
@@ -835,7 +840,7 @@ public void getInstanceConsoleReturnsErrorWhenRequiredFieldsAreMissing() {
835840
doReturn(new Pair<>(true, incompleteOutput)).when(provisioner)
836841
.getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt());
837842

838-
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd);
843+
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd);
839844

840845
assertNotNull(result);
841846
assertEquals("Missing required fields in output", result.getDetails());
@@ -852,7 +857,7 @@ public void getInstanceConsoleReturnsErrorWhenOutputParsingFails() {
852857
doReturn(new Pair<>(true, malformedOutput)).when(provisioner)
853858
.getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt());
854859

855-
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd);
860+
GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd);
856861

857862
assertNotNull(result);
858863
assertEquals("Failed to parse output", result.getDetails());
@@ -947,4 +952,22 @@ public void getSanitizedJsonStringForLogHandlesMalformedJsonGracefully() {
947952
String result = provisioner.getSanitizedJsonStringForLog(json);
948953
assertEquals("{password:\"secret\"", result);
949954
}
955+
956+
@Test
957+
public void getExtensionConfigureErrorReturnsMessageWhenHostNameIsNotBlank() {
958+
String result = provisioner.getExtensionConfigureError("test-extension", "test-host");
959+
assertEquals("Extension: test-extension not configured for host: test-host", result);
960+
}
961+
962+
@Test
963+
public void getExtensionConfigureErrorReturnsMessageWhenHostNameIsBlank() {
964+
String result = provisioner.getExtensionConfigureError("test-extension", "");
965+
assertEquals("Extension: test-extension not configured", result);
966+
}
967+
968+
@Test
969+
public void getExtensionConfigureErrorReturnsMessageWhenHostNameIsNull() {
970+
String result = provisioner.getExtensionConfigureError("test-extension", null);
971+
assertEquals("Extension: test-extension not configured", result);
972+
}
950973
}

0 commit comments

Comments
 (0)