Implement SSA of network-mounted HyperV virtual disks #6945

jerryk55 · 2016-02-25T18:28:19Z

Add changes to implement running SSA on HyperV virtual disks which are
network mounted on the server.

in the MiqScvmmVm.init_disk_info determine whether a disk is network mounted.
in the Vhdx and Vhd disk modules pass the network attribute when instantiating
a MiqHyperVDisk, and pass the attribute along when getting a parent disk (in the
case of checkpoints/snapshots).
Add the winrm-elevated accessor to MiqWinRM, and make sure MiqHyperVDisk uses the correct
accessor to run remote commands depending upon whether the virtual disk is remote-mounted
or not.
Allow scanning of Microsoft VMs without storage in the DB (See Issue Hyper-V Inventory Doesn't Add Storage for VMs with VHDs on Network Drive #5383)

Prerequisites:

@Fryguy @jrafanie @chessbyte @roliveri Please review, comment, and merge when appropriate.
I will reach out to the winrm-elevated gem owner to see what we can expect from him.

Thanks.

jerryk55 · 2016-02-25T18:58:37Z

(most) Rubocop issues addressed and re-pushed.

jerryk55 · 2016-02-25T19:41:52Z

Added the Gemfile changes for Rubyzip from #6938 otherwise dependencies cannot be met.

miq-bot · 2016-03-01T21:43:27Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

jerryk55 · 2016-03-01T21:57:01Z

@Fryguy @roliveri @djberg96 I have two PRs in to the winrm-elevated gem (I split the original one). One in particular changes the gemspec file to allow us to use a winrm gem of 1.5 or greater - it is set to ~>1.3 right now which will not allow us to use the "executor" methods of winrm 1.5 to speed up our SSA on HyperV.
When I spoke to @Fryguy about this he suggested going this route, adding the PR to the winrm-elevated repo, etc. When I submitted my original PR the owner said he didn't want to tie down the gem to any particular versions - however that is exactly what he is doing now. Unfortunately he has not been responding to my further attempts to clarify his stance.
How long should we wait before adding our own winrm-elevated repo? This PR, which allows us to run SSA on network-mounted disks, waits in the balance.

jerryk55 · 2016-03-02T18:15:58Z

@Fryguy @roliveri winrm-elevated gem changes have been merged into the gem as version 0.2.0. I have changed our Gemfile to point to the new version in this PR and pushed them but we cannot merge this PR until the gem is deployed. Based on the speed with which the owners typically do so, it should be available within the next day or so. Til then the tests for this PR will fail since the gem won't be available.

jerryk55 · 2016-03-03T13:18:25Z

winrm-elevated 0.2.0 has been distributed with the changes required by this PR.
@Fryguy @roliveri @jrafanie @chessbyte
Assuming tests pass this can be reviewed and merged when ready.

chessbyte · 2016-03-03T15:43:34Z

app/models/job_proxy_dispatcher.rb

@@ -281,7 +281,7 @@ def get_eligible_proxies_for_job(job)
    end

    if @vm.storage.nil?
-      unless ['Amazon', 'OpenStack'].include?(@vm.vendor)
+      unless %w(Amazon OpenStack Microsoft).include?(@vm.vendor)


@roliveri would be good (in a future PR) to ask the @vm object a question instead of having the knowledge here of which vendors are supported.

chessbyte · 2016-03-03T15:44:54Z

@jerryk55 any possibility of adding specs? /cc @roliveri

jerryk55 · 2016-03-03T15:49:41Z

@chessbyte I'm looking into what that would take...

roliveri · 2016-03-06T18:06:27Z

gems/pending/Scvmm/miq_hyperv_disk.rb

-    @total_copy_from_remote_time += t2 - t1
-    @total_read_execution_time += Time.now.getlocal - t1
-    buffer
+    until retries == BREAD_RETRIES


You could make this;

(0...BREAD_RETRIES).each do

Then, you wouldn't need to increment and compare the retries variable.

jerryk55 · 2016-03-07T16:53:02Z

@djberg96 or someone else on the @blomquisg team - if I can get your ok on the changes I've made in the ManageIQ::Providers::Microsoft::InfraManager space for this PR that would be helpful. Still looking into specs for this as well as comparison testing for performance.

miq-bot · 2016-03-08T19:30:17Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

jerryk55 · 2016-03-09T21:12:08Z

@djberg96 would still like sign-off on the minor (as I see it) changes made to the provider as mentioned previously.
@chessbyte @roliveri @Fryguy specs added for the interfaces modified by this PR. Have not gone as far as adding CamCorder recordings of filesystems/disks yet.
Review and merge when possible. Thanks.

jerryk55 · 2016-03-09T21:16:32Z

Rubocop spec complaints addressed as well.

Add changes to implement running SSA on HyperV virtual disks which are network mounted on the server. 1) in the MiqScvmmVm.init_disk_info determine whether a disk is network mounted. 2) in the Vhdx and Vhd disk modules pass the network attribute when instantiating a MiqHyperVDisk, and pass the attribute along when getting a parent disk (in the case of checkpoints/snapshots). 3) Add the winrm-elevated accessor to MiqWinRM, and make sure MiqHyperVDisk uses the correct accessor to run remote commands depending upon whether the virtual disk is remote-mounted or not. 4) Allow scanning of Microsoft VMs without storage in the DB (See Issue ManageIQ#5383)

Add rspec tests for various interfaces modified by this feature request, including: 1) Allow a network boolean argument to be passed to the MiqHyperVDisk object 2) Allow creation of the WinRM elevated runner 3) Validate interfaces added to the ManageIQ/Providers/Microsoft/InfraManager/Vm and Template objects. Also move winrm & winrm-elevated gems from the top-level Gemfile to that under gems/pending to allow gems/pending spec tests to succeed when run by Travis.

miq-bot · 2016-03-10T15:11:12Z

<github_pr_commenter_batch />Some comments on commits jerryk55/manageiq@69eaa9e~...a728b9b

Gemfile changes detected. /cc @JPrause @simaishi

miq-bot · 2016-03-10T15:11:21Z

Checked commits jerryk55/manageiq@69eaa9e~...a728b9b with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
23 files checked, 4 offenses detected

app/models/manageiq/providers/microsoft/infra_manager/template.rb

🔶 - Line 9, Col 7 - Style/PredicateName - Rename has_active_proxy? to active_proxy?.
🔶 - Line 13, Col 7 - Style/PredicateName - Rename has_proxy? to proxy?.

app/models/manageiq/providers/microsoft/infra_manager/vm.rb

🔶 - Line 24, Col 7 - Style/PredicateName - Rename has_active_proxy? to active_proxy?.
🔶 - Line 28, Col 7 - Style/PredicateName - Rename has_proxy? to proxy?.

djberg96 · 2016-03-11T23:14:55Z

Looks good to me.

I checked out this branch, added an Azure cloud provider instance, and an SCVMM infrastructure provider instance, and ran a refresh on each. Worked fine for both.

roliveri · 2016-03-12T20:45:01Z

@jerryk55 - just double checking before merge. All the updated GEM dependencies are available?

jerryk55 · 2016-03-14T13:06:37Z

@roliveri yes. However please hold off on the merge for one more check. I was testing on a Raleigh-based appliance that was cobbled together manually and ran into the 1500 operations limit on winrm - which should be fixed by the correct version of the gem. I need to re-confirm that fix.

jerryk55 · 2016-03-15T18:42:33Z

This can be merged. The winrm 1500 ops limit is unaffected by this PR which uses winrm-elevated for running powershell commands (which does not keep the command process running as of this writing).
Thank you @roliveri @djberg96 for your comments.

Implement SSA of network-mounted HyperV virtual disks

miq-bot added the dependencies label Feb 25, 2016

jerryk55 force-pushed the winrm-elevated branch from 0224a0c to e18054a Compare February 25, 2016 18:57

jerryk55 force-pushed the winrm-elevated branch from e18054a to 70b8215 Compare February 25, 2016 19:37

jerryk55 mentioned this pull request Feb 29, 2016

Upgrade RubyZip Gem to 1.2 #6938

Merged

jerryk55 force-pushed the winrm-elevated branch 3 times, most recently from f374397 to 591cf41 Compare March 2, 2016 18:12

jerryk55 force-pushed the winrm-elevated branch from 591cf41 to 83e7e82 Compare March 3, 2016 13:16

chessbyte reviewed Mar 3, 2016
View reviewed changes

roliveri reviewed Mar 6, 2016
View reviewed changes

roliveri added core/smart state enhancement labels Mar 7, 2016

jerryk55 force-pushed the winrm-elevated branch from 83e7e82 to 5c79fed Compare March 9, 2016 21:09

jerryk55 force-pushed the winrm-elevated branch from 5c79fed to 39ef820 Compare March 9, 2016 21:15

jerryk55 force-pushed the winrm-elevated branch 2 times, most recently from 4f8808e to 439a90e Compare March 9, 2016 21:53

jerryk55 added 2 commits March 10, 2016 10:08

jerryk55 force-pushed the winrm-elevated branch from 439a90e to a728b9b Compare March 10, 2016 15:08

chessbyte added a commit that referenced this pull request Mar 15, 2016

Merge pull request #6945 from jerryk55/winrm-elevated

a31aeeb

Implement SSA of network-mounted HyperV virtual disks

chessbyte merged commit a31aeeb into ManageIQ:master Mar 15, 2016

chessbyte added this to the Sprint 38 Ending Mar 28, 2016 milestone Mar 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SSA of network-mounted HyperV virtual disks #6945

Implement SSA of network-mounted HyperV virtual disks #6945

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

miq-bot commented Mar 1, 2016

jerryk55 commented Mar 1, 2016

jerryk55 commented Mar 2, 2016

jerryk55 commented Mar 3, 2016

chessbyte Mar 3, 2016

chessbyte commented Mar 3, 2016

jerryk55 commented Mar 3, 2016

roliveri Mar 6, 2016

jerryk55 Mar 7, 2016

jerryk55 commented Mar 7, 2016

miq-bot commented Mar 8, 2016

jerryk55 commented Mar 9, 2016

jerryk55 commented Mar 9, 2016

miq-bot commented Mar 10, 2016

miq-bot commented Mar 10, 2016

djberg96 commented Mar 11, 2016

roliveri commented Mar 12, 2016

jerryk55 commented Mar 14, 2016

jerryk55 commented Mar 15, 2016

Implement SSA of network-mounted HyperV virtual disks #6945

Implement SSA of network-mounted HyperV virtual disks #6945

Conversation

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

jerryk55 commented Feb 25, 2016

miq-bot commented Mar 1, 2016

jerryk55 commented Mar 1, 2016

jerryk55 commented Mar 2, 2016

jerryk55 commented Mar 3, 2016

chessbyte Mar 3, 2016

Choose a reason for hiding this comment

chessbyte commented Mar 3, 2016

jerryk55 commented Mar 3, 2016

roliveri Mar 6, 2016

Choose a reason for hiding this comment

jerryk55 Mar 7, 2016

Choose a reason for hiding this comment

jerryk55 commented Mar 7, 2016

miq-bot commented Mar 8, 2016

jerryk55 commented Mar 9, 2016

jerryk55 commented Mar 9, 2016

miq-bot commented Mar 10, 2016

miq-bot commented Mar 10, 2016

djberg96 commented Mar 11, 2016

roliveri commented Mar 12, 2016

jerryk55 commented Mar 14, 2016

jerryk55 commented Mar 15, 2016