You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/11/20 01:11:43 UTC

incubator-aurora git commit: Ensure all verbs return an exit code.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 5cdc3900c -> 1d78ac564


Ensure all verbs return an exit code.

Bugs closed: AURORA-923

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


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

Branch: refs/heads/master
Commit: 1d78ac5648e849452cea47c83a75713e2539f016
Parents: 5cdc390
Author: Zameer Manji <zm...@twopensource.com>
Authored: Wed Nov 19 16:01:22 2014 -0800
Committer: Bill Farner <wf...@apache.org>
Committed: Wed Nov 19 16:01:22 2014 -0800

----------------------------------------------------------------------
 .../python/apache/aurora/client/cli/__init__.py |  13 +-
 .../python/apache/aurora/client/cli/task.py     |   3 +-
 src/test/python/apache/aurora/client/cli/BUILD  |   2 +-
 .../apache/aurora/client/cli/test_kill.py       |   2 +-
 .../apache/aurora/client/cli/test_task.py       | 178 +++++++++++++++++++
 .../apache/aurora/client/cli/test_task_run.py   | 178 -------------------
 6 files changed, 190 insertions(+), 186 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d78ac56/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 5dc3c3b..a12d789 100644
--- a/src/main/python/apache/aurora/client/cli/__init__.py
+++ b/src/main/python/apache/aurora/client/cli/__init__.py
@@ -35,10 +35,11 @@ import getpass
 import logging
 import sys
 import traceback
-from abc import abstractmethod
+from abc import abstractmethod, abstractproperty
 from uuid import uuid1
 from zipfile import BadZipfile
 
+from twitter.common.lang import AbstractClass
 from twitter.common.python.pex import PexInfo
 
 from .command_hooks import GlobalCommandHookRegistry
@@ -198,7 +199,7 @@ class ConfigurationPlugin(object):
     """
 
 
-class AuroraCommand(object):
+class AuroraCommand(AbstractClass):
   def setup_options_parser(self, argparser):
     """Sets up command line options parsing for this command.
     This is a thin veneer over the standard python argparse system.
@@ -212,15 +213,15 @@ class AuroraCommand(object):
       raise TypeError('Command option object must be an instance of CommandOption')
     option.add_to_parser(argparser)
 
-  @property
+  @abstractproperty
   def help(self):
     """Returns the help message for this command"""
 
-  @property
+  @abstractproperty
   def usage(self):
     """Returns a short usage description of the command"""
 
-  @property
+  @abstractproperty
   def name(self):
     """Returns the command name"""
 
@@ -396,6 +397,7 @@ class CommandLine(object):
       return pre_result
     try:
       result = noun.execute(context)
+      assert result is not None, "Noun return value is None!"
       if result == EXIT_OK:
         print_aurora_log(TRANSCRIPT, "Command terminated successfully")
         GlobalCommandHookRegistry.run_post_hooks(context, context.options.noun,
@@ -523,5 +525,6 @@ class Verb(AuroraCommand):
     result += ["", self.help]
     return "\n".join(result)
 
+  @abstractmethod
   def execute(self, context):
     pass

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d78ac56/src/main/python/apache/aurora/client/cli/task.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/task.py b/src/main/python/apache/aurora/client/cli/task.py
index 37e0141..8a139db 100644
--- a/src/main/python/apache/aurora/client/cli/task.py
+++ b/src/main/python/apache/aurora/client/cli/task.py
@@ -23,7 +23,7 @@ from apache.aurora.client.api.command_runner import (
     DistributedCommandRunner,
     InstanceDistributedCommandRunner
 )
-from apache.aurora.client.cli import EXIT_INVALID_PARAMETER, Noun, print_aurora_log, Verb
+from apache.aurora.client.cli import EXIT_INVALID_PARAMETER, EXIT_OK, Noun, print_aurora_log, Verb
 from apache.aurora.client.cli.context import AuroraCommandContext
 from apache.aurora.client.cli.options import (
     CommandOption,
@@ -69,6 +69,7 @@ class RunCommand(Verb):
         context.options.ssh_user, instances, print_aurora_log)
     dcr.run(context.options.cmd, parallelism=context.options.num_threads,
         executor_sandbox=context.options.executor_sandbox)
+    return EXIT_OK
 
 
 class SshCommand(Verb):

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d78ac56/src/test/python/apache/aurora/client/cli/BUILD
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/BUILD b/src/test/python/apache/aurora/client/cli/BUILD
index 4692d31..b24c616 100644
--- a/src/test/python/apache/aurora/client/cli/BUILD
+++ b/src/test/python/apache/aurora/client/cli/BUILD
@@ -216,7 +216,7 @@ python_tests(
 
 python_tests(
   name='task',
-  sources = [ 'test_task_run.py'],
+  sources = [ 'test_task.py'],
   dependencies = [
     ':util',
     '3rdparty/python:mock',

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d78ac56/src/test/python/apache/aurora/client/cli/test_kill.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_kill.py b/src/test/python/apache/aurora/client/cli/test_kill.py
index de5fcb8..1eda72a 100644
--- a/src/test/python/apache/aurora/client/cli/test_kill.py
+++ b/src/test/python/apache/aurora/client/cli/test_kill.py
@@ -366,7 +366,7 @@ class TestClientKillCommand(AuroraClientCommandTest):
       api = mock_context.get_api('west')
       # set up an empty instance list in the getTasksWithoutConfigs response
       status_response = self.create_simple_success_response()
-      status_response.result = Result(scheduleStatusResult=ScheduleStatusResult(tasks=[[]]))
+      status_response.result = Result(scheduleStatusResult=ScheduleStatusResult(tasks=[]))
       mock_context.add_expected_status_query_result(status_response)
       api.kill_job.return_value = self.get_kill_job_response()
       with temporary_file() as fp:

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d78ac56/src/test/python/apache/aurora/client/cli/test_task.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_task.py b/src/test/python/apache/aurora/client/cli/test_task.py
new file mode 100644
index 0000000..c69a624
--- /dev/null
+++ b/src/test/python/apache/aurora/client/cli/test_task.py
@@ -0,0 +1,178 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import contextlib
+
+from mock import Mock, patch
+
+from apache.aurora.client.cli import EXIT_INVALID_PARAMETER, EXIT_OK
+from apache.aurora.client.cli.client import AuroraCommandLine
+
+from .util import AuroraClientCommandTest
+
+from gen.apache.aurora.api.ttypes import (
+    JobKey,
+    ResponseCode,
+    Result,
+    ScheduleStatus,
+    ScheduleStatusResult,
+    TaskQuery
+)
+
+MOCK_LOG_CONTENTS = []
+
+
+def mock_log(level, msg):
+  MOCK_LOG_CONTENTS.append((level, msg))
+
+
+class TestRunCommand(AuroraClientCommandTest):
+
+  @classmethod
+  def create_status_response(cls):
+    resp = cls.create_simple_success_response()
+    resp.result = Result(
+        scheduleStatusResult=ScheduleStatusResult(tasks=cls.create_scheduled_tasks()))
+    return resp
+
+  @classmethod
+  def create_failed_status_response(cls):
+    return cls.create_blank_response(ResponseCode.INVALID_REQUEST, 'No tasks found for query')
+
+  @classmethod
+  def create_mock_process(cls):
+    process = Mock()
+    process.communicate.return_value = ["hello", "world"]
+    return process
+
+  def test_successful_run(self):
+    """Test the run command."""
+    self.generic_test_successful_run(['task', 'run', 'west/bozo/test/hello', 'ls'], None)
+
+  def test_successful_run_with_instances(self):
+    """Test the run command."""
+    self.generic_test_successful_run(['task', 'run', 'west/bozo/test/hello/1-3', 'ls'], [1, 2, 3])
+
+  def generic_test_successful_run(self, cmd_args, instances):
+    """Common structure of all successful run tests.
+    Params:
+      cmd_args: the arguments to pass to the aurora command line to run this test.
+      instances: the list of instances that should be passed to a status query.
+         (The status query is the only visible difference between a sharded
+         run, and an all-instances run in the test.)
+    """
+    # Calls api.check_status, which calls scheduler_proxy.getJobs
+    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
+    mock_scheduler_proxy.getTasksStatus.return_value = self.create_status_response()
+    sandbox_args = {'slave_root': '/slaveroot', 'slave_run_directory': 'slaverun'}
+    with contextlib.nested(
+        patch('apache.aurora.client.cli.task.print_aurora_log', side_effect=mock_log),
+        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.cli.task.CLUSTERS', new=self.TEST_CLUSTERS),
+        patch('apache.aurora.client.api.command_runner.'
+              'InstanceDistributedCommandRunner.sandbox_args',
+            return_value=sandbox_args),
+        patch('subprocess.Popen', return_value=self.create_mock_process())) as (
+            _,
+            mock_scheduler_proxy_class,
+            mock_clusters,
+            mock_clusters_cli,
+            mock_runner_args_patch,
+            mock_subprocess):
+      cmd = AuroraCommandLine()
+      assert cmd.execute(cmd_args) == EXIT_OK
+      # The status command sends a getTasksStatus query to the scheduler,
+      # and then prints the result. The use of shards, above, should change
+      # this query - that's the focus of the instances test.
+      mock_scheduler_proxy.getTasksStatus.assert_called_with(TaskQuery(
+          jobKeys=[JobKey(role='bozo', environment='test', name='hello')],
+          statuses=set([ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.RESTARTING,
+              ScheduleStatus.PREEMPTING, ScheduleStatus.DRAINING]),
+          instanceIds=instances))
+
+      # The mock status call returns 3 three ScheduledTasks, so three commands should have been run
+      assert mock_subprocess.call_count == 3
+      mock_subprocess.assert_called_with(['ssh', '-n', '-q', 'bozo@slavehost',
+          'cd /slaveroot/slaves/*/frameworks/*/executors/thermos-1287391823/runs/'
+          'slaverun/sandbox;ls'],
+          stderr=-2, stdout=-1)
+      # Check that logging worked properly:
+      assert any("Running command" in entry[1] for entry in MOCK_LOG_CONTENTS)
+
+
+class TestSshCommand(AuroraClientCommandTest):
+
+  @classmethod
+  def create_status_response(cls):
+    resp = cls.create_simple_success_response()
+    resp.result = Result(
+        scheduleStatusResult=ScheduleStatusResult(tasks=cls.create_scheduled_tasks()))
+    return resp
+
+  @classmethod
+  def create_nojob_status_response(cls):
+    resp = cls.create_simple_success_response()
+    resp.result = Result(scheduleStatusResult=ScheduleStatusResult(tasks=[]))
+    return resp
+
+  @classmethod
+  def create_failed_status_response(cls):
+    return cls.create_blank_response(ResponseCode.INVALID_REQUEST, 'No tasks found for query')
+
+  def test_successful_ssh(self):
+    """Test the ssh command."""
+    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
+    mock_scheduler_proxy.getTasksStatus.return_value = self.create_status_response()
+    sandbox_args = {'slave_root': '/slaveroot', 'slave_run_directory': 'slaverun'}
+    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.command_runner.DistributedCommandRunner.sandbox_args',
+            return_value=sandbox_args),
+        patch('subprocess.call', return_value=0)) as (
+            mock_scheduler_proxy_class,
+            mock_clusters,
+            mock_runner_args_patch,
+            mock_subprocess):
+      cmd = AuroraCommandLine()
+      cmd.execute(['task', 'ssh', 'west/bozo/test/hello/1', '--command=ls'])
+
+      # The status command sends a getTasksStatus query to the scheduler,
+      # and then prints the result.
+      mock_scheduler_proxy.getTasksStatus.assert_called_with(TaskQuery(
+          jobKeys=[JobKey(role='bozo', environment='test', name='hello')],
+          instanceIds=set([1]),
+          statuses=set([ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.RESTARTING,
+              ScheduleStatus.PREEMPTING, ScheduleStatus.DRAINING
+              ])))
+      mock_subprocess.assert_called_with(['ssh', '-t', 'bozo@slavehost',
+          'cd /slaveroot/slaves/*/frameworks/*/executors/thermos-1287391823/runs/'
+          'slaverun/sandbox;ls'])
+
+  def test_ssh_job_not_found(self):
+    """Test the ssh command when the jobkey parameter specifies a job that isn't running."""
+    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
+    mock_scheduler_proxy.getTasksStatus.return_value = self.create_nojob_status_response()
+    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('subprocess.call', return_value=0)) as (
+            mock_scheduler_proxy_class,
+            mock_clusters,
+            mock_subprocess):
+      cmd = AuroraCommandLine()
+      result = cmd.execute(['task', 'ssh', 'west/bozo/test/hello/1', '--command=ls'])
+      assert result == EXIT_INVALID_PARAMETER
+      assert mock_subprocess.call_count == 0

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/1d78ac56/src/test/python/apache/aurora/client/cli/test_task_run.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_task_run.py b/src/test/python/apache/aurora/client/cli/test_task_run.py
deleted file mode 100644
index 8544617..0000000
--- a/src/test/python/apache/aurora/client/cli/test_task_run.py
+++ /dev/null
@@ -1,178 +0,0 @@
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-import contextlib
-
-from mock import Mock, patch
-
-from apache.aurora.client.cli import EXIT_INVALID_PARAMETER
-from apache.aurora.client.cli.client import AuroraCommandLine
-
-from .util import AuroraClientCommandTest
-
-from gen.apache.aurora.api.ttypes import (
-    JobKey,
-    ResponseCode,
-    Result,
-    ScheduleStatus,
-    ScheduleStatusResult,
-    TaskQuery
-)
-
-MOCK_LOG_CONTENTS = []
-
-
-def mock_log(level, msg):
-  MOCK_LOG_CONTENTS.append((level, msg))
-
-
-class TestRunCommand(AuroraClientCommandTest):
-
-  @classmethod
-  def create_status_response(cls):
-    resp = cls.create_simple_success_response()
-    resp.result = Result(
-        scheduleStatusResult=ScheduleStatusResult(tasks=cls.create_scheduled_tasks()))
-    return resp
-
-  @classmethod
-  def create_failed_status_response(cls):
-    return cls.create_blank_response(ResponseCode.INVALID_REQUEST, 'No tasks found for query')
-
-  @classmethod
-  def create_mock_process(cls):
-    process = Mock()
-    process.communicate.return_value = ["hello", "world"]
-    return process
-
-  def test_successful_run(self):
-    """Test the run command."""
-    self.generic_test_successful_run(['task', 'run', 'west/bozo/test/hello', 'ls'], None)
-
-  def test_successful_run_with_instances(self):
-    """Test the run command."""
-    self.generic_test_successful_run(['task', 'run', 'west/bozo/test/hello/1-3', 'ls'], [1, 2, 3])
-
-  def generic_test_successful_run(self, cmd_args, instances):
-    """Common structure of all successful run tests.
-    Params:
-      cmd_args: the arguments to pass to the aurora command line to run this test.
-      instances: the list of instances that should be passed to a status query.
-         (The status query is the only visible difference between a sharded
-         run, and an all-instances run in the test.)
-    """
-    # Calls api.check_status, which calls scheduler_proxy.getJobs
-    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
-    mock_scheduler_proxy.getTasksStatus.return_value = self.create_status_response()
-    sandbox_args = {'slave_root': '/slaveroot', 'slave_run_directory': 'slaverun'}
-    with contextlib.nested(
-        patch('apache.aurora.client.cli.task.print_aurora_log', side_effect=mock_log),
-        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.cli.task.CLUSTERS', new=self.TEST_CLUSTERS),
-        patch('apache.aurora.client.api.command_runner.'
-              'InstanceDistributedCommandRunner.sandbox_args',
-            return_value=sandbox_args),
-        patch('subprocess.Popen', return_value=self.create_mock_process())) as (
-            _,
-            mock_scheduler_proxy_class,
-            mock_clusters,
-            mock_clusters_cli,
-            mock_runner_args_patch,
-            mock_subprocess):
-      cmd = AuroraCommandLine()
-      cmd.execute(cmd_args)
-      # The status command sends a getTasksStatus query to the scheduler,
-      # and then prints the result. The use of shards, above, should change
-      # this query - that's the focus of the instances test.
-      mock_scheduler_proxy.getTasksStatus.assert_called_with(TaskQuery(
-          jobKeys=[JobKey(role='bozo', environment='test', name='hello')],
-          statuses=set([ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.RESTARTING,
-              ScheduleStatus.PREEMPTING, ScheduleStatus.DRAINING]),
-          instanceIds=instances))
-
-      # The mock status call returns 3 three ScheduledTasks, so three commands should have been run
-      assert mock_subprocess.call_count == 3
-      mock_subprocess.assert_called_with(['ssh', '-n', '-q', 'bozo@slavehost',
-          'cd /slaveroot/slaves/*/frameworks/*/executors/thermos-1287391823/runs/'
-          'slaverun/sandbox;ls'],
-          stderr=-2, stdout=-1)
-      # Check that logging worked properly:
-      assert any("Running command" in entry[1] for entry in MOCK_LOG_CONTENTS)
-
-
-class TestSshCommand(AuroraClientCommandTest):
-
-  @classmethod
-  def create_status_response(cls):
-    resp = cls.create_simple_success_response()
-    resp.result = Result(
-        scheduleStatusResult=ScheduleStatusResult(tasks=cls.create_scheduled_tasks()))
-    return resp
-
-  @classmethod
-  def create_nojob_status_response(cls):
-    resp = cls.create_simple_success_response()
-    resp.result = Result(scheduleStatusResult=ScheduleStatusResult(tasks=[]))
-    return resp
-
-  @classmethod
-  def create_failed_status_response(cls):
-    return cls.create_blank_response(ResponseCode.INVALID_REQUEST, 'No tasks found for query')
-
-  def test_successful_ssh(self):
-    """Test the ssh command."""
-    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
-    mock_scheduler_proxy.getTasksStatus.return_value = self.create_status_response()
-    sandbox_args = {'slave_root': '/slaveroot', 'slave_run_directory': 'slaverun'}
-    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.command_runner.DistributedCommandRunner.sandbox_args',
-            return_value=sandbox_args),
-        patch('subprocess.call', return_value=0)) as (
-            mock_scheduler_proxy_class,
-            mock_clusters,
-            mock_runner_args_patch,
-            mock_subprocess):
-      cmd = AuroraCommandLine()
-      cmd.execute(['task', 'ssh', 'west/bozo/test/hello/1', '--command=ls'])
-
-      # The status command sends a getTasksStatus query to the scheduler,
-      # and then prints the result.
-      mock_scheduler_proxy.getTasksStatus.assert_called_with(TaskQuery(
-          jobKeys=[JobKey(role='bozo', environment='test', name='hello')],
-          instanceIds=set([1]),
-          statuses=set([ScheduleStatus.RUNNING, ScheduleStatus.KILLING, ScheduleStatus.RESTARTING,
-              ScheduleStatus.PREEMPTING, ScheduleStatus.DRAINING
-              ])))
-      mock_subprocess.assert_called_with(['ssh', '-t', 'bozo@slavehost',
-          'cd /slaveroot/slaves/*/frameworks/*/executors/thermos-1287391823/runs/'
-          'slaverun/sandbox;ls'])
-
-  def test_ssh_job_not_found(self):
-    """Test the ssh command when the jobkey parameter specifies a job that isn't running."""
-    (mock_api, mock_scheduler_proxy) = self.create_mock_api()
-    mock_scheduler_proxy.getTasksStatus.return_value = self.create_nojob_status_response()
-    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('subprocess.call', return_value=0)) as (
-            mock_scheduler_proxy_class,
-            mock_clusters,
-            mock_subprocess):
-      cmd = AuroraCommandLine()
-      result = cmd.execute(['task', 'ssh', 'west/bozo/test/hello/1', '--command=ls'])
-      assert result == EXIT_INVALID_PARAMETER
-      assert mock_subprocess.call_count == 0