Skip to content

Conversation

@un-def
Copy link
Collaborator

@un-def un-def commented Feb 12, 2025

  • Add proxy_jump property
  • Store configuration as part of RemoteConnectionInfo
  • Always use a project key to connect to SSH instances, drop backward compatibility code (previously, the user-provided key was used, as the project key was not added to the SSH instance, this was fixed in Add missing project SSH key to on-prem instances #1716)

NOTE: services are not currently supported, proxy support will be added in a separate PR.

Part-of: #2010

* Add `proxy_jump` property
* Store configuration as part of `RemoteConnectionInfo`
* Always use a project key to connect to SSH instances,
  drop backward compatibility code (previously,
  the user-provided key was used, as the project key
  was not added to the SSH instance, this was fixed in
  #1716)

NOTE: services are not currently supported, proxy support will be added
in a separate PR.

Part-of: #2010
@un-def un-def force-pushed the issue_2010_ssh_fleet_proxy branch from a52d504 to 195c37e Compare February 12, 2025 09:41
@un-def un-def requested review from jvstme and r4victor February 12, 2025 10:12
ssh_user: str,
ssh_keys: List[SSHKey],
ssh_proxy: Optional[SSHConnectionParams],
ssh_proxy_keys: list[SSHKey],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess ssh_proxy_keys should be Optional?

Comment on lines +327 to +328
To be able to attach to runs, both explicitly with `dstack attach` and implicitly with `dstack apply`, you must either
add a front node key (`~/.ssh/head_node_key`) to an SSH agent or configure a key path in `~/.ssh/config`:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. Why do we require the user to configure a key path in ~/.ssh/config if user specifies identity_file: ~/.ssh/head_node_key? Can't we just use it to connect to the head node?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be two different users, one creating a fleet has the key, but other users who run workloads may not have the key. With regular setup, the key problem is solved via shim -- shim keeps user's key on the instance while a run is running, but on the head node we don't have any dstack agent to manage authorized keys, thus we require that each user has the head node key on their machine preconfigured. We could download the key to a user's machine though, but I'm not sure if this is a good idea.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd put this comment somewhere in the code.

un-def and others added 2 commits February 12, 2025 13:03
Co-authored-by: Victor Skvortsov <vds003@gmail.com>
@un-def un-def merged commit 5cc97ae into master Feb 13, 2025
24 checks passed
@un-def un-def deleted the issue_2010_ssh_fleet_proxy branch February 13, 2025 06:16
pranitnaik43 pushed a commit to bahaal-tech/dstack that referenced this pull request Mar 4, 2025
* Add `proxy_jump` property
* Store configuration as part of `RemoteConnectionInfo`
* Always use a project key to connect to SSH instances,
  drop backward compatibility code (previously,
  the user-provided key was used, as the project key
  was not added to the SSH instance, this was fixed in
  dstackai#1716)

Part-of: dstackai#2010
Co-authored-by: Victor Skvortsov <vds003@gmail.com>
pranitnaik43 pushed a commit to bahaal-tech/dstack that referenced this pull request Mar 5, 2025
* Add `proxy_jump` property
* Store configuration as part of `RemoteConnectionInfo`
* Always use a project key to connect to SSH instances,
  drop backward compatibility code (previously,
  the user-provided key was used, as the project key
  was not added to the SSH instance, this was fixed in
  dstackai#1716)

Part-of: dstackai#2010
Co-authored-by: Victor Skvortsov <vds003@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants