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 2022/07/28 19:21:14 UTC

[GitHub] [cloudstack] SadiJr opened a new pull request, #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

SadiJr opened a new pull request, #6581:
URL: https://github.com/apache/cloudstack/pull/6581

   ### Description
   
   Using the VMWare hypervisor, with the Veeam plugin active, it is not possible to attach or detach volumes on VMs attached in a Backup Offering. This PR aims to externalize the global configuration `backup.enable.attach.detach.of.volumes` to allow operators to define if it's possible or not to attach/detach volumes in VMs assigned to a Backup Offering.
   
   
   ### 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?
   It was tested in a local lab:
   
   1. I created a new VM and attached this VM to a Backup Offering;
   2. I tried to attach a new volume in this VM;
   3. Before, an exception was thrown;
   4. Now, with the global configuration `backup.enable.attach.detach.of.volumes` as true, it's possible to attach/detach volumes in VMs assigned to a Backup Offering.
   Also, I added new unit 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] blueorangutan commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1205010302

   @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] shwstppr commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
shwstppr commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1206291532

   @DaanHoogland checked the logs. Seems like some intermittent issue with the env. We may run tests again


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1399150046

   <b>Trillian test result (tid-5926)</b>
   Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server r8
   Total time taken: 48476 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6581-t5926-vmware-70u3.zip
   Smoke tests completed. 107 look OK, 0 have errors, 0 did not run
   Only failed and skipped 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 a diff in pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
SadiJr commented on code in PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#discussion_r936701790


##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2396,6 +2397,35 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
         return volumeToAttach;
     }
 
+    protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
+        if ((vm.getBackupOfferingId() != null || CollectionUtils.isNotEmpty(vm.getBackupVolumeList())) &&
+                BooleanUtils.isFalse(BackupManager.BackupEnableAttachDetachVolumes.value())) {
+            String errorMsg = "Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or "
+                    + "set the global configuration 'backup.enable.attach.detach.of.volumes' to true.";
+            if (attach)
+                errorMsg = "Unable to attach volume, please specify a VM that does not have any backups or set the global configuration "
+                        + "'backup.enable.attach.detach.of.volumes' to true.";
+            throw new InvalidParameterValueException(errorMsg);
+        }
+    }
+
+    protected String createVolumeInfoFromVolumes(List<VolumeVO> vmVolumes) {
+        try {
+            List<Backup.VolumeInfo> list = new ArrayList<>();
+            for (VolumeVO vol : vmVolumes) {
+                list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), vol.getVolumeType(), vol.getSize()));
+            }
+            return new Gson().toJson(list.toArray(), Backup.VolumeInfo[].class);

Review Comment:
   Done, thanks.



##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -736,12 +739,14 @@ protected VolumeVO updateVolume(VirtualDisk disk, Map<VirtualDisk, VolumeVO> dis
         volume.setPath(volumeName);
         volume.setPoolId(poolId);
         VirtualMachineDiskInfo diskInfo = getDiskInfo(vmToImport, poolId, volumeName);
-        volume.setChainInfo(new Gson().toJson(diskInfo));
+        volume.setChainInfo(GSON.toJson(diskInfo));

Review Comment:
   Done, thanks.



##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2396,6 +2397,35 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
         return volumeToAttach;
     }
 
+    protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
+        if ((vm.getBackupOfferingId() != null || CollectionUtils.isNotEmpty(vm.getBackupVolumeList())) &&

Review Comment:
   Done, 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] DaanHoogland commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1204886051

   Unfortunately I don´t have a setup with veeam, so my tests won´t help to assess the code/functionality.
   @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] blueorangutan commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1398281437

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5345


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1398221394

   @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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1206279745

   @shwstppr should we consider these errors in assessing this PR?
   
   > Trillian test result (tid-4619) Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7 Total time taken: 43275 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6581-t4619-vmware-67u3.zip Smoke tests completed. 100 look OK, 1 have errors Only failed tests results shown below:
   > Test 	Result 	Time (s) 	Test File
   > test_01_invalid_upgrade_kubernetes_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_02_upgrade_kubernetes_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_03_deploy_and_scale_kubernetes_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_04_autoscale_kubernetes_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_05_basic_lifecycle_kubernetes_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_06_delete_kubernetes_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_07_deploy_kubernetes_ha_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_08_upgrade_kubernetes_ha_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   > test_09_delete_kubernetes_ha_cluster 	`Failure` 	0.01 	test_kubernetes_clusters.py
   
   


-- 
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] sonarcloud[bot] commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1198591943

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6581)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL)
   
   [![0.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.9%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_coverage&view=list) [0.9% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1198547645

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6581 (SL-JID-2023)


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1207854532

   @GutoVeronezi are your concerns with this met (a.k.a. can you approve)?


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1204963069

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3901


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1205009075

   @blueorangutan test centos7 vmware-67u3


-- 
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] codecov[bot] commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1338102557

   # [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6581?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6581](https://codecov.io/gh/apache/cloudstack/pull/6581?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da049c4) into [main](https://codecov.io/gh/apache/cloudstack/commit/a63b2aba7aa3948f78d280a356c550c6638a137b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a63b2ab) will **increase** coverage by `0.00%`.
   > The diff coverage is `20.75%`.
   
   ```diff
   @@            Coverage Diff            @@
   ##               main    #6581   +/-   ##
   =========================================
     Coverage     11.48%   11.48%           
   - Complexity     7513     7515    +2     
   =========================================
     Files          2491     2491           
     Lines        246730   246776   +46     
     Branches      38552    38560    +8     
   =========================================
   + Hits          28332    28339    +7     
   - Misses       214816   214854   +38     
   - Partials       3582     3583    +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6581?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...rg/apache/cloudstack/backup/BackupManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6581/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2JhY2t1cC9CYWNrdXBNYW5hZ2VySW1wbC5qYXZh) | `5.65% <0.00%> (ø)` | |
   | [...ain/java/com/cloud/hypervisor/guru/VMwareGuru.java](https://codecov.io/gh/apache/cloudstack/pull/6581/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy92bXdhcmUvc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9ndXJ1L1ZNd2FyZUd1cnUuamF2YQ==) | `1.06% <3.33%> (+0.13%)` | :arrow_up: |
   | [...n/java/com/cloud/storage/VolumeApiServiceImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6581/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3N0b3JhZ2UvVm9sdW1lQXBpU2VydmljZUltcGwuamF2YQ==) | `13.18% <45.45%> (+0.33%)` | :arrow_up: |
   | [...apache/cloudstack/syslog/AlertsSyslogAppender.java](https://codecov.io/gh/apache/cloudstack/pull/6581/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9hbGVydC1oYW5kbGVycy9zeXNsb2ctYWxlcnRzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3N5c2xvZy9BbGVydHNTeXNsb2dBcHBlbmRlci5qYXZh) | `56.49% <0.00%> (-2.26%)` | :arrow_down: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1398294528

   @blueorangutan test rocky8 vmware-70u3


-- 
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 a diff in pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#discussion_r936627445


##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2396,6 +2397,35 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
         return volumeToAttach;
     }
 
+    protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
+        if ((vm.getBackupOfferingId() != null || CollectionUtils.isNotEmpty(vm.getBackupVolumeList())) &&
+                BooleanUtils.isFalse(BackupManager.BackupEnableAttachDetachVolumes.value())) {
+            String errorMsg = "Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or "
+                    + "set the global configuration 'backup.enable.attach.detach.of.volumes' to true.";
+            if (attach)
+                errorMsg = "Unable to attach volume, please specify a VM that does not have any backups or set the global configuration "
+                        + "'backup.enable.attach.detach.of.volumes' to true.";
+            throw new InvalidParameterValueException(errorMsg);
+        }
+    }
+
+    protected String createVolumeInfoFromVolumes(List<VolumeVO> vmVolumes) {
+        try {
+            List<Backup.VolumeInfo> list = new ArrayList<>();
+            for (VolumeVO vol : vmVolumes) {
+                list.add(new Backup.VolumeInfo(vol.getUuid(), vol.getPath(), vol.getVolumeType(), vol.getSize()));
+            }
+            return new Gson().toJson(list.toArray(), Backup.VolumeInfo[].class);

Review Comment:
   We could use `GsonHelper` here.



##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -736,12 +739,14 @@ protected VolumeVO updateVolume(VirtualDisk disk, Map<VirtualDisk, VolumeVO> dis
         volume.setPath(volumeName);
         volume.setPoolId(poolId);
         VirtualMachineDiskInfo diskInfo = getDiskInfo(vmToImport, poolId, volumeName);
-        volume.setChainInfo(new Gson().toJson(diskInfo));
+        volume.setChainInfo(GSON.toJson(diskInfo));

Review Comment:
   We could use `GsonHelper` here, instead of instantiate a new object.



##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2396,6 +2397,35 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
         return volumeToAttach;
     }
 
+    protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
+        if ((vm.getBackupOfferingId() != null || CollectionUtils.isNotEmpty(vm.getBackupVolumeList())) &&

Review Comment:
   We could invert this `if` to reduce indentation.



-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1240996531

   > @GutoVeronezi are your concerns with this met (a.k.a. can you approve)?
   
   Sorry for the delay, @DaanHoogland. Just reviewed again.
   


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1206307444

   @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] blueorangutan commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1198542912

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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] DaanHoogland commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1275781720

   @SadiJr can you look at the comments and the conflicts?


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1384201252

   seeing test_affinity_groups_projects failure on several PRs, not specificly this one. see #7098 


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1398222388

   @DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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 a diff in pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#discussion_r966215397


##########
server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java:
##########
@@ -1106,6 +1108,65 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEqualsStorag
         Assert.assertTrue(result);
     }
 
+    @Test
+    public void validateIfVMHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhichBackupOffering() {

Review Comment:
   ```suggestion
       public void validateIfVmHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhichBackupOffering() {
   ```



##########
server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java:
##########
@@ -1106,6 +1108,65 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEqualsStorag
         Assert.assertTrue(result);
     }
 
+    @Test
+    public void validateIfVMHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhichBackupOffering() {
+        try {
+            UserVmVO vm = Mockito.mock(UserVmVO.class);
+            when(vm.getBackupOfferingId()).thenReturn(1l);
+            volumeApiServiceImpl.validateIfVmHasBackups(vm, false);
+        } catch (Exception e) {
+            Assert.assertEquals("Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void validateIfVMHaveBackupsTestExceptionWhenTryToAttachVolumeFromVMWhichBackupOffering() {

Review Comment:
   ```suggestion
       public void validateIfVmHaveBackupsTestExceptionWhenTryToAttachVolumeFromVMWhichBackupOffering() {
   ```



##########
server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java:
##########
@@ -1106,6 +1108,65 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEqualsStorag
         Assert.assertTrue(result);
     }
 
+    @Test
+    public void validateIfVMHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhichBackupOffering() {
+        try {
+            UserVmVO vm = Mockito.mock(UserVmVO.class);
+            when(vm.getBackupOfferingId()).thenReturn(1l);
+            volumeApiServiceImpl.validateIfVmHasBackups(vm, false);
+        } catch (Exception e) {
+            Assert.assertEquals("Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void validateIfVMHaveBackupsTestExceptionWhenTryToAttachVolumeFromVMWhichBackupOffering() {
+        try {
+            UserVmVO vm = Mockito.mock(UserVmVO.class);
+            when(vm.getBackupOfferingId()).thenReturn(1l);
+            volumeApiServiceImpl.validateIfVmHasBackups(vm, true);
+        } catch (Exception e) {
+            Assert.assertEquals("Unable to attach volume, please specify a VM that does not have any backups or set the global configuration 'backup.enable.attach.detach.of.volumes' to true.", e.getMessage());
+        }
+    }
+
+    @Test
+    public void validateIfVMHaveBackupsTestSucessWhenVMDontHaveBackupOffering() {

Review Comment:
   ```suggestion
       public void validateIfVmHaveBackupsTestSucessWhenVMDontHaveBackupOffering() {
   ```



##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2397,6 +2398,35 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
         return volumeToAttach;
     }
 
+    protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
+        if ((vm.getBackupOfferingId() == null || CollectionUtils.isEmpty(vm.getBackupVolumeList())) || BooleanUtils.isTrue(BackupManager.BackupEnableAttachDetachVolumes.value())) {
+            return;
+        }
+        String errorMsg = "Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or "
+                + "set the global configuration 'backup.enable.attach.detach.of.volumes' to true.";
+        if (attach)
+            errorMsg = "Unable to attach volume, please specify a VM that does not have any backups or set the global configuration "
+                    + "'backup.enable.attach.detach.of.volumes' to true.";

Review Comment:
   We can use `BackupManager.BackupEnableAttachDetachVolumes.key()` instead of writing the config name again.



##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2397,6 +2398,35 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
         return volumeToAttach;
     }
 
+    protected void validateIfVmHasBackups(UserVmVO vm, boolean attach) {
+        if ((vm.getBackupOfferingId() == null || CollectionUtils.isEmpty(vm.getBackupVolumeList())) || BooleanUtils.isTrue(BackupManager.BackupEnableAttachDetachVolumes.value())) {
+            return;
+        }
+        String errorMsg = "Unable to detach volume, cannot detach volume from a VM that has backups. First remove the VM from the backup offering or "
+                + "set the global configuration 'backup.enable.attach.detach.of.volumes' to true.";
+        if (attach)
+            errorMsg = "Unable to attach volume, please specify a VM that does not have any backups or set the global configuration "
+                    + "'backup.enable.attach.detach.of.volumes' to true.";

Review Comment:
   ```suggestion
           if (attach) {
               errorMsg = "Unable to attach volume, please specify a VM that does not have any backups or set the global configuration "
                       + "'backup.enable.attach.detach.of.volumes' to true.";
           }            
   ```



##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -919,11 +942,25 @@ private Map<VirtualDisk, VolumeVO> getDisksMapping(Backup backup, List<VirtualDi
         if (vm == null) {
             throw new CloudRuntimeException("Failed to find the volumes details from the VM backup");
         }
+
         List<Backup.VolumeInfo> backedUpVolumes = vm.getBackupVolumeList();
         Map<String, Boolean> usedVols = new HashMap<>();
         Map<VirtualDisk, VolumeVO> map = new HashMap<>();
 
         for (Backup.VolumeInfo backedUpVol : backedUpVolumes) {
+            VolumeVO volumeExtra = _volumeDao.findByUuid(backedUpVol.getUuid());
+            if (volumeExtra != null) {
+                s_logger.debug(String.format("Marking volume [id: %s] of vm [id: %s, name: %s] as removed for the backup process.", backedUpVol.getUuid(), vm.getUuid(), vm.getInstanceName()));

Review Comment:
   We could use `ReflectionToStringBuilderUtils` here or the object's `toString()`.



##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -736,12 +740,14 @@ protected VolumeVO updateVolume(VirtualDisk disk, Map<VirtualDisk, VolumeVO> dis
         volume.setPath(volumeName);
         volume.setPoolId(poolId);
         VirtualMachineDiskInfo diskInfo = getDiskInfo(vmToImport, poolId, volumeName);
-        volume.setChainInfo(new Gson().toJson(diskInfo));
+        volume.setChainInfo(GSON.toJson(diskInfo));
         volume.setInstanceId(vm.getId());
         volume.setState(Volume.State.Ready);
         volume.setAttached(new Date());
         _volumeDao.update(volume.getId(), volume);
         if (volume.getRemoved() != null) {
+            s_logger.debug(String.format("Marking volume [uuid: %s] of restored VM [id: %s, name: %s] as non removed.", volume.getUuid(),
+                    vm.getUuid(), vm.getInstanceName()));

Review Comment:
   We could use `ReflectionToStringBuilderUtils` here or the object's `toString()`.



-- 
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] sonarcloud[bot] commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1204060449

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6581)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL)
   
   [![1.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '1.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_coverage&view=list) [1.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1201040297

   @SadiJr this sounds like a good enhancement. Did you test the backup functionality on VMs that had volumes attached/detacjhed most particularly; what if a volume is detached and after that a restore is done to a point where it was still attached; does this work?


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1205810178

   <b>Trillian test result (tid-4619)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43275 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6581-t4619-vmware-67u3.zip
   Smoke tests completed. 100 look OK, 1 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_02_upgrade_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_04_autoscale_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_05_basic_lifecycle_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_06_delete_kubernetes_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   test_09_delete_kubernetes_ha_cluster | `Failure` | 0.01 | test_kubernetes_clusters.py
   


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1207073139

   <b>Trillian test result (tid-4626)</b>
   Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 42221 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6581-t4626-vmware-67u3.zip
   Smoke tests completed. 101 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] acs-robot commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1203991034

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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] acs-robot commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1198542191

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1203998593

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6581 (SL-JID-2071)


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1398295134

   @DaanHoogland a Trillian-Jenkins test job (rocky8 mgmt + vmware-70u3) 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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1339533979

   @SadiJr will you consider the coments as well?


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1204887231

   @DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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] DaanHoogland commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1206307162

   @blueorangutan test centos7 vmware-67u3


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1203991924

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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] github-actions[bot] commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1274957471

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
SadiJr commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1203194304

   @DaanHoogland I tested to make backups of one VM, then attach a new volume, and make the backup again, to make sure this new attached volume appears in Veeam. I do the same test, but detaching the volume and make another backup. The full restore, for my tests, works well. The volume restore, for reasons I don't now, restores the entire VM with another name and then detach and attach the selected volume to the VM but, even if this process sounds wrong to me (as Veeam has the function to restore only a specific volume to a specific VM), in my tests, it worked correctly. But I would particularly be happy if someone else could also test it to ensure consistency


-- 
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 #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
SadiJr commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1384055687

   @GutoVeronezi done, thanks for the suggestions. @DaanHoogland done to, thanks for the advice.


-- 
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 a diff in pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#discussion_r1071266959


##########
server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java:
##########
@@ -1164,6 +1166,65 @@ public void doesTargetStorageSupportDiskOfferingTestDiskOfferingTagsEqualsStorag
         Assert.assertTrue(result);
     }
 
+    @Test
+    public void validateIfVmHaveBackupsTestExceptionWhenTryToDetachVolumeFromVMWhichBackupOffering() {

Review Comment:
   ```suggestion
       public void validateIfVmHaveBackupsTestExceptionWhenTryToDetachVolumeFromVmWithBackupOffering() {
   ```
   ?
   It sounds strange as it is written 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] sonarcloud[bot] commented on pull request #6581: [Veeam] enable volume attach/detach in VMs with Backup Offerings

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6581:
URL: https://github.com/apache/cloudstack/pull/6581#issuecomment-1384133075

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6581)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6581&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6581&resolved=false&types=CODE_SMELL)
   
   [![17.6%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '17.6%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_coverage&view=list) [17.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6581&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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