[Core] Support launching a new cluster from an existing cluster's disk #2098

Michaelvll · 2023-06-16T22:25:14Z

Fixes #2050. This PR only supports AWS and GCP as an experimental feature and unblocks the users.

~~It is blocked by #2087~~

sky launch --clone-disk-from existing-cluster -c new-cluster --cloud aws --region us-west-2 task.yaml

A problem with the image copying is that the copying can take significant of time.

TODOs:

Support GCP
Clean up the image after the target cluster being tear down.
docs for additional permissions required

Tested (run the relevant ones):

concretevitamin

Awesome @Michaelvll! Leaving some comments first, only a partial pass.

Some UX notes:

I tried to pass a smaller disk size than an existing cluster

» sky launch --clone dbg -c smoke --region us-west2 --cloud gcp --disk-size 50 --cpus 4
...
Launching a new cluster 'smoke' from the disk of 'dbg'. Proceed? [Y/n]:
<spinner -- creating image>
Image 'projects/skypilot-xxxx/global/images/skypilot-dbg-xxxxxxxx' for 'dbg' created successfully on GCP. Overriding task's image_id.
ValueError: Image 'projects/skypilot-xxxx/global/images/skypilot-dbg-xxxxxxxx' is 256.0GB, which is larger than the specified disk_size: 50 GB. Please specify a larger disk_size to use this image.

The error shows up only after the image creation (which is now leaked). Can we pre-check?

Btw, this also makes me wonder what _is_image_managed means. The above ended up with an error - did we remove the auto-created image managed by us?

A successful clone:


Launching a new cluster 'smoke' from the disk of 'dbg'. Proceed? [Y/n]:
Image 'projects/skypilot-375900/global/images/skypilot-dbg-1687454357' for 'dbg' created successfully on GCP. Overriding task's image_id.

The second line probably should have a logging prefix, otherwise easy to miss. Also, green the line?

sky/cli.py

sky/backends/backend_utils.py

sky/execution.py

sky/clouds/cloud.py

sky/utils/subprocess_utils.py

concretevitamin · 2023-06-24T14:50:53Z

An odd behavior - I think I did the following:

sky launch --clone dbg -c smoke --region us-west2 --cloud gcp --cpus 4  # Success.
sky down dbg

A day after,

sky cpunode -c smoke -i120
...
ValueError: Image 'projects/skypilot-12345/global/images/skypilot-dbg-xxxxx54357' not found in GCP.

On GCP console the image is indeed not there. Could it be sky down dbg deleted the image?

Michaelvll · 2023-06-24T22:01:51Z

Good catch! It was because we called the post_teardown_cleanup during the status refresh, which previously will delete image when the cluster is stopped. We should not delete the image when stopping the cluster. Fixed in the latest commit.

add image apis wip wip wip fix issues works!

fix fix better logging mypy

rename func Add image id in the message format longer wait time

refactor

concretevitamin

Thanks @Michaelvll! Did a pass, some comments.

sky/backends/backend_utils.py

concretevitamin · 2023-06-26T20:12:51Z

sky/execution.py

+            f'{reset}{bold}sky spot launch{reset} {yellow}or{reset} '
+            f'{bold}sky.spot_launch(){reset}.')
+
+    if Stage.CLONE_DISK in stages:


Seems like the 3 if clauses should be placed under an "if not cluster_exists:". Not feeling strongly, just felt that may be more readable.

Actually, we can move the CLONE_DISK stage up before the if above, so that we can still have the two original if in if not cluster_exists. The downside with this is that, it can be a little bit confusing when the order of the stages should be CLONE_DISK -> OPTIMIZE -> PROVISION ..., but now it is easier to read it as CLONE_DISK -> PROVISION -> OPTIMIZE -> PROVISION....

sky/clouds/gcp.py

sky/clouds/aws.py

concretevitamin · 2023-06-26T20:37:50Z

sky/clouds/aws.py

+            stream_logs=False)
+
+        instance_ids = json.loads(stdout.strip())
+        if len(instance_ids) != 1:


Good point! Although we checked the existance of the cluster in the caller. There might be some case where user manually terminates the original cluster, causing an issue. Added a condition for it.

sky/utils/subprocess_utils.py

…disk-from

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

…to clone-disk-from

concretevitamin

Thanks for solving a longstanding issue from GPU unavailability @Michaelvll!

sky/backends/backend_utils.py

concretevitamin · 2023-06-27T15:35:07Z

docs/source/cloud-setup/cloud-permissions/aws.rst

@@ -103,8 +103,22 @@ AWS accounts can be attached with a policy that limits the permissions of the ac
        ]
    }

-4. Click **Next: Tags** and follow the instructions to finish creating the policy. You can give the policy a descriptive name, such as ``minimal-skypilot-policy``.
-5. Go back to the previous window and click on the refresh button, and you can now search for the policy you just created.
+4. **Optional**: To allow ``--clone-disk-from`` to work, you need to add the following permissions to the policy above as well.


Suggested change

4. **Optional**: To allow ``--clone-disk-from`` to work, you need to add the following permissions to the policy above as well.

4. **Optional**: To allow ``sky launch --clone-disk-from`` to work, you need to add the following permissions to the policy above as well.

I think it may be ok to leave this as a comment, to see if the feature is more widely used first. OK either way.

Good point! I tends to have it in the doc, as this doc page is not target for normal users but for admins, i.e., there is limited downside/burden this new entry will bring to a normal user. And it is easier to point our pilot user to this page, when they want to use this feature.

docs/source/cloud-setup/cloud-permissions/gcp.rst

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

…to clone-disk-from

…disk-from

Michaelvll

Tested:

pytest tests/test_smoke.py::test_clone_disk_aws --aws
pytest tests/test_smoke.py::test_clone_disk_gcp
pytest tests/test_smoke.py
pytest tests/test_smoke.py --aws

Michaelvll added the blocked PR blocked by other issues label Jun 16, 2023

Michaelvll marked this pull request as draft June 16, 2023 22:54

Michaelvll marked this pull request as ready for review June 16, 2023 23:13

Michaelvll requested a review from concretevitamin June 16, 2023 23:28

Michaelvll force-pushed the clone-disk-from branch from 1b0136a to 7a4d629 Compare June 19, 2023 08:46

Michaelvll removed the blocked PR blocked by other issues label Jun 19, 2023

concretevitamin reviewed Jun 22, 2023

View reviewed changes

Michaelvll requested a review from concretevitamin June 23, 2023 17:52

Michaelvll added 15 commits June 25, 2023 16:27

wip

f447e0f

add image apis wip wip wip fix issues works!

delete the correct image_id

419a002

not support migrate to existing cluster

38d4693

fix fix better logging mypy

Add run and retry

5f003ee

rename func Add image id in the message format longer wait time

Remove image after cluster is removed

3360d34

refactor

make clone available for GCP

c12fda0

fix image deletion

5c2c3db

add permission docs

9cd39de

add permissions for the cloning in GCP

8625d7f

rename the feature

9c2b965

format

9091d92

Address comments

b8f2c88

format

14a3a99

format

2b1e2be

only delete image when termination happens

6df687d

Michaelvll force-pushed the clone-disk-from branch from 7230966 to 6df687d Compare June 25, 2023 23:57

fix

ce6bf86

concretevitamin reviewed Jun 26, 2023

View reviewed changes

Merge branch 'master' of github.com:skypilot-org/skypilot into clone-…

cf0f066

…disk-from

Michaelvll and others added 5 commits June 27, 2023 00:56

address comments

2651b18

Merge branch 'master' of github.com:skypilot-org/skypilot into clone-…

f3776f7

…disk-from

Update sky/resources.py

f1c98f7

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

fix docs

4a778d9

Merge branch 'clone-disk-from' of github.com:skypilot-org/skypilot in…

20e7da5

…to clone-disk-from

concretevitamin approved these changes Jun 27, 2023

View reviewed changes

Michaelvll and others added 9 commits June 27, 2023 10:39

comment

30205fd

address

4427e47

Update sky/backends/backend_utils.py

e8f2bd6

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

stack trace

2073c70

Merge branch 'clone-disk-from' of github.com:skypilot-org/skypilot in…

b549691

…to clone-disk-from

Merge branch 'master' of github.com:skypilot-org/skypilot into clone-…

8b2a94a

…disk-from

doc numerating

3514029

revert unnecessary changes

8d9baa3

add smoke tests

b5276bd

Michaelvll commented Jun 27, 2023

View reviewed changes

Michaelvll added 6 commits June 27, 2023 14:58

fix test

254de30

longer timeout

e1e53e7

sleep before image creation

7b85601

longer wait time

ee59b10

longer wiat time

4954fcb

fix azure test

f6c77a5

Michaelvll merged commit 286f582 into master Jun 28, 2023

Michaelvll deleted the clone-disk-from branch June 28, 2023 08:21

Michaelvll mentioned this pull request Sep 18, 2023

Can I specify a new instance type when restarting a cluster? #1928

Closed

Michaelvll mentioned this pull request Oct 10, 2023

Instance switching with the same disk #1620

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] Support launching a new cluster from an existing cluster's disk #2098

[Core] Support launching a new cluster from an existing cluster's disk #2098

Michaelvll commented Jun 16, 2023 •

edited

Loading

concretevitamin left a comment

concretevitamin commented Jun 24, 2023

Michaelvll commented Jun 24, 2023

concretevitamin left a comment

concretevitamin Jun 26, 2023

Michaelvll Jun 27, 2023

concretevitamin Jun 26, 2023

Michaelvll Jun 27, 2023

concretevitamin left a comment

concretevitamin Jun 27, 2023

Michaelvll Jun 27, 2023

Michaelvll left a comment •

edited

Loading

	4. Optional: To allow ``--clone-disk-from`` to work, you need to add the following permissions to the policy above as well.
	4. Optional: To allow ``sky launch --clone-disk-from`` to work, you need to add the following permissions to the policy above as well.

[Core] Support launching a new cluster from an existing cluster's disk #2098

[Core] Support launching a new cluster from an existing cluster's disk #2098

Conversation

Michaelvll commented Jun 16, 2023 • edited Loading

concretevitamin left a comment

Choose a reason for hiding this comment

concretevitamin commented Jun 24, 2023

Michaelvll commented Jun 24, 2023

concretevitamin left a comment

Choose a reason for hiding this comment

concretevitamin Jun 26, 2023

Choose a reason for hiding this comment

Michaelvll Jun 27, 2023

Choose a reason for hiding this comment

concretevitamin Jun 26, 2023

Choose a reason for hiding this comment

Michaelvll Jun 27, 2023

Choose a reason for hiding this comment

concretevitamin left a comment

Choose a reason for hiding this comment

concretevitamin Jun 27, 2023

Choose a reason for hiding this comment

Michaelvll Jun 27, 2023

Choose a reason for hiding this comment

Michaelvll left a comment • edited Loading

Choose a reason for hiding this comment

Michaelvll commented Jun 16, 2023 •

edited

Loading

Michaelvll left a comment •

edited

Loading