aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject git commit: Removing client HTTP health checks.
Date Tue, 29 Apr 2014 16:08:01 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 3074ef712 -> a42199ade


Removing client HTTP health checks.

Bugs closed: AURORA-361

Reviewed at https://reviews.apache.org/r/20726/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/a42199ad
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/a42199ad
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/a42199ad

Branch: refs/heads/master
Commit: a42199adea6e569bd0ef84311917f786c0c4f342
Parents: 3074ef7
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Tue Apr 29 08:59:03 2014 -0700
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Tue Apr 29 08:59:03 2014 -0700

----------------------------------------------------------------------
 .../apache/aurora/client/api/health_check.py    |  60 ---------
 .../aurora/client/api/instance_watcher.py       |   4 +-
 .../aurora/client/api/test_health_check.py      | 125 ++-----------------
 .../apache/aurora/client/cli/test_restart.py    |  10 +-
 .../apache/aurora/client/cli/test_update.py     |   6 +-
 .../aurora/client/commands/test_restart.py      |  10 +-
 .../aurora/client/commands/test_update.py       |   6 +-
 7 files changed, 30 insertions(+), 191 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/main/python/apache/aurora/client/api/health_check.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/api/health_check.py b/src/main/python/apache/aurora/client/api/health_check.py
index e3bbf71..e503344 100644
--- a/src/main/python/apache/aurora/client/api/health_check.py
+++ b/src/main/python/apache/aurora/client/api/health_check.py
@@ -16,8 +16,6 @@
 
 from abc import abstractmethod
 
-from apache.aurora.common.http_signaler import HttpSignaler
-
 from gen.apache.aurora.api.ttypes import ScheduleStatus
 
 from twitter.common import log
@@ -79,61 +77,3 @@ class StatusHealthCheck(HealthCheck):
         return Retriable.alive()
     else:
       return Retriable.dead()
-
-
-class HttpHealthCheck(HealthCheck):
-  """Verifies the health of a task based on http health checks. A new http signaler is created
for a
-  task iff,
-    1. The instance id of the task is unknown.
-    2. The instance id is known but the (host, port) is different for the task.
-  """
-  def __init__(self, http_signaler_factory=HttpSignaler):
-    self._http_signalers = {}
-    self._http_signaler_factory = http_signaler_factory
-
-  def health(self, task):
-    assigned_task = task.assignedTask
-    instance_id = assigned_task.instanceId
-    host_port = (assigned_task.slaveHost, assigned_task.assignedPorts['health'])
-    http_signaler = None
-    if instance_id in self._http_signalers:
-      checker_host_port, signaler = self._http_signalers.get(instance_id)
-      # Only reuse the health checker if it is for the same destination.
-      if checker_host_port == host_port:
-        http_signaler = signaler
-    if not http_signaler:
-      http_signaler = self._http_signaler_factory(host_port[1], host_port[0])
-      self._http_signalers[instance_id] = (host_port, http_signaler)
-    return Retriable.alive() if http_signaler.health()[0] else Retriable.dead()
-
-
-class ChainedHealthCheck(HealthCheck):
-  """Delegates health checks to configured health checkers."""
-  def __init__(self, *health_checkers):
-    self._health_checkers = health_checkers
-
-  def health(self, task):
-    for checker in self._health_checkers:
-      healthy, retriable = checker.health(task)
-      if not healthy:
-        return (healthy, retriable)
-    return Retriable.alive()
-
-
-class InstanceWatcherHealthCheck(HealthCheck):
-  """Makes the decision: if a task has health port, then use Status+HTTP, else use only status.
-     Caveat: Only works if either ALL tasks have a health port or none of them have a health
port.
-  """
-  # TODO(atollenaere) Refactor the code to use the executor StatusChecker/HealthChecker instead
-
-  def __init__(self, http_signaler_factory=HttpSignaler):
-    self._has_health_port = False
-    self._health_checker = StatusHealthCheck()
-    self._http_signaler_factory = http_signaler_factory
-
-  def health(self, task):
-    if not self._has_health_port and 'health' in task.assignedTask.assignedPorts:
-      log.debug('Health port detected, enabling HTTP checks')
-      self._health_checker = ChainedHealthCheck(self._health_checker, HttpHealthCheck(self._http_signaler_factory))
-      self._has_health_port = True
-    return self._health_checker.health(task)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/main/python/apache/aurora/client/api/instance_watcher.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/api/instance_watcher.py b/src/main/python/apache/aurora/client/api/instance_watcher.py
index 5308683..13a41d4 100644
--- a/src/main/python/apache/aurora/client/api/instance_watcher.py
+++ b/src/main/python/apache/aurora/client/api/instance_watcher.py
@@ -23,7 +23,7 @@ from gen.apache.aurora.api.ttypes import (
   TaskQuery,
 )
 
-from .health_check import InstanceWatcherHealthCheck
+from .health_check import StatusHealthCheck
 
 from twitter.common import log
 
@@ -68,7 +68,7 @@ class InstanceWatcher(object):
     """
     log.info('Watching instances: %s' % instance_ids)
     instance_ids = set(instance_ids)
-    health_check = health_check or InstanceWatcherHealthCheck()
+    health_check = health_check or StatusHealthCheck()
     now = self._clock.time()
     expected_healthy_by = now + self._restart_threshold
     max_time = now + self._restart_threshold + self._watch_secs

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/test/python/apache/aurora/client/api/test_health_check.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/api/test_health_check.py b/src/test/python/apache/aurora/client/api/test_health_check.py
index f59fe42..946b905 100644
--- a/src/test/python/apache/aurora/client/api/test_health_check.py
+++ b/src/test/python/apache/aurora/client/api/test_health_check.py
@@ -16,19 +16,19 @@
 
 import unittest
 
-from apache.aurora.common.http_signaler import HttpSignaler
 from apache.aurora.client.api.health_check import (
-  ChainedHealthCheck,
-  HealthCheck,
-  HealthStatus,
-  HttpHealthCheck,
-  NotRetriable,
-  Retriable,
-  InstanceWatcherHealthCheck,
-  StatusHealthCheck
+    HealthCheck,
+    NotRetriable,
+    Retriable,
+    StatusHealthCheck
 )
 
-from gen.apache.aurora.api.ttypes import *
+from gen.apache.aurora.api.ttypes import (
+    AssignedTask,
+    ScheduledTask,
+    ScheduleStatus,
+    TaskConfig
+)
 
 import mox
 import pytest
@@ -40,41 +40,23 @@ FAILED = ScheduleStatus.FAILED
 
 
 class HealthCheckTest(unittest.TestCase):
-  HOST = 'host-a'
-  HEALTH_PORT = 33333
 
   def setUp(self):
-    self._http_signaler = mox.MockObject(HttpSignaler)
     self._status_health_check = StatusHealthCheck()
-    self._http_health_check = HttpHealthCheck(self._http_signaler)
-    self._smart_health_check = InstanceWatcherHealthCheck(self._http_signaler)
     self._health_check_a = mox.MockObject(HealthCheck)
     self._health_check_b = mox.MockObject(HealthCheck)
 
-  def create_task(self, instance_id, task_id, status=RUNNING, host=HOST, port=HEALTH_PORT):
-    ports = {}
-    if port:
-      ports['health'] = port
+  def create_task(self, instance_id, task_id, status=RUNNING):
     assigned_task = AssignedTask(taskId=task_id,
                                  instanceId=instance_id,
-                                 task=TaskConfig(),
-                                 slaveHost=host,
-                                 assignedPorts=ports)
+                                 task=TaskConfig())
     return ScheduledTask(assignedTask=assigned_task, status=status)
 
-  def expect_http_signaler_creation(self, host=HOST, port=HEALTH_PORT):
-    self._http_signaler(port, host).AndReturn(self._http_signaler)
-
-  def expect_health_check(self, status=True):
-    self._http_signaler.health().AndReturn((status, 'reason'))
-
   def replay(self):
-    mox.Replay(self._http_signaler)
     mox.Replay(self._health_check_a)
     mox.Replay(self._health_check_b)
 
   def verify(self):
-    mox.Verify(self._http_signaler)
     mox.Verify(self._health_check_a)
     mox.Verify(self._health_check_b)
 
@@ -99,92 +81,9 @@ class HealthCheckTest(unittest.TestCase):
     assert self._status_health_check.health(task_a) == Retriable.alive()
     assert self._status_health_check.health(task_b) == NotRetriable.dead()
 
-  def test_http_health_check(self):
-    """Verify successful and failed http health checks for a task"""
-    task_a = self.create_task(0, 'a')
-    self.expect_http_signaler_creation()
-    self.expect_health_check()
-    self.expect_health_check(status=False)
-    self.replay()
-    assert self._http_health_check.health(task_a) == Retriable.alive()
-    assert self._http_health_check.health(task_a) == Retriable.dead()
-    self.verify()
-
-  def test_unmatched_host_port(self):
-    """Test if an instance with a modified a (host, port) triggers a new http health checker
creation"""
-    instance_id = 0
-    task_a = self.create_task(instance_id, 'a')
-    self.expect_http_signaler_creation()
-    self.expect_health_check()
-    task_b = self.create_task(instance_id, 'b', host='host-b', port=44444)
-    self.expect_http_signaler_creation(host='host-b', port=44444)
-    self.expect_health_check()
-    self.replay()
-    assert self._http_health_check.health(task_a) == Retriable.alive()
-    assert self._http_health_check.health(task_b) == Retriable.alive()
-    self.verify()
-
-  def test_simple_chained_health_check(self):
-    """Verify successful health check"""
-    task = self.create_task(0, 'a')
-    self._health_check_a.health(task).AndReturn(Retriable.alive())
-    self._health_check_b.health(task).AndReturn(Retriable.alive())
-    health_check = ChainedHealthCheck(self._health_check_a, self._health_check_b)
-    self.replay()
-    assert health_check.health(task) == Retriable.alive()
-    self.verify()
-
-  def test_failed_primary_health_check(self):
-    """Verifies that a secondary health check is not invoked when a primary check fails"""
-    task = self.create_task(0, 'a')
-    self._health_check_a.health(task).AndReturn(NotRetriable.dead())
-    self._health_check_a.health(task).AndReturn(Retriable.dead())
-    health_check = ChainedHealthCheck(self._health_check_a, self._health_check_b)
-    self.replay()
-    assert health_check.health(task) == NotRetriable.dead()
-    assert health_check.health(task) == Retriable.dead()
-    self.verify()
-
-  def test_failed_secondary_health_check(self):
-    """Verifies that a secondary health check is not invoked when a primary check fails"""
-    task = self.create_task(0, 'a')
-    self._health_check_a.health(task).AndReturn(Retriable.alive())
-    self._health_check_b.health(task).AndReturn(Retriable.dead())
-    health_check = ChainedHealthCheck(self._health_check_a, self._health_check_b)
-    self.replay()
-    assert health_check.health(task) == Retriable.dead()
-    self.verify()
-
   def test_health_statuses(self):
     """Verfies that the health status tuple (health, retry_status) are as expected"""
     assert Retriable.alive() == (True, True)
     assert Retriable.dead() == (False, True)
     assert NotRetriable.alive() == (True, False)
     assert NotRetriable.dead() == (False, False)
-
-  def test_instancewatcher_health_check(self):
-    """Verifies that if the task has no health port, only status check is performed"""
-    task = self.create_task(0, 'a', port=None)
-    self.replay()
-    assert self._smart_health_check.health(task) == Retriable.alive()
-    self.verify()
-
-  def test_instancewatcher_http_health_check(self):
-    """Verifies that http health check is performed if the task has a health port"""
-    task = self.create_task(0, 'a')
-    self.expect_http_signaler_creation()
-    self.expect_health_check(status=False)
-    self.replay()
-    assert self._smart_health_check.health(task) == Retriable.dead()
-    self.verify()
-
-  def test_instancewatcher_http_health_check_one_http_signaler(self):
-    """Verifies that upon multiple http health checks only one HttpHealthChecker is created"""
-    task = self.create_task(0, 'a')
-    self.expect_http_signaler_creation()
-    self.expect_health_check()
-    self.expect_health_check()
-    self.replay()
-    assert self._smart_health_check.health(task) == Retriable.alive()
-    assert self._smart_health_check.health(task) == Retriable.alive()
-    self.verify()

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/test/python/apache/aurora/client/cli/test_restart.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_restart.py b/src/test/python/apache/aurora/client/cli/test_restart.py
index 43412c4..9b7a2c6 100644
--- a/src/test/python/apache/aurora/client/cli/test_restart.py
+++ b/src/test/python/apache/aurora/client/cli/test_restart.py
@@ -16,7 +16,7 @@
 import contextlib
 import functools
 
-from apache.aurora.client.api.health_check import InstanceWatcherHealthCheck, Retriable
+from apache.aurora.client.api.health_check import StatusHealthCheck, Retriable
 from apache.aurora.client.cli import EXIT_API_ERROR
 from apache.aurora.client.cli.client import AuroraCommandLine
 from apache.aurora.client.cli.util import AuroraClientCommandTest
@@ -57,7 +57,7 @@ class TestRestartCommand(AuroraClientCommandTest):
 
   @classmethod
   def setup_health_checks(cls, mock_api):
-    mock_health_check = Mock(spec=InstanceWatcherHealthCheck)
+    mock_health_check = Mock(spec=StatusHealthCheck)
     mock_health_check.health.return_value = Retriable.alive()
     return mock_health_check
 
@@ -69,7 +69,7 @@ class TestRestartCommand(AuroraClientCommandTest):
     with contextlib.nested(
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
         patch('time.sleep', return_value=None)
@@ -100,7 +100,7 @@ class TestRestartCommand(AuroraClientCommandTest):
     with contextlib.nested(
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
         patch('time.sleep', return_value=None)):
@@ -123,7 +123,7 @@ class TestRestartCommand(AuroraClientCommandTest):
     with contextlib.nested(
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
         patch('time.sleep', return_value=None)):

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/test/python/apache/aurora/client/cli/test_update.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_update.py b/src/test/python/apache/aurora/client/cli/test_update.py
index e3e4332..cf077e8 100644
--- a/src/test/python/apache/aurora/client/cli/test_update.py
+++ b/src/test/python/apache/aurora/client/cli/test_update.py
@@ -19,7 +19,7 @@ import functools
 from twitter.common.contextutil import temporary_file
 
 from apache.aurora.client.api.updater import Updater
-from apache.aurora.client.api.health_check import InstanceWatcherHealthCheck, Retriable
+from apache.aurora.client.api.health_check import StatusHealthCheck, Retriable
 from apache.aurora.client.api.quota_check import QuotaCheck
 from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION
 from apache.aurora.client.cli.client import AuroraCommandLine
@@ -175,7 +175,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
 
   @classmethod
   def setup_health_checks(cls, mock_api):
-    mock_health_check = Mock(spec=InstanceWatcherHealthCheck)
+    mock_health_check = Mock(spec=StatusHealthCheck)
     mock_health_check.health.return_value = Retriable.alive()
     return mock_health_check
 
@@ -198,7 +198,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
     with contextlib.nested(
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('apache.aurora.client.api.quota_check.QuotaCheck', return_value=mock_quota_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/test/python/apache/aurora/client/commands/test_restart.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/commands/test_restart.py b/src/test/python/apache/aurora/client/commands/test_restart.py
index 526c501..9af6334 100644
--- a/src/test/python/apache/aurora/client/commands/test_restart.py
+++ b/src/test/python/apache/aurora/client/commands/test_restart.py
@@ -19,7 +19,7 @@ import functools
 
 from apache.aurora.client.commands.core import restart
 from apache.aurora.client.commands.util import AuroraClientCommandTest
-from apache.aurora.client.api.health_check import InstanceWatcherHealthCheck, Retriable
+from apache.aurora.client.api.health_check import StatusHealthCheck, Retriable
 from twitter.common.contextutil import temporary_file
 
 from gen.apache.aurora.api.ttypes import (
@@ -97,7 +97,7 @@ class TestRestartCommand(AuroraClientCommandTest):
 
   @classmethod
   def setup_health_checks(cls, mock_api):
-    mock_health_check = Mock(spec=InstanceWatcherHealthCheck)
+    mock_health_check = Mock(spec=StatusHealthCheck)
     mock_health_check.health.return_value = Retriable.alive()
     return mock_health_check
 
@@ -111,7 +111,7 @@ class TestRestartCommand(AuroraClientCommandTest):
         patch('twitter.common.app.get_options', return_value=mock_options),
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
         patch('time.sleep', return_value=None)
@@ -145,7 +145,7 @@ class TestRestartCommand(AuroraClientCommandTest):
         patch('twitter.common.app.get_options', return_value=mock_options),
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
         patch('time.sleep', return_value=None)
@@ -171,7 +171,7 @@ class TestRestartCommand(AuroraClientCommandTest):
         patch('twitter.common.app.get_options', return_value=mock_options),
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
         patch('time.sleep', return_value=None)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a42199ad/src/test/python/apache/aurora/client/commands/test_update.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/commands/test_update.py b/src/test/python/apache/aurora/client/commands/test_update.py
index f907837..6e145db 100644
--- a/src/test/python/apache/aurora/client/commands/test_update.py
+++ b/src/test/python/apache/aurora/client/commands/test_update.py
@@ -23,7 +23,7 @@ from apache.aurora.common.clusters import Clusters
 from apache.aurora.client.commands.core import update
 from apache.aurora.client.commands.util import AuroraClientCommandTest
 from apache.aurora.client.api.updater import Updater
-from apache.aurora.client.api.health_check import InstanceWatcherHealthCheck, Retriable
+from apache.aurora.client.api.health_check import StatusHealthCheck, Retriable
 from apache.aurora.client.api.quota_check import QuotaCheck
 from apache.aurora.client.hooks.hooked_api import HookedAuroraClientAPI
 from apache.aurora.config import AuroraConfig
@@ -198,7 +198,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
 
   @classmethod
   def setup_health_checks(cls, mock_api):
-    mock_health_check = Mock(spec=InstanceWatcherHealthCheck)
+    mock_health_check = Mock(spec=StatusHealthCheck)
     mock_health_check.health.return_value = Retriable.alive()
     return mock_health_check
 
@@ -219,7 +219,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
         patch('twitter.common.app.get_options', return_value=mock_options),
         patch('apache.aurora.client.api.SchedulerProxy', return_value=mock_scheduler_proxy),
         patch('apache.aurora.client.factory.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.instance_watcher.InstanceWatcherHealthCheck',
+        patch('apache.aurora.client.api.instance_watcher.StatusHealthCheck',
             return_value=mock_health_check),
         patch('apache.aurora.client.api.quota_check.QuotaCheck', return_value=mock_quota_check),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),


Mime
View raw message