You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by grkvlt <gi...@git.apache.org> on 2016/06/17 13:16:59 UTC
[GitHub] brooklyn-library pull request #44: RiakNode Internal Networking
GitHub user grkvlt opened a pull request:
https://github.com/apache/brooklyn-library/pull/44
RiakNode Internal Networking
Configure the internal ports required for Fiak intra-node communication using an optional `JcloudsSecurityGroupCustomizer`
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/brooklyn-library riak-node-port-range
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-library/pull/44.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 #44
----
commit 5720fc648fbe56b17a77d53bb0916a8f9837b8cc
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-06-16T07:22:28Z
riak-port-range-tmp
commit d829217fbedd9485921ccf0662e6d24b6a54f716
Author: Andrew Donald Kennedy <an...@cloudsoftcorp.com>
Date: 2016-06-17T13:14:06Z
Change RiakNode to configure internal ports using optional location customizer
----
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r71323278
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,15 +108,51 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
+ sensors().set(ERLANG_PORT_RANGE_END, erlangRangeEnd);
+
+ boolean configureInternalNetworking = config().get(CONFIGURE_INTERNAL_NETWORKING);
--- End diff --
I'd do the `Boolean.TRUE.equals(config().get(CONFIGURE_INTERNAL_NETWORKING))`, in case of null.
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r71323238
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,15 +108,51 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
--- End diff --
I suspect we get this being set for free, even if you don't include this line - but no strong feelings about leaving it in.
---
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-library issue #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
@aledsage you were right, the customizer synchronises on its class, so no need for external locking. I also removed the deprecation message, but kept the new config and sensor names, as I believe this is a safe change to make.
---
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-library issue #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
Thanks @grkvlt - I'm going to merge now. I made one other minor comment that is perhaps worth addressing, but not worth holding up this PR for.
---
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-library issue #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
@geomacy I'm going to try this out with the latest suite of Brooklyn/AMP/Clocker/jclouds-labs patches, and will report back
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r68455282
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,17 +110,59 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ PortRange erlangPorts = config().get(ERLANG_PORT_RANGE);
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ if (erlangRangeStart == null) erlangRangeStart = Iterables.get(erlangPorts, 0);
+ if (erlangRangeEnd == null) erlangRangeEnd = Iterables.getLast(erlangPorts);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
+ sensors().set(ERLANG_PORT_RANGE_END, erlangRangeEnd);
+
+ boolean configureInternalNetworking = config().get(CONFIGURE_INTERNAL_NETWORKING);
+ if (configureInternalNetworking) {
+ configureInternalNetworking();
+ }
- Set<Integer> ports = MutableSet.copyOf(super.getRequiredOpenPorts());
- Set<Integer> erlangPorts = ContiguousSet.create(Range.open(erlangRangeStart, erlangRangeEnd), DiscreteDomain.integers());
- ports.addAll(erlangPorts);
+ return super.getRequiredOpenPorts();
+ }
- return ports;
+ private void configureInternalNetworking() {
+ Location location = getDriver().getLocation();
+ if (!(location instanceof JcloudsSshMachineLocation)) {
+ LOG.info("Not running in a JcloudsSshMachineLocation, not adding IP permissions to {}", this);
+ return;
+ }
+ JcloudsMachineLocation machine = (JcloudsMachineLocation) location;
+ JcloudsLocationSecurityGroupCustomizer customizer = JcloudsLocationSecurityGroupCustomizer.getInstance(getApplicationId());
+
+ synchronized (getParent()) {
--- End diff --
Why synchronize on the parent? That feels like bad practice. Is there something better to synchronize on? Or can the `JcloudsLocationSecurityGroupCustomizer` handle the synchronizing internally?
---
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-library pull request #44: RiakNode Internal Networking
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r70430055
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +125,28 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port", "Handoff Listener Port", 8099);
--- End diff --
This still won't publish the sensor. Only `PortRange`s get published automatically.
---
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-library issue #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
@grkvlt finished reviewing again. Can you have a look at the backwards compatibility issue I've highlighted please.
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r68453409
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +127,37 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port.internal", "Handoff Listener Port", 8099);
--- End diff --
Let's indicate that it's `@beta` (e.g. in the description, and as a java annotation) if we think that the name will change?
---
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-library issue #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
@aledsage I'm going to merge this if there are no further objections
---
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-library pull request #44: RiakNode Internal Networking
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r67509587
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,17 +110,59 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ PortRange erlangPorts = config().get(ERLANG_PORT_RANGE);
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ if (erlangRangeStart == null) erlangRangeStart = Iterables.get(erlangPorts, 0);
+ if (erlangRangeEnd == null) erlangRangeEnd = Iterables.getLast(erlangPorts);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
+ sensors().set(ERLANG_PORT_RANGE_END, erlangRangeEnd);
+
+ boolean configureInternalNetworking = config().get(CONFIGURE_INTERNAL_NETWORKING);
+ if (configureInternalNetworking) {
+ configureInternalNetworking();
+ }
- Set<Integer> ports = MutableSet.copyOf(super.getRequiredOpenPorts());
- Set<Integer> erlangPorts = ContiguousSet.create(Range.open(erlangRangeStart, erlangRangeEnd), DiscreteDomain.integers());
- ports.addAll(erlangPorts);
+ return super.getRequiredOpenPorts();
+ }
- return ports;
+ private void configureInternalNetworking() {
+ Location location = getDriver().getLocation();
+ if (!(location instanceof JcloudsSshMachineLocation)) {
+ LOG.info("Not running in a JcloudsSshMachineLocation, not adding IP permissions to {}", this);
+ return;
+ }
+ JcloudsMachineLocation machine = (JcloudsMachineLocation) location;
+ JcloudsLocationSecurityGroupCustomizer customizer = JcloudsLocationSecurityGroupCustomizer.getInstance(getApplicationId());
+
+ synchronized (getParent()) {
+ String cidr = Cidr.UNIVERSAL.toString(); // TODO use more appropriate CIDR
--- End diff --
maybe update comment to explain what would be more appropriate (what's wrong with this line as it stands?)
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r68454770
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,17 +110,59 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ PortRange erlangPorts = config().get(ERLANG_PORT_RANGE);
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ if (erlangRangeStart == null) erlangRangeStart = Iterables.get(erlangPorts, 0);
+ if (erlangRangeEnd == null) erlangRangeEnd = Iterables.getLast(erlangPorts);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
--- End diff --
I'm a bit confused by the deprecation of this. I'd have expected:
* a `log.warn` when using the deprecated value (i.e. when it's not null), to alert people that they are using the deprecated value
* us not to use the sensor (other than to set it for backwards compatibility), given it's deprecated - but it's later used in `configureInternalNetwork`.
* changing the key's name might cause backwards compatibility problems (even though you have `@SetFromFlag` with the old name). The setFromFlag stuff doesn't work for inherited config, I believe.
* I'd have expected you to keep the old key (deprecated) and declare the new key; to read the value from both and to warn if `getRaw()` is non-null for the old one. If you do that, then we are free to delete it at some point. The way you've changed it, then it's hard to ever delete the `@SetFromFlag` name.
---
Using PortRange doesn't feel quite right. Our `PortRange` does not have to be linear (e.g. you could do `PortRanges.fromString("1,3,5")`).
We subsequently use it as though it must be a contiguous range, with a simple start and end value. Maybe we should just leave the configuration as that?
---
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-library pull request #44: RiakNode Internal Networking
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r67509044
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +127,37 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port.internal", "Handoff Listener Port", 8099);
--- End diff --
should the key be named '....internal.port' i.e. end in ".port", in order to be recognised as a port configuration parameter? And same below.
---
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-library pull request #44: RiakNode Internal Networking
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-library/pull/44
---
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-library pull request #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r67509622
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +127,37 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port.internal", "Handoff Listener Port", 8099);
--- End diff --
No, because they're not proper port config like you normall have named like that. The `.internal` suffix may change, depending on how networking extensions are implemented in future, 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] brooklyn-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r70428871
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -107,10 +107,10 @@
// NB these two needed for clients to access
@SetFromFlag("riakWebPort")
- PortAttributeSensorAndConfigKey RIAK_WEB_PORT = new PortAttributeSensorAndConfigKey("riak.webPort", "Riak Web Port", "8098+");
+ PortAttributeSensorAndConfigKey RIAK_WEB_PORT = ConfigKeys.newPortSensorAndConfigKey("riak.web.port", "Riak Web Port", "8098+");
--- End diff --
Changing this name breaks backwards compatibility. If someone has in their blueprint `riak.webPort: 1234` then that config option will now be ignored. Same for `riak.pbPort: 1235`.
---
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-library pull request #44: RiakNode Internal Networking
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r70440396
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +125,28 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port", "Handoff Listener Port", 8099);
--- End diff --
That's right - checked the sources. Please ignore.
---
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-library pull request #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r67509788
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,17 +110,59 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ PortRange erlangPorts = config().get(ERLANG_PORT_RANGE);
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ if (erlangRangeStart == null) erlangRangeStart = Iterables.get(erlangPorts, 0);
+ if (erlangRangeEnd == null) erlangRangeEnd = Iterables.getLast(erlangPorts);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
+ sensors().set(ERLANG_PORT_RANGE_END, erlangRangeEnd);
+
+ boolean configureInternalNetworking = config().get(CONFIGURE_INTERNAL_NETWORKING);
+ if (configureInternalNetworking) {
+ configureInternalNetworking();
+ }
- Set<Integer> ports = MutableSet.copyOf(super.getRequiredOpenPorts());
- Set<Integer> erlangPorts = ContiguousSet.create(Range.open(erlangRangeStart, erlangRangeEnd), DiscreteDomain.integers());
- ports.addAll(erlangPorts);
+ return super.getRequiredOpenPorts();
+ }
- return ports;
+ private void configureInternalNetworking() {
+ Location location = getDriver().getLocation();
+ if (!(location instanceof JcloudsSshMachineLocation)) {
+ LOG.info("Not running in a JcloudsSshMachineLocation, not adding IP permissions to {}", this);
+ return;
+ }
+ JcloudsMachineLocation machine = (JcloudsMachineLocation) location;
+ JcloudsLocationSecurityGroupCustomizer customizer = JcloudsLocationSecurityGroupCustomizer.getInstance(getApplicationId());
+
+ synchronized (getParent()) {
+ String cidr = Cidr.UNIVERSAL.toString(); // TODO use more appropriate CIDR
--- End diff --
Yes, I mean more _restrictive_ CIDR, really. But I don't think we have a good way of getting the network address for the hosts in Brooklyn at present.
---
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-library pull request #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r68481418
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,17 +110,59 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ PortRange erlangPorts = config().get(ERLANG_PORT_RANGE);
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ if (erlangRangeStart == null) erlangRangeStart = Iterables.get(erlangPorts, 0);
+ if (erlangRangeEnd == null) erlangRangeEnd = Iterables.getLast(erlangPorts);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
--- End diff --
@aledsage I'll revisit this - perhaps better to just have two integer config keys/sensors, and no port range, and avoid any deprecation.
---
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-library issue #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
@geomacy Tested this, and it works fine.
---
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-library issue #44: RiakNode Internal Networking
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
Looks good to me, but haven't tested it out myself.
---
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-library issue #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-library/pull/44
@aledsage I made the compatibility changes you suggested
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r70430508
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +125,28 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port", "Handoff Listener Port", 8099);
--- End diff --
@neykov are you sure? I thought `ConfigToAttributes.apply(Entity)` would get called, which would do this for anything of type `AttributeSensorAndConfigKey`. But I haven't tested this recently.
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r70429486
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -125,14 +125,28 @@
AttributeSensor<String> RIAK_NODE_NAME = Sensors.newStringSensor("riak.node", "Returns the riak node name as defined in vm.args");
- // these needed for nodes to talk to each other, but not clients (so ideally set up in the security group for internal access)
- PortAttributeSensorAndConfigKey HANDOFF_LISTENER_PORT = new PortAttributeSensorAndConfigKey("handoffListenerPort", "Handoff Listener Port", "8099+");
- PortAttributeSensorAndConfigKey EPMD_LISTENER_PORT = new PortAttributeSensorAndConfigKey("epmdListenerPort", "Erlang Port Mapper Daemon Listener Port", "4369");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_START = new PortAttributeSensorAndConfigKey("erlangPortRangeStart", "Erlang Port Range Start", "6000+");
- PortAttributeSensorAndConfigKey ERLANG_PORT_RANGE_END = new PortAttributeSensorAndConfigKey("erlangPortRangeEnd", "Erlang Port Range End", "7999+");
+ /*
+ * Needed for nodes to talk to each other, but not clients, so ideally set up in the security group for internal access and configured here.
+ * If {@link #CONFIGURE_INTERNAL_NETWORKING} is set, then a location customizer will be added to confiure the security group dynamically.
+ */
+
+ @SetFromFlag("handoffListenerPort")
+ ConfigKey<Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newIntegerConfigKey("riak.handoff.port", "Handoff Listener Port", 8099);
--- End diff --
This is no longer published as a sensor, which changes backwards compatibility. It probably won't affect anyone, but you can instead use:
@SetFromFlag("handoffListenerPort")
AttributeSensorAndConfigKey<Integer, Integer> HANDOFF_LISTENER_PORT = ConfigKeys.newSensorAndConfigKey(
Integer.class,
"riak.handoff.port",
"Handoff Listener Port",
8099);
Probably worth it, just in case.
---
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-library pull request #44: RiakNode Internal Networking
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r70430213
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -107,10 +107,10 @@
// NB these two needed for clients to access
@SetFromFlag("riakWebPort")
- PortAttributeSensorAndConfigKey RIAK_WEB_PORT = new PortAttributeSensorAndConfigKey("riak.webPort", "Riak Web Port", "8098+");
+ PortAttributeSensorAndConfigKey RIAK_WEB_PORT = ConfigKeys.newPortSensorAndConfigKey("riak.web.port", "Riak Web Port", "8098+");
--- End diff --
I'd be very cautious: I'd create a second config key with the old name, and fallback to that. And I'd make sure a sensor with the old name is published.
Looking at some customer bespoke code, they depend on "riak.pbPort" and "riak.webPort" being published as sensors (for their enricher that propagates the public endpoint). We can tell them to change this, but we should not break backwards compatibility lightly.
---
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-library pull request #44: RiakNode Internal Networking
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-library/pull/44#discussion_r68481397
--- Diff: software/nosql/src/main/java/org/apache/brooklyn/entity/nosql/riak/RiakNodeImpl.java ---
@@ -99,17 +110,59 @@ public boolean isPackageDownloadUrlProvided() {
@Override
protected Collection<Integer> getRequiredOpenPorts() {
- // TODO this creates a huge list of inbound ports; much better to define on a security group using range syntax!
- int erlangRangeStart = getConfig(ERLANG_PORT_RANGE_START).iterator().next();
- int erlangRangeEnd = getConfig(ERLANG_PORT_RANGE_END).iterator().next();
+ PortRange erlangPorts = config().get(ERLANG_PORT_RANGE);
+ Integer erlangRangeStart = config().get(ERLANG_PORT_RANGE_START);
+ Integer erlangRangeEnd = config().get(ERLANG_PORT_RANGE_END);
+ if (erlangRangeStart == null) erlangRangeStart = Iterables.get(erlangPorts, 0);
+ if (erlangRangeEnd == null) erlangRangeEnd = Iterables.getLast(erlangPorts);
+ sensors().set(ERLANG_PORT_RANGE_START, erlangRangeStart);
+ sensors().set(ERLANG_PORT_RANGE_END, erlangRangeEnd);
+
+ boolean configureInternalNetworking = config().get(CONFIGURE_INTERNAL_NETWORKING);
+ if (configureInternalNetworking) {
+ configureInternalNetworking();
+ }
- Set<Integer> ports = MutableSet.copyOf(super.getRequiredOpenPorts());
- Set<Integer> erlangPorts = ContiguousSet.create(Range.open(erlangRangeStart, erlangRangeEnd), DiscreteDomain.integers());
- ports.addAll(erlangPorts);
+ return super.getRequiredOpenPorts();
+ }
- return ports;
+ private void configureInternalNetworking() {
+ Location location = getDriver().getLocation();
+ if (!(location instanceof JcloudsSshMachineLocation)) {
+ LOG.info("Not running in a JcloudsSshMachineLocation, not adding IP permissions to {}", this);
+ return;
+ }
+ JcloudsMachineLocation machine = (JcloudsMachineLocation) location;
+ JcloudsLocationSecurityGroupCustomizer customizer = JcloudsLocationSecurityGroupCustomizer.getInstance(getApplicationId());
+
+ synchronized (getParent()) {
--- End diff --
Agree there is possibly something better that we could choose. I'll have another look, when I'm updating the code.
---
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.
---