(maint) Minor string allocation optimization
- Don't redefine string containing custom functions for runspace on each call. Share one global string.
* stable: (FM-5344) Prepare for release 2.0.2
Merge pull request #132 from MosesMendoza/MODULES-3640/windows/add_js…
…on_pure_restriction (MODULES-3640) Update modulesync 30fc4ab
* stable: (MODULES-3640) Update modulesync 30fc4ab
(MODULES-3399) Exit user scripts via $LASTEXITCODE
- Previously in v1 of the module, if the call operator & was used to execute an external script, and that script performed an "exit 1", that exit code would propagate. This technique could be used within an unless or onlyif to ensure idempotent behavior like: unless => '& exit-one.ps1' This would allow users to create fully reusable PowerShell scripts that could be invoked outside of the Puppet environment during development and testing. - Since code is invoked differently in the v2 module, that behavior changed, and any exit codes set externally in a PS script invoked by the call operator are not propagating. The custom PowerShell host SetShouldExit method is not being triggered in this situation. This commit restores the old behavior by performing an exit $LASTEXITCODE after executing the user code. NOTE: this has the side effect of returning an exit code that may be unexpected, based on the last external command invoked by PowerShell. Users will be expected to understand this when developing longer more complex scripts.
Merge pull request #129 from Iristyle/ticket/stable/MODULES-3399-set-…
…call-operator-exit-status (MODULES-3399) Exit user scripts via $LASTEXITCODE
* stable: (MODULES-3399) Exit user scripts via $LASTEXITCODE (maint) Minor string allocation optimization
(maint) Fix test failures for Puppet 4.6
- Some internal changes to transaction behavior that are part of Puppet 4.6, have induced a new problem with the existing tests that invoked the compiler. Transactionstore.yaml files are now produced when a transaction is processed, for the sake of identifying changes as part of the `corrective changes` feature. Without the Puppet[:statedir] explicitly set, compiling a catalog and executing a transaction will attempt to write a YAML file to c:\dev\null. Instead, explicitly set the path to allow for a real file to be written as part of the transaction.
Merge pull request #135 from Iristyle/maint/stable/PUP-4.6-fixes-tran…
…saction-persistence (maint) Fix test failures for Puppet 4.6
Merge pull request #134 from glennsarti/ticket/stable/modsync
(maint) modulesync 70360747
* stable: (maint) Fix test failures for Puppet 4.6 (maint) modulesync 70360747
(MODULES-3775) (msync 8d0455c) update travis/appveyer w/Ruby 2.3
Merge pull request #137 from MosesMendoza/MODULES-3775/stable/update_…
…appveyor_travis_to_ruby_23 (MODULES-3775) (msync 8d0455c) update travis/appveyer w/Ruby 2.3
Merge remote-tracking branch 'origin/stable'
* origin/stable: (MODULES-3775) (msync 8d0455c) update travis/appveyer w/Ruby 2.3
(MODULES-3565) Change the working directory if specified in the resource
Previously the cwd resource parameter was not being used during powershell script execution. This commit checks to see if the cwd parameter is specified on the resource and if so, modifies the `SessionStateProxy.Path.SetLocation` to use the specified location, otherwise it sets the location to the original working directory. The original working directory the current directory of the process which creates the powershell manager i.e. Puppet. The working directory is not modified across runspaces. This commit adds tests to ensure that an error is raised if a non-existant working directory is specified and that the working directory is set during the powershell session.
(maint) Fix stream redirection integration test
The integration test for redirecting streams was using the Warning stream. Redirecting the warning stream is not supported in PowerShell 2.0. This commit changes the source stream to the error stream which is compatible from version 2.0 and above.
(MODULES-3709) Respect resource timeout interval in powershell manager
Previously, the a timeout value could be specified in a resource however that value was ignore and the default of 300s was used by the powershell manager. This commit passes throught the timeout value from the resource into the powershell manager and also adds an acceptance test to verify this behaviour.
(MODULES-3588) Update documentation for change in MODULES-3399
The behavior about $LASTEXITCODE was modified in MODULES-3399 however there was no documentation to demonstrate the change. This commit modifies the README with the new behavior and example code. [ci skip]
Merge pull request #136 from glennsarti/ticket/master/MODULES-3709-ti…
…meout (MODULES-3709) Respect resource timeout interval in powershell manager
Merge branch 'master' into stable
* master: (MODULES-3709) Respect resource timeout interval in powershell manager (maint) Fix stream redirection integration test (MODULES-3565) Change the working directory if specified in the resource
(MODULES-3443) Emit better user code exceptions
Previously any error in a user supplied code block would error with `Exception calling "EndInvoke" with "1" argument(s): ....` however this is not very useful and hides required debugging information for users. This commit: - Emits the inner exception on user exit code if it exists. The inner exception is what caused the EndInvoke to fail and contains the true reason why an error was thrown - Modifies the error string output to emit the `ErrorRecord.InvocationInfo` which contains the line number and offset where the error occured and an excerpt of the offending line - Adds tests to expect correct messages for simple errors
Merge pull request #139 from glennsarti/ticket/master/MODULES-3443-er…
…rors (MODULES-3443) Emit better user code exceptions
Merge pull request #138 from glennsarti/ticket/master/MODULES-3588-do…
…c-script-exit-codes (MODULES-3588) Update documentation for change in MODULES-3399
(MODULES-3839) Emit better user code parsing exceptions
- Previously in MODULES-3443 the exception handler was updated to display more meaningful errors. However, one type of error was not handled correctly. If PowerShell generated an `IncompleteParseException` error, the message displayed to the user would suggest it was present at `throw $_.Exception.InnerException` in a line and offset that probably didn't exist. This is because unlike the `ParseException` type which gives useful information, an `IncompleteParseException` can not determine where in the script the error occurred. This commit changes catches the `IncompleteParseException` error and rethrows it as a simple text error. This suppresses the `line:xxx char:xx` text from being shown to the user, while still displaying the root cause of the error. This commit refactors the tests slightly so that different error types are exercised; Runtime, ParseException and IncompleteParseException.
Merge branch 'pr/141' into stable
* pr/141: (MODULES-3839) Emit better user code parsing exceptions closes #141
(maint) Fix Invoke-PowerShellUserCode output
- 0fffd4d changed the behavior of Invoke-PowerShellUserCode so that a working directory could be passed in. Unfortunately, it also changed the return value to include a PathInfo instance returned by the call to SessionStateProxy.Path.SetLocation. Ensure that value is not returned from the function.
(maint) Refactor PS event code to Signal-Event
- There are two codepaths that signal a named event. Build a cmdlet to encapsulate that functionality. - If a DebugPreference is set other than SilentlyContinue, system level debug output is emitted (and viewable by DbgView)
(maint) Don't stdin bootstrap powershell_manager
- Previously an ERB template was used to produce a PS file with a single variable embedded, and then that content is pushed over stdin (where it's subject to PowerShell behavior for input parsing) Not only is it slower to use, but there may be other additional interactive specific features that change stdin parsing behavior. WMF 5.1 (anniversary update), changed stdin parsing behavior for instance. Instead use the more first class PowerShell parameter parsing with a static PS1 file. This should provide a minor performance boost. This is another change needed on the path to refactoring stdin param passing.
(maint) Create Write-SystemDebugMessage function
- Refactor the Signal-Event debug stream writing to a separate function, and continue to guard it with the $DebugPreference. But further, add a $EmitDebugOutput switch to the PowerShell script that can also be used to produce this same debug output. Inside the Ruby code, pass this parameter through inline with the Puppet::Util::Log.level - when set to debug, enable the PS logging automatically. An alternate approach would have been to set the $DebugPreference to 'Continue', but that could impact the behavior of existing scripts. Consider this change at a later point, but make the safer change now so that a user parameter can be controlled independently of $DebugPreference This will become more useful later for emitting debug output for custom pipe IPC
Merge pull request #143 from Iristyle/maint/stable/minor-Invoke-Power…
…ShellUserCode-fix-and-signal-refactor (maint) Minor PS tweaks in prep for pipe rewrite
(MODULES-3875) Introduce new 64k pipe test
- Existing test verified that 64k generated within PowerShell could be sent back to Ruby without deadlocking the Ruby parent process. Add a new test that originates 96k on the Ruby side and ensures that the data is properly round-tripped back to Ruby after passing it over stdin to PowerShell (which is implemented under the hood with popen3 via a named pipe).
(MODULES-3875) Defensively exit PowerShellManager
- Previously it was possible to call exit against PowerShellManager and not have it completely terminate due to attempting to write to broken pipes, etc. Now verify state prior to attempting to close since Ruby will raise an IOError if calling close against an already closed IO instance Also ignore any errors raised when trying to 'exit' the instance
(MODULES-3875) Refactor stream error handling
- Instead of having write_sdin / drain_pipe contain their own
exception handlers, unify them under exec_read_result.
This also correct two errors in how existing errors were raised:
* The backtrace of the rescued exception was lost
* The message would often include "Operation successfully completed"
as the Windows::Error class is intended to be used after failures
in FFI calls to Win32 APIs, which automatically call GetLastError
to look up the reason for the API call failure. However, since
these errors are not the result of Windows API failures, the return
value of GetLastErorr is 0 and the message is misleading.
This will later allow other I/O and Pipe errors to be captured and
handled in a single location and prevent code duplication.(MODULES-3875) Add PowerShellManager.instance test
- This new test documents how the PowerShell provider makes use of the PowerShellManager. Specifically, it uses the .instance factory method and provides a command line. There were no existing tests that proved that the manager returns the same manager instance, bound to the same underlying PowerShell process - which more accureately represents how the provider uses the PowerShellManager These tests will be further expanded with new manager robustness code.
(MODULES-3875) PowerShellManager heals on failure
- Previously, an error that took out the PowerShell process could
render the module unable to process any more PS commands - and
anything else in the Puppet run using the module would fail.
A simple way to reproduce this is by explicitly calling:
[Diagnostics.Process]::GetCurrentProcess().Kill()
Or through a manifest such as:
exec { 'KillPS':
command => '([System.Diagnostics.Process]::GetCurrentProcess()).Kill()',
provider => powershell,
logoutput => true,
}
This could also happen if a pipe breaks with EPIPE, or if any of
the IO streams are closed to a bad file descriptor (EBADF) or a
more generic IOError.
- Error handling for IO reads / writes is unified under the
exec_read_result method. All EPIPE errors, IOError 'closed stream'
and EBADF (bad file descriptor) errors are considered non-terminal,
and simply inform the PowerShellManager that the current instance is
unusable and the next time PowerShellManager.instance is called that
a new PowerShell process should be created. It also ensures that the
exit code from the execution is a -1, with the stderr containing the
exception text and backtrace, which propagate through the provider
and show up as additional info when --debug is set.
- PowerShellManager.instance does this by explicitly calling an alive?
helper which makes sure the process is running and all the streams
are open and stat'able (stat was the only Ruby method that would
generate an error when the underlying file descriptor or handle was
bad). When they are not, a new PowerShell process is created and
this effectively self-heals between command execution.
- alive? is also used when reading the PowerShell stdout / stderr and
waiting for PowerShell to have signaled that the output is ready.
If this check is not performed, the PS process can die (as is the
case with the new tests), prior to signaling the event that Ruby
is waiting on, which deadlocks Puppet.
- Add a number of nefarious tests that tear down the underlying
PowerShell process unexpectedly or that close the stdin, stdout or
stderr streams in unexpected ways. Depending on Ruby version and
the sequence of events these situations have slightly different
failure modes which are captured in the tests.Merge pull request #144 from Iristyle/maint/stable/new-test-for-64k-i…
…nput-to-PS (MODULES-3875) Improve PowerShellManager resilience to failure
(maint) Remove unnecessary newlines
- In prior revisions with code being sent over stdin, it was necessary to send newlines to ensure the PowerShell input handling would parse user code. This hasn't been true for a while.
(maint) Remove unfiltered exception handler
- When processing command exectuion with exec_read_result, generic unfiltered exceptions were caught, only for the sake of adding an additional message / re-raising. Not only do this message add little value, but this approach was actually losing vital information from the original e.message that could actually pinpoint problems in the module. In Ruby 2.1, there is a better way of re-raising an exception with a nested exception (which would make additive behavior easier to implement), but since this module must support older Ruby versions the easiest thing to do is to remove the unneeded handler.
(MODULES-3144) Drain stdout / stderr in separate threads
- Instead of using a flawed method of alternating reads from stdout and stderr, rewrite the read_stdout method so that it uses individual threads to drain pipes, and synchronizes with a Mutex. Upon receiving a signal from PowerShell on the output ready event, ask the two threads to exit by unlocking the Ruby Mutex. That allows each thread to have .value called against it (which blocks until the thread terminates cleanly). Any exceptions from the threads still propagate as they did in the past when .value is called. - Necessarily refactor the is_readable? method so that each stream can check individually in it's own thread while draining, rather than making that part of the main event waiting loop. This continued stat'ing of the stream is a bit more expensive, but allows for more opportunity to propagate an EPIPE. Slightly rework the liveness check introduced in f7c08ef so that the main wait loop checks for process liveness and thread liveness. With no running child process there will never be a signal sent that will end the infinite loop. With the child process terminated, Ruby may still have valid file desriptors (which map to valid open Windows handles), that may never be considered readable by IO.select. This usually prevents any reads from occurring that might normally raise an EOFError. Also monitor the thread states to see if they're alive? and exit the wait loop when they error, assuming the call to .value will raise an exception after exiting the wait loop. - This has a nice side effect of guaranteeing that a pipe drain always returns an array of strings with nils removed, that can easily be concatenated to form a single string value for the first return value (output) from exec_read_result, and can be left untouched for the second return value (errors) to remain consistent with prior expectations
(MODULES-3144) Calculate wait time differently
- Use Time.now to measure the total time waited
(MODULES-3144) Redirect PS stderr to TextWriter
- Redirect [System.Console]::Error inside the PowerShell host, so that it can be passed back to the calling Ruby process via XML, like other output and the exit code. - Even with Console.Error redirected, it's still possible for native applications to write directly to the real stderr stream, which means that it must continue to be drained to prevent deadlocks when the stderr buffer is full. - Add note to documentation about [System.Console]::Error
(maint) Change PowerShellManager exit behavior
- Use of Timeout.timeout could be dangerous. Some messages were set but never used. - Trying to kill a process by PID is problematic because a race condition exists such that the waiter thread may die, the PID may be reclaimed by another process and the wrong process could be terminated. While the likelihood of this is low, such process termination should be avoided. Ruby will terminate it's child processes on exit anyhow. - Furthermore, the exit status of the parent powershell.exe process is not particularly useful in this case, as users are concerned with the exit values of their code - which is not what this value represents. - Assume that when exit has been requested, it will succeed and this manager instance will never be used again.
Merge pull request #145 from Iristyle/ticket/stable/MODULES-3144-drai…
…n-pipes-in-threads (MODULES-3144) Drain stdout / stderr in separate threads
(MODULES-3144) Fix race condition draining pipes
- The code as implemented was subtly flawed. When draining pipes in separate threads, reads proceed until PS has signaled that it has executed code and written output. Code existed to drain the pipe in case that race occurred. However, the code only performed a single read with the Ruby IO.gets method, which reads until the next newline character. The console width is defined as 120 characters, so a block of XML written as a result could end up split up / incomplete. - Instead, the code should have been reading while the pipe was considered readable, ensuring that it reads all remaining data after receiving a completion signal. - This bug was trivially reproducible on Windows 2008R2 + PS2, but not PS5. With the fix in place, the problem no longer occurs.
(MODULES-3443) Modify error test for differences in Powershell
Previously the integration tests for testing error behavior were passing on Powershell 3+ however the error parsing is different in Powershell 2.0. This commit modifies the test expectations for a line and char to returned but the content of the line and char values are ignored. This means the tests will now pass on all Powershell versions but still ensure the correct behavior.
(maint) Skip integration tests on PowerShell version 2.0 or less
Currently the integration tests are failing on PowerShell version 2.0 or less. This commit adds a helper function to determine the Powershell version without the Powershell Manager code, and then skips the PowerShell Manager integration tests if the PowerShell version is 2.0 or less.
(maint) Fix integration for case insensitive paths
Previously the tests would fail if the path differed with casing however under Windows paths are not case sensitive. This commit modifies the test by changing the string comparison to lowercase.
Merge pull request #148 from glennsarti/ticket/stable/MODULES-3443-fi…
…x-tests (MODULES-3443) Modify error test for differences in Powershell version