[no-relnote] Update E2E test suite #1242

ArangoGutierrez · 2025-04-24T12:21:52Z

This pull request introduces significant updates to the end-to-end (E2E) testing framework for the NVIDIA Kubernetes Device Plugin. The changes include enhancements to the CI workflow, a migration to the Ginkgo v2 testing framework, restructuring of test-related files, and updates to licensing information. Below is a categorized summary of the most important changes:

Workflow and CI Updates:

Added a new HELM_CHART environment variable to the E2E CI workflow (.github/workflows/e2e.yaml) and updated the make command to use a specific Makefile for E2E tests. This improves configurability and aligns the workflow with the new test structure.
Replaced the Slack alert notification step with a new Ginkgo log archiving step and updated the Slack notification configuration to use a JSON payload for better integration.

Migration to Ginkgo v2:

Updated the Makefile to include a ginkgo target for installing the Ginkgo v2 CLI and a new test-e2e target for running tests with Ginkgo. This replaces the previous custom test logic.
Renamed and restructured test files to align with Ginkgo's conventions, such as renaming common/kubernetes.go to cleanup_test.go and removing redundant utility functions. [1] [2]

Documentation Enhancements:

Added a comprehensive README.md for the E2E test suite, detailing prerequisites, environment variables, execution flow, and troubleshooting steps. This improves developer onboarding and test maintainability.

Licensing and Copyright:

Updated all test-related files to use the SPDX license headers, reflecting the transition to a 2025 copyright year and the Apache 2.0 license. [1] [2] [3]

Codebase Simplification:

Removed unused test utilities, such as gpu_job.go and taints.go, consolidating functionality into more focused test files. This reduces redundancy and simplifies the codebase. [1] [2]

These changes collectively modernize the testing framework, improve CI integration, and enhance developer experience.

Copilot

Pull Request Overview

This PR modernizes the NVIDIA Kubernetes Device Plugin’s E2E test framework by upgrading to Ginkgo v2, updating CI workflows, and adjusting file structure and licensing information.

Migrates tests from legacy framework patterns to Ginkgo v2 constructs
Updates CI workflow to use environment variables and a dedicated Makefile target
Removes deprecated utility files and refactors test helpers for consistency

Reviewed Changes

Copilot reviewed 405 out of 407 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/e2e/utils.go	Updated license headers and refactored helper functions with adjusted timeouts
tests/e2e/infra/aws.yaml	Modified configuration for helm chart deployments and toolkit settings
tests/e2e/gpu-feature-discovery_test.go	Migration to Ginkgo v2 and changes to use environment variables and updated client references
tests/e2e/e2e_test.go	Overhauled test entry point to use environment variables and new helper functions
tests/e2e/device-plugin_test.go	Refactored test structure and naming for Helm release names and client usage
tests/e2e/cleanup_test.go	Consolidated cleanup functions for node, resource, and CRD removal
tests/e2e/README.md	Added comprehensive documentation for running and configuring the E2E suite
.github/workflows/e2e.yaml	Updated workflow steps to use a Makefile in the E2E directory and improved artifact handling

Files not reviewed (2)

tests/e2e/Makefile: Language not supported
tests/go.mod: Language not supported

Comments suppressed due to low confidence (3)

tests/e2e/device-plugin_test.go:80

[nitpick] Switching to randomSuffix() for generating helmReleaseName improves uniqueness; consider updating any accompanying comments or documentation to reflect this change in naming strategy.

helmReleaseName = "nvdp-e2e-test-" + randomSuffix()

.github/workflows/e2e.yaml:66

[nitpick] The workflow now explicitly references a Makefile in the tests/e2e directory; verify that this path remains accurate and that all necessary targets are defined in the referenced Makefile.

make -f tests/e2e/Makefile test-e2e

tests/e2e/utils.go:45

The timeout for eventuallyNonControlPlaneNodes has been increased from 10 seconds to 1 minute; please confirm that this longer duration aligns with the overall test responsiveness and cluster performance expectations.

}).WithPolling(1 * time.Second).WithTimeout(1 * time.Minute).WithContext(ctx)

elezar · 2025-04-24T16:12:32Z

tests/e2e/infra/aws.yaml

Is the reason for this change just consistency with the DRA driver?

yes, I am now working to bring more consistency across all the repos we manage

tests/e2e/Makefile

.github/workflows/e2e.yaml

tests/e2e/README.md

tests/e2e/cleanup_test.go

tests/e2e/utils.go

tests/e2e/Makefile

Copilot

Pull Request Overview

This pull request modernizes the E2E test framework by migrating to Ginkgo v2, reorganizing test files, updating CI workflows, and refreshing licensing and documentation. Key changes include the restructuring and renaming of test files (e.g. renaming common/kubernetes.go to cleanup_test.go), enhancement of CI configurations (e.g. new Makefile target and Slack payload update), and removal of obsolete utilities.

Reviewed Changes

Copilot reviewed 406 out of 408 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/e2e/infra/aws.yaml	Adjusted configuration ordering and added new keys for CDI enablement.
tests/e2e/gpu-feature-discovery_test.go	Migrated test syntax and variable usage to align with Ginkgo v2 conventions.
tests/e2e/device-plugin_test.go	Updated Helm chart installation and job creation logic.
tests/e2e/cleanup_test.go	Refactored cleanup functions; moved functions from package common to e2e.
tests/e2e/README.md	Added comprehensive usage and troubleshooting documentation.
.github/workflows/e2e.yaml	Updated environment variables, Makefile target, and Slack notification steps.

Files not reviewed (2)

tests/e2e/Makefile: Language not supported
tests/go.mod: Language not supported

Comments suppressed due to low confidence (3)

tests/e2e/cleanup_test.go:267

The 'ctx' variable is used in the cleanupNodeFeatureRules function without being defined. Consider passing a context parameter to this function to correctly scope API calls.

nfrs, err := cli.NfdV1alpha1().NodeFeatureRules().List(ctx, metav1.ListOptions{})

tests/e2e/cleanup_test.go:69

The variable 'ctx' is used in the cleanupTestPods function without being defined or passed as an argument. Consider either passing a context parameter or declaring 'ctx' locally.

podList, err := clientSet.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{

tests/e2e/cleanup_test.go:114

The variable 'ctx' is used without being defined in this cleanup helper. Ensure that a valid context is passed to this function or declared within its scope to avoid runtime errors.

err := clientSet.CoreV1().Namespaces().Delete(ctx, testNamespace.Name, metav1.DeleteOptions{})

tests/e2e/device-plugin_test.go

ArangoGutierrez · 2025-05-07T09:14:51Z

Thanks for the review and approval @tariq1890 , any additional comments @elezar ?

ArangoGutierrez · 2025-05-14T14:17:47Z

Now that we have merged the PR in CTK can we do a final round here @elezar

ArangoGutierrez · 2025-07-24T10:00:51Z

Branch rebased on main, PR ready for review
@elezar

ArangoGutierrez · 2025-08-04T10:58:04Z

@elezar Now E2E pass
This PR includes the bump to Holodeck as well, to provide a full update to the E2E suite

elezar · 2025-08-13T12:07:21Z

tests/e2e/device-plugin_test.go

+		// Note: DaemonSet names are dynamically generated with the Helm release prefix,
+		// so we wait for all DaemonSets in the namespace rather than specific names
+		By("Waiting for all DaemonSets to be ready")
+		err = internal.WaitForAllDaemonSetsReady(ctx, clientSet, testNamespace.Name)


Do the DaemonSets also have app labels, or some other identifier that we could use to further restrict this set?

Yes, the DaemonSets have labels set during Helm install. But since the namespace is unique for the test run, this is enough to capture the objects we are monitoring.

But I have added label filtering

elezar · 2025-08-13T12:11:49Z

tests/e2e/e2e_test.go

+	Kubeconfig = os.Getenv("KUBECONFIG")
+	Expect(Kubeconfig).NotTo(BeEmpty(), "KUBECONFIG must be set")


Nit: In nvidia-container-toolkit we have the concept of a REQUIRED envvar that we encapsulated in a function.

elezar · 2025-08-13T12:14:30Z

tests/e2e/e2e_test.go

+
+// CreateOrUpdateJobsFromFile creates or updates jobs from a file
+func CreateOrUpdateJobsFromFile(ctx context.Context, cli clientset.Interface, filename, namespace string) ([]string, error) {
+	jobs, err := newJobFromfile(filepath.Join(packagePath, "data", filename))


My one question would be why don't we provide the full relative path to the file instead of ASSUMING data? Searching for data/job-1.yaml when we do decide to move these files (or add new ones) will be simpler than searching for each path component separately.

elezar · 2025-08-13T12:20:12Z

tests/e2e/e2e_test.go

+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				_, err = cli.BatchV1().Jobs(namespace).Create(ctx, job, metav1.CreateOptions{})
+				if err != nil {
+					return nil, fmt.Errorf("failed to create Job %s: %w", job.Name, err)
+				}
+			} else {
+				return nil, fmt.Errorf("failed to get Job %s: %w", job.Name, err)
+			}
+		}


Is this nesting patter common in other projects?
We could use:

Suggested change

if err != nil {

if apierrors.IsNotFound(err) {

_, err = cli.BatchV1().Jobs(namespace).Create(ctx, job, metav1.CreateOptions{})

if err != nil {

return nil, fmt.Errorf("failed to create Job %s: %w", job.Name, err)

}

} else {

return nil, fmt.Errorf("failed to get Job %s: %w", job.Name, err)

}

}

if apierrors.IsNotFound(err) {

if _, cerr := cli.BatchV1().Jobs(namespace).Create(ctx, job, metav1.CreateOptions{}); cerr != nil {

err = errors.Join(err, fmt.Errorf("failed to create job: %w", cerr))

}

}

if err != nil {

return nil, fmt.Errorf("failed to get Job %s: %w", job.Name, err)

}

elezar · 2025-08-13T12:21:02Z

tests/e2e/e2e_test.go

+		return nil, err
+	}
+
+	// TODO: find out a nicer way to decode multiple api objects from a single


How does kubectl handle this?

Copilot

Pull Request Overview

This pull request introduces significant updates to the end-to-end (E2E) testing framework for the NVIDIA Kubernetes Device Plugin, migrating from a custom test framework to Ginkgo v2 and enhancing CI integration.

Migration to Ginkgo v2 testing framework with modern patterns and improved test structure
Enhanced CI workflow configuration with updated Helm chart environment variables and improved logging
Comprehensive codebase refactoring with removal of legacy framework components and consolidation of test utilities

Reviewed Changes

Copilot reviewed 18 out of 52 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/go.mod	Updates Go version to 1.24.0 and refreshes dependency versions including Ginkgo v2 and test infrastructure libraries
tests/e2e/e2e_test.go	Complete rewrite implementing Ginkgo v2 patterns with suite-level setup, global clients, and comprehensive test utilities
tests/e2e/device-plugin_test.go	Modernizes device plugin tests using new Ginkgo v2 syntax and shared test infrastructure
tests/e2e/gpu-feature-discovery_test.go	Updates GFD tests to use new framework patterns and removes deprecated dependencies
tests/e2e/internal/kube.go	Adds new Kubernetes utility functions for DaemonSet and Deployment management with proper context handling
tests/e2e/infra/aws.yaml	Simplifies AWS infrastructure configuration by removing hardcoded IP ranges and updating container toolkit settings
tests/e2e/README.md	Provides comprehensive documentation for the updated E2E test suite with setup instructions and troubleshooting
tests/e2e/Makefile	Replaces custom test logic with Ginkgo CLI integration and adds proper artifact management
.github/workflows/e2e.yaml	Updates CI workflow to use new Makefile targets and improves artifact collection with Ginkgo log archiving

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

Copilot · 2025-08-14T09:26:39Z

tests/e2e/e2e_test.go

+	suiteName := "E2E K8s Device Plugin"
+
+	RegisterFailHandler(Fail)
+


TestMain should take *testing.M as parameter, not *testing.T. The correct signature is func TestMain(m *testing.M). This will cause compilation errors.

Suggested change

func TestMain(m *testing.M) {

Copilot · 2025-08-14T09:26:40Z

tests/e2e/e2e_test.go

+	// Log random seed for reproducibility
+	GinkgoWriter.Printf("Random seed: %d\n", GinkgoRandomSeed())
+
+	RunSpecs(t,


Since TestMain receives *testing.T (which should be *testing.M), this call to RunSpecs with t parameter will fail. RunSpecs expects *testing.T but TestMain should provide *testing.M and call m.Run().

Copilot · 2025-08-14T09:26:40Z

tests/e2e/e2e_test.go

+
+// eventuallyNonControlPlaneNodes is a helper for asserting node properties
+//
+//nolint:unused


The eventuallyNonControlPlaneNodes function is marked as unused but appears to be used in other test files. Consider removing the unused annotation or verifying if this function is actually needed.

Suggested change

//nolint:unused

PTAL

copy-pr-bot · 2025-09-15T14:39:29Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>

Signed-off-by: Evan Lezar <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 15 out of 18 changed files in this pull request and generated 5 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-17T14:47:36Z

tests/e2e/device-plugin_test.go

+			By("Creating a GPU job")
+			jobNames, err := CreateOrUpdateJobsFromFile(ctx, clientSet, testNamespace.Name, filepath.Join(projectRoot, "testdata", "job-1.yaml"))
+			Expect(err).NotTo(HaveOccurred())
+			Expect(jobNames).NotTo(HaveLen(1))


The assertion logic is inverted. This will fail when exactly one job is created (the expected case). Should be Expect(jobNames).To(HaveLen(1)) to verify that exactly one job was created.

Suggested change

Expect(jobNames).NotTo(HaveLen(1))

Expect(jobNames).To(HaveLen(1))

Copilot · 2025-09-17T14:47:36Z

tests/e2e/gpu-feature-discovery_test.go

+			fmt.Sprintf("image.repository=%s", ImageRepo),
+			fmt.Sprintf("image.tag=%s", ImageTag),
+			fmt.Sprintf("image.pullPolicy=%s", ImagePullPolicy),


These variables (ImageRepo, ImageTag, ImagePullPolicy) are referenced without pointer dereference, but they appear to be global variables that should be dereferenced. The original code used *ImageRepo, *ImageTag, *ImagePullPolicy.

Suggested change

fmt.Sprintf("image.repository=%s", ImageRepo),

fmt.Sprintf("image.tag=%s", ImageTag),

fmt.Sprintf("image.pullPolicy=%s", ImagePullPolicy),

fmt.Sprintf("image.repository=%s", *ImageRepo),

fmt.Sprintf("image.tag=%s", *ImageTag),

fmt.Sprintf("image.pullPolicy=%s", *ImagePullPolicy),

Copilot · 2025-09-17T14:47:37Z

tests/e2e/device-plugin_test.go

+			fmt.Sprintf("image.repository=%s", ImageRepo),
+			fmt.Sprintf("image.tag=%s", ImageTag),
+			fmt.Sprintf("image.pullPolicy=%s", ImagePullPolicy),


These variables (ImageRepo, ImageTag, ImagePullPolicy) are referenced without pointer dereference, but they appear to be global variables that should be dereferenced. The original code used *ImageRepo, *ImageTag, *ImagePullPolicy.

Suggested change

fmt.Sprintf("image.repository=%s", ImageRepo),

fmt.Sprintf("image.tag=%s", ImageTag),

fmt.Sprintf("image.pullPolicy=%s", ImagePullPolicy),

fmt.Sprintf("image.repository=%s", *ImageRepo),

fmt.Sprintf("image.tag=%s", *ImageTag),

fmt.Sprintf("image.pullPolicy=%s", *ImagePullPolicy),

Copilot · 2025-09-17T14:47:37Z

tests/e2e/gpu-feature-discovery_test.go

+			var err error
+			diagnosticsCollector, err = diagnostics.New(
+				diagnostics.WithNamespace(testNamespace.Name),
+				diagnostics.WithArtifactDir(LogArtifactDir),


LogArtifactDir variable is referenced without pointer dereference, but appears to be a global variable that should be dereferenced. The original code used *LogArtifactDir.

Suggested change

diagnostics.WithArtifactDir(LogArtifactDir),

diagnostics.WithArtifactDir(*LogArtifactDir),

Copilot · 2025-09-17T14:47:37Z

tests/e2e/device-plugin_test.go

+			var err error
+			diagnosticsCollector, err = diagnostics.New(
+				diagnostics.WithNamespace(testNamespace.Name),
+				diagnostics.WithArtifactDir(LogArtifactDir),


LogArtifactDir variable is referenced without pointer dereference, but appears to be a global variable that should be dereferenced. The original code used *LogArtifactDir.

Suggested change

diagnostics.WithArtifactDir(LogArtifactDir),

diagnostics.WithArtifactDir(*LogArtifactDir),

Signed-off-by: Evan Lezar <[email protected]>

ArangoGutierrez requested review from Copilot and elezar April 24, 2025 12:21

ArangoGutierrez self-assigned this Apr 24, 2025

Copilot AI reviewed Apr 24, 2025

View reviewed changes

ArangoGutierrez force-pushed the ginkgo-e2e branch from 9037244 to 2c5aadd Compare April 24, 2025 12:39

elezar reviewed Apr 24, 2025

View reviewed changes

tests/e2e/Makefile Outdated Show resolved Hide resolved

ArangoGutierrez force-pushed the ginkgo-e2e branch from 2c5aadd to 047d5f4 Compare April 25, 2025 11:57

ArangoGutierrez requested a review from elezar April 25, 2025 11:57

ArangoGutierrez force-pushed the ginkgo-e2e branch 3 times, most recently from b508d98 to 4937906 Compare April 25, 2025 18:33

tariq1890 reviewed Apr 30, 2025

View reviewed changes

ArangoGutierrez force-pushed the ginkgo-e2e branch from 4937906 to 60a5ecf Compare May 2, 2025 14:02

ArangoGutierrez requested review from Copilot and tariq1890 May 2, 2025 14:02

Copilot AI reviewed May 2, 2025

View reviewed changes

tests/e2e/device-plugin_test.go Outdated Show resolved Hide resolved

tariq1890 previously approved these changes May 2, 2025

View reviewed changes

ArangoGutierrez added the testing issue/PR to fix/edit/create/enhance a project unit/e2e test label May 26, 2025

ArangoGutierrez force-pushed the ginkgo-e2e branch from 60a5ecf to f98f08c Compare July 24, 2025 10:00

ArangoGutierrez force-pushed the ginkgo-e2e branch 2 times, most recently from 68f3bea to 474583c Compare July 24, 2025 10:37

ArangoGutierrez force-pushed the ginkgo-e2e branch 4 times, most recently from b90ff8e to a9b9002 Compare August 4, 2025 10:34

elezar reviewed Aug 13, 2025

View reviewed changes

ArangoGutierrez force-pushed the ginkgo-e2e branch from 4a340b3 to 2322a06 Compare August 13, 2025 13:54

ArangoGutierrez requested review from Copilot and elezar August 13, 2025 13:54

This comment was marked as outdated.

Sign in to view

ArangoGutierrez requested a review from Copilot August 14, 2025 09:24

Copilot AI reviewed Aug 14, 2025

View reviewed changes

ArangoGutierrez force-pushed the ginkgo-e2e branch 2 times, most recently from 8baf4e8 to 0ae8c6c Compare August 14, 2025 11:01

ArangoGutierrez force-pushed the ginkgo-e2e branch from 0ae8c6c to d98f962 Compare August 15, 2025 09:09

elezar mentioned this pull request Aug 19, 2025

Switch to single ci definition #1363

Merged

elezar force-pushed the ginkgo-e2e branch from d98f962 to 30b4508 Compare September 15, 2025 14:39

elezar force-pushed the ginkgo-e2e branch 3 times, most recently from 9052993 to 00b810a Compare September 17, 2025 12:24

ArangoGutierrez and others added 3 commits September 17, 2025 14:42

[no-relnote] Update E2E test suite

fc56d14

Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>

[no-relnote] Minor cleanups

67e0b07

Signed-off-by: Evan Lezar <[email protected]>

TOFIX: reduce diff

df67aa8

Signed-off-by: Evan Lezar <[email protected]>

elezar force-pushed the ginkgo-e2e branch from af86518 to df67aa8 Compare September 17, 2025 12:42

elezar added 2 commits September 17, 2025 15:43

[no-relnote] Set ginkgo version on install

189747b

Signed-off-by: Evan Lezar <[email protected]>

fixup! TOFIX: reduce diff

d2dde98

Signed-off-by: Evan Lezar <[email protected]>

ArangoGutierrez requested a review from Copilot September 17, 2025 14:46

Copilot AI reviewed Sep 17, 2025

View reviewed changes

fixup! [no-relnote] Minor cleanups

49680b4

Signed-off-by: Evan Lezar <[email protected]>

		Kubeconfig = os.Getenv("KUBECONFIG")
		Expect(Kubeconfig).NotTo(BeEmpty(), "KUBECONFIG must be set")

		suiteName := "E2E K8s Device Plugin"

		RegisterFailHandler(Fail)

	Expect(jobNames).NotTo(HaveLen(1))
	Expect(jobNames).To(HaveLen(1))

	diagnostics.WithArtifactDir(LogArtifactDir),
	diagnostics.WithArtifactDir(*LogArtifactDir),

[no-relnote] Update E2E test suite #1242

Are you sure you want to change the base?

[no-relnote] Update E2E test suite #1242

Uh oh!

Conversation

ArangoGutierrez commented Apr 24, 2025

Workflow and CI Updates:

Migration to Ginkgo v2:

Documentation Enhancements:

Licensing and Copyright:

Codebase Simplification:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

ArangoGutierrez commented May 7, 2025

Uh oh!

ArangoGutierrez commented May 14, 2025

Uh oh!

ArangoGutierrez commented Jul 24, 2025

Uh oh!

ArangoGutierrez commented Aug 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

copy-pr-bot bot commented Sep 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!