aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mchucarr...@apache.org
Subject git commit: Fix broken "large update" warning.
Date Wed, 10 Sep 2014 12:42:14 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master a2ce06ec0 -> 373a3928f


Fix broken "large update" warning.

At some point, the "large update" warning logic was revised so that the
warning shows whenever you do an update that either:
- the updated config had more than 4x as many instances as the running one; or
- the updated config had less than 4x as many instances as the running one.

(Seriously: either local >= 4 * remote or local <= 4 * remote". And running git blame
says it's all my fault.)

This fixes it: the correct logic is:
- the updated config has more than 4x as many as running; or
- the updated config has less than 1/4th as many as running.

Bugs closed: aurora-5860

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


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

Branch: refs/heads/master
Commit: 373a3928fed5f74e0806b90600d7a3e76bed82b6
Parents: a2ce06e
Author: Mark Chu-Carroll <mchucarroll@twopensource.com>
Authored: Wed Sep 10 08:30:04 2014 -0400
Committer: Mark Chu-Carroll <mchucarroll@twitter.com>
Committed: Wed Sep 10 08:30:04 2014 -0400

----------------------------------------------------------------------
 .../python/apache/aurora/client/cli/__init__.py |  2 +-
 .../python/apache/aurora/client/cli/jobs.py     |  5 +-
 .../apache/aurora/client/cli/test_update.py     | 89 ++++++++++++++++++--
 .../python/apache/aurora/client/cli/util.py     | 11 +++
 4 files changed, 99 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/373a3928/src/main/python/apache/aurora/client/cli/__init__.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/__init__.py b/src/main/python/apache/aurora/client/cli/__init__.py
index 6e553d8..e0c3050 100644
--- a/src/main/python/apache/aurora/client/cli/__init__.py
+++ b/src/main/python/apache/aurora/client/cli/__init__.py
@@ -405,7 +405,7 @@ class CommandLine(object):
     try:
       result = noun.execute(context)
       if result == EXIT_OK:
-        print_aurora_log(logging.INFO, "Command terminated successfully")
+        print_aurora_log(TRANSCRIPT, "Command terminated successfully")
         GlobalCommandHookRegistry.run_post_hooks(context, context.options.noun,
             context.options.verb, result)
       else:

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/373a3928/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 8f349c0..38b5f6e 100644
--- a/src/main/python/apache/aurora/client/cli/jobs.py
+++ b/src/main/python/apache/aurora/client/cli/jobs.py
@@ -629,8 +629,10 @@ to preview what changes will take effect.
         err_msg="Server could not populate job config for comparison; see log for details.")
     local_task_count = len(resp.result.populateJobResult.populated)
     remote_task_count = len(remote_tasks)
+
+    # Dangerous if it's more than a factor-of-four change in number of instances.
     if (local_task_count >= 4 * remote_task_count or
-        local_task_count <= 4 * remote_task_count or
+        4 * local_task_count <= remote_task_count or
         local_task_count == 0):
       context.print_out("Warning: this update is a large change. "
           "Press ^C within 5 seconds to abort")
@@ -650,6 +652,7 @@ to preview what changes will take effect.
         instances)
     context.check_and_log_response(resp, err_code=EXIT_COMMAND_FAILURE,
         err_msg="Update failed; see log for details.")
+    context.print_out("Update completed successfully")
     return EXIT_OK
 
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/373a3928/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 8b7d112..536d9c7 100644
--- a/src/test/python/apache/aurora/client/cli/test_update.py
+++ b/src/test/python/apache/aurora/client/cli/test_update.py
@@ -21,9 +21,9 @@ from apache.aurora.client.api.health_check import Retriable, StatusHealthCheck
 from apache.aurora.client.api.job_monitor import JobMonitor
 from apache.aurora.client.api.quota_check import QuotaCheck
 from apache.aurora.client.api.scheduler_mux import SchedulerMux
-from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION
+from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION, EXIT_OK
 from apache.aurora.client.cli.client import AuroraCommandLine
-from apache.aurora.client.cli.util import AuroraClientCommandTest, FakeAuroraCommandContext
+from apache.aurora.client.cli.util import AuroraClientCommandTest, FakeAuroraCommandContext,
IOMock
 from apache.aurora.config import AuroraConfig
 
 from gen.apache.aurora.api.constants import ACTIVE_STATES
@@ -88,7 +88,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
         assert mock_api.update_job.call_count == 0
 
   @classmethod
-  def setup_mock_scheduler_for_simple_update(cls, api):
+  def setup_mock_scheduler_for_simple_update(cls, api, count=20):
     """Set up all of the API mocks for scheduler calls during a simple update"""
     sched_proxy = api.scheduler_proxy
     # First, the updater acquires a lock
@@ -97,7 +97,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
     # Then it gets the status of the tasks for the updating job.
     cls.setup_get_tasks_status_calls(sched_proxy)
     # Next, it needs to populate the update config.
-    cls.setup_populate_job_config(sched_proxy)
+    cls.setup_populate_job_config(sched_proxy, count)
     # Then it does the update, which kills and restarts jobs, and monitors their
     # health with the status call.
     cls.setup_kill_tasks(sched_proxy)
@@ -118,11 +118,11 @@ class TestUpdateCommand(AuroraClientCommandTest):
     return kill_response
 
   @classmethod
-  def setup_populate_job_config(cls, api):
+  def setup_populate_job_config(cls, api, count=20):
     populate = cls.create_simple_success_response()
     populate.result.populateJobResult = Mock(spec=PopulateJobResult)
     api.populateJobConfig.return_value = populate
-    configs = [TaskConfig(numCpus=1.0, ramMb=1, diskMb=1) for i in range(20)]
+    configs = [TaskConfig(numCpus=1.0, ramMb=1, diskMb=1) for i in range(count)]
     populate.result.populateJobResult.populated = set(configs)
     return populate
 
@@ -202,6 +202,7 @@ class TestUpdateCommand(AuroraClientCommandTest):
         patch('apache.aurora.client.api.updater.QuotaCheck', return_value=mock_quota_check),
         patch('apache.aurora.client.api.updater.SchedulerMux', return_value=fake_mux),
         patch('time.time', side_effect=functools.partial(self.fake_time, self)),
+        patch('time.sleep', return_value=None),
         patch('threading._Event.wait')):
       with temporary_file() as fp:
         fp.write(self.get_valid_config())
@@ -224,6 +225,82 @@ class TestUpdateCommand(AuroraClientCommandTest):
       self.assert_correct_status_calls(mock_scheduler_proxy)
       assert mock_scheduler_proxy.releaseLock.call_count == 1
 
+  def test_updater_simple_small_doesnt_warn(self):
+    mock_out = IOMock()
+    mock_err = IOMock()
+    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
+    mock_health_check = self.setup_health_checks(mock_api)
+    mock_quota_check = self.setup_quota_check()
+    mock_job_monitor = self.setup_job_monitor()
+    fake_mux = self.FakeSchedulerMux()
+    self.setup_mock_scheduler_for_simple_update(mock_api)
+    # This doesn't work, because:
+    # - The mock_context stubs out the API.
+    # - the test relies on using live code in the API.
+    with contextlib.nested(
+        patch('apache.aurora.client.cli.jobs.AuroraCommandContext.print_out',
+            side_effect=mock_out.put),
+        patch('apache.aurora.client.cli.jobs.AuroraCommandContext.print_err',
+            side_effect=mock_err.put),
+        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.StatusHealthCheck',
+            return_value=mock_health_check),
+        patch('apache.aurora.client.api.updater.JobMonitor', return_value=mock_job_monitor),
+        patch('apache.aurora.client.api.updater.QuotaCheck', return_value=mock_quota_check),
+        patch('apache.aurora.client.api.updater.SchedulerMux', return_value=fake_mux),
+        patch('time.time', side_effect=functools.partial(self.fake_time, self)),
+        patch('time.sleep', return_value=None),
+        patch('threading._Event.wait')):
+      with temporary_file() as fp:
+        fp.write(self.get_valid_config())
+        fp.flush()
+        cmd = AuroraCommandLine()
+        cmd.execute(['job', 'update', 'west/bozo/test/hello', fp.name])
+      assert mock_out.get() == ['Update completed successfully']
+      assert mock_err.get() == []
+
+  def test_updater_simple_large_does_warn(self):
+    mock_out = IOMock()
+    mock_err = IOMock()
+    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
+    mock_health_check = self.setup_health_checks(mock_api)
+    mock_quota_check = self.setup_quota_check()
+    mock_job_monitor = self.setup_job_monitor()
+    fake_mux = self.FakeSchedulerMux()
+    self.setup_mock_scheduler_for_simple_update(mock_api, count=2)
+    # This doesn't work, because:
+    # - The mock_context stubs out the API.
+    # - the test relies on using live code in the API.
+    config = self.get_valid_config()
+    config = config.replace("instances = 20", "instances = 2")
+    print("CONFIG = %s" % config)
+    with contextlib.nested(
+        patch('apache.aurora.client.cli.jobs.AuroraCommandContext.print_out',
+            side_effect=mock_out.put),
+        patch('apache.aurora.client.cli.jobs.AuroraCommandContext.print_err',
+            side_effect=mock_err.put),
+        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.StatusHealthCheck',
+            return_value=mock_health_check),
+        patch('apache.aurora.client.api.updater.JobMonitor', return_value=mock_job_monitor),
+        patch('apache.aurora.client.api.updater.QuotaCheck', return_value=mock_quota_check),
+        patch('apache.aurora.client.api.updater.SchedulerMux', return_value=fake_mux),
+        patch('time.time', side_effect=functools.partial(self.fake_time, self)),
+        patch('time.sleep', return_value=None),
+        patch('threading._Event.wait')):
+      with temporary_file() as fp:
+        fp.write(config)
+        fp.flush()
+        cmd = AuroraCommandLine()
+        result = cmd.execute(['job', 'update', 'west/bozo/test/hello', fp.name])
+        assert result == EXIT_OK
+      assert mock_out.get() == [
+          "Warning: this update is a large change. Press ^C within 5 seconds to abort",
+          'Update completed successfully']
+      assert mock_err.get() == []
+
   @classmethod
   def assert_correct_addinstance_calls(cls, api):
     assert api.addInstances.call_count == 20

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/373a3928/src/test/python/apache/aurora/client/cli/util.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/util.py b/src/test/python/apache/aurora/client/cli/util.py
index 95a2123..ebabe52 100644
--- a/src/test/python/apache/aurora/client/cli/util.py
+++ b/src/test/python/apache/aurora/client/cli/util.py
@@ -262,3 +262,14 @@ jobs = [HELLO_WORLD]
   def get_invalid_config(cls, bad_clause):
     return cls.get_test_config(cls.TEST_CLUSTER, cls.TEST_ROLE, cls.TEST_ENV, cls.TEST_JOB,
         bad_clause)
+
+
+class IOMock(object):
+  def __init__(self):
+    self.out = []
+
+  def put(self, s):
+    self.out.append(s)
+
+  def get(self):
+    return self.out


Mime
View raw message