From f9a45dfa5b0f925d168e3ccd61ad1c42c9b2c091 Mon Sep 17 00:00:00 2001 From: tamarrow Date: Mon, 24 Oct 2016 11:13:52 -0700 Subject: [PATCH] pods: read from task log not executor (#815) --- cli/tests/integrations/common.py | 107 +++++++++++++++--- cli/tests/integrations/test_marathon_pod.py | 81 +++---------- cli/tests/integrations/test_task.py | 19 +++- .../data/binary-test/bin/dcos-fake-binary | 0 cli/tests/unit/data/helloworld-resource.json | 37 ++++++ dcos/mesos.py | 12 +- 6 files changed, 174 insertions(+), 82 deletions(-) create mode 100644 cli/tests/unit/data/binary-test/bin/dcos-fake-binary create mode 100644 cli/tests/unit/data/helloworld-resource.json diff --git a/cli/tests/integrations/common.py b/cli/tests/integrations/common.py index 9ac5e5c..36e6ceb 100644 --- a/cli/tests/integrations/common.py +++ b/cli/tests/integrations/common.py @@ -151,17 +151,17 @@ def wait_for_service(service_name, number_of_services=1, max_count=300): count += 1 -def add_job(app_path): +def add_job(job_path): """ Add a job, and wait for it to deploy - :param app_path: path to job's json definition - :type app_path: str + :param job_path: path to job's json definition + :type job_path: str :param wait: whether to wait for the deploy :type wait: bool :rtype: None """ - assert_command(['dcos', 'job', 'add', app_path]) + assert_command(['dcos', 'job', 'add', job_path]) def add_app(app_path, wait=True): @@ -202,16 +202,32 @@ def remove_app(app_id): assert_command(['dcos', 'marathon', 'app', 'remove', '--force', app_id]) -def remove_job(app_id): +def remove_pod(pod_id, force=True): + """ Remove a pod + + :param pod_id: id of app to remove + :type pod_id: str + :param force: whether to force a remove + :type force: bool + :rtype: None + """ + + cmd = ['dcos', 'marathon', 'pod', 'remove', pod_id] + if force: + cmd += ['--force'] + assert_command(cmd) + + +def remove_job(job_id): """ Remove a job - :param app_id: id of app to remove - :type app_id: str + :param job_id: id of job to remove + :type job_id: str :rtype: None """ assert_command(['dcos', 'job', 'remove', - '--stop-current-job-runs', app_id]) + '--stop-current-job-runs', job_id]) def package_install(package, deploy=False, args=[]): @@ -522,15 +538,78 @@ def app(path, app_id, wait=True): watch_all_deployments() +def add_pod(pod_path, wait=True): + """Add a pod, and wait for it to deploy + + :param pod_path: path to pod's json definition + :type pod_path: str + :param wait: whether to wait for the deploy + :type wait: bool + :rtype: None + """ + + cmd = ['dcos', 'marathon', 'pod', 'add', pod_path] + returncode, stdout, stderr = exec_command(cmd) + assert returncode == 0 + assert re.fullmatch('Created deployment \S+\n', stdout.decode('utf-8')) + assert stderr == b'' + + if wait: + watch_all_deployments() + + @contextlib.contextmanager -def job(path, app_id): - """Context manager that deploys an app on entrance, and removes it on +def pod(path, pod_id, wait=True): + """Context manager that deploys an pod on entrance, and removes it on exit + + :param path: path to pod's json definition: + :type path: str + :param pod_id: pod id + :type pod_id: str + :param wait: whether to wait for the deploy + :type wait: bool + :rtype: None + """ + + add_pod(path, wait) + try: + yield + finally: + remove_pod(pod_id) + watch_all_deployments() + + +@contextlib.contextmanager +def pods(pods): + """Context manager that deploys pods on entrance, and removes + them on exit. + + :param pods: dict of path/to/pod/json -> pod id + :type pods: {} + :rtype: None + """ + + for pod_path in pods: + add_pod(pod_path, wait=False) + watch_all_deployments() + + try: + yield + finally: + for pod_id in list(pods.values()): + remove_pod(pod_id) + watch_all_deployments() + + +@contextlib.contextmanager +def job(path, job_id): + """Context manager that deploys a job on entrance, and removes it on exit. - :param path: path to app's json definition: + :param path: path to job's json definition: :type path: str - :param app_id: app id - :type app_id: str + :param job_id: job id + :type job_id: str :param wait: whether to wait for the deploy :type wait: bool :rtype: None @@ -540,7 +619,7 @@ def job(path, app_id): try: yield finally: - remove_job(app_id) + remove_job(job_id) @contextlib.contextmanager diff --git a/cli/tests/integrations/test_marathon_pod.py b/cli/tests/integrations/test_marathon_pod.py index 5cd8858..718e2d3 100644 --- a/cli/tests/integrations/test_marathon_pod.py +++ b/cli/tests/integrations/test_marathon_pod.py @@ -1,4 +1,3 @@ -import contextlib import json import os import re @@ -6,7 +5,8 @@ import time import pytest -from .common import (assert_command, exec_command, file_json_ast, +from .common import (add_pod, assert_command, exec_command, + file_json_ast, pod, pods, remove_pod, watch_all_deployments) from ..fixtures.marathon import (DOUBLE_POD_FILE_PATH, DOUBLE_POD_ID, GOOD_POD_FILE_PATH, GOOD_POD_ID, @@ -28,24 +28,16 @@ _POD_UPDATE_CMD = _POD_BASE_CMD + ['update'] @pytest.mark.skipif(not _PODS_ENABLED, reason="Requires pods") -def test_pod_add_from_file_then_remove(): - returncode, stdout, stderr = _pod_add_from_file(GOOD_POD_FILE_PATH) - assert returncode == 0 - assert re.fullmatch('Created deployment \S+\n', stdout.decode('utf-8')) - assert stderr == b'' - - watch_all_deployments() - - # Explicitly testing non-forced-removal; can't use the context manager - _assert_pod_remove(GOOD_POD_ID, extra_args=[]) +def test_pod_add_from_file(): + add_pod(GOOD_POD_FILE_PATH) + remove_pod(GOOD_POD_ID, force=False) watch_all_deployments() @pytest.mark.skipif(not _PODS_ENABLED, reason="Requires pods") -def test_pod_add_from_stdin_then_force_remove(): - # Explicitly testing adding from stdin; can't use the context manager - _assert_pod_add_from_stdin(GOOD_POD_FILE_PATH) - _assert_pod_remove(GOOD_POD_ID, extra_args=['--force']) +def test_pod_add_from_stdin(): + _pod_add_from_stdin(GOOD_POD_FILE_PATH) + remove_pod(GOOD_POD_ID) watch_all_deployments() @@ -53,9 +45,9 @@ def test_pod_add_from_stdin_then_force_remove(): def test_pod_list(): expected_json = pod_list_fixture() - with _pods({GOOD_POD_ID: GOOD_POD_FILE_PATH, - DOUBLE_POD_ID: DOUBLE_POD_FILE_PATH, - TRIPLE_POD_ID: TRIPLE_POD_FILE_PATH}): + with pods({GOOD_POD_FILE_PATH: GOOD_POD_ID, + DOUBLE_POD_FILE_PATH: DOUBLE_POD_ID, + TRIPLE_POD_FILE_PATH: TRIPLE_POD_ID}): _assert_pod_list_json(expected_json) _assert_pod_list_table() @@ -65,7 +57,7 @@ def test_pod_list(): def test_pod_show(): expected_json = file_json_ast(GOOD_POD_STATUS_FILE_PATH) - with _pod(GOOD_POD_ID, GOOD_POD_FILE_PATH): + with pod(GOOD_POD_FILE_PATH, GOOD_POD_ID): _assert_pod_show(GOOD_POD_ID, expected_json) @@ -80,7 +72,7 @@ def test_pod_update_does_not_support_properties(): @pytest.mark.skipif(not _PODS_ENABLED, reason="Requires pods") def test_pod_update_from_stdin(): - with _pod(GOOD_POD_ID, GOOD_POD_FILE_PATH): + with pod(GOOD_POD_FILE_PATH, GOOD_POD_ID): # The deployment will never complete _assert_pod_update_from_stdin( extra_args=[], @@ -96,7 +88,7 @@ def test_pod_update_from_stdin(): @pytest.mark.skipif(not _PODS_ENABLED, reason="Requires pods") def test_pod_kill(): - with _pod(POD_KILL_ID, POD_KILL_FILE_PATH): + with pod(POD_KILL_FILE_PATH, POD_KILL_ID): kill_1, keep, kill_2 = _get_pod_instance_ids(POD_KILL_ID, 3) remove_args = [POD_KILL_ID, kill_1, kill_2] @@ -110,12 +102,7 @@ def test_pod_kill(): assert len(new_instance_ids) == 3 -def _pod_add_from_file(file_path): - cmd = _POD_ADD_CMD + [file_path] - return exec_command(cmd) - - -def _assert_pod_add_from_stdin(file_path): +def _pod_add_from_stdin(file_path): cmd = _POD_ADD_CMD with open(file_path) as fd: returncode, stdout, stderr = exec_command(cmd, stdin=fd) @@ -237,11 +224,6 @@ def _assert_pod_list_table(): assert len(stdout_lines) == 11 -def _assert_pod_remove(pod_id, extra_args): - cmd = _POD_REMOVE_CMD + [pod_id] + extra_args - assert_command(cmd, returncode=0, stdout=b'', stderr=b'') - - def _assert_pod_show(pod_id, expected_json): cmd = _POD_SHOW_CMD + [pod_id] returncode, stdout, stderr = exec_command(cmd) @@ -263,35 +245,6 @@ def _assert_pod_update_from_stdin(extra_args, pod_json_file_path): assert stderr == b'' -@contextlib.contextmanager -def _pod(pod_id, file_path): - with _pods({pod_id: file_path}): - yield - - -@contextlib.contextmanager -def _pods(ids_and_paths): - ids_and_results = {} - for pod_id, file_path in ids_and_paths.items(): - ids_and_results[pod_id] = _pod_add_from_file(file_path) - - try: - for pod_id, (returncode, stdout, stderr) in ids_and_results.items(): - assert returncode == 0 - assert re.fullmatch('Created deployment \S+\n', - stdout.decode('utf-8')) - assert stderr == b'' - - watch_all_deployments() - yield - finally: - for pod_id, results in ids_and_results.items(): - returncode, _, _ = results - if returncode == 0: - _assert_pod_remove(pod_id, extra_args=['--force']) - watch_all_deployments() - - def _wait_for_instances(expected_instances, max_attempts=10): """Polls the `pod list` command until the instance counts are as expected. @@ -309,8 +262,8 @@ def _wait_for_instances(expected_instances, max_attempts=10): assert stderr == b'' status_json = json.loads(stdout.decode('utf-8')) - actual_instances = {pod['id']: len(pod.get('instances', [])) - for pod in status_json} + actual_instances = {pod_obj['id']: len(pod_obj.get('instances', [])) + for pod_obj in status_json} if actual_instances == expected_instances: return diff --git a/cli/tests/integrations/test_task.py b/cli/tests/integrations/test_task.py index 7b9e285..fc8e8a4 100644 --- a/cli/tests/integrations/test_task.py +++ b/cli/tests/integrations/test_task.py @@ -12,7 +12,7 @@ import dcos.util as util from dcos.util import create_schema from .common import (add_app, app, assert_command, assert_lines, exec_command, - remove_app, watch_all_deployments) + pod, remove_app, watch_all_deployments) from ..fixtures.task import task_fixture SLEEP_COMPLETED = 'tests/data/marathon/apps/sleep-completed.json' @@ -122,6 +122,23 @@ def test_log_single_file(): assert len(stdout.decode('utf-8').split('\n')) > 0 +@pytest.mark.skipif('DCOS_PODS_ENABLED' not in os.environ, + reason="Requires pods") +def test_log_pod_task(): + good_pod_file = 'tests/data/marathon/pods/good.json' + with pod(good_pod_file, 'good-pod'): + + returncode, stdout, stderr = exec_command( + ['dcos', 'task', 'log', 'good-container', 'stderr']) + + # pod task log are not executor logs, so normal executor stderr + # logs shouldn't be seen and this pod shoudn't have any logging + # to stderr + assert returncode == 1 + assert stderr == b'No files exist. Exiting.\n' + assert stdout == b'' + + def test_log_missing_file(): """ Tail a single file on a single task """ returncode, stdout, stderr = exec_command( diff --git a/cli/tests/unit/data/binary-test/bin/dcos-fake-binary b/cli/tests/unit/data/binary-test/bin/dcos-fake-binary new file mode 100644 index 0000000..e69de29 diff --git a/cli/tests/unit/data/helloworld-resource.json b/cli/tests/unit/data/helloworld-resource.json new file mode 100644 index 0000000..a625680 --- /dev/null +++ b/cli/tests/unit/data/helloworld-resource.json @@ -0,0 +1,37 @@ +{ + "assets": { + "container": { + "docker": { + "python-3-image": "python:3" + } + } + }, + "cli": { + "binaries": { + "darwin": { + "x86-64": { + "contentHash": [ + { + "algo": "sha256", + "value": "80cdf124c718b33c0aac9e4876e03545add3384445fce37af5b1f07fbffda6f7" + } + ], + "kind": "executable", + "url": "https://downloads.dcos.io/cli/binaries/darwin/x86-64/0/dcos-helloworld" + } + }, + "linux": { + "x86-64": { + "contentHash": [ + { + "algo": "sha256", + "value": "97456616dc6255e6365dba7dae3f984d0c5194918405838762978725f49a3132" + } + ], + "kind": "executable", + "url": "https://downloads.dcos.io/cli/binaries/linux/x86-64/0/dcos-helloworld" + } + } + } + } +} diff --git a/dcos/mesos.py b/dcos/mesos.py index f2d14f7..cb83869 100644 --- a/dcos/mesos.py +++ b/dcos/mesos.py @@ -717,6 +717,7 @@ class Task(object): :returns: path to task's sandbox :rtype: str """ + return self.executor()['directory'] def __getitem__(self, name): @@ -840,9 +841,14 @@ class MesosFile(object): """ if self._task: - directory = self._task.directory() - if directory[-1] == '/': - return directory + self._path + directory = self._task.directory().rstrip('/') + executor = self._task.executor() + # executor.type is currently used only by pods. All tasks in a pod + # share an executor, so if this is a pod, get the task logs instead + # of the executor logs + if executor.get('type') == "DEFAULT": + task_id = self._task.dict().get('id') + return directory + '/tasks/{}/'.format(task_id) + self._path else: return directory + '/' + self._path else: