Skip to content
Permalink
Browse files
ensure install-label-salt-inferencing does not block
otherwise setting a shell env var to include a port will cause it to block prematurely.
also clean up javadoc around `getNonBlocking` behaviour.
  • Loading branch information
ahgittin committed Jan 21, 2016
1 parent b28ba02 commit 5e9012b23e2bf9ad6423a5e0ef13c7ea8153947d
Showing 6 changed files with 51 additions and 12 deletions.
@@ -19,7 +19,6 @@

package org.apache.brooklyn.core.objs;

import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;

import org.apache.brooklyn.api.mgmt.ExecutionContext;
@@ -28,8 +27,8 @@
import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.core.task.ValueResolver;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.time.Duration;

public abstract class AbstractConfigurationSupportInternal implements BrooklynObjectInternal.ConfigurationSupportInternal {

@@ -63,7 +62,7 @@ public <T> Maybe<T> getNonBlocking(ConfigKey<T> key) {
Object resolved = Tasks.resolving(unresolved)
.as(Object.class)
.defaultValue(marker)
.timeout(Duration.of(5, TimeUnit.MILLISECONDS))
.timeout(ValueResolver.REAL_REAL_QUICK_WAIT)
.context(getContext())
.swallowExceptions()
.get();
@@ -69,6 +69,9 @@ public interface ConfigurationSupportInternal extends Configurable.Configuration
/**
* Returns the uncoerced value for this config key, if available, not taking any default.
* If there is no local value and there is an explicit inherited value, will return the inherited.
* Returns {@link Maybe#absent()} if the key is not explicitly set on this object or an ancestor.
* <p>
* See also {@link #getLocalRaw(ConfigKey).
*/
@Beta
Maybe<Object> getRaw(ConfigKey<?> key);
@@ -82,6 +85,9 @@ public interface ConfigurationSupportInternal extends Configurable.Configuration
/**
* Returns the uncoerced value for this config key, if available,
* not following any inheritance chains and not taking any default.
* Returns {@link Maybe#absent()} if the key is not explicitly set on this object.
* <p>
* See also {@link #getRaw(ConfigKey).
*/
@Beta
Maybe<Object> getLocalRaw(ConfigKey<?> key);
@@ -96,6 +102,11 @@ public interface ConfigurationSupportInternal extends Configurable.Configuration
* Attempts to coerce the value for this config key, if available,
* taking a default and {@link Maybe#absent absent} if the uncoerced
* cannot be resolved within a short timeframe.
* <p>
* Note: if no value for the key is available, not even as a default,
* this returns a {@link Maybe#isPresent()} containing <code>null</code>
* (following the semantics of {@link #get(ConfigKey)}
* rather than {@link #getRaw(ConfigKey)}).
*/
@Beta
<T> Maybe<T> getNonBlocking(ConfigKey<T> key);
@@ -122,10 +122,10 @@ public static <T> T coerce(Object value, Class<T> targetType) {
return coerce(value, TypeToken.of(targetType));
}

/** @see #coerce(Object, Class) */
/** @see #coerce(Object, Class); allows a null value in the contents of the Maybe */
public static <T> Maybe<T> tryCoerce(Object value, TypeToken<T> targetTypeToken) {
try {
return Maybe.of( coerce(value, targetTypeToken) );
return Maybe.ofAllowingNull( coerce(value, targetTypeToken) );
} catch (Throwable t) {
Exceptions.propagateIfFatal(t);
return Maybe.absent(t);
@@ -42,6 +42,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.annotations.Beta;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
@@ -57,13 +58,24 @@
*/
public class ValueResolver<T> implements DeferredSupplier<T> {

// TODO most of these usages should be removed when we have
// an ability to run resolution in a non-blocking mode
// (i.e. running resolution tasks in the same thread,
// or in a context where they can only wait on subtasks
// which are guaranteed to have the same constraint)
/**
* Period to wait if we're expected to return real quick
* but we want fast things to have time to finish.
* <p>
* Timings are always somewhat arbitrary but this at least
* allows some intention to be captured in code rather than arbitrary values. */
@Beta
public static Duration REAL_QUICK_WAIT = Duration.millis(50);
/**
* Like {@link #REAL_QUICK_WAIT} but even smaller, for use when potentially
* resolving multiple items in sequence. */
@Beta
public static Duration REAL_REAL_QUICK_WAIT = Duration.millis(5);
/**
* Period to wait if we're expected to return quickly
* but we want to be a bit more generous for things to finish,
@@ -118,7 +118,8 @@ protected void preStartCustom(MachineLocation machine) {
entity().initDriver(machine);

// Note: must only apply config-sensors after adding to locations and creating driver;
// otherwise can't do things like acquire free port from location, or allowing driver to set up ports
// otherwise can't do things like acquire free port from location
// or allowing driver to set up ports; but must be careful in init not to block on these!
super.preStartCustom(machine);

entity().preStart();
@@ -21,11 +21,12 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import org.apache.brooklyn.api.entity.EntityLocal;
import org.apache.brooklyn.api.entity.drivers.downloads.DownloadResolver;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.entity.Entities;
import org.apache.brooklyn.core.objs.BrooklynObjectInternal.ConfigurationSupportInternal;
import org.apache.brooklyn.entity.software.base.lifecycle.ScriptHelper;
import org.apache.brooklyn.location.ssh.SshMachineLocation;
import org.apache.brooklyn.util.collections.MutableMap;
@@ -54,15 +55,30 @@ public VanillaSoftwareProcessSshDriver(EntityLocal entity, SshMachineLocation ma
*/
@Override
protected String getInstallLabelExtraSalt() {
// run non-blocking in case a value set later is used (e.g. a port)
Integer hash = hashCodeIfResolved(SoftwareProcess.DOWNLOAD_URL.getConfigKey(),
VanillaSoftwareProcess.INSTALL_COMMAND, SoftwareProcess.SHELL_ENVIRONMENT);

Maybe<Object> url = getEntity().getConfigRaw(SoftwareProcess.DOWNLOAD_URL, true);
String installCommand = getEntity().getConfig(VanillaSoftwareProcess.INSTALL_COMMAND);
Map<String, Object> shellEnv = getEntity().getConfig(VanillaSoftwareProcess.SHELL_ENVIRONMENT);
// if any of the above blocked then we must make a unique install label,
// as other yet-unknown config is involved
if (hash==null) return Identifiers.makeRandomId(8);

// TODO a user-friendly hash would be nice, but tricky since we don't want it to be too long or contain path chars
return Identifiers.makeIdFromHash(Objects.hash(url.or(null), installCommand, shellEnv));
// a user-friendly hash is nice, but tricky since it would have to be short;
// go with a random one unless it's totally blank
if (hash==0) return "default";
return Identifiers.makeIdFromHash(hash);
}

private Integer hashCodeIfResolved(ConfigKey<?> ...keys) {
int hash = 0;
for (ConfigKey<?> k: keys) {
Maybe<?> value = ((ConfigurationSupportInternal)getEntity().config()).getNonBlocking(k);
if (value.isAbsent()) return null;
hash = hash*31 + (value.get()==null ? 0 : value.get().hashCode());
}
return hash;
}

@Override
public void install() {
Maybe<Object> url = getEntity().getConfigRaw(SoftwareProcess.DOWNLOAD_URL, true);

0 comments on commit 5e9012b

Please sign in to comment.