You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/05/29 23:07:48 UTC
git commit: Fix logic error when populating task resources.
Repository: incubator-aurora
Updated Branches:
refs/heads/master 207928b22 -> ec563150e
Fix logic error when populating task resources.
Bugs closed: AURORA-483
Reviewed at https://reviews.apache.org/r/22007/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/ec563150
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/ec563150
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/ec563150
Branch: refs/heads/master
Commit: ec563150e63f1936d6e2b811aaa22ecf1e469a4a
Parents: 207928b
Author: Bill Farner <wf...@apache.org>
Authored: Thu May 29 14:05:16 2014 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Thu May 29 14:05:16 2014 -0700
----------------------------------------------------------------------
.../aurora/scheduler/MesosTaskFactory.java | 13 ++-
.../scheduler/configuration/Resources.java | 8 +-
.../scheduler/MesosTaskFactoryImplTest.java | 64 +++++++++------
.../scheduler/configuration/ResourcesTest.java | 85 ++++++++++++++++++++
4 files changed, 136 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ec563150/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java b/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
index f48ca51..979b4bb 100644
--- a/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
+++ b/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
@@ -20,7 +20,6 @@ import java.util.logging.Logger;
import javax.inject.Inject;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.protobuf.ByteString;
@@ -119,13 +118,11 @@ public interface MesosTaskFactory {
}
ITaskConfig config = task.getTask();
- List<Resource> resources;
- if (task.isSetAssignedPorts()) {
- resources = Resources.from(config)
- .toResourceList(ImmutableSet.copyOf(task.getAssignedPorts().values()));
- } else {
- resources = ImmutableList.of();
- }
+ // TODO(wfarner): Re-evaluate if/why we need to continue handling unset assignedPorts field.
+ List<Resource> resources = Resources.from(config)
+ .toResourceList(task.isSetAssignedPorts()
+ ? ImmutableSet.copyOf(task.getAssignedPorts().values())
+ : ImmutableSet.<Integer>of());
if (LOG.isLoggable(Level.FINE)) {
LOG.fine("Setting task resources to "
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ec563150/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
index 9ee2a85..1d557c7 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java
@@ -17,6 +17,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Set;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.base.Objects;
import com.google.common.base.Predicate;
@@ -112,8 +113,8 @@ public class Resources {
.add(Resources.makeMesosResource(CPUS, numCpus))
.add(Resources.makeMesosResource(DISK_MB, disk.as(Data.MB)))
.add(Resources.makeMesosResource(RAM_MB, ram.as(Data.MB)));
- if (selectedPorts.isEmpty()) {
- resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts));
+ if (!selectedPorts.isEmpty()) {
+ resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts));
}
return resourceBuilder.build();
@@ -312,7 +313,8 @@ public class Resources {
* @param values Values to translate into ranges.
* @return A mesos ranges resource.
*/
- static Resource makeMesosRangeResource(String name, Set<Integer> values) {
+ @VisibleForTesting
+ public static Resource makeMesosRangeResource(String name, Set<Integer> values) {
return Resource.newBuilder()
.setName(name)
.setType(Type.RANGES)
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ec563150/src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java b/src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
index fa601e1..e969747 100644
--- a/src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
@@ -13,6 +13,8 @@
*/
package org.apache.aurora.scheduler;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.twitter.common.quantity.Data;
import org.apache.aurora.gen.AssignedTask;
@@ -20,15 +22,13 @@ import org.apache.aurora.gen.Identity;
import org.apache.aurora.gen.TaskConfig;
import org.apache.aurora.scheduler.MesosTaskFactory.ExecutorConfig;
import org.apache.aurora.scheduler.MesosTaskFactory.MesosTaskFactoryImpl;
+import org.apache.aurora.scheduler.configuration.Resources;
import org.apache.aurora.scheduler.storage.entities.IAssignedTask;
import org.apache.mesos.Protos.CommandInfo;
import org.apache.mesos.Protos.CommandInfo.URI;
import org.apache.mesos.Protos.ExecutorInfo;
-import org.apache.mesos.Protos.Resource;
import org.apache.mesos.Protos.SlaveID;
import org.apache.mesos.Protos.TaskInfo;
-import org.apache.mesos.Protos.Value.Scalar;
-import org.apache.mesos.Protos.Value.Type;
import org.junit.Before;
import org.junit.Test;
@@ -40,13 +40,15 @@ public class MesosTaskFactoryImplTest {
private static final IAssignedTask TASK = IAssignedTask.build(new AssignedTask()
.setInstanceId(2)
.setTaskId("task-id")
+ .setAssignedPorts(ImmutableMap.of("http", 80))
.setTask(new TaskConfig()
.setOwner(new Identity("role", "user"))
.setEnvironment("environment")
.setJobName("job-name")
.setDiskMb(10)
.setRamMb(100)
- .setNumCpus(5)));
+ .setNumCpus(5)
+ .setRequestedPorts(ImmutableSet.of("http"))));
private static final SlaveID SLAVE = SlaveID.newBuilder().setValue("slave-id").build();
private MesosTaskFactory taskFactory;
@@ -56,28 +58,44 @@ public class MesosTaskFactoryImplTest {
taskFactory = new MesosTaskFactoryImpl(new ExecutorConfig(EXECUTOR_PATH));
}
+ private static final ExecutorInfo DEFAULT_EXECUTOR = ExecutorInfo.newBuilder()
+ .setExecutorId(MesosTaskFactoryImpl.getExecutorId(TASK.getTaskId()))
+ .setName(MesosTaskFactoryImpl.EXECUTOR_NAME)
+ .setSource(MesosTaskFactoryImpl.getInstanceSourceName(TASK.getTask(), TASK.getInstanceId()))
+ .addResources(Resources.makeMesosResource(Resources.CPUS, ResourceSlot.EXECUTOR_CPUS))
+ .addResources(
+ Resources.makeMesosResource(Resources.RAM_MB, ResourceSlot.EXECUTOR_RAM.as(Data.MB)))
+ .setCommand(CommandInfo.newBuilder()
+ .setValue("./executor.sh")
+ .addUris(URI.newBuilder().setValue(EXECUTOR_PATH).setExecutable(true)))
+ .build();
+
@Test
public void testExecutorInfoUnchanged() {
- // Tests against regression of MESOS-911.
TaskInfo task = taskFactory.createFrom(TASK, SLAVE);
+ assertEquals(DEFAULT_EXECUTOR, task.getExecutor());
+ assertEquals(ImmutableSet.of(
+ Resources.makeMesosResource(Resources.CPUS, TASK.getTask().getNumCpus()),
+ Resources.makeMesosResource(Resources.RAM_MB, TASK.getTask().getRamMb()),
+ Resources.makeMesosResource(Resources.DISK_MB, TASK.getTask().getDiskMb()),
+ Resources.makeMesosRangeResource(
+ Resources.PORTS,
+ ImmutableSet.copyOf(TASK.getAssignedPorts().values()))
+ ),
+ ImmutableSet.copyOf(task.getResourcesList()));
+ }
- ExecutorInfo expected = ExecutorInfo.newBuilder()
- .setExecutorId(MesosTaskFactoryImpl.getExecutorId(TASK.getTaskId()))
- .setName(MesosTaskFactoryImpl.EXECUTOR_NAME)
- .setSource(MesosTaskFactoryImpl.getInstanceSourceName(TASK.getTask(), TASK.getInstanceId()))
- .addResources(Resource.newBuilder()
- .setName("cpus")
- .setType(Type.SCALAR)
- .setScalar(Scalar.newBuilder().setValue(ResourceSlot.EXECUTOR_CPUS)))
- .addResources(Resource.newBuilder()
- .setName("mem")
- .setType(Type.SCALAR)
- .setScalar(Scalar.newBuilder().setValue(ResourceSlot.EXECUTOR_RAM.as(Data.MB))))
- .setCommand(CommandInfo.newBuilder()
- .setValue("./executor.sh")
- .addUris(URI.newBuilder().setValue(EXECUTOR_PATH).setExecutable(true)))
- .build();
-
- assertEquals(expected, task.getExecutor());
+ @Test
+ public void testCreateFromPortsUnset() {
+ AssignedTask assignedTask = TASK.newBuilder();
+ assignedTask.unsetAssignedPorts();
+ TaskInfo task = taskFactory.createFrom(IAssignedTask.build(assignedTask), SLAVE);
+ assertEquals(DEFAULT_EXECUTOR, task.getExecutor());
+ assertEquals(ImmutableSet.of(
+ Resources.makeMesosResource(Resources.CPUS, TASK.getTask().getNumCpus()),
+ Resources.makeMesosResource(Resources.RAM_MB, TASK.getTask().getRamMb()),
+ Resources.makeMesosResource(Resources.DISK_MB, TASK.getTask().getDiskMb())
+ ),
+ ImmutableSet.copyOf(task.getResourcesList()));
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ec563150/src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java b/src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java
index 08788a2..d6febb8 100644
--- a/src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java
@@ -22,7 +22,9 @@ import com.twitter.common.collections.Pair;
import com.twitter.common.quantity.Amount;
import com.twitter.common.quantity.Data;
+import org.apache.aurora.gen.TaskConfig;
import org.apache.aurora.scheduler.configuration.Resources.InsufficientResourcesException;
+import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
import org.apache.mesos.Protos;
import org.apache.mesos.Protos.Resource;
import org.apache.mesos.Protos.Value.Range;
@@ -31,6 +33,9 @@ import org.apache.mesos.Protos.Value.Type;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class ResourcesTest {
@@ -76,6 +81,12 @@ public class ResourcesTest {
}
}
+ @Test
+ public void testGetNoPorts() {
+ Resource portsResource = createPortRange(Pair.of(1, 5));
+ assertEquals(ImmutableSet.<Integer>of(), Resources.getPorts(createOffer(portsResource), 0));
+ }
+
private static final Resources NEGATIVE_ONE =
new Resources(-1.0, Amount.of(-1L, Data.MB), Amount.of(-1L, Data.MB), -1);
private static final Resources ONE =
@@ -160,6 +171,80 @@ public class ResourcesTest {
ImmutableSet.of(8, 2, 4, 5, 7, 9, 1));
}
+ private static final ITaskConfig TASK = ITaskConfig.build(new TaskConfig()
+ .setNumCpus(1.0)
+ .setRamMb(1024)
+ .setDiskMb(2048)
+ .setRequestedPorts(ImmutableSet.of("http", "debug")));
+
+ private static void assertLeftIsLarger(Resources left, Resources right) {
+ assertTrue(left.greaterThanOrEqual(right));
+ assertFalse(right.greaterThanOrEqual(left));
+ }
+
+ @Test
+ public void testAccessors() {
+ Resources resources = Resources.from(TASK);
+ assertEquals(TASK.getNumCpus(), resources.getNumCpus(), 1e-9);
+ assertEquals(Amount.of(TASK.getRamMb(), Data.MB), resources.getRam());
+ assertEquals(Amount.of(TASK.getDiskMb(), Data.MB), resources.getDisk());
+ assertEquals(TASK.getRequestedPorts().size(), resources.getNumPorts());
+
+ assertTrue(resources.greaterThanOrEqual(resources));
+ assertLeftIsLarger(
+ resources,
+ Resources.from(ITaskConfig.build(TASK.newBuilder().setNumCpus(0.5))));
+ assertLeftIsLarger(
+ resources,
+ Resources.from(ITaskConfig.build(TASK.newBuilder().setRamMb(512))));
+ assertLeftIsLarger(
+ resources,
+ Resources.from(ITaskConfig.build(TASK.newBuilder().setDiskMb(1024))));
+ assertLeftIsLarger(
+ resources,
+ Resources.from(
+ ITaskConfig.build(TASK.newBuilder().setRequestedPorts(ImmutableSet.of("http")))));
+ }
+
+ @Test
+ public void testToResourceList() {
+ Resources resources = Resources.from(TASK);
+ Set<Integer> ports = ImmutableSet.of(80, 443);
+ assertEquals(
+ ImmutableSet.of(
+ Resources.makeMesosResource(Resources.CPUS, TASK.getNumCpus()),
+ Resources.makeMesosResource(Resources.RAM_MB, TASK.getRamMb()),
+ Resources.makeMesosResource(Resources.DISK_MB, TASK.getDiskMb()),
+ Resources.makeMesosRangeResource(Resources.PORTS, ports)),
+ ImmutableSet.copyOf(resources.toResourceList(ports)));
+ }
+
+ @Test
+ public void testToResourceListInversible() {
+ Resources resources = Resources.from(TASK);
+ Resources inverse = Resources.from(resources.toResourceList(ImmutableSet.of(80, 443)));
+ assertEquals(resources, inverse);
+ assertEquals(resources.hashCode(), inverse.hashCode());
+ }
+
+ @Test
+ public void testEqualsBadType() {
+ Resources resources = Resources.from(TASK);
+ assertNotEquals(resources, "Hello");
+ assertNotEquals(resources, null);
+ }
+
+ @Test
+ public void testToResourceListNoPorts() {
+ Resources resources = Resources.from(TASK);
+ assertEquals(
+ ImmutableSet.of(
+ Resources.makeMesosResource(Resources.CPUS, TASK.getNumCpus()),
+ Resources.makeMesosResource(Resources.RAM_MB, TASK.getRamMb()),
+ Resources.makeMesosResource(Resources.DISK_MB, TASK.getDiskMb())),
+ ImmutableSet.copyOf(resources.toResourceList(ImmutableSet.<Integer>of())));
+ }
+
private void expectRanges(Set<Pair<Long, Long>> expected, Set<Integer> values) {
Resource resource = Resources.makeMesosRangeResource(NAME, values);
assertEquals(Type.RANGES, resource.getType());