You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/09/15 18:01:58 UTC
[GitHub] [cloudstack] SadiJr opened a new pull request #5457: Block expunge of VM with has backups
SadiJr opened a new pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457
### Description
This PR intends to block users from expunging a VM with has backups. The need for this change arose from a test in which a VM that had some backups in Veeam, when removed, did not have the jobs and backups removed from Veeam
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
### How Has This Been Tested?
- I created one VM and assign it to a Veeam Backup Offering;
- I maked some backups of this VM;
- I verified in Veeam if backups and jobs from this VM exists;
- I tried to expunge this VM.
- I received a expected error message.
- I removed VM from Backup Offering;
- I checked, in Veeam, if jobs and backups are removed;
- I retried to expunge VM;
- VM is expunged with success now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041805732
> @DaanHoogland
>
> I'm working to improve the Veeam integration, and allow removing the Veeam job without removing the backups. Once I'm done, I'll open a PR, but I'm not sure if it's essential to have an issue for this.
no need for an issue if we have a PR ;)
should we merge this or are you making it obsolete in the new PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1034814981
@DaanHoogland yes, from a functional point of view, your suggested approach does not cause any errors. I am just thinking of the scenario where the user has saved backups, destroys the VM, and without warning has all his backups deleted.
But maybe this is just an unnecessary concern, since if the user knowingly destroyed the VM, he is hardly concerned about the backups.
That said, I will apply your suggestion. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041869057
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2635
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041832505
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1032737844
@SadiJr as you explain it now, I would like to see the offering be automatically be unassigned on removal of the VM. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1035002094
@DaanHoogland No, when the Backup Offering is unassigned, the current implementation deletes the backups. Why the integration was done this way, I don't know. It does not make sense to us, but this is how it was implemented (this integration was not developed by us). That is why we are trying to prevent users from deleting VMs that have backup offerings assigned.
The initial intention of this PR was to prevent the destruction of a VM that had backups, as this would cause inconsistency in Veeam, where it would constantly be trying to make new backups of a VM that no longer exists. And, that is all we are trying to achieve, since, by the current code, we could not remove the backup job without removing the backups. Again, why this is as is, I have no idea. We are just trying to make this integration work, and currently, we are fixing these unexpected/odd/weird workflows.
What do you think? Should we return to the initial idea? And just block the deletion of the VM, until we have a solution to de-assign the VM from a backup offering without removing the backups?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1035025681
> What do you think? Should we return to the initial idea? And just block the deletion of the VM, until we have a solution to de-assign the VM from a backup offering without removing the backups?
Yes, sorry to have been a hassle. I think backups should be first class citizens that can be saved, moved around, started as VM or deleted independent of the initial VM they where a backup of, and I thought that was the case. This is why I was inclined on the assigning on delete scheme. If that is not possible preventing the deletion of a VM makes sense. I do think it is prone to improvement though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-921912197
@RodrigoDLopez
Thanks for the suggestion, it's a great one.
@DaanHoogland @rhtyd
Thanks for the points raised, about being able to remove a VM while keeping the backups, my point is:
- Currently, we have two Backup providers: DummyBackupProvider and VeeamBackupProvider (VMWare only).
- Both do not allow removing a job without also removing the backups.
So, imagine the scenario:
1. A user owns a VM associated with a backup offering.
2. The user removes the VM.
3. The backup offering continues to exist and consequently the provider job continues to run, which may be far from the ideal scenario and may cause errors at the provider.
Therefore, this PR is primarily intended to prevent the removal of a VM that has a backup offering associated with it. Also, I am marking this PR as Draft, until we I make the adjustments request by @rodrigo; the idea of showing for the user the backupofferings that the VM is associated with and its backups (if it has) is very interesting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041833112
@GutoVeronezi a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041838254
@DaanHoogland, regarding @SadiJr's comment (https://github.com/apache/cloudstack/pull/5457#issuecomment-1041836695), could we run the tests for this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1035114819
@DaanHoogland no problem. I'll revert to original changes, and work to improve the Veeam integration code to allow unassign a VM from a Backup Offering without deleting all the VM backups.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041732036
@DaanHoogland
I'm working to improve the Veeam integration, and allow removing the Veeam job without removing the backups. Once I'm done, I'll open a PR, but I'm not sure if it's essential to have an issue for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1042517429
<b>Trillian test result (tid-3368)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32548 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5457-t3368-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041463082
@DaanHoogland I revert to original changes. Anything else you want to comment on?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr edited a comment on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr edited a comment on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-921912197
@RodrigoDLopez
Thanks for the suggestion, it's a great one.
@DaanHoogland @rhtyd
Thanks for the points raised, about being able to remove a VM while keeping the backups, my point is:
- Currently, we have two Backup providers: DummyBackupProvider and VeeamBackupProvider (VMWare only).
- Both do not allow removing a job without also removing the backups.
So, imagine the scenario:
1. A user owns a VM associated with a backup offering.
2. The user removes the VM.
3. The backup offering continues to exist and consequently the provider job continues to run, which may be far from the ideal scenario and may cause errors at the provider.
Therefore, this PR is primarily intended to prevent the removal of a VM that has a backup offering associated with it. Also, I am marking this PR as Draft, until we I make the adjustments request by @RodrigoDLopez; the idea of showing for the user the backupofferings that the VM is associated with and its backups (if it has) is very interesting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041840047
Will keep en eye. I'll start it when packaging is successful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041688934
> @DaanHoogland I revert to original changes. Anything else you want to comment on?
no, for this PR we just block from deleting a VM that has a backup offering.
Do we need a ticket for further improvement? (I am not a Veeam user)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041836695
@DaanHoogland for now, I'm just focusing on making the ACS integration with Veeam allow this kind of operation, I haven't thought deeply about it yet. But I believe have no problem doing the merge now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041912867
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1034970501
> @DaanHoogland yes, from a functional point of view, your suggested approach does not cause any errors. I am just thinking of the scenario where the user has saved backups, destroys the VM, and without warning has all his backups deleted.
>
> But maybe this is just an unnecessary concern, since if the user knowingly destroyed the VM, he is hardly concerned about the backups.
Hold on @SadiJr, the backup still remain available after removing the offering, don't they?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] RodrigoDLopez commented on pull request #5457: Block expunge of VM with has backups
Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-920309246
@SadiJr nice enhancement.
I was thinking about a different approach. For now you are just notifying that this instance has backups that need to be deleted first before proceed with the delete instance.
what about, if we bring those backups to show to the operator and then offer to him the possibility to delete on the pop up that will be show with the exception message log.
this is just an idea that occurred to me. Perhaps other contributors might exhibit different behaviors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block expunge of VM which has backups
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-921613366
I do not like this functionally, as @rhtyd says backups are first class citizens and should be able to be restored even if the VM is gone. Or while it is still there.
@SadiJr I agree that the job should be removed but the backup is still valid. Is there a problem with the backup still being there? and with the job, do you mean the job definition in cloudstack or the actual job in Veeam?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #5457: Block expunge of VM with has backups
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-920594044
@SadiJr it depends, we do want user to be able to destroy and expunge a VM without deleting the backups. Backups are first class resources unlike snapshots.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1033813892
> @DaanHoogland I don't know. I am assuming that the end user is unaware of this behavior. In this case, automatically performing the unassign without alerting the user may result in the user thinking that an error has occurred in Veeam or ACS.
What error would that be, @SadiJr ?
To further clarify: I would say that on deletion of the VM, the Offering will be unassigned if it exists, before deleting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1041911805
@GutoVeronezi @SadiJr note that this change is probably not touched by any smoke tests. I'll start anyway to ensure no big regressions.
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1042682504
This was user tested and we have 2 lgtm. This requires special environmental setup, so trusting the user tests here and merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1032879781
@DaanHoogland I don't know. I am assuming that the end user is unaware of this behavior. In this case, automatically performing the unassign without alerting the user may result in the user thinking that an error has occurred in Veeam or ACS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5457: Block remove of VM which has backup offering
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5457:
URL: https://github.com/apache/cloudstack/pull/5457#issuecomment-1032667868
@DaanHoogland @rohityadavcloud
I was reviewing my PRs, and realized that I ended up not being clear in my explanation of my changes.
Currently, the integration with Veeam, does not allow users to remove specific backups. This can be seen in the org.apache.cloudstack.backup.VeeamBackupProvider.deleteBackup(Backup) method, where, when attempting to remove a specific backup, the following exception is thrown:
```
Veeam B&R plugin does not allow removal of backup restore point, to delete the backup chain remove VM from the backup offering
```
However, with Veeam, this operation results in Veeam's job continuing to exist in the backend, and consequently continuing to attempt to perform backup of a VM that no longer exists because the VM was deleted without unassigning it from the backip offering.
The goal of this PR is to make sure that the user who owns a VM that has a Backup Offering as well as backups is prevented from destroying the VM before the user assigns the VM from the backup offering.
I agree that ideally the right thing would be to allow removing the VM and removing the job in Veeam, but keeping the backups; however, the integration does not allow this kind of operation so far. Unfortunately I didn't get to look at the Veeam documentation on this, so I can't say if this is a Veeam or ACS limitation.
Finally, regarding @RodrigoDLopez suggestions, I came to the conclusion that they should be implemented, but in a separated PR, in order to separate the contexts, since the current PR is only about changing the API.
I will remove the draft of this PR and ask you to re-review it. Suggestions are always welcome :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org