re-remove sync method (and do some warning cleanups)

DaanHoogland · DaanHoogland · commit daeb86c67fdd · 2025-02-25T09:44:35.000+01:00
diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java b/api/src/main/java/org/apache/cloudstack/backup/BackupProvider.java
@@ -49,38 +49,38 @@ public interface BackupProvider {
 
     /**
      * Assign a VM to a backup offering or policy
-     * @param vm
-     * @param backupOffering
-     * @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 vm
-     * @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 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);
 
@@ -96,30 +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);
-
-    /**
-     * This method should reconcile and create backup entries for any backups created out-of-band
-     * @param vm
-     * @param metric
-     */
-    void syncBackups(VirtualMachine vm, Backup.Metric metric);
 }
diff --git a/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java b/plugins/backup/dummy/src/main/java/org/apache/cloudstack/backup/DummyBackupProvider.java
@@ -146,8 +146,4 @@ public Pair<Boolean, Backup> takeBackup(VirtualMachine vm) {
     public boolean deleteBackup(Backup backup, boolean forced) {
         return true;
     }
-
-    @Override
-    public void syncBackups(VirtualMachine vm, Backup.Metric metric) {
-    }
 }
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
@@ -101,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;
             }
         }
@@ -407,11 +407,6 @@ public boolean willDeleteBackupsOnOfferingRemoval() {
         return false;
     }
 
-    @Override
-    public void syncBackups(VirtualMachine vm, Backup.Metric metric) {
-        // TODO: check and sum/return backups metrics on per VM basis
-    }
-
     @Override
     public List<BackupOffering> listBackupOfferings(Long zoneId) {
         final List<BackupRepository> repositories = backupRepositoryDao.listByZoneAndProvider(zoneId, getName());
diff --git a/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java b/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java
@@ -30,16 +30,12 @@
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.component.AdapterBase;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallbackNoReturn;
-import com.cloud.utils.db.TransactionStatus;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.ssh.SshHelper;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.dao.VMInstanceDao;
 
-import org.apache.cloudstack.api.InternalIdentity;
 import org.apache.cloudstack.backup.dao.BackupDao;
 import org.apache.cloudstack.backup.dao.BackupOfferingDaoImpl;
 import org.apache.cloudstack.backup.networker.NetworkerClient;
@@ -567,86 +563,6 @@ public Map<VirtualMachine, Backup.Metric> getBackupMetrics(Long zoneId, List<Vir
         return metrics;
     }
 
-    @Override
-    public void syncBackups(VirtualMachine vm, Backup.Metric metric) {
-        final Long zoneId = vm.getDataCenterId();
-        Transaction.execute(new TransactionCallbackNoReturn() {
-            @Override
-            public void doInTransactionWithoutResult(TransactionStatus status) {
-                final List<Backup> backupsInDb = backupDao.listByVmId(null, vm.getId());
-                final ArrayList<String> backupsInNetworker = getClient(zoneId).getBackupsForVm(vm);
-                final List<Long> removeList = backupsInDb.stream().map(InternalIdentity::getId).collect(Collectors.toList());
-                for (final String networkerBackupId : backupsInNetworker ) {
-                    long vmBackupSize=0L;
-                    boolean backupExists = false;
-                    for (final Backup backupInDb : backupsInDb) {
-                        LOG.debug(String.format("Checking if Backup %s with external ID %s for VM %s is valid", backupsInDb, backupInDb.getName(), vm));
-                        if ( networkerBackupId.equals(backupInDb.getExternalId()) ) {
-                            LOG.debug(String.format("Found Backup %s in both Database and Networker", backupInDb));
-                            backupExists = true;
-                            removeList.remove(backupInDb.getId());
-                            if (metric != null) {
-                                LOG.debug(String.format("Update backup [%s] from [size: %s, protected size: %s] to [size: %s, protected size: %s].",
-                                        backupInDb, backupInDb.getSize(), backupInDb.getProtectedSize(),
-                                        metric.getBackupSize(), metric.getDataSize()));
-                                ((BackupVO) backupInDb).setSize(metric.getBackupSize());
-                                ((BackupVO) backupInDb).setProtectedSize(metric.getDataSize());
-                                backupDao.update(backupInDb.getId(), ((BackupVO) backupInDb));
-                            }
-                            break;
-                        }
-                    }
-                    if (backupExists) {
-                        continue;
-                    }
-                    // Technically an administrator can manually create a backup for a VM by utilizing the KVM scripts
-                    // with the proper parameters. So we will register any backups taken on the Networker side from
-                    // outside Cloudstack. If ever Networker will support KVM out of the box this functionality also will
-                    // ensure that SLA like backups will be found and registered.
-                    NetworkerBackup strayNetworkerBackup = getClient(vm.getDataCenterId()).getNetworkerBackupInfo(networkerBackupId);
-                    // Since running backups are already present in Networker Server but not completed
-                    // make sure the backup is not in progress at this time.
-                    if ( strayNetworkerBackup.getCompletionTime() != null) {
-                        BackupVO strayBackup = new BackupVO();
-                        strayBackup.setVmId(vm.getId());
-                        strayBackup.setExternalId(strayNetworkerBackup.getId());
-                        strayBackup.setType(strayNetworkerBackup.getType());
-                        SimpleDateFormat formatterDateTime = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ");
-                        try {
-                            strayBackup.setDate(formatterDateTime.parse(strayNetworkerBackup.getSaveTime()));
-                        } catch (ParseException e) {
-                            String msg = String.format("Unable to parse date [%s].", strayNetworkerBackup.getSaveTime());
-                            LOG.error(msg, e);
-                            throw new CloudRuntimeException(msg, e);
-                        }
-                        strayBackup.setStatus(Backup.Status.BackedUp);
-                        for ( Backup.VolumeInfo thisVMVol : vm.getBackupVolumeList()) {
-                            vmBackupSize += (thisVMVol.getSize() / 1024L /1024L);
-                        }
-                        strayBackup.setSize(vmBackupSize);
-                        strayBackup.setProtectedSize(strayNetworkerBackup.getSize().getValue() / 1024L );
-                        strayBackup.setBackupOfferingId(vm.getBackupOfferingId());
-                        strayBackup.setAccountId(vm.getAccountId());
-                        strayBackup.setDomainId(vm.getDomainId());
-                        strayBackup.setZoneId(vm.getDataCenterId());
-                        LOG.debug(String.format("Creating a new entry in backups: [id: %s, uuid: %s, vm_id: %s, external_id: %s, type: %s, date: %s, backup_offering_id: %s, account_id: %s, "
-                                        + "domain_id: %s, zone_id: %s].", strayBackup.getId(), strayBackup.getUuid(), strayBackup.getVmId(), strayBackup.getExternalId(),
-                                strayBackup.getType(), strayBackup.getDate(), strayBackup.getBackupOfferingId(), strayBackup.getAccountId(),
-                                strayBackup.getDomainId(), strayBackup.getZoneId()));
-                        backupDao.persist(strayBackup);
-                        LOG.warn("Added backup found in provider [" + strayBackup + "]");
-                    } else {
-                        LOG.debug ("Backup is in progress, skipping addition for this run");
-                    }
-                }
-                for (final Long backupIdToRemove : removeList) {
-                    LOG.warn(String.format("Removing backup with ID: [%s].", backupIdToRemove));
-                    backupDao.remove(backupIdToRemove);
-                }
-            }
-        });
-    }
-
     @Override
     public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoint, VirtualMachine vm, Backup.Metric metric) {
         // Technically an administrator can manually create a backup for a VM by utilizing the KVM scripts
diff --git a/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java b/plugins/backup/veeam/src/main/java/org/apache/cloudstack/backup/VeeamBackupProvider.java

Original file line number	Diff line number	Diff line change
`@@ -146,8 +146,4 @@ public Pair<Boolean, Backup> takeBackup(VirtualMachine vm) {`
`146`	`146`	`public boolean deleteBackup(Backup backup, boolean forced) {`
`147`	`147`	`return true;`
`148`	`148`	`}`
`149`		`-`
`150`		`- @Override`
`151`		`- public void syncBackups(VirtualMachine vm, Backup.Metric metric) {`
`152`		`- }`
`153`	`149`	`}`
Original file line number	Diff line number	Diff line change
`@@ -101,15 +101,15 @@ protected Host getLastVMHypervisorHost(VirtualMachine vm) {`
`101`	`101`	`// Try to find any Up host in the same cluster`
`102`	`102`	`for (final Host hostInCluster : hostDao.findHypervisorHostInCluster(host.getClusterId())) {`
`103`	`103`	`if (hostInCluster.getStatus() == Status.Up) {`
`104`		`- LOG.debug("Found Host {}", hostInCluster);`
	`104`	`+ LOG.debug("Found Host {} in cluster {}", hostInCluster, host.getClusterId());`
`105`	`105`	`return hostInCluster;`
`106`	`106`	`}`
`107`	`107`	`}`
`108`	`108`	`}`
`109`	`109`	`// Try to find any Host in the zone`
`110`	`110`	`for (final HostVO hostInZone : hostDao.listByDataCenterIdAndHypervisorType(host.getDataCenterId(), Hypervisor.HypervisorType.KVM)) {`
`111`	`111`	`if (hostInZone.getStatus() == Status.Up) {`
`112`		`- LOG.debug("Found Host {}", hostInZone);`
	`112`	`+ LOG.debug("Found Host {} in zone {}", hostInZone, host.getDataCenterId());`
`113`	`113`	`return hostInZone;`
`114`	`114`	`}`
`115`	`115`	`}`
`@@ -407,11 +407,6 @@ public boolean willDeleteBackupsOnOfferingRemoval() {`
`407`	`407`	`return false;`
`408`	`408`	`}`
`409`	`409`
`410`		`- @Override`
`411`		`- public void syncBackups(VirtualMachine vm, Backup.Metric metric) {`
`412`		`- // TODO: check and sum/return backups metrics on per VM basis`
`413`		`- }`
`414`		`-`
`415`	`410`	`@Override`
`416`	`411`	`public List<BackupOffering> listBackupOfferings(Long zoneId) {`
`417`	`412`	`final List<BackupRepository> repositories = backupRepositoryDao.listByZoneAndProvider(zoneId, getName());`