Skip to content
Permalink
Browse files
tidy several other places start(Collection<Location>) is assuming loc…
…ations are always passed in, not inherited
  • Loading branch information
ahgittin committed Jan 19, 2016
1 parent e3da35d commit ba8bb7f6dba6553888a669bd9cc1d4e3f27cb3dd
Showing 9 changed files with 38 additions and 22 deletions.
@@ -28,6 +28,7 @@
import org.apache.brooklyn.core.entity.Entities;
import org.apache.brooklyn.core.entity.trait.Startable;
import org.apache.brooklyn.core.feed.ConfigToAttributes;
import org.apache.brooklyn.core.location.Locations;
import org.apache.brooklyn.enricher.stock.Enrichers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -104,11 +105,14 @@ public DynamicCluster getCluster() {
@Override
public void start(Collection<? extends Location> locations) {
if (isLegacyConstruction()) {
// TODO should no longer be needed?
init();
}

if (locations.isEmpty()) locations = getLocations();
locations = MutableList.copyOf(Locations.getLocationsCheckingAncestors(locations, this));

Iterables.getOnlyElement(locations); // Assert just one
// set it; here we don't allow changing locations
addLocations(locations);

List<Entity> childrenToStart = MutableList.<Entity>of(getCluster());
@@ -38,6 +38,7 @@
import org.apache.brooklyn.core.entity.trait.Startable;
import org.apache.brooklyn.core.entity.trait.StartableMethods;
import org.apache.brooklyn.core.feed.ConfigToAttributes;
import org.apache.brooklyn.core.location.Locations;
import org.apache.brooklyn.enricher.stock.Enrichers;
import org.apache.brooklyn.entity.group.DynamicGroupImpl;
import org.apache.brooklyn.entity.proxy.LoadBalancer;
@@ -207,7 +208,8 @@ public void start(Collection<? extends Location> locations) {
init();
}

if (locations.isEmpty()) locations = getLocations();
locations = Locations.getLocationsCheckingAncestors(locations, this);
// store inherited locations
addLocations(locations);

LoadBalancer loadBalancer = getController();
@@ -229,7 +231,8 @@ public void start(Collection<? extends Location> locations) {
}
}

Entities.invokeEffectorList(this, childrenToStart, Startable.START, ImmutableMap.of("locations", locations)).get();
// don't propagate start locations
Entities.invokeEffectorList(this, childrenToStart, Startable.START, ImmutableMap.of("locations", MutableList.of())).get();
if (startControllerTask != null) {
startControllerTask.get();
}
@@ -430,7 +430,9 @@ public static boolean isResolverPrefixForSpec(LocationResolver resolver, String
public List<Location> resolve(Iterable<?> spec) {
List<Location> result = new ArrayList<Location>();
for (Object id : spec) {
if (id instanceof String) {
if (id==null) {
// drop a null entry
} if (id instanceof String) {
result.add(resolve((String) id));
} else if (id instanceof Location) {
result.add((Location) id);
@@ -314,14 +314,14 @@ public void setFactory(EntityFactory<?> factory) {

private Location getLocation(boolean required) {
Collection<? extends Location> ll = Locations.getLocationsCheckingAncestors(getLocations(), this);
try {
if (!required && ll.isEmpty()) return null;
return Iterables.getOnlyElement(ll);
} catch (Exception e) {
Exceptions.propagateIfFatal(e);
if (ll.isEmpty()) throw new IllegalStateException("No location available for "+this);
else throw new IllegalStateException("Ambiguous location for "+this+"; expected one but had "+ll);
if (ll.isEmpty()) {
if (!required) return null;
throw new IllegalStateException("No location available for "+this);
}
if (ll.size()>1) {
throw new IllegalStateException("Ambiguous location for "+this+"; expected one but had "+ll);
}
return Iterables.getOnlyElement(ll);
}

protected boolean isAvailabilityZoneEnabled() {
@@ -48,11 +48,12 @@ public class BasicStartableImpl extends AbstractEntity implements BasicStartable

@Override
public void start(Collection<? extends Location> locations) {
log.info("Starting entity "+this+" at "+locations);
try {
ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);

addLocations(locations);
locations = Locations.getLocationsCheckingAncestors(locations, this);
log.info("Starting entity "+this+" at "+locations);

// essentially does StartableMethods.start(this, locations),
// but optionally filters locations for each child
@@ -38,6 +38,7 @@ public DataEntityImpl() { }

@Override
public void start(Collection<? extends Location> locations) {
addLocations(locations);
connectSensors();
sensors().set(SERVICE_UP, Boolean.TRUE);
}
@@ -110,9 +110,9 @@ public void constructionRequiresThatNewEntityArgumentIsAnEntityFactory() throws
try {
app.createAndManageChild(EntitySpec.create(DynamicCluster.class)
.configure("factory", "error"));
fail();
Asserts.shouldHaveFailedPreviously();
} catch (Exception e) {
if (Exceptions.getFirstThrowableOfType(e, IllegalArgumentException.class) == null) throw e;
Asserts.expectedFailure(e);
}
}

@@ -121,9 +121,9 @@ public void startRequiresThatNewEntityArgumentIsGiven() throws Exception {
DynamicCluster c = app.createAndManageChild(EntitySpec.create(DynamicCluster.class));
try {
c.start(ImmutableList.of(loc));
fail();
Asserts.shouldHaveFailedPreviously();
} catch (Exception e) {
if (Exceptions.getFirstThrowableOfType(e, IllegalStateException.class) == null) throw e;
Asserts.expectedFailureOfType(e, IllegalStateException.class);
}
}

@@ -135,9 +135,9 @@ public void startThenStopThenStartWithNewLocationFails() throws Exception {
cluster.start(ImmutableList.of(loc));
cluster.stop();
cluster.start(ImmutableList.of(loc2));
fail();
Asserts.shouldHaveFailedPreviously();
} catch (Exception e) {
if (Exceptions.getFirstThrowableOfType(e, IllegalStateException.class) == null) throw e;
Asserts.expectedFailureOfType(e, IllegalStateException.class);
}
}

@@ -147,9 +147,9 @@ public void startMethodFailsIfLocationsParameterHasMoreThanOneElement() throws E
.configure(DynamicCluster.MEMBER_SPEC, EntitySpec.create(TestEntity.class)));
try {
cluster.start(ImmutableList.of(loc, loc2));
fail();
Asserts.shouldHaveFailedPreviously();
} catch (Exception e) {
if (Exceptions.getFirstThrowableOfType(e, IllegalArgumentException.class) == null) throw e;
Asserts.expectedFailureContainsIgnoreCase(e, "ambiguous");
}
}

@@ -21,13 +21,15 @@
import static com.google.common.base.Preconditions.checkNotNull;

import java.util.Collection;
import java.util.List;

import org.apache.brooklyn.api.location.Location;
import org.apache.brooklyn.api.mgmt.Task;
import org.apache.brooklyn.core.entity.AbstractEntity;
import org.apache.brooklyn.core.entity.Entities;
import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ComputeServiceIndicatorsFromChildrenAndMembers;
import org.apache.brooklyn.core.location.Locations;
import org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks;
import org.apache.brooklyn.util.collections.QuorumCheck;
import org.apache.brooklyn.util.core.config.ConfigBag;
@@ -68,7 +70,10 @@ public final void restart() {
* Subclasses should override {@link #doStart} to customise behaviour.
*/
@Override
public final void start(final Collection<? extends Location> locations) {
public final void start(Collection<? extends Location> locsO) {
addLocations(locsO);
final Collection<? extends Location> locations = Locations.getLocationsCheckingAncestors(locsO, this);

checkNotNull(locations, "locations");
if (DynamicTasks.getTaskQueuingContext() != null) {
doStart(locations);
@@ -49,7 +49,7 @@ public void start(final Collection<? extends Location> locations) {
try {
// Get an unsubmitted task for starting all the children of this entity in parallel,
// at the same location as this entity.
final TaskAdaptable<?> taskAdaptable = StartableMethods.startingChildren(this);
final TaskAdaptable<?> taskAdaptable = StartableMethods.startingChildren(this, locations);
logger.trace("{}, TaskAdaptable: {}", this, taskAdaptable);

// Submit the task to the ExecutionManager so that they actually get started

0 comments on commit ba8bb7f

Please sign in to comment.