You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by mc...@apache.org on 2014/05/13 21:24:53 UTC
git commit: Bugfix: restart doesn't notify user about invalid
max_total_failures option.
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 <mc...@twopensource.com>
Authored: Tue May 13 15:21:15 2014 -0400
Committer: Mark Chu-Carroll <mc...@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()