You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2015/05/28 01:14:58 UTC

[GitHub] incubator-brooklyn pull request: Support for deployment to windows...

GitHub user aledsage opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/664

    Support for deployment to windows (strike 2)

    Builds on https://github.com/apache/incubator-brooklyn/pull/650, incorporating many of the pull request comments from there.
    
    This also has @nakomis's commit(s) for https://github.com/apache/incubator-brooklyn/pull/639. Would be nice to address those before merge, but am happy if we merge then refactor so we can get the rest of this goodness.
    
    The following have been deferred for this PR, but really must be addressed soon:
    
    * Comments in PR #639 (@nakomis's PR on which this is built included those changes)
    
    * More tests (!), including:
    
      * WinRmMachineLocation's script execution
        (there is one test, which needs attention!)
      
      * VanillaWindowsProcess
        (there is one test, which needs attention - MssqlBlueprintLiveTest)
    
      * JcloudsWinRmMachineLocation's public/private/subnet addresses
    
    * Resolve naming differences in config between WinRM location/tool and SshTool/SshMachineLocation, such as SshTool#PROP_SSH_TRIES. Should these be merged int one well-named generic thing, deprecating the previous ssh config key?
    
    * Resolve naming differences in JcloudsLocation, where it's ssh-specific. For example, should WAIT_FOR_WINRM_AVAILABLE and WAIT_FOR_SSHABLE. Would probably have to deprecate the old key, and generalise the name.
    
    * JcloudsLocation:
    
      * Allow for user-specified credentials: want to support setting up a given user on the VM (or resetting the Administrator password), and to support vCloudDirector use-case where template contains username/password, so don't just use what is returned by jclouds.
    
      * Support for port-forwarding in JcloudsWinRmMachineLocation.
    
      * Resolve differences in "waitForReachable" between Windows and ssh code-paths.
    
      * Can we ensure the returned JcloudsMachineLocation will have the right node.getLoginPort for WinRM? How do we ensure that happens?
    
      * Generalise the sshHostAndPortOverride (e.g. rename to connectionHostAndPortOverride). Use it for the WinRM port in the windows case.
    
      * jcloudsLocation: need to release port-forwarding when it's a Windows VM
    
    * WinRmMachineLocation: respect the configured `port` as the one to use when connecting (essential for port-forwarding scenarios).
    
    * We should list (in docs / limitations) the main config options that are not supported (yet) on windows (rather than the user/customer discovering it by trying to configure it and eventually finding this log message, or just it being ignored silently (as would be the case for user/password configuration).
    
    
    Some unaddressed minor comments:
    
    * WinRmMachineLocation: worth a comment describing the file-copy approach used.
    
    * WinRmMachineLocation.getDefaultUserMetadataString: worth some javadoc to say what this default user metadata will do.
    
    * JcloudsLocation waitForReachable: would catch-block always be for a NumberFormatException? If so, can we be more specific?
    
    * I wonder why we don't set the parent inside JcloudsLocation.registerJcloudsSshMachineLocation.
    
    * AbstractSoftwareProcessWinRmDriver.WINDOWS_USERNAME: is inside the driver the best place to declare these sensors?
    
    * Unaddressed questions from PR #650:
    
      * in jcloudsLocation, "This changes the execCommands description. I take it that's deliberate? Was the full script being logged multiple times before?"
    
      * in jcloudsLocation, "What exception are you expecting here? Can we just program more defensively when building the message?"
    
      * in JcloudsLocation, "Remove try-catch! @Nakomis: why catch exception when doing log message? What exception happened during logging?"
    
      * AbstractSoftwareProcessWinRmDriver.rebootAndWait: Can we be more specific - e.g. what exception do you expect. And log if not exactly right. There's a risk we'll pretend the VM is rebooting when actually we got back an error about permissions or something like that.
    
    
    Other comments needing more thought (but less time critical):
    
    * DynamicCluster.grow will not behave right for availability zones if the memberSpec defines a location. It will use that location in preference to the cluster's location.
    
    * WinRmMachineLocation has a retry-loop when calling WinrmTool. This feels quite different from the SshMachineLocation implementation. I wonder if it's worth creating a wrapper of WinrmTool, such as for retries etc.
    
    * Not sure about the name VanillaWindowsProcessDriver versus VanillaSoftwareProcessDriver. They are both software processes.
    
    * Medium term, would be good to think more about how an entity author (working in yaml) can control the different phases. This is good enough for now, but we'll no doubt hit a use-case where one needs to reboot at a different phase, or where more phases would be useful (particularly for the equivalent of "sub-classing" within the yaml).
    @alasdairhodge has some good thoughts on this, as has @ahgittin - worth a bigger discussion on that.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aledsage/incubator-brooklyn nakomis-copy/winrm-location

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/664.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #664
    
----
commit 035186dd9747bb2d397565366b08ce8e9b87b7af
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-06T12:13:18Z

    Basic support for Windows BYON with WinRM pre-configured

commit cc80bfff403346fb91736df5c04ea017d8703d09
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-12T14:33:24Z

    Allows for lists in templateOptions

commit 07e9a4a94dcd2d300ab657efaa484ba73ff6d48f
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-08T14:41:14Z

    Adds WinRm support for EC2 Windows instances

commit 2a1999b4e4147dc32ddee4e93ed0271e8651a386
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-09T13:21:18Z

    Adds copyTo functions to WinRmMachineLocation

commit d7723b02bf02b13660253af11acc940c12a1d945
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-13T09:48:58Z

    Adds (WIP) outline for VanillaWindowsProcess

commit 539293559bbe95476992da77271486633405dc03
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-13T12:14:47Z

    Avoids unnecessary Arrays.copyOf in WinRmMachineLocation.copyTo

commit 4a7aace1198009959b2f8449c4d6ad13ede98004
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-15T15:12:44Z

    Implements support for VanillaWindowsProcess

commit 14fcf1a1d80133a90438ba56096ee86012ae7afa
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-15T15:13:53Z

    Adds support for ${attribute['attribute.name']} sensor access in freemarker templates

commit 25239f898e7b252090c769a689a5d6274401c7a6
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-22T12:23:18Z

    Removes default launch command from VanillaWindowsProcess, adds ExecutionPolicy to default WinRm usermetadata

commit 119f8b33aef78bf17769f187381f2f806eebdfcd
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-27T05:06:10Z

    Registers location in vmInstanceIds to allow it to be destroyed after use

commit 84d77a9f3a91dc098f7332e0ca4433371cbaec9d
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-27T17:26:56Z

    Fixes destruction of WinRmMachineLocation on stop

commit 397b9e9cb8b62d3338956b3c1aa8df2a7eddc788
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-04-28T09:57:07Z

    Adds apache header to StaticSensor

commit ae39d830ec7ca7e8d4193017095c46eeaefada3e
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-01T12:20:53Z

    Adds retry to WinRm commands

commit c3e67f7376207a0a6db92d09f861e79626fd34fb
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-06T13:18:13Z

    Fixes broken cluster location logic

commit 2a8ef52e9d7f4a7a4333919fd57380498637b492
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-12T14:56:09Z

    Adds support for getting return from WinRmMachineLocation execute commands

commit a13a49daa36ed89824d9128b2804c745c26133b5
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-13T07:58:30Z

    Adds performance counters

commit 7a89681ffbb4b46431ba64de85eddfcfaa1dfb6b
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-14T12:06:25Z

    Adds latches for pre-install files and pre-install/install/customize reboots

commit a7549daac067300eda84476da54c32a36bd732d3
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-14T12:09:35Z

    Changes DynamicCluster resize behaviour to favour the location defined on the memberspec (if present)

commit 663235c6bfe43c758a1eb0e85a116c314525a477
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-19T11:26:21Z

    Fixes JcloudsLocationTest.testInvokesCustomizerCallbacks

commit c3719fd0a2d314f58036af71a201f5e7b7a28963
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-21T12:06:27Z

    Updates ControlledDynamicWebAppClusterImpl to use location defined on controller, if present

commit 84ef3ebc2e3ccbeb746f00d91e3b439451ba78fe
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-21T12:07:32Z

    Adds MSSQL (YAML-based) blueprint

commit ef68c603063ace7baf84989c06214931dafd9e12
Author: Martin Harris <gi...@nakomis.com>
Date:   2015-05-22T15:23:04Z

    Adds basic documentation for re-authentication and redirecting stdout/stderr

commit 3053d993b93859d5d133dfeee38f3c37e05b2665
Author: Aled Sage <al...@gmail.com>
Date:   2015-05-27T08:54:12Z

    WinRM support: incorporate review comments PR #650
    
    See https://github.com/apache/incubator-brooklyn/pull/650

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Support for deployment to windows...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/664#issuecomment-106877217
  
    Closing, now that #665 has been merged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Support for deployment to windows...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/664


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---