[Core] Add sky debug-dump command for troubleshooting#8868
Conversation
Add `sky debug-dump` command to collect diagnostic information for troubleshooting SkyPilot issues. Creates a zip file containing logs, state, and configuration for specified clusters, requests, and/or managed jobs. Key implementation decisions: 1. Persistent storage: Debug dumps are stored in ~/.sky/debug_dumps/ rather than temp directories to allow re-downloading and prevent accidental deletion. This fixed a bug in the skeleton where the zip was created inside tempdir() context but returned after exit. 2. Cross-linking: When dumping a cluster, we automatically include associated requests, and vice versa. This ensures complete context without requiring users to manually specify all related resources. 3. --recent flag: Added `--recent N` option to dump all resources active within the last N hours, making it easy to capture recent activity without knowing specific IDs. 4. Server-side collection: Dumps are created on the API server and downloaded to the client, ensuring access to server-side logs and state even for remote API server deployments. 5. Graceful degradation: Each collection function catches exceptions and continues, creating partial dumps when some data is unavailable. 6. Debug logging: Added comprehensive debug logs throughout the dump flow to help troubleshoot issues with debug-dump itself. Files changed: - sky/utils/debug_utils.py: Core dump logic with 8 collection functions - sky/server/server.py: /debug/dump-download and /debug/dump-list endpoints - sky/client/sdk.py: create_debug_dump, download_debug_dump, list_debug_dumps - sky/client/cli/command.py: `sky debug-dump` command with full CLI - sky/core.py: Updated create_debug_dump with recent_hours param - sky/server/requests/payloads.py: Added recent_hours to request body Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Bump API_VERSION to 27 and add @versions.minimal_api_version(27) decorator to all debug dump SDK functions. This ensures users get a clear error message if they try to use these new functions against an older server that doesn't support the debug dump endpoints. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Simplify debug dump lifecycle with 1:1 create-download model: - Delete dump file automatically after successful download using starlette BackgroundTask - Remove list_debug_dumps SDK function and /debug/dump-list endpoint since dumps are ephemeral and don't accumulate This avoids the need for a separate cleanup/retention policy. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Include a summary file at the root of each debug dump containing: - requested: Original input parameters (request_ids, cluster_names, managed_job_ids, recent_hours) - collected: What was actually gathered after cross-linking, including counts and full lists of IDs/names - timing: Elapsed time and timestamp This helps users quickly understand the scope of the dump without manually inspecting each subdirectory. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Include client_info.json in debug dumps with: - SkyPilot version and commit - Python version and platform - User hash - Environment variables (SKYPILOT_DEBUG, SKYPILOT_DEV) - User config and merged config (with API endpoints redacted) The client gathers this info and sends it to the server in the request payload. This helps diagnose issues where client and server environments differ (e.g., remote API server scenarios). API endpoints in config are replaced with "<redacted>" to avoid exposing HTTP basic auth credentials or other sensitive info. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add SYSTEM_REQUEST_IDS constant and automatically include these requests in every debug dump: - skypilot-status-refresh-daemon - skypilot-volume-status-refresh-daemon - skypilot-server-on-boot-check These system requests are relevant for troubleshooting regardless of which specific resources the user requested to dump. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @cg505, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new sky debug-dump command, which is a valuable tool for troubleshooting. The implementation is comprehensive, collecting logs, state, and configuration for specified resources. The code is well-structured, with a new debug_utils.py module containing the core logic. Security considerations, such as preventing path traversal, have been taken into account. I have one suggestion for code simplification in sky/utils/debug_utils.py to improve maintainability. Overall, this is a solid contribution.
Resolve API_VERSION conflict: bump to 32 for debug dump endpoints. Update minimal_api_version decorators from 27 to 32.
- Fix managed job data access for remote controllers by using queue_v2() instead of direct DB queries - Change debug dump schedule type from SHORT to LONG for credential checks - Add orphan cleanup for dumps older than 1 hour - Enrich server_info.json with python version, OS platform, DB backend, and server uptime (via boot check request) - Surface collection errors in errors.json and summary.json warnings - Add human-readable timestamps alongside epoch values - Improve config redaction using config_utils.Config pattern - Add debug logging throughout dump creation - Dump all tasks for multi-task managed jobs - Add 48 unit tests for cross-linking and dump creation functions - Add 5 smoke tests covering --recent, -c, -r, -j flags and no-args error Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # sky/server/constants.py # sky/server/server.py
cg505
left a comment
There was a problem hiding this comment.
It would be great if there is a way for us to pull the helm values for the API deployment if possible as well.
…quest IDs Replace hardcoded SYSTEM_REQUEST_IDS list with dynamic construction from INTERNAL_REQUEST_DAEMONS and a new ON_BOOT_CHECK_REQUEST_ID constant, so debug dumps automatically include any new daemon request IDs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove read-only JOBS_QUEUE from request filter. Add matching for cancel-by-name, cancel-all (with user check), and cancel-all-users requests by fetching job details via queue_v2. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unnecessary try/except around JOB_CONTROLLER_NAME - Simplify _get_clusters_from_managed_jobs to just add controller name (actual job cluster info handled by _dump_managed_job_info) - Remove redundant created_at check in _populate_recent_context - Remove cluster_name gathering from _populate_recent_context (handled by _get_clusters_from_requests during cross-linking) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oss-link to debug dump Address review comments: add server environment variables to dump, copy debug log files from _DEBUG_LOG_DIR, add requests→jobs cross-linking via _get_managed_jobs_from_requests, wire errors through DebugDumpContext so cross-linking and recent-context functions record failures, rename 'warnings' to 'errors' in summary, add debug log for missing client info. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move all import-outside-toplevel imports to top-level (debug_utils is only imported server-side so no circular import risk). Remove inner try/except in _get_requests_from_managed_jobs since all attribute access uses getattr with defaults. Add error recording to debug log copy except block. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract _build_debug_dump helper for cross-linking, dumping all sections, and writing summary. Add a FileHandler at DEBUG level on the debug_utils logger during dump creation so all debug logs are captured into debug_dump.log in the zip. Handler is cleaned up in a finally block (remove, flush, close) to avoid clobbering existing loggers. No logger level changes needed since effective level is already DEBUG via inheritance from root sky logger. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a new gRPC RPC to ManagedJobsService that collects debug dump data (controller logs, job events, run logs, cluster info) from the jobs controller. This enables the debug dump to work in non-consolidation mode where the controller runs on a separate VM. The implementation uses a unified code path: gRPC when available, with CodeGen/SSH fallback. In consolidation mode, LocalResourcesHandle runs the CodeGen locally via LocalProcessCommandRunner. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/quicktest-core |
|
/smoke-test -k test_debug_dump_recent |
|
/smoke-test -k test_debug_dump_job |
|
/quicktest-core |
|
/smoke-test -k test_debug_dump_recent |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oke tests Fix string escaping in python3 -c assertions that are embedded in bash commands: use \\\" to produce \" in bash (which becomes " in Python), instead of \" which becomes " directly (breaking bash double-quote parsing). Also remove assert for 'timing' key which does not exist in summary.json. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/smoke-test -k test_debug_dump_recent |
|
/smoke-test -k test_debug_dump_job |
Use ThreadPoolExecutor to collect per-job debug data concurrently instead of sequentially. Cluster info collection remains sequential for deduplication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… unneeded \b Update debug-dump docstrings so the "at least one filter required" note appears in both CLI reference and Python SDK reference docs. Remove unnecessary \b before "Example usage:" in CLI help. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract the repeated try/except/errors.append pattern into a reusable context manager. Apply early returns in _collect_cluster_debug_manifest and _collect_controller_system_log_paths to reduce nesting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sk_yaml Replace the hardcoded frozenset with a programmatic derivation from RequestName, so new jobs.* request types are automatically included. Remove the one-line _redact_task_yaml wrapper — callers now call debug_dump_helpers.redact_task_yaml directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # sky/server/constants.py
Master merged with API_VERSION=45 (jobs.wait). Re-bump to 46 for our debug dump endpoints and update minimal_api_version checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the one-line _redact_task_yaml wrapper — callers now call debug_dump_helpers.redact_task_yaml directly. Add new jobs.wait request to _REQUEST_BODY_ALLOWLIST. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/smoke-test -k test_debug_dump |
…edact_task_yaml Fix smoke test failure: the "requested" section in summary.json now shows the original user-provided request IDs/prefixes, even if they don't match any existing requests. Previously, nonexistent request ID prefixes were silently dropped during resolution. Also inline _redact_task_yaml wrapper and add jobs.wait to allowlist. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/smoke-test -k test_debug_dump |
Summary
Adds a new
sky debug-dumpcommand that creates a comprehensive zip archive for troubleshooting SkyPilot issues. The dump collects server state, logs, and configuration into a single downloadable file, making it easy for users to share debugging information with the SkyPilot team.What gets collected:
SKYPILOT_*/SKY_*environment variables (sensitive values redacted)Targeting options (at least one required):
--cluster-names / -c: Specific clusters--job-ids / -j: Specific managed job IDs--request-ids / -r: Specific API request IDs--recent-minutes N: Everything active within the last N minutesArchitecture:
sky/utils/debug_utils.pyorchestrates the dump, collecting data from the DB and cluster/controller logssky/jobs/utils.py:collect_debug_dump_manifest()runs on managed job controllers to gather job-specific data (DB records inline, log file paths for rsync)sky/utils/debug_dump_helpers.pyavoid circular imports between debug_utils and jobs.utilsSecurity:
SKYPILOT_DB_CONNECTION_URI,SKYPILOT_INITIAL_BASIC_AUTH, etc.) are redacted totrue/falselast_creation_yamlanduser_yamlare redacted (secrets, Docker passwords)resolve().relative_to()) and manifest processingTest plan
Unit tests (
tests/unit_tests/test_sky/utils/test_debug_utils.py):--recentreturn_valuematching)RequestTaskFilter.finished_afterSQL generationlast_creation_yamlredaction inserialize_cluster_record_dump_request_id_info: happy path, not-found, DB failure, log file copying_dump_cluster_info: happy path, not-found, DB failure, events, associated requestsSmoke tests (
tests/smoke_tests/test_cli.py):--recent)--recent,-c,-r,-j)Manual testing:
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Review detail
close review: all changes in
sky/exceptsky/utils/debug_utils.pyandsky/utils/debug_dump_helpers.pyquick check:
sky/jobs/utils.pysky/utils/debug_utils.py(new file)sky/utils/debug_dump_helpers.py(new file)