Merge pull request #1107 from mesosphere/wip-1106-drexin

fixes #1106 - Return empty HealthCheck result instead of None, when no r...
d2iq-archive · Mar 27, 2015 · 8e30672 · 8e30672
2 parents 6f1aff3 + 3ec8f59
commit 8e30672
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 13 deletions.
diff --git a/src/main/resources/assets/js/components/TaskHealthComponent.jsx b/src/main/resources/assets/js/components/TaskHealthComponent.jsx
@@ -49,9 +49,9 @@ var TaskHealthComponent = React.createClass({
                   <dd className="text-muted">None</dd> :
                   <dd>{cResult.consecutiveFailures}</dd>}
                 <dt>Alive</dt>
-                {cResult.alive == null ?
-                  <dd>No</dd> :
-                  <dd>Yes</dd>}
+                {cResult.alive ?
+                  <dd>Yes</dd> :
+                  <dd>No</dd>}
               </dl>
             </div>
           );

diff --git a/src/main/resources/assets/js/models/Task.js b/src/main/resources/assets/js/models/Task.js
@@ -27,7 +27,7 @@ var Task = Backbone.Model.extend({
     var healthCheckResults = this.get("healthCheckResults");
     if (healthCheckResults != null) {
       health = healthCheckResults.every(function (hcr) {
-        if (hcr) {
+        if (hcr.firstSuccess) {
           nullResult = false;
           return hcr.alive;
         } else { // might be null

diff --git a/src/main/scala/mesosphere/marathon/api/v2/json/EnrichedTask.scala b/src/main/scala/mesosphere/marathon/api/v2/json/EnrichedTask.scala
@@ -7,5 +7,5 @@ import mesosphere.marathon.state.PathId
 case class EnrichedTask(
   appId: PathId,
   task: MarathonTask,
-  healthCheckResults: Seq[Option[Health]],
+  healthCheckResults: Seq[Health],
   servicePorts: Seq[Int] = Nil)
diff --git a/src/main/scala/mesosphere/marathon/health/HealthCheckActor.scala b/src/main/scala/mesosphere/marathon/health/HealthCheckActor.scala
@@ -118,7 +118,7 @@ class HealthCheckActor(
   }
 
   def receive: Receive = {
-    case GetTaskHealth(taskId) => sender() ! taskHealth.get(taskId)
+    case GetTaskHealth(taskId) => sender() ! taskHealth.getOrElse(taskId, Health(taskId))
 
     case GetAppHealth =>
       sender() ! AppHealth(taskHealth.values.toSeq)

diff --git a/src/main/scala/mesosphere/marathon/health/HealthCheckManager.scala b/src/main/scala/mesosphere/marathon/health/HealthCheckManager.scala
@@ -55,7 +55,7 @@ trait HealthCheckManager {
   /**
     * Returns the health status of the supplied task.
     */
-  def status(appId: PathId, taskId: String): Future[Seq[Option[Health]]]
+  def status(appId: PathId, taskId: String): Future[Seq[Health]]
 
   /**
     * Returns the health status of all tasks of the supplied app.

diff --git a/src/main/scala/mesosphere/marathon/health/MarathonHealthCheckManager.scala b/src/main/scala/mesosphere/marathon/health/MarathonHealthCheckManager.scala
@@ -175,17 +175,17 @@ class MarathonHealthCheckManager @Inject() (
 
   override def status(
     appId: PathId,
-    taskId: String): Future[Seq[Option[Health]]] =
+    taskId: String): Future[Seq[Health]] =
     withReadLock {
       import mesosphere.marathon.health.HealthCheckActor.GetTaskHealth
       implicit val timeout: Timeout = Timeout(2, SECONDS)
 
       val maybeAppVersion: Option[Timestamp] = taskTracker.getVersion(appId, taskId)
 
-      val taskHealth: Seq[Future[Option[Health]]] = maybeAppVersion.map { appVersion =>
+      val taskHealth: Seq[Future[Health]] = maybeAppVersion.map { appVersion =>
         listActive(appId, appVersion).toSeq.collect {
           case ActiveHealthCheck(_, actor) =>
-            (actor ? GetTaskHealth(taskId)).mapTo[Option[Health]]
+            (actor ? GetTaskHealth(taskId)).mapTo[Health]
         }
       }.getOrElse(Nil)
 

diff --git a/src/test/scala/mesosphere/marathon/health/MarathonHealthCheckManagerTest.scala b/src/test/scala/mesosphere/marathon/health/MarathonHealthCheckManagerTest.scala
@@ -122,14 +122,14 @@ class MarathonHealthCheckManagerTest extends MarathonSpec with Logging {
     hcManager.add(appId, version, healthCheck)
 
     val status1 = Await.result(hcManager.status(appId, taskId.getValue), 5.seconds)
-    assert(status1 == Seq(None))
+    assert(status1 == Seq(Health(taskId.getValue)))
 
     // send unhealthy task status
     EventFilter.info(start = "Received health result: [", occurrences = 1).intercept {
       hcManager.update(taskStatus.toBuilder.setHealthy(false).build, version)
     }
 
-    val Seq(Some(health2)) = Await.result(hcManager.status(appId, taskId.getValue), 5.seconds)
+    val Seq(health2) = Await.result(hcManager.status(appId, taskId.getValue), 5.seconds)
     assert(health2.lastFailure.isDefined)
     assert(health2.lastSuccess.isEmpty)
 
@@ -138,7 +138,7 @@ class MarathonHealthCheckManagerTest extends MarathonSpec with Logging {
       hcManager.update(taskStatus.toBuilder.setHealthy(true).build, version)
     }
 
-    val Seq(Some(health3)) = Await.result(hcManager.status(appId, taskId.getValue), 5.seconds)
+    val Seq(health3) = Await.result(hcManager.status(appId, taskId.getValue), 5.seconds)
     assert(health3.lastFailure.isDefined)
     assert(health3.lastSuccess.isDefined)
     assert(health3.lastSuccess > health3.lastFailure)