Repository: incubator-brooklyn Updated Branches: refs/heads/master 738747907 -> 1a48100b6
Fixes issue where port was not being opened if port range was overridden Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/af19467e Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/af19467e Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/af19467e Branch: refs/heads/master Commit: af19467ee1840daccba83ca09e04f17d69c081a7 Parents: b5cc277 Author: Martin Harris <[email protected]> Authored: Thu Jun 25 15:14:30 2015 +0100 Committer: Martin Harris <[email protected]> Committed: Fri Jun 26 12:39:43 2015 +0100 ---------------------------------------------------------------------- .../SameServerDriverLifecycleEffectorTasks.java | 25 +++++++++++++++-- .../entity/basic/SoftwareProcessImpl.java | 29 +++++++++++++++----- .../entity/basic/SoftwareProcessEntityTest.java | 20 +++++++++++++- 3 files changed, 64 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/af19467e/software/base/src/main/java/brooklyn/entity/basic/SameServerDriverLifecycleEffectorTasks.java ---------------------------------------------------------------------- diff --git a/software/base/src/main/java/brooklyn/entity/basic/SameServerDriverLifecycleEffectorTasks.java b/software/base/src/main/java/brooklyn/entity/basic/SameServerDriverLifecycleEffectorTasks.java index 34c3f04..bb3b1fd 100644 --- a/software/base/src/main/java/brooklyn/entity/basic/SameServerDriverLifecycleEffectorTasks.java +++ b/software/base/src/main/java/brooklyn/entity/basic/SameServerDriverLifecycleEffectorTasks.java @@ -29,6 +29,7 @@ import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import com.google.common.reflect.TypeToken; import brooklyn.config.ConfigKey; import brooklyn.entity.Entity; @@ -40,6 +41,8 @@ import brooklyn.location.PortRange; import brooklyn.location.basic.LocationConfigKeys; import brooklyn.management.TaskAdaptable; import brooklyn.util.collections.MutableSet; +import brooklyn.util.flags.TypeCoercions; +import brooklyn.util.guava.Maybe; import brooklyn.util.task.DynamicTasks; public class SameServerDriverLifecycleEffectorTasks extends MachineLifecycleEffectorTasks { @@ -64,9 +67,27 @@ public class SameServerDriverLifecycleEffectorTasks extends MachineLifecycleEffe /** @return the ports required for a specific child entity */ protected Collection<Integer> getRequiredOpenPorts(Entity entity) { Set<Integer> ports = MutableSet.of(22); + /* TODO: This won't work if there's a port collision, which will cause the corresponding port attribute + to be incremented until a free port is found. In that case the entity will use the free port, but the + firewall will open the initial port instead. Mostly a problem for SameServerEntity, localhost location. + */ + // TODO: Remove duplication between this and SoftwareProcessImpl.getRequiredOpenPorts for (ConfigKey<?> k: entity.getEntityType().getConfigKeys()) { - if (PortRange.class.isAssignableFrom(k.getType())) { - PortRange p = (PortRange) entity.getConfig(k); + Object value; + if (PortRange.class.isAssignableFrom(k.getType()) || k.getName().matches(".*\\.port")) { + value = entity.config().get(k); + } else { + // config().get() will cause this to block until all config has been resolved + // using config().getRaw(k) means that we won't be able to use e.g. 'http.port: $brooklyn:component("x").attributeWhenReady("foo")' + // but that's unlikely to be used + Maybe<Object> maybeValue = ((AbstractEntity.BasicConfigurationSupport)entity.config()).getRaw(k); + value = maybeValue.isPresent() ? maybeValue.get() : null; + } + + Maybe<PortRange> maybePortRange = TypeCoercions.tryCoerce(value, new TypeToken<PortRange>() {}); + + if (maybePortRange.isPresentAndNonNull()) { + PortRange p = maybePortRange.get(); if (p != null && !p.isEmpty()) ports.add(p.iterator().next()); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/af19467e/software/base/src/main/java/brooklyn/entity/basic/SoftwareProcessImpl.java ---------------------------------------------------------------------- diff --git a/software/base/src/main/java/brooklyn/entity/basic/SoftwareProcessImpl.java b/software/base/src/main/java/brooklyn/entity/basic/SoftwareProcessImpl.java index a651f65..3d5511f 100644 --- a/software/base/src/main/java/brooklyn/entity/basic/SoftwareProcessImpl.java +++ b/software/base/src/main/java/brooklyn/entity/basic/SoftwareProcessImpl.java @@ -18,6 +18,8 @@ */ package brooklyn.entity.basic; +import brooklyn.util.flags.TypeCoercions; +import brooklyn.util.guava.Maybe; import groovy.time.TimeDuration; import java.util.Collection; @@ -66,6 +68,7 @@ import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; +import com.google.common.reflect.TypeToken; /** * An {@link Entity} representing a piece of software which can be installed, run, and controlled. @@ -448,15 +451,27 @@ public abstract class SoftwareProcessImpl extends AbstractEntity implements Soft Set<ConfigKey<?>> configKeys = Sets.newHashSet(allConfig.keySet()); configKeys.addAll(getEntityType().getConfigKeys()); + /* TODO: This won't work if there's a port collision, which will cause the corresponding port attribute + to be incremented until a free port is found. In that case the entity will use the free port, but the + firewall will open the initial port instead. Mostly a problem for SameServerEntity, localhost location. + */ for (ConfigKey<?> k: configKeys) { - if (PortRange.class.isAssignableFrom(k.getType())) { - PortRange p = (PortRange)getConfig(k); + Object value; + if (PortRange.class.isAssignableFrom(k.getType()) || k.getName().matches(".*\\.port")) { + value = config().get(k); + } else { + // config().get() will cause this to block until all config has been resolved + // using config().getRaw(k) means that we won't be able to use e.g. 'http.port: $brooklyn:component("x").attributeWhenReady("foo")' + // but that's unlikely to be used + Maybe<Object> maybeValue = config().getRaw(k); + value = maybeValue.isPresent() ? maybeValue.get() : null; + } + + Maybe<PortRange> maybePortRange = TypeCoercions.tryCoerce(value, new TypeToken<PortRange>() {}); + + if (maybePortRange.isPresentAndNonNull()) { + PortRange p = maybePortRange.get(); if (p != null && !p.isEmpty()) ports.add(p.iterator().next()); - } else if(k.getName().matches(".*\\.port")){ - Object value = getConfig(k); - if (value instanceof Integer){ - ports.add((Integer)value); - } } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/af19467e/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java ---------------------------------------------------------------------- diff --git a/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java b/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java index c503b84..55deb4e 100644 --- a/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java +++ b/software/base/src/test/java/brooklyn/entity/basic/SoftwareProcessEntityTest.java @@ -30,6 +30,7 @@ import brooklyn.entity.proxying.EntitySpec; import brooklyn.entity.proxying.ImplementedBy; import brooklyn.entity.software.MachineLifecycleEffectorTasksTest; import brooklyn.entity.trait.Startable; +import brooklyn.event.basic.PortAttributeSensorAndConfigKey; import brooklyn.location.Location; import brooklyn.location.LocationSpec; import brooklyn.location.basic.FixedListMachineProvisioningLocation; @@ -59,6 +60,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import java.util.ArrayList; +import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -410,9 +412,18 @@ public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport { doTestReleaseEvenIfErrorDuringStop(SimulatedFailInChildOnStopDriver.class); } + @Test + public void testOpenPortsWithPortRangeConfig() throws Exception { + MyService entity = app.createAndManageChild(EntitySpec.create(MyService.class) + .configure("http.port", "9999+")); + Assert.assertTrue(entity.getRequiredOpenPorts().contains(9999)); + } + @ImplementedBy(MyServiceImpl.class) public interface MyService extends SoftwareProcess { + PortAttributeSensorAndConfigKey HTTP_PORT = Attributes.HTTP_PORT; public SoftwareProcessDriver getDriver(); + public Collection<Integer> getRequiredOpenPorts(); } public static class MyServiceImpl extends SoftwareProcessImpl implements MyService { @@ -425,7 +436,14 @@ public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport { } @Override - public Class<?> getDriverInterface() { return SimulatedDriver.class; } + public Class<?> getDriverInterface() { + return SimulatedDriver.class; + } + + @Override + public Collection<Integer> getRequiredOpenPorts() { + return super.getRequiredOpenPorts(); + } } @ImplementedBy(MyServiceWithVersionImpl.class)
