resolve merge problems in the backup framework (#10457)

Co-authored-by: Wei Zhou <weizhou@apache.org>

Author: DaanHoogland

api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java

@@ -49,39 +49,38 @@ public interface BackupProvider {
 
     /**
      * Assign a VM to a backup offering or policy
-     * @param vm
-     * @param backup
-     * @param policy
-     * @return
+     * @param vm the machine to back up
+     * @param backupOffering the SLA definition for the backup
+     * @return succeeded?
      */
     boolean assignVMToBackupOffering(VirtualMachine vm, BackupOffering backupOffering);
 
     /**
      * Removes a VM from a backup offering or policy
-     * @param vm
-     * @return
+     * @param vm the machine to stop backing up
+     * @return succeeded?
      */
     boolean removeVMFromBackupOffering(VirtualMachine vm);
 
     /**
-     * Whether the provide will delete backups on removal of VM from the offfering
+     * Whether the provider will delete backups on removal of VM from the offering
      * @return boolean result
      */
     boolean willDeleteBackupsOnOfferingRemoval();
 
     /**
      * Starts and creates an adhoc backup process
      * for a previously registered VM backup
-     * @param backup
-     * @return
+     * @param vm the machine to make a backup of
+     * @return the result and {code}Backup{code} {code}Object{code}
      */
     Pair<Boolean, Backup> takeBackup(VirtualMachine vm);
 
     /**
      * Delete an existing backup
-     * @param backuo The backup to exclude
+     * @param backup The backup to exclude
      * @param forced Indicates if backup will be force removed or not
-     * @return
+     * @return succeeded?
      */
     boolean deleteBackup(Backup backup, boolean forced);
 
@@ -97,23 +96,23 @@ public interface BackupProvider {
 
     /**
      * Returns backup metrics for a list of VMs in a zone
-     * @param zoneId
-     * @param vms
-     * @return
+     * @param zoneId the zone for which to return metrics
+     * @param vms a list of machines to get measurements for
+     * @return a map of machine -> backup metrics
      */
     Map<VirtualMachine, Backup.Metric> getBackupMetrics(Long zoneId, List<VirtualMachine> vms);
 
     /**
      * This method should TODO
-     * @param
+     * @param vm the machine to get restore point for
      */
-    public List<Backup.RestorePoint> listRestorePoints(VirtualMachine vm);
+    List<Backup.RestorePoint> listRestorePoints(VirtualMachine vm);
 
     /**
      * This method should TODO
-     * @param
-     * @param
-     * @param metric
+     * @param restorePoint the restore point to create a backup for
+     * @param vm The machine for which to create a backup
+     * @param metric the metric object to update with the new backup data
      */
     Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoint, VirtualMachine vm, Backup.Metric metric);
 }

plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java

@@ -112,7 +112,7 @@ public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoi
 
     @Override
     public boolean removeVMFromBackupOffering(VirtualMachine vm) {
-        logger.debug(String.format("Removing VM %s from backup offering by the Dummy Backup Provider", vm));
+        logger.debug("Removing VM {} from backup offering by the Dummy Backup Provider", vm);
         return true;
     }
 
@@ -123,15 +123,15 @@ public boolean willDeleteBackupsOnOfferingRemoval() {
 
     @Override
     public Pair<Boolean, Backup> takeBackup(VirtualMachine vm) {
-        logger.debug(String.format("Starting backup for VM %s on Dummy provider", vm));
+        logger.debug("Starting backup for VM {} on Dummy provider", vm);
 
         BackupVO backup = new BackupVO();
         backup.setVmId(vm.getId());
         backup.setExternalId("dummy-external-id");
         backup.setType("FULL");
         backup.setDate(new Date());
         backup.setSize(1024000L);
-        backup.setProtectedSize(1 * Resource.ResourceType.bytesToGiB);
+        backup.setProtectedSize(Resource.ResourceType.bytesToGiB);
         backup.setStatus(Backup.Status.BackedUp);
         backup.setBackupOfferingId(vm.getBackupOfferingId());
         backup.setAccountId(vm.getAccountId());

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

@@ -17,7 +17,6 @@
 package org.apache.cloudstack.backup;
 
 import com.cloud.agent.AgentManager;
-import com.cloud.dc.dao.ClusterDao;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.OperationTimedoutException;
 import com.cloud.host.Host;
@@ -36,8 +35,8 @@
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.dao.VMInstanceDao;
+
 import org.apache.cloudstack.backup.dao.BackupDao;
-import org.apache.cloudstack.backup.dao.BackupOfferingDao;
 import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
@@ -70,15 +69,9 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
     @Inject
     private BackupRepositoryDao backupRepositoryDao;
 
-    @Inject
-    private BackupOfferingDao backupOfferingDao;
-
     @Inject
     private HostDao hostDao;
 
-    @Inject
-    private ClusterDao clusterDao;
-
     @Inject
     private VolumeDao volumeDao;
 
@@ -108,15 +101,15 @@ protected Host getLastVMHypervisorHost(VirtualMachine vm) {
             // Try to find any Up host in the same cluster
             for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {
                 if (hostInCluster.getStatus() == Status.Up) {
-                    LOG.debug("Found Host {}", hostInCluster);
+                    LOG.debug("Found Host {} in cluster {}", hostInCluster, host.getClusterId());
                     return hostInCluster;
                 }
             }
         }
         // Try to find any Host in the zone
         for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {
             if (hostInZone.getStatus() == Status.Up) {
-                LOG.debug("Found Host {}", hostInZone);
+                LOG.debug("Found Host {} in zone {}", hostInZone, host.getDataCenterId());
                 return hostInZone;
             }
         }
@@ -166,7 +159,7 @@ public Pair<Boolean, Backup> takeBackup(final VirtualMachine vm) {
             command.setVolumePaths(volumePaths);
         }
 
-        BackupAnswer answer = null;
+        BackupAnswer answer;
         try {
             answer = (BackupAnswer) agentManager.send(host.getId(), command);
         } catch (AgentUnavailableException e) {
@@ -204,7 +197,7 @@ private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
                 virtualSize += volume.getSize();
             }
         }
-        backup.setProtectedSize(Long.valueOf(virtualSize));
+        backup.setProtectedSize(virtualSize);
         backup.setStatus(Backup.Status.BackingUp);
         backup.setBackupOfferingId(vm.getBackupOfferingId());
         backup.setAccountId(vm.getAccountId());
@@ -231,7 +224,7 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
         restoreCommand.setVmExists(vm.getRemoved() == null);
         restoreCommand.setVmState(vm.getState());
 
-        BackupAnswer answer = null;
+        BackupAnswer answer;
         try {
             answer = (BackupAnswer) agentManager.send(host.getId(), restoreCommand);
         } catch (AgentUnavailableException e) {
@@ -298,7 +291,7 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
         restoreCommand.setVmState(vmNameAndState.second());
         restoreCommand.setRestoreVolumeUUID(volumeUuid);
 
-        BackupAnswer answer = null;
+        BackupAnswer answer;
         try {
             answer = (BackupAnswer) agentManager.send(hostVO.getId(), restoreCommand);
         } catch (AgentUnavailableException e) {
@@ -350,7 +343,7 @@ public boolean deleteBackup(Backup backup, boolean forced) {
         DeleteBackupCommand command = new DeleteBackupCommand(backup.getExternalId(), backupRepository.getType(),
                 backupRepository.getAddress(), backupRepository.getMountOptions());
 
-        BackupAnswer answer = null;
+        BackupAnswer answer;
         try {
             answer = (BackupAnswer) agentManager.send(host.getId(), command);
         } catch (AgentUnavailableException e) {
@@ -363,7 +356,7 @@ public boolean deleteBackup(Backup backup, boolean forced) {
             return backupDao.remove(backup.getId());
         }
 
-        logger.debug("There was an error removing the backup with id " + backup.getId());
+        logger.debug("There was an error removing the backup with id {}", backup.getId());
         return false;
     }
 

plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java

@@ -21,6 +21,7 @@
 import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.utils.script.Script;
 import com.cloud.storage.StoragePoolHostVO;
 import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeVO;
@@ -34,6 +35,7 @@
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.dao.VMInstanceDao;
+
 import org.apache.cloudstack.backup.dao.BackupDao;
 import org.apache.cloudstack.backup.dao.BackupOfferingDaoImpl;
 import org.apache.cloudstack.backup.networker.NetworkerClient;
@@ -44,7 +46,9 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.xml.utils.URI;
 import org.apache.cloudstack.backup.networker.api.NetworkerBackup;
+
 import javax.inject.Inject;
+
 import java.net.URISyntaxException;
 import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
@@ -60,7 +64,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import com.cloud.utils.script.Script;
 
 public class NetworkerBackupProvider extends AdapterBase implements BackupProvider, Configurable {
 
@@ -169,7 +172,7 @@ protected HostVO getLastVMHypervisorHost(VirtualMachine vm) {
             List<HostVO> altClusterHosts = hostDao.findHypervisorHostInCluster(host.getClusterId());
             for (final HostVO candidateClusterHost : altClusterHosts) {
                 if ( candidateClusterHost.getStatus() == Status.Up ) {
-                    LOG.debug(String.format("Found Host %s", candidateClusterHost));
+                    LOG.debug("Found Host {}", candidateClusterHost);
                     return candidateClusterHost;
                 }
             }
@@ -178,7 +181,7 @@ protected HostVO getLastVMHypervisorHost(VirtualMachine vm) {
         List<HostVO> altZoneHosts = hostDao.findByDataCenterId(host.getDataCenterId());
         for (final HostVO candidateZoneHost : altZoneHosts) {
             if ( candidateZoneHost.getStatus() == Status.Up && candidateZoneHost.getHypervisorType() == Hypervisor.HypervisorType.KVM ) {
-                LOG.debug("Found Host " + candidateZoneHost);
+                LOG.debug("Found Host {}", candidateZoneHost);
                 return candidateZoneHost;
             }
         }
@@ -229,13 +232,13 @@ private String executeBackupCommand(HostVO host, String username, String passwor
             Pair<Boolean, String> response = SshHelper.sshExecute(host.getPrivateIpAddress(), 22,
                     username, null, password, command, 120000, 120000, 3600000);
             if (!response.first()) {
-                LOG.error(String.format("Backup Script failed on HYPERVISOR %s due to: %s", host, response.second()));
+                LOG.error("Backup Script failed on HYPERVISOR {} due to: {}", host, response.second());
             } else {
-                LOG.debug(String.format("Networker Backup Results: %s", response.second()));
+                LOG.debug("Networker Backup Results: {}", response.second());
             }
             Matcher saveTimeMatcher = saveTimePattern.matcher(response.second());
             if (saveTimeMatcher.find()) {
-                LOG.debug(String.format("Got saveTimeMatcher: %s", saveTimeMatcher.group(1)));
+                LOG.debug("Got saveTimeMatcher: {}", saveTimeMatcher.group(1));
                 return saveTimeMatcher.group(1);
             }
         } catch (final Exception e) {
@@ -251,9 +254,9 @@ private boolean executeRestoreCommand(HostVO host, String username, String passw
                 username, null, password, command, 120000, 120000, 3600000);
 
             if (!response.first()) {
-                LOG.error(String.format("Restore Script failed on HYPERVISOR %s due to: %s", host, response.second()));
+                LOG.error("Restore Script failed on HYPERVISOR {} due to: {}", host, response.second());
             } else {
-                LOG.debug(String.format("Networker Restore Results: %s",response.second()));
+                LOG.debug("Networker Restore Results: {}",response.second());
                 return true;
             }
         } catch (final Exception e) {
@@ -310,7 +313,7 @@ public boolean removeVMFromBackupOffering(VirtualMachine vm) {
         List<String> backupsTaken = getClient(vm.getDataCenterId()).getBackupsForVm(vm);
 
         for (String backupId : backupsTaken) {
-            LOG.debug("Trying to remove backup with id" + backupId);
+            LOG.debug("Trying to remove backup with id {}", backupId);
             getClient(vm.getDataCenterId()).deleteBackupForVM(backupId);
         }
 
@@ -327,18 +330,18 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
         final NetworkerBackup networkerBackup=getClient(zoneId).getNetworkerBackupInfo(externalBackupId);
         final String SSID = networkerBackup.getShortId();
 
-        LOG.debug(String.format("Restoring vm %s from backup %s on the Networker Backup Provider", vm, backup));
+        LOG.debug("Restoring vm {} from backup {} on the Networker Backup Provider", vm, backup);
 
         if ( SSID.isEmpty() ) {
-            LOG.debug("There was an error retrieving the SSID for backup with id " + externalBackupId + " from EMC NEtworker");
+            LOG.debug("There was an error retrieving the SSID for backup with id {} from EMC NEtworker", externalBackupId);
             return false;
         }
 
         // Find where the VM was last running
         hostVO = getLastVMHypervisorHost(vm);
         // Get credentials for that host
         Ternary<String, String, String> credentials = getKVMHyperisorCredentials(hostVO);
-        LOG.debug("The SSID was reported successfully " + externalBackupId);
+        LOG.debug("The SSID was reported successfully {}", externalBackupId);
         try {
             networkerServer = getUrlDomain(NetworkerUrl.value());
         } catch (URISyntaxException e) {
@@ -355,14 +358,14 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) {
             script.add("-v");
 
         Date restoreJobStart = new Date();
-        LOG.debug(String.format("Starting Restore for VM %s and %s at %s", vm, SSID, restoreJobStart));
+        LOG.debug("Starting Restore for VM {} and {} at {}", vm, SSID, restoreJobStart);
 
         if ( executeRestoreCommand(hostVO, credentials.first(), credentials.second(), script.toString()) ) {
             Date restoreJobEnd = new Date();
-            LOG.debug("Restore Job for SSID " + SSID + " completed successfully at " + restoreJobEnd);
+            LOG.debug("Restore Job for SSID {} completed successfully at {}", SSID, restoreJobEnd);
             return true;
         } else {
-            LOG.debug("Restore Job for SSID " + SSID + " failed!");
+            LOG.debug("Restore Job for SSID {} failed!", SSID);
             return false;
         }
     }
@@ -383,7 +386,7 @@ public Pair<Boolean, String> restoreBackedUpVolume(Backup backup, String volumeU
         final String destinationNetworkerClient = hostVO.getName().split("\\.")[0];
         Long restoredVolumeDiskSize = 0L;
 
-        LOG.debug(String.format("Restoring volume %s with uuid %s from backup %s on the Networker Backup Provider", volume, volumeUuid, backup));
+        LOG.debug("Restoring volume {} with uuid {} from backup {} on the Networker Backup Provider", volume, volumeUuid, backup);
 
         if ( SSID.isEmpty() ) {
             LOG.debug("There was an error retrieving the SSID for backup with id " + externalBackupId + " from EMC NEtworker");
@@ -536,8 +539,8 @@ public boolean deleteBackup(Backup backup, boolean forced) {
     @Override
     public Map<VirtualMachine, Backup.Metric> getBackupMetrics(Long zoneId, List<VirtualMachine> vms) {
         final Map<VirtualMachine, Backup.Metric> metrics = new HashMap<>();
-        Long vmBackupSize=0L;
-        Long vmBackupProtectedSize=0L;
+        long vmBackupSize=0L;
+        long vmBackupProtectedSize=0L;
 
         if (CollectionUtils.isEmpty(vms)) {
             LOG.warn("Unable to get VM Backup Metrics because the list of VMs is empty.");
@@ -584,7 +587,7 @@ public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoi
                 throw new CloudRuntimeException(msg, e);
             }
             backup.setStatus(Backup.Status.BackedUp);
-            Long vmBackupProtectedSize=0L;
+            long vmBackupProtectedSize=0L;
             for (Backup.VolumeInfo thisVMVol : vm.getBackupVolumeList()) {
                 vmBackupProtectedSize += (thisVMVol.getSize() / 1024L / 1024L);
             }
@@ -605,9 +608,7 @@ public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoi
     public List<Backup.RestorePoint> listRestorePoints(VirtualMachine vm) {
         final Long zoneId = vm.getDataCenterId();
         final ArrayList<String> backupIds = getClient(zoneId).getBackupsForVm(vm);
-        List<Backup.RestorePoint> restorePoints =
-            backupIds.stream().map(id -> new Backup.RestorePoint(id, null, null)).collect(Collectors.toList());
-        return restorePoints;
+        return backupIds.stream().map(id -> new Backup.RestorePoint(id, null, null)).collect(Collectors.toList());
     }
 
     @Override