feat: add e2e tests to Helm CI workflow

[Goku2099]

This PR extends the existing Helm CI workflow by adding an E2E validation step.

#3230

[Copilot]

Pull request overview

This PR extends the Helm CI workflow to run end-to-end validation by provisioning a Kind cluster and executing the Go E2E test suite, addressing Issue #3230.

Changes:

• Add a Kind cluster setup step to the Helm CI workflow.
• Run Go E2E tests (make test-e2e) as part of the Helm workflow.

[Goku2099]

@andreyvelich The Coveralls step is failing with HTTP 530 (error code 1016).

This appears to be an external Coveralls issue rather than a CI or coverage configuration problem. The coverage file is generated successfully, but the upload fails.

Could you please confirm if we should re-run the job or temporarily ignore this failure?

[Goku2099]

@andreyvelich can you review this one too.

[Goku2099]

Earlier it looked like a config difference because Helm wasn’t enabling the feature gate, but after updating Helm to use manager.config.featureGates, both setups should be aligned now.

With that change, most tests pass, but the DeadlineExceeded case still fails in Helm while kustomize passes. The TrainJob fails as expected, but the underlying JobSet isn’t getting deleted and the test times out waiting for it, which looks similar to #3358.

So it doesn’t seem to be just a config difference anymore there might be some difference in how resources are applied or reconciled between Helm and kustomize. I’ll investigate further.

[Goku2099]

hey @andreyvelich
I investigated the Helm e2e failure for the DeadlineExceeded case.

The TrainJob is correctly marked as failed with DeadlineExceeded, but the test fails because the underlying JobSet is not getting deleted.

In the controller, the deletion happens here:

trainer/pkg/controller/trainjob_controller.go

Lines 185 to 194 in 9c598fb

	if now.After(deadline) {
	ctrl.LoggerFrom(ctx).V(2).Info("TrainJob deadline exceeded, marking as failed",
	"activeDeadlineSeconds", trainJob.Spec.ActiveDeadlineSeconds,
	"startTime", startTime,
	"deadline", deadline)
	setFailedCondition(trainJob, constants.TrainJobDeadlineExceededMessage, trainer.TrainJobDeadlineExceededReason)
	jobSet := &jobsetv1alpha2.JobSet{
	ObjectMeta: metav1.ObjectMeta{Name: trainJob.Name, Namespace: trainJob.Namespace},
	}
	if err := client.IgnoreNotFound(r.client.Delete(ctx, jobSet)); err != nil {

We are deleting the JobSet using a constructed object (only name/namespace), without fetching the existing resource first.

From what I observed:

• Kustomize e2e passes → JobSet gets deleted
• Helm e2e fails → JobSet remains and the test times out

So the behavior is inconsistent even though the same test is used.

I’m trying to understand:

1. What guarantees JobSet deletion in this flow?
2. Is deleting via a constructed object intentional, or should we fetch the JobSet before deleting it?
3. Could differences between Helm and kustomize (e.g., reconciliation timing or resource state) affect deletion behavior here?

[andreyvelich]

I am not sure why we have different logic for Helm vs Kustomize.
Can you try to deploy it locally on Kind cluster and verify?

@aniket2405 @jaiakash @astefanutti @abhijeet-dhumal @robert-bell @XploY04 @Krishna-kg732 Any thoughts?

[robert-bell]

My guess is there's a difference between the helm and kustomize manifests. @Goku2099 you could try rendering both of them locally and comparing.

[XploY04]

Kustomize grants create, delete, get, list, patch, update, watch on jobsets.jobset.x-k8s.io. The Helm ClusterRole is missing delete.

At pkg/controller/trainjob_controller.go:194 the controller calls Delete on the JobSet for DeadlineExceeded and swallows the error. Under Helm the API denies with forbidden, so the TrainJob still gets the failed condition but the JobSet lingers and the e2e times out. Under Kustomize the verb is granted, delete succeeds.

Fix is one line: add - delete to the jobsets verbs in the Helm ClusterRole. @Goku2099 please add this and try.

[andreyvelich]

Great catch @XploY04!
@Goku2099 please can we ensure that ClusterRole in Helm charts is aligned with this: https://github.com/kubeflow/trainer/blob/master/manifests/base/rbac/role.yaml#L40

[Goku2099]

@andreyvelich
I updated the RBAC by adding the missing delete verb.
However, now the CI is failing during the image build step with package installation errors. This seems unrelated to the RBAC change, so I’m investigating and fixing the Docker build issues to get CI green.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[XploY04]

@Goku2099 I have made the fix in a new PR for the failing GPU test, please wait.

[Goku2099]

@Goku2099 I have made the fix in a new PR for the failing GPU test, please wait.

Thanks for the update! I’ll wait for your PR and rebase/adjust mine accordingly

[andreyvelich]

@Goku2099 Can you rebase your PR please?

[Goku2099]

@XploY04 @robert-bell Thanks for the help and guidance on this
@andreyvelich CI is green now, could you please review

[andreyvelich]

Thanks for this work @Goku2099!
/lgtm
/approve

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [andreyvelich]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[andreyvelich]

/hold cancel