Add e2e test for train API

[helenxie-bit]

What this PR does / why we need it:
Add an e2e test in the test_e2e_train_api.py for the train API.

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Fixes #

Checklist:

[coveralls]

Pull Request Test Coverage Report for Build 12425333047

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 12381609583:	0.0%
Covered Lines:	85
Relevant Lines:	85

---

💛 - Coveralls

[helenxie-bit]

I noticed that other e2e tests include separate test functions for gang scheduling and plain scheduling. Should the e2e test for the train API also cover both cases? Currently, it fails under gang scheduling, and I’m not sure why.

Oh, I see, we run our integration test with scheduling plugins as well. Maybe to unblock this PR, let's create separate GitHub action for now, sorry to ask you again refactor this. In the V2, we can refactor it to make it more consistent.

@andreyvelich No problem at all. We can also keep the e2e test for train API in integration tests, but skip this if use gang scheduling. Which way do you think is better?

[andreyvelich]

We can also keep the e2e test for train API in integration tests, but skip this if use gang scheduling

Yes, maybe it is less changes. Let's just log in the tests that we don't run this tests with gang-scheduling.

[helenxie-bit]

@andreyvelich The e2e test for PyTorchJob is still failing due to an image pull backoff error. I suspect it might be caused by insufficient disk space. I think we have two options:

1. Run the e2e test for the train API as a separate GitHub Action.
2. Reduce the size of the kubeflow/pytorch-dist-mnist:latest image by switching to a CPU-based version.

Which approach do you think is better?

[andreyvelich]

Run the e2e test for the train API as a separate GitHub Action.

Can you try to separate them and see if issue will be resolved ?
cc @kubeflow/wg-training-leads

[helenxie-bit]

Run the e2e test for the train API as a separate GitHub Action.

Can you try to separate them and see if issue will be resolved ? cc @kubeflow/wg-training-leads

@andreyvelich I've separated the e2e test for train API and now it works. Please review when you have time.

[andreyvelich]

Thank you for this overall lgtm, just small comment.
/assign @deepanker13 @kubeflow/wg-training-leads @Electronic-Waste

[deepanker13]

/lgtm
Thanks @helenxie-bit

[google-oss-prow]

New changes are detected. LGTM label has been removed.

[helenxie-bit]

I've updated the Kubernetes version to v1.31.4. Please review when you have time @andreyvelich @Electronic-Waste @kubeflow/wg-training-leads

[Electronic-Waste]

Basically LGTM. I left some comments for you @helenxie-bit

[Electronic-Waste]

Shall we change the Kubernetes version to be aligned with other ci tests? Like:

training-operator/.github/workflows/test-example-notebooks.yaml

Lines 16 to 18 in 69094e1

	matrix:
	kubernetes-version: ["v1.28.7", "v1.29.2", "v1.30.6"]
	python-version: ["3.9", "3.10", "3.11"]

[andreyvelich]

Just to save compute resources, I think for now we can just run this test on a single k8s version, since we run the rests E2E tests on the all versions.
WDYT @Electronic-Waste ?

[Electronic-Waste]

Yeah, I agree. Maybe we can select one k8s version from this list:)