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

Integrate kind for local testing #242

Merged
merged 5 commits into from
Nov 22, 2024
Merged

Integrate kind for local testing #242

merged 5 commits into from
Nov 22, 2024

Conversation

IrvingMg
Copy link
Collaborator

@IrvingMg IrvingMg commented Nov 12, 2024

Fixes / Features

  • Integrate kind for Local Testing

As part of the kjob integration in #212, we are developing new features where having a full cluster on GKE with GPUs, Vertex AI, etc., feels like overkill. To simplify the development and testing process, this PR implements a local testing environment using kind.

This PR adds a new command to xpk for managing local Kubernetes clusters with kind, as well as the --kind-cluster flag for the batch command. The command to create a local cluster looks like this:

python3 xpk.py kind create --cluster xpk-test

Once the local cluster is set up, you can use the --kind-cluster flag to run xpk commands against the local cluster instead of GKE. For example:

python3 xpk.py batch [other-options] --kind-cluster

This PR implements both the command for creating and managing local kind clusters and the --kind-cluster flag specifically for the batch command, which interacts with kjob and is well-suited for testing the local environment. Support for local testing may be extended to other xpk commands as needed.

For more details on how to set up and use the local testing environment, please refer to the updated README.

Testing / Documentation

Testing details.

  • [ y/n ] Tests pass
  • [ y/n ] Appropriate changes to documentation are included in the PR

Copy link

google-cla bot commented Nov 12, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@IrvingMg IrvingMg force-pushed the feature/local-testing branch 3 times, most recently from c8ff0b7 to 5699bd7 Compare November 12, 2024 15:47
@IrvingMg
Copy link
Collaborator Author

cc: @mbobrovskyi @mwysokin

@IrvingMg IrvingMg force-pushed the feature/local-testing branch from 5699bd7 to 8786049 Compare November 12, 2024 15:58
@pawloch00
Copy link
Collaborator

I doubt if we should merge it into main. Let me ask about it

@pawloch00
Copy link
Collaborator

In my opinion merging it to main with only batch command being executed localy is not worth it. If more commands can be added and kind can be used for ex. integration tests in github pipeline than I approve it

@IrvingMg
Copy link
Collaborator Author

In my opinion merging it to main with only batch command being executed localy is not worth it. If more commands can be added and kind can be used for ex. integration tests in github pipeline than I approve it

I agree it might seem not worth it as we're only adding the option of local testing for batch command. However, I think this may be a good opportunity to introduce local testing. To cover all the cases for local testing in a single PR seems to be a big effort, so I think we can add support for other commands bit by bit.

Besides that, currently, we have a good couple of examples - #236, #244 - where having the option of testing locally is useful. As these features don't require to have a cluster with special resources such as GPU, TPU, etc., I think we can avoid creating a cluster on GKE, saving time and costs, for testing by using a local cluster.

@pawloch00
Copy link
Collaborator

OK, it would be nice to create integration test using kind as followup task for this PR and use it in github pipeline where possible

pawloch00
pawloch00 previously approved these changes Nov 15, 2024
@IrvingMg IrvingMg force-pushed the feature/local-testing branch 3 times, most recently from 5baf4d2 to 8446fcc Compare November 20, 2024 15:37
@IrvingMg IrvingMg force-pushed the feature/local-testing branch from 8446fcc to 6693e4c Compare November 21, 2024 09:01
@IrvingMg IrvingMg merged commit 6e346b5 into main Nov 22, 2024
5 checks passed
@IrvingMg IrvingMg deleted the feature/local-testing branch November 22, 2024 10:01
@IrvingMg
Copy link
Collaborator Author

OK, it would be nice to create integration test using kind as followup task for this PR and use it in github pipeline where possible

Opened issue for integration test #267

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.

3 participants