Skip to content

Commit 7f4e6a9

Browse files
Pearl1594Pearl1594
authored
NAS B&R Plugin enhancements (#9666)
* NAS B&R Plugin enhancements * Prevent printing mount opts which may include password by removing from response * revert marvin change * add sanity checks to validate minimum qemu and libvirt versions * check is user running script is part of libvirt group * revert changes of retore expunged VM * add code coverage ignore file * remove check * issue with listing schedules and add defensive checks * redirect logs to agent log file * add some more debugging * remove test file * prevent deletion of cks cluster when vms associated to a backup offering * delete all snapshot policies when bkp offering is disassociated from a VM * Fix `updateTemplatePermission` when the UI is set to a language other than English (#9766) * Fix updateTemplatePermission UI in non-english language * Improve fix --------- * Add nobrl in the mountopts for cifs file system * Fix restoration of VM / volumes with cifs * add cifs utils for el8 * add cifs-utils for ubuntu cloudstack-agent * syntax error * remove required constraint on both vmid and id params for the delete bkp schedule command
1 parent a6b1403 commit 7f4e6a9

File tree

12 files changed

+155
-42
lines changed

12 files changed

+155
-42
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/user/backup/DeleteBackupScheduleCmd.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.cloudstack.api.BaseCmd;
2727
import org.apache.cloudstack.api.Parameter;
2828
import org.apache.cloudstack.api.ServerApiException;
29+
import org.apache.cloudstack.api.response.BackupScheduleResponse;
2930
import org.apache.cloudstack.api.response.SuccessResponse;
3031
import org.apache.cloudstack.api.response.UserVmResponse;
3132
import org.apache.cloudstack.backup.BackupManager;
@@ -54,10 +55,16 @@ public class DeleteBackupScheduleCmd extends BaseCmd {
5455
@Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID,
5556
type = CommandType.UUID,
5657
entityType = UserVmResponse.class,
57-
required = true,
5858
description = "ID of the VM")
5959
private Long vmId;
6060

61+
@Parameter(name = ApiConstants.ID,
62+
type = CommandType.UUID,
63+
entityType = BackupScheduleResponse.class,
64+
description = "ID of the schedule",
65+
since = "4.20.1")
66+
private Long id;
67+
6168
/////////////////////////////////////////////////////
6269
/////////////////// Accessors ///////////////////////
6370
/////////////////////////////////////////////////////
@@ -66,14 +73,17 @@ public Long getVmId() {
6673
return vmId;
6774
}
6875

76+
public Long getId() { return id; }
77+
78+
6979
/////////////////////////////////////////////////////
7080
/////////////// API Implementation///////////////////
7181
/////////////////////////////////////////////////////
7282

7383
@Override
7484
public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
7585
try {
76-
boolean result = backupManager.deleteBackupSchedule(getVmId());
86+
boolean result = backupManager.deleteBackupSchedule(this);
7787
if (result) {
7888
SuccessResponse response = new SuccessResponse(getCommandName());
7989
response.setResponseName(getCommandName());

‎api/src/main/java/org/apache/cloudstack/api/response/BackupRepositoryResponse.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,6 @@ public class BackupRepositoryResponse extends BaseResponse {
5757
@Param(description = "backup type")
5858
private String type;
5959

60-
@SerializedName(ApiConstants.MOUNT_OPTIONS)
61-
@Param(description = "mount options for the backup repository")
62-
private String mountOptions;
63-
6460
@SerializedName(ApiConstants.CAPACITY_BYTES)
6561
@Param(description = "capacity of the backup repository")
6662
private Long capacityBytes;
@@ -112,14 +108,6 @@ public void setAddress(String address) {
112108
this.address = address;
113109
}
114110

115-
public String getMountOptions() {
116-
return mountOptions;
117-
}
118-
119-
public void setMountOptions(String mountOptions) {
120-
this.mountOptions = mountOptions;
121-
}
122-
123111
public String getProviderName() {
124112
return providerName;
125113
}

‎api/src/main/java/org/apache/cloudstack/backup/BackupManager.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.cloudstack.api.command.admin.backup.ImportBackupOfferingCmd;
2323
import org.apache.cloudstack.api.command.admin.backup.UpdateBackupOfferingCmd;
2424
import org.apache.cloudstack.api.command.user.backup.CreateBackupScheduleCmd;
25+
import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd;
2526
import org.apache.cloudstack.api.command.user.backup.ListBackupOfferingsCmd;
2627
import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd;
2728
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -111,10 +112,10 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer
111112

112113
/**
113114
* Deletes VM backup schedule for a VM
114-
* @param vmId
115+
* @param cmd
115116
* @return
116117
*/
117-
boolean deleteBackupSchedule(Long vmId);
118+
boolean deleteBackupSchedule(DeleteBackupScheduleCmd cmd);
118119

119120
/**
120121
* Creates backup of a VM

‎packaging/el8/cloud.spec‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ Requires: iproute
114114
Requires: ipset
115115
Requires: perl
116116
Requires: rsync
117+
Requires: cifs-utils
117118
Requires: (python3-libvirt or python3-libvirt-python)
118119
Requires: (qemu-img or qemu-tools)
119120
Requires: qemu-kvm

‎plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
221221
restoreCommand.setBackupPath(backup.getExternalId());
222222
restoreCommand.setBackupRepoType(backupRepository.getType());
223223
restoreCommand.setBackupRepoAddress(backupRepository.getAddress());
224+
restoreCommand.setMountOptions(backupRepository.getMountOptions());
224225
restoreCommand.setVmName(vm.getName());
225226
restoreCommand.setVolumePaths(getVolumePaths(volumes));
226227
restoreCommand.setVmExists(vm.getRemoved() == null);
@@ -289,6 +290,7 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
289290
restoreCommand.setVmName(vmNameAndState.first());
290291
restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID)));
291292
restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT));
293+
restoreCommand.setMountOptions(backupRepository.getMountOptions());
292294
restoreCommand.setVmExists(null);
293295
restoreCommand.setVmState(vmNameAndState.second());
294296
restoreCommand.setRestoreVolumeUUID(volumeUuid);
@@ -373,8 +375,12 @@ public Map<VirtualMachine, Backup.Metric> getBackupMetrics(Long zoneId, List<Vir
373375
Long vmBackupSize = 0L;
374376
Long vmBackupProtectedSize = 0L;
375377
for (final Backup backup: backupDao.listByVmId(null, vm.getId())) {
376-
vmBackupSize += backup.getSize();
377-
vmBackupProtectedSize += backup.getProtectedSize();
378+
if (Objects.nonNull(backup.getSize())) {
379+
vmBackupSize += backup.getSize();
380+
}
381+
if (Objects.nonNull(backup.getProtectedSize())) {
382+
vmBackupProtectedSize += backup.getProtectedSize();
383+
}
378384
}
379385
Backup.Metric vmBackupMetric = new Backup.Metric(vmBackupSize,vmBackupProtectedSize);
380386
LOG.debug("Metrics for VM {} is [backup size: {}, data size: {}].", vm, vmBackupMetric.getBackupSize(), vmBackupMetric.getDataSize());

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser
6767
int lastIndex = volumePath.lastIndexOf("/");
6868
newVolumeId = volumePath.substring(lastIndex + 1);
6969
restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, restoreVolumeUuid,
70-
new Pair<>(vmName, command.getVmState()));
70+
new Pair<>(vmName, command.getVmState()), mountOptions);
7171
} else if (Boolean.TRUE.equals(vmExists)) {
7272
restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions);
7373
} else {
@@ -80,7 +80,7 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser
8080
private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupPath,
8181
String backupRepoType, String backupRepoAddress, String mountOptions) {
8282
String diskType = "root";
83-
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
83+
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
8484
try {
8585
for (int idx = 0; idx < volumePaths.size(); idx++) {
8686
String volumePath = volumePaths.get(idx);
@@ -101,7 +101,7 @@ private void restoreVolumesOfExistingVM(List<String> volumePaths, String backupP
101101

102102
private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmName, String backupPath,
103103
String backupRepoType, String backupRepoAddress, String mountOptions) {
104-
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
104+
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
105105
String diskType = "root";
106106
try {
107107
for (int i = 0; i < volumePaths.size(); i++) {
@@ -121,8 +121,8 @@ private void restoreVolumesOfDestroyedVMs(List<String> volumePaths, String vmNam
121121
}
122122

123123
private void restoreVolume(String backupPath, String backupRepoType, String backupRepoAddress, String volumePath,
124-
String diskType, String volumeUUID, Pair<String, VirtualMachine.State> vmNameAndState) {
125-
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType);
124+
String diskType, String volumeUUID, Pair<String, VirtualMachine.State> vmNameAndState, String mountOptions) {
125+
String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions);
126126
Pair<String, String> bkpPathAndVolUuid;
127127
try {
128128
bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, volumeUUID);
@@ -145,12 +145,22 @@ private void restoreVolume(String backupPath, String backupRepoType, String back
145145
}
146146

147147

148-
private String mountBackupDirectory(String backupRepoAddress, String backupRepoType) {
148+
private String mountBackupDirectory(String backupRepoAddress, String backupRepoType, String mountOptions) {
149149
String randomChars = RandomStringUtils.random(5, true, false);
150150
String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIX , randomChars);
151151
try {
152152
mountDirectory = Files.createTempDirectory(mountDirectory).toString();
153+
String mountOpts = null;
154+
if (Objects.nonNull(mountOptions)) {
155+
mountOpts = mountOptions;
156+
if ("cifs".equals(backupRepoType)) {
157+
mountOpts += ",nobrl";
158+
}
159+
}
153160
String mount = String.format(MOUNT_COMMAND, backupRepoType, backupRepoAddress, mountDirectory);
161+
if (Objects.nonNull(mountOpts)) {
162+
mount += " -o " + mountOpts;
163+
}
154164
Script.runSimpleBashScript(mount);
155165
} catch (Exception e) {
156166
throw new CloudRuntimeException(String.format("Failed to mount %s to %s", backupRepoType, backupRepoAddress), e);

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.concurrent.Executors;
3737
import java.util.concurrent.ScheduledExecutorService;
3838
import java.util.concurrent.TimeUnit;
39+
import java.util.stream.Collectors;
3940

4041
import javax.inject.Inject;
4142
import javax.naming.ConfigurationException;
@@ -1467,6 +1468,10 @@ public boolean deleteKubernetesCluster(DeleteKubernetesClusterCmd cmd) throws Cl
14671468
}
14681469

14691470
List<KubernetesClusterVmMapVO> vmMapList = kubernetesClusterVmMapDao.listByClusterId(kubernetesClusterId);
1471+
List<VMInstanceVO> vms = vmMapList.stream().map(vmMap -> vmInstanceDao.findById(vmMap.getVmId())).collect(Collectors.toList());
1472+
if (checkIfVmsAssociatedWithBackupOffering(vms)) {
1473+
throw new CloudRuntimeException("Unable to delete Kubernetes cluster, as node(s) are associated to a backup offering");
1474+
}
14701475
for (KubernetesClusterVmMapVO vmMap : vmMapList) {
14711476
try {
14721477
userVmService.destroyVm(vmMap.getVmId(), expunge);
@@ -1489,6 +1494,15 @@ public Boolean doInTransaction(TransactionStatus status) {
14891494
}
14901495
}
14911496

1497+
public static boolean checkIfVmsAssociatedWithBackupOffering(List<VMInstanceVO> vms) {
1498+
for(VMInstanceVO vm : vms) {
1499+
if (Objects.nonNull(vm.getBackupOfferingId())) {
1500+
return true;
1501+
}
1502+
}
1503+
return false;
1504+
}
1505+
14921506
@Override
14931507
public ListResponse<KubernetesClusterResponse> listKubernetesClusters(ListKubernetesClustersCmd cmd) {
14941508
if (!KubernetesServiceEnabled.value()) {

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ public boolean destroy() throws CloudRuntimeException {
245245
init();
246246
validateClusterSate();
247247
this.clusterVMs = kubernetesClusterVmMapDao.listByClusterId(kubernetesCluster.getId());
248+
List<VMInstanceVO> vms = this.clusterVMs.stream().map(vmMap -> vmInstanceDao.findById(vmMap.getVmId())).collect(Collectors.toList());
249+
if (KubernetesClusterManagerImpl.checkIfVmsAssociatedWithBackupOffering(vms)) {
250+
throw new CloudRuntimeException("Unable to delete Kubernetes cluster, as node(s) are associated to a backup offering");
251+
}
248252
boolean cleanupNetwork = true;
249253
final KubernetesClusterDetailsVO clusterDetails = kubernetesClusterDetailsDao.findDetail(kubernetesCluster.getId(), "networkCleanup");
250254
if (clusterDetails != null) {

‎scripts/vm/hypervisor/kvm/nasbackup.sh‎

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,58 @@ NAS_ADDRESS=""
3131
MOUNT_OPTS=""
3232
BACKUP_DIR=""
3333
DISK_PATHS=""
34+
logFile="/var/log/cloudstack/agent/agent.log"
35+
36+
log() {
37+
[[ "$verb" -eq 1 ]] && builtin echo "$@"
38+
if [[ "$1" == "-ne" || "$1" == "-e" || "$1" == "-n" ]]; then
39+
builtin echo -e "$(date '+%Y-%m-%d %H-%M-%S>')" "${@: 2}" >> "$logFile"
40+
else
41+
builtin echo "$(date '+%Y-%m-%d %H-%M-%S>')" "$@" >> "$logFile"
42+
fi
43+
}
44+
45+
vercomp() {
46+
local IFS=.
47+
local i ver1=($1) ver2=($3)
48+
49+
# Compare each segment of the version numbers
50+
for ((i=0; i<${#ver1[@]}; i++)); do
51+
if [[ -z ${ver2[i]} ]]; then
52+
ver2[i]=0
53+
fi
54+
55+
if ((10#${ver1[i]} > 10#${ver2[i]})); then
56+
return 0 # Version 1 is greater
57+
elif ((10#${ver1[i]} < 10#${ver2[i]})); then
58+
return 2 # Version 2 is greater
59+
fi
60+
done
61+
return 0 # Versions are equal
62+
}
63+
64+
sanity_checks() {
65+
hvVersion=$(virsh version | grep hypervisor | awk '{print $(NF)}')
66+
libvVersion=$(virsh version | grep libvirt | awk '{print $(NF)}' | tail -n 1)
67+
apiVersion=$(virsh version | grep API | awk '{print $(NF)}')
68+
69+
# Compare qemu version (hvVersion >= 4.2.0)
70+
vercomp "$hvVersion" ">=" "4.2.0"
71+
hvStatus=$?
72+
73+
# Compare libvirt version (libvVersion >= 7.2.0)
74+
vercomp "$libvVersion" ">=" "7.2.0"
75+
libvStatus=$?
76+
77+
if [[ $hvStatus -eq 0 && $libvStatus -eq 0 ]]; then
78+
log -ne "Success... [ QEMU: $hvVersion Libvirt: $libvVersion apiVersion: $apiVersion ]"
79+
else
80+
echo "Failure... Your QEMU version $hvVersion or libvirt version $libvVersion is unsupported. Consider upgrading to the required minimum version of QEMU: 4.2.0 and Libvirt: 7.2.0"
81+
exit 1
82+
fi
83+
84+
log -ne "Environment Sanity Checks successfully passed"
85+
}
3486

3587
### Operation methods ###
3688

@@ -79,7 +131,7 @@ backup_stopped_vm() {
79131
name="root"
80132
for disk in $DISK_PATHS; do
81133
volUuid="${disk##*/}"
82-
qemu-img convert -O qcow2 $disk $dest/$name.$volUuid.qcow2
134+
qemu-img convert -O qcow2 $disk $dest/$name.$volUuid.qcow2 | tee -a "$logFile"
83135
name="datadisk"
84136
done
85137
sync
@@ -99,7 +151,16 @@ delete_backup() {
99151
mount_operation() {
100152
mount_point=$(mktemp -d -t csbackup.XXXXX)
101153
dest="$mount_point/${BACKUP_DIR}"
102-
mount -t ${NAS_TYPE} ${NAS_ADDRESS} ${mount_point} $([[ ! -z "${MOUNT_OPTS}" ]] && echo -o ${MOUNT_OPTS})
154+
if [ ${NAS_TYPE} == "cifs" ]; then
155+
MOUNT_OPTS="${MOUNT_OPTS},nobrl"
156+
fi
157+
mount -t ${NAS_TYPE} ${NAS_ADDRESS} ${mount_point} $([[ ! -z "${MOUNT_OPTS}" ]] && echo -o ${MOUNT_OPTS}) | tee -a "$logFile"
158+
if [ $? -eq 0 ]; then
159+
log -ne "Successfully mounted ${NAS_TYPE} store"
160+
else
161+
echo "Failed to mount ${NAS_TYPE} store"
162+
exit 1
163+
fi
103164
}
104165

105166
function usage {
@@ -157,6 +218,9 @@ while [[ $# -gt 0 ]]; do
157218
esac
158219
done
159220

221+
# Perform Initial sanity checks
222+
sanity_checks
223+
160224
if [ "$OP" = "backup" ]; then
161225
STATE=$(virsh -c qemu:///system list | grep $VM | awk '{print $3}')
162226
if [ "$STATE" = "running" ]; then

‎server/src/main/java/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5440,7 +5440,6 @@ public BackupRepositoryResponse createBackupRepositoryResponse(BackupRepository
54405440
response.setAddress(backupRepository.getAddress());
54415441
response.setProviderName(backupRepository.getProvider());
54425442
response.setType(backupRepository.getType());
5443-
response.setMountOptions(backupRepository.getMountOptions());
54445443
response.setCapacityBytes(backupRepository.getCapacityBytes());
54455444
response.setObjectName("backuprepository");
54465445
DataCenter zone = ApiDBUtils.findZoneById(backupRepository.getZoneId());

0 commit comments

Comments
 (0)