Skip to content

Convert k8s.io/kubelet/pkg/apis/dra from gogo to protoc #133026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Convert k8s.io/kubelet/pkg/apis/dra from gogo to protoc #133026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion hack/unwanted-dependencies.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@
"k8s.io/client-go",
"k8s.io/code-generator",
"k8s.io/kube-aggregator",
"k8s.io/kubelet",
"k8s.io/kubernetes",
"k8s.io/metrics"
],
Expand Down
11 changes: 3 additions & 8 deletions hack/update-codegen.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1020,10 +1020,8 @@ function codegen::protobindings() {

# Each element of this array is a directory containing subdirectories which
# eventually contain a file named "api.proto".
local apis_using_gogo=(
local apis=(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we discover those instead of listing explicitly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(separate PR, not blocking this one)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you asking if we can automatically distinguish which api.proto files are using gogo and which are using standard protoc, or if we can discover all api.proto files anywhere in the tree?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second. We switched to protoc everywhere. Now instead of listing APIs maybe we can discover them in the tree

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it's possible, can look into doing that as a follow-up

"staging/src/k8s.io/kubelet/pkg/apis/dra"
)
local apis_using_protoc=(
"staging/src/k8s.io/kubelet/pkg/apis/deviceplugin"
"staging/src/k8s.io/kubelet/pkg/apis/podresources"
"staging/src/k8s.io/kms/apis"
Expand All @@ -1034,7 +1032,6 @@ function codegen::protobindings() {
"staging/src/k8s.io/externaljwt/apis"
"staging/src/k8s.io/kubelet/pkg/apis/dra-health"
)
local apis=("${apis_using_gogo[@]}" "${apis_using_protoc[@]}")

kube::log::status "protobuf bindings: ${#apis[@]} targets"
if [[ "${DBG_CODEGEN}" == 1 ]]; then
Expand All @@ -1050,15 +1047,13 @@ function codegen::protobindings() {
done

if kube::protoc::check_protoc >/dev/null; then
hack/_update-generated-proto-bindings-dockerized.sh gogo "${apis_using_gogo[@]}"
hack/_update-generated-proto-bindings-dockerized.sh protoc "${apis_using_protoc[@]}"
hack/_update-generated-proto-bindings-dockerized.sh protoc "${apis[@]}"
Comment on lines -1054 to +1050
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll follow-up on the script part in a separate PR.

Copy link
Member

@liggitt liggitt Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow-up is good, if we don't have any gogo proto bindings now, let's delete all the gogo logic from that path completely (drop the generator arg check, delete generate_proto_gogo, etc)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref: #133271

else
kube::log::status "protoc ${PROTOC_VERSION} not found (can install with hack/install-protoc.sh); generating containerized..."
# NOTE: All output from this script needs to be copied back to the calling
# source tree. This is managed in kube::build::copy_output in build/common.sh.
# If the output set is changed update that function.
build/run.sh hack/_update-generated-proto-bindings-dockerized.sh gogo "${apis_using_gogo[@]}"
build/run.sh hack/_update-generated-proto-bindings-dockerized.sh protoc "${apis_using_protoc[@]}"
build/run.sh hack/_update-generated-proto-bindings-dockerized.sh protoc "${apis[@]}"
fi
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/kubelet/cm/dra/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (m *Manager) prepareResources(ctx context.Context, pod *v1.Pod) error {
// Loop through all drivers and prepare for calling NodePrepareResources.
claim := &drapb.Claim{
Namespace: claimInfo.Namespace,
UID: string(claimInfo.ClaimUID),
Uid: string(claimInfo.ClaimUID),
Name: claimInfo.ClaimName,
}
for driverName := range claimInfo.DriverState {
Expand Down Expand Up @@ -403,7 +403,7 @@ func (m *Manager) prepareResources(ctx context.Context, pod *v1.Pod) error {
return fmt.Errorf("internal error: unable to get claim info for ResourceClaim %s", claim.Name)
}
for _, device := range result.GetDevices() {
info.addDevice(plugin.DriverName(), state.Device{PoolName: device.PoolName, DeviceName: device.DeviceName, RequestNames: device.RequestNames, CDIDeviceIDs: device.CDIDeviceIDs})
info.addDevice(plugin.DriverName(), state.Device{PoolName: device.PoolName, DeviceName: device.DeviceName, RequestNames: device.RequestNames, CDIDeviceIDs: device.CdiDeviceIds})
}
return nil
})
Expand Down Expand Up @@ -448,7 +448,7 @@ func (m *Manager) prepareResources(ctx context.Context, pod *v1.Pod) error {

func lookupClaimRequest(claims []*drapb.Claim, claimUID string) *drapb.Claim {
for _, claim := range claims {
if claim.UID == claimUID {
if claim.Uid == claimUID {
return claim
}
}
Expand Down Expand Up @@ -565,7 +565,7 @@ func (m *Manager) unprepareResources(ctx context.Context, podUID types.UID, name
// Loop through all drivers and prepare for calling NodeUnprepareResources.
claim := &drapb.Claim{
Namespace: claimInfo.Namespace,
UID: string(claimInfo.ClaimUID),
Uid: string(claimInfo.ClaimUID),
Name: claimInfo.ClaimName,
}
for driverName := range claimInfo.DriverState {
Expand Down
16 changes: 8 additions & 8 deletions pkg/kubelet/cm/dra/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,17 @@ func (s *fakeDRADriverGRPCServer) NodePrepareResources(ctx context.Context, req
}

if s.prepareResourcesResponse == nil {
cdiDeviceName := "claim-" + req.Claims[0].UID
cdiDeviceName := "claim-" + req.Claims[0].Uid
cdiID := s.driverName + "/" + driverClassName + "=" + cdiDeviceName
return &drapb.NodePrepareResourcesResponse{
Claims: map[string]*drapb.NodePrepareResourceResponse{
req.Claims[0].UID: {
req.Claims[0].Uid: {
Devices: []*drapb.Device{
{
PoolName: poolName,
DeviceName: deviceName,
RequestNames: []string{req.Claims[0].Name},
CDIDeviceIDs: []string{cdiID},
CdiDeviceIds: []string{cdiID},
},
},
},
Expand All @@ -110,7 +110,7 @@ func (s *fakeDRADriverGRPCServer) NodeUnprepareResources(ctx context.Context, re
if s.unprepareResourcesResponse == nil {
return &drapb.NodeUnprepareResourcesResponse{
Claims: map[string]*drapb.NodeUnprepareResourceResponse{
req.Claims[0].UID: {},
req.Claims[0].Uid: {},
},
}, nil
}
Expand Down Expand Up @@ -567,7 +567,7 @@ func TestPrepareResources(t *testing.T) {
PoolName: poolName,
DeviceName: deviceName,
RequestNames: []string{requestName},
CDIDeviceIDs: []string{cdiID},
CdiDeviceIds: []string{cdiID},
},
},
},
Expand All @@ -587,7 +587,7 @@ func TestPrepareResources(t *testing.T) {
PoolName: poolName,
DeviceName: deviceName,
RequestNames: []string{requestName},
CDIDeviceIDs: []string{cdiID},
CdiDeviceIds: []string{cdiID},
},
},
},
Expand Down Expand Up @@ -615,7 +615,7 @@ func TestPrepareResources(t *testing.T) {
PoolName: poolName,
DeviceName: deviceName,
RequestNames: []string{requestName},
CDIDeviceIDs: []string{cdiID},
CdiDeviceIds: []string{cdiID},
},
},
},
Expand All @@ -636,7 +636,7 @@ func TestPrepareResources(t *testing.T) {
PoolName: poolName,
DeviceName: deviceName,
RequestNames: []string{requestName},
CDIDeviceIDs: []string{cdiID},
CdiDeviceIds: []string{cdiID},
},
},
},
Expand Down
5 changes: 3 additions & 2 deletions pkg/kubelet/cm/dra/plugin/dra_plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
type fakeGRPCServer struct {
drapbv1beta1.UnimplementedDRAPluginServer
drahealthv1alpha1.UnimplementedDRAResourceHealthServer
drapbv1.UnsafeDRAPluginServer
}

var _ drapbv1.DRAPluginServer = &fakeGRPCServer{}
Expand All @@ -49,7 +50,7 @@ func (f *fakeGRPCServer) NodePrepareResources(ctx context.Context, in *drapbv1.N
Devices: []*drapbv1.Device{
{
RequestNames: []string{"test-request"},
CDIDeviceIDs: []string{"test-cdi-id"},
CdiDeviceIds: []string{"test-cdi-id"},
},
},
}}}, nil
Expand Down Expand Up @@ -162,7 +163,7 @@ func TestGRPCConnIsReused(t *testing.T) {
Claims: []*drapbv1.Claim{
{
Namespace: "dummy-namespace",
UID: "dummy-uid",
Uid: "dummy-uid",
Name: "dummy-claim",
},
},
Expand Down
7 changes: 4 additions & 3 deletions staging/src/k8s.io/dynamic-resource-allocation/kubeletplugin/draplugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,7 @@ func (d *Helper) serializeGRPCIfEnabled() (func(), error) {
// It prevents polluting the public API with these implementation details.
type nodePluginImplementation struct {
*Helper
drapbv1.UnsafeDRAPluginServer
}

// NodePrepareResources implements [drapbv1.NodePrepareResources].
Expand Down Expand Up @@ -867,7 +868,7 @@ func (d *nodePluginImplementation) NodePrepareResources(ctx context.Context, req
RequestNames: stripSubrequestNames(result.Requests),
PoolName: result.PoolName,
DeviceName: result.DeviceName,
CDIDeviceIDs: result.CDIDeviceIDs,
CdiDeviceIds: result.CDIDeviceIDs,
}
devices = append(devices, device)
}
Expand Down Expand Up @@ -904,7 +905,7 @@ func (d *nodePluginImplementation) getResourceClaims(ctx context.Context, claims
if claim.Status.Allocation == nil {
return resourceClaims, fmt.Errorf("claim %s/%s not allocated", claimReq.Namespace, claimReq.Name)
}
if claim.UID != types.UID(claimReq.UID) {
if claim.UID != types.UID(claimReq.Uid) {
return resourceClaims, fmt.Errorf("claim %s/%s got replaced", claimReq.Namespace, claimReq.Name)
}
resourceClaims = append(resourceClaims, claim)
Expand All @@ -922,7 +923,7 @@ func (d *nodePluginImplementation) NodeUnprepareResources(ctx context.Context, r

claims := make([]NamespacedObject, 0, len(req.Claims))
for _, claim := range req.Claims {
claims = append(claims, NamespacedObject{UID: types.UID(claim.UID), NamespacedName: types.NamespacedName{Name: claim.Name, Namespace: claim.Namespace}})
claims = append(claims, NamespacedObject{UID: types.UID(claim.Uid), NamespacedName: types.NamespacedName{Name: claim.Name, Namespace: claim.Namespace}})
}
result, err := d.plugin.UnprepareResourceClaims(ctx, claims)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion staging/src/k8s.io/kubelet/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ godebug default=go1.24

require (
github.com/emicklei/go-restful/v3 v3.12.2
github.com/gogo/protobuf v1.3.2
github.com/stretchr/testify v1.10.0
go.uber.org/goleak v1.3.0
google.golang.org/grpc v1.72.1
Expand All @@ -30,6 +29,7 @@ require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fxamacker/cbor/v2 v2.9.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/google/go-cmp v0.7.0 // indirect
github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
Loading