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 2016/03/29 00:40:44 UTC

[GitHub] brooklyn-server pull request: Windows @ GCE: exec startup script t...

GitHub user aledsage opened a pull request:

    https://github.com/apache/brooklyn-server/pull/89

    Windows @ GCE: exec startup script to enable WinRM

    Longer term, we should look at how better to configure a startup script. Different clouds within jclouds seem to use userMetadata in different ways, making our code fiddly.
    
    Note this change isn't enough to get Windows@GCE working. We also need to write some code for generating + retrieving a Windows password. See https://cloud.google.com/compute/docs/instances/automate-pw-generation
    
    If testing, the following location config is also important for users, to avoid jclouds waiting for port 22 to be reachable (and thus failing): `templateOptions: { autoCreateKeyPair: false }`


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

    $ git pull https://github.com/aledsage/brooklyn-server fix/Windows-start-script-on-gce

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

    https://github.com/apache/brooklyn-server/pull/89.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 #89
    
----
commit c56f713a3566b7e5f64f0e0ee9c0d1b612d46774
Author: Aled Sage <al...@gmail.com>
Date:   2016-03-28T22:37:01Z

    Windows @ GCE: exec startup script to enable WinRM

----


---
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] brooklyn-server pull request: Windows @ GCE: exec startup script t...

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

    https://github.com/apache/brooklyn-server/pull/89#discussion_r57705093
  
    --- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
    @@ -1536,10 +1536,45 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config) {
             }
             TemplateOptions options = template.getOptions();
     
    +        // For windows, we need a startup-script to be executed that will enable winrm access.
    +        // If there is already conflicting userMetadata, then don't replace it (and just warn).
    +        // TODO this injection is hacky and (currently) cloud specific.
             boolean windows = isWindows(template, config);
             if (windows) {
    -            if (!(config.containsKey(JcloudsLocationConfig.USER_METADATA_STRING) || config.containsKey(JcloudsLocationConfig.USER_METADATA_MAP))) {
    -                config.put(JcloudsLocationConfig.USER_METADATA_STRING, WinRmMachineLocation.getDefaultUserMetadataString(config()));
    +            String initScript = WinRmMachineLocation.getDefaultUserMetadataString(config());
    +            String provider = getProvider();
    +            if ("google-compute-engine".equals(provider)) {
    +                // see https://cloud.google.com/compute/docs/startupscript:
    +                // Set "sysprep-specialize-script-cmd" in metadata.
    +                String startupScriptKey = "sysprep-specialize-script-cmd";
    +                Object metadataMapRaw = config.get(USER_METADATA_MAP);
    +                if (metadataMapRaw instanceof Map) {
    +                    Map<?,?> metadataMap = (Map<?, ?>) metadataMapRaw;
    +                    if (metadataMap.containsKey(startupScriptKey)) {
    +                        LOG.warn("Not adding startup-script for Windows VM on "+provider+", because already has key "+startupScriptKey+" in config "+USER_METADATA_MAP.getName());
    +                    } else {
    +                        Map<Object, Object> metadataMapReplacement = MutableMap.copyOf(metadataMap);
    +                        metadataMapReplacement.put(startupScriptKey, initScript);
    +                        config.put(USER_METADATA_MAP, metadataMapReplacement);
    +                        LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
    +                    }
    +                } else if (metadataMapRaw == null) {
    +                    Map<String, String> metadataMapReplacement = MutableMap.of(startupScriptKey, initScript);
    +                    config.put(USER_METADATA_MAP, metadataMapReplacement);
    +                    LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
    +                }
    +            } else {
    +                // For AWS and vCloudDirector, we just set user_metadata_string.
    +                // For Azure-classic, there is no capability to execute a startup script.
    +                boolean userMetadataString = config.containsKey(JcloudsLocationConfig.USER_METADATA_STRING);
    +                boolean userMetadataMap = config.containsKey(JcloudsLocationConfig.USER_METADATA_MAP);
    +                if (!(userMetadataString || userMetadataMap)) {
    --- End diff --
    
    Yes, it would have been (I left the names as-is). Feel free to rename that in a new PR.


---
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] brooklyn-server pull request: Windows @ GCE: exec startup script t...

Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/89#discussion_r57688815
  
    --- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
    @@ -1536,10 +1536,45 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config) {
             }
             TemplateOptions options = template.getOptions();
     
    +        // For windows, we need a startup-script to be executed that will enable winrm access.
    +        // If there is already conflicting userMetadata, then don't replace it (and just warn).
    +        // TODO this injection is hacky and (currently) cloud specific.
             boolean windows = isWindows(template, config);
             if (windows) {
    -            if (!(config.containsKey(JcloudsLocationConfig.USER_METADATA_STRING) || config.containsKey(JcloudsLocationConfig.USER_METADATA_MAP))) {
    -                config.put(JcloudsLocationConfig.USER_METADATA_STRING, WinRmMachineLocation.getDefaultUserMetadataString(config()));
    +            String initScript = WinRmMachineLocation.getDefaultUserMetadataString(config());
    +            String provider = getProvider();
    +            if ("google-compute-engine".equals(provider)) {
    +                // see https://cloud.google.com/compute/docs/startupscript:
    +                // Set "sysprep-specialize-script-cmd" in metadata.
    +                String startupScriptKey = "sysprep-specialize-script-cmd";
    +                Object metadataMapRaw = config.get(USER_METADATA_MAP);
    +                if (metadataMapRaw instanceof Map) {
    +                    Map<?,?> metadataMap = (Map<?, ?>) metadataMapRaw;
    +                    if (metadataMap.containsKey(startupScriptKey)) {
    +                        LOG.warn("Not adding startup-script for Windows VM on "+provider+", because already has key "+startupScriptKey+" in config "+USER_METADATA_MAP.getName());
    +                    } else {
    +                        Map<Object, Object> metadataMapReplacement = MutableMap.copyOf(metadataMap);
    +                        metadataMapReplacement.put(startupScriptKey, initScript);
    +                        config.put(USER_METADATA_MAP, metadataMapReplacement);
    +                        LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
    +                    }
    +                } else if (metadataMapRaw == null) {
    +                    Map<String, String> metadataMapReplacement = MutableMap.of(startupScriptKey, initScript);
    +                    config.put(USER_METADATA_MAP, metadataMapReplacement);
    +                    LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
    +                }
    +            } else {
    +                // For AWS and vCloudDirector, we just set user_metadata_string.
    +                // For Azure-classic, there is no capability to execute a startup script.
    +                boolean userMetadataString = config.containsKey(JcloudsLocationConfig.USER_METADATA_STRING);
    +                boolean userMetadataMap = config.containsKey(JcloudsLocationConfig.USER_METADATA_MAP);
    +                if (!(userMetadataString || userMetadataMap)) {
    --- End diff --
    
    Isn't it better to name it `hasMetadataString` and `hasMetadataMap`


---
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] brooklyn-server pull request: Windows @ GCE: exec startup script t...

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

    https://github.com/apache/brooklyn-server/pull/89#issuecomment-202743991
  
    LGTM


---
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] brooklyn-server pull request: Windows @ GCE: exec startup script t...

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

    https://github.com/apache/brooklyn-server/pull/89#issuecomment-202610373
  
    @andreaturli @bostko @neykov can you review please? Would like this available tomorrow (Tuesday) morning if possible!


---
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] brooklyn-server pull request: Windows @ GCE: exec startup script t...

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

    https://github.com/apache/brooklyn-server/pull/89


---
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.
---