Skip to content
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

[Core] Set file permission when writing the ssh config file #3151

Merged
merged 9 commits into from
Feb 14, 2024

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Feb 13, 2024

We now explicitly set the permission for the ssh config file writen by SkyPilot, because sometimes if the user changed the username of their local machine, and still use the conda environment with skypilot installed under the old /home/old-user folder. In this case, the files writen to ~/.sky/generated/ssh can have a permssion dwxr-rw-rw-- which violates the permission required for ssh, causing all ssh fails.

This is one of the potential reasons for #2689. This PR also prints out the output of the last ssh command if the wait ssh timeout.

Reproducible steps:

  1. docker run -it --rm centos:centos7 /bin/bash
  2. adduser -m test-original; adduser -m test-new
  3. passwd test-original; passwd test-new
  4. sermod -aG wheel test-new; sermod -aG wheel test-original
  5. su test-original; # setup miniconda
  6. setfacl -R -m u:test-new:rwx /home/test-original/
  7. su test-new; /home/test-original/miniconda3/bin/conda init; # install skypilot
  8. sky launch --cloud gcp --cpus 2 echo hi; # task submission fails due to ssh permission issue with the folder
Bad owner or permissions on /home/test-new/.sky/generated/ssh/test-new-user

By deleting ~/.sky/generated/ssh and switch to this PR, the problem goes away.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

sky/provision/provisioner.py Show resolved Hide resolved
sky/provision/provisioner.py Show resolved Hide resolved
@Michaelvll Michaelvll merged commit e47697f into master Feb 14, 2024
19 checks passed
@Michaelvll Michaelvll deleted the set-file-mode branch February 14, 2024 05:58
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.

None yet

2 participants