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/AWS] Skip association with no subnetId and fix autodown for spot instances #2921

Merged
merged 2 commits into from
Dec 30, 2023

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Dec 29, 2023

Fixes #2920.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • sky launch --cloud aws --region us-west-2 -c test-west --cpus 2 echo hi
  • 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

Copy link
Member

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

LGTM.

Btw,

sky launch --cloud aws -i0 --down --use-spot -y --region us-east-2 

launched successfully but ran into


/Users/zongheng/anaconda/envs/py38/lib/python3.8/multiprocessing/resource_tracker.py:216: UserWarning: resource_tracker: There appear to be 4 leaked semaphore objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '
sky.exceptions.NotSupportedError: The following features are not supported by AWS:
        Feature  Reason
        stop     Stopping spot instances is currently not supported on AWS.

which appears to be a bug in the last PR (we probably should check --down).

sky/provision/aws/config.py Outdated Show resolved Hide resolved
@Michaelvll Michaelvll changed the title [AWS] Skip association with no subnetId [Core/AWS] Skip association with no subnetId and fix autodown for spot instances Dec 30, 2023
@Michaelvll
Copy link
Collaborator Author

LGTM.

Btw,

sky launch --cloud aws -i0 --down --use-spot -y --region us-east-2 

launched successfully but ran into


/Users/zongheng/anaconda/envs/py38/lib/python3.8/multiprocessing/resource_tracker.py:216: UserWarning: resource_tracker: There appear to be 4 leaked semaphore objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '
sky.exceptions.NotSupportedError: The following features are not supported by AWS:
        Feature  Reason
        stop     Stopping spot instances is currently not supported on AWS.

which appears to be a bug in the last PR (we probably should check --down).

Great catch! Just fixed it in this PR. PTAL.

Copy link
Member

@concretevitamin concretevitamin 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 @Michaelvll.

@Michaelvll Michaelvll merged commit 3eb6f2f into master Dec 30, 2023
19 checks passed
@Michaelvll Michaelvll deleted the fix-subnet branch December 30, 2023 05:35
@concretevitamin
Copy link
Member

With

sky launch --cloud aws -i0 --down --use-spot -y --region us-east-2 

it now shows the following at the end

...

* 1 cluster has auto{stop,down} scheduled. Refresh statuses with: sky status --refresh

/Users/zongheng/anaconda/envs/py38/lib/python3.8/multiprocessing/resource_tracker.py:216: UserWarning: resource_tracker: There appear to be 4 leaked semaphore objects to clean up at shutdown

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.

[AWS] KeyError 'SubnetId' when multiple Main route tables appear in a same region
2 participants