Conversation
Introduces docs/dra/pct-design.md — the first draft of a design for exposing nri-plugins Priority Core Turbo (Intel Speed Select SST-CP + SST-TF) capacity as a Kubernetes Dynamic Resource Allocation resource. Scope is intentionally narrow at this stage: PCT priority only, topology-aware policy only. The design premise is that kube-scheduler needs visibility into per-punit HP-CPU headroom to avoid bin-packing HP pods onto SST-TF-ineligible nodes where they would fall back silently to lower-bucket turbo. Assumed KEP baseline: KEP-5075 required, KEP-5517 required, KEP-5941 optional stretch. The core proposal is Model B — one DRA device per SST-TF punit with hp-cpus and lp-cpus consumable capacities — with an opt-in path to Model C (per-CPU children under a shared counter) when KEP-5941 lands. Draft state: carries four open decisions (cpuClass scope, KEP-5941 target level, config-key naming, driver name) that later commits resolve while broadening scope beyond PCT.
CLAUDE.md is auto-loaded into every session, so it should stay short
and repo-specific. Move accumulated DRA landscape / KEP details / PR
docs/dra/{landscape,pr-536-analysis,design,plan}.md, keeping only a
pointer list in CLAUDE.md. Also add the PCT allocator to the "primary
areas of interest" list.
Reference notes for anyone doing DRA-related work in this repo: KEP-5517 (Node Allocatable Resources) API and behavior; a map of related DRA KEPs (consumable capacity, shared consumable capacity, node allocatable, list-type attributes, standard numaNode, etc.); architectural summaries of the SIG-node reference implementations (dra-driver-cpu and dra-example-driver) with the specific patterns we borrow vs. those we do not; and the current A/B/C design options being weighed for nri-plugins DRA integration. Not auto-loaded; CLAUDE.md points at it when a task touches DRA. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures what klihub's DRA-driver prototype (containers#536) does and how, so future DRA work in this repo starts from an informed baseline. Covers the CDI + env-var bridge (DRA_CPU<N>=1 injected into containers, parsed at NRI CreateContainer) that remains a valid mechanism; the prototype scars that KEP-5517 supersedes (getClaimedCPUs env-var-parsing for accounting, mirror-in-spec.cpu workaround); and a table mapping which pieces to reuse vs. drop when building on this. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renamed pct-design.md -> design.md and rewrote to reflect a series of design decisions made in review: - Scope broadened from "PCT priority only" to "full cpuClass surface" (EPP, freqGovernor, uncore min/max, disabledCstates, PCT priority, SST CLOS binding). PCT-tier admission-time enforcement remains the concrete v1 value proposition; other cpuClass fields become device attributes. - v1 targets the topology-aware policy only. Balloons is a v2 target with a separate driver (nri.balloons.cpu) sharing implementation code with topology-aware (nri.topology-aware.cpu). - Device model: per-(cpuClass x punit) devices with consumable capacity (KEP-5075) and NodeAllocatableResources.mapping (KEP-5517). Model C (KEP-5941 shared counters) supported as opt-in when the upstream KEP lands. - Resolved: multi-class-per-tier overcommit via cpuClass.dra.publish (default true) and refuse-to-start Configure-time validation. - Resolved: per-policy dra config block under each policy CR. - Resolved: driver naming per policy; v2 code-sharing verification checklist. - Documented as open: class-derived attribute freshness (options A-F, recommendation F). Reference-driver context added: neither dra-driver-cpu nor dra-example-driver supports live reconfigure; restart-the-DaemonSet is their workaround, not open to nri-plugins. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ten-step landable-PR plan for the topology-aware DRA integration described in design.md, ordered so the first five steps need no live cluster (unit-testable), and the last three (wire-up, Helm, e2e) are the cluster-dependent tail. Each step is scoped to be independently reviewable with a clear rationale, files touched, verification approach, and risk assessment. Step 6 includes a detailed Imports & deps subsection tiering the Kubernetes helper packages needed to build a DRA driver (must-have, should-use, not-adopted, deferred to v2) so the imports story is not re-litigated at PR review time. Cross-cutting concerns (feature-gate probing, testing philosophy) and explicit "not part of v1" carve-outs (balloons, Model C, class-freshness resolution, controller/webhook binaries) close out the doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Convert bare identifiers and paths across CLAUDE.md and docs/dra/*.md to markdown links so all cross-references render as clickable on GitHub: - KEP-NNNN mentions link to their kubernetes/enhancements issue - PR containers#536 mentions link to containers#536 - Cross-doc references (design.md, landscape.md, pr-536-analysis.md, plan.md) use markdown links - Repo-internal directories in CLAUDE.md link to the repo tree paths - External-repo file references (dra-driver-cpu, dra-example-driver, kubernetes/kubernetes) link to specific blob/tree URLs; file:line references use #Lnnn anchors Also fixes two stale filename references (pct-design.md -> design.md) missed during the earlier rename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
That directory does not exist. Topology-aware policy code lives at cmd/plugins/topology-aware/policy/, alongside the plugin entry point. Reworded the bullet to point at the actual policy directory instead of a non-existent one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the four-bullet "Takeaway for future DRA work" section with a three-tier reuse breakdown that names concrete commits and files: - Tier 1 — direct cherry-pick: six PR containers#536 commits that are pure refactors or additive utilities, orthogonal to the DRA design. - Tier 2 — salvage-and-adapt: named files/functions where the mechanism is correct but the surrounding shape (package layout, env-var naming, driver name) changes. - Tier 3 — learn-from-only: files whose semantics moved under the design changes (device schema, per-(class x punit) accounting, obsolete user-facing workarounds). Motivation: the previous binary "reusable vs obsolete" framing understated the middle tier. Future implementers now have a concrete commit-and-file map instead of an abstract "reusable pieces" hint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both reference drivers (dra-driver-cpu, dra-example-driver) use the upstream CDI Go library rather than hand-rolling YAML. PR containers#536 hand- rolled its writer, but that carries no atomic-write guarantee, no spec validation, and no version tracking. Not a dep in nri-plugins today. - plan.md step 7: CDI writer bullet points at the library instead of reusing PR containers#536's shape. Names the specific API surface (cdiapi.Cache.WriteSpec, RemoveSpec; specs-go types; GenerateTransientSpecName for per-claim filenames) and points at dra-driver-cpu's cdi.go as the pattern source. - plan.md step 6 Imports & deps: adds the three library packages (pkg/cdi, specs-go, pkg/parser) to the Must-have tier. - pr-536-analysis.md: moves the CDI writer bullet from Tier 2 (salvage-and-adapt) to Tier 3 (learn-from-only), with a note explaining what to import instead. - landscape.md: one-line entry under Other reference code linking to cncf-tags/container-device-interface with a sentence on why drivers should import it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Detailed TDD-ordered plan for the first landable PR of the DRA integration: lift Kubernetes client bootstrap out of pkg/agent into a new pkg/kubernetes/client package, and move the existing pkg/agent/watch package to pkg/kubernetes/watch verbatim (git mv, history preserved). The plan is derived from three commits on the pr-536-dra branch (5dcb66d, 42ec102, 8814064) — re-derived cleanly with tests added, since PR containers#536 shipped without tests. Deliberately does NOT adopt PR containers#536's parallel pkg/kubernetes/watch ObjectClient redesign; that API-change work is out of scope for a "pure refactor" step 1. Structure: 9 tasks, each with Files: blocks and tests-first checkboxes. Task 1-2 establish client package + options + methods. Task 3 adds content-type options with a two-pass retry-when-config- not-set mechanism (order-independence). Task 4 moves the watch package and adds minimum tests it currently lacks. Tasks 5-6 reserved for numbering stability across review rounds. Task 7 rewires pkg/agent/agent.go to use the new client, removes httpCli field, adds four getters (NodeName, KubeClient, KubeConfig string, RestConfig). Tasks 8-9 are acceptance and doc updates. Plan went through two rounds of plan-review agent auto-review; final verdict "APPROVE with minor polish" — all 15 first-round findings addressed, plus 8 second-round polish items applied. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First task of the DRA v1 step 1 refactor (see docs/plans/20260820- dra-step1-kubernetes-client-watch-lift.md). Introduces the pkg/kubernetes/client package that will replace the inline kube- client bootstrap currently in pkg/agent/agent.go. Scope of this commit: package skeleton only — the Client struct, functional Option type, and the two rest-config helpers (GetConfigForFile, InClusterConfig) wrapping clientcmd. BuildConfigFromFlags and rest.InClusterConfig respectively. Tests first per TDD: four cases cover success, missing file, malformed file, and not-in-cluster (guarded by KUBERNETES_SERVICE_HOST for CI that happens to run inside a Pod). Fixture kubeconfig points at example.com — clientcmd only parses, so no network is required. Follow-up tasks add New() with the option catalog, Client methods (RestConfig deep-copy, HttpClient, K8sClient, Close), and the content-type options with retry-when-config-not-set. Then pkg/agent/agent.go is rewired to use the new package. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second task of the DRA v1 step 1 refactor (see docs/plans/20260820- dra-step1-kubernetes-client-watch-lift.md). Adds the New() constructor with functional options and the Client's method surface: - Options: WithKubeConfig, WithInClusterConfig, WithKubeOrInClusterConfig, WithRestConfig (deep-copies input via rest.CopyConfig), WithHttpClient. - Methods: RestConfig (returns rest.CopyConfig(c.cfg) — top-level and value-struct fields safe to overwrite; nested map/slice contents share storage per upstream convention), HttpClient, K8sClient, Close (idempotent, nil-safe). - errRetryWhenConfigSet sentinel prepared for Task 3's content-type options. - New() falls back to WithInClusterConfig if no option set a config. Tests: 11 new cases covering all option paths (kubeconfig success/ missing, in-cluster fallback, RestConfig acceptance, HttpClient injection), rest.CopyConfig semantics on both directions, and Close idempotency. In-cluster paths guarded by KUBERNETES_SERVICE_HOST so tests skip cleanly when the CI happens to run inside a Pod. Plan updated in place: earlier promise of "safe to mutate at any depth" was aspirational — rest.CopyConfig shares nested map/slice storage (Impersonate.Extra, TLSClientConfig.CAData, etc.). Matched to upstream contract and to what dra-driver-cpu and PR containers#536's WithRestConfig side expect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Third task of the DRA v1 step 1 refactor (see docs/plans/20260820- dra-step1-kubernetes-client-watch-lift.md). Adds the two content-type options and their constants, exercising the retry-when-config-not-set mechanism already prepared in Task 2's New() implementation. Additions: - ContentTypeJSON and ContentTypeProtobuf constants. - WithAcceptContentTypes(...string) — joins values with commas, sets cfg.AcceptContentTypes. - WithContentType(string) — sets cfg.ContentType. - Both return errRetryWhenConfigSet when c.cfg is nil so New()'s two-pass logic re-applies them after a config-source option runs. Tests (5 new cases): - Basic content-type application (assert config field is set as expected). - Order-independence: config-source-first vs content-type-first yields identical clients — verifies the retry mechanism. - Content-type-only-no-config-source: falls back to WithInClusterConfig which errors outside a cluster; guarded by KUBERNETES_SERVICE_HOST. - Multiple retries: two WithAcceptContentTypes deferred, retry list ordering preserved (last write wins within the retry list). Semantics note documented in the multi-retry test: retry-list options apply AFTER all non-retry options, so callers should not mix retry- required and non-retry setters for the same field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fourth task of the DRA v1 step 1 refactor (see docs/plans/20260820-
dra-step1-kubernetes-client-watch-lift.md). Relocates the watch
package to make it available to future consumers outside the agent
(the DRA driver in later plan steps).
Changes:
- git mv pkg/agent/watch/{watch,object,file}.go
pkg/kubernetes/watch/ — preserves history via git rename detection.
- No API change: Object(ctx, ns, name, CreateFn) and File(path,
UnmarshalFn) signatures unchanged; ObjectWatch.Stop(),
FileWatch.Stop(), and the type-alias/const surface (Interface,
Event, EventType, Added/Modified/Deleted/Bookmark/Error) unchanged.
- Logger tag changed from logger.Get("agent") to logger.Get("watch")
so operator log-grep matches the new package location.
- Import path in pkg/agent/agent.go:36 updated from
pkg/agent/watch to pkg/kubernetes/watch. Plan-deviation note:
this update was scheduled for Task 7 but is included here to keep
the tree compiling between per-task commits. Task 7 no longer needs
to touch that import.
Tests (new, since the moved package shipped without any):
- Type-alias assertions: compile-time verification that Interface,
EventType, Event are true aliases of k8s.io/apimachinery/pkg/watch
types.
- Event-type constant equality: each re-exported const equals its
upstream value.
- Object watch happy path: events pushed through the fake Interface
from a CreateFn arrive on ResultChan in order.
- Object.Stop() idempotency: two calls do not panic.
- File watch Create-vs-Write distinction: defensive guard that Create
fsnotify events emit Added and Write events emit Modified — PR
containers#536's parallel implementation ships a copy-paste bug emitting
Added for both, and this test guards against re-introducing it.
Uses O_WRONLY|O_APPEND for the modify step so we get a pure Write
event, not the O_CREATE|O_TRUNC that os.WriteFile does.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Load-bearing task of the DRA v1 step 1 refactor (see docs/plans/ 20260820-dra-step1-kubernetes-client-watch-lift.md Task 7). Replaces the inline Kubernetes client bootstrap with the new pkg/kubernetes/ client wrapper, and exposes agent state that future consumers (the DRA driver in later plan steps) need. Changes: - Agent.httpCli field removed. All uses replaced with a.k8sCli.HttpClient() (where a.k8sCli is now *client.Client). - Agent.k8sCli field type changed from *k8sclient.Clientset to *client.Client. Existing use at a.k8sCli.CoreV1().Nodes().Watch(...) keeps compiling because *client.Client embeds *kubernetes.Clientset. - setupClients: replaced ~25 lines of inline REST config resolution and Clientset construction with client.New( client.WithKubeOrInClusterConfig(a.kubeConfig)). Passes a.k8sCli.HttpClient() and a.k8sCli.RestConfig() to ConfigInterface.SetKubeClient — the callback signature is unchanged, so downstream consumers are unaffected. - configure() NRT client init: uses a.k8sCli.RestConfig() and a.k8sCli.HttpClient() instead of a.getRESTConfig() + a.httpCli. Guards on a.k8sCli == nil to avoid a nil-panic if configure is invoked before setupClients completes. - cleanupClients: collapsed to a.k8sCli.Close() (nil-safe from the client package) + nil-out of a.k8sCli and a.nrtCli. - getRESTConfig: removed entirely. Its logic now lives inside client.New's option chain (WithKubeOrInClusterConfig). - Removed unused imports "k8s.io/client-go/tools/clientcmd" and k8sclient "k8s.io/client-go/kubernetes". Getters added on *Agent (public surface): - NodeName() string — the kubernetes node name. - KubeClient() *client.Client — the shared client wrapper; nil before setupClients has run successfully. - KubeConfig() string — the kubeconfig file path (or empty for in-cluster). Matches PR containers#536 commit 8814064's signature. - RestConfig() *rest.Config — shortcut for KubeClient().RestConfig(); nil-safe before setupClients. Tests (new pkg/agent/agent_test.go): - Getters return zero values on a freshly-constructed Agent (before setupClients). - Getters return non-nil values once a.k8sCli is populated with a real client.Client built from the fixture kubeconfig (../kubernetes/client/testdata/kubeconfig-example.yaml). Preexisting failure in pkg/sysfs (unrelated: panics in Ginkgo-based cache-discovery and cluster-CPUSet tests) verified via stash-and- retest before applying — not introduced by this change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 1 of docs/dra/plan.md landed as commits 022b876...f595440 on the DRA branch. Added a "Landed:" pointer in plan.md's Step 1 section linking to the archived plan file. Moved docs/plans/20260820-dra-step1-kubernetes-client-watch-lift.md to docs/plans/completed/ per the plan's own Task 9. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plan for pkg/resmgr/dra/ package skeleton — the structural placeholder that enforces resolved decision 6 (no policy-binary imports). Three production files (doc.go, deps.go, plugin.go) plus a test file. Plan went through auto-review; three important items applied: license headers, Deps description consistency (empty struct, no interfaces yet), and TestNoCmdPluginsImport implementation details. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 2 of the DRA integration plan (docs/dra/plan.md). Creates the pkg/resmgr/dra package as a structural placeholder — physically separating the shared DRA kubelet-plugin code from any policy binary and enforcing the code-sharing contract (design.md resolved decision 6). Four files: - doc.go: package comment with the import-boundary contract - deps.go: Deps struct (empty; fields added in Step 6) - plugin.go: Plugin struct + New() constructor returning errNotImplemented - plugin_test.go: two tests — TestNew_ReturnsNotImplemented verifies the sentinel error, TestNoCmdPluginsImport runs go list -deps ./... and fails if any transitive dep is from cmd/ No functional DRA code is introduced in this step. No existing files are modified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prevent go test -c compiled test binaries from being accidentally staged. Caught when pkg/resmgr/dra/dra.test was included in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plan for cpuClass.dra.publish config field + ValidateCPUClassesForDRA. The plan went through two rounds of auto-review; all blockers from pass 1 addressed and two documentation-sync items added in pass 2. Key design decisions captured: - ValidateCPUClassesForDRA(classes, sharedCounters) — no []Punit parameter in v1 since CPUClass carries no punit affinity; tier-only validation is the static maximum. Per-punit deferred to Step 5. - Non-PCT classes exempt from tier check, still published by Step 5. - Assoc-only tiers keyed by SstClosID string label. - CPUClassDRA type stays in config API; validation function goes in pkg/resmgr/cpuclass/dra.go (extends in Step 5). - Deep copy via make generate, four CRD YAML files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 3 of the DRA integration plan (docs/dra/plan.md).
Config API (pkg/apis/config/v1alpha1/resmgr/policy/cpuclass.go):
- Add CPUClassDRA struct (Publish *bool, +k8s:deepcopy-gen=true)
- Add DRA *CPUClassDRA field to CPUClass with json:"dra,omitempty"
- Add DRAPublish() bool getter (nil DRAConfig -> true, nil Publish ->
true, otherwise *DRA.Publish)
DRA validation (pkg/resmgr/cpuclass/dra.go):
- Add ValidateCPUClassesForDRA(classes, sharedCounters bool) error.
Groups DRA-published PCT classes by tier label ("pctPriority=high",
"pctPriority=low", "closID=<N>"). If any tier has >1 published class
and sharedCounters is false, returns an error naming the tier, the
conflicting classes (sorted), and the two resolutions. Non-PCT
classes are exempt from the check. Called at driver Configure time,
not at config load time.
Regenerated (make generate):
- pkg/apis/config/.../zz_generated.deepcopy.go: DeepCopyInto/DeepCopy
for CPUClassDRA, updated CPUClass.DeepCopyInto for the DRA field
- All four CRD YAML files (config/crd/bases/ + deployment/helm/ copies)
Tests:
- TestCPUClassDRAPublish (table-driven, 4 cases)
- TestCPUClassDRADeepCopy (pointer independence check)
- TestValidateCPUClassesForDRA (table-driven, 13 cases: empty list,
non-PCT exempt, single HP/LP, HP+LP ok, assoc-only same/different
CLOS, two HP both published, sharedCounters bypass, one opted out,
>2 conflicting classes, mixed PCT+non-PCT)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 3 of docs/dra/plan.md landed as commit a0fd7f1. Updates: - plan.md Step 3: update Actions to reflect final implementation (ValidateCPUClassesForDRA in pkg/resmgr/cpuclass/dra.go, no []Punit parameter, tier-only classification, non-PCT exempt, add Landed pointer). - plan.md Step 5: note that dra.go already exists after Step 3 and must be extended; use DRAPublish() getter not DRA.Publish directly. - design.md §"v1 approach": document that validation is tier-only in v1 (CPUClass has no punit affinity), link to ValidateCPUClassesForDRA, explain non-PCT class exemption and per-punit deferral to Step 5. - Move step 3 plan to docs/plans/completed/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkg/sysfs/testdata/, pkg/resmgr/lib/memory/testdata/, and pkg/topology/testdata/ contain mock /sys directory trees used by test suites as fake hardware topology. They are not tracked in upstream (containers/nri-plugins) either. Adding them to .gitignore so they don't show up as untracked files after running the test suite. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plan for pct.Allocator.PickHpCpus / ReleaseHpCpus + capacity helpers. Went through two rounds of auto-review; two sets of blockers resolved: Pass 1: re-keyed from punit index to (pkgID, punitID); separated hpDRAUsed from hpUsed to prevent clearHpUsage clobber; added Punits() []PunitInfo as the exported surface for Step 5. Pass 2: corrected hpReserveCpus description (union into 'used' before MaxHpCpus subtraction, not exclude from 'free'); fixed hpInUseCpus loop to range over punits not hpUsed; dropped PunitInfo.Idx; split Task 2 into 2a (additive) and 2b (hint-machinery changes); added TestHpReserveRoomWithDRAHolds and non-vacuous hpInUseCpus fixture. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 4 of the DRA integration plan (docs/dra/plan.md).
New exported surface on pct.Allocator:
- PunitInfo{PkgID, PunitID, HPCapacity, NonHPCapacity} — snapshot
type returned by Punits(). Step 5 (DRADevices) uses HPCapacity and
NonHPCapacity to set RequestPolicy.max values for DRA devices.
- Punits() []PunitInfo — per-punit capacity snapshot. Returns nil
when the allocator is inactive.
- PickHpCpus(pkgID, punitID, n int, held cpuset.CPUSet) — selects n
HP-eligible CPUs from the named punit. Enforces GuaranteedHpCpus
cap. Writes to hpDRAUsed (not hpUsed), so clearHpUsage called from
the non-DRA UseClass path cannot evict DRA holds.
- ReleaseHpCpus(pkgID, punitID int, cpus cpuset.CPUSet) — idempotent
release; ignores unknown punits and CPUs not in hpDRAUsed.
New Allocator state:
- hpDRAUsed map[int]cpuset.CPUSet — DRA-claimed HP CPUs, index-keyed
like hpUsed. Separate map prevents the non-DRA clearHpUsage path
from evicting DRA holds.
Hint machinery updates:
- hpInUseCpus: loop now ranges over punits (not hpUsed) so DRA-only
punits are not skipped; unions hpUsed[i] and hpDRAUsed[i].
- hpReserveCpus: unions hpUsed[i] and hpDRAUsed[i] before subtracting
from MaxHpCpus, so DRA holds reduce reported HP room and prevent
hint-driven HP over-subscription.
Private helpers (used by Punits()):
- punitIdxByID(pkgID, punitID) — (PkgID, PunitID) → index lookup.
- punitHPCapacity(idx) — GuaranteedHpCpus or 0 if ineligible.
- punitNonHPCapacity(idx) — non-HP allocatable CPUs with
allowed.Size()>0 guard.
Tests (8 new, all in pct_test.go):
TestPunitHPCapacity, TestPunitNonHPCapacity, TestAllocatorPunits,
TestPickHpCpus, TestReleaseHpCpus — cover success, error, edge cases.
TestHpDRAUsedIsolation — non-HP UseClass overlapping a DRA-held CPU
does not evict it from hpDRAUsed; DRA holds visible in hpInUseCpus
on a DRA-only punit (non-vacuous fixture).
TestHpReserveRoomWithDRAHolds — DRA holds reduce hpReserveCpus room.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Landed as commit 5c4c04f. Updated plan.md Step 4 with Landed pointer and the key deviations from the original spec: (pkgID,punitID) keying, hpDRAUsed separate map, PunitInfo/Punits() export for Step 5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plan for cpuclass.Handler.DRADevices() — emits []resapi.Device in
Model B shape per (published class × punit). Went through three rounds
of auto-review; all blockers resolved.
Key decisions captured:
- API field names from k8s.io/api@v0.36.0: NodeAllocatableResourceMappings
+ {CapacityKey, AllocationMultiplier} (not the KEP draft names).
- go.mod bump to v0.36.3 as standalone Task 1.
- buildDRADevices() pure builder + Handler.DRADevices() thin wrapper
for testability without SST/sysfs fakes.
- pct.Allocator.IsHPClass() exported in Task 2.
- Topology ints (nri/packageID, nri/punitID) always emitted even when 0.
- Device name: sanitizeBase(63-suffix-disambig), per-class base
pre-computed; dedup seen-map keyed on className not base.
- Frequency attrs and numaNode explicitly deferred with rationale.
- Non-SST-TF (no punits) and non-PCT class overlap as known v1 limits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plan for the e2e test proving the full DRA claim path for the topology-aware policy (ResourceClaim -> CDI -> CLOS association), including the harness provisioning change needed to enable the DRAConsumableCapacity/DRANodeAllocatableResources alpha gates, since the e2e VM currently runs a bare kubeadm init with no gate support. Reviewed and revised through four automated plan-review passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Step 10 (the last step of the DRA v1 plan) has landed; move its plan file alongside steps 1-9's, per this branch's established convention.
…ager/kubelet debug logging Independent of k8s_feature_gates -- either can be set alone or together; provision.yaml writes the kubeadm --config file whenever at least one is non-empty. Verified live: kubelet and kube-scheduler both emit DRA-specific log lines (dra_plugin_manager, PreFilter/PreBind.DynamicResources, nodeallocatabledynamicresources.go's "Patched pod status with NodeAllocatableResourceClaimStatuses") at --v=5 on a gated VM. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
k8s.io/api v0.37 (Kubernetes 1.37) tombstones Device.NodeAllocatableResourceMappings (CapacityKey/AllocationMultiplier), replacing it with the differently-shaped Device.NodeAllocatableResources (Mapping/Overhead). A binary built against k8s.io/api v0.36 sending the old field name gets it silently pruned by a 1.37 API server, so KEP-5517 support silently no-oped there even with the gate on. Bump k8s.io/api, apimachinery, client-go, kubelet, dynamic-resource-allocation to v0.37.0; rework dra.go's device construction to the new NodeAllocatableResources/Mapping/CapacityMultiplier shape; add Plugin.WatchHealthStatus (new kubeletplugin.DRAPlugin interface method in v0.37; this driver doesn't do per-device health reporting, so it returns ErrHealthNotSupported); update the e2e test's feature-gate probe to check the new field name. This moves the effective floor from Kubernetes 1.36 to 1.37 -- a single compiled binary can only speak one wire-shape for this field. Verified live against a real Kubernetes 1.37.0 cluster: Test verdict: PASS, full scheduler DynamicResources flow observable at --v=5. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… bump Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s.Mapping Kubernetes 1.37 independently tombstones NodeAllocatableResourceClaimStatus.Resources map[ResourceName]Quantity on the pod-status side (separate from the Device-side rename fixed in 38f6959), replacing it with Mapping []NodeAllocatableMappedResources{Name, Quantity} -- a list, not a map. Update wait-node-allocatable-claim-status's assertion to match. Verified live: Test verdict: PASS against a real Kubernetes 1.37.0 gated VM, full scheduler DynamicResources flow observable at --v=5. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… 1.37 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new kubeadm --config generation in test/e2e/playbook/provision.yaml is likely invalid for kubeadm init (schema/structure), risking broken e2e cluster provisioning.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces end-to-end scaffolding for Kubernetes DRA (Dynamic Resource Allocation) support in nri-plugins, centering on the topology-aware policy: wiring in resmgr lifecycle seams (post-unlock hook + safe shutdown ordering), adding cpuclass→DRA device publication primitives and persistence helpers, and extending e2e/Helm/CRD/docs to enable and exercise the feature.
Changes:
- Add DRA enablement knobs across e2e provisioning (feature gates / verbosity), Helm values/templates, and topology-aware policy configuration/CRDs/docs.
- Extend resmgr/policy interfaces with
Stop()andPostReconfigure()and plumb a “post-unlock” hook + safeStop()ordering to avoid lock/I/O deadlocks. - Add new DRA support packages and helpers (CDI writer, claim state persistence) plus a shared Kubernetes client/watch wrapper and related unit tests.
File summaries
| File | Description |
|---|---|
| test/e2e/run.sh | Adds env knobs for feature gates and component log verbosity; prints them in run summary. |
| test/e2e/run_tests.sh | Preserves caller-provided vm_name to avoid collisions between runs. |
| test/e2e/policies.test-suite/topology-aware/helm-config.yaml.in | Adds optional dra.enabled Helm config injection for topology-aware tests. |
| test/e2e/playbook/provision.yaml | Adds kubeadm config generation path for feature gates / verbosity and conditional init logic. |
| test/e2e/files/Vagrantfile.in | Plumbs new Kubernetes feature gate / verbosity env vars into Ansible provisioning vars. |
| pkg/resmgr/resource-manager.go | Adds post-reconfigure hook, safe policy Stop-before-lock shutdown, lock helper, kube client wrapper, and passes new policy options. |
| pkg/resmgr/resource_manager_test.go | Adds unit tests for shutdown ordering, lock helper, typed-nil-safe kube client wrapper, and post-reconfigure behavior. |
| pkg/resmgr/policy/policy.go | Extends backend/policy APIs with Stop() and PostReconfigure() and forwards new backend options. |
| pkg/resmgr/policy/policy_test.go | Adds tests for Stop/PostReconfigure forwarding and new options plumbing/lock contract. |
| pkg/resmgr/main/main.go | Ensures resource manager Stop is invoked after Start returns. |
| pkg/resmgr/main/main_test.go | Adds tests asserting Stop is called after Start (including error case). |
| pkg/resmgr/dra/state.go | Adds claim state marshaling and cache-backed claim store implementation. |
| pkg/resmgr/dra/state_test.go | Adds round-trip tests for claim persistence and cache-backed store behavior. |
| pkg/resmgr/dra/logging.go | Adds logr bridge backed by repo logger for kubelet-plugin integration. |
| pkg/resmgr/dra/doc.go | Adds package-level docs and import-boundary constraints for dra package. |
| pkg/resmgr/dra/deps.go | Defines DRA plugin dependency surface (device lister/allocator/store/CDI writer, etc.). |
| pkg/resmgr/dra/cdi.go | Implements CDI writer using upstream CDI library (spec writing, listing, sanitization). |
| pkg/resmgr/dra/cdi_test.go | Adds CDI writer and name-sanitization tests. |
| pkg/resmgr/cpuclass/internal/pct/pct.go | Adds DRA HP accounting, punit capacity snapshots, and HP allocation/accounting helpers. |
| pkg/resmgr/cpuclass/dra.go | Adds DRA device schema/constants + publication builder and config validation helpers. |
| pkg/resmgr/cpuclass/cpuclass.go | Stores last-applied classes and exposes DRA-related pass-through methods on Handler. |
| pkg/resmgr/cpuclass/cpuclass_dra_test.go | Adds tests for Handler’s DRA ClaimAllocator pass-through methods. |
| pkg/resmgr/cache/container.go | Exposes CDI device names on cache containers. |
| pkg/resmgr/cache/container_test.go | Adds tests for CDI device name extraction and test helper to populate CDI devices. |
| pkg/resmgr/cache/cache.go | Extends cache Container interface with GetCDIDeviceNames(). |
| pkg/kubernetes/watch/watch.go | Adjusts logger name for watch package. |
| pkg/kubernetes/watch/watch_test.go | Adds unit tests for type aliases and file/object watch behaviors. |
| pkg/kubernetes/watch/object.go | Adds single-object watch wrapper with reopen/backoff semantics. |
| pkg/kubernetes/watch/file.go | Adds fsnotify-based file watch wrapper producing k8s-like watch events. |
| pkg/kubernetes/client/testdata/kubeconfig-example.yaml | Adds a minimal kubeconfig fixture for unit tests. |
| pkg/kubernetes/client/client.go | Adds shared Kubernetes client wrapper with option-based config and retry-on-config-set options. |
| pkg/kubernetes/client/client_test.go | Adds comprehensive unit tests for client option semantics and copy/close behavior. |
| pkg/apis/config/v1alpha1/resmgr/policy/zz_generated.deepcopy.go | Updates generated deep-copies for new cpuClass DRA fields. |
| pkg/apis/config/v1alpha1/resmgr/policy/topologyaware/zz_generated.deepcopy.go | Updates generated deep-copies for new topology-aware DRA config fields. |
| pkg/apis/config/v1alpha1/resmgr/policy/topologyaware/config.go | Adds dra config block + nil-safe getters for DRA enablement/sharedCounters. |
| pkg/apis/config/v1alpha1/resmgr/policy/topologyaware/config_test.go | Adds tests for new nil-safe DRA getters and validation baseline. |
| pkg/apis/config/v1alpha1/resmgr/policy/cpuclass.go | Adds cpuClass-level dra.publish opt-out and DRAPublish() helper. |
| pkg/apis/config/v1alpha1/resmgr/policy/cpuclass_test.go | Adds tests for DRAPublish() defaults and deepcopy behavior. |
| pkg/agent/agent.go | Switches agent to shared Kubernetes client/watch wrappers; adds getters for node name/config/client/rest config. |
| pkg/agent/agent_test.go | Adds tests for new agent getters (zero-value and initialized cases). |
| go.mod | Bumps Kubernetes deps to v0.37.0 and adds DRA/CDI/logr dependencies. |
| docs/resource-policy/policy/topology-aware.md | Documents topology-aware DRA feature, prerequisites, enabling, and ResourceClaim usage. |
| docs/plans/completed/20260820-dra-step3-cpuclass-dra-publish.md | Adds completed plan doc for step 3 (cpuClass DRA publish + validation). |
| docs/plans/completed/20260820-dra-step2-resmgr-dra-skeleton.md | Adds completed plan doc for step 2 (resmgr DRA package skeleton). |
| docs/dra/pr-536-analysis.md | Adds analysis notes of prior DRA prototype for context/reference. |
| deployment/helm/topology-aware/values.yaml | Adds config.dra values (enabled/sharedCounters). |
| deployment/helm/topology-aware/templates/deviceclass.yaml | Adds conditional DeviceClass manifest for DRA driver. |
| deployment/helm/topology-aware/templates/daemonset.yaml | Adds conditional mounts needed for kubelet plugins registry and CDI dir. |
| deployment/helm/topology-aware/templates/clusterrole.yaml | Adds conditional RBAC for DRA resourceslices/resourceclaims. |
| deployment/helm/topology-aware/crds/config.nri_topologyawarepolicies.yaml | Updates Helm CRD with new DRA fields (policy-level + cpuClass-level). |
| deployment/helm/balloons/crds/config.nri_balloonspolicies.yaml | Updates Helm CRD with new cpuClass DRA publish field. |
| config/crd/bases/config.nri_topologyawarepolicies.yaml | Updates base CRD with new DRA fields (policy-level + cpuClass-level). |
| config/crd/bases/config.nri_balloonspolicies.yaml | Updates base CRD with new cpuClass DRA publish field. |
| cmd/plugins/topology-aware/policy/resources.go | Adds DRA claim CPU accounting in supply (claim/unclaim + clone support). |
| cmd/plugins/topology-aware/policy/mocks_test.go | Extends container mocks for CDI devices and cpuset tracking for tests. |
| cmd/plugins/topology-aware/policy/dra.go | Adds topology-aware DRA plugin construction, post-reconfigure publish hook, and device diff helpers. |
| cmd/plugins/topology-aware/policy/dra_adapter.go | Adds adapter bridging topology-aware policy cpuclass handler to DRA deps/interfaces. |
| cmd/plugins/topology-aware/policy/dra_adapter_test.go | Adds tests verifying adapter forwards to the current cpuclass handler instance. |
| cmd/plugins/topology-aware/policy/cache_test.go | Updates allocation marshalling fixtures for updated fields. |
| cmd/plugins/template/policy/template-policy.go | Implements new no-op Stop() / PostReconfigure() policy interface methods. |
| cmd/plugins/balloons/policy/balloons-policy.go | Implements new no-op Stop() / PostReconfigure() policy interface methods. |
| CLAUDE.md | Adds repo context file for tooling/workflow (project overview + DRA pointers). |
| .gitignore | Ignores *.test and extracted sysfs/topology fixture directories. |
Review details
- Files reviewed: 82/86 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+511
to
+517
| - name: Write kubeadm --config with feature gates and/or log verbosity for apiserver/scheduler/controller-manager/kubelet | ||
| ansible.builtin.copy: | ||
| dest: /root/kubeadm-config.yaml | ||
| content: | | ||
| apiVersion: kubeadm.k8s.io/v1beta4 | ||
| kind: ClusterConfiguration | ||
| networking: |
Comment on lines
91
to
93
| err := m.mgr.Start() | ||
| m.mgr.Stop() | ||
| return err |
Comment on lines
+549
to
+552
| featureGates: | ||
| {% for gate in k8s_feature_gates.split(',') %} | ||
| {{ gate.split('=')[0] }}: {{ gate.split('=')[1] }} | ||
| {% endfor %} |
There was a problem hiding this comment.
🟡 Changes recommended
Capacity accounting can overpublish or double-book CPUs, while failed reconfiguration can leave stale DRA resources advertised.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 82/86 changed files
- Comments generated: 9
- Review effort level: Balanced
Comment on lines
+79
to
+81
| if sharedCounters { | ||
| return nil | ||
| } |
Comment on lines
+185
to
+186
| if err := m.policy.PostReconfigure(); err != nil { | ||
| log.Errorf("failed to run post-reconfigure hook: %v", err) |
| if !a.hpEligiblePunit[idx] { | ||
| return 0 | ||
| } | ||
| return a.punits[idx].GuaranteedHpCpus |
Comment on lines
+63
to
+67
| kubeClient := opts.KubeClientFn() | ||
| if kubeClient == nil { | ||
| log.Warnf("dra: no kube client available yet, DRA plugin not started") | ||
| return nil | ||
| } |
Comment on lines
+429
to
+431
| // isolated/sharable. (Re-pinning any reserved-type container *already* | ||
| // running on that CPU is a separate, pre-existing gap: updateSharedAllocations | ||
| // explicitly skips cpuType == cpuReserved grants; not addressed here.) |
Comment on lines
+170
to
+173
| return func(c *Client) error { | ||
| c.cfg = rest.CopyConfig(cfg) | ||
| return nil | ||
| } |
Comment on lines
+789
to
+792
| Each defined `cpuClass` is published as a DRA device under the | ||
| `nri.topology-aware.cpu` `DeviceClass`, with the class's config attributes (such as | ||
| `nri/pctPriority`) exposed as device attributes. A `ResourceClaim` can select on any | ||
| of these attributes. For example, to request 2 CPUs from a high-priority PCT class: |
| (`DRAConsumableCapacity` / consumable capacity) alpha feature gate enabled on the | ||
| API server and kubelet. The `DeviceClass` object below uses the `resource.k8s.io/v1` | ||
| API, which requires Kubernetes 1.34+. | ||
| - Optionally, Kubernetes 1.36+, with the |
Comment on lines
+60
to
+66
| # NOTE for callers enabling DRAConsumableCapacity/DRANodeAllocatableResources | ||
| # (KEP-5075/KEP-5517): also pin k8s_release explicitly to >=1.36, e.g. | ||
| # k8s_release=1.36 k8s_feature_gates="DRAConsumableCapacity=true,DRANodeAllocatableResources=true" ... | ||
| # DRANodeAllocatableResources is alpha only since Kubernetes 1.36 (the | ||
| # higher of the two gates' floors); do not rely on the "latest" default | ||
| # above without pinning, since "latest" drifting below 1.36 later would | ||
| # silently break a gated run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.