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

Improve the speed of finding an individual VM #422

Merged
merged 5 commits into from
Mar 19, 2018

Conversation

swalberg
Copy link
Collaborator

@swalberg swalberg commented Mar 15, 2018

This PR makes use of the
ContainerView
to retrieve a cut down list of servers from which the desired object can
be found, after which the object itself can be downloaded. Previously,
we'd download each VM (about 3.5K and several round trips to the server)
to find the VM we want.

From

sean~/play/knife-vsphere (find_speed %)$ time knife vsphere vm disk list myserver
  0          Hard disk 1      nosnapshots_ds4 64.00 GiB

real    1m56.153s
user    0m9.536s
sys    0m1.713s

to

sean~/play/knife-vsphere (find_speed *%)$ time be knife vsphere vm disk list myserver
/Users/sean/.gem/ruby/2.4.1/gems/builder-3.2.2/lib/builder/xchar.rb:111: warning: constant ::Fixnum is deprecated
  0          Hard disk 1      nosnapshots_ds4 64.00 GiB

real    0m2.929s
user    0m1.038s
sys    0m0.236s

This'll have to be retrofitted to the other commands, which we can do
over time.

See vmware-archive/rbvmomi#117 for some more context

Description

Issues Resolved

Check List

  • All tests pass.
  • All style checks pass.
  • Functionality includes testing.
  • Functionality has been documented in the README if applicable

@swalberg swalberg requested a review from jjasghar March 15, 2018 18:32
end

def get_vm_by_name(vmname)
vm = get_all_vm_objects.find { |r| r['name'] == vmname }

Choose a reason for hiding this comment

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

Prefer detect over find.

)]
],
propSet: [
{ type: 'VirtualMachine', pathSet: properties}

Choose a reason for hiding this comment

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

Space inside } missing.

type: 'ContainerView',
path: 'view',
skip: false
)]

Choose a reason for hiding this comment

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

Closing array brace must be on the line after the last array element when opening brace is on a separate line from the first array element.

name: 'traverseEntities',
type: 'ContainerView',
path: 'view',
skip: false

Choose a reason for hiding this comment

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

Trailing whitespace detected.

type: ['VirtualMachine'],
recursive: true)

filterSpec = RbVmomi::VIM.PropertyFilterSpec(

Choose a reason for hiding this comment

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

Use snake_case for variable names.

def get_all_vm_objects(properties = ['name'])
pc = vim_connection.serviceInstance.content.propertyCollector
viewmgr = vim_connection.serviceInstance.content.viewManager
rootFolder = vim_connection.serviceInstance.content.rootFolder

Choose a reason for hiding this comment

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

Use snake_case for variable names.

@@ -0,0 +1,41 @@
# Some helpers for faster searching of the inventory
module SearchHelper

Choose a reason for hiding this comment

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

Extra empty line detected at module body beginning.

This PR makes use of the
[ContainerView](https://www.vmware.com/support/developer/vc-sdk/visdk41pubs/ApiReference/vim.view.ViewManager.html)
to retrieve a cut down list of servers from which the desired object can
be found, after which the object itself can be downloaded. Previously,
we'd download each VM (about 3.5K and several round trips to the server)
to find the VM we want.

From
```
sean~/play/knife-vsphere (find_speed %)$ time knife vsphere vm disk list myserver
  0          Hard disk 1      nosnapshots_ds4 64.00 GiB

real    1m56.153s
user    0m9.536s
sys    0m1.713s
```
to
```
sean~/play/knife-vsphere (find_speed *%)$ time be knife vsphere vm disk list myserver
/Users/sean/.gem/ruby/2.4.1/gems/builder-3.2.2/lib/builder/xchar.rb:111: warning: constant ::Fixnum is deprecated
  0          Hard disk 1      nosnapshots_ds4 64.00 GiB

real    0m2.929s
user    0m1.038s
sys    0m0.236s
```

This'll have to be retrofitted to the other commands, which we can do
over time.

See vmware-archive/rbvmomi#117 for some more context
@swalberg swalberg merged commit 8b1ab19 into chef:master Mar 19, 2018
@swalberg swalberg deleted the find_speed branch March 19, 2018 16:21
@jjasghar
Copy link

🤘

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.

None yet

3 participants