aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mchucarr...@apache.org
Subject git commit: Bugfix: restart doesn't notify user about invalid max_total_failures option.
Date Tue, 13 May 2014 19:24:53 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 8db874e37 -> d931a314d


Bugfix: restart doesn't notify user about invalid max_total_failures option.

Bugs closed: aurora-399

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


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

Branch: refs/heads/master
Commit: d931a314d1c900c08b3f07a91b739b99d7d0c8f1
Parents: 8db874e
Author: Mark Chu-Carroll <mchucarroll@twopensource.com>
Authored: Tue May 13 15:21:15 2014 -0400
Committer: Mark Chu-Carroll <mchucarroll@twitter.com>
Committed: Tue May 13 15:21:15 2014 -0400

----------------------------------------------------------------------
 .../python/apache/aurora/client/cli/jobs.py     |  9 ++++++
 .../apache/aurora/client/commands/core.py       |  4 +++
 .../apache/aurora/client/cli/test_restart.py    | 13 ++++-----
 .../aurora/client/commands/test_restart.py      | 30 ++++++++++++++++++++
 4 files changed, 49 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/d931a314/src/main/python/apache/aurora/client/cli/jobs.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/jobs.py b/src/main/python/apache/aurora/client/cli/jobs.py
index 6fc08da..c318926 100644
--- a/src/main/python/apache/aurora/client/cli/jobs.py
+++ b/src/main/python/apache/aurora/client/cli/jobs.py
@@ -457,6 +457,15 @@ class RestartCommand(Verb):
 Restarts are fully controlled client-side, so aborting halts the restart."""
 
   def execute(self, context):
+    # Check for negative max_total_failures option - negative is an error.
+    # for per-shard failures, negative means "no limit", so it's allowed.
+    if context.options.max_total_failures < 0:
+      context.print_err("max_total_failures option must be >0, but you specified %s" %
+          context.options.max_total_failures)
+      context.print_log(logging.INFO, "Error: max_total_failures option=%s" %
+          context.options.max_total_failures)
+      return EXIT_INVALID_PARAMETER
+
     job = context.options.instance_spec.jobkey
     instances = (None if context.options.instance_spec.instance == ALL_INSTANCES else
         context.options.instance_spec.instance)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/d931a314/src/main/python/apache/aurora/client/commands/core.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/commands/core.py b/src/main/python/apache/aurora/client/commands/core.py
index 4ce6035..9b4c76a 100644
--- a/src/main/python/apache/aurora/client/commands/core.py
+++ b/src/main/python/apache/aurora/client/commands/core.py
@@ -670,6 +670,10 @@ def restart(args, options):
 
   Restarts are fully controlled client-side, so aborting halts the restart.
   """
+  if options.max_total_failures < 0:
+    print("max_total_failures option must be >0, but you specified %s" % options.max_total_failures,
+      file=sys.stderr)
+    exit(1)
   maybe_disable_hooks(options)
   api, job_key, config_file = LiveJobDisambiguator.disambiguate_args_or_die(
       args, options, make_client_factory())

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/d931a314/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 d365ef4..68951fa 100644
--- a/src/test/python/apache/aurora/client/cli/test_restart.py
+++ b/src/test/python/apache/aurora/client/cli/test_restart.py
@@ -20,7 +20,7 @@ from mock import Mock, patch
 from twitter.common.contextutil import temporary_file
 
 from apache.aurora.client.api.health_check import Retriable, StatusHealthCheck
-from apache.aurora.client.cli import EXIT_API_ERROR
+from apache.aurora.client.cli import EXIT_API_ERROR, EXIT_INVALID_PARAMETER
 from apache.aurora.client.cli.client import AuroraCommandLine
 from apache.aurora.client.cli.util import AuroraClientCommandTest
 
@@ -107,12 +107,13 @@ class TestRestartCommand(AuroraClientCommandTest):
         fp.write(self.get_valid_config())
         fp.flush()
         cmd = AuroraCommandLine()
-        cmd.execute(['job', 'restart', '--batch-size=5', '--strict', 'west/bozo/test/hello/0-100',
-            fp.name])
-
-        assert mock_scheduler_proxy.getTasksStatus.call_count == 1
+        result = cmd.execute(['job', 'restart', '--batch-size=5', '--max-total-failures=-1',
+            'west/bozo/test/hello', fp.name])
+        assert result == EXIT_INVALID_PARAMETER
+        assert mock_scheduler_proxy.getTasksStatus.call_count == 0
         assert mock_scheduler_proxy.restartShards.call_count == 0
 
+
   def test_restart_failed_status(self):
     (mock_api, mock_scheduler_proxy) = self.create_mock_api()
     mock_health_check = self.setup_health_checks(mock_api)
@@ -165,8 +166,6 @@ class TestRestartCommand(AuroraClientCommandTest):
         # by the getTasksStatus call.
         mock_log.assert_called_with(20, 'Error executing command: %s', 'Damn')
 
-
-
   def test_restart_failed_restart(self):
     # Test the client-side updater logic in its simplest case: everything succeeds, and no
rolling
     # updates.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/d931a314/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 fba529f..18e90aa 100644
--- a/src/test/python/apache/aurora/client/commands/test_restart.py
+++ b/src/test/python/apache/aurora/client/commands/test_restart.py
@@ -133,6 +133,36 @@ class TestRestartCommand(AuroraClientCommandTest):
         mock_scheduler_proxy.restartShards.assert_called_with(JobKey(environment=self.TEST_ENV,
             role=self.TEST_ROLE, name=self.TEST_JOB), [15, 16, 17, 18, 19], None)
 
+  def test_restart_simple_invalid_max_failures(self):
+    # Test the client-side restart logic in its simplest case: everything succeeds
+    mock_options = self.setup_mock_options()
+    mock_options.max_total_failures = -1
+    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
+    mock_health_check = self.setup_health_checks(mock_api)
+    self.setup_mock_scheduler_for_simple_restart(mock_api)
+    with contextlib.nested(
+        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.StatusHealthCheck',
+            return_value=mock_health_check),
+        patch('time.time', side_effect=functools.partial(self.fake_time, self)),
+        patch('time.sleep', return_value=None)
+    ) as (options, scheduler_proxy_class, test_clusters, mock_health_check_factory,
+        time_patch, sleep_patch):
+
+      with temporary_file() as fp:
+        fp.write(self.get_valid_config())
+        fp.flush()
+        self.assertRaises(SystemExit, restart, ['west/mchucarroll/test/hello'], mock_options)
+
+        # Like the update test, the exact number of calls here doesn't matter.
+        # what matters is that it must have been called once before batching, plus
+        # at least once per batch, and there are 4 batches.
+        assert mock_scheduler_proxy.getTasksStatus.call_count == 0
+        # called once per batch
+        assert mock_scheduler_proxy.restartShards.call_count == 0
+
   def test_restart_failed_status(self):
     mock_options = self.setup_mock_options()
     (mock_api, mock_scheduler_proxy) = self.create_mock_api()


Mime
View raw message