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 2014/11/28 19:23:28 UTC
[2/5] incubator-brooklyn git commit: PortForwardManager tests and
fixes:
PortForwardManager tests and fixes:
- Add tests for PortForwardManager
- Deprecate isClient()
- AcquirePort uses the first port in range as well (i.e. use
`getAndIncrement()` rather than `incrementAndGet()`)
- Fix forgetPortMappings(Location)
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/4a415043
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/4a415043
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/4a415043
Branch: refs/heads/master
Commit: 4a415043228820d49ed6fa42692bfa76117264d2
Parents: 804c9eb
Author: Aled Sage <al...@gmail.com>
Authored: Fri Nov 28 11:49:57 2014 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Fri Nov 28 11:49:57 2014 +0000
----------------------------------------------------------------------
.../location/access/PortForwardManager.java | 15 +-
.../access/PortForwardManagerClient.java | 26 ++-
.../location/access/PortForwardManagerImpl.java | 4 +-
.../access/PortForwardManagerRebindTest.java | 39 +++--
.../location/access/PortForwardManagerTest.java | 157 +++++++++++++++++++
5 files changed, 216 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4a415043/core/src/main/java/brooklyn/location/access/PortForwardManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/access/PortForwardManager.java b/core/src/main/java/brooklyn/location/access/PortForwardManager.java
index 7a20157..40f76c5 100644
--- a/core/src/main/java/brooklyn/location/access/PortForwardManager.java
+++ b/core/src/main/java/brooklyn/location/access/PortForwardManager.java
@@ -120,12 +120,6 @@ public interface PortForwardManager extends Location {
*/
public boolean forgetPortMappings(Location location);
- /**
- * Returns true if this implementation is a client which is immutable/safe for serialization
- * i.e. it delegates to something on an entity or location elsewhere.
- */
- public boolean isClient();
-
public String toVerboseString();
@@ -191,6 +185,15 @@ public interface PortForwardManager extends Location {
@Deprecated
public boolean forgetPublicIpHostname(String publicIpId);
+ /**
+ * Returns true if this implementation is a client which is immutable/safe for serialization
+ * i.e. it delegates to something on an entity or location elsewhere.
+ *
+ * @deprecated since 0.7.0; no need to separate client-proxy from impl
+ */
+ @Deprecated
+ public boolean isClient();
+
///////////////////////////////////////////////////////////////////////////////////
// Deprecated; just internal
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4a415043/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java b/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java
index f651a9c..f3767fd 100644
--- a/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java
+++ b/core/src/main/java/brooklyn/location/access/PortForwardManagerClient.java
@@ -35,8 +35,11 @@ import com.google.common.net.HostAndPort;
/**
* @deprecated since 0.7.0; just use the {@link PortForwardManager}, or a direct reference to its impl {@link PortForwardManagerImpl}
*/
+@Deprecated
public class PortForwardManagerClient implements PortForwardManager {
+ private static final long serialVersionUID = -295204304305332895L;
+
protected final Supplier<PortForwardManager> delegateSupplier;
private transient volatile PortForwardManager _delegate;
@@ -141,11 +144,6 @@ public class PortForwardManagerClient implements PortForwardManager {
}
@Override
- public boolean isClient() {
- return true;
- }
-
- @Override
public String toVerboseString() {
return getClass().getName()+"[wrapping="+getDelegate().toVerboseString()+"]";
}
@@ -163,6 +161,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; use {@link #acquirePublicPort(String)}, and then use {@link #associate(String, HostAndPort, int)} or {@link #associate(String, HostAndPort, Location, int)}
*/
+ @Override
@Deprecated
public int acquirePublicPort(String publicIpId, Location l, int privatePort) {
return getDelegate().acquirePublicPort(publicIpId, l, privatePort);
@@ -173,6 +172,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; use {@link #associate(String, HostAndPort, int)} or {@link #associate(String, HostAndPort, Location, int)}
*/
+ @Override
@Deprecated
public PortMapping acquirePublicPortExplicit(String publicIpId, int publicPort) {
return getDelegate().acquirePublicPortExplicit(publicIpId, publicPort);
@@ -188,6 +188,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated Use {@link #associate(String, HostAndPort, Location, int)}
*/
+ @Override
@Deprecated
public void associate(String publicIpId, int publicPort, Location l, int privatePort) {
getDelegate().associate(publicIpId, publicPort, l, privatePort);
@@ -200,6 +201,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated Use {@link #associate(String, HostAndPort, int)} or {@link #associate(String, HostAndPort, Location, int)}
*/
+ @Override
@Deprecated
public void recordPublicIpHostname(String publicIpId, String hostnameOrPublicIpAddress) {
getDelegate().recordPublicIpHostname(publicIpId, hostnameOrPublicIpAddress);
@@ -210,6 +212,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated Use {@link #lookup(String, int)} or {@link #lookup(Location, int)}
*/
+ @Override
@Deprecated
public String getPublicIpHostname(String publicIpId) {
return getDelegate().getPublicIpHostname(publicIpId);
@@ -220,11 +223,18 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated Use {@link #forgetPortMapping(String, int)} or {@link #forgetPortMapping(Location, int)}
*/
+ @Override
@Deprecated
public boolean forgetPublicIpHostname(String publicIpId) {
return getDelegate().forgetPublicIpHostname(publicIpId);
}
+ @Override
+ @Deprecated
+ public boolean isClient() {
+ return true;
+ }
+
///////////////////////////////////////////////////////////////////////////////////
// Deprecated; just internal
@@ -235,6 +245,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; this method will be internal only
*/
+ @Override
@Deprecated
public PortMapping getPortMappingWithPublicSide(String publicIpId, int publicPort) {
return getDelegate().getPortMappingWithPublicSide(publicIpId, publicPort);
@@ -245,6 +256,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; this method will be internal only
*/
+ @Override
@Deprecated
public Collection<PortMapping> getPortMappingWithPublicIpId(String publicIpId) {
return getDelegate().getPortMappingWithPublicIpId(publicIpId);
@@ -255,6 +267,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; this method will be internal only
*/
+ @Override
@Deprecated
public boolean forgetPortMapping(PortMapping m) {
return getDelegate().forgetPortMapping(m);
@@ -267,6 +280,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; this method will be internal only
*/
+ @Override
@Deprecated
public HostAndPort getPublicHostAndPort(PortMapping m) {
return getDelegate().getPublicHostAndPort(m);
@@ -277,6 +291,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; this method will be internal only
*/
+ @Override
@Deprecated
public Collection<PortMapping> getLocationPublicIpIds(Location l) {
return getDelegate().getLocationPublicIpIds(l);
@@ -287,6 +302,7 @@ public class PortForwardManagerClient implements PortForwardManager {
*
* @deprecated since 0.7.0; this method will be internal only
*/
+ @Override
@Deprecated
public PortMapping getPortMappingWithPrivateSide(Location l, int privatePort) {
return getDelegate().getPortMappingWithPrivateSide(l, privatePort);
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4a415043/core/src/main/java/brooklyn/location/access/PortForwardManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/access/PortForwardManagerImpl.java b/core/src/main/java/brooklyn/location/access/PortForwardManagerImpl.java
index 759eccb..5b8c072 100644
--- a/core/src/main/java/brooklyn/location/access/PortForwardManagerImpl.java
+++ b/core/src/main/java/brooklyn/location/access/PortForwardManagerImpl.java
@@ -155,7 +155,7 @@ public class PortForwardManagerImpl extends AbstractLocation implements PortForw
protected int getNextPort() {
// far too simple -- see javadoc above
- return portReserved.incrementAndGet();
+ return portReserved.getAndIncrement();
}
@Override
@@ -215,7 +215,7 @@ public class PortForwardManagerImpl extends AbstractLocation implements PortForw
public boolean forgetPortMappings(Location l) {
List<PortMapping> result = Lists.newArrayList();
synchronized (this) {
- for (Iterator<PortMapping> iter = result.iterator(); iter.hasNext();) {
+ for (Iterator<PortMapping> iter = mappings.values().iterator(); iter.hasNext();) {
PortMapping m = iter.next();
if (l.equals(m.target)) {
iter.remove();
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4a415043/core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java b/core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java
index 3c5cb14..5ad0296 100644
--- a/core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java
+++ b/core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java
@@ -19,8 +19,7 @@
package brooklyn.location.access;
import static org.testng.Assert.assertEquals;
-
-import java.util.Map;
+import static org.testng.Assert.assertNotEquals;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -39,7 +38,6 @@ import brooklyn.location.LocationSpec;
import brooklyn.location.basic.SshMachineLocation;
import brooklyn.test.entity.TestEntity;
import brooklyn.test.entity.TestEntityImpl;
-import brooklyn.util.flags.SetFromFlag;
import brooklyn.util.net.Networking;
import com.google.common.base.Predicates;
@@ -50,11 +48,9 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
private static final Logger log = LoggerFactory.getLogger(PortForwardManagerRebindTest.class);
- private Map<HostAndPort, HostAndPort> portMapping;
private String machineAddress = "1.2.3.4";
private SshMachineLocation origSimulatedMachine;
-
@Override
@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
@@ -67,7 +63,7 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
}
@Test
- public void testHostAndPortTransformingEnricher() throws Exception {
+ public void testAssociationPreservedOnRebind() throws Exception {
String publicIpId = "5.6.7.8";
String publicAddress = "5.6.7.8";
@@ -80,6 +76,7 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
newApp = rebind();
+ // After rebind, confirm that lookups still work
TestEntity newEntity = (TestEntity) Iterables.find(newApp.getChildren(), Predicates.instanceOf(TestEntity.class));
Location newSimulatedMachine = newApp.getManagementContext().getLocationManager().getLocation(origSimulatedMachine.getId());
PortForwardManager newPortForwardManager = newEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
@@ -89,7 +86,7 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
}
@Test
- public void testHostAndPortTransformingEnricherLegacy() throws Exception {
+ public void testAssociationPreservedOnRebindLegacy() throws Exception {
String publicIpId = "5.6.7.8";
String publicAddress = "5.6.7.8";
@@ -104,6 +101,7 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
newApp = rebind();
+ // After rebind, confirm that lookups still work
TestEntity newEntity = (TestEntity) Iterables.find(newApp.getChildren(), Predicates.instanceOf(TestEntity.class));
Location newSimulatedMachine = newApp.getManagementContext().getLocationManager().getLocation(origSimulatedMachine.getId());
PortForwardManager newPortForwardManager = newEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
@@ -113,6 +111,27 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
assertEquals(newPortForwardManager.lookup(publicIpId, 80), HostAndPort.fromParts(publicAddress, 40080));
}
+ @Test
+ public void testAcquirePortCounterPreservedOnRebindLegacy() throws Exception {
+ String publicIpId = "5.6.7.8";
+
+ TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class).impl(MyEntity.class));
+ PortForwardManager origPortForwardManager = origEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
+
+ // We first wait for persisted, to ensure that it is the PortForwardManager.onChanged that is causing persistence.
+ RebindTestUtils.waitForPersisted(origApp);
+ int acquiredPort = origPortForwardManager.acquirePublicPort(publicIpId);
+
+ newApp = rebind();
+
+ // After rebind, confirm that lookups still work
+ TestEntity newEntity = (TestEntity) Iterables.find(newApp.getChildren(), Predicates.instanceOf(TestEntity.class));
+ PortForwardManager newPortForwardManager = newEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
+
+ int acquiredPort2 = newPortForwardManager.acquirePublicPort(publicIpId);
+ assertNotEquals(acquiredPort, acquiredPort2);
+ }
+
public static class MyEntity extends TestEntityImpl {
public static final ConfigKey<PortForwardManager> PORT_FORWARD_MANAGER = ConfigKeys.newConfigKey(PortForwardManager.class, "myentity.portForwardManager");
public static final AttributeSensor<PortForwardManager> PORT_FORWARD_MANAGER_LIVE = Sensors.newSensor(PortForwardManager.class, "myentity.portForwardManager.live");
@@ -124,12 +143,8 @@ public class PortForwardManagerRebindTest extends RebindTestFixtureWithApp {
if (getConfig(PORT_FORWARD_MANAGER) == null) {
PortForwardManager pfm = (PortForwardManager) getManagementContext().getLocationRegistry().resolve("portForwardManager(scope=global)");
setAttribute(PORT_FORWARD_MANAGER_LIVE, pfm);
- setConfig(PORT_FORWARD_MANAGER, PortForwardManagerClient.fromAttributeOnEntity(this, PORT_FORWARD_MANAGER_LIVE));
+ setConfig(PORT_FORWARD_MANAGER, pfm);
}
}
-
- @SetFromFlag("myconfigflagname")
- public static final ConfigKey<String> MY_CONFIG_WITH_FLAGNAME = ConfigKeys.newStringConfigKey("myentity.myconfigwithflagname");
}
-
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/4a415043/core/src/test/java/brooklyn/location/access/PortForwardManagerTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/location/access/PortForwardManagerTest.java b/core/src/test/java/brooklyn/location/access/PortForwardManagerTest.java
new file mode 100644
index 0000000..62c4f65
--- /dev/null
+++ b/core/src/test/java/brooklyn/location/access/PortForwardManagerTest.java
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.location.access;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertNull;
+
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.config.BrooklynProperties;
+import brooklyn.entity.BrooklynAppUnitTestSupport;
+import brooklyn.entity.basic.Entities;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.basic.SshMachineLocation;
+import brooklyn.test.entity.LocalManagementContextForTests;
+import brooklyn.util.net.Networking;
+
+import com.google.common.net.HostAndPort;
+
+public class PortForwardManagerTest extends BrooklynAppUnitTestSupport {
+
+ private static final Logger log = LoggerFactory.getLogger(PortForwardManagerTest.class);
+
+ private Map<HostAndPort, HostAndPort> portMapping;
+ private SshMachineLocation machine1;
+ private SshMachineLocation machine2;
+ private PortForwardManager pfm;
+
+ @Override
+ @BeforeMethod(alwaysRun=true)
+ public void setUp() throws Exception {
+ super.setUp();
+
+ pfm = (PortForwardManager) mgmt.getLocationRegistry().resolve("portForwardManager(scope=global)");
+
+ machine1 = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .configure("address", Networking.getInetAddressWithFixedName("1.2.3.4"))
+ .configure("port", 1234)
+ .configure("user", "myuser"));
+ machine2 = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .configure("address", Networking.getInetAddressWithFixedName("1.2.3.5"))
+ .configure("port", 1234)
+ .configure("user", "myuser"));
+ }
+
+ @Test
+ public void testAssociateWithLocation() throws Exception {
+ String publicIpId = "myipid";
+ String publicAddress = "5.6.7.8";
+
+ pfm.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), machine1, 80);
+
+ assertEquals(pfm.lookup(machine1, 80), HostAndPort.fromParts(publicAddress, 40080));
+ assertEquals(pfm.lookup(publicIpId, 80), HostAndPort.fromParts(publicAddress, 40080));
+ }
+
+ @Test
+ public void testAssociateWithoutLocation() throws Exception {
+ String publicIpId = "myipid";
+ String publicAddress = "5.6.7.8";
+
+ pfm.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), 80);
+
+ assertEquals(pfm.lookup(publicIpId, 80), HostAndPort.fromParts(publicAddress, 40080));
+ assertNull(pfm.lookup(machine1, 80));
+ }
+
+ @Test
+ public void testAcquirePortDoesNotReturnDuplicate() throws Exception {
+ String publicIpId = "myipid";
+
+ int port1 = pfm.acquirePublicPort(publicIpId);
+ int port2 = pfm.acquirePublicPort(publicIpId);
+ assertNotEquals(port1, port2);
+ }
+
+ @Test
+ public void testAcquirePortRespectsStartingPortNumber() throws Exception {
+ BrooklynProperties props = BrooklynProperties.Factory.newEmpty();
+ props.put(PortForwardManager.PORT_FORWARD_MANAGER_STARTING_PORT, 1234);
+ LocalManagementContextForTests mgmt2 = new LocalManagementContextForTests(props);
+ try {
+ PortForwardManager pfm2 = (PortForwardManager) mgmt2.getLocationRegistry().resolve("portForwardManager(scope=global)");
+ int port = pfm2.acquirePublicPort("myipid");
+ assertEquals(port, 1234);
+ } finally {
+ Entities.destroyAll(mgmt2);
+ }
+ }
+
+ @Test
+ public void testForgetPortMapping() throws Exception {
+ String publicIpId = "myipid";
+ String publicAddress = "5.6.7.8";
+
+ pfm.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), machine1, 80);
+ pfm.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40081), machine1, 81);
+ pfm.forgetPortMapping(publicIpId, 40080);
+
+ assertNull(pfm.lookup(publicIpId, 80));
+ assertNull(pfm.lookup(machine1, 80));
+ assertEquals(pfm.lookup(publicIpId, 81), HostAndPort.fromParts(publicAddress, 40081));
+ assertEquals(pfm.lookup(publicIpId, 81), HostAndPort.fromParts(publicAddress, 40081));
+ }
+
+ @Test
+ public void testForgetPortMappingsOfMachine() throws Exception {
+ String publicIpId = "myipid";
+ String publicIpId2 = "myipid2";
+ String publicAddress = "5.6.7.8";
+
+ pfm.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), machine1, 80);
+ pfm.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40081), machine1, 81);
+ pfm.associate(publicIpId2, HostAndPort.fromParts(publicAddress, 40082), machine2, 80);
+ pfm.forgetPortMappings(machine1);
+
+ assertNull(pfm.lookup(machine1, 80));
+ assertNull(pfm.lookup(machine1, 81));
+ assertNull(pfm.lookup(publicIpId, 80));
+ assertEquals(pfm.lookup(machine2, 80), HostAndPort.fromParts(publicAddress, 40082));
+ }
+
+ @Test
+ public void testAssociateLegacy() throws Exception {
+ String publicIpId = "myipid";
+ String publicAddress = "5.6.7.8";
+
+ pfm.acquirePublicPortExplicit(publicIpId, 40080);
+ pfm.recordPublicIpHostname(publicIpId, publicAddress);
+ pfm.associate(publicIpId, 40080, machine1, 80);
+
+ assertEquals(pfm.lookup(publicIpId, 80), HostAndPort.fromParts(publicAddress, 40080));
+ assertEquals(pfm.lookup(machine1, 80), HostAndPort.fromParts(publicAddress, 40080));
+ }
+}