You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/10/11 21:16:57 UTC
[2/4] brooklyn-library git commit: Improvements for PR #66
Improvements for PR #66
* Avoid NPE in in AbstractController.computePortsAndUrls() if inferUrl
returns null.
* Remove some duplication from inferUrl*()
* Renames inferUrlPublic() to inferUrlForPublic() - and same for subnet
* testMainUriSensorsCorrectlyComputedWithoutDomain to use machineLocation
extending HasSubnetHostname, so subnet address set as expected.
* Tidy deprecated code and generics in test.
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-library/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-library/commit/023e4928
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-library/tree/023e4928
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-library/diff/023e4928
Branch: refs/heads/master
Commit: 023e49287c8acdb6daf3af171e7e7277b145f63a
Parents: 313203f
Author: Aled Sage <al...@gmail.com>
Authored: Tue Oct 11 20:16:47 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Oct 11 20:16:47 2016 +0100
----------------------------------------------------------------------
.../entity/proxy/AbstractControllerImpl.java | 72 +++++++++++---------
.../entity/proxy/AbstractControllerTest.java | 46 ++++++++-----
2 files changed, 69 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/023e4928/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
index be60cf2..3277e38 100644
--- a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
+++ b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
@@ -54,6 +54,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Objects;
+import com.google.common.base.Optional;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -230,7 +231,15 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl impleme
public String getDomain() {
return getAttribute(DOMAIN_NAME);
}
-
+
+ protected String getDomainWithoutWildcard() {
+ String domain = getDomain();
+ if (domain != null && domain.startsWith("*.")) {
+ domain = domain.replace("*.", ""); // Strip wildcard
+ }
+ return domain;
+ }
+
@Override
public Integer getPort() {
if (isSsl())
@@ -267,39 +276,21 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl impleme
return isSsl() ? "https" : "http";
}
- protected String inferUrlSubnet() {
- String protocol = checkNotNull(getProtocol(), "no protocol configured");
- String domain = getDomain();
- if (domain != null && domain.startsWith("*.")) {
- domain = domain.replace("*.", ""); // Strip wildcard
- }
- Integer port = checkNotNull(getPort(), "no port configured (the requested port may be in use)");
-
+ protected String inferUrlForSubnet() {
+ String domain = getDomainWithoutWildcard();
if (domain==null) domain = getAttribute(Attributes.SUBNET_ADDRESS);
- if (domain==null) return null;
- return protocol+"://"+domain+":"+port+"/"+getConfig(SERVICE_UP_URL_PATH);
+ return inferUrl(domain, Optional.<Integer>absent());
}
- protected String inferUrlPublic() {
- String protocol = checkNotNull(getProtocol(), "no protocol configured");
- String domain = getDomain();
- if (domain != null && domain.startsWith("*.")) {
- domain = domain.replace("*.", ""); // Strip wildcard
- }
- Integer port = checkNotNull(getPort(), "no port configured (the requested port may be in use)");
-
+ protected String inferUrlForPublic() {
+ String domain = getDomainWithoutWildcard();
if (domain==null) domain = getAttribute(Attributes.ADDRESS);
- if (domain==null) return null;
- return protocol+"://"+domain+":"+port+"/"+getConfig(SERVICE_UP_URL_PATH);
+ return inferUrl(domain, Optional.<Integer>absent());
}
/** returns URL, if it can be inferred; null otherwise */
protected String inferUrl(boolean requireManagementAccessible) {
- String protocol = checkNotNull(getProtocol(), "no protocol configured");
- String domain = getDomain();
- if (domain != null && domain.startsWith("*.")) {
- domain = domain.replace("*.", ""); // Strip wildcard
- }
+ String domain = getDomainWithoutWildcard();
Integer port = checkNotNull(getPort(), "no port configured (the requested port may be in use)");
if (requireManagementAccessible) {
HostAndPort accessible = BrooklynAccessUtils.getBrooklynAccessibleAddress(this, port);
@@ -309,8 +300,15 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl impleme
}
}
if (domain==null) domain = Machines.findSubnetHostname(this).orNull();
- if (domain==null) return null;
- return protocol+"://"+domain+":"+port+"/"+getConfig(SERVICE_UP_URL_PATH);
+ return inferUrl(domain, Optional.of(port));
+ }
+
+ protected String inferUrl(String host, Optional<Integer> portOverride) {
+ if (host == null) return null;
+ String protocol = checkNotNull(getProtocol(), "no protocol configured");
+ int port = portOverride.isPresent() ? portOverride.get() : checkNotNull(getPort(), "no port configured (the requested port may be in use)");
+ String path = getConfig(SERVICE_UP_URL_PATH);
+ return protocol+"://"+host+":"+port+"/"+path;
}
protected String inferUrl() {
@@ -343,14 +341,26 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl impleme
ConfigToAttributes.apply(this);
sensors().set(PROTOCOL, inferProtocol());
- sensors().set(MAIN_URI, URI.create(inferUrl()));
- sensors().set(MAIN_URI_MAPPED_SUBNET, URI.create(inferUrlSubnet()));
- sensors().set(MAIN_URI_MAPPED_PUBLIC, URI.create(inferUrlPublic()));
+ sensors().set(MAIN_URI, createUriOrNull(inferUrl()));
+ sensors().set(MAIN_URI_MAPPED_SUBNET, createUriOrNull(inferUrlForSubnet()));
+ sensors().set(MAIN_URI_MAPPED_PUBLIC, createUriOrNull(inferUrlForPublic()));
sensors().set(ROOT_URL, inferUrl());
checkNotNull(getPortNumberSensor(), "no sensor configured to infer port number");
}
+ private URI createUriOrNull(String val) {
+ if (val == null) {
+ return null;
+ }
+ try {
+ return URI.create(val);
+ } catch (IllegalArgumentException e) {
+ LOG.warn("Invalid URI for {}: {}", this, val);
+ return null;
+ }
+ }
+
@Override
protected void connectSensors() {
super.connectSensors();
http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/023e4928/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
index 45397be..831df4c 100644
--- a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
+++ b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
@@ -31,7 +31,6 @@ import java.util.Map;
import java.util.Set;
import org.apache.brooklyn.api.entity.Entity;
-import org.apache.brooklyn.api.entity.EntityLocal;
import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.location.Location;
import org.apache.brooklyn.api.location.LocationSpec;
@@ -43,6 +42,7 @@ import org.apache.brooklyn.core.entity.Attributes;
import org.apache.brooklyn.core.entity.EntityAsserts;
import org.apache.brooklyn.core.entity.factory.EntityFactory;
import org.apache.brooklyn.core.entity.trait.Startable;
+import org.apache.brooklyn.core.location.HasSubnetHostname;
import org.apache.brooklyn.core.location.PortRanges;
import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
import org.apache.brooklyn.core.test.entity.TestEntity;
@@ -155,7 +155,7 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport {
public void testUpdateCalledWithAddressesOfNewChildren() {
// First child
cluster.resize(1);
- EntityLocal child = (EntityLocal) Iterables.getOnlyElement(cluster.getMembers());
+ Entity child = Iterables.getOnlyElement(cluster.getMembers());
List<Collection<String>> u = Lists.newArrayList(controller.getUpdates());
assertTrue(u.isEmpty(), "expected empty list but got "+u);
@@ -171,7 +171,7 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport {
public void run() {
assertEquals(cluster.getMembers().size(), 2);
}});
- EntityLocal child2 = (EntityLocal) Iterables.getOnlyElement(MutableSet.builder().addAll(cluster.getMembers()).remove(child).build());
+ Entity child2 = Iterables.getOnlyElement(MutableSet.<Entity>builder().addAll(cluster.getMembers()).remove(child).build());
child2.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
child2.sensors().set(Startable.SERVICE_UP, true);
@@ -192,8 +192,8 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport {
// Get some children, so we can remove one...
cluster.resize(2);
for (Entity it: cluster.getMembers()) {
- ((EntityLocal)it).sensors().set(ClusteredEntity.HTTP_PORT, 1234);
- ((EntityLocal)it).sensors().set(Startable.SERVICE_UP, true);
+ it.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
+ it.sensors().set(Startable.SERVICE_UP, true);
}
assertEventuallyAddressesMatchCluster();
@@ -208,17 +208,17 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport {
// Get some children, so we can remove one...
cluster.resize(2);
for (Entity it: cluster.getMembers()) {
- ((EntityLocal)it).sensors().set(ClusteredEntity.HTTP_PORT, 1234);
- ((EntityLocal)it).sensors().set(Startable.SERVICE_UP, true);
+ it.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
+ it.sensors().set(Startable.SERVICE_UP, true);
}
assertEventuallyAddressesMatchCluster();
// Now unset host/port, and remove children
// Note the unsetting of hostname is done in SoftwareProcessImpl.stop(), so this is realistic
for (Entity it : cluster.getMembers()) {
- ((EntityLocal)it).sensors().set(ClusteredEntity.HTTP_PORT, null);
- ((EntityLocal)it).sensors().set(ClusteredEntity.HOSTNAME, null);
- ((EntityLocal)it).sensors().set(Startable.SERVICE_UP, false);
+ it.sensors().set(ClusteredEntity.HTTP_PORT, null);
+ it.sensors().set(ClusteredEntity.HOSTNAME, null);
+ it.sensors().set(Startable.SERVICE_UP, false);
}
assertEventuallyAddressesMatch(ImmutableList.<Entity>of());
}
@@ -314,24 +314,34 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport {
@Test
public void testMainUriSensorsCorrectlyComputedWithoutDomain() throws Exception {
+ // The MachineLocation needs to implement HasSubnetHostname for the Attributes.SUBNET_HOSTNAME
+ // to be set with the subnet addresss (otherwise it will fall back to using machine.getAddress()).
+ // See Machines.getSubnetHostname.
+
TrackingAbstractController controller2 = app.addChild(EntitySpec.create(TrackingAbstractController.class)
.configure(TrackingAbstractController.SERVER_POOL, cluster)
.configure(TrackingAbstractController.PROXY_HTTP_PORT, PortRanges.fromInteger(8081))
- .location(LocationSpec.create(SshMachineLocation.class)
+ .location(LocationSpec.create(SshMachineLocationWithSubnetHostname.class)
.configure("address", Inet4Address.getByName("1.1.1.1"))
.configure(SshMachineLocation.PRIVATE_ADDRESSES, ImmutableList.of("2.2.2.2"))));
controller2.start(ImmutableList.<Location>of());
- // Unfortunately the Attributes.SUBNET_HOSTNAME is 1.1.1.1, because SshMachineLocation does not implement
- // HasSubnetHostname (see Machines.getSubnetHostname). It falls back to using the machine.getAddress().
- // Hence the MAIN_URI uses 1.1.1.1.
EntityAsserts.assertAttributeEquals(controller2, Attributes.ADDRESS, "1.1.1.1");
EntityAsserts.assertAttributeEquals(controller2, Attributes.SUBNET_ADDRESS, "2.2.2.2");
- EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI, URI.create("http://1.1.1.1:8081/"));
+ EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI, URI.create("http://2.2.2.2:8081/"));
EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI_MAPPED_PUBLIC, URI.create("http://1.1.1.1:8081/"));
EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI_MAPPED_SUBNET, URI.create("http://2.2.2.2:8081/"));
}
-
+ public static class SshMachineLocationWithSubnetHostname extends SshMachineLocation implements HasSubnetHostname {
+ @Override public String getSubnetHostname() {
+ return getSubnetIp();
+ }
+ @Override public String getSubnetIp() {
+ Set<String> addrs = getPrivateAddresses();
+ return (addrs.isEmpty()) ? getAddress().getHostAddress() : Iterables.get(addrs, 0);
+ }
+ }
+
private void assertEventuallyAddressesMatchCluster() {
assertEventuallyAddressesMatch(cluster.getMembers());
}
@@ -388,10 +398,10 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport {
@SetFromFlag("hostAndPort")
public static final AttributeSensor<String> HOST_AND_PORT = Attributes.HOST_AND_PORT;
- MachineProvisioningLocation provisioner;
+ MachineProvisioningLocation<MachineLocation> provisioner;
public void start(Collection<? extends Location> locs) {
- provisioner = (MachineProvisioningLocation) locs.iterator().next();
+ provisioner = (MachineProvisioningLocation<MachineLocation>) locs.iterator().next();
MachineLocation machine;
try {
machine = provisioner.obtain(MutableMap.of());