DRA: structured parameters #123516

pohly · 2024-02-26T15:18:52Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

"Structured parameters" are a new approach for handling scheduling and autoscaling with dynamic resource allocation: drivers publish information in a format defined and understood by Kubernetes, the scheduler handles the claim allocation without communicating with some DRA driver controller. The Cluster Autoscaler can use the information to simulate scale up and down.

Which issue(s) this PR fixes:

Related-to: kubernetes/enhancements#4381

Special notes for your reviewer:

To keep the PR manageable, the new functionality gets introduced in stages with one commit building on top of the previous:

in-tree API types without any specific structured parameter model
the "named resources" structured parameter model: this commit can serve as reference for the future where a new model needs to be added
node authorizer + admission
scheduler
scheduler_perf integration test
kubelet

Does this PR introduce a user-facing change?

Dynamic Resource Allocation: DRA drivers may now use "structured parameters" to let the scheduler handle claim allocation.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]:  https://github.com/kubernetes/enhancements/issues/4381

k8s-ci-robot · 2024-02-26T15:18:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

This adds support for semantic version comparison to the CEL support in the "named resources" structured parameter model. For example, it can be used to check that an instance supports a certain API level. To minimize the risk, the new "semver" type is only defined in the CEL environment for DRA expressions, not in the base library. See kubernetes#123664 for a PR which adds it to the base library. Validation of semver strings is done with the regular expression from semver.org. The actual evaluation at runtime then uses semver/v4.

While currently those objects only get published by the kubelet for node-local resources, this could change once we also support network-attached resources. Dropping the "Node" prefix enables such a future extension. The NodeName in ResourceSlice and StructuredResourceHandle then becomes optional. The kubelet still needs to provide one and it must match its own node name, otherwise it doesn't have permission to access ResourceSlice objects.

This should better run with multiple nodes, it's more realistic that way.

Storing a modified claim with allocation and the original resource version in the assume cache was not reliable: if an update was received, it replaced the modified claim and the resource that was reserved for the claim might have been used for some other claim. To fix this, the in-flight claims are now stored in the map instead of just a boolean and the status stored there overrides whatever is in the assume cache. Logging got extended to diagnose this problem better. It started to occur in E2E tests after splitting the claim update so that first the finalizer is set and then the status, because setting the finalizer triggered an update.

This finishes the shuffling around of test scenarios so that all of them which make sense with structured parameters are also executed with those.

There are two approaches for making new versioned CEL features available in the release where they get introduced: - Always use the environment for "StoredExpressions". - Use an older version (typically 1.0) and only bump it up later. The second approach was used before, so this is now also done here.

thockin · 2024-03-07T21:37:40Z

I'll approve for API, who hold the kubelet LGTM?

/approve

k8s-ci-robot · 2024-03-07T21:46:24Z

@pohly: The following tests 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-capz-windows-master	`8ea1a9a`	link	false	`/test pull-kubernetes-e2e-capz-windows-master`
pull-kubernetes-linter-hints	`6a361e1`	link	false	`/test pull-kubernetes-linter-hints`

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.

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

pohly · 2024-03-07T22:52:24Z

@klueska is okay with merging, see #123516 (comment).

The wording isn't exactly LGTM, but I know that he doesn't want any further changes in this PR as far as kubelet is concerned.

pohly · 2024-03-07T23:04:10Z

@MaryamTavakkoli wrote:

/milestone v1.30

but that wasn't sufficient:

You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Milestone Maintainers Team and have them propose you as an additional delegate for this responsibility.

Can one of the @kubernetes/milestone-maintainers add the milestone?

The exception has been granted.

sanchita-07 · 2024-03-08T00:44:17Z

/milestone v1.30

dims · 2024-03-08T01:44:32Z

i'll cast the lgtm based on @klueska 's comments above and with my sig-node reviewer hat on.

/approve
/lgtm

thanks @pohly

k8s-ci-robot · 2024-03-08T01:44:39Z

LGTM label has been added.

Git tree hash: df4d9cba0ea326b3ede6b6bc49a86e4fdcc62ca0

dims · 2024-03-08T01:44:40Z

/skip

k8s-ci-robot · 2024-03-08T01:45:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, dims, klueska, pohly, soltysh, thockin

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:

~~api/OWNERS~~ [thockin]
~~cmd/kubelet/OWNERS~~ [dims,klueska,thockin]
~~hack/OWNERS~~ [dims,pohly,soltysh,thockin]
~~pkg/apis/OWNERS~~ [thockin]
~~pkg/controller/resourceclaim/OWNERS~~ [dims,klueska,pohly,soltysh,thockin]
~~pkg/generated/openapi/OWNERS~~ [dims,thockin]
~~pkg/kubeapiserver/authorizer/OWNERS~~ [dims,thockin]
~~pkg/kubelet/OWNERS~~ [dims,klueska,thockin]
~~pkg/printers/OWNERS~~ [dims,soltysh,thockin]
~~pkg/registry/OWNERS~~ [dims,thockin]
~~pkg/scheduler/OWNERS~~ [alculquicondor,dims,thockin]
~~plugin/pkg/admission/noderestriction/OWNERS~~ [dims,thockin]
~~plugin/pkg/auth/authorizer/OWNERS~~ [dims,thockin]
~~staging/publishing/OWNERS~~ [dims,thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/client-go/OWNERS~~ [dims,thockin]
~~staging/src/k8s.io/dynamic-resource-allocation/OWNERS~~ [dims,klueska,pohly,thockin]
~~staging/src/k8s.io/kubelet/pkg/apis/dra/OWNERS~~ [dims,klueska,pohly,thockin]
~~test/e2e/dra/OWNERS~~ [dims,klueska,pohly,soltysh,thockin]
~~test/e2e_node/OWNERS~~ [dims,klueska,pohly,soltysh,thockin]
~~test/integration/etcd/OWNERS~~ [dims,pohly,soltysh,thockin]
~~test/integration/scheduler_perf/OWNERS~~ [alculquicondor,dims,pohly,soltysh,thockin]

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

mrunalp · 2024-03-14T00:58:39Z

api/discovery/aggregated_v2beta1.json

+              "resource": "resourceslices",
+              "responseKind": {
+                "group": "",
+                "kind": "ResourceSlice",


Can we update the KEP to capture the name change and other deviations?

Yes, absolutely.

See kubernetes/enhancements#4561

k8s-ci-robot requested review from apelisse and ardaguclu February 26, 2024 15:19

pohly force-pushed the dra-structured-parameters branch from 16b2228 to 76c97b5 Compare February 26, 2024 18:02

pohly mentioned this pull request Feb 26, 2024

DRA: structured parameters kubernetes/enhancements#4381

Open

4 tasks

pohly changed the title ~~DRA: structured parameters~~ WIP: DRA: structured parameters Feb 26, 2024

pohly mentioned this pull request Feb 26, 2024

dra e2e: fix stack unwinding in helper function #123521

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2024

bart0sh added this to WIP in SIG Node PR Triage Feb 27, 2024

pohly added 7 commits March 7, 2024 22:22

dra e2e: run more test scenarios with structured parameters

234dc1f

dra e2e: move ResourceSlice test

2c6246c

This should better run with multiple nodes, it's more realistic that way.

dra e2e: enable more tests for usage with structured parameters

7f5566a

This finishes the shuffling around of test scenarios so that all of them which make sense with structured parameters are also executed with those.

pohly force-pushed the dra-structured-parameters branch from 9b453d1 to 6a361e1 Compare March 7, 2024 21:27

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

k8s-ci-robot added this to the v1.30 milestone Mar 8, 2024

k8s-ci-robot assigned dims Mar 8, 2024

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

k8s-ci-robot merged commit 7ea3d02 into kubernetes:master Mar 8, 2024
19 checks passed

SIG Node CI/Test Board automation moved this from PRs - Needs Approver to Done Mar 8, 2024

SIG Node PR Triage automation moved this from WIP to Done Mar 8, 2024

pohly mentioned this pull request Mar 11, 2024

DRA: integration with cluster autoscaler #118612

Open

mrunalp reviewed Mar 14, 2024

View reviewed changes

pohly mentioned this pull request Mar 14, 2024

DRA: test for structured parameters #123938

Merged

pohly mentioned this pull request Mar 27, 2024

DRA: update structured parameters KEP to match API in 1.30 kubernetes/enhancements#4561

Merged

cici37 added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Apr 11, 2024

k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Apr 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRA: structured parameters #123516

DRA: structured parameters #123516

pohly commented Feb 26, 2024 •

edited

k8s-ci-robot commented Feb 26, 2024

thockin commented Mar 7, 2024

k8s-ci-robot commented Mar 7, 2024 •

edited

pohly commented Mar 7, 2024

pohly commented Mar 7, 2024

sanchita-07 commented Mar 8, 2024

dims commented Mar 8, 2024

k8s-ci-robot commented Mar 8, 2024

dims commented Mar 8, 2024

k8s-ci-robot commented Mar 8, 2024

mrunalp Mar 14, 2024

pohly Mar 14, 2024

pohly Mar 27, 2024

DRA: structured parameters #123516

DRA: structured parameters #123516

Conversation

pohly commented Feb 26, 2024 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Feb 26, 2024

thockin commented Mar 7, 2024

k8s-ci-robot commented Mar 7, 2024 • edited

pohly commented Mar 7, 2024

pohly commented Mar 7, 2024

sanchita-07 commented Mar 8, 2024

dims commented Mar 8, 2024

k8s-ci-robot commented Mar 8, 2024

dims commented Mar 8, 2024

k8s-ci-robot commented Mar 8, 2024

mrunalp Mar 14, 2024

Choose a reason for hiding this comment

pohly Mar 14, 2024

Choose a reason for hiding this comment

pohly Mar 27, 2024

Choose a reason for hiding this comment

pohly commented Feb 26, 2024 •

edited

k8s-ci-robot commented Mar 7, 2024 •

edited