Skip to content

Conversation

@EricBryann
Copy link
Contributor

@EricBryann EricBryann commented Oct 27, 2025

PR to fix #6567

When dpkg runs into an error, the code defaults to yum, and some don't have yum installed, resulting in
sudo: yum: command not found

This PR checks for existence of yum before rclone install

Tested (run the relevant ones):

  • Code formatting: install pre-commit (auto-check on commit) or bash format.sh
  • Any manual or new tests for this PR (please specify below)
  • All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
  • Relevant individual tests: /smoke-test -k test_name (CI) or pytest tests/test_smoke.py::test_name (local)
  • Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

@kevinmingtarja kevinmingtarja self-requested a review October 27, 2025 03:47
Copy link
Collaborator

@kevinmingtarja kevinmingtarja left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @EricBryann!

Let's add a smoke test on AWS for this :)

Some pointers:

  • rclone is needed only with MOUNT_CACHED mode (or on arm64 arch),
  • So you can copy test_kubernetes_storage_mounts_cached as a starting point
  • Let's test with two different AMIs that are different linux distros, one with dpkg, the other with yum. We can take ubuntu and amazon linux 2023 for example

@EricBryann EricBryann force-pushed the check-bin-for-rclone-install branch from 7598e34 to 40c1c3f Compare October 28, 2025 13:26
@kevinmingtarja
Copy link
Collaborator

/smoke-test -k storage_mounts_cached --aws

@kevinmingtarja kevinmingtarja self-requested a review October 30, 2025 21:46
@kevinmingtarja
Copy link
Collaborator

kevinmingtarja commented Oct 31, 2025

/smoke-test

@kevinmingtarja
Copy link
Collaborator

/smoke-test -k storage --aws
/smoke-test -k storage --gcp
/smoke-test -k storage --kubernetes

@kevinmingtarja
Copy link
Collaborator

/smoke-test -k test_aws_storage_mounts_cached --aws

@kevinmingtarja
Copy link
Collaborator

/smoke-test -k test_aws_storage_mounts_cached

@kevinmingtarja
Copy link
Collaborator

The two test failures are unrelated, fixed by #7818.

Copy link
Collaborator

@kevinmingtarja kevinmingtarja left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution @EricBryann ! Just left one nit.

Comment on lines +317 to +318
'ami-0f5fcdfbd140e4ab7', # dpkg
'ami-0a5a5b7e2278263e5' # yum
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Could we keep the aws ec2 describe-images ... commands comment that you had in the prev commit here for future reference.

@kevinmingtarja
Copy link
Collaborator

/smoke-test -k storage --nebius

@kevinmingtarja
Copy link
Collaborator

/smoke-test -k storage --azure

@kevinmingtarja kevinmingtarja merged commit 62ac17a into skypilot-org:master Nov 3, 2025
26 of 28 checks passed
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.

sudo: yum: command not found with MOUNT_CACHED

2 participants