Conversation
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. 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>
8abfa6f to
70f2a87
Compare
| // DRAConfig provides configuration data for the DRA (Dynamic Resource | ||
| // Allocation) driver. The driver is owned by the resource manager, not by a | ||
| // policy, so this is common configuration for all policies. | ||
| type DRAConfig struct { |
There was a problem hiding this comment.
nit: I wonder if this should go under pkg/apis/config/v1alpha1/resmgr/dra (mirroring where the driver skeleton itself lives which IIUC this is the configuration for), for consistency with our other stuff. I think this might have been modelled after AgentConfig, which I suspect is one of the existing inconsistencies. Not entirely sure though ATM, and not insisting on this at all. Just something that caught my eye so I thought I should mention it.
There was a problem hiding this comment.
Okay, sorry. I think I was clearly wrong. That's the DRA skeleton, but this configures stuff in pkg/resmgr/dra.go. This is in the right place then I think.
Create and start the kubelet plugin from the resource manager, passing it the resource manager's lock so kubelet requests serialize against NRI ones. Enabling or disabling DRA needs a restart. Add the RBAC and host mounts the driver needs to the helm charts. Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
| @@ -223,6 +233,10 @@ func (m *resmgr) start(cfg cfgapi.ResmgrConfig) error { | |||
| func (m *resmgr) Stop() { | |||
There was a problem hiding this comment.
@klihub This function is not called anywhere as far as I can see. It's not related to this PR, but we may want to either remove it if it's not needed or call it when resource manager is stopped.
There was a problem hiding this comment.
@bart0sh Looking at how fairly plain it is, let's just get rid of it, if it is unused.
There was a problem hiding this comment.
@bart0sh, I ran into that, too, when looking for a spot for clearing published extended resources.
Discussion about such clean-ups finally produced this open issue:
but based on @marquiz's advice, I finally gave up whole idea of trying to clear published resources on highly unreliable plugin exit handlers. Instead of following plugin pod/container life cycle, extended resource (un)publishing follows reconciliation model: if the desired state expressed in most recent configuration is "don't publish", and if the plugin notices resources being published anyway, then it clears them.
There was a problem hiding this comment.
@askervin Having some cleanup capabilities is not mandatory, but it's not a bad thing either. Can we simply catch SIGTERM or/and other signals and call resmgr.Stop in the signal handler? Did you try this approach?
Anyway, this is not part of DRA, so I'll leave this code here. If we decide to remove it, it would be better to do it in another PR.
70f2a87 to
5cea989
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.