DRA driver skeleton - #788
Conversation
8abfa6f to
70f2a87
Compare
b0d6ae8 to
5cea989
Compare
Add the kubelet-facing side of a DRA driver: registration, the gRPC lifecycle, and publishing a single empty ResourceSlice so the driver is visible in the cluster. Claims are answered with empty results for now, under a lock provided by the owner, so kubelet requests serialize against NRI ones. The helper reports two very different things through HandleError: ResourceSlice publications it will retry, and its gRPC servers giving up. Tell the two apart with kubeletplugin.ErrRecoverable, log the recoverable ones, and ask the owner to shut down on the rest. Otherwise the driver stays registered with kubelet without being able to serve it, and nothing we could do here would bring it back. Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
Add a DRA section to the common resource manager configuration, with a single option for turning the DRA driver on. It defaults to off, so existing deployments register no driver and publish no ResourceSlices. Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
Create and start the kubelet plugin from the resource manager as its owner: the plugin takes the resource manager's lock, so kubelet requests serialize against NRI ones, and asks it to shut down when it runs into an error it cannot recover from. Enabling or disabling DRA needs a restart. Add the RBAC and host mounts the driver needs to the helm charts. Shutting down on request stops the config agent, which unwinds Start() the same way a SIGTERM does. Route the signal handler through the same call, so both paths shut down identically. Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
5cea989 to
9e17251
Compare
|
@klihub Rebased on top of fresh main. Updated HandleError to properly process unrecoverable driver errors. PTAL. |
| // the active policy provides. A policy which provides no devices leaves a | ||
| // registered driver with no devices, which claims cannot be made against. | ||
| // +optional | ||
| Enabled bool `json:"enabled,omitempty"` |
There was a problem hiding this comment.
@bart0sh, @klihub, what do you think, would it enable being more user-friendly in the next DRA steps, if DRAConfig.Enabled would be *bool instead of bool?
I'm considering if we could avoid a small piece of user-facing configuration complexity (as dependencies between configuration options), and pay the price in the complexity of policies. That is, if a user has given a policy configuration that clearly states "I want to publish _____ as a DRA resource", then the policy could enable DRA, unless DRAConfig.Enabled has been explicitly set to false. With *bool, DRAConfig.Enabled == nil could be interpreted as "false unless needed by some other option". I don't know yet about the topology-aware policy, but I'm absolutely sure that at least the balloons policy will have per-balloonType DRA parameters...
On one hand, this may sound and be a little thing in the configuration, but on the other hand, I see that they are piling up. For instance, if a balloons user configures "preferCloseToDevices: podresourceapi: vendor.com/gpu", it is obvious that the user wants her container near a GPU. However, balloons requires that user configures the main switch for kubelet pod resources API (agent.podResourceAPI: true), too, otherwise her preferCloseToDevices will be just wishful thinking.
That said, I'm fine merging this PR as is, or with simple bool -> *bool change with no other changes, because building a way for a policy to either modify common configuration, or express wishes/new defaults on it, is far from the purpose of this and follow-up PRs. But I'm hoping we'd be able to keep user configuration complexity minimal, as (especially with balloons) it's already pretty complex.
There was a problem hiding this comment.
@askervin Good point, thanks! Enabled is now a *bool (see the latest commit in this PR).
There was a problem hiding this comment.
@bart0sh, @klihub, what do you think, would it enable being more user-friendly in the next DRA steps, if DRAConfig.Enabled would be *bool instead of bool?
Yes. I think unless there is a pressing reason, it is better to idiomatically always go for a pointer instead of a direct value/scalar in these cases. That allows us to change default behavior later, without using kubebuilder tags.
Unset means off, but stays distinguishable from an explicit false, so that a policy whose own configuration asks for DRA devices can later turn the driver on without overriding a user who has turned it off. Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
220da24 to
fbdc752
Compare
3 participants
Add a DRA driver skeleton to the resource manager
This PR adds DRA driver - registration,
ResourceSlicepublishing and the two claim entry points — and wires it into the resource manager behind adra.enabledconfiguration option that defaults to off. No policy publishes devices yet, so with this PR alone an enabled driver registers and publishes an emptyResourceSlice. Policies get to say what they provide in the PRs that follow.Why the driver belongs to the resource manager
The driver is node-scoped infrastructure: one plugin registration, one socket, one
ResourceSlicepublisher per node, all of which outlive any particular allocation decision. Device semantics — what a device is, which ones exist,what allocating one means — belong to the active policy. So
pkg/resmgr/draknows how to be a DRA driver and nothing about CPUs, memory or cache; it never inspects a device's attributes. The package comment states that boundary, and the tests assert it.What is here
pkg/resmgr/dra— the plugin:New,Start, an idempotentStop, andPrepareResourceClaims/UnprepareResourceClaims. Built onk8s.io/dynamic-resource-allocation/kubeletplugin.dra.enabledin the common resource manager configuration, exposed by all three policy CRDs. "Common" here means the resource manager reads the switch from one place because it owns the driver — each policy CRD still carries its ownspec.dra.enabled, so DRA is enabled per policy (and per node group, via the usual config CR layering), not cluster-wide.setupDRA/startDRAfromstart(),Stop()from shutdown.resource.k8s.io/resourceslicesRBAC and the kubelet plugin and plugin-registry host mounts, in all three charts, gated onconfig.dra.enabledso a disabled deployment renders exactly as before.Details worth a reviewer's attention
The driver is named after the active policy —
topology-aware.nri.io. The published devices are the policy's, so a node running a different policy publishes devices of a different kind, and aDeviceClassselecting ondevice.driverstays specific without further qualification. The cost is that switching a node's policy orphans the old driver'sResourceSlicesuntil the node object goes away.The plugin takes the resource manager's lock directly.
dra.Optionshas async.Locker, and the resource manager passes itself: it embedssync.RWMutexalready. Every kubelet request takes that lock, which serializes claim handling against NRI request handling.Stop()stops the plugin before taking the lock.Helper.Stop()drains the gRPC handlers in flight, and a handler arriving mid-drain blocks on the lock, so stopping the plugin under the lock deadlocks. There is a test for the ordering,not just a comment: it starts a real plugin against a fake clientset, holds the lock to stand in for a request being served, and waits for the plugin's socket to disappear.
Turning DRA on or off at runtime is refused, with a message asking for a restart. Both directions have to happen with the lock released, and reconfiguration holds it.
Missing cluster access degrades rather than fails. A plugin configured from a local file has no kubernetes client and no node name; enabling DRA there logs a warning and leaves DRA off instead of refusing to start.
Testing
go test ./...passes. Unit tests cover the plugin (registration, double start, idempotent stop, both claim entry points, and the boundary property that device attributes are never read), the configuration option across all three policy types, and the resource manager wiring including the shutdown ordering.helm templateandhelm lintverified on all three charts with DRA both enabled and disabled.An end-to-end test arrives with the policy-side implementation, and will skip cleanly on clusters without the required feature gates.