You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by ashley-taylor <no...@github.com> on 2014/04/09 04:23:28 UTC
[jclouds] JCLOUDS-528 ability to disable public ips from open socket
finder (#341)
ability to disable public ips from open socket finder
You can merge this Pull Request by running:
git pull https://github.com/ashley-taylor/jclouds master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/341
-- Commit Summary --
* JCLOUDS-528
-- File Changes --
M compute/src/main/java/org/jclouds/compute/config/ComputeServiceProperties.java (5)
M compute/src/main/java/org/jclouds/compute/util/ConcurrentOpenSocketFinder.java (16)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/341.patch
https://github.com/jclouds/jclouds/pull/341.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1214](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1214/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40028507
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -175,8 +179,14 @@ public String toString() {
> };
> }
>
> - private static FluentIterable<String> checkNodeHasIps(NodeMetadata node) {
> - FluentIterable<String> ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> + private static FluentIterable<String> checkNodeHasIps(NodeMetadata node, boolean excludePublic) {
> + FluentIterable<String> ips;
> + if(excludePublic){
> + ips = FluentIterable.from(node.getPrivateAddresses());
> + }else{
> + ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> + }
Formatting. How about
```
Iterable<String> ips = node.getPrivateAddresses();
if (!excludePublicIps) {
ips = concat(ips, node.getPublicAddresses());
}
checkState(size(ips) > 0, "node does not have IP addresses configured: " + node);
return FluentIterable.from(ips);
```
which removes the `else`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11421293
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #747](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/747/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40047725
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #748](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/748/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40172414
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the changes @ashley-taylor ! I think this is good to go now. Could you squash the commits into a single one so we can cleanly merge it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40182738
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -175,10 +179,17 @@ public String toString() {
> };
> }
>
> - private static FluentIterable<String> checkNodeHasIps(NodeMetadata node) {
> - FluentIterable<String> ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> - checkState(size(ips) > 0, "node does not have IP addresses configured: " + node);
> - return ips;
> + private static FluentIterable<String> checkNodeHasIps(NodeMetadata node, AllowedInterfaces allowedInterfaces) {
> + ImmutableSet.Builder<String> ipsBuilder = ImmutableSet.builder();
> + if (allowedInterfaces.scanPublic) {
> + ipsBuilder.addAll(node.getPublicAddresses());
> + }
> + if (allowedInterfaces.scanPrivate) {
> + ipsBuilder.addAll(node.getPrivateAddresses());
> + }
> + ImmutableSet<String> ips = ipsBuilder.build();
> + checkState(!ips.isEmpty(), "node does not have IP addresses configured: " + node);
[minor] Prefer the form:
```java
checkState(!ips.isEmpty(), "node does not have IP addresses configured: %s", node);
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11473174
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Gaul <no...@github.com>.
> @@ -191,4 +204,22 @@ private static void blockOn(Iterable<ListenableFuture<?>> immutableList) {
> throw propagate(e);
> }
> }
> +
> +
> + @VisibleForTesting
> + enum AllowedInterfaces{
All Java enum are implicitly static.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11467129
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #741](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/741/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39925322
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -175,8 +179,14 @@ public String toString() {
> };
> }
>
> - private static FluentIterable<String> checkNodeHasIps(NodeMetadata node) {
> - FluentIterable<String> ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> + private static FluentIterable<String> checkNodeHasIps(NodeMetadata node, boolean excludePublic) {
[minor] Rename to `excludePublicIps`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11421181
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1216](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1216/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40042733
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -191,4 +204,22 @@ private static void blockOn(Iterable<ListenableFuture<?>> immutableList) {
> throw propagate(e);
> }
> }
> +
> +
> + @VisibleForTesting
> + enum AllowedInterfaces{
`static`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11466976
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Ignasi Barrera <no...@github.com>.
>
> + @Test
> + public void testSocketFinderAllowedInterfacesPrivate() throws Exception {
> + ConcurrentOpenSocketFinder finder = new ConcurrentOpenSocketFinder(socketAlwaysOpen, nodeRunning, userExecutor);
> + finder.allowedInterfaces = AllowedInterfaces.PRIVATE;
> + HostAndPort result = finder.findOpenSocketOnNode(node, 22, 2000, MILLISECONDS);
> + assertEquals(result, HostAndPort.fromParts(PRIVATE_IP, 22));
> + }
> +
> + @Test
> + public void testSocketFinderAllowedInterfacesPublic() throws Exception {
> + ConcurrentOpenSocketFinder finder = new ConcurrentOpenSocketFinder(socketAlwaysOpen, nodeRunning, userExecutor);
> + finder.allowedInterfaces = AllowedInterfaces.PUBLIC;
> + HostAndPort result = finder.findOpenSocketOnNode(node, 22, 2000, MILLISECONDS);
> + assertEquals(result, HostAndPort.fromParts(PUBLIC_IP, 22));
> + }
As your change only modifies the `checkNodeHasIps` method (and not the check socket logic), perhaps I'd make that method visible for testing and just test that concrete method, with the different inputs, instead of going through the entire socket finder path. That would lead to simpler tests and reduce the indererminism the timeouts/mock executors may introduce.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11473292
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
PS: You should be able to see the Checkstyle result by looking at the latest jclouds-pull-requests build. Those builds should be publicly accessible.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40044846
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1211](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1211/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39925572
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -175,10 +182,17 @@ public String toString() {
> };
> }
>
> - private static FluentIterable<String> checkNodeHasIps(NodeMetadata node) {
> - FluentIterable<String> ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> - checkState(size(ips) > 0, "node does not have IP addresses configured: " + node);
> - return ips;
> + private static FluentIterable<String> checkNodeHasIps(NodeMetadata node, AllowedInterfaces allowedInterfaces) {
> + ImmutableSet.Builder<String> ipsBuilder = ImmutableSet.builder();
> + if(allowedInterfaces.scanPublic){
> + ipsBuilder.addAll(node.getPublicAddresses());
> + }
> + if(allowedInterfaces.scanPrivate){
> + ipsBuilder.addAll(node.getPrivateAddresses());
> + }
Spaces around `(...)`. But we can also take care of this during merging, if necessary.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11470574
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -63,6 +64,9 @@
> private final SocketOpen socketTester;
> private final Predicate<AtomicReference<NodeMetadata>> nodeRunning;
> private final ListeningExecutorService userExecutor;
> + @Inject(optional = true)
> + @Named(EXCLUDE_PUBLIC_IP)
> + private boolean excludePublicIp;
Does this need to be optional, or is there some way we can include it in the constructor params (defaulted to false) and make it `final`? Otherwise, at least explicitly set it to `false` here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11421151
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=b5218e0ce6351a9655bff3b59f83b53c6f5b8ab1)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40239149
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by ashley-taylor <no...@github.com>.
Commits squashed.
Thanks for the feedback and the quick responses after my commits.
Cheers, Ashley
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40191796
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> All changes must include the corresponding unit tests in the ConcurrentOpenSocketFinderTest
class.
Er...yes, doh! Good catch, @nacx...thanks! ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39959144
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -196,4 +233,14 @@ public boolean apply(HostAndPort input) {
> }
> }
> };
> +
> + private SocketOpen selectOpenSocket(final String ip){
Add a space before `{`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11467141
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
Closed #341.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1013](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1013/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40042312
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @ashley-taylor! A couple comments:
* The name of the property seems too generic (one reading it might not understand where it applies). Could you rename it so it reflects better it applies to socket testing?
* Could it be interesting to also allow to exclude the private ones too? Perhaps having an enum "SocketTesting" with "public", "private" and "all" options could lead to more flexible/complete configuration.
* All changes **must** include the corresponding unit tests in the ConcurrentOpenSocketFinderTest class.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39933486
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1220](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1220/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40195643
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -191,4 +204,22 @@ private static void blockOn(Iterable<ListenableFuture<?>> immutableList) {
> throw propagate(e);
> }
> }
> +
> +
> + @VisibleForTesting
> + enum AllowedInterfaces{
[minor] Still missing the space.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11473312
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by ashley-taylor <no...@github.com>.
> @@ -63,6 +64,9 @@
> private final SocketOpen socketTester;
> private final Predicate<AtomicReference<NodeMetadata>> nodeRunning;
> private final ListeningExecutorService userExecutor;
> + @Inject(optional = true)
> + @Named(EXCLUDE_PUBLIC_IP)
> + private boolean excludePublicIp;
reading the Guice faq optional/default values can not be specified in the constructor, have added an explicit default value to the line.
If there is a preferred alternative to this approach please let me know as I am new to Guice.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11421764
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> +
> + @Test
> + public void testSocketFinderAllowedInterfacesAllPublic() throws Exception {
> + SocketOpen socketOpen = selectOpenSocket(PUBLIC_IP);
> + ConcurrentOpenSocketFinder finder = new ConcurrentOpenSocketFinder(socketOpen, nodeRunning, userExecutor);
> + HostAndPort result = finder.findOpenSocketOnNode(node, 22, 2000, MILLISECONDS);
> + assertEquals(result, HostAndPort.fromParts(PUBLIC_IP, 22));
> + }
> +
> + @Test
> + public void testSocketFinderAllowedInterfacesAllPrivate() throws Exception {
> + SocketOpen socketOpen = selectOpenSocket(PRIVATE_IP);
> + ConcurrentOpenSocketFinder finder = new ConcurrentOpenSocketFinder(socketOpen, nodeRunning, userExecutor);
> + HostAndPort result = finder.findOpenSocketOnNode(node, 22, 2000, MILLISECONDS);
> + assertEquals(result, HostAndPort.fromParts(PRIVATE_IP, 22));
> + }
What are these two testing? That, if `ALL` is set, it's capable of finding both public and private? In that case, would it be more accurate to set up the tests so that **both** addresses are listening (but one earlier than the other), and showing that the finder finds the earlier one in both cases?
Does that make some kind of sense? ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11467176
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> checkState(size(ips) > 0, "node does not have IP addresses configured: " + node);
> - return ips;
> + return FluentIterable.from(ips);
With this setup, I think we can use a builder:
```
ImmutableSet.Builder<String> ipsBuilder = ImmutableSet.builder();
if (allowedInterfaces.scanPublic) {
ipsBuilder.addAll(node.getPublicAddresses());
}
if (allowedInterfaces.scanPrivate) {
ipsBuilder.addAll(node.getPrivateAddresses());
}
ImmutableSet<String> ips = ipsBuilder.build();
checkState(!ips.isEmpty(), "node does not have IP addresses configured: " + node);
return FluentIterable.from(ips);
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11467269
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1018](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1018/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40194787
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1015](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1015/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40047550
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1217](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1217/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40047919
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -175,8 +179,14 @@ public String toString() {
> };
> }
>
> - private static FluentIterable<String> checkNodeHasIps(NodeMetadata node) {
> - FluentIterable<String> ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> + private static FluentIterable<String> checkNodeHasIps(NodeMetadata node, boolean excludePublic) {
> + FluentIterable<String> ips;
> + if(excludePublic){
> + ips = FluentIterable.from(node.getPrivateAddresses());
> + }else{
> + ips = FluentIterable.from(concat(node.getPublicAddresses(), node.getPrivateAddresses()));
> + }
> + System.out.println(ips);
Please remove the println - debugging statement?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11421188
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1008](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1008/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39928186
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #742](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/742/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39928208
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
Thanks for the updates, @ashley-taylor! Some minor formatting things, but we can also take care of those during merging. A few [unused imports](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/746/org.apache.jclouds$jclouds-compute/violations/file/src/main/java/org/jclouds/compute/util/ConcurrentOpenSocketFinder.java/) found by CheckStyle, though - if you clean those up, could you also handle the spaces?
Otherwise, good to go for me. @nacx: functionality OK for you too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40044811
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1016](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1016/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40171952
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1212](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1212/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39928463
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1011](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1011/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40027947
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #744](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/744/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40028164
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
> @@ -191,4 +204,22 @@ private static void blockOn(Iterable<ListenableFuture<?>> immutableList) {
> throw propagate(e);
> }
> }
> +
> +
> + @VisibleForTesting
> + enum AllowedInterfaces{
> All Java enum are implicitly static.
Erm...yes...doh. Blanked out the `enum`. Thanks ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11467321
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #746](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/746/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40042459
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #750](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/750/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40195287
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1218](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1218/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40172660
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1007](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1007/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39924932
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
Some minor comments. Thanks, @ashley-taylor!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-39925000
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Andrew Phillips <no...@github.com>.
>
> + @Test
> + public void testSocketFinderAllowedInterfacesPrivate() throws Exception {
> + ConcurrentOpenSocketFinder finder = new ConcurrentOpenSocketFinder(socketAlwaysOpen, nodeRunning, userExecutor);
> + finder.allowedInterfaces = ConcurrentOpenSocketFinder.AllowedInterfaces.PRIVATE;
Static import `ConcurrentOpenSocketFinder.AllowedInterfaces`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341/files#r11467121
Re: [jclouds] JCLOUDS-528 ability to disable public ips from open
socket finder (#341)
Posted by Ignasi Barrera <no...@github.com>.
Just a couple minor style comments, and one other minor comment/advice about the tests. Functionality LGTM!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/341#issuecomment-40050434