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/08/22 03:05:24 UTC

git commit: Fix bug in handling of pystachio bindings.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master c545e656b -> 4bbf29bad


Fix bug in handling of pystachio bindings.

Also, add new unit test to prevent regressions.

Bugs closed: aurora-659

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


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

Branch: refs/heads/master
Commit: 4bbf29bade1cb8921ea042995864337063e688f7
Parents: c545e65
Author: Mark Chu-Carroll <mc...@twopensource.com>
Authored: Thu Aug 21 21:02:17 2014 -0400
Committer: Mark Chu-Carroll <mc...@twitter.com>
Committed: Thu Aug 21 21:02:17 2014 -0400

----------------------------------------------------------------------
 .../python/apache/aurora/client/cli/context.py  | 19 +++++++++++-
 .../python/apache/aurora/client/cli/options.py  |  4 +--
 .../apache/aurora/client/cli/test_create.py     | 32 ++++++++++++++++++++
 .../python/apache/aurora/client/cli/util.py     | 30 ++++++++++++++++++
 4 files changed, 82 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4bbf29ba/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 0e10589..0108d91 100644
--- a/src/main/python/apache/aurora/client/cli/context.py
+++ b/src/main/python/apache/aurora/client/cli/context.py
@@ -38,6 +38,22 @@ from gen.apache.aurora.api.ttypes import ResponseCode
 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 
+  expected by pystachio.
+  """
+  result = []
+  for b in 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]})
+  return result
+
+
+
 class AuroraCommandContext(Context):
   """A context object used by Aurora commands to manage command processing state
   and common operations.
@@ -65,11 +81,12 @@ class AuroraCommandContext(Context):
       # file without double-reading.
       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(
         jobname,
         config_file,
         self.options.read_json,
-        self.options.bindings,
+        bindings,
         select_cluster=jobkey.cluster,
         select_role=jobkey.role,
         select_env=jobkey.env)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4bbf29ba/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 bb61dbe..36f80aa 100644
--- a/src/main/python/apache/aurora/client/cli/options.py
+++ b/src/main/python/apache/aurora/client/cli/options.py
@@ -188,8 +188,8 @@ BATCH_OPTION = CommandOption('--batch-size', type=int, default=5,
 
 BIND_OPTION = CommandOption('--bind', type=str, default=[], dest='bindings',
     action='append',
-    metavar="pystachio-binding",
-    help='Bind a thermos mustache variable name to a value. '
+    metavar="var=value",
+    help='Bind a pystachio variable name to a value. '
     'Multiple flags may be used to specify multiple values.')
 
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4bbf29ba/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 ca635bd..b70363a 100644
--- a/src/test/python/apache/aurora/client/cli/test_create.py
+++ b/src/test/python/apache/aurora/client/cli/test_create.py
@@ -223,3 +223,35 @@ class TestClientCreateCommand(AuroraClientCommandTest):
             fp.name])
         assert result == EXIT_UNKNOWN_ERROR
         assert api.create_job.call_count == 0
+
+  def test_simple_successful_create_job_with_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)):
+      mock_query = self.create_mock_query()
+      mock_context.add_expected_status_query_result(
+        self.create_mock_status_query_result(ScheduleStatus.PENDING))
+      mock_context.add_expected_status_query_result(
+        self.create_mock_status_query_result(ScheduleStatus.RUNNING))
+      api = mock_context.get_api('west')
+      api.create_job.return_value = self.get_createjob_response()
+      mock_context.enable_reveal_errors()
+
+      # 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()
+        cmd.execute(['job', 'create', '--wait-until=RUNNING', '--bind', 'cluster_binding=west',
+            '--bind', 'instances_binding=20', '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)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/4bbf29ba/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 552537a..95a2123 100644
--- a/src/test/python/apache/aurora/client/cli/util.py
+++ b/src/test/python/apache/aurora/client/cli/util.py
@@ -12,6 +12,7 @@
 # limitations under the License.
 #
 
+import textwrap
 import unittest
 
 from mock import Mock
@@ -201,6 +202,28 @@ HELLO_WORLD = Job(
 jobs = [HELLO_WORLD]
 """
 
+  UNBOUND_CONFIG = textwrap.dedent("""\
+      HELLO_WORLD = Job(
+        name = '%(job)s',
+        role = '%(role)s',
+        cluster = '{{cluster_binding}}',
+        environment = '%(env)s',
+        instances = '{{instances_binding}}',
+        update_config = UpdateConfig(
+          batch_size = 1,
+          restart_threshold = 60,
+          watch_secs = 45,
+          max_per_shard_failures = 2,
+        ),
+        task = Task(
+          name = 'test',
+          processes = [Process(name = 'hello_world', cmdline = 'echo {{thermos.ports[http]}}')],
+          resources = Resources(cpu = 0.1, ram = 64 * MB, disk = 64 * MB),
+        )
+      )
+      jobs = [HELLO_WORLD]
+""")
+
   TEST_ROLE = 'bozo'
 
   TEST_ENV = 'test'
@@ -225,6 +248,13 @@ jobs = [HELLO_WORLD]
         'inner': filler}
 
   @classmethod
+  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
   def get_valid_config(cls):
     return cls.get_test_config(cls.TEST_CLUSTER, cls.TEST_ROLE, cls.TEST_ENV, cls.TEST_JOB)