diff --git a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java index 34f8d7b8bcdd..7c2d3fc76fc1 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java @@ -33,6 +33,7 @@ public class TakeBackupCommand extends Command { private List volumePools; private List volumePaths; private Boolean quiesce; + private Integer quiesceTimeout; @LogLevel(LogLevel.Log4jLevel.Off) private String mountOptions; @@ -117,6 +118,14 @@ public void setQuiesce(Boolean quiesce) { this.quiesce = quiesce; } + public Integer getQuiesceTimeout() { + return quiesceTimeout; + } + + public void setQuiesceTimeout(Integer quiesceTimeout) { + this.quiesceTimeout = quiesceTimeout; + } + public String getMode() { return mode; } diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index 08c54100bc22..20f02ca4c48e 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -117,6 +117,15 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co ConfigKey.Scope.Zone, NASBackupIncrementalEnabled.key()); + ConfigKey NASBackupQuiesceAgentTimeout = new ConfigKey<>("Advanced", Integer.class, + "nas.backup.quiesce.agent.timeout", + "30", + "Timeout in seconds for the guest agent filesystem freeze and thaw commands of a quiesced NAS backup. " + + "Set to 0 to use the libvirt default (5 seconds unless changed on the host).", + true, + ConfigKey.Scope.Zone, + BackupFrameworkEnabled.key()); + @Inject private BackupDao backupDao; @@ -578,6 +587,7 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce command.setBackupRepoAddress(backupRepository.getAddress()); command.setMountOptions(backupRepository.getMountOptions()); command.setQuiesce(quiesceVM); + command.setQuiesceTimeout(NASBackupQuiesceAgentTimeout.valueIn(vm.getDataCenterId())); command.setMode(decision.mode); command.setBitmapNew(decision.bitmapNew); command.setBitmapParent(decision.bitmapParent); @@ -645,10 +655,11 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce logger.error("Backup cleanup failed for VM {}. Leaving the backup in Error state. Backup should be manually deleted to free up the space", vm.getInstanceName()); backupVO.setStatus(Backup.Status.Error); backupDao.update(backupVO.getId(), backupVO); - } else { - backupVO.setStatus(Backup.Status.Failed); - backupDao.remove(backupVO.getId()); + // Return the row to the caller to record its schedule - e.g. Scheduled or Manual. + return new Pair<>(false, backupVO); } + backupVO.setStatus(Backup.Status.Failed); + backupDao.remove(backupVO.getId()); return new Pair<>(false, null); } } @@ -1254,7 +1265,8 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[]{ NASBackupRestoreMountTimeout, NASBackupFullEvery, - NASBackupIncrementalEnabled + NASBackupIncrementalEnabled, + NASBackupQuiesceAgentTimeout }; } diff --git a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java index 7c5edba1e2b6..52b64d7df412 100644 --- a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java +++ b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java @@ -202,16 +202,7 @@ public void testGetBackupStorageStats() { Assert.assertEquals(Long.valueOf(3000L), result.second()); } - @Test - public void takeBackupSuccessfully() throws AgentUnavailableException, OperationTimedoutException { - Long vmId = 1L; - Long hostId = 2L; - Long backupOfferingId = 3L; - Long accountId = 4L; - Long domainId = 5L; - Long zoneId = 6L; - Long backupId = 7L; - + private VMInstanceVO mockRunningVmForBackup(Long vmId, Long hostId, Long backupOfferingId, Long accountId, Long domainId, Long zoneId) { VMInstanceVO vm = mock(VMInstanceVO.class); Mockito.when(vm.getId()).thenReturn(vmId); Mockito.when(vm.getHostId()).thenReturn(hostId); @@ -241,6 +232,16 @@ public void takeBackupSuccessfully() throws AgentUnavailableException, Operation Mockito.when(volume2.getState()).thenReturn(Volume.State.Ready); Mockito.when(volume2.getSize()).thenReturn(200L); Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume1, volume2)); + return vm; + } + + @Test + public void takeBackupSuccessfully() throws AgentUnavailableException, OperationTimedoutException { + Long backupOfferingId = 3L; + Long accountId = 4L; + Long domainId = 5L; + Long zoneId = 6L; + VMInstanceVO vm = mockRunningVmForBackup(1L, 2L, backupOfferingId, accountId, domainId, zoneId); BackupAnswer answer = mock(BackupAnswer.class); Mockito.when(answer.getResult()).thenReturn(true); @@ -269,6 +270,48 @@ public void takeBackupSuccessfully() throws AgentUnavailableException, Operation Mockito.verify(agentManager).send(anyLong(), Mockito.any(TakeBackupCommand.class)); } + @Test + public void takeBackupFailureNeedingCleanupReturnsErrorBackup() throws AgentUnavailableException, OperationTimedoutException { + VMInstanceVO vm = mockRunningVmForBackup(1L, 2L, 3L, 4L, 5L, 6L); + + BackupAnswer answer = mock(BackupAnswer.class); + Mockito.when(answer.getResult()).thenReturn(false); + Mockito.when(answer.getNeedsCleanup()).thenReturn(true); + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(TakeBackupCommand.class); + Mockito.when(agentManager.send(anyLong(), commandCaptor.capture())).thenReturn(answer); + + Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + Pair result = nasBackupProvider.takeBackup(vm, true); + + Assert.assertFalse(result.first()); + Assert.assertNotNull(result.second()); + Assert.assertEquals(Backup.Status.Error, result.second().getStatus()); + Mockito.verify(backupDao, Mockito.never()).remove(Mockito.anyLong()); + + TakeBackupCommand command = commandCaptor.getValue(); + Assert.assertTrue(command.getQuiesce()); + Assert.assertEquals(Integer.valueOf(30), command.getQuiesceTimeout()); + } + + @Test + public void takeBackupFailureWithoutCleanupRemovesBackup() throws AgentUnavailableException, OperationTimedoutException { + VMInstanceVO vm = mockRunningVmForBackup(1L, 2L, 3L, 4L, 5L, 6L); + + BackupAnswer answer = mock(BackupAnswer.class); + Mockito.when(answer.getResult()).thenReturn(false); + Mockito.when(answer.getNeedsCleanup()).thenReturn(false); + Mockito.when(agentManager.send(anyLong(), Mockito.any(TakeBackupCommand.class))).thenReturn(answer); + + Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + Pair result = nasBackupProvider.takeBackup(vm, false); + + Assert.assertFalse(result.first()); + Assert.assertNull(result.second()); + Mockito.verify(backupDao).remove(Mockito.anyLong()); + } + @Test public void testGetVMHypervisorHost() { Long hostId = 1L; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java index e76c9a2a3871..b7e609587650 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java @@ -143,6 +143,7 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo TakeBackupCommand command, String vmName, String backupRepoType, String backupRepoAddress, String mountOptions, String backupPath, List diskPaths, String mode, String bitmapNew, String bitmapParent, List parentPaths, int timeout) { + boolean quiesce = command.getQuiesce() != null && command.getQuiesce(); List argv = new ArrayList<>(Arrays.asList( libvirtComputingResource.getNasBackupPath(), "-o", "backup", @@ -151,9 +152,14 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo "-s", backupRepoAddress, "-m", Objects.nonNull(mountOptions) ? mountOptions : "", "-p", backupPath, - "-q", command.getQuiesce() != null && command.getQuiesce() ? "true" : "false", + "-q", quiesce ? "true" : "false", "-d", CollectionUtils.isEmpty(diskPaths) ? "" : String.join(",", diskPaths) )); + Integer quiesceTimeout = command.getQuiesceTimeout(); + if (quiesce && quiesceTimeout != null && quiesceTimeout > 0) { + argv.add("--quiesce-timeout"); + argv.add(String.valueOf(quiesceTimeout)); + } if (StringUtils.isNotBlank(mode)) { argv.add("-M"); argv.add(mode); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java new file mode 100644 index 000000000000..7ef64fd53b50 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapperTest.java @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource.wrapper; + +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.List; + +import org.apache.cloudstack.backup.BackupAnswer; +import org.apache.cloudstack.backup.TakeBackupCommand; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.utils.Pair; +import com.cloud.utils.script.Script; + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtTakeBackupCommandWrapperTest { + + private LibvirtTakeBackupCommandWrapper wrapper; + private LibvirtComputingResource libvirtComputingResource; + private TakeBackupCommand command; + + @Before + public void setUp() { + wrapper = new LibvirtTakeBackupCommandWrapper(); + libvirtComputingResource = Mockito.mock(LibvirtComputingResource.class); + when(libvirtComputingResource.getNasBackupPath()).thenReturn("nasbackup.sh"); + command = new TakeBackupCommand("i-2-3-VM", "i-2-3-VM/2026.10.04.00.00.00"); + command.setBackupRepoType("nfs"); + command.setBackupRepoAddress("10.0.0.1:/backup"); + command.setWait(60); + } + + @SuppressWarnings("unchecked") + private List runAndCaptureArgv() { + try (MockedStatic