aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject git commit: Fix logic error when populating task resources.
Date Thu, 29 May 2014 21:07:48 GMT
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 <wfarner@apache.org>
Authored: Thu May 29 14:05:16 2014 -0700
Committer: Bill Farner <wfarner@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());


Mime
View raw message