-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LIU-427: Add private key support for remote submission. #298
base: master
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis PR adds support for specifying a custom SSH private key when making remote submissions through the Slurm client. The implementation modifies the SlurmClient class and remote utility functions to accept and use a private key path parameter, providing more flexibility for different SSH configurations and operating systems. Sequence diagram for Remote Submission with SSH KeysequenceDiagram
participant User
participant SlurmClient
participant Remote
User->>SlurmClient: Initialize with ssh_key
SlurmClient->>Remote: execRemote(host, command, username, pkeyPath=ssh_key)
Remote-->>SlurmClient: Execute command
SlurmClient->>Remote: copyTo(host, tjob, job_file_name, username, pkeyPath=ssh_key)
Remote-->>SlurmClient: Copy file
SlurmClient->>Remote: execRemote(host, sbatch command, username, pkeyPath=ssh_key)
Remote-->>SlurmClient: Submit job
Class diagram for SlurmClient with SSH Key SupportclassDiagram
class SlurmClient {
- String host
- String username
- String ssh_key
+ mk_session_dir(String dlg_root)
+ submit_job()
}
SlurmClient : +__init__(String facility, String host, Boolean remote, String pip_name, String username, String ssh_key="")
note for SlurmClient "Added ssh_key attribute and updated methods to use it"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @myxie - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider enhancing the --ssh_key help text to include details about expected key format and common file locations (e.g., ~/.ssh/id_rsa, ~/.ssh/id_ed25519)
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
JIRA Ticket
Type
Problem/Issue
When we use Paramiko to make the connection to Slurm in the
SlurmClient
class, we do not pass a private-key to ourremote
utility functions. This means that Paramiko will attempt to find a private key file in the~/.ssh
directory, and specifically namedid_rsa
(or similar).This is fine if using passwordless-SSH on Linux, but can cause issues if using an alternative SSH server/configuration, and on a different OS (e.g. macOS).
Solution
I have added an option to pass in an
ssh_key
via the command line and theSlurmClient
class. This has the added bonus of adding the path of any private key file through a DALiuGE component (e.g. #298).Checklist
[ ] Unittests added[ ] Documentation addedSummary by Sourcery
New Features:
Summary by Sourcery
New Features: