From 6fe60d603c0fdd277d7ab2c30fda7ba3ca60185d Mon Sep 17 00:00:00 2001 From: SadiJr Date: Tue, 14 Feb 2023 13:23:28 -0300 Subject: [PATCH 1/4] Validate failure state in Veeam restore process --- .../cloudstack/backup/veeam/VeeamClient.java | 44 ++++++++++++++++++- .../backup/veeam/VeeamClientTest.java | 38 ++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java index 701c45f1a9d0..77d113af09be 100644 --- a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java +++ b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java @@ -345,7 +345,26 @@ private boolean checkTaskStatus(final HttpResponse response) throws IOException String type = pair.second(); String path = url.replace(apiURI.toString(), ""); if (type.equals("RestoreSession")) { - return checkIfRestoreSessionFinished(type, path); + for (int j = 0; j < restoreTimeout; j++) { + HttpResponse relatedResponse = get(path); + RestoreSession session = parseRestoreSessionResponse(relatedResponse); + if (session.getResult().equals("Success")) { + return true; + } + if (session.getResult().equalsIgnoreCase("Failed")) { + String sessionUid = session.getUid(); + LOG.error(String.format("Failed to restore backup [%s] of VM [%s] due to [%s].", + sessionUid, session.getVmDisplayName(), + getRestoreVmErrorDescription(StringUtils.substringAfterLast(sessionUid, ":")))); + throw new CloudRuntimeException(String.format("Restore job [%s] failed.", sessionUid)); + } + LOG.debug(String.format("Waiting %s seconds, out of a total of %s seconds, for the backup process to finish.", j, restoreTimeout)); + try { + Thread.sleep(1000); + } catch (InterruptedException ignored) { + } + } + throw new CloudRuntimeException("Related job type: " + type + " was not successful"); } } return true; @@ -930,6 +949,29 @@ public Pair restoreVMToDifferentLocation(String restorePointId, return new Pair<>(result.first(), restoreLocation); } + /** + * Tries to retrieve the error's descripton of the Veeam restore task that errored. + * @param uid Session uid in Veeam of restore process; + * @return the description found in Veeam about the cause of error in restore process. + */ + protected String getRestoreVmErrorDescription(String uid) { + LOG.debug(String.format("Trying to find cause of error in restore process [%s].", uid)); + List cmds = Arrays.asList( + String.format("$restoreUid = '%s'", uid), + "$restore = Get-VBRRestoreSession -Id $restoreUid", + "if ($restore) {", + "Write-Output $restore.Description", + "} else {", + "Write-Output 'Cannot find restore session with provided uid $restoreUid'", + "}" + ); + Pair result = executePowerShellCommands(cmds); + if (result != null && result.first()) { + return result.second(); + } + return String.format("Failed to get description of failed restore session [%s]. Please contact an administrator.", uid); + } + private boolean isLegacyServer() { return this.veeamServerVersion != null && (this.veeamServerVersion > 0 && this.veeamServerVersion < 11); } diff --git a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java index 06804d68da27..1d0b41687fbe 100644 --- a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java +++ b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java @@ -58,6 +58,8 @@ public class VeeamClientTest { private VeeamClient mockClient; private static final SimpleDateFormat newDateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); + private VeeamClient mock = Mockito.mock(VeeamClient.class); + @Rule public WireMockRule wireMockRule = new WireMockRule(9399); @@ -170,6 +172,42 @@ public void checkIfRestoreSessionFinishedTestTimeoutException() throws IOExcepti Mockito.verify(mockClient, times(10)).get(Mockito.anyString()); } + @Test + public void getRestoreVmErrorDescriptionTestFindErrorDescription() { + Pair response = new Pair<>(true, "Example of error description found in Veeam."); + Mockito.when(mock.getRestoreVmErrorDescription("uuid")).thenCallRealMethod(); + Mockito.when(mock.executePowerShellCommands(Mockito.any())).thenReturn(response); + String result = mock.getRestoreVmErrorDescription("uuid"); + Assert.assertEquals("Example of error description found in Veeam.", result); + } + + @Test + public void getRestoreVmErrorDescriptionTestNotFindErrorDescription() { + Pair response = new Pair<>(true, "Cannot find restore session with provided uid uuid"); + Mockito.when(mock.getRestoreVmErrorDescription("uuid")).thenCallRealMethod(); + Mockito.when(mock.executePowerShellCommands(Mockito.any())).thenReturn(response); + String result = mock.getRestoreVmErrorDescription("uuid"); + Assert.assertEquals("Cannot find restore session with provided uid uuid", result); + } + + @Test + public void getRestoreVmErrorDescriptionTestWhenPowerShellOutputIsNull() { + Mockito.when(mock.getRestoreVmErrorDescription("uuid")).thenCallRealMethod(); + Mockito.when(mock.executePowerShellCommands(Mockito.any())).thenReturn(null); + String result = mock.getRestoreVmErrorDescription("uuid"); + Assert.assertEquals("Failed to get description of failed restore session [uuid]. Please contact an administrator.", result); + } + + @Test + public void getRestoreVmErrorDescriptionTestWhenPowerShellOutputIsFalse() { + Pair response = new Pair<>(false, null); + Mockito.when(mock.getRestoreVmErrorDescription("uuid")).thenCallRealMethod(); + Mockito.when(mock.executePowerShellCommands(Mockito.any())).thenReturn(response); + String result = mock.getRestoreVmErrorDescription("uuid"); + Assert.assertEquals("Failed to get description of failed restore session [uuid]. Please contact an administrator.", result); + } + + private void verifyBackupMetrics(Map metrics) { Assert.assertEquals(2, metrics.size()); From 993cc04b96ab50f0f057b0ce5252a4b511f6b720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 20 Jun 2024 15:11:48 -0300 Subject: [PATCH 2/4] Address Daan review, and properly call method --- .../cloudstack/backup/veeam/VeeamClient.java | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java index 77d113af09be..e1521a949b63 100644 --- a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java +++ b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java @@ -345,26 +345,7 @@ private boolean checkTaskStatus(final HttpResponse response) throws IOException String type = pair.second(); String path = url.replace(apiURI.toString(), ""); if (type.equals("RestoreSession")) { - for (int j = 0; j < restoreTimeout; j++) { - HttpResponse relatedResponse = get(path); - RestoreSession session = parseRestoreSessionResponse(relatedResponse); - if (session.getResult().equals("Success")) { - return true; - } - if (session.getResult().equalsIgnoreCase("Failed")) { - String sessionUid = session.getUid(); - LOG.error(String.format("Failed to restore backup [%s] of VM [%s] due to [%s].", - sessionUid, session.getVmDisplayName(), - getRestoreVmErrorDescription(StringUtils.substringAfterLast(sessionUid, ":")))); - throw new CloudRuntimeException(String.format("Restore job [%s] failed.", sessionUid)); - } - LOG.debug(String.format("Waiting %s seconds, out of a total of %s seconds, for the backup process to finish.", j, restoreTimeout)); - try { - Thread.sleep(1000); - } catch (InterruptedException ignored) { - } - } - throw new CloudRuntimeException("Related job type: " + type + " was not successful"); + checkIfRestoreSessionFinished(type, path); } } return true; @@ -380,17 +361,29 @@ private boolean checkTaskStatus(final HttpResponse response) throws IOException return false; } + + /** + * Checks the status of the restore session. Checked states are "Success" and "Failure".
+ * There is also a timeout defined in the global configuration, backup.plugin.veeam.restore.timeout,
+ * that is used to wait for the restore to complete before throwing a {@link CloudRuntimeException}. + */ protected boolean checkIfRestoreSessionFinished(String type, String path) throws IOException { - for (int j = 0; j < this.restoreTimeout; j++) { + for (int j = 0; j < restoreTimeout; j++) { HttpResponse relatedResponse = get(path); RestoreSession session = parseRestoreSessionResponse(relatedResponse); if (session.getResult().equals("Success")) { return true; } + if (session.getResult().equalsIgnoreCase("Failed")) { String sessionUid = session.getUid(); + LOG.error(String.format("Failed to restore backup [%s] of VM [%s] due to [%s].", + sessionUid, session.getVmDisplayName(), + getRestoreVmErrorDescription(StringUtils.substringAfterLast(sessionUid, ":")))); throw new CloudRuntimeException(String.format("Restore job [%s] failed.", sessionUid)); } + LOG.debug(String.format("Waiting %s seconds, out of a total of %s seconds, for the backup process to finish.", j, restoreTimeout)); + try { Thread.sleep(1000); } catch (InterruptedException ignored) { From d4329971e865d8d1ca0344585db29a34e6a7552e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 20 Jun 2024 15:14:48 -0300 Subject: [PATCH 3/4] Address bryan's reviews --- .../apache/cloudstack/backup/veeam/VeeamClient.java | 12 ++++++------ .../cloudstack/backup/veeam/VeeamClientTest.java | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java index e1521a949b63..7c565c720b58 100644 --- a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java +++ b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java @@ -382,7 +382,7 @@ protected boolean checkIfRestoreSessionFinished(String type, String path) throws getRestoreVmErrorDescription(StringUtils.substringAfterLast(sessionUid, ":")))); throw new CloudRuntimeException(String.format("Restore job [%s] failed.", sessionUid)); } - LOG.debug(String.format("Waiting %s seconds, out of a total of %s seconds, for the backup process to finish.", j, restoreTimeout)); + LOG.debug(String.format("Waiting %s seconds, out of a total of %s seconds, for the restore backup process to finish.", j, restoreTimeout)); try { Thread.sleep(1000); @@ -943,12 +943,12 @@ public Pair restoreVMToDifferentLocation(String restorePointId, } /** - * Tries to retrieve the error's descripton of the Veeam restore task that errored. - * @param uid Session uid in Veeam of restore process; - * @return the description found in Veeam about the cause of error in restore process. + * Tries to retrieve the error's description of the Veeam restore task that resulted in an error. + * @param uid Session uid in Veeam of the restore process; + * @return the description found in Veeam about the cause of error in the restore process. */ protected String getRestoreVmErrorDescription(String uid) { - LOG.debug(String.format("Trying to find cause of error in restore process [%s].", uid)); + LOG.debug(String.format("Trying to find the cause of error in the restore process [%s].", uid)); List cmds = Arrays.asList( String.format("$restoreUid = '%s'", uid), "$restore = Get-VBRRestoreSession -Id $restoreUid", @@ -962,7 +962,7 @@ protected String getRestoreVmErrorDescription(String uid) { if (result != null && result.first()) { return result.second(); } - return String.format("Failed to get description of failed restore session [%s]. Please contact an administrator.", uid); + return String.format("Failed to get the description of the failed restore session [%s]. Please contact an administrator.", uid); } private boolean isLegacyServer() { diff --git a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java index 1d0b41687fbe..2f340471d186 100644 --- a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java +++ b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java @@ -195,7 +195,7 @@ public void getRestoreVmErrorDescriptionTestWhenPowerShellOutputIsNull() { Mockito.when(mock.getRestoreVmErrorDescription("uuid")).thenCallRealMethod(); Mockito.when(mock.executePowerShellCommands(Mockito.any())).thenReturn(null); String result = mock.getRestoreVmErrorDescription("uuid"); - Assert.assertEquals("Failed to get description of failed restore session [uuid]. Please contact an administrator.", result); + Assert.assertEquals("Failed to get the description of the failed restore session [uuid]. Please contact an administrator.", result); } @Test @@ -204,7 +204,7 @@ public void getRestoreVmErrorDescriptionTestWhenPowerShellOutputIsFalse() { Mockito.when(mock.getRestoreVmErrorDescription("uuid")).thenCallRealMethod(); Mockito.when(mock.executePowerShellCommands(Mockito.any())).thenReturn(response); String result = mock.getRestoreVmErrorDescription("uuid"); - Assert.assertEquals("Failed to get description of failed restore session [uuid]. Please contact an administrator.", result); + Assert.assertEquals("Failed to get the description of the failed restore session [uuid]. Please contact an administrator.", result); } From d1acd02355e5b1085b7049160532151dc4d126ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 21 Jun 2024 09:28:59 -0300 Subject: [PATCH 4/4] remove return --- .../java/org/apache/cloudstack/backup/veeam/VeeamClient.java | 4 ++-- .../org/apache/cloudstack/backup/veeam/VeeamClientTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java index 7c565c720b58..befeb231015a 100644 --- a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java +++ b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/veeam/VeeamClient.java @@ -367,12 +367,12 @@ private boolean checkTaskStatus(final HttpResponse response) throws IOException * There is also a timeout defined in the global configuration, backup.plugin.veeam.restore.timeout,
* that is used to wait for the restore to complete before throwing a {@link CloudRuntimeException}. */ - protected boolean checkIfRestoreSessionFinished(String type, String path) throws IOException { + protected void checkIfRestoreSessionFinished(String type, String path) throws IOException { for (int j = 0; j < restoreTimeout; j++) { HttpResponse relatedResponse = get(path); RestoreSession session = parseRestoreSessionResponse(relatedResponse); if (session.getResult().equals("Success")) { - return true; + return; } if (session.getResult().equalsIgnoreCase("Failed")) { diff --git a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java index 2f340471d186..26b2449b0fe4 100644 --- a/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java +++ b/plugins/backup/veeam/src/test/java/org/apache/cloudstack/backup/veeam/VeeamClientTest.java @@ -163,7 +163,7 @@ public void checkIfRestoreSessionFinishedTestTimeoutException() throws IOExcepti Mockito.when(mockClient.get(Mockito.anyString())).thenReturn(httpResponse); Mockito.when(mockClient.parseRestoreSessionResponse(httpResponse)).thenReturn(restoreSession); Mockito.when(restoreSession.getResult()).thenReturn("No Success"); - Mockito.when(mockClient.checkIfRestoreSessionFinished(Mockito.eq("RestoreTest"), Mockito.eq("any"))).thenCallRealMethod(); + Mockito.doCallRealMethod().when(mockClient).checkIfRestoreSessionFinished(Mockito.eq("RestoreTest"), Mockito.eq("any")); mockClient.checkIfRestoreSessionFinished("RestoreTest", "any"); fail(); } catch (Exception e) {