Skip to content

Prepare for running on GKE runners #1681

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

Merged
merged 1 commit into from
May 10, 2025
Merged

Conversation

shralex
Copy link
Collaborator

@shralex shralex commented May 2, 2025

This PR is refactoring the code to be able to use GKE cloud runners. One of the current requirements of this framework is using containers.

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed.

@shralex shralex force-pushed the test_cloud_runners1 branch 11 times, most recently from 9759789 to 66313b0 Compare May 5, 2025 21:27
@shralex shralex force-pushed the test_cloud_runners1 branch from 66313b0 to 94cbd45 Compare May 9, 2025 22:16
@shralex shralex force-pushed the test_cloud_runners1 branch from 94cbd45 to 716fb30 Compare May 9, 2025 23:37
@shralex shralex force-pushed the test_cloud_runners1 branch from 716fb30 to 12ea975 Compare May 9, 2025 23:49
@shralex shralex force-pushed the test_cloud_runners1 branch from 12ea975 to 2518186 Compare May 9, 2025 23:50
This PR is testing the use of GKE cloud runners.
@shralex shralex force-pushed the test_cloud_runners1 branch from 2518186 to af0d7e1 Compare May 10, 2025 00:58
@shralex shralex changed the title [DO NOT MERGE] Run Tests on GKE runners Prepare for running on GKE runners May 10, 2025
@@ -67,6 +67,7 @@ jobs:
with:
device_type: tpu
device_name: v4-8
# cloud_runner: linux-x86-ct4p-240-4tpu
Copy link
Collaborator

Choose a reason for hiding this comment

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

what are these comments for? Generally we should not comment out code, its quite confusing

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this intentional - is the cloud_runner a cpu runner? why is it under the tpu_unit_tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are three groups of cloud runners, each one with 1 tag. This one is the tag for TPU runners. You can see them in the runners page. This PR added a new parameter to run_tests_internal (cloud_runner). Basically, once we want to enable this we just need to uncomment these lines. The reason I'm doing this PR now is to allow Rohan to build upon it to enable CPU cloud runners first, because this part should already work.

@copybara-service copybara-service bot merged commit 8323f74 into main May 10, 2025
27 checks passed
@copybara-service copybara-service bot deleted the test_cloud_runners1 branch May 10, 2025 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants