Skip to content

Fix autocomplete of migrateSystemVm #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2025
Merged

Conversation

vits-hugs
Copy link
Contributor

@vits-hugs vits-hugs commented Mar 17, 2025

Description

Right now, the autocomplete of the storageid parameter on the migrateSystemVm API is returning system VMs IDs instead of the expected storage IDs, as shown below. This bug also sets the CloudMonkey autocomplete cache for others APIs, such as migrateVirtualMachine.

Apache CloudStack 🐵 CloudMonkey 6.4.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues

(localcloud) 🐱 > migrate systemvm storageid=
0124fcb6-ca16-421e-b332-d1438f63ac5a (v-12-VM)     159e8d32-5f74-41b1-ba17-8b0e734143f0 (s-14-VM)   
(localcloud) 🐱 > migrate virtualmachine storageid=
0124fcb6-ca16-421e-b332-d1438f63ac5a (v-12-VM)     159e8d32-5f74-41b1-ba17-8b0e734143f0 (s-14-VM)    

This PR corrects this behavior by adding a verification so that the autocompleteAPI struct only changes its Name member to listSystemVms when it's defined to listVirtualMachines, thus not calling the listSystemVms API to autocomplete the storageid parameter.

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)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale

  • Major
  • Minor

How Has This Been Tested?

On CloudMonkey the autocomplete of all parameters in migrateSystemVm was called, and verified to be the correct answer, as shown below.

Apache CloudStack 🐵 CloudMonkey 6.4.0
Report issues: https://github.com/apache/cloudstack-cloudmonkey/issues

(localcloud) 🐱 > migrate systemvm storageid=
10d65800-bbda-347a-bbb0-250e6d7d1586 (nfs-1)                          817d7155-a725-4cf7-b2bb-bc14b238eb4a (iscsi-1)       
(localcloud) 🐱 > migrate systemvm virtualmachineid=
0124fcb6-ca16-421e-b332-d1438f63ac5a (v-12-VM)                        159e8d32-5f74-41b1-ba17-8b0e734143f0 (s-14-VM)      

Copy link
Member

@winterhazel winterhazel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested manually. I verified that:

  • Before the patch, storageid autocompletes to system VM UUIDs
  • After the patch, storageid autocompletes to primary storage UUIDs
  • After the patch, the other parameters (autoselect, hostid, virtualmachineid) were not affected

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rohityadavcloud rohityadavcloud added this to the 6.5.0 milestone Mar 27, 2025
@rohityadavcloud rohityadavcloud merged commit 91f5600 into apache:main Mar 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants