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/06/01 13:54:54 UTC
[2/3] brooklyn-server git commit: BROOKLYN-259: incorporate review
comments (PR #132)
BROOKLYN-259: incorporate review comments (PR #132)
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a927f667
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a927f667
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a927f667
Branch: refs/heads/master
Commit: a927f667a4e57c5c15f71b673e7dcd31e099c16c
Parents: 9da32f6
Author: Aled Sage <al...@gmail.com>
Authored: Thu May 19 22:12:54 2016 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Fri May 20 00:18:44 2016 +0100
----------------------------------------------------------------------
.../core/location/AbstractLocationResolver.java | 16 +++++++++++---
.../FixedListMachineProvisioningLocation.java | 4 ++--
.../localhost/LocalhostLocationResolver.java | 2 +-
.../jclouds/JcloudsByonLocationResolver.java | 23 ++++++++++----------
.../location/jclouds/JcloudsLocation.java | 16 ++++++++------
.../JcloudsByonLocationResolverAwsLiveTest.java | 21 ++++++++++--------
.../JcloudsByonLocationResolverStubbedTest.java | 4 ++--
7 files changed, 50 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocationResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocationResolver.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocationResolver.java
index 0a836e6..667e535 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocationResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocationResolver.java
@@ -103,12 +103,22 @@ public abstract class AbstractLocationResolver implements LocationResolver {
String namedLocation = (String) locationFlags.get(LocationInternal.NAMED_SPEC_NAME.getName());
// prefer args map over location flags
- Map<String, Object> filteredProperties = getFilteredLocationProperties(getPrefix(), namedLocation, globalProperties);
- ConfigBag flags = ConfigBag.newInstance(parsedSpec.argsMap).putIfAbsent(locationFlags).putIfAbsent(filteredProperties);
+ ConfigBag flags = ConfigBag.newInstance(parsedSpec.argsMap).putIfAbsent(locationFlags);
+
+ Map<String, Object> filteredProperties = getFilteredLocationProperties(getPrefix(), namedLocation, flags.getAllConfig(), globalProperties);
+ flags.putIfAbsent(filteredProperties);
return flags;
}
-
+
+ protected Map<String, Object> getFilteredLocationProperties(String provider, String namedLocation, Map<String, ?> prioritisedProperties, Map<String, ?> globalProperties) {
+ return getFilteredLocationProperties(provider, namedLocation, globalProperties);
+ }
+
+ /**
+ * @deprecated since 0.10.0; use {@link #getFilteredLocationProperties(String, String, Map, Map)}
+ */
+ @Deprecated
protected Map<String, Object> getFilteredLocationProperties(String provider, String namedLocation, Map<String, ?> globalProperties) {
return new LocationPropertiesFromBrooklynProperties().getLocationProperties(getPrefix(), namedLocation, globalProperties);
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java
index 15db018..72d462f 100644
--- a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java
+++ b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java
@@ -107,7 +107,7 @@ implements MachineProvisioningLocation<T>, Closeable {
public static final ConfigKey<List<LocationSpec<? extends MachineLocation>>> MACHINE_SPECS = ConfigKeys.newConfigKey(
new TypeToken<List<LocationSpec<? extends MachineLocation>>>() {},
"byon.machineSpecs",
- "Specs of machines that should be immediatly instantiated on init",
+ "Specs of machines that should be immediately instantiated on init",
ImmutableList.<LocationSpec<? extends MachineLocation>>of());
/**
@@ -126,7 +126,7 @@ implements MachineProvisioningLocation<T>, Closeable {
public static final ConfigKey<Supplier<? extends List<? extends MachineLocation>>> INITIAL_MACHINES_FACTORY = ConfigKeys.newConfigKey(
new TypeToken<Supplier<? extends List<? extends MachineLocation>>>() {},
"byon.initialMachinesFactory",
- "Factory for creating the machines that should be immediatly instantiated on init",
+ "Factory for creating the machines that should be immediately instantiated on init",
null);
private final Object lock = new Object();
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java
index 65ed85c..b26a398 100644
--- a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java
@@ -70,7 +70,7 @@ public class LocalhostLocationResolver extends AbstractLocationResolver {
}
@Override
- protected Map<String, Object> getFilteredLocationProperties(String provider, String namedLocation, Map<String, ?> globalProperties) {
+ protected Map<String, Object> getFilteredLocationProperties(String provider, String namedLocation, Map<String, ?> prioritisedProperties, Map<String, ?> globalProperties) {
return new LocalhostPropertiesFromBrooklynProperties().getLocationProperties("localhost", namedLocation, globalProperties);
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java
index d832a04..04c1126 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java
@@ -55,7 +55,7 @@ import com.google.common.collect.Lists;
*
* @author aled
*/
-@SuppressWarnings({"unchecked","rawtypes"})
+@SuppressWarnings({"unchecked"})
public class JcloudsByonLocationResolver extends AbstractLocationResolver implements LocationResolver {
public static final Logger log = LoggerFactory.getLogger(JcloudsByonLocationResolver.class);
@@ -81,23 +81,26 @@ public class JcloudsByonLocationResolver extends AbstractLocationResolver implem
protected SpecParser getSpecParser() {
return new AbstractLocationResolver.SpecParser(getPrefix()).setExampleUsage("\"jcloudsByon(provider='aws-ec2',region='us-east-1',hosts='i-12345678,i-90123456')\"");
}
-
+
+ @Override
+ protected Map<String, Object> getFilteredLocationProperties(String provider, String namedLocation, Map<String, ?> prioritisedProperties, Map<String, ?> globalProperties) {
+ String providerOrApi = (String) prioritisedProperties.get("provider");
+ String regionName = (String) prioritisedProperties.get("region");
+ return new JcloudsPropertiesFromBrooklynProperties().getJcloudsProperties(providerOrApi, regionName, namedLocation, globalProperties);
+ }
+
@Override
protected ConfigBag extractConfig(Map<?,?> locationFlags, String spec, final LocationRegistry registry) {
ConfigBag config = super.extractConfig(locationFlags, spec, registry);
-
+
String providerOrApi = (String) config.getStringKey("provider");
String regionName = (String) config.getStringKey("region");
String endpoint = (String) config.getStringKey("endpoint");
- String namedLocation = (String) config.get(LocationInternal.NAMED_SPEC_NAME);
config.remove(LocationInternal.NAMED_SPEC_NAME.getName());
Object hosts = config.getStringKey("hosts");
config.remove("hosts");
- Map jcloudsProperties = new JcloudsPropertiesFromBrooklynProperties().getJcloudsProperties(providerOrApi, regionName, namedLocation, config.getAllConfig());
- config.putIfAbsent(jcloudsProperties);
-
if (Strings.isEmpty(providerOrApi)) {
throw new IllegalArgumentException("Invalid location '"+spec+"'; provider must be defined");
}
@@ -107,7 +110,7 @@ public class JcloudsByonLocationResolver extends AbstractLocationResolver implem
final String jcloudsSpec = "jclouds:"+providerOrApi + (regionName != null ? ":"+regionName : "") + (endpoint != null ? ":"+endpoint : "");
final Maybe<LocationSpec<? extends Location>> jcloudsLocationSpec = registry.getLocationSpec(jcloudsSpec, config.getAllConfig());
- if (jcloudsLocationSpec.isAbsent()) {
+ if (jcloudsLocationSpec.isAbsentOrNull()) {
throw new IllegalArgumentException("Invalid location '"+spec+"'; referrenced jclouds spec '"+jcloudsSpec+"' cannot be resolved");
} else if (!JcloudsLocation.class.isAssignableFrom(jcloudsLocationSpec.get().getType())) {
throw new IllegalArgumentException("Invalid location '"+spec+"'; referrenced spec '"+jcloudsSpec+"' of type "+jcloudsLocationSpec.get().getType()+" rather than JcloudsLocation");
@@ -147,8 +150,6 @@ public class JcloudsByonLocationResolver extends AbstractLocationResolver implem
Supplier<List<JcloudsMachineLocation>> machinesFactory = new Supplier<List<JcloudsMachineLocation>>() {
@Override
public List<JcloudsMachineLocation> get() {
- // FIXME Can't change the parent of the JcloudsMachineLocation; but the FixedListMachineLocation will expect it to be the parent?
- // We'll get errors on rebind. Need to reproduce/investigate.
List<JcloudsMachineLocation> result = Lists.newArrayList();
JcloudsLocation jcloudsLocation = (JcloudsLocation) managementContext.getLocationManager().createLocation(jcloudsLocationSpec.get());
for (Map<?,?> machineFlags : machinesFlags) {
@@ -167,8 +168,6 @@ public class JcloudsByonLocationResolver extends AbstractLocationResolver implem
}
};
- ConfigBag flags = ConfigBag.newInstance(jcloudsProperties);
-
config.put(FixedListMachineProvisioningLocation.INITIAL_MACHINES_FACTORY, machinesFactory);
return config;
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
index d55589a..e30bc0d 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
@@ -1701,11 +1701,12 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
sshProps.put("port", hostAndPort.getPort());
sshProps.put(AbstractLocation.TEMPORARY_LOCATION.getName(), true);
sshProps.put(LocalLocationManager.CREATE_UNMANAGED.getName(), true);
+ sshProps.remove("id");
sshProps.remove("password");
sshProps.remove("privateKeyData");
sshProps.remove("privateKeyFile");
sshProps.remove("privateKeyPassphrase");
-
+
if (initialPassword.isPresent()) sshProps.put("password", initialPassword.get());
if (initialPrivateKey.isPresent()) sshProps.put("privateKeyData", initialPrivateKey.get());
@@ -2166,15 +2167,16 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im
OsCredential osCredentials = LocationConfigUtils.getOsCredential(config).checkNoErrors().logAnyWarnings();
String pkd = osCredentials.getPrivateKeyData();
String password = osCredentials.getPassword();
+ LoginCredentials expectedCredentials = node.getCredentials();
if (Strings.isNonBlank(pkd)) {
- LoginCredentials expectedCredentials = LoginCredentials.fromCredentials(new Credentials(user, pkd));
- //override credentials
- node = NodeMetadataBuilder.fromNodeMetadata(node).credentials(expectedCredentials).build();
+ expectedCredentials = LoginCredentials.fromCredentials(new Credentials(user, pkd));
} else if (Strings.isNonBlank(password)) {
- LoginCredentials expectedCredentials = LoginCredentials.fromCredentials(new Credentials(user, password));
- //override credentials
- node = NodeMetadataBuilder.fromNodeMetadata(node).credentials(expectedCredentials).build();
+ expectedCredentials = LoginCredentials.fromCredentials(new Credentials(user, password));
+ } else if (expectedCredentials == null) {
+ //need some kind of credential object, or will get NPE later
+ expectedCredentials = LoginCredentials.fromCredentials(new Credentials(user, null));
}
+ node = NodeMetadataBuilder.fromNodeMetadata(node).credentials(expectedCredentials).build();
return node;
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverAwsLiveTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverAwsLiveTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverAwsLiveTest.java
index 2285e79..881e363 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverAwsLiveTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverAwsLiveTest.java
@@ -25,12 +25,12 @@ import java.net.InetAddress;
import java.util.Map;
import java.util.Set;
-import org.testng.annotations.AfterClass;
-import org.testng.annotations.BeforeClass;
-import org.testng.annotations.Test;
import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
import org.apache.brooklyn.util.collections.MutableMap;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -56,6 +56,7 @@ public class JcloudsByonLocationResolverAwsLiveTest extends AbstractJcloudsLiveT
classEc2Vm = (JcloudsSshMachineLocation)classEc2Loc.obtain(MutableMap.<String,Object>builder()
.put("hardwareId", AWS_EC2_SMALL_HARDWARE_ID)
.put("inboundPorts", ImmutableList.of(22))
+ .put("user", "myuser")
.build());
awsVmUser = classEc2Vm.getUser();
awsVmInstanceId = classEc2Vm.getNode().getProviderId(); // id without region (e.g. "i-6ff96d2f" instead of "eu-west-1/i-6ff96d2f")
@@ -147,16 +148,19 @@ public class JcloudsByonLocationResolverAwsLiveTest extends AbstractJcloudsLiveT
// prefer location-generic if nothing else
brooklynProperties.put("brooklyn.location.keyPair", "keyPair-inLocationGeneric");
- // prefer deprecated properties in "named" over those less specific
- brooklynProperties.put("brooklyn.location.named.mynamed.private-key-data", "privateKeyData-inNamed");
- brooklynProperties.put("brooklyn.jclouds.aws-ec2.privateKeyData", "privateKeyData-inProviderSpecific");
- brooklynProperties.put("brooklyn.jclouds.privateKeyData", "privateKeyData-inJcloudsGeneric");
-
// prefer "named" over everything else: confirm deprecated don't get transformed to overwrite it accidentally
brooklynProperties.put("brooklyn.location.named.mynamed.privateKeyPassphrase", "privateKeyPassphrase-inNamed");
brooklynProperties.put("brooklyn.jclouds.aws-ec2.private-key-passphrase", "privateKeyPassphrase-inProviderSpecific");
brooklynProperties.put("brooklyn.jclouds.private-key-passphrase", "privateKeyPassphrase-inJcloudsGeneric");
+ // Things get strange when we mess with the private/public key config; in JcloudsLocation.findNodeOrThrow
+ // it validates the LocationConfigUtils.getOsCredential, and in createJcloudsSshMachineLocation it
+ // only includes things like privateKeyData if it was actually successfully read. If we supply values
+ // that are invalid then we can end up with null at the end. But if we miss out the value entirely then
+ // we can can exceptions telling us it was mis-configured! It seems simplest to include this privateKeyData
+ // config here so that "it works", but to not assert that the machine ends up with such an invalid key value!
+ brooklynProperties.put("brooklyn.location.named.mynamed.privateKeyData", "privateKeyData-inNamed");
+
Map<String, Object> conf = resolve("named:mynamed").obtain().config().getBag().getAllConfig();
assertEquals(conf.get("user"), awsVmUser);
@@ -166,7 +170,6 @@ public class JcloudsByonLocationResolverAwsLiveTest extends AbstractJcloudsLiveT
assertEquals(conf.get("loginUser"), "loginUser-inJcloudsGeneric");
assertEquals(conf.get("imageId"), "imageId-inJcloudsGenericDeprecated");
assertEquals(conf.get("keyPair"), "keyPair-inLocationGeneric");
- assertEquals(conf.get("privateKeyData"), "privateKeyData-inNamed");
assertEquals(conf.get("privateKeyPassphrase"), "privateKeyPassphrase-inNamed");
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a927f667/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedTest.java
index 86a66f7..fc41d70 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedTest.java
@@ -101,8 +101,8 @@ public class JcloudsByonLocationResolverStubbedTest extends AbstractJcloudsStubb
assertEquals(machine.getPrivateAddresses(), ImmutableSet.of(nodePrivateAddress));
// TODO what user/password should we expect? Fails below because has "dummy":
-// assertEquals(machine.getUser(), "myuser");
-// assertEquals(machine.config().get(JcloudsLocationConfig.PASSWORD), "mypassword");
+ assertEquals(machine.getUser(), "myuser");
+ assertEquals(machine.config().get(JcloudsLocationConfig.PASSWORD), "mypassword");
}
@Test