Skip to content
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

(POOLER-70) Refactor clone_vm to take pool configuration object #189

Conversation

glennsarti
Copy link
Contributor

Previously, the clone_vm method took various VSphere specific parameters e.g.
template folder. However in order make VMPooler less VSphere specific this
method should just take the pool configuration and then it can determine the
appropriate settings itself. This commit also moves the threading to a clone_vm
while the actual method which does the work is now _clone_vm as per all other
multithread worker methods in pool_manager. This commit also updates the spec
tests appropriately.

@glennsarti
Copy link
Contributor Author

The diff for this looks strange because clone_vm has been re purposed. All of the worker code was moved to _clone_vm

Previously, the clone_vm method took various VSphere specific parameters e.g.
template folder.  However in order make VMPooler less VSphere specific this
method should just take the pool configuration and then it can determine the
appropriate settings itself.  This commit also moves the threading to a clone_vm
while the actual method which does the work is now _clone_vm as per all other
multithread worker methods in pool_manager.  This commit also updates the spec
tests appropriately.
@glennsarti glennsarti force-pushed the ticket/maint/rename-moves-and-clonevm branch from fc0d778 to ac7d700 Compare March 2, 2017 05:53
@underscorgan
Copy link
Contributor

This diff is much easier with ?w=1 :D, looks good!

@underscorgan underscorgan merged commit 850919f into puppetlabs:master Mar 2, 2017
@glennsarti glennsarti deleted the ticket/maint/rename-moves-and-clonevm branch March 2, 2017 23:07
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.

3 participants