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.
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
Simplify the unregistration of csiplugin #89934
Conversation
/assign @saad-ali |
/priority backlog |
pkg/kubelet/pluginmanager/operationexecutor/operation_generator.go
Outdated
Show resolved
Hide resolved
d0a8dbd
to
b54e7d9
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saad-ali, tedyu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! Just one suggestion for consistency. The PluginInfo
objects in actual_state_of_world_test.go
need to be updated with the new fields to be added.
@taragu Can you be a bit specific on how the PluginInfo object should be populated in the test ? |
@tedyu sure, here for example: https://github.com/kubernetes/kubernetes/blob/b54e7d92652797d8485596440960490d66c505be/pkg/kubelet/pluginmanager/cache/actual_state_of_world_test.go#L30 we can change it to the following just for completion purposes
|
Signed-off-by: Ted Yu <yuzhihong@gmail.com>
b54e7d9
to
1001be8
Compare
@taragu |
/lgtm |
/hold cancel |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
This PR simplifies the unregistration of csiplugin by dropping SocketPluginHandlers.
Which issue(s) this PR fixes:
Addresses concerns raised in #88006 (comment)
Fixes #87282
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: