[CLI] add support for cluster management #13835
Conversation
I can run this with ``` python -m pytest tests/tests_clusters/test_cluster_lifecycle.py ``` test fails ofc because both the assertions don't work yet
supported commands: - lightning clusters create - lightning clusters list - lightning clusters delete
verified e2e via ``` python -m pytest tests/tests_clusters/test_cluster_lifecycle.py::test_cluster_list ```
I can't validate this just yet because I don't have the feature flag enabled in prod.
github-actions
bot
added
the
app (removed)
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
Co-authored-by: Laverne Henderson <[email protected]>
for more information, see https://pre-commit.ci
instead of providing a list of default we'll populate this on the backend
|
LGTM! |
| os.environ.get("LIGHTNING_BYOC_EXTERNAL_ID") is None, | ||
| reason="missing LIGHTNING_BYOC_EXTERNAL_ID environment variable", | ||
| ) | ||
| def test_cluster_lifecycle() -> None: |
There was a problem hiding this comment.
This PR introduced this new tests_clusters directory. But which testing job runs it?
There was a problem hiding this comment.
These aren't hooked up to our CI today because during manual verification cluster creation took more than one hour. There's an internal ticket tracking hooking this up to a CI - GRID-9821.
I think we can add them to our CI when we add path filtering, so they only run when the cluster CLI is changed to not slow down PR checks. Or alternatively, mark it as optional and just keep it running.
See the ticket as there are other concerns around this as well.
There was a problem hiding this comment.
Is there a reason why these tests are maintained outside the app folder?
There was a problem hiding this comment.
@awaelchli there is - they don't execute any apps and don't depend on any apps.
There was a problem hiding this comment.
I would suggest removing this file for now until they actually get hooked into CI. Not only they can silently break, they also give a false sense of security to unaware contributors.
There was a problem hiding this comment.
let me talk to @Borda next week about this; as this is part of our cloud offering we might be able to shift them to the non-public part of our codebase.
What does this PR do?
This PR adds support to manage Bring-Your-Own-Credentials lightning compute clusters.
The initial implementation exposes cluster creation, cluster listing and cluster deletion.
The feature is currently only available as closed preview and requires allow listing by a Lightning AI employees.
I've added two end-to-end tests and a couple of unit tests to ensure the CLI works as expected.
The general flow is aligned with how the
lightingCLI works today:Create clusters
This command creates a BYOC compute cluster. The provider arguments are specific to the only cloud provider we support today, AWS.
list clusters
This command does not take any arguments. It lists clusters available to the current user
Delete cluster
The command deletes a BYOC compute cluster.
Does your PR introduce any breaking changes? If yes, please list them.
No breaking changes
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃