RodrigoDLopez opened a new pull request #4257:
URL: https://github.com/apache/cloudstack/pull/4257


   ## Description
   
   With this commit [Fix limitation on tag matching in 'migrateVolume' with 
disk 
offering](https://github.com/apache/cloudstack/commit/756a7e89cbcec7a1aa98b3f470cbdf8181af75ad)
 the author changed the volume migrate API to allow operators to override the 
volume placement. Moreover, the tag validation would only happen if the 
operator changes the disk offering. However, further, but with this commit 
[Vmware offline 
migration](https://github.com/apache/cloudstack/commit/b363fd49f70ac2092ebe6226a72a3d911dc99e1f),
  cloudstack started to validate the tags again when the operator executes the 
volume migrate command without. This broke the override mechanism that has been 
previously introduced into ACS.
   
   If the operator wants to migrate volumes, without changing the disk 
offering, we don't need to check if the tag match with the storage pool tag. As 
discussed merged in the past, the operator should be able to override the disk 
placement (without changing the disk offering). So this PR removes this 
unnecessary check for tags preventing `Unexpected exception while executing 
MigrateVolumeCmd`.
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviors, steps to 
reproduce. -->
   **Steps to reproduce**
   To reproduce this error is necessary:
   * DO: a disk offering with tag 'A';
   * SP1: a Storage pool with tag 'A';
   * SP2: a Storage pool with tag 'B';
   * Create a volume using 'DO' disk offering, this will be allocated at SP1;
   * Try to migrate this volume to SP2
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be 
closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   ## Types of changes
   <!--- What types of changes does your code introduce? Put an `x` in all the 
boxes that apply: -->
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ## How Has This Been Tested?
   To run tests, I used `cloudmonkey` to migrate volumes
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to 
-->
   <!-- see how your change affects other areas of the code, etc. -->
   
   **Without this PR**
   We can not migrate de volume, because of that unnecessary verification
   ```
   () 🐱 > migrate volume livemigrate=true 
volumeid=a0f4e7f8-56d2-408b-9ef4-34461ff59b93 
storageid=5154b842-5d4f-3589-af96-b3e0f6b82e55
   {
     "accountid": "d9feed3f-88a1-11ea-85e0-525400ab8632",
     "cmd": 
"org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin",
     "completed": "2020-08-07T11:09:06-0400",
     "created": "2020-08-07T11:09:05-0400",
     "jobid": "6d6b3880-54a1-4345-a178-323182c67b12",
     "jobprocstatus": 0,
     "jobresult": {
       "errorcode": 530,
       "errortext": "Migration target pool [null, tags:ssd2] has no matching 
tags for volume [ROOT-90, uuid:a0f4e7f8-56d2-408b-9ef4-34461ff59b93, tags:ssd]"
     },
     "jobresultcode": 530,
     "jobresulttype": "object",
     "jobstatus": 2,
     "userid": "d9ffc914-88a1-11ea-85e0-525400ab8632"
   }
   ```
   
   **With this changes**
   All good, as expected.
   ```
   () 🐱 > migrate volume livemigrate=true 
volumeid=a0f4e7f8-56d2-408b-9ef4-34461ff59b93 
storageid=5154b842-5d4f-3589-af96-b3e0f6b82e55
   {
     "volume": {
       "account": "admin",
       "clusterid": "a593b11e-d89e-41a3-9626-88250d7ca927",
       "clustername": "Cluster",
       "created": "2020-08-07T11:15:18-0400",
       "destroyed": false,
       "deviceid": 0,
       "displayvolume": true,
       "domain": "ROOT",
       "domainid": "7243f453-889e-11ea-85e0-525400ab8632",
       "hypervisor": "KVM",
       "id": "a0f4e7f8-56d2-408b-9ef4-34461ff59b93",
       "isextractable": false,
       "name": "ROOT-90",
       "path": "53f80541-0819-4628-9318-3c570b5e9656",
       "podid": "3a0f99ab-c0f4-4317-bdfa-f1c0396acb76",
       "podname": "Pod",
       "provisioningtype": "thin",
       "quiescevm": false,
       "serviceofferingdisplaytext": "tiny with tag",
       "serviceofferingid": "18b614bf-9104-4bc0-98b0-24cecdfb2a41",
       "serviceofferingname": "tiny with tag",
       "size": 214749184,
       "state": "Ready",
       "storage": "primary2",
       "storageid": "5154b842-5d4f-3589-af96-b3e0f6b82e55",
       "storagetype": "shared",
       "tags": [],
       "templatedisplaytext": "tiny",
       "templateid": "ee7531b0-9d6f-4021-a17f-07ceb183b6d6",
       "templatename": "tiny",
       "type": "ROOT",
       "virtualmachineid": "9ae4614f-2917-4eb1-a0ff-752db65cf624",
       "vmdisplayname": "VM-9ae4614f-2917-4eb1-a0ff-752db65cf624",
       "vmname": "VM-9ae4614f-2917-4eb1-a0ff-752db65cf624",
       "vmstate": "Stopped",
       "zoneid": "9ee874c2-0425-4d65-aa09-6ab7a9d1040e",
       "zonename": "Zone"
     }
   }
   ```
   <!-- Please read the 
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md)
 document -->


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to