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/10/02 22:16:51 UTC

git commit: Fix multiple errors involving bindings, and fix result codes.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 102b83c7e -> dcaec498a


Fix multiple errors involving bindings, and fix result codes.

- Cause an error to be raised by an unresolved binding in a configuration file.
- Fix incorrect structuring of processed bindings.
- Add a test to confirm that unbound parameters generate errors.

While I'm in the code, also fix a problem where result codes aren't returned correctly
to the shell.

Bugs closed: aurora-781

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


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

Branch: refs/heads/master
Commit: dcaec498af556043e1d515d9ab9ef915e35831c5
Parents: 102b83c
Author: Mark Chu-Carroll <mc...@twopensource.com>
Authored: Thu Oct 2 16:13:19 2014 -0400
Committer: Mark Chu-Carroll <mc...@twitter.com>
Committed: Thu Oct 2 16:13:19 2014 -0400

----------------------------------------------------------------------
 .../python/apache/aurora/client/cli/client.py   |  2 +-
 .../python/apache/aurora/client/cli/context.py  | 18 ++++++++++---
 .../python/apache/aurora/client/cli/options.py  | 11 ++++++--
 .../aurora/client/cli/standalone_client.py      |  2 +-
 .../apache/aurora/client/cli/test_create.py     | 27 +++++++++++++++++++-
 .../python/apache/aurora/client/cli/util.py     |  3 +--
 6 files changed, 53 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dcaec498/src/main/python/apache/aurora/client/cli/client.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/client.py b/src/main/python/apache/aurora/client/cli/client.py
index 0cb6944..22ce318 100644
--- a/src/main/python/apache/aurora/client/cli/client.py
+++ b/src/main/python/apache/aurora/client/cli/client.py
@@ -63,7 +63,7 @@ def proxy_main():
   v2 = AuroraClientV2CommandProcessor()
   v1 = AuroraClientV1CommandProcessor()
   bridge = Bridge([v2, v1], default=v1)
-  bridge.execute(sys.argv)
+  sys.exit(bridge.execute(sys.argv))
 
 
 if __name__ == '__main__':

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dcaec498/src/main/python/apache/aurora/client/cli/context.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/context.py b/src/main/python/apache/aurora/client/cli/context.py
index 102d207..f639af7 100644
--- a/src/main/python/apache/aurora/client/cli/context.py
+++ b/src/main/python/apache/aurora/client/cli/context.py
@@ -18,6 +18,8 @@ import logging
 from collections import namedtuple
 from fnmatch import fnmatch
 
+from pystachio import Ref
+
 from apache.aurora.client.base import synthesize_url
 from apache.aurora.client.cli import (
     Context,
@@ -41,7 +43,7 @@ PartialJobKey = namedtuple('PartialJobKey', ['cluster', 'role', 'env', 'name'])
 def bindings_to_list(bindings):
   """Pystachio takes bindings in the form of a list of dictionaries. Each pystachio binding
   becomes another dictionary in the list. So we need to convert the bindings specified by
-  the user from a list of "name=value" formatted strings to a list of the dictionaries 
+  the user from a list of "name=value" formatted strings to a list of the dictionaries
   expected by pystachio.
   """
   result = []
@@ -49,7 +51,11 @@ def bindings_to_list(bindings):
     binding_parts = b.split("=")
     if len(binding_parts) != 2:
       raise ValueError('Binding parameter must be formatted name=value')
-    result.append({binding_parts[0]: binding_parts[1]})
+    try:
+      ref = Ref.from_address(binding_parts[0])
+    except Ref.InvalidRefError as e:
+      raise ValueError("Could not parse binding parameter %s: %s" % (b, e))
+    result.append({ref: binding_parts[1]})
   return result
 
 
@@ -82,7 +88,7 @@ class AuroraCommandContext(Context):
       with open(config_file, "r") as fp:
         self.print_log(TRANSCRIPT, "Config: %s" % fp.readlines())
       bindings = bindings_to_list(self.options.bindings) if self.options.bindings else None
-      return get_config(
+      result = get_config(
         jobname,
         config_file,
         self.options.read_json,
@@ -90,6 +96,12 @@ class AuroraCommandContext(Context):
         select_cluster=jobkey.cluster,
         select_role=jobkey.role,
         select_env=jobkey.env)
+      check_result = result.raw().check()
+      if not check_result.ok():
+        self.print_err(str(check_result))
+        raise self.CommandError(EXIT_INVALID_CONFIGURATION,
+            "Error in configuration")
+      return result
     except Exception as e:
       raise self.CommandError(EXIT_INVALID_CONFIGURATION, 'Error loading configuration: %s' % e)
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dcaec498/src/main/python/apache/aurora/client/cli/options.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/options.py b/src/main/python/apache/aurora/client/cli/options.py
index c2d422a..9a81a1d 100644
--- a/src/main/python/apache/aurora/client/cli/options.py
+++ b/src/main/python/apache/aurora/client/cli/options.py
@@ -186,8 +186,9 @@ BATCH_OPTION = CommandOption('--batch-size', type=int, default=5,
         help='Number of instances to be operate on in one iteration')
 
 
-BIND_OPTION = CommandOption('--bind', type=str, default=[], dest='bindings',
+BIND_OPTION = CommandOption('--bind', dest='bindings',
     action='append',
+    default=[],
     metavar="var=value",
     help='Bind a pystachio variable name to a value. '
     'Multiple flags may be used to specify multiple values.')
@@ -229,7 +230,12 @@ INSTANCES_SPEC_ARGUMENT = CommandOption('instance_spec', type=instance_specifier
         'If INSTANCES is omitted, then all instances will be operated on.'))
 
 
-JOBSPEC_ARGUMENT = CommandOption('jobspec', type=AuroraJobKey.from_path,
+def jobkeytype(v):
+  """wrapper for AuroraJobKey.from_path that improves error messages"""
+  return AuroraJobKey.from_path(v)
+
+
+JOBSPEC_ARGUMENT = CommandOption('jobspec', type=jobkeytype,
     metavar="CLUSTER/ROLE/ENV/NAME",
     help='Fully specified job key, in CLUSTER/ROLE/ENV/NAME format')
 
@@ -243,6 +249,7 @@ JSON_WRITE_OPTION = CommandOption('--write-json', default=False, dest='write_jso
     action='store_true',
     help='Generate command output in JSON format')
 
+
 MAX_TOTAL_FAILURES_OPTION = CommandOption('--max-total-failures', type=int, default=0,
      help='Maximum number of instance failures to be tolerated in total before aborting.')
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dcaec498/src/main/python/apache/aurora/client/cli/standalone_client.py
----------------------------------------------------------------------
diff --git a/src/main/python/apache/aurora/client/cli/standalone_client.py b/src/main/python/apache/aurora/client/cli/standalone_client.py
index 30d8f75..fd2232b 100644
--- a/src/main/python/apache/aurora/client/cli/standalone_client.py
+++ b/src/main/python/apache/aurora/client/cli/standalone_client.py
@@ -125,7 +125,7 @@ def proxy_main():
   client = AuroraCommandLine()
   if len(sys.argv) == 1:
     sys.argv.append("help")
-  client.execute(sys.argv[1:])
+  sys.exit(client.execute(sys.argv[1:]))
 
 if __name__ == '__main__':
   proxy_main()

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dcaec498/src/test/python/apache/aurora/client/cli/test_create.py
----------------------------------------------------------------------
diff --git a/src/test/python/apache/aurora/client/cli/test_create.py b/src/test/python/apache/aurora/client/cli/test_create.py
index 31fa56f..6e55188 100644
--- a/src/test/python/apache/aurora/client/cli/test_create.py
+++ b/src/test/python/apache/aurora/client/cli/test_create.py
@@ -290,10 +290,35 @@ class TestClientCreateCommand(AuroraClientCommandTest):
         fp.flush()
         cmd = AuroraCommandLine()
         cmd.execute(['job', 'create', '--wait-until=RUNNING', '--bind', 'cluster_binding=west',
-            '--bind', 'instances_binding=20', 'west/bozo/test/hello',
+            '--bind', 'instances_binding=20', '--bind', 'TEST_BATCH=1',
+            'west/bozo/test/hello',
             fp.name])
 
       # Now check that the right API calls got made.
       # Check that create_job was called exactly once, with an AuroraConfig parameter.
       self.assert_create_job_called(api)
       self.assert_scheduler_called(api, mock_query, 2)
+
+  def test_failed_create_job_with_incomplete_bindings(self):
+    """Run a test of the "create" command against a mocked-out API:
+    Verifies that the creation command sends the right API RPCs, and performs the correct
+    tests on the result."""
+
+    mock_context = FakeAuroraCommandContext()
+    with contextlib.nested(
+        patch('threading._Event.wait'),
+        patch('apache.aurora.client.cli.jobs.Job.create_context', return_value=mock_context)):
+
+      # This is the real test: invoke create as if it had been called by the command line.
+      with temporary_file() as fp:
+        fp.write(self.get_unbound_test_config())
+        fp.flush()
+        cmd = AuroraCommandLine()
+        result = cmd.execute(['job', 'create', '--wait-until=RUNNING',
+            '--bind', 'cluster_binding=west',
+           'west/bozo/test/hello',
+            fp.name])
+        assert result == EXIT_INVALID_CONFIGURATION
+      assert mock_context.get_err() == [
+          "TypeCheck(FAILED): MesosJob[update_config] failed: "
+          "UpdateConfig[batch_size] failed: u'{{TEST_BATCH}}' not an integer"]

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dcaec498/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 a50b83c..e1ee884 100644
--- a/src/test/python/apache/aurora/client/cli/util.py
+++ b/src/test/python/apache/aurora/client/cli/util.py
@@ -211,7 +211,7 @@ jobs = [HELLO_WORLD]
         environment = '%(env)s',
         instances = '{{instances_binding}}',
         update_config = UpdateConfig(
-          batch_size = 1,
+          batch_size = "{{TEST_BATCH}}",
           restart_threshold = 60,
           watch_secs = 45,
           max_per_shard_failures = 2,
@@ -254,7 +254,6 @@ jobs = [HELLO_WORLD]
   def get_unbound_test_config(cls, role=None, env=None, job=None):
     result = cls.UNBOUND_CONFIG % {'job': job or cls.TEST_JOB, 'role': role or cls.TEST_ROLE,
         'env': env or cls.TEST_ENV}
-    print("CONFIG:===========================\n%s\n=============================" % result)
     return result
 
   @classmethod