Skip to content
This repository was archived by the owner on Nov 12, 2025. It is now read-only.

Windows script files #950

Open
bostko wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Windows script files #950

bostko wants to merge 1 commit into from
+450 −58

Conversation

@bostko
Copy link
Contributor

@bostko bostko commented Oct 12, 2015

The PR includes #947 and it is
Dependent on cloudsoft/winrm4j#2

Introduces support for executing multiline commands from a script. This is needed so we can have more control over how commands are executed and catch properly their exit code.
Just like in ssh it uploads multiline commands into a script file on the machine and it is executing it from there.
Multiline commands into scripts is available only in WinRmMachineLocation. If we add it to AbstractSoftwareProcessWinRmDriver it will allow writing more complicated Windows Entities.

The most important part here is that I wrote Live tests for executing scripts so we can advice users better how to write Windows scripts properly.

@bostko bostko force-pushed the windows_file_scripts branch from 402b255 to a42ec9a Compare October 13, 2015 19:21
sjcorbett
sjcorbett reviewed Oct 15, 2015
View reviewed changes
core/src/main/java/org/apache/brooklyn/location/winrm/WinRmMachineLocation.java Outdated
Copy link
Contributor

@sjcorbett sjcorbett Oct 15, 2015

Choose a reason for hiding this comment

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

Please configure your IDE to not reorder imports.

@bostko bostko force-pushed the windows_file_scripts branch 3 times, most recently from 36d8ed0 to 1fa3057 Compare October 15, 2015 14:31
@bostko bostko mentioned this pull request Oct 15, 2015
Closed
@bostko bostko force-pushed the windows_file_scripts branch 6 times, most recently from c4c3e9f to 71ba10a Compare October 16, 2015 17:25
@bostko bostko changed the title Windows file scripts Windows script files Oct 16, 2015
@bostko bostko force-pushed the windows_file_scripts branch from 71ba10a to 8d929b3 Compare October 17, 2015 16:01
@ahgittin
Copy link
Contributor

ahgittin commented Oct 28, 2015

@bostko any update on this? it looks like we are still waiting on cloudsoft/winrm4j#2 -- it looks like you addressed @aledsage 's remaining comment about @since there so maybe @rdowner or @alasdairhodge could take a quick look and merge that, cut a new release of that, then bump the version of that used in brooklyn (in this PR?) and then merge this?

@bostko bostko force-pushed the windows_file_scripts branch 2 times, most recently from acc2d97 to 7dad21b Compare October 28, 2015 14:01
@bostko
Copy link
Contributor Author

bostko commented Oct 28, 2015

@ahgittin Yes it is still waiting for cloudsoft/winrm4j#2
I've upgraded the winrm4j version in this PR as well.

@bostko bostko mentioned this pull request Nov 10, 2015
Merged
@bostko bostko force-pushed the windows_file_scripts branch from 7dad21b to 40b5a90 Compare November 18, 2015 22:12
@bostko bostko mentioned this pull request Nov 24, 2015
Merged
@bostko bostko mentioned this pull request Jan 11, 2016
Closed
@bostko bostko force-pushed the windows_file_scripts branch 3 times, most recently from 6e383a5 to 55562c9 Compare January 12, 2016 21:15
@bostko
Copy link
Contributor Author

bostko commented Jan 12, 2016

Can you review it again? @aledsage
There could be some minor comments. I'll review those in the morning when I am fresh.

@bostko
Copy link
Contributor Author

bostko commented Jan 13, 2016

The current implementation of the code and WindowsScriptsWinrmExitStatusLiveTest the test is failing to instantiate runner in other words the driver which is using WinRmScriptTool.
Any ideas how to refactor it so I can have a live test for WinRmScriptTool?

@bostko
Copy link
Contributor Author

bostko commented Jan 13, 2016

Done.
VanillaWindowsProcessWinrmStreamsLiveTest, VanillaWindowsProcessWinrmStreamsLiveTest, WindowsScriptsWinrmExitStatusLiveTest are passing.
Can you review?

@bostko bostko force-pushed the windows_file_scripts branch from 55562c9 to 32dc230 Compare January 13, 2016 15:45
aledsage
aledsage reviewed Jan 15, 2016
View reviewed changes
...ver/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java Outdated
Copy link
Contributor

@aledsage aledsage Jan 15, 2016

Choose a reason for hiding this comment

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

@bostko see @neykov 's comments in cloudsoft/winrm4j#2, saying "we should keep the (List<String>)...".

Copy link
Contributor Author

@bostko bostko Jan 16, 2016

Choose a reason for hiding this comment

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

@aledsage I kept it. It is here

@bostko bostko force-pushed the windows_file_scripts branch 8 times, most recently from f9f96d0 to 8672510 Compare January 18, 2016 22:22
aledsage
aledsage reviewed Jan 19, 2016
View reviewed changes
...oftware/winrm/src/main/java/org/apache/brooklyn/location/winrm/NaiveWindowsScriptRunner.java Outdated
Copy link
Contributor

@aledsage aledsage Jan 19, 2016

Choose a reason for hiding this comment

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

Why rename this to "Naive" instead of "Native"?

Copy link
Contributor Author

@bostko bostko Jan 19, 2016

Choose a reason for hiding this comment

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

I named it similar to NaiveScriptRunner.
Docs added.

@aledsage
Copy link
Contributor

aledsage commented Jan 19, 2016

#1158 contains some of these same changes (it switches to using the pure-java winrm4j client v0.2.0).

@bostko bostko force-pushed the windows_file_scripts branch from 8672510 to f3ddf8f Compare January 19, 2016 07:19
@bostko bostko force-pushed the windows_file_scripts branch 2 times, most recently from fa7e0b8 to dec1f2c Compare January 31, 2016 21:12
WinRM commands
a3f57a6 
Use executePsCommand and executeCmdCommand instead of executeScript

WinRM Script tests
@bostko bostko force-pushed the windows_file_scripts branch from dec1f2c to a3f57a6 Compare February 1, 2016 08:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bostko @ahgittin @aledsage @sjcorbett