Repository: aurora Updated Branches: refs/heads/master 3a9aabda9 -> 42a497438
Sort the set objects inside TaskConfig during Job diff. Sort the entires in `set` fields inside `TaskConfig` as strings before shelling out to diff so that the output is consistent and meaningful. Testing Done: ./build-support/jenkins/build.sh Bugs closed: AURORA-1913 Reviewed at https://reviews.apache.org/r/58054/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/42a49743 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/42a49743 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/42a49743 Branch: refs/heads/master Commit: 42a497438ce1e3dad82f7fbe6380a7f3dd2d4e35 Parents: 3a9aabd Author: Santhosh Kumar Shanmugham <[email protected]> Authored: Wed Mar 29 22:28:26 2017 -0700 Committer: Santhosh Kumar <[email protected]> Committed: Wed Mar 29 22:28:26 2017 -0700 ---------------------------------------------------------------------- .../apache/aurora/client/cli/diff_formatter.py | 28 ++++++- .../aurora/client/cli/test_diff_formatter.py | 79 ++++++++++++-------- .../python/apache/aurora/client/cli/util.py | 2 +- 3 files changed, 72 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/42a49743/src/main/python/apache/aurora/client/cli/diff_formatter.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/client/cli/diff_formatter.py b/src/main/python/apache/aurora/client/cli/diff_formatter.py index a1b9bb3..7871777 100644 --- a/src/main/python/apache/aurora/client/cli/diff_formatter.py +++ b/src/main/python/apache/aurora/client/cli/diff_formatter.py @@ -53,6 +53,22 @@ class DiffFormatter(object): def __repr__(self): return self.data + # Sorting sets, hence they turn into lists + if task.constraints: + task.constraints = sorted(task.constraints, key=str) + + if task.metadata: + task.metadata = sorted(task.metadata, key=str) + + if task.resources: + task.resources = sorted(task.resources, key=str) + + if task.requestedPorts: + task.requestedPorts = sorted(task.requestedPorts, key=str) + + if task.mesosFetcherUris: + task.mesosFetcherUris = sorted(task.mesosFetcherUris, key=str) + task.executorConfig = ExecutorConfig( name=AURORA_EXECUTOR_NAME, data=RawRepr(pretty_executor)) @@ -72,12 +88,16 @@ class DiffFormatter(object): self._dump_tasks(local_tasks, local) with NamedTemporaryFile() as remote: self._dump_tasks(remote_tasks, remote) - result = subprocess.call("%s %s %s" % ( - diff_program, quote(remote.name), quote(local.name)), shell=True) + process = subprocess.Popen( + [diff_program, quote(remote.name), quote(local.name)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdout, stderr = process.communicate() # Unlike most commands, diff doesn't return zero on success; it returns # 1 when a successful diff is non-empty. - if result not in (0, 1): - raise self.context.CommandError(EXIT_COMMAND_FAILURE, "Error running diff command") + if process.poll() not in (0, 1): + raise self.context.CommandError(EXIT_COMMAND_FAILURE, "Error running diff: %s" % stderr) + self.context.print_out(stdout) def _show_diff(self, header, configs_summaries, local_task=None): def min_start(ranges): http://git-wip-us.apache.org/repos/asf/aurora/blob/42a49743/src/test/python/apache/aurora/client/cli/test_diff_formatter.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/client/cli/test_diff_formatter.py b/src/test/python/apache/aurora/client/cli/test_diff_formatter.py index a145d88..65e76ef 100644 --- a/src/test/python/apache/aurora/client/cli/test_diff_formatter.py +++ b/src/test/python/apache/aurora/client/cli/test_diff_formatter.py @@ -12,12 +12,11 @@ # limitations under the License. # -import contextlib -import os import textwrap +from copy import deepcopy import pytest -from mock import Mock, call, patch +from mock import Mock, call from pystachio import Empty from apache.aurora.client.cli import Context @@ -33,6 +32,7 @@ from .util import AuroraClientCommandTest, FakeAuroraCommandContext, mock_verb_o from gen.apache.aurora.api.constants import ACTIVE_STATES from gen.apache.aurora.api.ttypes import ( ConfigGroup, + Constraint, GetJobUpdateDiffResult, Range, Result, @@ -66,9 +66,10 @@ class TestDiffFormatter(AuroraClientCommandTest): )) @classmethod - def get_job_update_diff_result(cls): + def get_job_update_diff_result(cls, task=None): diff = cls.create_simple_success_response() - task = cls.create_task_config('foo') + if task is None: + task = cls.create_task_config('foo') diff.result = Result(getJobUpdateDiffResult=GetJobUpdateDiffResult( add=set([ConfigGroup( config=task, @@ -102,26 +103,48 @@ class TestDiffFormatter(AuroraClientCommandTest): self._fake_context.get_job_config = Mock(return_value=config) formatter = DiffFormatter(self._fake_context, config) local_task = self.create_scheduled_tasks()[0].assignedTask.task + local_task.constraints = set([Constraint(name='host'), Constraint(name='rack')]) self._mock_api.get_job_update_diff.return_value = self.get_job_update_diff_result() - with contextlib.nested( - patch('subprocess.call', return_value=0), - patch('json.loads', return_value={})) as (subprocess_patch, _): - - formatter.show_job_update_diff(self._mock_options.instance_spec.instance, local_task) - - assert self._mock_api.get_job_update_diff.mock_calls == [ - call(config, self._mock_options.instance_spec.instance) - ] - assert "\n".join(self._fake_context.get_out()) == textwrap.dedent("""\ - This job update will: - add instances: [10], [12-14] - update instances: [11] - with diff:\n\n - not change instances: [0-9]""") - assert subprocess_patch.call_count == 1 - assert subprocess_patch.call_args[0][0].startswith( - os.environ.get('DIFF_VIEWER', 'diff') + ' ') + formatter.show_job_update_diff(self._mock_options.instance_spec.instance, local_task) + + assert self._mock_api.get_job_update_diff.mock_calls == [ + call(config, self._mock_options.instance_spec.instance) + ] + assert "\n".join(self._fake_context.get_out()) == textwrap.dedent("""\ + This job update will: + add instances: [10], [12-14] + update instances: [11] + with diff:\n + 2c2,3 + < 'constraints': None, + --- + > 'constraints': [ Constraint(name='host', constraint=None), + > Constraint(name='rack', constraint=None)], + \n + not change instances: [0-9]""") + + def test_show_job_update_diff_no_diff_out_of_order_constraints(self): + config = self.get_job_config() + self._fake_context.get_job_config = Mock(return_value=config) + formatter = DiffFormatter(self._fake_context, config) + local_task = self.create_scheduled_tasks()[0].assignedTask.task + local_task.constraints = set([Constraint(name='host'), Constraint(name='rack')]) + remote_task = deepcopy(local_task) + remote_task.constraints = set([Constraint(name='rack'), Constraint(name='host')]) + self._mock_api.get_job_update_diff.return_value = self.get_job_update_diff_result(remote_task) + + formatter.show_job_update_diff(self._mock_options.instance_spec.instance, local_task) + + assert self._mock_api.get_job_update_diff.mock_calls == [ + call(config, self._mock_options.instance_spec.instance) + ] + assert "\n".join(self._fake_context.get_out()) == textwrap.dedent("""\ + This job update will: + add instances: [10], [12-14] + update instances: [11] + with diff:\n\n\n + not change instances: [0-9]""") def test_show_job_update_diff_without_task_diff(self): config = self.get_job_config() @@ -181,12 +204,4 @@ class TestDiffFormatter(AuroraClientCommandTest): self._mock_api.build_query.return_value = query local_tasks = [] - with contextlib.nested( - patch('subprocess.call', return_value=0), - patch('json.loads', return_value={})) as (subprocess_patch, _): - - formatter.diff_no_update_details(local_tasks) - - assert subprocess_patch.call_count == 1 - assert subprocess_patch.call_args[0][0].startswith( - os.environ.get('DIFF_VIEWER', 'diff') + ' ') + formatter.diff_no_update_details(local_tasks) http://git-wip-us.apache.org/repos/asf/aurora/blob/42a49743/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 c65ae75..43db828 100644 --- a/src/test/python/apache/aurora/client/cli/util.py +++ b/src/test/python/apache/aurora/client/cli/util.py @@ -224,7 +224,7 @@ class AuroraClientCommandTest(unittest.TestCase): def create_task_config(cls, name): return TaskConfig( maxTaskFailures=1, - executorConfig=ExecutorConfig(data='fake data'), + executorConfig=ExecutorConfig(data='{"fake": "data"}'), metadata=[], job=JobKey(role=cls.TEST_ROLE, environment=cls.TEST_ENV, name=name), numCpus=2,
