Simplify project structure#255
Simplify project structure#255dabradley wants to merge 5 commits intokubernetes-sigs:developmentfrom
Conversation
8c97b55 to
49e9dfa
Compare
There was a problem hiding this comment.
Pull request overview
This PR simplifies the project structure by removing the pkg/csi-common directory containing redundant default implementations of CSI interfaces. The newer CSI libraries provide base classes (csi.UnimplementedControllerServer, csi.UnimplementedNodeServer, csi.UnimplementedIdentityServer) that automatically handle unimplemented methods, making the custom default implementations unnecessary.
Changes:
- Removed the entire
pkg/csi-commondirectory containing redundant default server implementations and tests - Moved the
CSIDriverstruct and capability management methods directly into theDriverstruct in the azurelustre package - Embedded CSI unimplemented server structs directly in the
Drivertype - Improved error messages for unimplemented methods to be more descriptive
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/csi-common/nodeserver-default.go | Deleted redundant default node server implementation |
| pkg/csi-common/nodeserver-default_test.go | Deleted corresponding tests |
| pkg/csi-common/identityserver-default.go | Deleted redundant default identity server implementation |
| pkg/csi-common/identityserver-default_test.go | Deleted corresponding tests |
| pkg/csi-common/driver.go | Deleted redundant CSIDriver struct and methods |
| pkg/csi-common/driver_test.go | Deleted corresponding tests |
| pkg/csi-common/controllerserver-default.go | Deleted redundant default controller server implementation |
| pkg/csi-common/controllerserver-default_test.go | Deleted corresponding tests |
| pkg/azurelustre/utils.go | Changed package from csicommon to azurelustre, removed factory functions for default servers |
| pkg/azurelustre/utils_test.go | Changed package from csicommon to azurelustre |
| pkg/azurelustre/server.go | Changed package from csicommon to azurelustre |
| pkg/azurelustre/server_test.go | Changed package from csicommon to azurelustre |
| pkg/azurelustre/azurelustre.go | Added CSIDriver struct and capability methods, embedded unimplemented server structs |
| pkg/azurelustre/azurelustre_test.go | Added tests for capability management methods |
| pkg/azurelustre/nodeserver.go | Improved error messages for unimplemented methods |
| pkg/azurelustre/nodeserver_test.go | Added tests for unimplemented node methods |
| pkg/azurelustre/controllerserver_test.go | Added tests for unimplemented controller methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pull Request Test Coverage Report for Build 21041124610Details
💛 - Coveralls |
49e9dfa to
5896bfa
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dabradley, jeffbearer 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 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The newer versions of the CSI libraries have base classes that will automatically respond to unimplemented API calls appropriately. The csi-common directory only contained files that specifically returned these values, and in some cases contained redundant implementations of API functionality that were shadowed by the real driver files. This PR removes all of this redundant code to help improve maintainability and remove unnecessary and potentially confusing complexity. The existing tests were moved into the appropriate driver files, those these already didn't necessarily exhaustively test the unimplemented responses.
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Release note: