Repository: incubator-brooklyn Updated Branches: refs/heads/master d2e1487fb -> e5800c9cb
Fix DynamicCluster.replaceMember for MultiLocation - When using replaceMember with a SoftwareProcess, the member had two locations (the MachineProvisioningLocation and the SshMachineLocation). This was causing an exception to be thrown. Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/5a939a2b Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/5a939a2b Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/5a939a2b Branch: refs/heads/master Commit: 5a939a2bb4386a403938d8a50d870fb57239d8df Parents: 217bd01 Author: Aled Sage <[email protected]> Authored: Fri Feb 20 14:23:58 2015 +0000 Committer: Aled Sage <[email protected]> Committed: Fri Feb 20 14:23:58 2015 +0000 ---------------------------------------------------------------------- .../entity/group/DynamicClusterImpl.java | 37 +++--- ...rWithAvailabilityZonesMultiLocationTest.java | 114 +++++++++++++++++++ 2 files changed, 138 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5a939a2b/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 b21f9fc..0b77910 100644 --- a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java +++ b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java @@ -23,6 +23,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -50,6 +51,7 @@ import brooklyn.entity.proxying.EntitySpec; import brooklyn.entity.trait.Startable; import brooklyn.entity.trait.StartableMethods; import brooklyn.location.Location; +import brooklyn.location.MachineProvisioningLocation; import brooklyn.location.basic.Locations; import brooklyn.location.cloud.AvailabilityZoneExtension; import brooklyn.management.Task; @@ -71,6 +73,7 @@ import brooklyn.util.text.Strings; import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -455,23 +458,31 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus if (isAvailabilityZoneEnabled()) { // this member's location could be a machine provisioned by a sub-location, or the actual sub-location List<Location> subLocations = findSubLocations(getLocation()); - Location actualMemberLoc = checkNotNull(Iterables.getOnlyElement(member.getLocations()), "member's location (%s)", member); - Location contenderMemberLoc = actualMemberLoc; + Collection<Location> actualMemberLocs = member.getLocations(); boolean foundMatch = false; - do { - if (subLocations.contains(contenderMemberLoc)) { - memberLoc = contenderMemberLoc; - foundMatch = true; - LOG.debug("In {} replacing member {} ({}), inferred its sub-location is {}", new Object[] {this, memberId, member, memberLoc}); - } - contenderMemberLoc = contenderMemberLoc.getParent(); - } while (!foundMatch && contenderMemberLoc != null); + for (Iterator<Location> iter = actualMemberLocs.iterator(); !foundMatch && iter.hasNext();) { + Location actualMemberLoc = iter.next(); + Location contenderMemberLoc = actualMemberLoc; + do { + if (subLocations.contains(contenderMemberLoc)) { + memberLoc = contenderMemberLoc; + foundMatch = true; + LOG.debug("In {} replacing member {} ({}), inferred its sub-location is {}", new Object[] {this, memberId, member, memberLoc}); + } + contenderMemberLoc = contenderMemberLoc.getParent(); + } while (!foundMatch && contenderMemberLoc != null); + } if (!foundMatch) { - memberLoc = actualMemberLoc; - LOG.warn("In {} replacing member {} ({}), could not find matching sub-location; falling back to its actual location: {}", new Object[] {this, memberId, member, memberLoc}); + if (actualMemberLocs.isEmpty()) { + memberLoc = subLocations.get(0); + LOG.warn("In {} replacing member {} ({}), has no locations; falling back to first availability zone: {}", new Object[] {this, memberId, member, memberLoc}); + } else { + memberLoc = Iterables.tryFind(actualMemberLocs, Predicates.instanceOf(MachineProvisioningLocation.class)).or(Iterables.getFirst(actualMemberLocs, null)); + LOG.warn("In {} replacing member {} ({}), could not find matching sub-location; falling back to its actual location: {}", new Object[] {this, memberId, member, memberLoc}); + } } else if (memberLoc == null) { // impossible to get here, based on logic above! - throw new IllegalStateException("Unexpected condition! cluster="+this+"; member="+member+"; actualMemberLoc="+actualMemberLoc); + throw new IllegalStateException("Unexpected condition! cluster="+this+"; member="+member+"; actualMemberLocs="+actualMemberLocs); } } else { memberLoc = getLocation(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/5a939a2b/software/base/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesMultiLocationTest.java ---------------------------------------------------------------------- diff --git a/software/base/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesMultiLocationTest.java b/software/base/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesMultiLocationTest.java new file mode 100644 index 0000000..9115d62 --- /dev/null +++ b/software/base/src/test/java/brooklyn/entity/group/DynamicClusterWithAvailabilityZonesMultiLocationTest.java @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package brooklyn.entity.group; + +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; + +import java.util.List; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import brooklyn.entity.BrooklynAppUnitTestSupport; +import brooklyn.entity.Entity; +import brooklyn.entity.basic.EmptySoftwareProcess; +import brooklyn.entity.basic.EntityLocal; +import brooklyn.entity.basic.EntityPredicates; +import brooklyn.entity.basic.SoftwareProcess; +import brooklyn.entity.proxying.EntitySpec; +import brooklyn.location.Location; +import brooklyn.location.LocationSpec; +import brooklyn.location.MachineLocation; +import brooklyn.location.MachineProvisioningLocation; +import brooklyn.location.basic.LocalhostMachineProvisioningLocation; +import brooklyn.location.basic.MultiLocation; +import brooklyn.test.Asserts; + +import com.google.common.base.Predicate; +import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; + +/** + * Uses {@link SoftwareProcess}, so test can't be in core project. + * + * Different from {@link DynamicClusterWithAvailabilityZonesTest} in the use of {@link MultiLocation}. + * However, the difference is important: the {@link SoftwareProcess} entity has two locations + * (the {@link MachineProvisioningLocation} and the {@link MachineLocation}, which was perviously + * causing a failure - now fixed and tested here. + */ +public class DynamicClusterWithAvailabilityZonesMultiLocationTest extends BrooklynAppUnitTestSupport { + + private DynamicCluster cluster; + + private LocalhostMachineProvisioningLocation subLoc1; + private LocalhostMachineProvisioningLocation subLoc2; + private MultiLocation<?> multiLoc; + + @BeforeMethod(alwaysRun=true) + @Override + public void setUp() throws Exception { + super.setUp(); + cluster = app.createAndManageChild(EntitySpec.create(DynamicCluster.class) + .configure(DynamicCluster.ENABLE_AVAILABILITY_ZONES, true) + .configure(DynamicCluster.INITIAL_SIZE, 0) + .configure(DynamicCluster.MEMBER_SPEC, EntitySpec.create(EmptySoftwareProcess.class))); + + subLoc1 = app.newLocalhostProvisioningLocation(ImmutableMap.of("displayName", "loc1")); + subLoc2 = app.newLocalhostProvisioningLocation(ImmutableMap.of("displayName", "loc2")); + multiLoc = mgmt.getLocationManager().createLocation(LocationSpec.create(MultiLocation.class) + .configure(MultiLocation.SUB_LOCATIONS, ImmutableList.<MachineProvisioningLocation<?>>of(subLoc1, subLoc2))); + } + + @Test + public void testReplacesEntityInSameZone() throws Exception { + ((EntityLocal)cluster).config().set(DynamicCluster.ENABLE_AVAILABILITY_ZONES, true); + cluster.start(ImmutableList.of(multiLoc)); + + cluster.resize(4); + List<String> locsUsed = getLocationNames(getLocationsOf(cluster.getMembers(), Predicates.instanceOf(MachineProvisioningLocation.class))); + Asserts.assertEqualsIgnoringOrder(locsUsed, ImmutableList.of("loc1", "loc1", "loc2", "loc2")); + + String idToRemove = Iterables.getFirst(cluster.getMembers(), null).getId(); + String idAdded = cluster.replaceMember(idToRemove); + locsUsed = getLocationNames(getLocationsOf(cluster.getMembers(), Predicates.instanceOf(MachineProvisioningLocation.class))); + Asserts.assertEqualsIgnoringOrder(locsUsed, ImmutableList.of("loc1", "loc1", "loc2", "loc2")); + assertNull(Iterables.find(cluster.getMembers(), EntityPredicates.idEqualTo(idToRemove), null)); + assertNotNull(Iterables.find(cluster.getMembers(), EntityPredicates.idEqualTo(idAdded), null)); + } + + protected List<Location> getLocationsOf(Iterable<? extends Entity> entities, Predicate<? super Location> filter) { + List<Location> result = Lists.newArrayList(); + for (Entity entity : entities) { + Iterables.addAll(result, Iterables.filter(entity.getLocations(), filter)); + } + return result; + } + + protected List<String> getLocationNames(Iterable<? extends Location> locs) { + List<String> result = Lists.newArrayList(); + for (Location subLoc : locs) { + result.add(subLoc.getDisplayName()); + } + return result; + } +}
