[Core] Auto-configure Windows SSH config when running in WSL#8669
Conversation
Add a new section documenting how to configure VSCode's Remote-SSH extension to work with SkyPilot clusters launched from WSL. The issue is that SkyPilot creates SSH config in WSL's filesystem, but VSCode on Windows uses the native Windows SSH client which cannot access WSL paths directly. The solution uses a batch file wrapper that calls WSL's SSH, allowing VSCode to share the same SSH configuration as the WSL environment. https://claude.ai/code/session_016YUauKpvTitHzn1vQVdCqa
When SkyPilot runs in WSL, it now automatically: 1. Detects the WSL environment via /proc/version 2. Finds the Windows user's home directory 3. Creates SSH config entries in Windows' ~/.ssh/config 4. Uses UNC paths (\\wsl$\...) for SSH keys stored in WSL This enables VSCode's Remote-SSH extension on Windows to connect to SkyPilot clusters launched from WSL without manual configuration. The implementation adds: - is_wsl(): Detect if running in WSL - get_wsl_windows_home(): Get Windows home directory path - get_wsl_path_for_windows(): Convert WSL paths to UNC paths - SSHConfigHelper methods for Windows SSH config management Updated documentation to explain the automatic setup and provide the manual batch file approach as a fallback option. https://claude.ai/code/session_016YUauKpvTitHzn1vQVdCqa
Summary of ChangesHello @Michaelvll, 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 significantly enhances the developer experience for Windows users leveraging WSL with SkyPilot. By introducing automatic detection of the WSL environment and intelligently configuring SSH settings on both the Linux subsystem and the Windows host, it streamlines the process of connecting to SkyPilot clusters via VSCode's Remote-SSH extension. This change reduces manual configuration steps and improves cross-platform workflow integration. 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. 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 automatic configuration of Windows SSH settings when SkyPilot runs in WSL, significantly improving the user experience for VSCode Remote-SSH integration. The changes include new utility functions for WSL detection and path conversion, and modifications to SSHConfigHelper to manage Windows SSH configurations. The documentation has also been updated to reflect these new capabilities and provide manual fallback instructions. Overall, this is a valuable enhancement for Windows users leveraging WSL.
|
|
||
| .. code-block:: batch | ||
|
|
||
| C:\Windows\system32\wsl.exe ssh %* |
There was a problem hiding this comment.
The batch script uses C:\Windows\system32\wsl.exe ssh %*. While %* correctly passes all arguments, it might be more robust to explicitly quote arguments if ssh commands can contain spaces or special characters. However, for typical SSH usage, this should be fine.
| return _is_wsl_cached | ||
|
|
||
| _is_wsl_cached = False | ||
| if platform.system() != 'Linux': |
There was a problem hiding this comment.
The is_wsl() function catches FileNotFoundError and PermissionError but then silently passes. While this prevents crashes, it might be beneficial to log a debug message in such cases to help diagnose issues if WSL detection fails unexpectedly, especially if /proc/version is unreadable for some reason.
| if windows_home[1] == ':': | ||
| drive = windows_home[0].lower() | ||
| windows_home = f'/mnt/{drive}{windows_home[2:]}' |
There was a problem hiding this comment.
The path conversion logic windows_home = userprofile.replace('\\', '/') and windows_home = f'/mnt/{drive}{windows_home[2:]}' assumes a standard Windows drive letter (e.g., C:). While this is common, it might not cover all edge cases, such as network drives mapped to letters or non-standard drive configurations. However, for typical USERPROFILE paths, this should be sufficient.
| if windows_home[1] == ':': | ||
| drive = windows_home[0].lower() | ||
| windows_home = f'/mnt/{drive}{windows_home[2:]}' | ||
| except (subprocess.TimeoutExpired, FileNotFoundError, OSError): |
There was a problem hiding this comment.
Similar to is_wsl(), the get_wsl_windows_home() function also silently passes on exceptions from subprocess.run. Adding a debug log for subprocess.TimeoutExpired, FileNotFoundError, or OSError could aid in troubleshooting if cmd.exe execution fails.
| except (FileNotFoundError, PermissionError): | ||
| pass |
There was a problem hiding this comment.
The get_wsl_path_for_windows function defaults distro_name to 'Ubuntu' if it cannot be determined from WSL_DISTRO_NAME or /etc/os-release. While 'Ubuntu' is a common WSL distribution, this default might be incorrect for users running other distributions (e.g., Debian, Fedora). It might be safer to raise a warning or error if the distro name cannot be reliably determined, or at least log a message indicating the fallback to 'Ubuntu'.
- Reuse common_utils.is_wsl() instead of duplicating WSL detection - Extract path conversion helpers (_convert_windows_path_to_wsl, _convert_wsl_path_to_windows) - Extract _get_wsl_distro_name() for cleaner code - Use project-standard sky_logging instead of inline logging import - Move subprocess import to module level - Simplify _add_cluster_to_windows_ssh_config() logic: - Early exit for docker/proxy commands at method start - Use any() with generator for include_found check - Remove redundant variable assignments in loop Co-Authored-By: Claude <noreply@anthropic.com>
Remove detailed manual configuration instructions for WSL users since SkyPilot now automatically sets up Windows SSH config. Replace with a simple tip note. Co-Authored-By: Claude <noreply@anthropic.com>
… SSH - Fix is_wsl() to check platform.uname().release instead of uname()[3], as "microsoft" appears in the release string (e.g., 6.6.87.2-microsoft-standard-WSL2) - Add proxy command support for Windows by wrapping with wsl.exe bash -c - Copy SSH keys to Windows filesystem to avoid UNC path permission issues - Clean up copied keys when clusters are removed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused functions: get_wsl_path_for_windows(), _get_wsl_distro_name() - Extract _get_windows_userprofile_via_cmd() helper for cleaner control flow - Simplify _add_cluster_to_windows_ssh_config() signature to use boolean - Remove unnecessary context manager around print statement - Use modern class definition style (remove object inheritance) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add explicit check=False to subprocess.run call - Fix line length issues by wrapping long lines - Reformat method signature to single line per yapf Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add section to CONTRIBUTING.md documenting how to create a Windows 11 VM on Azure for testing WSL-specific features like the automatic Windows SSH config setup. Co-Authored-By: Claude <noreply@anthropic.com>
Use raw string (r"...") instead of regular string for the sed command containing regex special characters (\? and \s). This eliminates the SyntaxWarning in Python 3.12+. Co-Authored-By: Claude <noreply@anthropic.com>
4878377 to
0f51416
Compare
- Use functools.lru_cache for thread-safe caching of Windows home directory
- Fix shell escaping in proxy command conversion using single quotes to
prevent expansion of $, backticks, and other metacharacters
- Fix SSH key cleanup to use consistent naming ({cluster_name}.pem)
- Add comprehensive unit tests for WSL path conversion and SSH config helpers
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
6d0ab40 to
e2d9ba0
Compare
Replace manual single-quote escaping with shlex.quote() for cleaner, more maintainable shell argument quoting. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
/quicktest-core --base-branch releases/0.11.1 ✅ |
Print the output of sky show-gpus commands before checking with grep to improve debugging when tests fail. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Changes
is_wsl(),get_wsl_windows_home(),get_wsl_path_for_windows())SSHConfigHelperto also write Windows SSH config when in WSL\\wsl$\...) for SSH keys stored in WSLTest plan
Tested on a Windows VM on Azure:
sky status -uin WSL to launch a cluster and verify SSH configs are created in both WSL and Windows🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com