You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2015/05/28 14:07:25 UTC
[1/6] incubator-brooklyn git commit: Improvements to
JcloudsLocationSecurityGroupCustomizer
Repository: incubator-brooklyn
Updated Branches:
refs/heads/master 7aa53e0bf -> 42e9aad4e
Improvements to JcloudsLocationSecurityGroupCustomizer
* Class is no longer final. Useful methods are exposed to subclasses.
* 0.0.0.0/0 used for CIDR for SSH so that Brooklyn failing over doesn't
break its SSH access to machines.
* Incorporates SecurityGroupDefinitions.
* Adds ICMP permission to shared group.
* Predicate<Exception> can be provided to indicate that failed requests
may be retried.
* Includes AwsExceptionRetryPredicate to retry InvalidGroup.InUse,
DependencyViolation and RequestLimitExceeded errors.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/64ae942b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/64ae942b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/64ae942b
Branch: refs/heads/master
Commit: 64ae942b4296119b468fa2522ea53142e6851070
Parents: 78776ca
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Authored: Fri May 1 18:42:59 2015 +0100
Committer: Sam Corbett <sa...@cloudsoftcorp.com>
Committed: Fri May 1 18:42:59 2015 +0100
----------------------------------------------------------------------
.../JcloudsLocationSecurityGroupCustomizer.java | 164 ++++++++++++++++---
...oudsLocationSecurityGroupCustomizerTest.java | 107 +++++++++++-
2 files changed, 241 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/64ae942b/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
index 6c18ed4..98edfcf 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
@@ -18,12 +18,14 @@
*/
package brooklyn.location.jclouds.networking;
-import java.util.Collection;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
+import org.jclouds.aws.AWSResponseException;
import org.jclouds.compute.ComputeService;
import org.jclouds.compute.domain.SecurityGroup;
import org.jclouds.compute.domain.Template;
@@ -39,6 +41,8 @@ import brooklyn.location.geo.LocalhostExternalIpLoader;
import brooklyn.location.jclouds.BasicJcloudsLocationCustomizer;
import brooklyn.location.jclouds.JcloudsLocation;
import brooklyn.location.jclouds.JcloudsSshMachineLocation;
+import brooklyn.management.ManagementContext;
+import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.net.Cidr;
import brooklyn.util.task.Tasks;
import brooklyn.util.time.Duration;
@@ -47,22 +51,25 @@ import com.google.common.annotations.Beta;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
import com.google.common.base.Throwables;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
/**
* Configures custom security groups on Jclouds locations.
- *
- * TODO this should allow {@link SecurityGroupDefinition} instances to be applied
+ *
+ * @since 0.7.0
*/
@Beta
-public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
+public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocationCustomizer {
private static final Logger LOG = LoggerFactory.getLogger(JcloudsLocationSecurityGroupCustomizer.class);
@@ -77,19 +84,31 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
/** Caches the base security group that should be shared between all instances in the same Jclouds location */
private final Cache<Location, SecurityGroup> sharedGroupCache = CacheBuilder.newBuilder().build();
+
/** Caches security groups unique to instances */
private final Cache<String, SecurityGroup> uniqueGroupCache = CacheBuilder.newBuilder().build();
+ /** The context for this location customizer. */
private final String applicationId;
- private final Supplier<Cidr> brooklynCidrSupplier;
- JcloudsLocationSecurityGroupCustomizer(String applicationId) {
- this(applicationId, new LocalhostExternalIpCidrSupplier());
+ /** The CIDR for addresses that may SSH to machines. */
+ private Supplier<Cidr> sshCidrSupplier;
+
+ /**
+ * A predicate indicating whether the customiser can retry a request to add a security group
+ * or a rule after an throwable is thrown.
+ */
+ private Predicate<Exception> isExceptionRetryable = Predicates.alwaysFalse();
+
+ protected JcloudsLocationSecurityGroupCustomizer(String applicationId) {
+ // Would be better to restrict with something like LocalhostExternalIpCidrSupplier, but
+ // we risk making machines inaccessible from Brooklyn when HA fails over.
+ this(applicationId, Suppliers.ofInstance(new Cidr("0.0.0.0/0")));
}
- JcloudsLocationSecurityGroupCustomizer(String applicationId, Supplier<Cidr> cidrSupplier) {
+ protected JcloudsLocationSecurityGroupCustomizer(String applicationId, Supplier<Cidr> sshCidrSupplier) {
this.applicationId = applicationId;
- this.brooklynCidrSupplier = cidrSupplier;
+ this.sshCidrSupplier = sshCidrSupplier;
}
/**
@@ -112,34 +131,61 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
return getInstance(entity.getApplicationId());
}
- /** @see #addPermissionsToLocation(brooklyn.location.jclouds.JcloudsSshMachineLocation, java.util.Collection) */
- public void addPermissionsToLocation(final JcloudsSshMachineLocation location, IpPermission... permissions) {
+ /**
+ * @param predicate
+ * A predicate whose return value indicates whether a request to add a security group
+ * or permission may be retried after its input {@link Exception} was thrown.
+ * @return this
+ */
+ public JcloudsLocationSecurityGroupCustomizer setRetryExceptionPredicate(Predicate<Exception> predicate) {
+ this.isExceptionRetryable = checkNotNull(predicate, "predicate");
+ return this;
+ }
+
+ /**
+ * @param cidrSupplier A supplier returning a CIDR for hosts that are allowed to SSH to locations.
+ */
+ public JcloudsLocationSecurityGroupCustomizer setSshCidrSupplier(Supplier<Cidr> cidrSupplier) {
+ this.sshCidrSupplier = checkNotNull(cidrSupplier, "cidrSupplier");
+ return this;
+ }
+
+ /** @see #addPermissionsToLocation(brooklyn.location.jclouds.JcloudsSshMachineLocation, java.lang.Iterable) */
+ public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsSshMachineLocation location, IpPermission... permissions) {
addPermissionsToLocation(location, ImmutableList.copyOf(permissions));
+ return this;
}
+ /** @see #addPermissionsToLocation(brooklyn.location.jclouds.JcloudsSshMachineLocation, java.lang.Iterable) */
+ public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsSshMachineLocation location, SecurityGroupDefinition securityGroupDefinition) {
+ addPermissionsToLocation(location, securityGroupDefinition.getPermissions());
+ return this;
+ }
+
/**
* Applies the given security group permissions to the given location.
- * <p/>
+ * <p>
* Takes no action if the location's compute service does not have a security group extension.
* @param permissions The set of permissions to be applied to the location
* @param location Location to gain permissions
*/
- public void addPermissionsToLocation(final JcloudsSshMachineLocation location, final Collection<IpPermission> permissions) {
+ public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsSshMachineLocation location, final Iterable<IpPermission> permissions) {
ComputeService computeService = location.getParent().getComputeService();
String nodeId = location.getNode().getId();
addPermissionsToLocation(permissions, nodeId, computeService);
+ return this;
}
/**
* Applies the given security group permissions to the given node with the given compute service.
- * <p/>
+ * <p>
* Takes no action if the compute service does not have a security group extension.
* @param permissions The set of permissions to be applied to the node
* @param nodeId The id of the node to update
* @param computeService The compute service to use to apply the changes
*/
@VisibleForTesting
- void addPermissionsToLocation(Collection<IpPermission> permissions, final String nodeId, ComputeService computeService) {
+ void addPermissionsToLocation(Iterable<IpPermission> permissions, final String nodeId, ComputeService computeService) {
if (!computeService.getSecurityGroupExtension().isPresent()) {
LOG.warn("Security group extension for {} absent; cannot update node {} with {}",
new Object[] {computeService, nodeId, permissions});
@@ -221,7 +267,7 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
* SSH access on port 22 and allows communication on all ports between machines in
* the same group. Security groups are reused when templates have equal
* {@link org.jclouds.compute.domain.Template#getLocation locations}.
- * <p/>
+ * <p>
* This method is called by Brooklyn when obtaining machines, as part of the
* {@link brooklyn.location.jclouds.JcloudsLocationCustomizer} contract. It
* should not be called from anywhere else.
@@ -279,7 +325,8 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
final String groupName = getNameForSharedSecurityGroup();
// Could sort-and-search if straight search is too expensive
Optional<SecurityGroup> shared = Iterables.tryFind(securityApi.listSecurityGroupsInLocation(location), new Predicate<SecurityGroup>() {
- @Override public boolean apply(final SecurityGroup input) {
+ @Override
+ public boolean apply(final SecurityGroup input) {
// endsWith because Jclouds prepends 'jclouds#' to security group names.
return input.getName().endsWith(groupName);
}
@@ -299,7 +346,7 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
* Creates a security group with rules to:
* <ul>
* <li>Allow SSH access on port 22 from the world</li>
- * <li>Allow all communication between machines in the same group</li>
+ * <li>Allow TCP, UDP and ICMP communication between machines in the same group</li>
* </ul>
* @param groupName The name of the security group to create
* @param location The location in which the security group will be created
@@ -318,31 +365,44 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
.toPort(65535);
addPermission(allWithinGroup.ipProtocol(IpProtocol.TCP).build(), group, securityApi);
addPermission(allWithinGroup.ipProtocol(IpProtocol.UDP).build(), group, securityApi);
+ addPermission(allWithinGroup.ipProtocol(IpProtocol.ICMP).fromPort(-1).toPort(-1).build(), group, securityApi);
IpPermission sshPermission = IpPermission.builder()
.fromPort(22)
.toPort(22)
.ipProtocol(IpProtocol.TCP)
- .cidrBlock(brooklynCidrSupplier.get().toString())
+ .cidrBlock(getBrooklynCidrBlock())
.build();
addPermission(sshPermission, group, securityApi);
return group;
}
- private SecurityGroup addSecurityGroupInLocation(String groupName, Location location, SecurityGroupExtension securityApi) {
+ protected SecurityGroup addSecurityGroupInLocation(final String groupName, final Location location, final SecurityGroupExtension securityApi) {
LOG.debug("Creating security group {} in {}", groupName, location);
- return securityApi.createSecurityGroup(groupName, location);
+ Callable<SecurityGroup> callable = new Callable<SecurityGroup>() {
+ @Override
+ public SecurityGroup call() throws Exception {
+ return securityApi.createSecurityGroup(groupName, location);
+ }
+ };
+ return runOperationWithRetry(callable);
}
- private SecurityGroup addPermission(IpPermission permission, SecurityGroup group, SecurityGroupExtension securityApi) {
+ protected SecurityGroup addPermission(final IpPermission permission, final SecurityGroup group, final SecurityGroupExtension securityApi) {
LOG.debug("Adding permission to security group {}: {}", group.getName(), permission);
- return securityApi.addIpPermission(permission, group);
+ Callable<SecurityGroup> callable = new Callable<SecurityGroup>() {
+ @Override
+ public SecurityGroup call() throws Exception {
+ return securityApi.addIpPermission(permission, group);
+ }
+ };
+ return runOperationWithRetry(callable);
}
/** @return the CIDR block used to configure Brooklyn's in security groups */
public String getBrooklynCidrBlock() {
- return brooklynCidrSupplier.get().toString();
+ return sshCidrSupplier.get().toString();
}
/**
@@ -366,6 +426,62 @@ public final class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLo
}
/**
+ * Runs the given callable. Repeats until the operation succeeds or {@link #isExceptionRetryable} indicates
+ * that the request cannot be retried.
+ */
+ protected <T> T runOperationWithRetry(Callable<T> operation) {
+ int backoff = 64;
+ Exception lastException = null;
+ for (int retries = 0; retries < 100; retries++) {
+ try {
+ return operation.call();
+ } catch (Exception e) {
+ lastException = e;
+ if (isExceptionRetryable.apply(e)) {
+ LOG.debug("Attempt #{} failed to add security group: {}", retries + 1, e.getMessage());
+ try {
+ Thread.sleep(backoff);
+ } catch (InterruptedException e1) {
+ throw Exceptions.propagate(e1);
+ }
+ backoff = backoff << 1;
+ } else {
+ break;
+ }
+ }
+ }
+
+ throw new RuntimeException("Unable to add security group rule; repeated errors from provider", lastException);
+ }
+
+ /**
+ * @return
+ * A predicate that is true if an exception contains an {@link org.jclouds.aws.AWSResponseException}
+ * whose error code is either <code>InvalidGroup.InUse</code>, <code>DependencyViolation</code> or
+ * <code>RequestLimitExceeded</code>.
+ */
+ public static Predicate<Exception> newAwsExceptionRetryPredicate() {
+ return new AwsExceptionRetryPredicate();
+ }
+
+ private static class AwsExceptionRetryPredicate implements Predicate<Exception> {
+ // Error reference: http://docs.aws.amazon.com/AWSEC2/latest/APIReference/errors-overview.html
+ private static final Set<String> AWS_ERRORS_TO_RETRY = ImmutableSet.of(
+ "InvalidGroup.InUse", "DependencyViolation", "RequestLimitExceeded");
+
+ @Override
+ public boolean apply(Exception input) {
+ @SuppressWarnings("ThrowableResultOfMethodCallIgnored")
+ AWSResponseException exception = Exceptions.getFirstThrowableOfType(input, AWSResponseException.class);
+ if (exception != null) {
+ String code = exception.getError().getCode();
+ return AWS_ERRORS_TO_RETRY.contains(code);
+ }
+ return false;
+ }
+ }
+
+ /**
* A supplier of CIDRs that loads the external IP address of the localhost machine.
*/
private static class LocalhostExternalIpCidrSupplier implements Supplier<Cidr> {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/64ae942b/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java b/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
index c2fcbb0..643f78f 100644
--- a/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
+++ b/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
@@ -29,10 +29,13 @@ import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertTrue;
import java.net.URI;
import java.util.Collections;
+import org.jclouds.aws.AWSResponseException;
+import org.jclouds.aws.domain.AWSError;
import org.jclouds.compute.ComputeService;
import org.jclouds.compute.domain.SecurityGroup;
import org.jclouds.compute.domain.Template;
@@ -44,15 +47,16 @@ import org.jclouds.net.domain.IpProtocol;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import brooklyn.location.jclouds.JcloudsLocation;
-import brooklyn.util.collections.MutableMap;
-import brooklyn.util.net.Cidr;
-
import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import brooklyn.location.jclouds.JcloudsLocation;
+import brooklyn.util.collections.MutableMap;
+import brooklyn.util.net.Cidr;
+
public class JcloudsLocationSecurityGroupCustomizerTest {
JcloudsLocationSecurityGroupCustomizer customizer;
@@ -100,9 +104,9 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
customizer.customize(jcloudsLocationB, computeService, template);
// One group with three permissions shared by both locations.
- // Expect TCP+UDP between members of group and SSH to Brooklyn
+ // Expect TCP, UDP and ICMP between members of group and SSH to Brooklyn
verify(securityApi).createSecurityGroup(anyString(), eq(location));
- verify(securityApi, times(3)).addIpPermission(any(IpPermission.class), eq(group));
+ verify(securityApi, times(4)).addIpPermission(any(IpPermission.class), eq(group));
// New groups set on options
verify(templateOptions, times(2)).securityGroups(anyString());
}
@@ -183,6 +187,86 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
verify(securityApi, never()).addIpPermission(any(IpPermission.class), eq(sharedGroup));
}
+ @Test
+ public void testAddRuleNotRetriedByDefault() {
+ IpPermission ssh = newPermission(22);
+ String nodeId = "node";
+ SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
+ SecurityGroup uniqueGroup = newGroup("unique");
+ when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, uniqueGroup));
+ when(securityApi.addIpPermission(eq(ssh), eq(uniqueGroup)))
+ .thenThrow(new RuntimeException("exception creating " + ssh));
+
+ try {
+ customizer.addPermissionsToLocation(ImmutableList.of(ssh), nodeId, computeService);
+ } catch (Exception e) {
+ assertTrue(e.getMessage().contains("repeated errors from provider"), "message=" + e.getMessage());
+ }
+ verify(securityApi, never()).createSecurityGroup(anyString(), any(Location.class));
+ verify(securityApi, times(1)).addIpPermission(ssh, uniqueGroup);
+ }
+
+ @Test
+ public void testCustomExceptionRetryablePredicate() {
+ final String message = "testCustomExceptionRetryablePredicate";
+ Predicate<Exception> messageChecker = new Predicate<Exception>() {
+ @Override
+ public boolean apply(Exception input) {
+ Throwable t = input;
+ while (t != null) {
+ if (t.getMessage().contains(message)) {
+ return true;
+ } else {
+ t = t.getCause();
+ }
+ }
+ return false;
+ }
+ };
+ customizer.setRetryExceptionPredicate(messageChecker);
+
+ IpPermission ssh = newPermission(22);
+ String nodeId = "node";
+ SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
+ SecurityGroup uniqueGroup = newGroup("unique");
+ when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, uniqueGroup));
+ when(securityApi.addIpPermission(eq(ssh), eq(uniqueGroup)))
+ .thenThrow(new RuntimeException(new Exception(message)))
+ .thenThrow(new RuntimeException(new Exception(message)))
+ .thenReturn(sharedGroup);
+
+ customizer.addPermissionsToLocation(ImmutableList.of(ssh), nodeId, computeService);
+
+ verify(securityApi, never()).createSecurityGroup(anyString(), any(Location.class));
+ verify(securityApi, times(3)).addIpPermission(ssh, uniqueGroup);
+ }
+
+ @Test
+ public void testAddRuleRetriedOnAwsFailure() {
+ IpPermission ssh = newPermission(22);
+ String nodeId = "nodeId";
+ SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
+ SecurityGroup uniqueGroup = newGroup("unique");
+ customizer.setRetryExceptionPredicate(JcloudsLocationSecurityGroupCustomizer.newAwsExceptionRetryPredicate());
+ when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, uniqueGroup));
+ when(securityApi.addIpPermission(any(IpPermission.class), eq(uniqueGroup)))
+ .thenThrow(newAwsResponseExceptionWithCode("InvalidGroup.InUse"))
+ .thenThrow(newAwsResponseExceptionWithCode("DependencyViolation"))
+ .thenThrow(newAwsResponseExceptionWithCode("RequestLimitExceeded"))
+ .thenThrow(newAwsResponseExceptionWithCode("Blocked"))
+ .thenReturn(sharedGroup);
+
+ try {
+ customizer.addPermissionsToLocation(ImmutableList.of(ssh), nodeId, computeService);
+ } catch (Exception e) {
+ String expected = "repeated errors from provider";
+ assertTrue(e.getMessage().contains(expected), "expected exception message to contain " + expected + ", was: " + e.getMessage());
+ }
+
+ verify(securityApi, never()).createSecurityGroup(anyString(), any(Location.class));
+ verify(securityApi, times(4)).addIpPermission(ssh, uniqueGroup);
+ }
+
private SecurityGroup newGroup(String id) {
URI uri = null;
String ownerId = null;
@@ -206,4 +290,15 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
.cidrBlock("0.0.0.0/0")
.build();
}
+
+ private AWSError newAwsErrorWithCode(String code) {
+ AWSError e = new AWSError();
+ e.setCode(code);
+ return e;
+ }
+
+ private Exception newAwsResponseExceptionWithCode(String code) {
+ AWSResponseException e = new AWSResponseException("irrelevant message", null, null, newAwsErrorWithCode(code));
+ return new RuntimeException(e);
+ }
}
[5/6] incubator-brooklyn git commit: This closes #654
Posted by he...@apache.org.
This closes #654
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/8ab0fbb6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/8ab0fbb6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/8ab0fbb6
Branch: refs/heads/master
Commit: 8ab0fbb6b12ae1d01819eb7e14a34d9f8fca155b
Parents: a5b601f 9141ca4
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 13:06:39 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 13:06:39 2015 +0100
----------------------------------------------------------------------
.../src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
[4/6] incubator-brooklyn git commit: This closes #662
Posted by he...@apache.org.
This closes #662
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/a5b601ff
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/a5b601ff
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/a5b601ff
Branch: refs/heads/master
Commit: a5b601ffae29f61683ce72701651bc3cc0116b6a
Parents: 7aa53e0 316b421
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 13:00:42 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 13:00:42 2015 +0100
----------------------------------------------------------------------
.../main/java/brooklyn/util/net/Networking.java | 128 +++++++++++++++----
.../brooklyn/util/net/NetworkingUtilsTest.java | 24 +++-
2 files changed, 117 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
[3/6] incubator-brooklyn git commit: Fix for the "unable to find a
free port" error that has been plaguing the jenkins builds
Posted by he...@apache.org.
Fix for the "unable to find a free port" error that has been plaguing the jenkins builds
If the free port check fails for a specific interface, do an additional check if able to bind to an OS picked port on the same IP. If it fails, then there's something wrong with the interface, not just a port already bound error.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/316b421b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/316b421b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/316b421b
Branch: refs/heads/master
Commit: 316b421bc47240c794dc483ea6def3672fe4649d
Parents: 776ad43
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Wed May 27 14:51:27 2015 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Thu May 28 10:19:39 2015 +0300
----------------------------------------------------------------------
.../main/java/brooklyn/util/net/Networking.java | 128 +++++++++++++++----
.../brooklyn/util/net/NetworkingUtilsTest.java | 24 +++-
2 files changed, 117 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/316b421b/utils/common/src/main/java/brooklyn/util/net/Networking.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/net/Networking.java b/utils/common/src/main/java/brooklyn/util/net/Networking.java
index 7a7368c..4337dc4 100644
--- a/utils/common/src/main/java/brooklyn/util/net/Networking.java
+++ b/utils/common/src/main/java/brooklyn/util/net/Networking.java
@@ -79,17 +79,14 @@ public class Networking {
throw new IllegalArgumentException("Invalid start port: " + port);
}
- // For some operations it's not valid to pass ANY_NIC (0.0.0.0).
- // We substitute for the loopback address in those cases.
- InetAddress localAddressNotAny = (localAddress==null || ANY_NIC.equals(localAddress))
- ? LOOPBACK
- : localAddress;
-
Stopwatch watch = Stopwatch.createStarted();
try {
//despite http://stackoverflow.com/questions/434718/sockets-discover-port-availability-using-java
//(recommending the following) it isn't 100% reliable (e.g. nginx will happily coexist with ss+ds)
- //so we also do the above check
+ //
+ //Svet - SO_REUSEADDR (enabled below) will allow one socket to listen on 0.0.0.0:X and another on
+ //192.168.0.1:X which explains the above comment (nginx sets SO_REUSEADDR as well). Moreover there
+ //is no TIME_WAIT for listening sockets without any connections so why enable it at all.
ServerSocket ss = null;
DatagramSocket ds = null;
try {
@@ -105,19 +102,11 @@ public class Networking {
ds.setReuseAddress(true);
ds.bind(new InetSocketAddress(localAddress, port));
} catch (IOException e) {
+ if (log.isTraceEnabled()) log.trace("Failed binding to " + localAddress + " : " + port, e);
return false;
} finally {
- if (ds != null) {
- ds.close();
- }
-
- if (ss != null) {
- try {
- ss.close();
- } catch (IOException e) {
- /* should not be thrown */
- }
- }
+ closeQuietly(ds);
+ closeQuietly(ss);
}
if (localAddress==null || ANY_NIC.equals(localAddress)) {
@@ -129,24 +118,84 @@ public class Networking {
} catch (SocketException e) {
throw Exceptions.propagate(e);
}
+ // When using a specific interface saw failures not caused by port already bound:
+ // * java.net.SocketException: No such device
+ // * java.net.BindException: Cannot assign requested address
+ // * probably many more
+ // Check if the address is still valid before marking the port as not available.
+ boolean foundAvailableInterface = false;
while (nis.hasMoreElements()) {
NetworkInterface ni = nis.nextElement();
Enumeration<InetAddress> as = ni.getInetAddresses();
while (as.hasMoreElements()) {
InetAddress a = as.nextElement();
- if (!isPortAvailable(a, port))
- return false;
+ if (!isPortAvailable(a, port)) {
+ if (isAddressValid(a)) {
+ if (log.isTraceEnabled()) log.trace("Port {} : {} @ {} is taken and the address is valid", new Object[] {a, port, nis});
+ return false;
+ }
+ } else {
+ foundAvailableInterface = true;
+ }
}
}
+ if (!foundAvailableInterface) {
+ //Aborting with an error, even nextAvailablePort won't be able to find a free port.
+ throw new RuntimeException("Unable to bind on any network interface, even when letting the OS pick a port. Possible causes include file handle exhaustion, port exhaustion. Failed on request for " + localAddress + ":" + port + ".");
+ }
}
-
+
return true;
} finally {
// Until timeout was added, was taking 1min5secs for /fe80:0:0:0:1cc5:1ff:fe81:a61d%8 : 8081
- if (log.isTraceEnabled()) log.trace("Took {} to determine if port {} : {} was available",
- new Object[] {Time.makeTimeString(watch.elapsed(TimeUnit.MILLISECONDS), true), localAddress, port});
+ // Svet - Probably caused by the now gone new Socket().connect() call, SO_TIMEOUT doesn't
+ // influence bind(). Doesn't hurt having it though.
+ long elapsed = watch.elapsed(TimeUnit.SECONDS);
+ boolean isDelayed = (elapsed >= 1);
+ boolean isDelayedByMuch = (elapsed >= 30);
+ if (isDelayed || log.isTraceEnabled()) {
+ String msg = "Took {} to determine if port was available for {} : {}";
+ Object[] args = new Object[] {Time.makeTimeString(watch.elapsed(TimeUnit.MILLISECONDS), true), localAddress, port};
+ if (isDelayedByMuch) {
+ log.warn(msg, args);
+ } else if (isDelayed) {
+ log.debug(msg, args);
+ } else {
+ log.trace(msg, args);
+ }
+ }
}
}
+
+ /**
+ * Bind to the specified IP, but let the OS pick a port.
+ * If the operation fails we know it's not because of
+ * non-available port, the interface could be down.
+ *
+ * If there's port exhaustion on a single interface we won't catch it
+ * and declare the port is free. Doesn't matter really because the
+ * subsequent bind of the caller will fail anyway and nextAvailablePort
+ * wouldn't be able to find a free one either.
+ */
+ private static boolean isAddressValid(InetAddress addr) {
+ ServerSocket ss;
+ try {
+ ss = new ServerSocket();
+ ss.setSoTimeout(250);
+ } catch (IOException e) {
+ throw Exceptions.propagate(e);
+ }
+ try {
+ ss.bind(new InetSocketAddress(addr, 0));
+ return true;
+ } catch (IOException e) {
+ if (log.isTraceEnabled()) log.trace("Binding on {} failed, interface could be down, being reconfigured, file handle exhaustion, port exhaustion, etc.", addr);
+ return false;
+ } finally {
+ closeQuietly(ss);
+ }
+ }
+
/** returns the first port available on the local machine >= the port supplied */
public static int nextAvailablePort(int port) {
checkArgument(port >= MIN_PORT_NUMBER && port <= MAX_PORT_NUMBER, "requested port %s is outside the valid range of %s to %s", port, MIN_PORT_NUMBER, MAX_PORT_NUMBER);
@@ -440,11 +489,7 @@ public class Networking {
public static boolean isReachable(HostAndPort endpoint) {
try {
Socket s = new Socket(endpoint.getHostText(), endpoint.getPort());
- try {
- s.close();
- } catch (Exception e) {
- log.debug("Error closing socket, opened temporarily to check reachability to "+endpoint+" (continuing)", e);
- }
+ closeQuietly(s);
return true;
} catch (Exception e) {
if (log.isTraceEnabled()) log.trace("Error reaching "+endpoint+" during reachability check (return false)", e);
@@ -452,6 +497,33 @@ public class Networking {
}
}
+ public static void closeQuietly(Socket s) {
+ if (s != null) {
+ try {
+ s.close();
+ } catch (IOException e) {
+ /* should not be thrown */
+ }
+ }
+ }
+
+ public static void closeQuietly(ServerSocket s) {
+ if (s != null) {
+ try {
+ s.close();
+ } catch (IOException e) {
+ /* should not be thrown */
+ }
+ }
+ }
+
+ public static void closeQuietly(DatagramSocket s) {
+ if (s != null) {
+ s.close();
+ }
+ }
+
+
// TODO go through nic's, looking for public, private, etc, on localhost
/**
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/316b421b/utils/common/src/test/java/brooklyn/util/net/NetworkingUtilsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/brooklyn/util/net/NetworkingUtilsTest.java b/utils/common/src/test/java/brooklyn/util/net/NetworkingUtilsTest.java
index a5623d9..48202a6 100644
--- a/utils/common/src/test/java/brooklyn/util/net/NetworkingUtilsTest.java
+++ b/utils/common/src/test/java/brooklyn/util/net/NetworkingUtilsTest.java
@@ -20,12 +20,14 @@ package brooklyn.util.net;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
import java.io.IOException;
import java.net.InetAddress;
+import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.UnknownHostException;
import java.util.concurrent.TimeUnit;
@@ -96,7 +98,7 @@ public class NetworkingUtilsTest {
}
}
- @Test(groups="Integration")
+ @Test
public void testIsPortAvailableReportsTrueWhenPortIsFree() throws Exception {
int port = 58769;
int numFree = 0;
@@ -108,10 +110,7 @@ public class NetworkingUtilsTest {
fail("This test requires that at least some ports near 58769+ not be in use.");
}
- // Integration because fails in apache jenkins sometimes with "could not get a port".
- // Could all the ports between 58767 and 60000 be in use (!) or is there a restriction in
- // the environment?
- @Test(groups="Integration")
+ @Test
public void testIsPortAvailableReportsFalseWhenPortIsInUse() throws Exception {
int port = 58767;
ServerSocket ss = null;
@@ -139,8 +138,7 @@ public class NetworkingUtilsTest {
}});
}
- // See comment on {@link #testIsPortAvailableReportsFalseWhenPortIsInUse()} for why this is integration.
- @Test(groups="Integration")
+ @Test
public void testIsPortAvailableReportsPromptly() throws Exception {
// repeat until we can get an available port
int port = 58767;
@@ -157,6 +155,18 @@ public class NetworkingUtilsTest {
Assert.assertTrue(available);
}
+
+ @Test
+ public void testIsPortAvailableValidatesAddress() throws Exception {
+ ServerSocket ss = new ServerSocket();
+ ss.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0));
+ int boundPort = ss.getLocalPort();
+ assertTrue(ss.isBound());
+ assertNotEquals(boundPort, 0);
+ //will run isAddressValid before returning
+ assertFalse(Networking.isPortAvailable(boundPort));
+ ss.close();
+ }
//just some system health-checks... localhost may not resolve properly elsewhere
//(e.g. in geobytes, AddressableLocation, etc) if this does not work
[2/6] incubator-brooklyn git commit: Replaced Jetty6Server download
url
Posted by he...@apache.org.
Replaced Jetty6Server download url
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/9141ca4b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/9141ca4b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/9141ca4b
Branch: refs/heads/master
Commit: 9141ca4b0cecff8005058a822332d469651fb54b
Parents: 4d673a8
Author: Miguel Barrientos <mb...@lcc.uma.es>
Authored: Fri May 22 15:15:20 2015 +0200
Committer: Miguel Barrientos <mb...@lcc.uma.es>
Committed: Fri May 22 15:15:20 2015 +0200
----------------------------------------------------------------------
.../src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9141ca4b/software/webapp/src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java b/software/webapp/src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java
index 89510ad..cf407b2 100644
--- a/software/webapp/src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java
+++ b/software/webapp/src/main/java/brooklyn/entity/webapp/jetty/Jetty6Server.java
@@ -49,7 +49,7 @@ public interface Jetty6Server extends JavaWebAppSoftwareProcess, UsesJmx, HasSho
@SetFromFlag("downloadUrl")
BasicAttributeSensorAndConfigKey<String> DOWNLOAD_URL = new BasicAttributeSensorAndConfigKey<String>(
- SoftwareProcess.DOWNLOAD_URL, "http://dist.codehaus.org/jetty/jetty-${version}/jetty-${version}.zip");
+ SoftwareProcess.DOWNLOAD_URL, "http://get.jenv.mvnsearch.org/download/jetty/jetty-${version}.zip");
AttributeSensor<Integer> RESPONSES_4XX_COUNT =
Sensors.newIntegerSensor("webapp.responses.4xx", "Responses in the 400's");
[6/6] incubator-brooklyn git commit: This closes #623
Posted by he...@apache.org.
This closes #623
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/42e9aad4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/42e9aad4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/42e9aad4
Branch: refs/heads/master
Commit: 42e9aad4e935b791bdfa10e304ee531dced19bde
Parents: 8ab0fbb 64ae942
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 13:06:59 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 13:06:59 2015 +0100
----------------------------------------------------------------------
.../JcloudsLocationSecurityGroupCustomizer.java | 164 ++++++++++++++++---
...oudsLocationSecurityGroupCustomizerTest.java | 107 +++++++++++-
2 files changed, 241 insertions(+), 30 deletions(-)
----------------------------------------------------------------------