[BOLT-72] Don't invoke new powershell instances Created: 2017/09/08 Updated: 2017/11/29 Resolved: 2017/11/29
|Project:||Puppet Task Runner|
|Fix Version/s:||BOLT 0.9.0|
|Remaining Estimate:||Not Specified|
|Time Spent:||Not Specified|
|Original Estimate:||Not Specified|
|Release Notes:||New Feature|
|Release Notes Summary:||This improves performance of the WinRM transport by running PowerShell-based Bolt scripts and tasks in the connected WinRM session, rather than creating new PowerShell processes. This also fixes an issue with properly reflecting failure status on PowerShell versions older than PowerShell 5. Users should not rely on variables set between commands persisting, even those set in the $global scope. Users should not rely on [Console]::WriteLine to produce output from PowerShell code.|
|QA Risk Assessment:||Needs Assessment|
When executing a script or task, we copy the file to the remote system over winrm, and then invoke powershell.exe on the file. Invoking powershell is slow, and we could instead eval the script contents in our current powershell instance.
However, to do that we need to make sure we clear any state from a previously executed command or script, which includes clearing PT_ environment variables. Longer term we probably need to implement runspaces like we do for the powershell module. But for this ticket, I think it's ok to just clear environment variables and no longer invoke another powershell instance.
|Comment by Josh Cooper [ 2017/09/08 ]|
For example, we should be able to change:
|Comment by Ethan Brown [ 2017/11/07 ]|
Note that this as currently described, this runs a bit counter to
I believe that (after some research into how "isolated" script executions are given the current model), that this ticket should be about more generally optimizing the PowerShell workflow for executing scripts / tasks.
|Comment by Ethan Brown [ 2017/11/07 ]|
I put together a few quick tests around understanding if there is bleed across invocations, but I think some more work should be done to fully vet.
It appears that the between the PowerShell ServerRemoteHost using a RunspacePool when executing PowerShell code, and the winrm gem performing runspace management, that each command invocation is isolated already.
I ran some simple tests with Bolt over command, script and task execution, and noticed that no state was shared from one command to the next. Note that in this case, script and task were both running against new powershell.exe instances, but the behavior of command is enough to conclude that state is not shared.
So in broader terms, I think that we can move forward on simplifying the PowerShell execution mechanism so that Invoke-Interpreter is not called under the hood to launch a new powershell.exe for the sake of isolating runs. This also means that we shouldn't need anything like the runspace manager in the PowerShell module at https://github.com/puppetlabs/puppetlabs-powershell/blob/master/lib/puppet_x/templates/powershell/init_ps.ps1 as runspaces are already being managed. It seems reasonable to assume that ResetRunspaceState is already called on the Runspaces as they're handed out ... but worst case scenario we may have to perform some up-front Runspace cleanup like what's done in the PowerShell module at https://github.com/puppetlabs/puppetlabs-powershell/blob/d0166e4ec6b7e6f1abdb21f9efb439a700940ee1/lib/puppet_x/templates/powershell/init_ps.ps1#L414-L463
When this change is made, some tests should be added to show state doesn't bleed (we may have to ask the winrm library to use some well-known GUIDs to get the same runspace out of the pool)
|Comment by Ethan Brown [ 2017/11/21 ]|
Determined that it's still best to send the entire file over rather than doing an inline embed / eval, given some of the concessions that have to be made for that to work.
From notes on the PR
This PR also fixes handling of exit status properly when run on PowerShell < 5. There is no ticket filed, and we're currently not testing on PS < 5, though I did put together a PR that was attempting to downgrade PowerShell to 4 in AppVeyor for running tests - https://github.com/puppetlabs/bolt/pull/162. So far, that attempt has been a failure, so it might be simplest to enable tests in Jenkins / vmpooler for downlevel PowerShell.
|Comment by Michelle Fredette [ 2017/11/28 ]|
added new feature release note.