DRA: CRUD conformance tests #133747

pohly · 2025-08-28T09:15:44Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Promoting real tests (#133132) turned out to be harder than expected (should be rewritten to be self-contained, additional reviews, etc.).

They would not achieve 100% endpoint+operation coverage because real tests only use some of the operations. Therefore each API type has to be covered with CRUD-style tests which only exercise the apiserver, then maybe additional functional tests can be added later (depending on time and motivation).

Which issue(s) this PR is related to:

#114183
#133691
KEP: kubernetes/enhancements#4381

Special notes for your reviewer:

The machinery for testing different API types is meant to be reusable, so it gets added in the new e2e/framework/conformance helper package.

Does this PR introduce a user-facing change?

NONE

k8s-ci-robot · 2025-08-28T09:15:53Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

pohly · 2025-08-28T09:16:36Z

/test pull-kubernetes-audit-kind-conformance

DeleteCollection not implemented yet, checking whether the job finds that...

pohly · 2025-08-28T13:28:16Z

/test pull-kubernetes-audit-kind-conformance

Now with DeleteCollection, non-namespace list and API get.

pohly · 2025-08-28T13:28:38Z

/test pull-kubernetes-e2e-kind

michaelasp · 2025-10-01T18:37:23Z

/retest
The fix was merged after the trigger

It's a nested map which looks a lot nicer as YAML, in particular when it represents a Kubernetes object. Unit+integration tests using ktesting+gomega and E2E tests benefit from this change.

That was the original intent, but the implementation then ended up checking ResourceClaims in all namespaces. Depending on timing this was merely misleading (showing ResourceClaim changes from a different test running in parallel), but with upcoming CRUD tests which intentionally set an allocation result without a finalizer it breaks the non-CRUD tests when they check the those CRUD ResourceClaims.

pohly · 2025-10-02T14:08:17Z

I had to rebase to get access to apimachineryutils.HaveValidResourceVersion. https://github.com/kubernetes/kubernetes/compare/839045e442ff485ce395d4fb90901d83e8794fc6..2500c4d0c2811b6be8ac9a2ea5f2f43e5b1e33b2 is that rebase with no content changes.

The actual changes are in https://github.com/kubernetes/kubernetes/compare/2500c4d0c2811b6be8ac9a2ea5f2f43e5b1e33b2..8093062d4cc36bd8dc3be4c6e44ef13a2b667ec3:

checking ResourceVersion
simplifying the event sequence check
unstructured.Unstructured rendering as YAML in Gomega output

Small cleanups in https://github.com/kubernetes/kubernetes/compare/8093062d4cc36bd8dc3be4c6e44ef13a2b667ec3..d9df6f82a77e659855720ea6f4be01967e2874f3

test/e2e/framework/conformance/conformance.go

Promoting real tests turned out to be harder than expected (should be rewritten to be self-contained, additional reviews, etc.). They would not achieve 100% endpoint+operation coverage because real tests only use some of the operations. Therefore each API type has to be covered with CRUD-style tests which only exercise the apiserver, then maybe additional functional tests can be added later (depending on time and motivation). The machinery for testing different API types is meant to be reusable, so it gets added in the new e2e/framework/conformance helper package.

pohly · 2025-10-02T16:54:46Z

/test pull-kubernetes-unit

#134386

/test pull-kubernetes-node-e2e-containerd-2-0-dra-alpha-beta-features pull-kubernetes-e2e-gce

Cluster resp. machine didn't come up. I know that Ben wants me to investigate this because someone has to, but I don't have the time right now to learn about that stuff... 😬

liggitt · 2025-10-02T20:25:30Z

I think there are probably opportunities to simplify the framework a little to make it easier to follow (and if we want to do that we should make tweaks before this attracts a bunch of other callers and starts getting promoted into actual conformance), but I'm very happy with the current state of what this is testing. It represents the best practices we know of from other conformance tests (tolerate non-instantanous deletes with finalizers, use robust informer-driven checks of watch event invariants, etc).

/lgtm
/approve

k8s-ci-robot · 2025-10-02T20:25:37Z

LGTM label has been added.

Git tree hash: 0611c95f523a99f462b9118ca4a0127e4d12ca65

BenTheElder

Agree to merge and iterate from here, I think there's room to make this even slightly more guarded against misuse and simplify a bit but overall I think this will be a big improvement over hand-writing this style of test

/lgtm
/approve

BenTheElder · 2025-10-02T20:30:05Z

test/e2e/dra/dra.go

+						},
+					},
+					UpdateSpec: func(obj *resourceapi.DeviceClass) *resourceapi.DeviceClass {
+						obj.Spec.Selectors[0].CEL.Expression = "1 == 0" // Still matches no devices.


heh. cc @jpbetz @cici37

BenTheElder · 2025-10-02T20:32:49Z

test/e2e/framework/conformance/conformance.go

+	//
+	// This should not be enabled in conformance tests because admission
+	// is allowed to modify what is being stored.
+	ContentVerificationEnabled bool


Seems like there's room to validate this struct, e.g. checking that if this bool is set then StrategicMergePatchSpec needs to be set if UpdateSpec is set?

I like what we're doing here overall, but it seems like if you don't copy-paste from another test, it might be easy to use incorrectly, even though it's still easier than writing these from scratch.

k8s-ci-robot · 2025-10-02T20:34:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, liggitt, pohly

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [BenTheElder,liggitt]
~~test/OWNERS~~ [BenTheElder,liggitt,pohly]

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

liggitt · 2025-10-02T21:58:24Z

{Failed  === RUN   TestNamespaceScopedWatch/namespaced_watch,_request_with_name,_request_without_namespace,_field_selector_with_metadata.name_and_spec.nodeName
    utils.go:158: incorrect event:   watch.Event{
        - 	Type: "ADDED",
        + 	Type: "ERROR",
        - 	Object: &example.Pod{
        - 		ObjectMeta: v1.ObjectMeta{Name: "t12-foo1", Namespace: "t12-ns1", ResourceVersion: "50"},
        - 		Spec:       example.PodSpec{NodeName: "t12-bar1"},
        - 	},
        + 	Object: s`&Status{ListMeta:ListMeta{SelfLink:,ResourceVersion:,Continue:,RemainingItemCount:nil,},Status:Failure,Message:Internal error occurred: etcd event received with PrevKv=nil (key="/pods/t12-ns1/t12-foo1", modRevision=49, type=PUT),Reason:InternalError,Detail`...,
          }

fyi @serathius

/retest

aojea · 2025-10-02T22:39:31Z

The integration failure is

#133861

The e2e is :

Kubernetes e2e suite: [It] [sig-node] [DRA] control plane [ConformanceCandidate] supports reusing resources expand_less

aojea · 2025-10-02T22:39:40Z

/retest

liggitt · 2025-10-02T22:49:41Z

The e2e is :

Kubernetes e2e suite: [It] [sig-node] [DRA] control plane [ConformanceCandidate] supports reusing resources expand_less

hmmm, is the failure any conflict with the DRA resources created by this test?

k8s-triage-robot · 2025-10-03T02:07:20Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

pohly · 2025-10-06T15:29:02Z

hmmm, is the failure any conflict with the DRA resources created by this test?

Doesn't seem likely. Each DRA test uses a unique driver name and the objects created by these new tests shouldn't interfere with any concurrently running test. Let's keep an eye open for it though before promoting the new tests.

Speaking of which, that promotion is in #134439, pending for now until the tests have enough soak time.

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Aug 28, 2025

pohly marked this pull request as draft August 28, 2025 09:15

github-project-automation bot added this to SIG Node CI/Test Board Aug 28, 2025

k8s-ci-robot added the wg/device-management Categorizes an issue or PR as relevant to WG Device Management. label Aug 28, 2025

github-project-automation bot moved this to Triage in SIG Node CI/Test Board Aug 28, 2025

k8s-ci-robot removed the do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Aug 28, 2025

github-project-automation bot added this to Dynamic Resource Allocation and SIG Node: code and documentation PRs Aug 28, 2025

github-project-automation bot moved this to 🆕 New in Dynamic Resource Allocation Aug 28, 2025

github-project-automation bot moved this to Triage in SIG Node: code and documentation PRs Aug 28, 2025

k8s-ci-robot requested review from BenTheElder and klueska August 28, 2025 09:16

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 28, 2025

pohly force-pushed the dra-e2e-crud-conformance branch from f28bd30 to eb603fc Compare August 28, 2025 13:27

dims mentioned this pull request Aug 28, 2025

[DRA] Conformance Endpoints Promoted Without Tests and Untested Conformance Endpoints for 1.34.0 (34 operations) #133691

Open

pohly force-pushed the dra-e2e-crud-conformance branch 2 times, most recently from 2500c4d to 8093062 Compare October 2, 2025 14:00

pohly added 2 commits October 2, 2025 16:07

test: format unstructured as YAML

2ac9ff1

It's a nested map which looks a lot nicer as YAML, in particular when it represents a Kubernetes object. Unit+integration tests using ktesting+gomega and E2E tests benefit from this change.

pohly force-pushed the dra-e2e-crud-conformance branch from 8093062 to d9df6f8 Compare October 2, 2025 14:07

liggitt reviewed Oct 2, 2025

View reviewed changes

test/e2e/framework/conformance/conformance.go Outdated Show resolved Hide resolved

test/e2e/framework/conformance/conformance.go Show resolved Hide resolved

test/e2e/framework/conformance/conformance.go Outdated Show resolved Hide resolved

liggitt reviewed Oct 2, 2025

View reviewed changes

test/e2e/framework/conformance/conformance.go Outdated Show resolved Hide resolved

pohly force-pushed the dra-e2e-crud-conformance branch from d9df6f8 to f95d531 Compare October 2, 2025 15:43

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 2, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 2, 2025

BenTheElder reviewed Oct 2, 2025

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 2, 2025

k8s-ci-robot merged commit 0dd78f6 into kubernetes:master Oct 3, 2025
23 checks passed

k8s-ci-robot added this to the v1.35 milestone Oct 3, 2025

github-project-automation bot moved this from Triage to Done in SIG Node: code and documentation PRs Oct 3, 2025

github-project-automation bot moved this from PRs - Needs Reviewer to Done in SIG Node CI/Test Board Oct 3, 2025

pohly moved this from 👀 In review to ✅ Done in Dynamic Resource Allocation Oct 4, 2025

DRA: CRUD conformance tests #133747

DRA: CRUD conformance tests #133747

Uh oh!

Conversation

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

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

k8s-ci-robot commented Aug 28, 2025

Uh oh!

pohly commented Aug 28, 2025

Uh oh!

pohly commented Aug 28, 2025

Uh oh!

pohly commented Aug 28, 2025

Uh oh!

michaelasp commented Oct 1, 2025

Uh oh!

pohly commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pohly commented Oct 2, 2025

Uh oh!

liggitt commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 2, 2025

Uh oh!

BenTheElder left a comment

Choose a reason for hiding this comment

Uh oh!

BenTheElder Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

BenTheElder Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Oct 2, 2025

Uh oh!

liggitt commented Oct 2, 2025

Uh oh!

aojea commented Oct 2, 2025

Uh oh!

aojea commented Oct 2, 2025

Uh oh!

liggitt commented Oct 2, 2025

Uh oh!

k8s-triage-robot commented Oct 3, 2025

Uh oh!

Uh oh!

pohly commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

pohly commented Aug 28, 2025 •

edited

Loading

pohly commented Oct 2, 2025 •

edited

Loading

liggitt commented Oct 2, 2025 •

edited

Loading