DRA: Make GetPCIeRootAttributeByPCIBusID filesystem-independent by ffromani · Pull Request #137220 · kubernetes/kubernetes

ffromani · 2026-02-24T13:27:20Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Make GetPCIeRootAttributeByPCIBusID filesystem-independent. The code now accepts an optional fs.ReadLinkFS, so it doesn't depend on the host FS anymore.
The change is backward compatible.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

Slack thread: https://kubernetes.slack.com/archives/C0409NGC1TK/p1771854023048309

Does this PR introduce a user-facing change?

The GetPCIeRootAttributeByPCIBusID helper now accepts a `fs.ReadLinkFS` optional argument to be filesystem-independenent

WIP Signed-off-by: Francesco Romani <fromani@redhat.com>

mechanical change, no intended change in behavior. Signed-off-by: Francesco Romani <fromani@redhat.com>

k8s-ci-robot · 2026-02-24T13:27:28Z

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.

Details

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.

ffromani · 2026-02-24T13:27:36Z

/sig node
/wg device-management

ffromani · 2026-02-24T13:28:06Z

/cc @everpeace

ffromani · 2026-02-24T13:29:12Z

/hold

The change is reviewable, but I want to give it a go with real DRA driver code.

everpeace

@ffromani
Thank you for the improvement!! LGTM overall, but only a few nits comments. PTAL 🙇

everpeace · 2026-02-24T14:10:55Z

 //
 // ref: https://wiki.xenproject.org/wiki/Bus:Device.Function_(BDF)_Notation
-func GetPCIeRootAttributeByPCIBusID(pciBusID string) (DeviceAttribute, error) {
+func GetPCIeRootAttributeByPCIBusID(pciBusID string, mods ...MachineModifier) (DeviceAttribute, error) {


If we'd like to make this function os-agnostic, should we also define this method in os agnostic file (e.g. pci.go)??

This is an excellent point but we're not there yet I believe. We are improving but still the function IMO still make implicit assumptions about the linux sysfs layout, hence is not really yet independent.
That said, if other reviewers agree with a move, I can move into a generic file

ffromani · 2026-02-26T10:27:08Z

/hold cancel

the tests in this PR and @byako 's tests both validate the sysfs templacement using tempfiles. In the CPU DRA Driver i'm playing with testing/fstest.MapFS and while the final PR is not yet ready, it's sufficient to validate the API here.

Make the internal `sysfs` abstraction public and overridable, in order to make the code easier to test in 3rd party DRA drivers. In order to enable overriding, we move from a custom mock to the standard `fs.FS` (and related) interface. We extend the `GetPCIeRootAttributeByPCIBusID` API with functional options to enable the override while being backward compatible. Finally, the tests make sure to set relative symlinks, mimicing more closely what the kernel does. Signed-off-by: Francesco Romani <fromani@redhat.com>

pohly · 2026-03-02T08:48:16Z

/assign @everpeace

everpeace

Thanks for the improvement!!

/lgtm

everpeace · 2026-03-02T09:56:41Z

+			// per docs, the testing package ensures cleanup, so no need to do that ourselves
+			testMachinePath := t.TempDir()


k8s-ci-robot · 2026-03-02T09:59:12Z

LGTM label has been added.

Details

Git tree hash: 1111e42e07e64c128857355912e5df72d5529154

ffromani · 2026-03-02T10:10:40Z

Thanks for the improvement!!

thank you for creating this helper and for the reviews!

pohly · 2026-03-02T10:50:56Z

/approve

k8s-ci-robot · 2026-03-02T10:51:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ffromani, pohly

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:

~~staging/src/k8s.io/dynamic-resource-allocation/OWNERS~~ [pohly]

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

k8s-ci-robot · 2026-03-02T11:41:21Z

@ffromani: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-e2e-gce	`efd6f8b`	link	unknown	`/test pull-kubernetes-e2e-gce`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

ffromani · 2026-03-02T11:50:19Z

/retest

Import code at k/k@b96a4039358 We need the code merged in kubernetes/kubernetes#137220 but we can't wait to rebase on top of kube 1.36.0. We will drop this carryover and just depend on the kube libs when we actually rebase. Signed-off-by: Francesco Romani <fromani@redhat.com>

Import code at k/k@b96a4039358 We need the code merged in kubernetes/kubernetes#137220 and kubernetes/kubernetes#137524 but we can't wait to rebase on top of kube 1.36.0. We will drop this carryover and just depend on the kube libs when we actually rebase. Signed-off-by: Francesco Romani <fromani@redhat.com>

Import code at k/k@b96a4039358 We need the code merged in kubernetes/kubernetes#137220 and kubernetes/kubernetes#137524 but we can't wait to rebase on top of kube 1.36.0. We will drop this carryover and just depend on the kube libs when we actually rebase. IMPORT NOTICE: trivial reformatting applied to comply with this project rules. No functional changes performed. Mechanical change only: `gci write ./internal/deviceattribute` Signed-off-by: Francesco Romani <fromani@redhat.com>

ffromani added 2 commits February 24, 2026 11:06

dra: deviceattr: add Clean for safety

8bb22a5

WIP Signed-off-by: Francesco Romani <fromani@redhat.com>

dra: deviceattr: split long expression for readability

ebed4d4

mechanical change, no intended change in behavior. Signed-off-by: Francesco Romani <fromani@redhat.com>

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 24, 2026

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. wg/device-management Categorizes an issue or PR as relevant to WG Device Management. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 24, 2026

github-project-automation Bot added this to Dynamic Resource Allocation and SIG Node: code and documentation PRs Feb 24, 2026

github-project-automation Bot moved this to Triage in SIG Node: code and documentation PRs Feb 24, 2026

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

k8s-ci-robot requested review from bart0sh and pohly February 24, 2026 13:27

k8s-ci-robot requested a review from everpeace February 24, 2026 13:28

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 24, 2026

ffromani changed the title ~~Dra helper sysfs repleaceable~~ Make GetPCIeRootAttributeByPCIBusID filesystem-independent Feb 24, 2026

ffromani changed the title ~~Make GetPCIeRootAttributeByPCIBusID filesystem-independent~~ DRA: Make GetPCIeRootAttributeByPCIBusID filesystem-independent Feb 24, 2026

ffromani force-pushed the dra-helper-sysfs-repleaceable branch from 15826e8 to d87b2ad Compare February 24, 2026 13:59

everpeace reviewed Feb 24, 2026

View reviewed changes

pohly reviewed Feb 24, 2026

View reviewed changes

Comment thread staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go Outdated

Comment thread staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go Outdated

ffromani force-pushed the dra-helper-sysfs-repleaceable branch from d87b2ad to 504acd3 Compare February 24, 2026 14:34

pohly reviewed Feb 26, 2026

View reviewed changes

Comment thread staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go

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

ffromani force-pushed the dra-helper-sysfs-repleaceable branch from 504acd3 to efd6f8b Compare February 26, 2026 11:02

pohly moved this from 🆕 New to 👀 In review in Dynamic Resource Allocation Mar 2, 2026

k8s-ci-robot assigned everpeace Mar 2, 2026

everpeace reviewed Mar 2, 2026

View reviewed changes

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

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

k8s-ci-robot merged commit d6d0c93 into kubernetes:master Mar 2, 2026
20 of 21 checks passed

k8s-ci-robot added this to the v1.36 milestone Mar 2, 2026

github-project-automation Bot moved this from Triage to Done in SIG Node: code and documentation PRs Mar 2, 2026

ffromani deleted the dra-helper-sysfs-repleaceable branch March 2, 2026 14:06

pohly moved this from 👀 In review to ✅ Done in Dynamic Resource Allocation Mar 3, 2026

everpeace mentioned this pull request Mar 7, 2026

split linux and portable deviceattribute unit tests #137524

Merged

		// per docs, the testing package ensures cleanup, so no need to do that ourselves
		testMachinePath := t.TempDir()

Conversation

ffromani commented Feb 24, 2026

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 Feb 24, 2026

Uh oh!

ffromani commented Feb 24, 2026

Uh oh!

ffromani commented Feb 24, 2026

Uh oh!

ffromani commented Feb 24, 2026

Uh oh!

everpeace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

everpeace Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ffromani Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ffromani commented Feb 26, 2026

Uh oh!

pohly commented Mar 2, 2026

Uh oh!

everpeace left a comment

Choose a reason for hiding this comment

Uh oh!

everpeace Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 2, 2026

Uh oh!

ffromani commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pohly commented Mar 2, 2026

Uh oh!

k8s-ci-robot commented Mar 2, 2026

Uh oh!

k8s-ci-robot commented Mar 2, 2026

Uh oh!

ffromani commented Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

everpeace Feb 24, 2026 •

edited

Loading

ffromani commented Mar 2, 2026 •

edited

Loading