WinRM support: incorporate review comments PR #650 See https://github.com/apache/incubator-brooklyn/pull/650
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/9c37f241 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/9c37f241 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/9c37f241 Branch: refs/heads/master Commit: 9c37f2416b08c8f00123c60ba27f0d0ace10dff1 Parents: 7cd5a25 Author: Aled Sage <[email protected]> Authored: Wed May 27 09:54:12 2015 +0100 Committer: Richard Downer <[email protected]> Committed: Thu May 28 17:40:46 2015 +0100 ---------------------------------------------------------------------- .../entity/group/DynamicClusterImpl.java | 40 +- .../windows/WindowsPerformanceCounterFeed.java | 169 ++------ .../location/basic/WinRmMachineLocation.java | 119 ++++-- .../java/brooklyn/util/flags/TypeCoercions.java | 40 +- .../entity/group/DynamicClusterTest.java | 15 + .../basic/ByonLocationResolverTest.java | 15 +- .../util/internal/TypeCoercionsTest.java | 26 +- .../jclouds/BasicJcloudsLocationCustomizer.java | 30 ++ .../location/jclouds/JcloudsLocation.java | 383 +++++++++---------- .../jclouds/JcloudsLocationCustomizer.java | 26 ++ .../jclouds/JcloudsMachineLocation.java | 45 +++ .../jclouds/JcloudsSshMachineLocation.java | 6 +- .../jclouds/JcloudsWinRmMachineLocation.java | 127 +++++- .../jclouds/SudoTtyFixingCustomizer.java | 8 +- .../JcloudsLocationSecurityGroupCustomizer.java | 8 +- .../location/jclouds/JcloudsLocationTest.java | 4 +- pom.xml | 2 +- .../AbstractSoftwareProcessWinRmDriver.java | 22 +- .../basic/AbstractVanillaProcessDriver.java | 22 -- .../brooklyn/entity/basic/SoftwareProcess.java | 9 + .../entity/basic/SoftwareProcessImpl.java | 8 +- .../basic/VanillaSoftwareProcessDriver.java | 2 +- .../entity/basic/VanillaWindowsProcess.java | 18 +- .../basic/VanillaWindowsProcessDriver.java | 2 +- .../entity/basic/VanillaWindowsProcessImpl.java | 1 + .../brooklyn/entity/software/StaticSensor.java | 10 +- .../winrm/WindowsPerformanceCounterSensors.java | 12 +- ...rWithAvailabilityZonesMultiLocationTest.java | 2 +- .../entity/software/StaticSensorTest.java | 55 +++ .../basic/WinRmMachineLocationLiveTest.java | 91 +++++ .../brooklyn/entity/database/mssql/mssql.yaml | 13 +- .../database/mssql/MssqlBlueprintLiveTest.java | 60 +++ .../launcher/src/test/resources/mssql-test.yaml | 60 +++ 33 files changed, 985 insertions(+), 465 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java index 5c168aa..e269880 100644 --- a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java +++ b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java @@ -438,7 +438,7 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus /** * {@inheritDoc} * - * <strong>Note</strong> for sub-clases; this method can be called while synchronized on {@link #mutex}. + * <strong>Note</strong> for sub-classes; this method can be called while synchronized on {@link #mutex}. */ @Override public String replaceMember(String memberId) { @@ -485,11 +485,10 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus throw new IllegalStateException("Unexpected condition! cluster="+this+"; member="+member+"; actualMemberLocs="+actualMemberLocs); } } else { - if (getMemberSpec() != null && getMemberSpec().getLocations().size() > 0) { - memberLoc = getMemberSpec().getLocations().iterator().next(); - } else { - memberLoc = getLocation(); - } + // Replacing member, so new member should be in the same location as that being replaced. + // Expect this to agree with `getMemberSpec().getLocations()` (if set). If not, then + // presumably there was a reason this specific member was started somewhere else! + memberLoc = getLocation(); } Entity replacement = replaceMember(member, memberLoc, ImmutableMap.of()); @@ -588,22 +587,25 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus // choose locations to be deployed to List<Location> chosenLocations; - chosenLocations = getMemberSpec() == null ? null : getMemberSpec().getLocations(); - if (chosenLocations == null || chosenLocations.size() == 0) { + List<Location> memberLocations = getMemberSpec() == null ? null : getMemberSpec().getLocations(); + if (memberLocations != null && memberLocations.size() > 0) { + // The memberSpec overrides the location passed to cluster.start(); use + // the location defined on the member. if (isAvailabilityZoneEnabled()) { - List<Location> subLocations = getNonFailedSubLocations(); - Multimap<Location, Entity> membersByLocation = getMembersByLocation(); - chosenLocations = getZonePlacementStrategy().locationsForAdditions(membersByLocation, subLocations, delta); - if (chosenLocations.size() != delta) { - throw new IllegalStateException("Node placement strategy chose " + Iterables.size(chosenLocations) - + ", when expected delta " + delta + " in " + this); - } - } else { - chosenLocations = Collections.nCopies(delta, getLocation()); + LOG.warn("Cluster {} has availability-zone enabled, but memberSpec overrides location with {}; using " + + "memberSpec's location; availability-zone behaviour will not apply", this, memberLocations); + } + chosenLocations = Collections.nCopies(delta, memberLocations.get(0)); + } else if (isAvailabilityZoneEnabled()) { + List<Location> subLocations = getNonFailedSubLocations(); + Multimap<Location, Entity> membersByLocation = getMembersByLocation(); + chosenLocations = getZonePlacementStrategy().locationsForAdditions(membersByLocation, subLocations, delta); + if (chosenLocations.size() != delta) { + throw new IllegalStateException("Node placement strategy chose " + Iterables.size(chosenLocations) + + ", when expected delta " + delta + " in " + this); } } else { - // FIXME: Tidy this up! - chosenLocations = Collections.nCopies(delta, chosenLocations.get(0)); + chosenLocations = Collections.nCopies(delta, getLocation()); } // create and start the entities http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java b/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java index 35ffa93..a64d2e6 100644 --- a/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java +++ b/core/src/main/java/brooklyn/event/feed/windows/WindowsPerformanceCounterFeed.java @@ -20,14 +20,13 @@ package brooklyn.event.feed.windows; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import io.cloudsoft.winrm4j.winrm.WinRmToolResponse; -import java.math.BigInteger; +import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.SortedMap; -import java.util.SortedSet; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; @@ -49,23 +48,16 @@ import brooklyn.event.basic.Sensors; import brooklyn.event.feed.AbstractFeed; import brooklyn.event.feed.PollHandler; import brooklyn.event.feed.Poller; -import brooklyn.event.feed.ssh.SshPollValue; -import brooklyn.location.basic.SshMachineLocation; import brooklyn.location.basic.WinRmMachineLocation; import brooklyn.management.ExecutionContext; +import brooklyn.util.exceptions.Exceptions; import brooklyn.util.flags.TypeCoercions; -import brooklyn.util.guava.Maybe; -import brooklyn.util.task.ssh.SshTasks; -import brooklyn.util.task.system.ProcessTaskFactory; -import brooklyn.util.task.system.ProcessTaskWrapper; import brooklyn.util.time.Duration; -import io.cloudsoft.winrm4j.winrm.WinRmToolResponse; import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -107,8 +99,8 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { private static final Joiner JOINER_ON_COMMA = Joiner.on(','); @SuppressWarnings("serial") - public static final ConfigKey<List<WindowsPerformanceCounterPollConfig<?>>> POLLS = ConfigKeys.newConfigKey( - new TypeToken<List<WindowsPerformanceCounterPollConfig<?>>>() {}, + public static final ConfigKey<Collection<WindowsPerformanceCounterPollConfig<?>>> POLLS = ConfigKeys.newConfigKey( + new TypeToken<Collection<WindowsPerformanceCounterPollConfig<?>>>() {}, "polls"); public static Builder builder() { @@ -180,13 +172,13 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { if (configCopy.getPeriod() < 0) configCopy.period(builder.period); polls.add(configCopy); } - setConfig(POLLS, polls); + config().set(POLLS, polls); initUniqueTag(builder.uniqueTag, polls); } @Override protected void preStart() { - List<WindowsPerformanceCounterPollConfig<?>> polls = getConfig(POLLS); + Collection<WindowsPerformanceCounterPollConfig<?>> polls = getConfig(POLLS); long minPeriod = Integer.MAX_VALUE; List<String> performanceCounterNames = Lists.newArrayList(); @@ -200,13 +192,16 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { .add("-Counter") .add(JOINER_ON_COMMA.join(Iterables.transform(performanceCounterNames, QuoteStringFunction.INSTANCE))) .add("-SampleInterval") - .add("2") // TODO: This should be a config key + .add("2") // TODO: extract SampleInterval as a config key .build(); String command = String.format("(%s).CounterSamples.CookedValue", JOINER_ON_SPACE.join(allParams)); - log.debug("Windows performance counter poll command will be: {}", command); + log.debug("Windows performance counter poll command for {} will be: {}", entity, command); - getPoller().scheduleAtFixedRate(new GetPerformanceCountersJob<WinRmToolResponse>(getEntity(), command), - new SendPerfCountersToSensors(getEntity(), getConfig(POLLS)), 100L); + GetPerformanceCountersJob<WinRmToolResponse> job = new GetPerformanceCountersJob<WinRmToolResponse>(getEntity(), command); + getPoller().scheduleAtFixedRate( + new CallInEntityExecutionContext<WinRmToolResponse>(entity, job), + new SendPerfCountersToSensors(getEntity(), getConfig(POLLS)), + minPeriod); } private static class GetPerformanceCountersJob<T> implements Callable<T> { @@ -240,7 +235,6 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { * @param <T> The type of the {@link java.util.concurrent.Callable}. */ private static class CallInEntityExecutionContext<T> implements Callable<T> { - private final Callable<T> job; private EntityLocal entity; @@ -251,25 +245,26 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { @Override public T call() throws Exception { - final ExecutionContext executionContext - - = ((EntityInternal) entity).getManagementSupport().getExecutionContext(); + ExecutionContext executionContext = ((EntityInternal) entity).getManagementSupport().getExecutionContext(); return executionContext.submit(Maps.newHashMap(), job).get(); } } private static class SendPerfCountersToSensors implements PollHandler<WinRmToolResponse> { - private final EntityLocal entity; private final List<WindowsPerformanceCounterPollConfig<?>> polls; - - public SendPerfCountersToSensors(EntityLocal entity, List<WindowsPerformanceCounterPollConfig<?>> polls) { + private final Set<AttributeSensor<?>> failedAttributes = Sets.newLinkedHashSet(); + + public SendPerfCountersToSensors(EntityLocal entity, Collection<WindowsPerformanceCounterPollConfig<?>> polls) { this.entity = entity; - this.polls = polls; + this.polls = ImmutableList.copyOf(polls); } @Override public boolean checkSuccess(WinRmToolResponse val) { + // TODO not just using statucCode; also looking at absence of stderr. + // Status code is (empirically) unreliable: it returns 0 sometimes even when failed + // (but never returns non-zero on success). if (val.getStatusCode() != 0) return false; String stderr = val.getStdErr(); if (stderr == null || stderr.length() != 0) return false; @@ -282,11 +277,20 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { public void onSuccess(WinRmToolResponse val) { String[] values = val.getStdOut().split("\r\n"); for (int i = 0; i < polls.size(); i++) { - Class<?> clazz = polls.get(i).getSensor().getType(); - Maybe<? extends Object> maybeValue = TypeCoercions.tryCoerce(values[i], TypeToken.of(clazz)); - Object value = maybeValue.isAbsent() ? null : maybeValue.get(); - AttributeSensor<Object> attribute = (AttributeSensor<Object>) Sensors.newSensor(clazz, polls.get(i).getSensor().getName(), polls.get(i).getDescription()); - entity.setAttribute(attribute, value); + WindowsPerformanceCounterPollConfig<?> config = polls.get(i); + Class<?> clazz = config.getSensor().getType(); + AttributeSensor<Object> attribute = (AttributeSensor<Object>) Sensors.newSensor(clazz, config.getSensor().getName(), config.getDescription()); + try { + Object value = TypeCoercions.coerce(values[i], TypeToken.of(clazz)); + entity.setAttribute(attribute, value); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + if (failedAttributes.add(attribute)) { + log.warn("Failed to coerce value '{}' to {} for {} -> {}", new Object[] {values[i], clazz, entity, attribute}); + } else { + if (log.isTraceEnabled()) log.trace("Failed (repeatedly) to coerce value '{}' to {} for {} -> {}", new Object[] {values[i], clazz, entity, attribute}); + } + } } } @@ -295,7 +299,9 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { log.error("Windows Performance Counter query did not respond as expected. exitcode={} stdout={} stderr={}", new Object[]{val.getStatusCode(), val.getStdOut(), val.getStdErr()}); for (WindowsPerformanceCounterPollConfig<?> config : polls) { - entity.setAttribute(Sensors.newSensor(config.getSensor().getClass(), config.getPerformanceCounterName(), config.getDescription()), null); + Class<?> clazz = config.getSensor().getType(); + AttributeSensor<?> attribute = Sensors.newSensor(clazz, config.getSensor().getName(), config.getDescription()); + entity.setAttribute(attribute, null); } } @@ -319,103 +325,6 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed { } } - /** - * A poll handler that takes the result of the <tt>typeperf</tt> invocation and sets the appropriate sensors. - */ - private static class SendPerfCountersToSensors2 implements PollHandler<SshPollValue> { - - private static final Set<? extends Class<? extends Number>> INTEGER_TYPES - = ImmutableSet.of(Integer.class, Long.class, Byte.class, Short.class, BigInteger.class); - - private final EntityLocal entity; - private final SortedMap<String, AttributeSensor> sensorMap; - - public SendPerfCountersToSensors2(EntityLocal entity, List<WindowsPerformanceCounterPollConfig<?>> polls) { - this.entity = entity; - - sensorMap = Maps.newTreeMap(); - for (WindowsPerformanceCounterPollConfig<?> config : polls) { - sensorMap.put(config.getPerformanceCounterName(), config.getSensor()); - } - } - - @Override - public boolean checkSuccess(SshPollValue val) { - if (val.getExitStatus() != 0) return false; - String stderr = val.getStderr(); - if (stderr == null || stderr.length() != 0) return false; - String out = val.getStdout(); - if (out == null || out.length() == 0) return false; - return true; - } - - @Override - public void onSuccess(SshPollValue val) { - String stdout = val.getStdout(); - Matcher matcher = lineWithPerfData.matcher(stdout); - if (!matcher.find()) { - onFailure(val); - return; - } - - String group = matcher.group(0); - Iterator<String> values = new PerfCounterValueIterator(group); - for (AttributeSensor sensor : sensorMap.values()) { - if (!values.hasNext()) { - // The perf counter response has fewer elements than expected - onFailure(val); - return; - } - String value = values.next(); - - Class sensorType = sensor.getType(); - if (INTEGER_TYPES.contains(sensorType)) { - // Windows always returns decimal-formatted numbers (e.g. 1234.00000), even for integer counters. - // Integer.valueOf() throws a NumberFormatException if it sees something in that format. So for - // pure integer sensors, we truncate the decimal part. - int decimalAt = value.indexOf('.'); - if (decimalAt >= 0) - value = value.substring(0, decimalAt); - } - entity.setAttribute(sensor, TypeCoercions.coerce(value, sensorType)); - } - - if (values.hasNext()) { - // The perf counter response has more elements than expected - onFailure(val); - return; - } - } - - @Override - public void onFailure(SshPollValue val) { - log.error("Windows Performance Counter query did not respond as expected. exitcode={} stdout={} stderr={}", - new Object[]{val.getExitStatus(), val.getStdout(), val.getStderr()}); - for (AttributeSensor attribute : sensorMap.values()) { - entity.setAttribute(attribute, null); - } - } - - @Override - public void onException(Exception exception) { - log.error("Detected exception while retrieving Windows Performance Counters from entity " + - entity.getDisplayName(), exception); - for (AttributeSensor attribute : sensorMap.values()) { - entity.setAttribute(attribute, null); - } - } - - @Override - public String toString() { - return super.toString()+"["+getDescription()+"]"; - } - - @Override - public String getDescription() { - return ""+sensorMap; - } - } - static class PerfCounterValueIterator implements Iterator<String> { // This pattern matches the contents of the first field, and optionally matches the rest of the line as http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java b/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java index 1f651e1..ee32568 100644 --- a/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java +++ b/core/src/main/java/brooklyn/location/basic/WinRmMachineLocation.java @@ -18,6 +18,10 @@ */ package brooklyn.location.basic; +import static com.google.common.base.Preconditions.checkNotNull; +import io.cloudsoft.winrm4j.winrm.WinRmTool; +import io.cloudsoft.winrm4j.winrm.WinRmToolResponse; + import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -31,7 +35,6 @@ import java.util.Set; import javax.annotation.Nullable; import org.apache.commons.codec.binary.Base64; -import org.python.core.PyException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,39 +44,56 @@ import brooklyn.location.MachineDetails; import brooklyn.location.MachineLocation; import brooklyn.location.OsDetails; import brooklyn.util.exceptions.Exceptions; -import brooklyn.util.flags.SetFromFlag; +import brooklyn.util.stream.Streams; import brooklyn.util.time.Duration; import brooklyn.util.time.Time; -import io.cloudsoft.winrm4j.winrm.WinRmTool; -import io.cloudsoft.winrm4j.winrm.WinRmToolResponse; import com.google.common.base.Charsets; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; public class WinRmMachineLocation extends AbstractLocation implements MachineLocation { private static final Logger LOG = LoggerFactory.getLogger(WinRmMachineLocation.class); - public static final ConfigKey<String> WINDOWS_USERNAME = ConfigKeys.newStringConfigKey("windows.username", + // FIXME Respect `port` config when using {@link WinRmTool} + public static final ConfigKey<Integer> WINRM_PORT = ConfigKeys.newIntegerConfigKey( + "port", + "WinRM port to use when connecting to the remote machine", + 5985); + + // TODO merge with {link SshTool#PROP_USER} and {@link SshMachineLocation#user} + public static final ConfigKey<String> USER = ConfigKeys.newStringConfigKey("user", "Username to use when connecting to the remote machine"); - public static final ConfigKey<String> WINDOWS_PASSWORD = ConfigKeys.newStringConfigKey("windows.password", + // TODO merge with {link SshTool#PROP_PASSWORD} + public static final ConfigKey<String> PASSWORD = ConfigKeys.newStringConfigKey("password", "Password to use when connecting to the remote machine"); public static final ConfigKey<Integer> COPY_FILE_CHUNK_SIZE_BYTES = ConfigKeys.newIntegerConfigKey("windows.copy.file.size.bytes", "Size of file chunks (in bytes) to be used when copying a file to the remote server", 1024); - @SetFromFlag - protected String user; + public static final ConfigKey<InetAddress> ADDRESS = ConfigKeys.newConfigKey( + InetAddress.class, + "address", + "Address of the remote machine"); - @SetFromFlag(nullable = false) - protected InetAddress address; + public static final ConfigKey<Integer> EXECUTION_ATTEMPTS = ConfigKeys.newIntegerConfigKey( + "windows.exec.attempts", + "Number of attempts to execute a remote command", + 1); + + // TODO See SshTool#PROP_SSH_TRIES, where it was called "sshTries"; remove duplication? Merge into one well-named thing? + public static final ConfigKey<Integer> EXEC_TRIES = ConfigKeys.newIntegerConfigKey( + "execTries", + "Max number of times to attempt WinRM operations", + 10); @Override public InetAddress getAddress() { - return address; + return getConfig(ADDRESS); } @Override @@ -89,17 +109,19 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc @Nullable @Override public String getHostname() { - return address.getHostAddress(); + InetAddress address = getAddress(); + return (address != null) ? address.getHostAddress() : null; } @Override public Set<String> getPublicAddresses() { - return null; + InetAddress address = getAddress(); + return (address == null) ? ImmutableSet.<String>of() : ImmutableSet.of(address.getHostAddress()); } - + @Override public Set<String> getPrivateAddresses() { - return null; + return ImmutableSet.of(); } public WinRmToolResponse executeScript(String script) { @@ -107,32 +129,53 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc } public WinRmToolResponse executeScript(List<String> script) { + int execTries = getRequiredConfig(EXEC_TRIES); Collection<Throwable> exceptions = Lists.newArrayList(); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < execTries; i++) { try { - WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUsername(), getPassword()); - WinRmToolResponse response = winRmTool.executeScript(script); - return response; + return executeScriptNoRetry(script); } catch (Exception e) { - LOG.warn("ignoring winrm exception and retrying:", e); + Exceptions.propagateIfFatal(e); + if (i == (execTries+1)) { + LOG.info("Propagating WinRM exception (attempt "+(i+1)+" of "+execTries+")", e); + } else if (i == 0) { + LOG.warn("Ignoring WinRM exception and retrying (attempt "+(i+1)+" of "+execTries+")", e); + } else { + LOG.debug("Ignoring WinRM exception and retrying (attempt "+(i+1)+" of "+execTries+")", e); + } exceptions.add(e); } } throw Exceptions.propagate("failed to execute shell script", exceptions); } + protected WinRmToolResponse executeScriptNoRetry(List<String> script) { + WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUser(), getPassword()); + WinRmToolResponse response = winRmTool.executeScript(script); + return response; + } + public WinRmToolResponse executePsScript(String psScript) { return executePsScript(ImmutableList.of(psScript)); } public WinRmToolResponse executePsScript(List<String> psScript) { + int execTries = getRequiredConfig(EXEC_TRIES); Collection<Throwable> exceptions = Lists.newArrayList(); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < execTries; i++) { try { return executePsScriptNoRetry(psScript); } catch (Exception e) { - LOG.warn("ignoring winrm exception and retrying after 5 seconds:", e); - Time.sleep(Duration.FIVE_SECONDS); + Exceptions.propagateIfFatal(e); + if (i == (execTries+1)) { + LOG.info("Propagating WinRM exception (attempt "+(i+1)+" of "+execTries+")", e); + } else if (i == 0) { + LOG.warn("Ignoring WinRM exception and retrying after 5 seconds (attempt "+(i+1)+" of "+execTries+")", e); + Time.sleep(Duration.FIVE_SECONDS); + } else { + LOG.debug("Ignoring WinRM exception and retrying after 5 seconds (attempt "+(i+1)+" of "+execTries+")", e); + Time.sleep(Duration.FIVE_SECONDS); + } exceptions.add(e); } } @@ -140,21 +183,27 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc } public WinRmToolResponse executePsScriptNoRetry(List<String> psScript) { - WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUsername(), getPassword()); + WinRmTool winRmTool = WinRmTool.connect(getHostname(), getUser(), getPassword()); WinRmToolResponse response = winRmTool.executePs(psScript); return response; } - public int copyTo(File source, File destination) { + public int copyTo(File source, String destination) { + FileInputStream sourceStream = null; try { - return copyTo(new FileInputStream(source), destination); + sourceStream = new FileInputStream(source); + return copyTo(sourceStream, destination); } catch (FileNotFoundException e) { throw Exceptions.propagate(e); + } finally { + if (sourceStream != null) { + Streams.closeQuietly(sourceStream); + } } } - public int copyTo(InputStream source, File destination) { - executePsScript(ImmutableList.of("rm -ErrorAction SilentlyContinue " + destination.getPath())); + public int copyTo(InputStream source, String destination) { + executePsScript(ImmutableList.of("rm -ErrorAction SilentlyContinue " + destination)); try { int chunkSize = getConfig(COPY_FILE_CHUNK_SIZE_BYTES); byte[] inputData = new byte[chunkSize]; @@ -167,8 +216,8 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc } else { chunk = Arrays.copyOf(inputData, bytesRead); } - executePsScript(ImmutableList.of("If ((!(Test-Path " + destination.getPath() + ")) -or ((Get-Item '" + destination.getPath() + "').length -eq " + - expectedFileSize + ")) {Add-Content -Encoding Byte -path " + destination.getPath() + + executePsScript(ImmutableList.of("If ((!(Test-Path " + destination + ")) -or ((Get-Item '" + destination + "').length -eq " + + expectedFileSize + ")) {Add-Content -Encoding Byte -path " + destination + " -value ([System.Convert]::FromBase64String(\"" + new String(Base64.encodeBase64(chunk)) + "\"))}")); expectedFileSize += bytesRead; } @@ -179,14 +228,18 @@ public class WinRmMachineLocation extends AbstractLocation implements MachineLoc } } - public String getUsername() { - return config().get(WINDOWS_USERNAME); + public String getUser() { + return config().get(USER); } private String getPassword() { - return config().get(WINDOWS_PASSWORD); + return config().get(PASSWORD); } + private <T> T getRequiredConfig(ConfigKey<T> key) { + return checkNotNull(getConfig(key), "key %s must be set", key); + } + public static String getDefaultUserMetadataString() { // Using an encoded command obviates the need to escape String unencodePowershell = Joiner.on("\r\n").join(ImmutableList.of( http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/main/java/brooklyn/util/flags/TypeCoercions.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/flags/TypeCoercions.java b/core/src/main/java/brooklyn/util/flags/TypeCoercions.java index 74ada77..332c864 100644 --- a/core/src/main/java/brooklyn/util/flags/TypeCoercions.java +++ b/core/src/main/java/brooklyn/util/flags/TypeCoercions.java @@ -31,7 +31,6 @@ import java.math.BigInteger; import java.net.InetAddress; import java.net.URI; import java.net.URL; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; @@ -73,6 +72,7 @@ import com.google.common.base.Predicate; import com.google.common.collect.HashBasedTable; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -710,11 +710,43 @@ public class TypeCoercions { return QuorumChecks.of(input); } }); - registerAdapter(ArrayList.class, String[].class, new Function<ArrayList, String[]>() { + registerAdapter(Iterable.class, String[].class, new Function<Iterable, String[]>() { @Nullable @Override - public String[] apply(@Nullable ArrayList arrayList) { - return (String[]) arrayList.toArray(new String[]{}); + public String[] apply(@Nullable Iterable list) { + if (list == null) return null; + String[] result = new String[Iterables.size(list)]; + int count = 0; + for (Object element : list) { + result[count++] = coerce(element, String.class); + } + return result; + } + }); + registerAdapter(Iterable.class, Integer[].class, new Function<Iterable, Integer[]>() { + @Nullable + @Override + public Integer[] apply(@Nullable Iterable list) { + if (list == null) return null; + Integer[] result = new Integer[Iterables.size(list)]; + int count = 0; + for (Object element : list) { + result[count++] = coerce(element, Integer.class); + } + return result; + } + }); + registerAdapter(Iterable.class, int[].class, new Function<Iterable, int[]>() { + @Nullable + @Override + public int[] apply(@Nullable Iterable list) { + if (list == null) return null; + int[] result = new int[Iterables.size(list)]; + int count = 0; + for (Object element : list) { + result[count++] = coerce(element, int.class); + } + return result; } }); registerAdapter(String.class, Map.class, new Function<String,Map>() { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java index 58949d7..947068d 100644 --- a/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java +++ b/core/src/test/java/brooklyn/entity/group/DynamicClusterTest.java @@ -936,6 +936,21 @@ public class DynamicClusterTest extends BrooklynAppUnitTestSupport { assertFirstAndNonFirstCounts(cluster.getMembers(), 1, 2); } + @Test + public void testPrefersMemberSpecLocation() throws Exception { + DynamicCluster cluster = app.createAndManageChild(EntitySpec.create(DynamicCluster.class) + .configure(DynamicCluster.MEMBER_SPEC, EntitySpec.create(TestEntity.class) + .location(loc2)) + .configure(DynamicCluster.INITIAL_SIZE, 1)); + + cluster.start(ImmutableList.of(loc)); + assertEquals(ImmutableList.copyOf(cluster.getLocations()), ImmutableList.of(loc)); + + Entity member = Iterables.getOnlyElement(cluster.getMembers()); + assertEquals(ImmutableList.copyOf(member.getLocations()), ImmutableList.of(loc2)); + } + + private void assertFirstAndNonFirstCounts(Collection<Entity> members, int expectedFirstCount, int expectedNonFirstCount) { Set<Entity> found = MutableSet.of(); for (Entity e: members) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java b/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java index 0d6e5ff..c83d2f0 100644 --- a/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java +++ b/core/src/test/java/brooklyn/location/basic/ByonLocationResolverTest.java @@ -60,8 +60,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -public class - ByonLocationResolverTest { +public class ByonLocationResolverTest { private static final Logger log = LoggerFactory.getLogger(ByonLocationResolverTest.class); @@ -316,19 +315,19 @@ public class @Test public void testWindowsMachines() throws Exception { - brooklynProperties.put("brooklyn.location.byon.windows.username", "myuser"); - brooklynProperties.put("brooklyn.location.byon.windows.password", "mypassword"); + brooklynProperties.put("brooklyn.location.byon.user", "myuser"); + brooklynProperties.put("brooklyn.location.byon.password", "mypassword"); String spec = "byon"; Map<String, ?> flags = ImmutableMap.of( "hosts", ImmutableList.of("1.1.1.1", "2.2.2.2"), "osfamily", "windows" ); MachineProvisioningLocation<MachineLocation> provisioner = resolve(spec, flags); - MachineLocation location = provisioner.obtain(ImmutableMap.of()); + WinRmMachineLocation location = (WinRmMachineLocation) provisioner.obtain(ImmutableMap.of()); - assertEquals(location.config().get(WinRmMachineLocation.WINDOWS_USERNAME), "myuser"); - assertEquals(location.config().get(WinRmMachineLocation.WINDOWS_PASSWORD), "mypassword"); - Assert.assertNotNull(provisioner); + assertEquals(location.config().get(WinRmMachineLocation.USER), "myuser"); + assertEquals(location.config().get(WinRmMachineLocation.PASSWORD), "mypassword"); + assertEquals(location.config().get(WinRmMachineLocation.ADDRESS).getHostAddress(), "1.1.1.1"); } @Test http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java b/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java index 3133b22..3c09e1b 100644 --- a/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java +++ b/core/src/test/java/brooklyn/util/internal/TypeCoercionsTest.java @@ -22,7 +22,7 @@ import static org.testng.Assert.assertEquals; import java.math.BigDecimal; import java.math.BigInteger; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Map; @@ -35,6 +35,7 @@ import org.testng.Assert; import org.testng.annotations.Test; import brooklyn.entity.basic.Lifecycle; +import brooklyn.util.collections.MutableSet; import brooklyn.util.flags.ClassCoercionException; import brooklyn.util.flags.TypeCoercions; import brooklyn.util.text.StringPredicates; @@ -43,7 +44,6 @@ import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; import com.google.common.reflect.TypeToken; public class TypeCoercionsTest { @@ -188,6 +188,21 @@ public class TypeCoercionsTest { } @Test + public void testIterableToArrayCoercion() { + String[] s = TypeCoercions.coerce(ImmutableList.of("a", "b"), String[].class); + Assert.assertTrue(Arrays.equals(s, new String[] {"a", "b"}), "result="+Arrays.toString(s)); + + Integer[] i = TypeCoercions.coerce(ImmutableList.of(1, 2), Integer[].class); + Assert.assertTrue(Arrays.equals(i, new Integer[] {1, 2}), "result="+Arrays.toString(i)); + + int[] i2 = TypeCoercions.coerce(ImmutableList.of(1, 2), int[].class); + Assert.assertTrue(Arrays.equals(i2, new int[] {1, 2}), "result="+Arrays.toString(i2)); + + int[] i3 = TypeCoercions.coerce(MutableSet.of("1", 2), int[].class); + Assert.assertTrue(Arrays.equals(i3, new int[] {1, 2}), "result="+Arrays.toString(i3)); + } + + @Test public void testListEntryCoercion() { List<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<List<Class<?>>>() { }); Assert.assertEquals(s, ImmutableList.of(Integer.class, Double.class)); @@ -204,7 +219,7 @@ public class TypeCoercionsTest { Collection<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<Collection<Class<?>>>() { }); Assert.assertEquals(s, ImmutableList.of(Integer.class, Double.class)); } - + @Test public void testMapValueCoercion() { Map<?,?> s = TypeCoercions.coerce(ImmutableMap.of("int", "java.lang.Integer", "double", "java.lang.Double"), new TypeToken<Map<String, Class<?>>>() { }); @@ -309,11 +324,6 @@ public class TypeCoercionsTest { assertEquals(TypeCoercions.coerce("1.0", Number.class), (Number) Double.valueOf(1.0)); } - @Test - public void testArrayListToArray() { - assertEquals(TypeCoercions.coerce(Lists.newArrayList("Foo", "Bar", "Baz"), String[].class), new String[] {"Foo", "Bar", "Baz"}); - } - @Test(expectedExceptions = ClassCoercionException.class) public void testInvalidCoercionThrowsClassCoercionException() { TypeCoercions.coerce(new Object(), TypeToken.of(Integer.class)); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java index 30f7700..502c40c 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/BasicJcloudsLocationCustomizer.java @@ -52,16 +52,46 @@ public class BasicJcloudsLocationCustomizer implements JcloudsLocationCustomizer } @Override + public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) { + if (machine instanceof JcloudsSshMachineLocation) { + customize(location, computeService, (JcloudsSshMachineLocation)machine); + } else { + // no-op + } + } + + @Override + public void preRelease(JcloudsMachineLocation machine) { + if (machine instanceof JcloudsSshMachineLocation) { + preRelease((JcloudsSshMachineLocation)machine); + } else { + // no-op + } + } + + @Override + public void postRelease(JcloudsMachineLocation machine) { + if (machine instanceof JcloudsSshMachineLocation) { + postRelease((JcloudsSshMachineLocation)machine); + } else { + // no-op + } + } + + @Override + @Deprecated public void customize(JcloudsLocation location, ComputeService computeService, JcloudsSshMachineLocation machine) { // no-op } @Override + @Deprecated public void preRelease(JcloudsSshMachineLocation machine) { // no-op } @Override + @Deprecated public void postRelease(JcloudsSshMachineLocation machine) { // no-op } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java index df1c089..a4f6d75 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java @@ -22,10 +22,11 @@ import static brooklyn.util.JavaGroovyEquivalents.elvis; import static brooklyn.util.JavaGroovyEquivalents.groovyTruth; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.jclouds.compute.options.RunScriptOptions.Builder.overrideLoginCredentials; import static org.jclouds.scriptbuilder.domain.Statements.exec; +import io.cloudsoft.winrm4j.pywinrm.Session; +import io.cloudsoft.winrm4j.pywinrm.WinRMFactory; import java.io.ByteArrayOutputStream; import java.io.File; @@ -150,8 +151,6 @@ import brooklyn.util.text.Strings; import brooklyn.util.text.TemplateProcessor; import brooklyn.util.time.Duration; import brooklyn.util.time.Time; -import io.cloudsoft.winrm4j.pywinrm.Session; -import io.cloudsoft.winrm4j.pywinrm.WinRMFactory; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Charsets; @@ -572,7 +571,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im CloudMachineNamer cloudMachineNamer = getCloudMachineNamer(setup); String groupId = elvis(setup.get(GROUP_ID), cloudMachineNamer.generateNewGroupId(setup)); NodeMetadata node = null; - MachineLocation machineLocation = null; + JcloudsMachineLocation machineLocation = null; try { LOG.info("Creating VM "+setup.getDescription()+" in "+this); @@ -644,11 +643,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im boolean windows = node.getOperatingSystem().getFamily().equals(OsFamily.WINDOWS); if (windows) { - // FIXME: Tidy this up and allow for user-specified credentials + // FIXME Allow for user-specified credentials: + // - want to support setting up a given user on the VM (or resetting the Administrator password) + // - and support vCloudDirector use-case where template contains username/password, so don't just use + // what is returned by jclouds. setup.put(USER, node.getCredentials().getUser()); setup.put(PASSWORD, node.getCredentials().getOptionalPassword().orNull()); } + // TODO How do we influence the node.getLoginPort, so it is set correctly for Windows? // Setup port-forwarding, if required Optional<HostAndPort> sshHostAndPortOverride; if (usePortForwarding) { @@ -662,6 +665,18 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im sshHostAndPortOverride = Optional.absent(); } + if (waitForSshable) { + if (windows) { + // TODO Should we generalise `waitForSshable` to a `waitForReachable`? + // TODO Does jclouds support any windows user setup? + waitForWinRmAvailable(node, node.getCredentials(), setup); + } else if (skipJcloudsSshing) { + // once that host:port is definitely reachable, we can create the user + waitForReachable(computeService, node, sshHostAndPortOverride, node.getCredentials(), setup); + userCredentials = createUser(computeService, node, sshHostAndPortOverride, setup); + } + } + if (waitForSshable && skipJcloudsSshing && !windows) { // once that host:port is definitely reachable, we can create the user waitForReachable(computeService, node, sshHostAndPortOverride, node.getCredentials(), setup); @@ -701,19 +716,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im } usableTimestamp = Duration.of(provisioningStopwatch); - JcloudsSshMachineLocation jcloudsSshMachineLocation = null; - WinRmMachineLocation winRmMachineLocation = null; +// JcloudsSshMachineLocation jcloudsSshMachineLocation = null; +// WinRmMachineLocation winRmMachineLocation = null; // Create a JcloudsSshMachineLocation, and register it if (windows) { - // FIMXE: Need to write WinRM equivalent of getPublicHostname - String hostName = node.getPublicAddresses().iterator().next(); - winRmMachineLocation = registerWinRmMachineLocation(hostName, setup, node.getId()); - machineLocation = winRmMachineLocation; + machineLocation = registerWinRmMachineLocation(computeService, node, setup); } else { - jcloudsSshMachineLocation = registerJcloudsSshMachineLocation(computeService, node, userCredentials, sshHostAndPortOverride, setup); - machineLocation = jcloudsSshMachineLocation; - if (template!=null && jcloudsSshMachineLocation.getTemplate()==null) { - jcloudsSshMachineLocation.template = template; + machineLocation = registerJcloudsSshMachineLocation(computeService, node, userCredentials, sshHostAndPortOverride, setup); + if (template!=null && machineLocation.getTemplate()==null) { + ((JcloudsSshMachineLocation)machineLocation).template = template; } } @@ -737,12 +748,12 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im if (portForwardManager != null) { for(Integer containerPort : portMappings.keySet()) { Integer hostPort = portMappings.get(containerPort); - String dockerHost = jcloudsSshMachineLocation.getSshHostAndPort().getHostText(); - portForwardManager.associate(node.getId(), HostAndPort.fromParts(dockerHost, hostPort), jcloudsSshMachineLocation, containerPort); + String dockerHost = ((JcloudsSshMachineLocation)machineLocation).getSshHostAndPort().getHostText(); + portForwardManager.associate(node.getId(), HostAndPort.fromParts(dockerHost, hostPort), machineLocation, containerPort); } } else { LOG.warn("No port-forward manager for {} so could not associate docker port-mappings for {}", - this, jcloudsSshMachineLocation); + this, machineLocation); } } @@ -754,8 +765,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im List<String> setupScripts = setup.get(JcloudsLocationConfig.CUSTOM_MACHINE_SETUP_SCRIPT_URL_LIST); Collection<String> allScripts = new MutableList<String>().appendIfNotNull(setupScript).appendAll(setupScripts); for (String setupScriptItem : allScripts) { - if (Strings.isNonBlank(setupScript)) { - customisationForLogging.add("custom setup script " + setupScript); + if (Strings.isNonBlank(setupScriptItem)) { + customisationForLogging.add("custom setup script " + setupScriptItem); String setupVarsString = setup.get(JcloudsLocationConfig.CUSTOM_MACHINE_SETUP_SCRIPT_VARS); Map<String, String> substitutions = (setupVarsString != null) @@ -764,20 +775,20 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im String scriptContent = ResourceUtils.create(this).getResourceAsString(setupScriptItem); String script = TemplateProcessor.processTemplateContents(scriptContent, getManagementContext(), substitutions); if (windows) { - winRmMachineLocation.executeScript(ImmutableList.copyOf((script.replace("\r", "").split("\n")))); + ((WinRmMachineLocation)machineLocation).executeScript(ImmutableList.copyOf((script.replace("\r", "").split("\n")))); } else { - jcloudsSshMachineLocation.execCommands("Customizing node " + this, ImmutableList.of(script)); + ((SshMachineLocation)machineLocation).execCommands("Customizing node " + this, ImmutableList.of(script)); } } } if (setup.get(JcloudsLocationConfig.MAP_DEV_RANDOM_TO_DEV_URANDOM)) { if (windows) { - LOG.warn("Ignoring flag MAP_DEV_RANDOM_TO_DEV_URANDOM on Windows location {}", winRmMachineLocation); + LOG.warn("Ignoring flag MAP_DEV_RANDOM_TO_DEV_URANDOM on Windows location {}", machineLocation); } else { customisationForLogging.add("point /dev/random to urandom"); - jcloudsSshMachineLocation.execCommands("using urandom instead of random", + ((SshMachineLocation)machineLocation).execCommands("using urandom instead of random", Arrays.asList("sudo mv /dev/random /dev/random-real", "sudo ln -s /dev/urandom /dev/random")); } } @@ -786,11 +797,11 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im if (setup.get(GENERATE_HOSTNAME)) { if (windows) { // TODO: Generate Windows Hostname - LOG.warn("Ignoring flag GENERATE_HOSTNAME on Windows location {}", winRmMachineLocation); + LOG.warn("Ignoring flag GENERATE_HOSTNAME on Windows location {}", machineLocation); } else { customisationForLogging.add("configure hostname"); - jcloudsSshMachineLocation.execCommands("Generate hostname " + node.getName(), + ((SshMachineLocation)machineLocation).execCommands("Generate hostname " + node.getName(), Arrays.asList("sudo hostname " + node.getName(), "sudo sed -i \"s/HOSTNAME=.*/HOSTNAME=" + node.getName() + "/g\" /etc/sysconfig/network", "sudo bash -c \"echo 127.0.0.1 `hostname` >> /etc/hosts\"") @@ -800,13 +811,13 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im if (setup.get(OPEN_IPTABLES)) { if (windows) { - LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", winRmMachineLocation); + LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", machineLocation); } else { @SuppressWarnings("unchecked") Iterable<Integer> inboundPorts = (Iterable<Integer>) setup.get(INBOUND_PORTS); if (inboundPorts == null || Iterables.isEmpty(inboundPorts)) { - LOG.info("No ports to open in iptables (no inbound ports) for {} at {}", jcloudsSshMachineLocation, this); + LOG.info("No ports to open in iptables (no inbound ports) for {} at {}", machineLocation, this); } else { customisationForLogging.add("open iptables"); @@ -818,39 +829,39 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im for (String rule : iptablesRules) { batch.add(rule); if (batch.size() == 50) { - jcloudsSshMachineLocation.execCommands("Inserting iptables rules, 50 command batch", batch); + ((SshMachineLocation)machineLocation).execCommands("Inserting iptables rules, 50 command batch", batch); batch.clear(); } } if (batch.size() > 0) { - jcloudsSshMachineLocation.execCommands("Inserting iptables rules", batch); + ((SshMachineLocation)machineLocation).execCommands("Inserting iptables rules", batch); } - jcloudsSshMachineLocation.execCommands("List iptables rules", ImmutableList.of(IptablesCommands.listIptablesRule())); + ((SshMachineLocation)machineLocation).execCommands("List iptables rules", ImmutableList.of(IptablesCommands.listIptablesRule())); } } } if (setup.get(STOP_IPTABLES)) { if (windows) { - LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", winRmMachineLocation); + LOG.warn("Ignoring flag OPEN_IPTABLES on Windows location {}", machineLocation); } else { customisationForLogging.add("stop iptables"); List<String> cmds = ImmutableList.of(IptablesCommands.iptablesServiceStop(), IptablesCommands.iptablesServiceStatus()); - jcloudsSshMachineLocation.execCommands("Stopping iptables", cmds); + ((SshMachineLocation)machineLocation).execCommands("Stopping iptables", cmds); } } List<String> extraKeyUrlsToAuth = setup.get(EXTRA_PUBLIC_KEY_URLS_TO_AUTH); if (extraKeyUrlsToAuth!=null && !extraKeyUrlsToAuth.isEmpty()) { if (windows) { - LOG.warn("Ignoring flag EXTRA_PUBLIC_KEY_URLS_TO_AUTH on Windows location", winRmMachineLocation); + LOG.warn("Ignoring flag EXTRA_PUBLIC_KEY_URLS_TO_AUTH on Windows location", machineLocation); } else { List<String> extraKeyDataToAuth = MutableList.of(); for (String keyUrl : extraKeyUrlsToAuth) { extraKeyDataToAuth.add(ResourceUtils.create().getResourceAsString(keyUrl)); } - jcloudsSshMachineLocation.execCommands("Authorizing ssh keys", + ((SshMachineLocation)machineLocation).execCommands("Authorizing ssh keys", ImmutableList.of(new AuthorizeRSAPublicKeys(extraKeyDataToAuth).render(org.jclouds.scriptbuilder.domain.OsFamily.UNIX))); } } @@ -862,48 +873,32 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im // Apply any optional app-specific customization. for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) { - if (windows) { - LOG.warn("Ignoring customizer {} on Windows location {}", customizer, winRmMachineLocation); - // TODO: WinRm based JcloudsLocationCustomizer - } else { - customizer.customize(this, computeService, jcloudsSshMachineLocation); - } + customizer.customize(this, computeService, machineLocation); } customizedTimestamp = Duration.of(provisioningStopwatch); - String logMessage; - - if (windows) { - // TODO: More complete logging for WinRmMachineLocation - // FIXME: Remove try-catch! - try { - logMessage = "Finished VM " + setup.getDescription() + " creation:" - + " " + winRmMachineLocation.getConfig(WinRmMachineLocation.WINDOWS_USERNAME) + "@" + machineLocation.getAddress() - + " username=" + winRmMachineLocation.getUsername() - + " ready after " + Duration.of(provisioningStopwatch).toStringRounded() - + " (" + template + " template built in " + Duration.of(templateTimestamp).toStringRounded() + ";" - + " " + node + " provisioned in " + Duration.of(provisionTimestamp).subtract(templateTimestamp).toStringRounded() + ";"; - } catch (Exception e){ - logMessage = Arrays.toString(e.getStackTrace()); - } - } else { - logMessage = "Finished VM "+setup.getDescription()+" creation:" - + " "+jcloudsSshMachineLocation.getUser()+"@"+machineLocation.getAddress()+":"+jcloudsSshMachineLocation.getPort() + try { + String logMessage = "Finished VM "+setup.getDescription()+" creation:" + + " "+machineLocation.getUser()+"@"+machineLocation.getAddress()+":"+machineLocation.getPort() + (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS)) - ? "password=" + userCredentials.getOptionalPassword().or("<absent>") - + " && key=" + userCredentials.getOptionalPrivateKey().or("<absent>") - : "") + ? "password=" + userCredentials.getOptionalPassword().or("<absent>") + + " && key=" + userCredentials.getOptionalPrivateKey().or("<absent>") + : "") + " ready after "+Duration.of(provisioningStopwatch).toStringRounded() + " ("+template+" template built in "+Duration.of(templateTimestamp).toStringRounded()+";" + " "+node+" provisioned in "+Duration.of(provisionTimestamp).subtract(templateTimestamp).toStringRounded()+";" - + " "+jcloudsSshMachineLocation+" ssh usable in "+Duration.of(usableTimestamp).subtract(provisionTimestamp).toStringRounded()+";" + + " "+machineLocation+" connection usable in "+Duration.of(usableTimestamp).subtract(provisionTimestamp).toStringRounded()+";" + " and os customized in "+Duration.of(customizedTimestamp).subtract(usableTimestamp).toStringRounded()+" - "+Joiner.on(", ").join(customisationForLogging)+")"; + LOG.info(logMessage); + } catch (Exception e){ + // TODO Remove try-catch! @Nakomis: why did you add it? What exception happened during logging? + Exceptions.propagateIfFatal(e); + LOG.warn("Problem generating log message summarising completion of jclouds machine provisioning "+machineLocation+" by "+this, e); } - - LOG.info(logMessage); return machineLocation; + } catch (Exception e) { if (e instanceof RunNodesException && ((RunNodesException)e).getNodeErrors().size() > 0) { node = Iterables.get(((RunNodesException)e).getNodeErrors().keySet(), 0); @@ -927,53 +922,6 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im } } - private void waitForWinRmAvailable(final NodeMetadata node, final LoginCredentials expectedCredentials, ConfigBag setup) { - // TODO: Reduce / remove duplication between this and waitForReachable - String waitForWinRmAvailable = setup.get(WAIT_FOR_WINRM_AVAILABLE); - checkArgument(!"false".equalsIgnoreCase(waitForWinRmAvailable), "waitForWinRmAvailable called despite waitForWinRmAvailable=%s", waitForWinRmAvailable); - - long delayMs = -1; - try { - delayMs = Time.parseTimeString(""+waitForWinRmAvailable); - } catch (Exception e) { - // normal if 'true'; just fall back to default - } - if (delayMs<0) - delayMs = Time.parseTimeString(WAIT_FOR_WINRM_AVAILABLE.getDefaultValue()); - - // FIXME: remove this - LOG.info("Address: " + node.getPublicAddresses().iterator().next()); - LOG.info("User: " + expectedCredentials.getUser()); - LOG.info("Password: " + expectedCredentials.getOptionalPassword().get()); - final Session session = WinRMFactory.INSTANCE.createSession(node.getPublicAddresses().iterator().next(), - expectedCredentials.getUser(), expectedCredentials.getOptionalPassword().get()); - - Callable<Boolean> checker = new Callable<Boolean>() { - public Boolean call() { - return session.run_cmd("hostname").getStatusCode() == 0; - }}; - - Stopwatch stopwatch = Stopwatch.createStarted(); - - ReferenceWithError<Boolean> reachable = new Repeater() - .every(1, SECONDS) - .until(checker) - .limitTimeTo(delayMs, MILLISECONDS) - .runKeepingError(); - - if (!reachable.getWithoutError()) { - throw new IllegalStateException("WinRm failed for "+ - expectedCredentials.getUser()+"@"+node.getPublicAddresses().iterator().next()+" ("+setup.getDescription()+") after waiting "+ - Time.makeTimeStringRounded(delayMs), reachable.getError()); - } - - LOG.debug("VM {}: is available via WinRm after {} on {}@{}",new Object[] { - setup.getDescription(), Time.makeTimeStringRounded(stopwatch), - expectedCredentials.getUser(), node.getPublicAddresses().iterator().next()}); - - } - - // ------------- constructing the template, etc ------------------------ private static interface CustomizeTemplateBuilder { @@ -1074,15 +1022,15 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im .put(USER_METADATA_STRING, new CustomizeTemplateOptions() { public void apply(TemplateOptions t, ConfigBag props, Object v) { if (t instanceof EC2TemplateOptions) { + // See AWS docs: http://docs.aws.amazon.com/AWSEC2/latest/WindowsGuide/UsingConfig_WinAMI.html#user-data-execution if (v==null) return; String data = v.toString(); if (!(data.startsWith("<script>") || data.startsWith("<powershell>"))) { data = "<script> " + data + " </script>"; } ((EC2TemplateOptions)t).userData(data.getBytes()); - // TODO avail in next jclouds thanks to @andreaturli -// } else if (t instanceof SoftLayerTemplateOptions) { -// ((SoftLayerTemplateOptions)t).userData(Strings.toString(v)); + } else if (t instanceof SoftLayerTemplateOptions) { + ((SoftLayerTemplateOptions)t).userData(Strings.toString(v)); } else { LOG.info("ignoring userDataString({}) in VM creation because not supported for cloud/type ({})", v, t.getClass()); } @@ -1092,9 +1040,8 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im if (t instanceof EC2TemplateOptions) { byte[] bytes = toByteArray(v); ((EC2TemplateOptions)t).userData(bytes); - // TODO avail in next jclouds thanks to @andreaturli -// } else if (t instanceof SoftLayerTemplateOptions) { -// ((SoftLayerTemplateOptions)t).userData(Strings.toString(v)); + } else if (t instanceof SoftLayerTemplateOptions) { + ((SoftLayerTemplateOptions)t).userData(Strings.toString(v)); } else { LOG.info("ignoring userData({}) in VM creation because not supported for cloud/type ({})", v, t.getClass()); } @@ -1932,22 +1879,41 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im } } - protected WinRmMachineLocation registerWinRmMachineLocation(String vmHostname, ConfigBag setup, String nodeId) { - WinRmMachineLocation winRmMachineLocation = createWinRmMachineLocation(vmHostname, setup); + protected JcloudsWinRmMachineLocation registerWinRmMachineLocation(ComputeService computeService, NodeMetadata node, ConfigBag setup) { + // FIMXE: Need to write WinRM equivalent of getPublicHostname + String vmHostname = node.getPublicAddresses().iterator().next(); + + JcloudsWinRmMachineLocation winRmMachineLocation = createWinRmMachineLocation(computeService, node, vmHostname, setup); winRmMachineLocation.setParent(this); - vmInstanceIds.put(winRmMachineLocation, nodeId); + vmInstanceIds.put(winRmMachineLocation, node.getId()); return winRmMachineLocation; } - protected WinRmMachineLocation createWinRmMachineLocation(String vmHostname, ConfigBag setup) { + protected JcloudsWinRmMachineLocation createWinRmMachineLocation(ComputeService computeService, NodeMetadata node, String vmHostname, ConfigBag setup) { + String nodeAvailabilityZone = extractAvailabilityZone(setup, node); + String nodeRegion = extractRegion(setup, node); + if (nodeRegion == null) { + // e.g. rackspace doesn't have "region", so rackspace-uk is best we can say (but zone="LON") + nodeRegion = extractProvider(setup, node); + } + if (isManaged()) { - return getManagementContext().getLocationManager().createLocation(LocationSpec.create(WinRmMachineLocation.class) + return getManagementContext().getLocationManager().createLocation(LocationSpec.create(JcloudsWinRmMachineLocation.class) + .configure("jcloudsParent", this) .configure("displayName", vmHostname) .configure("address", vmHostname) .configure("user", getUser(setup)) - .configure(WinRmMachineLocation.WINDOWS_USERNAME, setup.get(USER)) - .configure(WinRmMachineLocation.WINDOWS_PASSWORD, setup.get(PASSWORD)) - ); + .configure(WinRmMachineLocation.USER, setup.get(USER)) + .configure(WinRmMachineLocation.PASSWORD, setup.get(PASSWORD)) + .configure("node", node) + .configureIfNotNull(CLOUD_AVAILABILITY_ZONE_ID, nodeAvailabilityZone) + .configureIfNotNull(CLOUD_REGION_ID, nodeRegion) + .configure(CALLER_CONTEXT, setup.get(CALLER_CONTEXT)) + .configure(SshMachineLocation.DETECT_MACHINE_DETAILS, setup.get(SshMachineLocation.DETECT_MACHINE_DETAILS)) + .configureIfNotNull(SshMachineLocation.SCRIPT_DIR, setup.get(SshMachineLocation.SCRIPT_DIR)) + .configureIfNotNull(USE_PORT_FORWARDING, setup.get(USE_PORT_FORWARDING)) + .configureIfNotNull(PORT_FORWARDER, setup.get(PORT_FORWARDER)) + .configureIfNotNull(PORT_FORWARDING_MANAGER, setup.get(PORT_FORWARDING_MANAGER))); } else { throw new UnsupportedOperationException("Cannot create WinRmMachineLocation because " + this + " is not managed"); } @@ -1988,31 +1954,28 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im @Override - public void release(MachineLocation machine) { - String instanceId = vmInstanceIds.remove(machine); + public void release(MachineLocation rawMachine) { + String instanceId = vmInstanceIds.remove(rawMachine); if (instanceId == null) { - LOG.info("Attempted release of unknown machine "+machine+" in "+toString()); - throw new IllegalArgumentException("Unknown machine "+machine); + LOG.info("Attempted release of unknown machine "+rawMachine+" in "+toString()); + throw new IllegalArgumentException("Unknown machine "+rawMachine); } - + JcloudsMachineLocation machine = (JcloudsMachineLocation) rawMachine; + LOG.info("Releasing machine {} in {}, instance id {}", new Object[] {machine, this, instanceId}); Exception tothrow = null; - if (machine instanceof JcloudsSshMachineLocation) { - ConfigBag setup = config().getBag(); - for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) { - try { - customizer.preRelease((JcloudsSshMachineLocation) machine); - } catch (Exception e) { - LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+ - "; ignoring and continuing, " - + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e); - if (tothrow==null) tothrow = e; - } + ConfigBag setup = config().getBag(); + for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) { + try { + customizer.preRelease(machine); + } catch (Exception e) { + LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+ + "; ignoring and continuing, " + + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e); + if (tothrow==null) tothrow = e; } - } else { - LOG.warn("Releasing non-jclouds machine "+machine+" from "+this+"; skipping customizers"); } try { @@ -2038,17 +2001,14 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im removeChild(machine); - if (machine instanceof JcloudsSshMachineLocation) { - ConfigBag setup = config().getBag(); - for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) { - try { - customizer.postRelease((JcloudsSshMachineLocation) machine); - } catch (Exception e) { - LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+ - "; ignoring and continuing, " - + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e); - if (tothrow==null) tothrow = e; - } + for (JcloudsLocationCustomizer customizer : getCustomizers(setup)) { + try { + customizer.postRelease(machine); + } catch (Exception e) { + LOG.error("Problem invoking pre-release customizer "+customizer+" for machine "+machine+" in "+this+", instance id "+instanceId+ + "; ignoring and continuing, " + + (tothrow==null ? "will throw subsequently" : "swallowing due to previous error")+": "+e, e); + if (tothrow==null) tothrow = e; } } @@ -2189,44 +2149,55 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im return null; } + private void waitForWinRmAvailable(final NodeMetadata node, final LoginCredentials expectedCredentials, ConfigBag setup) { + String waitForWinrmAvailable = setup.get(WAIT_FOR_WINRM_AVAILABLE); + checkArgument(!"false".equalsIgnoreCase(waitForWinrmAvailable), "waitForWinRmAvailable called despite waitForWinRmAvailable=%s", waitForWinrmAvailable); + Duration timeout = null; + try { + timeout = Duration.parse(waitForWinrmAvailable); + } catch (Exception e) { + // TODO will this just be a NumberFormatException? If so, catch that specificially + // normal if 'true'; just fall back to default + } + if (timeout == null) { + timeout = Duration.parse(WAIT_FOR_WINRM_AVAILABLE.getDefaultValue()); + } + + // TODO Use getFirstReachableAddress, when have getLoginPort set correctly + String user = expectedCredentials.getUser(); + String password = expectedCredentials.getOptionalPassword().orNull(); + String vmIp = node.getPublicAddresses().iterator().next(); + final Session session = WinRMFactory.INSTANCE.createSession(vmIp, user, password); + + Callable<Boolean> checker = new Callable<Boolean>() { + public Boolean call() { + return session.run_cmd("hostname").getStatusCode() == 0; + }}; + String connectionDetails = user+"@"+vmIp; + + waitForReachable(checker, connectionDetails, expectedCredentials, setup, timeout); + } + protected void waitForReachable(final ComputeService computeService, final NodeMetadata node, Optional<HostAndPort> hostAndPortOverride, final LoginCredentials expectedCredentials, ConfigBag setup) { String waitForSshable = setup.get(WAIT_FOR_SSHABLE); checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForReachable called despite waitForSshable=%s", waitForSshable); - String vmIp = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getHostText() : JcloudsUtil.getFirstReachableAddress(computeService.getContext(), node); - if (vmIp==null) LOG.warn("Unable to extract IP for "+node+" ("+setup.getDescription()+"): subsequent connection attempt will likely fail"); - - int vmPort = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getPortOrDefault(22) : 22; - - long delayMs = -1; + Duration timeout = null; try { - delayMs = Time.parseTimeString(""+waitForSshable); + timeout = Duration.parse(waitForSshable); } catch (Exception e) { // normal if 'true'; just fall back to default } - if (delayMs<0) - delayMs = Time.parseTimeString(WAIT_FOR_SSHABLE.getDefaultValue()); - - String user = expectedCredentials.getUser(); - if (LOG.isDebugEnabled()) { - Optional<String> password; - Optional<String> key; - if (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))) { - password = expectedCredentials.getOptionalPassword(); - key = expectedCredentials.getOptionalPrivateKey(); - } else { - password = expectedCredentials.getOptionalPassword().isPresent() ? Optional.of("******") : Optional.<String>absent(); - key = expectedCredentials.getOptionalPrivateKey().isPresent() ? Optional.of("******") : Optional.<String>absent(); - } - LOG.debug("VM {}: reported online, now waiting {} for it to be sshable on {}@{}:{}{}; using credentials password={}; key={}", - new Object[] { - setup.getDescription(), Time.makeTimeStringRounded(delayMs), - user, vmIp, vmPort, - Objects.equal(user, getUser(setup)) ? "" : " (setup user is different: "+getUser(setup)+")", - password.or("<absent>"), - key.or("<absent>") - }); + if (timeout == null) { + timeout = Duration.parse(WAIT_FOR_SSHABLE.getDefaultValue()); } + + String user = expectedCredentials.getUser(); + String vmIp = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getHostText() : JcloudsUtil.getFirstReachableAddress(computeService.getContext(), node); + if (vmIp==null) LOG.warn("Unable to extract IP for "+node+" ("+setup.getDescription()+"): subsequent connection attempt will likely fail"); + int vmPort = hostAndPortOverride.isPresent() ? hostAndPortOverride.get().getPortOrDefault(22) : 22; + + String connectionDetails = user + "@" + vmIp + ":" + vmPort; Callable<Boolean> checker; if (hostAndPortOverride.isPresent()) { @@ -2246,25 +2217,51 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im }}; } + waitForReachable(checker, connectionDetails, expectedCredentials, setup, timeout); + } + + protected void waitForReachable(Callable<Boolean> checker, String connectionDetails, LoginCredentials expectedCredentials, ConfigBag setup, Duration timeout) { + String user = expectedCredentials.getUser(); + if (LOG.isDebugEnabled()) { + Optional<String> password; + Optional<String> key; + if (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))) { + password = expectedCredentials.getOptionalPassword(); + key = expectedCredentials.getOptionalPrivateKey(); + } else { + password = expectedCredentials.getOptionalPassword().isPresent() ? Optional.of("******") : Optional.<String>absent(); + key = expectedCredentials.getOptionalPrivateKey().isPresent() ? Optional.of("******") : Optional.<String>absent(); + } + LOG.debug("VM {}: reported online, now waiting {} for it to be contactable on {}{}; using credentials password={}; key={}", + new Object[] { + setup.getDescription(), timeout, + connectionDetails, + Objects.equal(user, getUser(setup)) ? "" : " (setup user is different: "+getUser(setup)+")", + password.or("<absent>"), + key.or("<absent>") + }); + } + Stopwatch stopwatch = Stopwatch.createStarted(); ReferenceWithError<Boolean> reachable = new Repeater() - .every(1,SECONDS) - .until(checker) - .limitTimeTo(delayMs, MILLISECONDS) - .runKeepingError(); + .every(1,SECONDS) + .until(checker) + .limitTimeTo(timeout) + .runKeepingError(); if (!reachable.getWithoutError()) { - throw new IllegalStateException("SSH failed for "+ - user+"@"+vmIp+" ("+setup.getDescription()+") after waiting "+ - Time.makeTimeStringRounded(delayMs), reachable.getError()); + throw new IllegalStateException("Connection failed for " + +connectionDetails+" ("+setup.getDescription()+") after waiting " + +Time.makeTimeStringRounded(timeout), reachable.getError()); } - LOG.debug("VM {}: is sshable after {} on {}@{}",new Object[] { + LOG.debug("VM {}: connection succeeded after {} on {}",new Object[] { setup.getDescription(), Time.makeTimeStringRounded(stopwatch), - user, vmIp}); + connectionDetails}); } + // -------------------- hostnames ------------------------ // hostnames are complicated, but irregardless, this code could be cleaned up! http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9c37f241/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java index 94dc610..a21891c 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocationCustomizer.java @@ -63,17 +63,43 @@ public interface JcloudsLocationCustomizer { * <p/> * If {@link brooklyn.location.jclouds.JcloudsLocationConfig#WAIT_FOR_SSHABLE} is true the * machine is guaranteed to be SSHable when this method is called. + * + * @since 0.7.0; use {@link #customize(JcloudsLocation, ComputeService, JcloudsMachineLocation)} */ + @Deprecated void customize(JcloudsLocation location, ComputeService computeService, JcloudsSshMachineLocation machine); /** * Override to handle machine-related cleanup before Jclouds is called to release (destroy) the machine. + * + * @since 0.7.0; use {@link #preRelease(JcloudsMachineLocation)} */ + @Deprecated void preRelease(JcloudsSshMachineLocation machine); /** * Override to handle machine-related cleanup after Jclouds is called to release (destroy) the machine. + * + * @since 0.7.0; use {@link #postRelesae(JcloudsMachineLocation)} */ + @Deprecated void postRelease(JcloudsSshMachineLocation machine); + /** + * Override to configure the given machine once it has been created and started by Jclouds. + * <p/> + * If {@link brooklyn.location.jclouds.JcloudsLocationConfig#WAIT_FOR_SSHABLE} is true the + * machine is guaranteed to be SSHable when this method is called. + */ + void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine); + + /** + * Override to handle machine-related cleanup before Jclouds is called to release (destroy) the machine. + */ + void preRelease(JcloudsMachineLocation machine); + + /** + * Override to handle machine-related cleanup after Jclouds is called to release (destroy) the machine. + */ + void postRelease(JcloudsMachineLocation machine); }
