-
Notifications
You must be signed in to change notification settings - Fork 501
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
[OCI] Re-Implementation with the new provision API framework. #4119
base: master
Are you sure you want to change the base?
Conversation
8f9b57e
to
3c3bcee
Compare
Resolved conflict. |
Hi @cblmemo @Michaelvll , thanks for support this OCI provisioning API migration PR. Could you please take a review if your time possible? Thanks in advance. |
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.
Thanks for contributing to this @HysunHe ! This looks awesome. Left some nits and will test it out tomorrow!
Hi @cblmemo , Thanks for your detail review and kind comments. I've addressed (or commented) all of them and well performed functionality tests. |
Hi @cblmemo ,could you please take the review for the latest changes which addressed the previous comments? Thanks a lot :) |
Hi @HysunHe , thanks for the reminder! I tested this PR with D 11-02 13:06:14 provisioner.py:151] raise exceptions.ServiceError(
D 11-02 13:06:14 provisioner.py:151] oci.exceptions.ServiceError: {'target_service': 'compute', 'status': 400, 'code': 'InvalidParameter', 'opc-request-id': '2085EBD06D624CE9A952258F3B61A3FB/5C41B95C78B9AFBC18E5A15CF96F98DA/E27434EFC36C28FF3644A68FC7C96A21', 'message': 'Invalid ratio of memory in GB to OCPUs. Current ratio: 16.0. Valid ratio range: 0 - 0', 'operation_name': 'launch_instance', 'timestamp': '2024-11-02T20:06:14.589716+00:00', 'client_version': 'Oracle-PythonSDK/2.110.0', 'request_endpoint': 'POST https://iaas.us-sanjose-1.oraclecloud.com/20160918/instances', 'logging_tips': 'To get more info on the failing request, refer to https://docs.oracle.com/en-us/iaas/tools/python/latest/logging.html for ways to log the request/response details.', 'troubleshooting_tips': "See https://docs.oracle.com/iaas/Content/API/References/apierrors.htm#apierrors_400__400_invalidparameter for more information about resolving this error. Also see https://docs.oracle.com/iaas/api/#/en/iaas/20160918/Instance/LaunchInstance for details on this operation's requirements. If you are unable to resolve this compute issue, please contact Oracle support and provide them this full error message."} Also, I noticed there are several unresolved comments - could you take a look on them as well? ;) |
Co-authored-by: Tian Xia <cblmemo@gmail.com>
Ahh, lost those "hidden" ones. Now all nits are addressed. |
emm, looks cannot reproduce this on my env. Would you please paste the messages regarding to the chosen resource (as below): (sky) hysunhe@HYHE-PF1ZGYCQ:~$ sky launch --cloud oci --num-nodes 2 whoami --dryrun
|
Hi @cblmemo, could you please double confirm if you're using an trial account and have reached the resource limit? Also you may go to https://cloud.oracle.com/limits?region=us-sanjose-1 and check the resource type "Cores for Standard.E4.Flex ...". |
This is the PR to implement the OCI support in SkyPilot under its new provision API framework. The old implementation is deleted.
Leveraging the SkyPilot new provision API framework, this PR is expected to solve the TIMEOUT issue in the "old" implementation for some OCI VMs which needs long time to complete the setup/init.
It also includes some minor enhancement/fix.
This PR should be review/merged after #4080
All functions in the "old" implemented are now tested against with the new implementation.
All tests are passed.
Tested (run the relevant ones):
bash format.sh
pytest tests/test_smoke.py
pytest tests/test_smoke.py::test_fill_in_the_name
conda deactivate; bash -i tests/backward_compatibility_tests.sh