feat(ci): add comprehensive Helm integration tests with JobSet dependency fix #2783

ombhojane · 2025-08-07T21:20:15Z

What this PR does / why we need it:

This PR adds comprehensive Helm integration tests to the GitHub Actions workflow to ensure the Kubeflow Trainer Helm chart functions correctly across different environments and configurations.

The implementation includes:

Chart linting and validation using chart-testing (ct)
Automated dependency resolution for JobSet integration
Kind cluster setup for realistic testing environment
Combined lint-and-install testing as recommended in code review
Comprehensive verification of deployed components
Upgrade testing capabilities
Proper cleanup procedures

Additionally, this PR fixes a critical issue with the JobSet dependency in Chart.yaml where the OCI registry path was incorrect, preventing successful chart installations.

Which issue(s) this PR fixes :

Fixes #2577

Technical Details

Key Changes Made:

Fixed JobSet Dependency: Corrected OCI registry path from oci://registry.k8s.io/jobset/charts/jobset to oci://registry.k8s.io/jobset/charts
Implemented ct lint-and-install: Addressed review feedback from @andreyvelich to use combined testing approach
Added Chart Testing Configuration: Created .github/ct.yaml with proper timeout and dependency settings
Enhanced Verification: Added comprehensive checks for pods, services, CRDs, and component health

Workflow Features:

Automatically detects target branch for PR and push events
Sets up Kind cluster with fallback to project's make targets
Runs dependency updates and template validation
Performs installation testing with proper timeout handling
Verifies both Trainer and JobSet components are deployed correctly
Includes upgrade testing capabilities
Ensures proper cleanup of test resources

Testing Approach:

Lint Phase: Validates chart structure, dependencies, and templating
Install Phase: Tests actual deployment in Kind cluster environment
Verification Phase: Confirms all expected components are running
Upgrade Phase: Tests chart upgrade scenarios

This addresses all requirements from issue #2577 and incorporates feedback from the previous PR review process, ensuring the Trainer Helm chart maintains quality and reliability standards.

google-oss-prow · 2025-08-07T21:20:21Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign andreyvelich for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

kramaranya · 2025-08-07T21:36:18Z

Thank you for your first contribution @ombhojane! 🎉
Could you please sign your commits?

ombhojane · 2025-08-08T04:10:31Z

Hey @kramaranya
I've signed the commits & pushed the changes:

andreyvelich · 2025-08-08T16:34:52Z

/ok-to-test

coveralls · 2025-08-08T16:39:30Z

Pull Request Test Coverage Report for Build 18153337284

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 55.174%

Totals
Change from base Build 18132966300:	0.0%
Covered Lines:	1093
Relevant Lines:	1981

💛 - Coveralls

ombhojane · 2025-08-11T05:28:17Z

hey @andreyvelich
i've resolved the issues raised while actions were running
please review

andreyvelich

/ok-to-test

@ombhojane Can you sign your commits please ?

andreyvelich · 2025-08-11T13:52:08Z

/retest

andreyvelich

Thanks @ombhojane!
I left my initial comments.

.github/workflows/test-helm.yaml

charts/kubeflow-trainer/Chart.yaml

ombhojane · 2025-08-11T14:06:05Z

thanks for detailed review, i'm going thru & resolving this

Signed-off-by: ombhojane <[email protected]>

…tall commands compatible with Debian image (kubeflow#2528) Signed-off-by: Debabrata47 <[email protected]> Signed-off-by: ombhojane <[email protected]>

Signed-off-by: ombhojane <[email protected]>

ombhojane · 2025-08-12T08:26:57Z

hey @andreyvelich
can you please review the codes.

andreyvelich

/cc @kubeflow/kubeflow-trainer-team @astefanutti

.github/ct.yaml

cmd/initializers/dataset/Dockerfile

.github/workflows/test-helm.yaml

andreyvelich · 2025-08-12T18:30:55Z

.github/workflows/test-helm.yaml

+          # Wait for JobSet to be ready
+          kubectl wait --for=condition=ready pod -l app.kubernetes.io/name=jobset --timeout=300s -n jobset-system
+
+      - name: Create test values file for chart testing


Can we leverage the e2e-setup-cluster.sh script to deploy Helm Chart ?

I think, the steps are similar, and we should just deploy it with Helm Charts instead of Kustomize manifests.

I agree with @andreyvelich this should ideally not be duplicated.

leveraged e2e-setup-cluster-helm.sh

@ombhojane As I can see, you still build and load images as part of your e2e-setup-cluster-helm.sh script.
I suggest that you just add condition here, to whether you want to deploy Kubeflow Trainer with Kustomize or Helm Charts:

trainer/hack/e2e-setup-cluster.sh

Lines 43 to 54 in 5c02086

echo "Deploy Kubeflow Trainer control plane"

E2E_MANIFESTS_DIR="artifacts/e2e/manifests"

mkdir -p "${E2E_MANIFESTS_DIR}"

cat <<EOF >"${E2E_MANIFESTS_DIR}/kustomization.yaml"

apiVersion: kustomize.config.k8s.io/v1beta1

kind: Kustomization

resources:

- ../../../manifests/overlays/manager

images:

- name: "${CONTROLLER_MANAGER_CI_IMAGE_NAME}"

newTag: "${CONTROLLER_MANAGER_CI_IMAGE_TAG}"

EOF

In that case, you don't need to maintain separate script just for Helm deployment.

google-oss-prow · 2025-08-12T18:31:27Z

@andreyvelich: GitHub didn't allow me to request PR reviews from the following users: kubeflow/kubeflow-trainer-team.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @kubeflow/kubeflow-trainer-team @astefanutti

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

astefanutti · 2025-08-21T08:43:35Z

.github/workflows/test-helm.yaml

+          # Wait for JobSet to be ready
+          kubectl wait --for=condition=ready pod -l app.kubernetes.io/name=jobset --timeout=300s -n jobset-system
+
+      - name: Create test values file for chart testing


I agree with @andreyvelich this should ideally not be duplicated.

.github/workflows/test-helm.yaml

ombhojane · 2025-08-21T09:49:10Z

sure, I'll enhance the flow as suggested

…m-integration-tests

andreyvelich · 2025-10-08T01:56:05Z

.github/workflows/test-helm.yaml

+      - name: Setup cluster and deploy with Helm
+        run: |
+          make test-e2e-setup-cluster-helm K8S_VERSION=1.32.0
+
+      - name: Run E2E smoke tests
+        run: |
+          NAMESPACE="kubeflow-system"


After you modify the e2e setup script you should be able to just run:

- name: Setup cluster run: | make test-e2e-setup-cluster K8S_VERSION=${{ matrix.kubernetes-version }} DEPLOYMENT_METHOD=helm - name: Run e2e with Go run: | make test-e2e || (kubectl logs -n kubeflow-system -l app.kubernetes.io/name=trainer && exit 1)

google-oss-prow bot requested review from astefanutti and kuizhiqing August 7, 2025 21:20

google-oss-prow bot added the size/L label Aug 7, 2025

ombhojane force-pushed the helm-integration-tests branch from 81f7fc6 to 6c7a173 Compare August 8, 2025 04:07

google-oss-prow bot added the ok-to-test label Aug 8, 2025

andreyvelich reviewed Aug 11, 2025

View reviewed changes

ombhojane force-pushed the helm-integration-tests branch from b0c7f9f to a6407b7 Compare August 11, 2025 15:33

ombhojane and others added 9 commits August 12, 2025 01:34

implemented Helm integration tests

d3a73f3

Signed-off-by: ombhojane <[email protected]>

feat(initializer): Updated base image to Debian image and changed ins…

eb623e7

…tall commands compatible with Debian image (kubeflow#2528) Signed-off-by: Debabrata47 <[email protected]> Signed-off-by: ombhojane <[email protected]>

test case issue resolved

ca6d760

Signed-off-by: ombhojane <[email protected]>

Helm integration test

b356099

Signed-off-by: ombhojane <[email protected]>

remove namespace specification

50fbeac

Signed-off-by: ombhojane <[email protected]>

target branch fix

716a0f9

Signed-off-by: ombhojane <[email protected]>

repository name fixed

519e66b

Signed-off-by: ombhojane <[email protected]>

error handling

00360b8

Signed-off-by: ombhojane <[email protected]>

dry run fixed

6fa4375

Signed-off-by: ombhojane <[email protected]>

ombhojane force-pushed the helm-integration-tests branch from a68cbfc to 6fa4375 Compare August 11, 2025 20:04

andreyvelich reviewed Aug 12, 2025

View reviewed changes

astefanutti reviewed Aug 21, 2025

View reviewed changes

ombhojane added 2 commits October 1, 2025 10:44

Merge branch 'master' of https://github.com/kubeflow/trainer into hel…

4c76c56

…m-integration-tests

refactor: updated tests

5c02086

andreyvelich reviewed Oct 8, 2025

View reviewed changes

	echo "Deploy Kubeflow Trainer control plane"
	E2E_MANIFESTS_DIR="artifacts/e2e/manifests"
	mkdir -p "${E2E_MANIFESTS_DIR}"
	cat <<EOF >"${E2E_MANIFESTS_DIR}/kustomization.yaml"
	apiVersion: kustomize.config.k8s.io/v1beta1
	kind: Kustomization
	resources:
	- ../../../manifests/overlays/manager
	images:
	- name: "${CONTROLLER_MANAGER_CI_IMAGE_NAME}"
	newTag: "${CONTROLLER_MANAGER_CI_IMAGE_TAG}"
	EOF

feat(ci): add comprehensive Helm integration tests with JobSet dependency fix #2783

Are you sure you want to change the base?

feat(ci): add comprehensive Helm integration tests with JobSet dependency fix #2783

Uh oh!

Conversation

ombhojane commented Aug 7, 2025

Technical Details

Key Changes Made:

Workflow Features:

Testing Approach:

Uh oh!

google-oss-prow bot commented Aug 7, 2025

Uh oh!

kramaranya commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ombhojane commented Aug 8, 2025

Uh oh!

andreyvelich commented Aug 8, 2025

Uh oh!

coveralls commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18153337284

Details

💛 - Coveralls

Uh oh!

ombhojane commented Aug 11, 2025

Uh oh!

andreyvelich left a comment

Choose a reason for hiding this comment

Uh oh!

andreyvelich commented Aug 11, 2025

Uh oh!

andreyvelich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ombhojane commented Aug 11, 2025

Uh oh!

ombhojane commented Aug 12, 2025

Uh oh!

andreyvelich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andreyvelich Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

astefanutti Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

ombhojane Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ombhojane Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

andreyvelich Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

google-oss-prow bot commented Aug 12, 2025

Uh oh!

astefanutti Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ombhojane commented Aug 21, 2025

Uh oh!

andreyvelich Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

kramaranya commented Aug 7, 2025 •

edited

Loading

coveralls commented Aug 8, 2025 •

edited

Loading