Simplify attach process
This pull request focuses on simplifying the volume attachment logic in the ControllerServer by removing redundant checks and related methods. The most important changes include the removal of the checkAttachmentCapacity function and its associated methods, as well as the related test cases.
Context
CSI caller (like Kubernetes) should already respect volume limitation from NodeGetInfo.
We don't need another check and logic at attach.
Motivation
Reduce logic and complexity and rely more on Linode API calls. If there is an error, return it to the user for easier debug.
Risk
Making more failing attach requests and maybe hitting a rate limit. This should not be a problem, as CSI attacher have a backoff logic. Moreover, current limit logic is already doing api calls that should have hit API ratelimit.
Possible Mitigations
- Implementing a rate limiting on api call.
-
As GCP, implementing a
csiErrorBackoffmechanism
IMHO, both are not mandatory for the moment.
Simplification of volume attachment logic:
- Removed the
checkAttachmentCapacityfunction and its invocation from theControllerPublishVolumemethod ininternal/driver/controllerserver.go. - Deleted the
canAttachfunction, which was used to determine if an additional volume could be attached to an instance, frominternal/driver/controllerserver_helper.go. - Removed the
checkAttachmentCapacityfunction, which checked if an instance could accommodate additional volume attachments, frominternal/driver/controllerserver_helper.go.
Removal of related test cases:
- Deleted the
TestCheckAttachmentCapacitytest function frominternal/driver/controllerserver_helper_test.go, which tested thecheckAttachmentCapacitylogic.
General:
- [ ] Have you removed all sensitive information, including but not limited to access keys and passwords?
- [ ] Have you checked to ensure there aren't other open or closed Pull Requests for the same bug/feature/question?
Pull Request Guidelines:
- [ ] Does your submission pass tests?
- [ ] Have you added tests?
- [ ] Are you addressing a single feature in this PR?
- [ ] Are your commits atomic, addressing one change per commit?
- [ ] Are you following the conventions of the language?
- [ ] Have you saved your large formatting changes for a different PR, so we can focus on your work?
- [ ] Have you explained your rationale for why this feature is needed?
- [ ] Have you linked your PR to an open issue
Codecov Report
Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
Project coverage is 75.07%. Comparing base (
17b7d1b) to head (efdefae).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| internal/driver/controllerserver_helper.go | 75.00% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #322 +/- ##
==========================================
+ Coverage 74.79% 75.07% +0.28%
==========================================
Files 22 22
Lines 2396 2359 -37
==========================================
- Hits 1792 1771 -21
+ Misses 499 489 -10
+ Partials 105 99 -6
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.