From 00a4f832b36f83cf48fce13a1a77dcb1d79430fd Mon Sep 17 00:00:00 2001 From: Santanu Sinha Date: Sun, 3 May 2026 17:28:02 +0530 Subject: [PATCH] Minor fixes in log tailing and describe --- README.md | 7 ++++ drove-cli/SKILL.md | 6 ++- drove.py | 33 ---------------- drovecli.py | 3 +- plugins/describe.py | 24 +++++++----- plugins/tasks.py | 4 +- tests/mock_server.py | 36 +++-------------- tests/test_offline_appinstances.py | 27 +++++++++++++ tests/test_offline_localservices.py | 28 +++++++++++++ tests/test_offline_tasks.py | 61 +++++++++++++++++++++++++++++ 10 files changed, 152 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 38c9f6d..1d44dc7 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,13 @@ drove -c prod apps info Use `drove -h` or `drove -h` for detailed help. +### Task Logs + +```bash +# Tail task logs (default log file: output.log) +drove tasks tail --log output.log +``` + ## Global Options ``` diff --git a/drove-cli/SKILL.md b/drove-cli/SKILL.md index b2d16b0..b5a3d65 100644 --- a/drove-cli/SKILL.md +++ b/drove-cli/SKILL.md @@ -131,14 +131,14 @@ drove apps destroy MY_APP-1 ### Tail logs from a running instance ```bash drove appinstances list MY_APP-1 # find instance IDs -drove appinstances tail MY_APP-1 AI-abc123 +drove appinstances tail MY_APP-1 AI-abc123 --log output.log ``` ### Run a one-shot task and monitor it ```bash drove tasks create my_task.json drove tasks show MY_APP T0042 -drove tasks tail MY_APP T0042 +drove tasks tail MY_APP T0042 --log output.log ``` ### Activate a local service @@ -147,6 +147,8 @@ drove localservices create my_service.json drove localservices conftest MY_SVC-1 # validate with one instance first drove localservices activate MY_SVC-1 drove localservices summary MY_SVC-1 +drove lsinstances list MY_SVC-1 # find local service instance IDs +drove lsinstances tail MY_SVC-1 SI-abc123 --log output.log ``` --- diff --git a/drove.py b/drove.py index d3d27ba..1199a68 100755 --- a/drove.py +++ b/drove.py @@ -53,36 +53,3 @@ def run(): if __name__ == '__main__': run() - - -def get_parser(): - parser = build_parser() - client = None - client = drovecli.DroveCli(parser) - return client.parser - - -def run(): - parser = build_parser() - client = None - try: - client = drovecli.DroveCli(parser) - client.run() - except (BrokenPipeError, IOError, KeyboardInterrupt): - pass - except droveclient.DroveException as e: - debug = True if client is not None and client.debug else False - droveutils.print_drove_error(e, debug) - if debug: - traceback.print_exc() - - except Exception as e: - print("Drove CLI error: " + str(e)) - debug = True if client is not None and client.debug else False - if debug: - traceback.print_exc() - else: - parser.print_help() - -if __name__ == '__main__': - run() diff --git a/drovecli.py b/drovecli.py index 344fd19..d752048 100644 --- a/drovecli.py +++ b/drovecli.py @@ -51,7 +51,8 @@ def run(self): if args.plugin: plugin = self.plugins.get(args.plugin) if plugin and plugin.needs_client(): - droveclient.build_drove_client(plugin.drove_client, args) + if droveclient.build_drove_client(plugin.drove_client, args) is None: + return args.func(args) def show_help(self, options: SimpleNamespace) -> None: diff --git a/plugins/describe.py b/plugins/describe.py index dca4c0f..d5f33f3 100644 --- a/plugins/describe.py +++ b/plugins/describe.py @@ -325,10 +325,12 @@ def describe_instance(self, options: SimpleNamespace): print(f" Updated: {droveutils.to_date(raw.get('updated', 0))}") local_info = raw.get('localInfo', {}) - if local_info: + host_name = local_info.get('hostname') or raw.get('hostname') + executor_id = local_info.get('executorId') or raw.get('executorId') + if host_name or executor_id: self._print_section("Host Information") - print(f" Hostname: {local_info.get('hostname', 'N/A')}") - print(f" Executor ID: {local_info.get('executorId', 'N/A')}") + print(f" Hostname: {host_name or 'N/A'}") + print(f" Executor ID: {executor_id or 'N/A'}") ports = local_info.get('ports', {}) if ports: @@ -396,10 +398,12 @@ def describe_task(self, options: SimpleNamespace): print(f" Updated: {droveutils.to_date(raw.get('updated', 0))}") local_info = raw.get('localInfo', {}) - if local_info: + host_name = local_info.get('hostname') or raw.get('hostname') + executor_id = local_info.get('executorId') or raw.get('executorId') + if host_name or executor_id: self._print_section("Host Information") - print(f" Hostname: {local_info.get('hostname', 'N/A')}") - print(f" Executor ID: {local_info.get('executorId', 'N/A')}") + print(f" Hostname: {host_name or 'N/A'}") + print(f" Executor ID: {executor_id or 'N/A'}") ports = local_info.get('ports', {}) if ports: @@ -547,10 +551,12 @@ def describe_lsinstance(self, options: SimpleNamespace): print(f" Updated: {droveutils.to_date(raw.get('updated', 0))}") local_info = raw.get('localInfo', {}) - if local_info: + host_name = local_info.get('hostname') or raw.get('hostname') + executor_id = local_info.get('executorId') or raw.get('executorId') + if host_name or executor_id: self._print_section("Host Information") - print(f" Hostname: {local_info.get('hostname', 'N/A')}") - print(f" Executor ID: {local_info.get('executorId', 'N/A')}") + print(f" Hostname: {host_name or 'N/A'}") + print(f" Executor ID: {executor_id or 'N/A'}") ports = local_info.get('ports', {}) if ports: diff --git a/plugins/tasks.py b/plugins/tasks.py index 4067385..86e9f4b 100644 --- a/plugins/tasks.py +++ b/plugins/tasks.py @@ -48,7 +48,7 @@ def populate_options(self, drove_client: droveclient.DroveClient, subparser: arg sub_parser = commands.add_parser("tail", help="Tail log for task") sub_parser.add_argument("source_app", metavar="source-app", help="Name of the Drove application that started the task") sub_parser.add_argument("task_id", metavar="task-id", help="Task ID") - sub_parser.add_argument("--file", "-f", default = "output.log", help="Log filename to tail. Default is to tail output.log") + sub_parser.add_argument("--log", "-l", default = "output.log", help="Log filename to tail. Default is to tail output.log") sub_parser.set_defaults(func=self.log_tail) sub_parser = commands.add_parser("download", help="Download log for task") @@ -149,7 +149,7 @@ def show_logs_list(self, options): droveutils.list_logs(self.drove_client, "tasks", options.source_app, options.task_id) def log_tail(self, options): - droveutils.tail_log(self.drove_client, "tasks", options.source_app, options.task_id, options.file) + droveutils.tail_log(self.drove_client, "tasks", options.source_app, options.task_id, options.log) def log_download(self, options): filename = options.tasklogfile diff --git a/tests/mock_server.py b/tests/mock_server.py index 08dcd14..72a69fd 100644 --- a/tests/mock_server.py +++ b/tests/mock_server.py @@ -149,16 +149,8 @@ "appId": "TEST_APP-1", "appName": "TEST_APP", "executorId": EXECUTOR_ID, - "localInfo": { - "hostname": "exec-host-1", - "ports": { - "main": { - "hostPort": 32000, - "containerPort": 8080, - "portType": "TCP", - } - }, - }, + "hostname": "exec-host-1", + "localInfo": {}, "state": "HEALTHY", "errorMessage": "", "resources": [ @@ -347,16 +339,8 @@ def scale_app(self, app_id: str, count: int): "appId": app_id, "appName": app["summary"]["name"], "executorId": self.executor_info["state"]["executorId"], - "localInfo": { - "hostname": "exec-host-1", - "ports": { - "main": { - "hostPort": 32000 + i, - "containerPort": 8080, - "portType": "TCP", - } - }, - }, + "hostname": "exec-host-1", + "localInfo": {}, "state": "HEALTHY", "errorMessage": "", "resources": [ @@ -440,16 +424,8 @@ def activate_ls(self, svc_id: str): "serviceId": svc_id, "serviceName": svc["summary"]["name"], "executorId": self.executor_info["state"]["executorId"], - "localInfo": { - "hostname": "exec-host-1", - "ports": { - "main": { - "hostPort": 33000, - "containerPort": 8080, - "portType": "TCP", - } - }, - }, + "hostname": "exec-host-1", + "localInfo": {}, "state": "HEALTHY", "errorMessage": "", "resources": [ diff --git a/tests/test_offline_appinstances.py b/tests/test_offline_appinstances.py index 7aeab6b..feee521 100644 --- a/tests/test_offline_appinstances.py +++ b/tests/test_offline_appinstances.py @@ -8,6 +8,9 @@ The mock server seeds TEST_APP-1 with one HEALTHY instance (AI-test-app-inst-001). """ import json +import os +import subprocess +from pathlib import Path import pytest pytestmark = pytest.mark.offline @@ -16,6 +19,23 @@ SEEDED_INST_ID = "AI-test-app-inst-001" +def local_drove(*args, timeout=30): + cli_dir = Path(__file__).resolve().parents[1] + cmd = ["python3", "drove.py"] + endpoint = os.environ.get("DROVE_ENDPOINT") + if endpoint: + cmd += ["-e", endpoint] + cmd += list(args) + return subprocess.run( + cmd, + capture_output=True, + text=True, + timeout=timeout, + cwd=str(cli_dir), + env=os.environ.copy(), + ) + + def _get_instance_id(app_id: str) -> str: """Return the first AI- instance ID for the given app from mock server.""" from conftest import drove_ok @@ -103,3 +123,10 @@ def test_describe_instance_contains_app_id(self, offline_env): from conftest import drove_ok out = drove_ok("describe", "instance", SEEDED_APP_ID, SEEDED_INST_ID) assert "TEST_APP" in out or SEEDED_INST_ID in out + + def test_describe_instance_uses_top_level_host_fields(self, offline_env): + result = local_drove("describe", "instance", SEEDED_APP_ID, SEEDED_INST_ID) + assert result.returncode == 0 + assert "Hostname:" in result.stdout + assert "exec-host-1" in result.stdout + assert "Executor ID:" in result.stdout diff --git a/tests/test_offline_localservices.py b/tests/test_offline_localservices.py index 3b67b96..f02f9d7 100644 --- a/tests/test_offline_localservices.py +++ b/tests/test_offline_localservices.py @@ -11,6 +11,9 @@ not conflict with the seeded EXISTING_SVC (TEST_LOCAL_SERVICE-1). """ import json +import os +import subprocess +from pathlib import Path import pytest pytestmark = pytest.mark.offline @@ -19,6 +22,23 @@ CLI_SVC_ID = "OFFLINE_TEST_SERVICE-1" # lifecycle tests use offline_test_service.json +def local_drove(*args, timeout=30): + cli_dir = Path(__file__).resolve().parents[1] + cmd = ["python3", "drove.py"] + endpoint = os.environ.get("DROVE_ENDPOINT") + if endpoint: + cmd += ["-e", endpoint] + cmd += list(args) + return subprocess.run( + cmd, + capture_output=True, + text=True, + timeout=timeout, + cwd=str(cli_dir), + env=os.environ.copy(), + ) + + # --------------------------------------------------------------------------- # Smoke / read-only tests against the seeded inactive service # --------------------------------------------------------------------------- @@ -240,3 +260,11 @@ def test_describe_lsinstance_json(self, seeded_active_svc): inst_id, "--json") data = json.loads(out) assert isinstance(data, dict) + + def test_describe_lsinstance_uses_top_level_host_fields(self, seeded_active_svc): + inst_id = _get_ls_instance_id(seeded_active_svc) + result = local_drove("describe", "lsinstance", seeded_active_svc, inst_id) + assert result.returncode == 0 + assert "Hostname:" in result.stdout + assert "exec-host-1" in result.stdout + assert "Executor ID:" in result.stdout diff --git a/tests/test_offline_tasks.py b/tests/test_offline_tasks.py index fcae766..8fbe993 100644 --- a/tests/test_offline_tasks.py +++ b/tests/test_offline_tasks.py @@ -10,6 +10,10 @@ `tasks show` works for both RUNNING and terminal-state tasks. """ import json +import os +import subprocess +import sys +from pathlib import Path import pytest pytestmark = pytest.mark.offline @@ -21,6 +25,23 @@ from conftest import TASK_SOURCE, TASK_ID, APP_SPEC, TASK_SPEC +def local_drove(*args, timeout=30): + cli_dir = Path(__file__).resolve().parents[1] + cmd = ["python3", "drove.py"] + endpoint = os.environ.get("DROVE_ENDPOINT") + if endpoint: + cmd += ["-e", endpoint] + cmd += list(args) + return subprocess.run( + cmd, + capture_output=True, + text=True, + timeout=timeout, + cwd=str(cli_dir), + env=os.environ.copy(), + ) + + @pytest.fixture(scope="module") def app_for_offline_tasks(offline_env): """ @@ -119,3 +140,43 @@ def test_describe_task_json(self, app_for_offline_tasks): if result.returncode == 0: data = json.loads(result.stdout) assert isinstance(data, dict) + +class TestOfflineTaskCliRegressions: + def test_tasks_tail_log_flag_does_not_override_global_config_file(self): + cli_dir = Path(__file__).resolve().parents[1] + sys.path.insert(0, str(cli_dir)) + try: + import drove as drove_entry + import drovecli as drove_cli + + parser = drove_entry.build_parser() + client = drove_cli.DroveCli(parser) + args = client.parser.parse_args([ + "tasks", "tail", TASK_SOURCE, TASK_ID, "--log", "output.log" + ]) + + assert args.log == "output.log" + assert args.file is None + finally: + sys.path.pop(0) + + def test_describe_task_uses_top_level_host_fields(self, app_for_offline_tasks): + from conftest import drove, drove_ok + + drove("tasks", "kill", TASK_SOURCE, TASK_ID, check=False, timeout=10) + drove_ok("tasks", "create", TASK_SPEC, timeout=10) + + result = local_drove("describe", "task", TASK_SOURCE, TASK_ID, timeout=10) + assert result.returncode == 0 + assert "Hostname:" in result.stdout + assert "exec-host-1" in result.stdout + assert "Executor ID:" in result.stdout + + def test_missing_config_file_does_not_trigger_none_type_crash(self): + result = local_drove( + "-f", "/tmp/__missing_drove_config__.json", "tasks", "list", timeout=10 + ) + + assert "unsupported operand type" not in result.stdout.lower() + assert "unsupported operand type" not in result.stderr.lower() +