Skip to content

Fix JcloudsLocation config usage #710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asfgit merged 5 commits into from
May 31, 2017
Merged

Fix JcloudsLocation config usage #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b5f748
Fix JcloudsLocation.registerMachine(ConfigBag)
aledsage May 30, 2017
b987fb1
JcloudsLocation: call unmanage after createTemporarySshMachineLocation
aledsage May 30, 2017
3f06cad
JcloudsLocation: use merged config for WAIT_WINDOWS_TO_START
aledsage May 30, 2017
4105fd4
JcloudsLocation.createTemporarySshMachineLocation: sshToolClass not s…
aledsage May 30, 2017
49543d0
Improve javadoc of JcloudsLocation.registerMachine
aledsage May 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...louds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;

/**
Expand Down Expand Up @@ -155,7 +156,7 @@ public List<JcloudsMachineLocation> get() {
for (Map<?,?> machineFlags : machinesFlags) {
try {
jcloudsLocation.config().putAll(machineFlags);
JcloudsMachineLocation machine = jcloudsLocation.registerMachine(jcloudsLocation.config().getBag());
JcloudsMachineLocation machine = jcloudsLocation.registerMachine(ImmutableMap.of());
result.add(machine);
} catch (NoMachinesAvailableException e) {
Map<?,?> sanitizedMachineFlags = Sanitizer.sanitize(machineFlags);
Expand Down
53 changes: 32 additions & 21 deletions locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1611,10 +1611,6 @@ protected SshMachineLocation createTemporarySshMachineLocation(HostAndPort hostA
sshProps.put("port", hostAndPort.getPort());
sshProps.put(AbstractLocation.TEMPORARY_LOCATION.getName(), true);
sshProps.put(LocalLocationManager.CREATE_UNMANAGED.getName(), true);
String sshClass = config().get(SshMachineLocation.SSH_TOOL_CLASS);
if (Strings.isNonBlank(sshClass)) {
sshProps.put(SshMachineLocation.SSH_TOOL_CLASS.getName(), sshClass);
}

sshProps.remove("id");
sshProps.remove("password");
Expand Down Expand Up @@ -1727,7 +1723,9 @@ protected LoginCredentials createUser(
LOG.warn("exit code {} when creating user for {}; usage may subsequently fail", exitcode, node);
}
} finally {
getManagementContext().getLocationManager().unmanage(sshLoc);
if (getManagementContext().getLocationManager().isManaged(sshLoc)) {
getManagementContext().getLocationManager().unmanage(sshLoc);
}
Streams.closeQuietly(sshLoc);
}
}
Expand Down Expand Up @@ -1772,11 +1770,11 @@ protected UserCreation createUserStatements(@Nullable Image image, ConfigBag con

// ----------------- registering existing machines ------------------------

protected MachineLocation registerMachine(NodeMetadata metadata) throws NoMachinesAvailableException {
protected JcloudsMachineLocation registerMachine(NodeMetadata metadata) throws NoMachinesAvailableException {
return registerMachine(MutableMap.of(), metadata);
}

protected MachineLocation registerMachine(Map<?, ?> flags, NodeMetadata metadata) throws NoMachinesAvailableException {
protected JcloudsMachineLocation registerMachine(Map<?, ?> flags, NodeMetadata metadata) throws NoMachinesAvailableException {
ConfigBag setup = ConfigBag.newInstanceExtending(config().getBag(), flags);
if (!setup.containsKey("id")) setup.putStringKey("id", metadata.getId());
setHostnameUpdatingCredentials(setup, metadata);
Expand All @@ -1786,14 +1784,27 @@ protected MachineLocation registerMachine(Map<?, ?> flags, NodeMetadata metadata
/**
* Brings an existing machine with the given details under management.
* <p>
* Required fields are:
* <ul>
* <li>id: the jclouds VM id, e.g. "eu-west-1/i-5504f21d" (NB this is {@see JcloudsMachineLocation#getJcloudsId()} not #getId())
* <li>hostname: the public hostname or IP of the machine, e.g. "ec2-176-34-93-58.eu-west-1.compute.amazonaws.com"
* <li>userName: the username for sshing into the machine (for use if it is not a Windows system)
* The args passed in are used to match against an existing machine. The machines are listed
* (see @link #listMachines()}), and each is compared against the given args. There should
* be exactly one matching machine.
* <p>
* Arguments that can be used for matching are:
* <ul>
* <li>{@code id}: the cloud provider's VM id, e.g. "eu-west-1/i-5504f21d" (NB this is
* {@see JcloudsMachineLocation#getJcloudsId()} not #getId())
* <li>{@code hostname}: the public hostname or IP of the machine,
* e.g. "ec2-176-34-93-58.eu-west-1.compute.amazonaws.com"
* </ul>
*
* Other config options can also be passed in, for subsequent usage of the machine. For example,
* {@code user} will deterine the username subsequently used for ssh or WinRM. See the standard
* config options of {@link JcloudsLocation}, {@link SshMachineLocation} and
* {@link WinRmMachineLocation}.
*
* @throws IllegalArgumentException if there is not exactly one match
*/
public JcloudsMachineLocation registerMachine(ConfigBag setup) throws NoMachinesAvailableException {
public JcloudsMachineLocation registerMachine(ConfigBag flags) throws NoMachinesAvailableException {
ConfigBag setup = ConfigBag.newInstanceExtending(config().getBag(), flags.getAllConfig());
NodeMetadata node = findNodeOrThrow(setup);
return registerMachineLocation(setup, node);
}
Expand Down Expand Up @@ -1871,9 +1882,8 @@ protected NodeMetadata findNodeOrThrow(ConfigBag config) {
return node;
}

public MachineLocation registerMachine(Map<?,?> flags) throws NoMachinesAvailableException {
ConfigBag setup = ConfigBag.newInstanceExtending(config().getBag(), flags);
return registerMachine(setup);
public JcloudsMachineLocation registerMachine(Map<?,?> flags) throws NoMachinesAvailableException {
return registerMachine(ConfigBag.newInstance(flags));
}

/**
Expand Down Expand Up @@ -2454,7 +2464,7 @@ public Boolean call() {
if (success) {
credsSuccessful.set(machinesToTry.getRight());

String verifyWindowsUp = getConfig(WinRmMachineLocation.WAIT_WINDOWS_TO_START);
String verifyWindowsUp = setup.get(WinRmMachineLocation.WAIT_WINDOWS_TO_START);
if (Strings.isBlank(verifyWindowsUp) || verifyWindowsUp.equals("false")) {
return true;
}
Expand Down Expand Up @@ -2901,11 +2911,9 @@ Maybe<String> getHostnameAws(NodeMetadata node, Optional<HostAndPort> sshHostAnd
}

String getHostnameAws(HostAndPort hostAndPort, LoginCredentials userCredentials, ConfigBag setup) {
SshMachineLocation sshLocByIp = null;
// TODO messy way to get an SSH session
SshMachineLocation sshLocByIp = createTemporarySshMachineLocation(hostAndPort, userCredentials, setup);
try {
// TODO messy way to get an SSH session
sshLocByIp = createTemporarySshMachineLocation(hostAndPort, userCredentials, setup);

ByteArrayOutputStream outStream = new ByteArrayOutputStream();
ByteArrayOutputStream errStream = new ByteArrayOutputStream();
int exitcode = sshLocByIp.execCommands(
Expand All @@ -2921,6 +2929,9 @@ String getHostnameAws(HostAndPort hostAndPort, LoginCredentials userCredentials,
}
throw new IllegalStateException("Could not obtain aws-ec2 hostname for vm "+hostAndPort+"; exitcode="+exitcode+"; stdout="+outString+"; stderr="+new String(errStream.toByteArray()));
} finally {
if (getManagementContext().getLocationManager().isManaged(sshLocByIp)) {
getManagementContext().getLocationManager().unmanage(sshLocByIp);
}
Streams.closeQuietly(sshLocByIp);
}
}
Expand Down
15 changes: 8 additions & 7 deletions ...s/src/test/java/org/apache/brooklyn/location/jclouds/DefaultConnectivityResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ public void testRespectsHostAndPortOverride() throws Exception {
.portForwardSshOverride(HostAndPort.fromParts("10.1.1.4", 4361))
.build();
DefaultConnectivityResolver customizer = new DefaultConnectivityResolver();
ManagementAddressResolveResult result = customizer.resolve(jcloudsLocation, newNodeMetadata(), ConfigBag.newInstance(), options);
ConfigBag configBag = jcloudsLocation.config().getBag();
ManagementAddressResolveResult result = customizer.resolve(jcloudsLocation, newNodeMetadata(), configBag, options);
assertEquals(result.hostAndPort().getHostText(), "10.1.1.4");
assertEquals(result.hostAndPort().getPort(), 4361);
}
Expand All @@ -72,7 +73,7 @@ public void testObtainsHostnameFromAwsMachine() throws Exception {
.userCredentials(credential)
.build();
DefaultConnectivityResolver customizer = new DefaultConnectivityResolver();
ConfigBag configBag = ConfigBag.newInstance();
ConfigBag configBag = jcloudsLocation.config().getBag();
ManagementAddressResolveResult result = customizer.resolve(
jcloudsLocation, newNodeMetadata(), configBag, options);
assertEquals(result.hostAndPort().getHostText(), expectedHostname);
Expand All @@ -91,7 +92,7 @@ public RecordingSshTool.CustomResponse generate(RecordingSshTool.ExecParams exec
});
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
DefaultConnectivityResolver customizer = new DefaultConnectivityResolver();
final ConfigBag config = ConfigBag.newInstance(ImmutableMap.of(
final ConfigBag config = ConfigBag.newInstanceExtending(jcloudsLocation.config().getBag(), ImmutableMap.of(
JcloudsLocationConfig.WAIT_FOR_SSHABLE, "1ms",
JcloudsLocationConfig.POLL_FOR_FIRST_REACHABLE_ADDRESS, "1ms"));
assertTrue(customizer.checkCredential(
Expand All @@ -114,7 +115,7 @@ public RecordingWinRmTool.CustomResponse generate(RecordingWinRmTool.ExecParams
});
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
DefaultConnectivityResolver customizer = new DefaultConnectivityResolver();
final ConfigBag config = ConfigBag.newInstance(ImmutableMap.of(
final ConfigBag config = ConfigBag.newInstanceExtending(jcloudsLocation.config().getBag(), ImmutableMap.of(
JcloudsLocationConfig.WAIT_FOR_WINRM_AVAILABLE, "1ms",
JcloudsLocationConfig.POLL_FOR_FIRST_REACHABLE_ADDRESS, "1ms"));
assertTrue(customizer.checkCredential(
Expand Down Expand Up @@ -146,7 +147,7 @@ public RecordingSshTool.CustomResponse generate(RecordingSshTool.ExecParams exec
return new RecordingSshTool.CustomResponse(exitCode, "", "");
}
});
ConfigBag config = ConfigBag.newInstance(ImmutableMap.of(
ConfigBag config = ConfigBag.newInstanceExtending(jcloudsLocation.config().getBag(), ImmutableMap.of(
JcloudsLocationConfig.LOOKUP_AWS_HOSTNAME, false,
JcloudsLocationConfig.WAIT_FOR_SSHABLE, "1ms",
JcloudsLocationConfig.POLL_FOR_FIRST_REACHABLE_ADDRESS, "1ms",
Expand Down Expand Up @@ -194,7 +195,7 @@ public void testMode(NetworkMode mode, Set<HostAndPort> reachableIps, String exp
JcloudsLocationConfig.CONNECTIVITY_RESOLVER, customizer));

ConnectivityResolverOptions options = newResolveOptionsForIps(reachableIps, Duration.millis(100)).build();
ConfigBag configBag = ConfigBag.newInstance();
ConfigBag configBag = jcloudsLocation.config().getBag();

ManagementAddressResolveResult result = customizer.resolve(jcloudsLocation, newNodeMetadata(), configBag, options);
assertEquals(result.hostAndPort().getHostText(), expectedIp);
Expand Down Expand Up @@ -223,7 +224,7 @@ public void testModeUnavailable(NetworkMode mode, Set<HostAndPort> reachableIps)
JcloudsLocationConfig.CONNECTIVITY_RESOLVER, customizer));

ConnectivityResolverOptions options = newResolveOptionsForIps(reachableIps, Duration.ONE_MILLISECOND).build();
ConfigBag configBag = ConfigBag.newInstance();
ConfigBag configBag = jcloudsLocation.config().getBag();
customizer.resolve(jcloudsLocation, newNodeMetadata(), configBag, options);
}

Expand Down