You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ygy <gi...@git.apache.org> on 2015/05/13 02:30:26 UTC

[GitHub] incubator-brooklyn pull request: BROOKLYN-143 - Initial support fo...

GitHub user ygy opened a pull request:

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

    BROOKLYN-143 - Initial support for Hazelcast

    Provides the basic functionality for cluster of hazelcast nodes

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

    $ git pull https://github.com/ygy/incubator-brooklyn feature-hazelcast

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

    https://github.com/apache/incubator-brooklyn/pull/642.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 #642
    
----
commit 8121096b2962099b0e4299ea10e5dcba157965c3
Author: Yavor Yanchev <ya...@yanchev.com>
Date:   2015-05-13T00:05:54Z

    BROOKLYN-143 - Initial support for Hazelcast

----


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-124340223
  
    Thanks @ygy - sorry we've been ignoring this for far too long! 
    
    I'm running the tests now, and will then merge. It would also be good to have integration tests that run locally, assuming that is feasible?


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-102404835
  
    This part of the implementation is not finalized yet. It is using TCP/IP autodiscovery. 
    By default, TCP/IP autodiscovery requires only one (well known) member. Additional nodes (members) use it to know about each other. Once a new member connect to the "known-member", it will receive the information for all the other members of the cluster. 
    Current implementation is suitable for development purposes, but not for real production environment. I will extend it soon, adding some tests as well.
    
    On the other hand side, Hazelcast doesn't provide a cli tool for adding new members. Nodes configuration is described in the config file (hazelcast.xml). Adding a new node (member) should be added in the config on every single node. Additionally, It requires node restart when configuration is changed. I'm still trying to figure out the correct way for editing and applying multiple configurations of already running services in Brooklyn.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-103646558
  
    @aledsage thanks for the feedback. I've adjusted the code based on it.
    
    I'm working on the updating cluster's members configuration. Please see my previous comment https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-102404835


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30494599
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNodeSshDriver.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import static java.lang.String.format;
    +import java.util.List;
    +import brooklyn.entity.basic.AbstractSoftwareProcessSshDriver;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.location.basic.SshMachineLocation;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.os.Os;
    +import brooklyn.util.ssh.BashCommands;
    +
    +import com.google.common.collect.ImmutableList;
    +
    +public class HazelcastNodeSshDriver extends AbstractSoftwareProcessSshDriver implements HazelcastNodeDriver {
    +
    +    public HazelcastNodeSshDriver(EntityLocal entity, SshMachineLocation machine) {
    +        super(entity, machine);
    +    }
    +
    +    @Override
    +    public void preInstall() {
    +        resolver = Entities.newDownloader(this);
    +    }
    +
    +    @Override
    +    public void install() {
    +        List<String> urls = resolver.getTargets();
    +        String saveAs = resolver.getFilename();
    +        
    +        List<String> commands = ImmutableList.<String>builder()
    +            .add(BashCommands.installJavaLatestOrWarn())
    +            .addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
    +            .build();
    +        
    +        newScript(INSTALLING).body.append(commands).execute();
    +    }
    +
    +    @Override
    +    public void customize() {
    +
    +    	ImmutableList.Builder<String> commands = new ImmutableList.Builder<String>()
    +                .add("mkdir -p lib conf log")
    +                .add(String.format("cp %s/%s %s/lib/", getInstallDir(), resolver.getFilename(), getRunDir()));
    +
    +        newScript(CUSTOMIZING)
    +                .body.append(commands.build())
    +                .failOnNonZeroResultCode()
    +                .execute();
    +        
    +        copyTemplate(entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL), Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    +        
    +    }
    +
    +    @Override
    +    public void launch() {
    +        
    +        entity.setAttribute(HazelcastNode.PID_FILE, Os.mergePathsUnix(getRunDir(), PID_FILENAME));
    +        
    +        StringBuilder commandBuilder = new StringBuilder()
    +            .append(format("nohup java -cp ./lib/hazelcast-%s.jar", getVersion()));
    +        	
    +        if (entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL) != null) {
    --- End diff --
    
    The `customize()` has always copied them config file (without checking for null), so presumably would have failed if there was no config set. We should only reach this point if url was non-null?
    
    What would the default be in launch if the `-Dhazelcast.config` was missed out?


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-102152722
  
    great start -- will be nice having this.  
    
    i've not tried running this yet, but did have one question:  how do the nodes get told about each others' address?  normally i'd expect to see a list of other cluster addresses in the config template, or some CLI args in the launch step.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30406354
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNode.java ---
    @@ -0,0 +1,61 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import brooklyn.catalog.Catalog;
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.SoftwareProcess;
    +import brooklyn.entity.java.UsesJava;
    +import brooklyn.entity.java.UsesJmx;
    +import brooklyn.entity.proxying.ImplementedBy;
    +import brooklyn.event.basic.BasicAttributeSensorAndConfigKey;
    +import brooklyn.event.basic.BasicAttributeSensorAndConfigKey.StringAttributeSensorAndConfigKey;
    +import brooklyn.util.flags.SetFromFlag;
    +import brooklyn.util.javalang.JavaClassNames;
    +
    +/**
    + * An {@link brooklyn.entity.Entity} that represents an Hazelcast node
    + */
    +@Catalog(name="Hazelcast Node", description="Hazelcast is a clustering and highly scalable data distribution platform for Java.")
    +
    +@ImplementedBy(HazelcastNodeImpl.class)
    +public interface HazelcastNode extends SoftwareProcess, UsesJava, UsesJmx {
    +    @SetFromFlag("version")
    +    ConfigKey<String> SUGGESTED_VERSION = ConfigKeys.newConfigKeyWithDefault(SoftwareProcess.SUGGESTED_VERSION, "3.4.2");
    +    
    +    @SetFromFlag("downloadUrl")
    +    BasicAttributeSensorAndConfigKey<String> DOWNLOAD_URL = new BasicAttributeSensorAndConfigKey<String>(
    +            SoftwareProcess.DOWNLOAD_URL, "https://repo1.maven.org/maven2/com/hazelcast/hazelcast/${version}/hazelcast-${version}.jar");
    +    
    +    @SetFromFlag("configFileUrl")
    +    ConfigKey<String> TEMPLATE_CONFIGURATION_URL = ConfigKeys.newStringConfigKey(
    +            "hazelcast.node.template.configuration.url", "Template file (in freemarker format) for the hazelcast.xml file", 
    +            JavaClassNames.resolveClasspathUrl(HazelcastNode.class, "hazelcast-brooklyn.xml"));
    +
    +    @SetFromFlag("nodeName")
    +    StringAttributeSensorAndConfigKey NODE_NAME = new StringAttributeSensorAndConfigKey("hazelcast.node.name", 
    +            "Node name (or randomly selected if not set", null);
    +    
    +    @SetFromFlag("clusterName")
    +    StringAttributeSensorAndConfigKey CLUSTER_NAME = new StringAttributeSensorAndConfigKey("hazelcast.node.cluster.name", 
    +            "Cluster name", null);
    --- End diff --
    
    Thanks for the remark.
    
    A single JVM can run multiple Hazelcast instances. They are specified in the ```<group>``` element of the configuration file.
    ```hazelcast.node.cluster.name``` was intended to be used as ```<name>``` value in the ```<group>``` element.
    Often group and cluster names names are identical.
    
    Indeed, current constant's name may lead to inconsistency. I will rename it and properly document the new one.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r35397530
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNodeSshDriver.java ---
    @@ -0,0 +1,159 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import static java.lang.String.format;
    +
    +import java.util.List;
    +import java.util.concurrent.ExecutionException;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.java.JavaSoftwareProcessSshDriver;
    +import brooklyn.location.basic.SshMachineLocation;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.os.Os;
    +import brooklyn.util.ssh.BashCommands;
    +
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.Lists;
    +
    +public class HazelcastNodeSshDriver extends JavaSoftwareProcessSshDriver implements HazelcastNodeDriver {
    +    
    +    private static final Logger LOG = LoggerFactory.getLogger(HazelcastNodeSshDriver.class);
    +
    +    public HazelcastNodeSshDriver(EntityLocal entity, SshMachineLocation machine) {
    +        super(entity, machine);
    +    }
    +
    +    @Override
    +    public void preInstall() {
    +        resolver = Entities.newDownloader(this);
    +    }
    +
    +    @Override
    +    public void install() {
    +        List<String> urls = resolver.getTargets();
    +        String saveAs = resolver.getFilename();
    +        
    +        List<String> commands = ImmutableList.<String>builder()
    +            .add(BashCommands.installJavaLatestOrWarn())
    +            .addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
    +            .build();
    +        
    +        newScript(INSTALLING).body.append(commands).execute();
    +    }
    +
    +    @Override
    +    public void customize() {
    +        if (LOG.isInfoEnabled()) {
    +            LOG.info("Customizing {}", entity.getAttribute(HazelcastNode.NODE_NAME));
    +        }
    +        
    +        ImmutableList.Builder<String> commands = new ImmutableList.Builder<String>()
    +                .add("mkdir -p lib conf log")
    +                .add(String.format("cp %s/%s %s/lib/", getInstallDir(), resolver.getFilename(), getRunDir()));
    +
    +        newScript(CUSTOMIZING)
    +                .body.append(commands.build())
    +                .failOnNonZeroResultCode()
    +                .execute();
    +        
    +        copyTemplate(entity.getConfig(HazelcastNode.CONFIG_TEMPLATE_URL), Os.mergePathsUnix(getRunDir(), "conf", getConfigFileName()));
    +        
    +    }
    +
    +    @Override
    +    public void launch() {
    +        
    +        entity.setAttribute(HazelcastNode.PID_FILE, Os.mergePathsUnix(getRunDir(), PID_FILENAME));
    +        
    +        String maxHeapMemorySize = getHeapMemorySize();
    +        
    +        if (LOG.isInfoEnabled()) {
    +            LOG.info("Launching {} with heap memory of {}", entity, maxHeapMemorySize);
    +        }
    +        
    +        // Setting initial heap size (Xms) size to match max heap size (Xms) at first
    +        String initialHeapMemorySize = maxHeapMemorySize;
    +        
    +        StringBuilder commandBuilder = new StringBuilder()
    +            .append(format("nohup java -cp ./lib/%s", resolver.getFilename()))
    +            .append(format(" -Xmx%s -Xms%s", maxHeapMemorySize, initialHeapMemorySize))
    +            .append(format(" -Dhazelcast.config=./conf/%s", getConfigFileName()))
    +            .append(format(" com.hazelcast.core.server.StartServer >> %s 2>&1 </dev/null &", getLogFileLocation()));
    +        
    +        newScript(MutableMap.of(USE_PID_FILE, true), LAUNCHING)
    +            .updateTaskAndFailOnNonZeroResultCode()
    +            .body.append(commandBuilder.toString())
    +            .execute();
    +    }
    +       
    +    public String getConfigFileName() {
    +        return entity.getConfig(HazelcastNode.CONFIG_FILE_NAME);
    +    }
    +    
    +    public String getHeapMemorySize() {
    +        return entity.getConfig(HazelcastNode.NODE_HEAP_MEMORY_SIZE);
    +    }
    +    
    +    @Override
    +    public boolean isRunning() {       
    +        return newScript(MutableMap.of(USE_PID_FILE, true), CHECK_RUNNING).execute() == 0;
    +    }
    +    
    +    @Override
    +    public void stop() {
    +        newScript(MutableMap.of(USE_PID_FILE, true), STOPPING).execute();
    +    }
    +    
    +    @Override
    +    public void kill() {
    +        newScript(MutableMap.of(USE_PID_FILE, true), KILLING).execute();
    +    }
    +
    +    public List<String> getHazelcastNodesList() throws ExecutionException, InterruptedException {
    +        HazelcastCluster cluster = (HazelcastCluster) entity.getParent();
    +        List<String> result = Lists.newArrayList();
    +
    +        for (Entity member : cluster.getMembers()) {
    +            String address = Entities.attributeSupplierWhenReady(member, HazelcastNode.SUBNET_ADDRESS).get();
    +            Integer port = Entities.attributeSupplierWhenReady(member, HazelcastNode.NODE_PORT).get();
    +            
    +            String addressAndPort = String.format("%s:%d", address, port);
    +            
    +            if (LOG.isInfoEnabled()) {
    +                LOG.info("Adding {} to the members' list of {}", addressAndPort, entity.getAttribute(HazelcastNode.NODE_NAME));
    +            }
    +            result.add(addressAndPort);
    +        }
    +        
    +        return result;
    +    }
    +
    +    @Override
    +    protected String getLogFileLocation() {
    +        return Os.mergePathsUnix(getRunDir(),"/log/out.log");
    --- End diff --
    
    Personal preference for `console` or `console.log`; the name `out.log` makes it feel like it might be just stdout, but it also has stderr. No strong feelings though.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30494790
  
    --- Diff: sandbox/nosql/src/main/resources/brooklyn/entity/nosql/hazelcast/hazelcast-brooklyn.xml ---
    @@ -0,0 +1,58 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +
    +<hazelcast xsi:schemaLocation="http://www.hazelcast.com/schema/config hazelcast-config-3.4.xsd"
    +           xmlns="http://www.hazelcast.com/schema/config"
    +           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    +    <group>
    +        <name>dev</name>
    +        <password>dev-pass</password>
    +    </group>
    +    <management-center enabled="false">http://localhost:8080/mancenter</management-center>
    +    <network>
    +        <port auto-increment="true" port-count="100">5701</port>
    +        <outbound-ports>
    +            <!--
    +            Allowed port range when connecting to other nodes.
    +            0 or * means use system provided port.
    +            -->
    +            <ports>0</ports>
    +        </outbound-ports>
    +        <join>
    +            <multicast enabled="false" />
    +
    +            <tcp-ip enabled="true">
    +                <interface>127.0.0.1</interface>
    --- End diff --
    
    This looks like a strange ip to use for join? Will it only work for clusters if multiple nodes are running on same machine?
    
    Probably best to inject this using freemarker.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30494688
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNodeSshDriver.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import static java.lang.String.format;
    +import java.util.List;
    +import brooklyn.entity.basic.AbstractSoftwareProcessSshDriver;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.location.basic.SshMachineLocation;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.os.Os;
    +import brooklyn.util.ssh.BashCommands;
    +
    +import com.google.common.collect.ImmutableList;
    +
    +public class HazelcastNodeSshDriver extends AbstractSoftwareProcessSshDriver implements HazelcastNodeDriver {
    +
    +    public HazelcastNodeSshDriver(EntityLocal entity, SshMachineLocation machine) {
    +        super(entity, machine);
    +    }
    +
    +    @Override
    +    public void preInstall() {
    +        resolver = Entities.newDownloader(this);
    +    }
    +
    +    @Override
    +    public void install() {
    +        List<String> urls = resolver.getTargets();
    +        String saveAs = resolver.getFilename();
    +        
    +        List<String> commands = ImmutableList.<String>builder()
    +            .add(BashCommands.installJavaLatestOrWarn())
    +            .addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
    +            .build();
    +        
    +        newScript(INSTALLING).body.append(commands).execute();
    +    }
    +
    +    @Override
    +    public void customize() {
    +
    +    	ImmutableList.Builder<String> commands = new ImmutableList.Builder<String>()
    +                .add("mkdir -p lib conf log")
    +                .add(String.format("cp %s/%s %s/lib/", getInstallDir(), resolver.getFilename(), getRunDir()));
    +
    +        newScript(CUSTOMIZING)
    +                .body.append(commands.build())
    +                .failOnNonZeroResultCode()
    +                .execute();
    +        
    +        copyTemplate(entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL), Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    +        
    +    }
    +
    +    @Override
    +    public void launch() {
    +        
    +        entity.setAttribute(HazelcastNode.PID_FILE, Os.mergePathsUnix(getRunDir(), PID_FILENAME));
    +        
    +        StringBuilder commandBuilder = new StringBuilder()
    +            .append(format("nohup java -cp ./lib/hazelcast-%s.jar", getVersion()));
    +        	
    +        if (entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL) != null) {
    +            commandBuilder.append(" -Dhazelcast.config=" + Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    +        }
    +
    +        commandBuilder.append(" com.hazelcast.core.server.StartServer >> ./log/out.log 2>&1 </dev/null &");
    +        
    +        newScript(MutableMap.of(USE_PID_FILE, true), LAUNCHING)
    +            .updateTaskAndFailOnNonZeroResultCode()
    +            .body.append(commandBuilder.toString())
    +            .execute();
    +    }
    +       
    +    public String getConfigFile() {
    +        return "hazelcast.xml";
    --- End diff --
    
    Perhaps this should return `Os.mergePathsUnix(getRunDir(), "conf", getConfigFile())`. There are two callers of `getConfigFile()`, and they both do this.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-124354454
  
    @ygy - can you please close this PR. I created a new PR and squashed your commits in it, and merged that. Therefore all your changes here are now in master. See https://github.com/apache/incubator-brooklyn/pull/767 and https://github.com/apache/incubator-brooklyn/commit/e37f05856c7067081f8e37d3fbdc84408e1bbb9d


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30494424
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNodeSshDriver.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import static java.lang.String.format;
    +import java.util.List;
    +import brooklyn.entity.basic.AbstractSoftwareProcessSshDriver;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.location.basic.SshMachineLocation;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.os.Os;
    +import brooklyn.util.ssh.BashCommands;
    +
    +import com.google.common.collect.ImmutableList;
    +
    +public class HazelcastNodeSshDriver extends AbstractSoftwareProcessSshDriver implements HazelcastNodeDriver {
    +
    +    public HazelcastNodeSshDriver(EntityLocal entity, SshMachineLocation machine) {
    +        super(entity, machine);
    +    }
    +
    +    @Override
    +    public void preInstall() {
    +        resolver = Entities.newDownloader(this);
    +    }
    +
    +    @Override
    +    public void install() {
    +        List<String> urls = resolver.getTargets();
    +        String saveAs = resolver.getFilename();
    +        
    +        List<String> commands = ImmutableList.<String>builder()
    +            .add(BashCommands.installJavaLatestOrWarn())
    +            .addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
    +            .build();
    +        
    +        newScript(INSTALLING).body.append(commands).execute();
    +    }
    +
    +    @Override
    +    public void customize() {
    +
    +    	ImmutableList.Builder<String> commands = new ImmutableList.Builder<String>()
    +                .add("mkdir -p lib conf log")
    +                .add(String.format("cp %s/%s %s/lib/", getInstallDir(), resolver.getFilename(), getRunDir()));
    +
    +        newScript(CUSTOMIZING)
    +                .body.append(commands.build())
    +                .failOnNonZeroResultCode()
    +                .execute();
    +        
    +        copyTemplate(entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL), Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    +        
    +    }
    +
    +    @Override
    +    public void launch() {
    +        
    +        entity.setAttribute(HazelcastNode.PID_FILE, Os.mergePathsUnix(getRunDir(), PID_FILENAME));
    +        
    +        StringBuilder commandBuilder = new StringBuilder()
    +            .append(format("nohup java -cp ./lib/hazelcast-%s.jar", getVersion()));
    +        	
    +        if (entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL) != null) {
    +            commandBuilder.append(" -Dhazelcast.config=" + Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    --- End diff --
    
    I have a preference for using `Os.mergePathsUnix` instead. The `mergePaths` (sort of) takes the settings of the OS running Brooklyn, rather than the target OS that the hazelcast instance is being deployed to. But saying that, it always uses '/' as the separator because that works on both unix and windows.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30493848
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNodeSshDriver.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import static java.lang.String.format;
    +import java.util.List;
    +import brooklyn.entity.basic.AbstractSoftwareProcessSshDriver;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.location.basic.SshMachineLocation;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.os.Os;
    +import brooklyn.util.ssh.BashCommands;
    +
    +import com.google.common.collect.ImmutableList;
    +
    +public class HazelcastNodeSshDriver extends AbstractSoftwareProcessSshDriver implements HazelcastNodeDriver {
    +
    +    public HazelcastNodeSshDriver(EntityLocal entity, SshMachineLocation machine) {
    +        super(entity, machine);
    +    }
    +
    +    @Override
    +    public void preInstall() {
    +        resolver = Entities.newDownloader(this);
    +    }
    +
    +    @Override
    +    public void install() {
    +        List<String> urls = resolver.getTargets();
    +        String saveAs = resolver.getFilename();
    +        
    +        List<String> commands = ImmutableList.<String>builder()
    +            .add(BashCommands.installJavaLatestOrWarn())
    +            .addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
    +            .build();
    +        
    +        newScript(INSTALLING).body.append(commands).execute();
    +    }
    +
    +    @Override
    +    public void customize() {
    +
    +    	ImmutableList.Builder<String> commands = new ImmutableList.Builder<String>()
    +                .add("mkdir -p lib conf log")
    +                .add(String.format("cp %s/%s %s/lib/", getInstallDir(), resolver.getFilename(), getRunDir()));
    +
    +        newScript(CUSTOMIZING)
    +                .body.append(commands.build())
    +                .failOnNonZeroResultCode()
    +                .execute();
    +        
    +        copyTemplate(entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL), Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    +        
    +    }
    +
    +    @Override
    +    public void launch() {
    +        
    +        entity.setAttribute(HazelcastNode.PID_FILE, Os.mergePathsUnix(getRunDir(), PID_FILENAME));
    +        
    +        StringBuilder commandBuilder = new StringBuilder()
    +            .append(format("nohup java -cp ./lib/hazelcast-%s.jar", getVersion()));
    --- End diff --
    
    This jar file is referred to by name in `install()` (as resolver.getFilename()), in `customize()` (again calling resolver.getFilename(), and again here as `hazelcast-%s.jar`. Suggest changing this to use same name resolution technique as install and launch.


---
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: BROOKLYN-143 - Initial support fo...

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

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


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30493993
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNodeSshDriver.java ---
    @@ -0,0 +1,112 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import static java.lang.String.format;
    +import java.util.List;
    +import brooklyn.entity.basic.AbstractSoftwareProcessSshDriver;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.location.basic.SshMachineLocation;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.os.Os;
    +import brooklyn.util.ssh.BashCommands;
    +
    +import com.google.common.collect.ImmutableList;
    +
    +public class HazelcastNodeSshDriver extends AbstractSoftwareProcessSshDriver implements HazelcastNodeDriver {
    +
    +    public HazelcastNodeSshDriver(EntityLocal entity, SshMachineLocation machine) {
    +        super(entity, machine);
    +    }
    +
    +    @Override
    +    public void preInstall() {
    +        resolver = Entities.newDownloader(this);
    +    }
    +
    +    @Override
    +    public void install() {
    +        List<String> urls = resolver.getTargets();
    +        String saveAs = resolver.getFilename();
    +        
    +        List<String> commands = ImmutableList.<String>builder()
    +            .add(BashCommands.installJavaLatestOrWarn())
    +            .addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
    +            .build();
    +        
    +        newScript(INSTALLING).body.append(commands).execute();
    +    }
    +
    +    @Override
    +    public void customize() {
    +
    +    	ImmutableList.Builder<String> commands = new ImmutableList.Builder<String>()
    +                .add("mkdir -p lib conf log")
    +                .add(String.format("cp %s/%s %s/lib/", getInstallDir(), resolver.getFilename(), getRunDir()));
    +
    +        newScript(CUSTOMIZING)
    +                .body.append(commands.build())
    +                .failOnNonZeroResultCode()
    +                .execute();
    +        
    +        copyTemplate(entity.getConfig(HazelcastNode.TEMPLATE_CONFIGURATION_URL), Os.mergePaths(getRunDir(), "conf", getConfigFile()));
    +        
    +    }
    +
    +    @Override
    +    public void launch() {
    +        
    +        entity.setAttribute(HazelcastNode.PID_FILE, Os.mergePathsUnix(getRunDir(), PID_FILENAME));
    +        
    +        StringBuilder commandBuilder = new StringBuilder()
    +            .append(format("nohup java -cp ./lib/hazelcast-%s.jar", getVersion()));
    --- End diff --
    
    I see you've followed the same pattern as elsewhere for setting the `resolver` field in the superclass. However, I think that might be a (subtly) bad pattern. If we stop and restart Brooklyn, then it restores the entities based on their config+attributes, and calling `rebind()`. I don't think anything will re-set the `resolver` field in the driver.
    
    I suggest you don't worry about that in your pull request. We should refactor + fix that across the board (probably in the super-class) in a separate pull request, rather than doing something specific for hazelcast.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#discussion_r30356595
  
    --- Diff: sandbox/nosql/src/main/java/brooklyn/entity/nosql/hazelcast/HazelcastNode.java ---
    @@ -0,0 +1,61 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.nosql.hazelcast;
    +
    +import brooklyn.catalog.Catalog;
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.SoftwareProcess;
    +import brooklyn.entity.java.UsesJava;
    +import brooklyn.entity.java.UsesJmx;
    +import brooklyn.entity.proxying.ImplementedBy;
    +import brooklyn.event.basic.BasicAttributeSensorAndConfigKey;
    +import brooklyn.event.basic.BasicAttributeSensorAndConfigKey.StringAttributeSensorAndConfigKey;
    +import brooklyn.util.flags.SetFromFlag;
    +import brooklyn.util.javalang.JavaClassNames;
    +
    +/**
    + * An {@link brooklyn.entity.Entity} that represents an Hazelcast node
    + */
    +@Catalog(name="Hazelcast Node", description="Hazelcast is a clustering and highly scalable data distribution platform for Java.")
    +
    +@ImplementedBy(HazelcastNodeImpl.class)
    +public interface HazelcastNode extends SoftwareProcess, UsesJava, UsesJmx {
    +    @SetFromFlag("version")
    +    ConfigKey<String> SUGGESTED_VERSION = ConfigKeys.newConfigKeyWithDefault(SoftwareProcess.SUGGESTED_VERSION, "3.4.2");
    +    
    +    @SetFromFlag("downloadUrl")
    +    BasicAttributeSensorAndConfigKey<String> DOWNLOAD_URL = new BasicAttributeSensorAndConfigKey<String>(
    +            SoftwareProcess.DOWNLOAD_URL, "https://repo1.maven.org/maven2/com/hazelcast/hazelcast/${version}/hazelcast-${version}.jar");
    +    
    +    @SetFromFlag("configFileUrl")
    +    ConfigKey<String> TEMPLATE_CONFIGURATION_URL = ConfigKeys.newStringConfigKey(
    +            "hazelcast.node.template.configuration.url", "Template file (in freemarker format) for the hazelcast.xml file", 
    +            JavaClassNames.resolveClasspathUrl(HazelcastNode.class, "hazelcast-brooklyn.xml"));
    +
    +    @SetFromFlag("nodeName")
    +    StringAttributeSensorAndConfigKey NODE_NAME = new StringAttributeSensorAndConfigKey("hazelcast.node.name", 
    +            "Node name (or randomly selected if not set", null);
    +    
    +    @SetFromFlag("clusterName")
    +    StringAttributeSensorAndConfigKey CLUSTER_NAME = new StringAttributeSensorAndConfigKey("hazelcast.node.cluster.name", 
    +            "Cluster name", null);
    --- End diff --
    
    i don't know enough about hazelcast but is the value of this (cluster name at node) the same as the value of the `hazelcast.cluster.name` ?  if so we could have the same key name or use the same constant.


---
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: BROOKLYN-143 - Initial support fo...

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

    https://github.com/apache/incubator-brooklyn/pull/642#issuecomment-103006674
  
    @ygy looking really good. Only very minor low-level comments from me.
    
    Like @ahgittin I'm very interested to see how clustering is handled. A new node would want the address(es) of existing nodes. And presumably existing nodes would want to be updated with the addresses of new nodes in case the process restarted?


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