Adds WinRmTool interface

[aledsage]

We want the interface/class within Brooklyn so that the rest of the code doesn't depend on winrm4j directly.

We also want to be able to swap out the Winrm4jTool for an alternative implementation, just through configuration.

Note that the unqualified interface name / class name are the same as the names in io.cloudsoft.winrm4j.winrm.WinRmTool and io.cloudsoft.winrm4j.winrm.WinRmToolResponse. Is that a good idea? Any better suggestions for names?

[hzbarcea]

I didn't review this thoroughly and this is a comment related less to this particular PR as to a more general design aspect. I think the bigger issue is the fact that we handle windows as a special case (see all the if (windows) constructs in ./locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java) is what I would address first. It may inform the way we implement the WinRm support.

I have a more concrete proposal, but still not 100% fleshed out. I will post it on the dev@ list. If you want this PR merged, I am ok with it, but I think we'll revisit it in not far in the future.

[aledsage]

Thanks @hzbarcea

The JcloudsLocation code needs a complete re-write. It's too big, too many if statements, too much cloud-specific code in the middle of methods (e.g. if aws, or if docker), not unit testable, a fairly ad hoc set of injection points for customising the provisioning behaviour, etc. I also agree about the way if windows being a big code smell.

I think that at some level of our code, we'll hit the use of an ssh-client or a winrm-client (unless we went all-in with chef/puppet/salt etc). Therefore I think the bulk of this PR is not related to the problems you refer to. We want to remove the dependency on io.cloudsoft.winrm4j that had leaked into too many parts of the code.

As for WinRmMachineLocation, I think this code improves it by putting the right (or at least more right) responsibilities into the winrm-client.

I'd therefore like us to merge it before we tackle the big JcloudsLocation and other such classes.

[bostko]

Do you have to put null here?

[bostko]

Very good start for putting alternative WinRm implementations!

As part of this refactoring I want to remind you to review #950 .
My opinion is that we have to have the basic executeCommand(String) signature instead of executeScript(String).
If we want to execute scripts is supposed to be configurable e.g. error logging, exit on first error. This requires extra work which shouldn't be mandatory.
Interesting that SshTool has execCommands(List<String>...) and execScript(List<String>,...)

Basically I think that we have to have only executeCommand(String) or executeCommands(List<String>) in WinRmTool interface.

[ygy]

@aledsage
LGTM.
There are some Live tests failings, but they are not related to this PR. Most of them will just need adjusting the asserts

[neykov]

Looks good, merging.