Install a separate driver instance for each test by willie-yao · Pull Request #201 · kubernetes-sigs/dra-example-driver

willie-yao · 2026-05-13T17:42:17Z

Each It block now installs its own DRA driver release with a unique driver name, and uninstalls it on teardown.

Fixes #171

Changes:

New test/e2e/driver_test.go with installDriver(ctx, DriverConfig) helper. Shells out to helm (already required by setup-e2e.sh); honors HELM_CHART_PATH env var.
deployManifest accepts a DriverConfig and substitutes gpu.example.com -> per-test driver name (and example.com/gpu -> per-test extended resource name) in demo manifests before applying.
Beyond helm --wait, polls explicitly for DeviceClass + ResourceSlices + webhook readiness before returning from install.
Per-test DeferCleanup LIFO: driver-log diagnostics -> helm uninstall -> namespace delete with foreground propagation + termination wait.
setup-e2e.sh no longer installs the driver. Cert-manager + kind cluster + image build stay (cluster-wide infra).
Webhook specs share one driver pinned to gpu.example.com and run Ordered, Serial so their static testdata stays valid.

Signed-off-by: William Yao <william2000yao@gmail.com>

willie-yao · 2026-05-13T17:46:29Z

+
+// verifyWebhook waits until the validating webhook is serving for the given
+// DeviceClass by creating a dry-run ResourceClaim until it succeeds.
+func verifyWebhook(ctx context.Context, deviceClassName string) {


This code is mostly moved from e2e_setup_test.go at line 107: https://github.com/willie-yao/dra-example-driver/blob/main/test/e2e/e2e_setup_test.go#L107

Changes: now takes a deviceClassNAme parameter, the test claim's Name is suffixed with it due to parallel webhook installs and added DeviceClass to error messages

willie-yao · 2026-05-13T17:48:08Z

+	driverPods, err := clientset.CoreV1().Pods(cfg.Namespace).List(ctx, metav1.ListOptions{
+		LabelSelector: driverPodSelector,
+	})
+	if err != nil {
+		fmt.Fprintf(GinkgoWriter, "Failed to list driver pods: %v\n", err)
+		return
+	}
+	for _, pod := range driverPods.Items {
+		for _, c := range pod.Spec.Containers {
+			stream, err := clientset.CoreV1().Pods(cfg.Namespace).GetLogs(pod.Name, &corev1.PodLogOptions{
+				Container: c.Name,
+				TailLines: &tailLines,
+			}).Stream(ctx)
+			if err != nil {
+				fmt.Fprintf(GinkgoWriter,
+					"Driver pod %s, container %s: failed to get logs: %v\n",
+					pod.Name, c.Name, err)
+				continue
+			}
+			buf := new(bytes.Buffer)
+			_, _ = io.Copy(buf, stream)
+			stream.Close()
+			fmt.Fprintf(GinkgoWriter,
+				"Driver pod %s, container %s (last %d lines):\n%s\n",
+				pod.Name, c.Name, tailLines, buf.String())
+		}
+	}


This is moved from e2e_setup_test.go on line 192: https://github.com/willie-yao/dra-example-driver/blob/main/test/e2e/e2e_setup_test.go#L192

willie-yao · 2026-05-13T17:48:26Z

/assign @nojnhuh

nojnhuh · 2026-05-13T18:22:44Z

+
+// runHelm executes a helm subcommand and returns its combined output.
+func runHelm(ctx context.Context, args ...string) (string, error) {
+	cmd := exec.CommandContext(ctx, "helm", args...)


Can we use Helm's Go library instead of invoking the binary? To keep dependencies separate, we should turn the test directory into a separate module like CAPZ.

Done in 784f6e9. CAPZ doesn't actually have an entirely different submodule but uses a .golangci.yml, but I think a sub-module would work better here anyways.

Ah, CAPI does this but I forgot CAPZ doesn't.

Signed-off-by: William Yao <william2000yao@gmail.com>

k8s-ci-robot · 2026-05-13T19:50:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: willie-yao
Once this PR has been reviewed and has the lgtm label, please ask for approval from nojnhuh. For more information see the Code Review Process.

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

Details

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

Drop the partitionable-devices spec and helmUpgradeDriver helper from main: they assume a single global driver, which is incompatible with the per-test driver install model on this branch. The spec is reintroduced adapted to the new model in a follow-up commit.

The spec from kubernetes-sigs#150 reconfigured a single global driver via helmUpgradeDriver and ran Serial. With the per-test driver install model on this branch it can just install its own release with kubeletPlugin.gpuPartitions=4 via DriverConfig.ExtraValues and run in parallel with everything else.

Signed-off-by: William Yao <william2000yao@gmail.com>

nojnhuh

Sorry for the annoying comments on the workspace/lint periphery here. I promise I'm starting to look at the main point of this PR.

nojnhuh · 2026-05-14T21:09:44Z

 	golangci-lint run ./...
+	cd test && golangci-lint run --build-tags=e2e ./...


This hides lint failures in the test module if any issues are found in the main module. Can we combine into one call?

Suggested change

golangci-lint run ./...

cd test && golangci-lint run --build-tags=e2e ./...

golangci-lint run ./...

golangci-lint run --build-tags=e2e ./... ./test/...

Tried combining it with golangci-lint run --build-tags=e2e ./... ./test/... but it silently doesn't lint the test module. It logs pattern ./test/...: directory prefix test does not contain main module or its selected dependencies and exits 0 with 0 issues. I'm thinking to do it by just creating another make target for test

.PHONY: lint lint-root lint-test lint: @$(MAKE) -k lint-root lint-test lint-root: golangci-lint run ./... lint-test: cd test && golangci-lint run --build-tags=e2e ./...

With a workspace set up, it seems to work for me: willie-yao/dra-example-driver@separate-driver-install...nojnhuh:dra-example-driver:workspace-lint

% make lint golangci-lint run --build-tags=e2e ./... ./test/... cmd/dra-example-webhook/main.go:183:1: Comment should end in a period (godot) // function ^ test/e2e/e2e_setup_test.go:54:1: Comment should end in a period (godot) // driverPodSelector finds kubelet plugin Pods within an installed driver's release namespace ^ 2 issues: * godot: 2 make: *** [Makefile:83: lint] Error 1

Ah that was completely my bad. I had v0.0.0 set before when testing something. Setting it to v0.2.1 fixed it! Thanks for checking this for me

nojnhuh · 2026-05-14T21:20:34Z

+	k8s.io/api v0.36.0
+	k8s.io/apimachinery v0.36.0
+	k8s.io/client-go v0.36.0
+	sigs.k8s.io/dra-example-driver v0.0.0


go mod tidy seems to want to put v0.2.1 here if this doesn't already exist. That should still ultimately resolve to the local version though and not that actual tagged version.

I confirmed locally that with replace => ../ present, v0.2.1 or any other version resolves to the local checkout. Should I change this to v0.2.1?

Signed-off-by: William Yao <william2000yao@gmail.com>

nojnhuh · 2026-05-15T19:43:06Z

+// runUpgradeOrInstall emulates `helm upgrade --install`: if a release with
+// the configured name already exists it is upgraded in place, otherwise a
+// fresh install is performed.
+func runUpgradeOrInstall(ctx context.Context, actionCfg *action.Configuration, install *action.Install, chrt *chart.Chart, values map[string]any) {


In practice, will this ever actually perform an upgrade of an existing Helm release? If we expect each test to have a separate instance of the driver, then I think we'd want to error if that driver already exists instead of upgrade and risk affecting a different test.

The script used helm upgrade --install before mostly to be copy-pasteable, but that was always expected to first be run on a fresh kind cluster without the driver installed where a plain helm install should have worked.

nojnhuh · 2026-05-15T19:49:45Z

+	actionCfg := newHelmActionConfig(cfg.Namespace, registryClient)
+
+	install := action.NewInstall(actionCfg)
+	install.ReleaseName = cfg.ReleaseName


Could we automatically generate a release name instead? That will make sure there aren't conflicts between tests, tests don't have to decide for themselves what the driver should be called, and we don't have to validate them up front.

nojnhuh · 2026-05-15T20:02:59Z

+	Expect(validation.IsDNS1123Label(cfg.ReleaseName)).To(BeEmpty(),
+		"DriverConfig.ReleaseName %q must be a valid DNS-1123 label", cfg.ReleaseName)
+	if cfg.Namespace == "" {
+		cfg.Namespace = cfg.ReleaseName


Like the release name, can we generate a new, separate namespace for each instance of the driver?

nojnhuh · 2026-05-15T20:07:03Z

+	// maxDriverNameLen is the longest driver name that fits within Linux's
+	// 108-byte UNIX_PATH_MAX after the kubelet appends its registrar socket
+	// prefix and per-pod UID suffix.
+	maxDriverNameLen = 28


Can you expand on how this value was calculated?

nojnhuh · 2026-05-15T20:13:16Z

 			name     string
 			fileName string
 		}{
 			{name: "v1 ResourceClaim", fileName: "invalid_rc_v1.yaml"},


Does it simplify things if we construct these ResourceClaims inline vs. reading them from a file?

nojnhuh · 2026-05-15T20:25:20Z

+// with the per-test driver name and (when set) extended resource name.
+func substituteDriverIdentifiers(raw string, drv DriverConfig) string {
+	GinkgoHelper()
+	out := strings.ReplaceAll(raw, defaultDeviceClassName, drv.DriverName)


Eventually, the manifests will be referencing other drivers in addition to gpu.example.com. Some manifests may even rely on multiple separate instances of the driver running different profiles. Not a blocker for this PR, but we should find a better way to handle dynamic driver names.

Install a separate driver instance for each test

5b3d5f0

Signed-off-by: William Yao <william2000yao@gmail.com>

github-project-automation Bot added this to Dynamic Resource Allocation May 13, 2026

github-project-automation Bot moved this to 🆕 New in Dynamic Resource Allocation May 13, 2026

k8s-ci-robot requested review from byako and pohly May 13, 2026 17:42

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 13, 2026

willie-yao commented May 13, 2026

View reviewed changes

k8s-ci-robot assigned nojnhuh May 13, 2026

nojnhuh reviewed May 13, 2026

View reviewed changes

Use Helm Go SDK from a separate test module

784f6e9

Signed-off-by: William Yao <william2000yao@gmail.com>

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 13, 2026

willie-yao commented May 13, 2026

View reviewed changes

Comment thread .github/workflows/lint.yaml Outdated

nojnhuh reviewed May 13, 2026

View reviewed changes

Comment thread test/go.mod Outdated

nojnhuh moved this from 🆕 New to 👀 In review in Dynamic Resource Allocation May 13, 2026

willie-yao added 2 commits May 13, 2026 13:23

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2026

fix: install golangci-lint and lint both modules

c5642db

Signed-off-by: William Yao <william2000yao@gmail.com>

harche mentioned this pull request May 14, 2026

Add KEP-4680 device health reporting with message field support #127

Draft

nojnhuh reviewed May 14, 2026

View reviewed changes

willie-yao added 2 commits May 14, 2026 17:14

lint nits

69c3002

Signed-off-by: William Yao <william2000yao@gmail.com>

Use workspace

27b36c3

Signed-off-by: William Yao <william2000yao@gmail.com>

willie-yao force-pushed the separate-driver-install branch from e5f08a6 to 27b36c3 Compare May 15, 2026 17:17

nojnhuh reviewed May 15, 2026

View reviewed changes

		golangci-lint run ./...
		cd test && golangci-lint run --build-tags=e2e ./...

Conversation

willie-yao commented May 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willie-yao commented May 13, 2026

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!

k8s-ci-robot commented May 13, 2026

Uh oh!

Uh oh!

Uh oh!

nojnhuh left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants