linode / linode-blockstorage-csi-driver

Container Storage Interface (CSI) Driver for Linode Block Storage
Apache License 2.0
68 stars 57 forks source link

[Bug Fix] AttachVolume error where volume region is empty #301

Closed komer3 closed 2 weeks ago

komer3 commented 3 weeks ago

volumeContext[VolumeTopologyRegion] was not a reliable source to get volumes region. We saw multiple instances where this was returning an empty string which was causing the region mismatch failure. Switching to just use volume obj returned by API to validate is a better and more robust approach since LinodeVolume Obj returned by the API will always have the correct region field.

General:

Pull Request Guidelines:

  1. [ ] Does your submission pass tests?
  2. [ ] Have you added tests?
  3. [ ] Are you addressing a single feature in this PR?
  4. [ ] Are your commits atomic, addressing one change per commit?
  5. [ ] Are you following the conventions of the language?
  6. [ ] Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. [ ] Have you explained your rationale for why this feature is needed?
  8. [ ] Have you linked your PR to an open issue
codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 74.83%. Comparing base (12e5659) to head (b40379a). Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/driver/controllerserver_helper.go 33.33% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #301 +/- ## ======================================= Coverage 74.83% 74.83% ======================================= Files 22 22 Lines 2356 2356 ======================================= Hits 1763 1763 Misses 491 491 Partials 102 102 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

komer3 commented 2 weeks ago

@srust The region mismatch error serves as a critical validation check (sort of double checking). This issue typically surfaces only in specific multi-region scenarios where cluster configuration may be incomplete.

Primary Scenario

This issue occurs when:

Example Case

Consider this setup:

In this case, the CSI driver defaults to creating the volume in ORD (fallback region) instead of the intended IAD region, triggering the mismatch error.

Note: This error should only manifest when utilizing the CSI driver's multi-region capabilities.

This error should not be triggered with how we currently use and deploy CSI driver. Replacing the use of VolumeContext should make this check more reliable and not trigger anymore unless in very specific cases such as the one I mentioned above.

Hope that answers your question! :)