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

Use Winrm v2 and let winrm-fs do file uploads #543

Merged
merged 1 commit into from
Sep 7, 2016
Merged

Conversation

mwrock
Copy link
Contributor

@mwrock mwrock commented Sep 2, 2016

This PR accomplishes 3 things:

  • Updates WinRM gem usage to 2.0 API
  • Replaces file upload code with wintm-fs (same code used by test-kitchen, inspec and vagrant) with its 1.0 implementation its orders of mafnitude faster the the former implementation and no longer constrained to 8k chunks.
  • Use one powershell based shell throughout an entire converge. This is simpler, faster and follows the same approach as test-kitchen.

@mwrock mwrock changed the title WIP: Use Winrm v2 and let winrm-fs do file uploads Use Winrm v2 and let winrm-fs do file uploads Sep 3, 2016
@mwrock
Copy link
Contributor Author

mwrock commented Sep 3, 2016

No longer WIP and tested using chef-provisioning-vsphere

@mwrock
Copy link
Contributor Author

mwrock commented Sep 6, 2016

cc @tyler-ball @jkeiser

@tyler-ball
Copy link
Contributor

Thanks @mwrock !

@tyler-ball tyler-ball merged commit a56abd8 into master Sep 7, 2016
This was referenced Sep 7, 2016
Copy link
Contributor

@hh hh left a comment

Choose a reason for hiding this comment

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

We lost the ability to run powershell scripts after this PR, I commented on some possible fixes that will get us up and running again.

else
command_executor.run_powershell_script(command, &block)
end
session.run(command, &block)
Copy link
Contributor

Choose a reason for hiding this comment

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

@mwrock @tyler-ball we need to default to session.run_powershell_script and only use session.run_cmd if raw options are passed the :raw options
/cc @ajeba99

Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -41,8 +41,7 @@ def setup_convergence(action_handler, machine)

action_handler.open_stream(machine.node['name']) do |stdout|
action_handler.open_stream(machine.node['name']) do |stderr|
machine.execute(action_handler, "powershell.exe -ExecutionPolicy Unrestricted -NoProfile \"& \"\"#{convergence_options[:install_script_path]}\"\"\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

@mwrock @tyler-ball this is a must have! We need ExecutionPolicy Unrestricted in order to run powershell
/cc @ajeba99

require 'winrm'
::WinRM::WinRMWebService.new(endpoint, type, options)
::WinRM::Connection.new(options).shell(:powershell)
Copy link
Contributor

Choose a reason for hiding this comment

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

Setting shell to powershell here may get rid of the need for the :raw block above

@mwrock
Copy link
Contributor Author

mwrock commented Oct 17, 2016

Hi @hh can you provide an example of what does not work? We made the breaking change in 2.0 to no longer support :raw and now all transport commands use a powershell shell.

@hh
Copy link
Contributor

hh commented Oct 17, 2016

It's not so much around the :raw, it's more around the ExecutionPolicy

In https://github.com/chef/chef-provisioning/pull/543/files/45e94cc3dbd716c9dc6e380dbd94adf9aa15b28e#diff-72ecfcec074b5e5bdfc04d91922a92fcL44 we lost:

   machine.execute(action_handler, "powershell.exe -ExecutionPolicy Unrestricted -NoProfile \"& \"\"#{convergence_options[:install_script_path]}\"\"\"",

Which since chef-dk 0.18.26 results in:

    - write file C:\chef\install.ps1 on base-2012r2-2016-08-01
    [base-2012r2-2016-08-01] File C:\chef\install.ps1 cannot be loaded because running scripts is disabled on this system. For more information, see about_Execution_Policies at http://go.microsoft.com/fwlink/?LinkID=135170.
                             At line:1 char:3
                             + & "C:\chef\install.ps1"
                             +   ~~~~~~~~~~~~~~~~~~~~~
                                 + CategoryInfo          : SecurityError: (:) [], PSSecurityException
                                 + FullyQualifiedErrorId : UnauthorizedAccess


@hh
Copy link
Contributor

hh commented Oct 17, 2016

#550 should fix it

@mwrock
Copy link
Contributor Author

mwrock commented Oct 17, 2016

Ah I see. So the big difference with powershell in winrm v2's :powershell shell is that it takes you directly to a powershell session and does not go through a cmd.exe process. So downfall of calling powershell.exe from a powershell shell is you are creating another powershell process from a powershell process. There are 2 ways to work around this scenario:

  1. Make sure your base images set a reasonable execution timeout
  2. Call Set-ExecutionPolicy unrestricted as the first command. However this will persist

@ajeba99
Copy link

ajeba99 commented Oct 18, 2016

Added #551

bglimepoint pushed a commit to bglimepoint/chef-provisioning that referenced this pull request Oct 29, 2018
Use Winrm v2 and let winrm-fs do file uploads
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants