Refactor shared plugin types for DRA reuse#1276
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
the
do-not-merge/work-in-progress
✅ Deploy Preview for kubernetes-sigs-kmm ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomerNewman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
| type DevicePluginSpec struct { | ||
| Container DevicePluginContainerSpec `json:"container"` | ||
| // DevicePluginContainerSpec is a backward-compatible alias for PluginContainerSpec. | ||
| type DevicePluginContainerSpec = PluginContainerSpec |
There was a problem hiding this comment.
why do we need this?
There was a problem hiding this comment.
The type alias keeps all existing code that references DevicePluginContainerSpec compiling without changes — controllers, tests, and the webhook all use this name. In the next PR (Story 2), the new DRASpec will reference PluginContainerSpec directly, so the shared type gets a generic name while the alias preserves backward compatibility.
| // DevicePluginContainerSpec is a backward-compatible alias for PluginContainerSpec. | ||
| type DevicePluginContainerSpec = PluginContainerSpec | ||
|
|
||
| type PluginSpec struct { |
There was a problem hiding this comment.
Why do we need PluginSpec and DevicePLuginSpec? We can keep DevicePluginSpec, which will contain the Container and InitContainer and PluginContainerSpec field
There was a problem hiding this comment.
PluginSpec is the shared base that both DevicePluginSpec and the upcoming DRASpec (Story 2) will use. DRA drivers need the same pod-level fields — container, init container, service account, volumes. Extracting them into PluginSpec now avoids duplicating those fields across both specs. DevicePluginSpec wraps it with json:",inline" so the existing CRD schema is unchanged (verified via zero CRD diff).
TomerNewman
force-pushed
the
MGMT-24467-refactor-shared-plugin-types
branch
from
2bf0ac0 to
38121f7
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1276 +/- ##
==========================================
- Coverage 79.09% 72.97% -6.13%
==========================================
Files 51 66 +15
Lines 5109 4703 -406
==========================================
- Hits 4041 3432 -609
- Misses 882 1108 +226
+ Partials 186 163 -23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Can we remove |
TomerNewman
force-pushed
the
MGMT-24467-refactor-shared-plugin-types
branch
from
38121f7 to
7134479
Compare
|
Something isn't clear to me.
|
| DevicePlugin: &kmmv1beta1.DevicePluginSpec{ | ||
| Container: kmmv1beta1.DevicePluginContainerSpec{Image: devicePluginImage}, | ||
| Volumes: []v1.Volume{vol}, | ||
| CommonSpec: kmmv1beta1.CommonSpec{ |
There was a problem hiding this comment.
Lets add a unit test that verifies the backword compatibility also
There was a problem hiding this comment.
added "DevicePluginSpec backward compatibility" unit test
TomerNewman
force-pushed
the
MGMT-24467-refactor-shared-plugin-types
branch
3 times, most recently
from
cd6c2d4 to
881acc1
Compare
Extract shared container and pod configuration types (CommonContainerSpec, CommonSpec) from DevicePluginContainerSpec and DevicePluginSpec. The original types are preserved as a type alias and inline embed respectively, keeping JSON serialization and CRD schema identical. This refactoring enables the upcoming DRA support to reuse the same base types without duplicating field definitions.
TomerNewman
force-pushed
the
MGMT-24467-refactor-shared-plugin-types
branch
from
881acc1 to
e195f33
Compare
k8s-ci-robot
added
size/M
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
5 participants
Refactor shared plugin types for DRA reuse
Summary
Extracts shared container and pod configuration types (
PluginContainerSpec,PluginSpec) from the existingDevicePluginContainerSpecandDevicePluginSpecstructs. The original types are preserved as a backward-compatible type alias and inline embed, keeping JSON serialization and CRD schema identical.This refactoring enables the upcoming DRA support to reuse the same base types without duplicating field definitions.
Changes
api/v1beta1/module_types.go:DevicePluginContainerSpectoPluginContainerSpec(7 fields, all struct tags preserved)type DevicePluginContainerSpec = PluginContainerSpec(backward-compatible alias)PluginSpecstruct (5 fields)DevicePluginSpecto embedPluginSpecwithjson:",inline"api/v1beta1/zz_generated.deepcopy.go: Regenerated — deepcopy methods now generated forPluginContainerSpecandPluginSpecinternal/controllers/device_plugin_reconciler_test.go: Updated 3 struct literals to usePluginSpec:embedded field in composite literalsTesting
internal/controllers)kmm.sigs.x-k8s.io_modules.yamlis identical before and aftermake generate,make manifests,make fmt,make vet,make lint,make unit-testall passAcceptance Criteria
PluginContainerSpecis defined with fields:Command,Args,Env,Image,ImagePullPolicy,Resources,VolumeMountsDevicePluginContainerSpecis a type alias forPluginContainerSpec(backward compatible)PluginSpecis defined with fields:Container,InitContainer,ServiceAccountName,Volumes,AutomountServiceAccountTokenDevicePluginSpecembedsPluginSpecinline so existing YAML field names are unchangedDevicePluginContainerSpecandDevicePluginSpeccompile without modification