Refactor NodeServer to eliminate Linode API dependency and support isolated driver roles #415
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jescarri
force-pushed
the
remove_linode_api_ds
branch
from
1a5d390 to
89f69d9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #415 +/- ##
==========================================
+ Coverage 73.65% 74.01% +0.35%
==========================================
Files 23 24 +1
Lines 2775 2763 -12
==========================================
+ Hits 2044 2045 +1
+ Misses 594 586 -8
+ Partials 137 132 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
komer3
reviewed
Apr 25, 2025
Author
|
Related To: #417 |
jescarri
force-pushed
the
remove_linode_api_ds
branch
from
0f9e85c to
7361046
Compare
jescarri
changed the title
komer3
reviewed
Apr 30, 2025
jescarri
force-pushed
the
remove_linode_api_ds
branch
from
7361046 to
ebfd4ca
Compare
jescarri
force-pushed
the
remove_linode_api_ds
branch
from
ebfd4ca to
c66838b
Compare
komer3
reviewed
May 2, 2025
Contributor
komer3
left a comment
komer3
left a comment
There was a problem hiding this comment.
Just one small thing. Apart from that everything looks good to me!
jescarri
force-pushed
the
remove_linode_api_ds
branch
from
1913f4f to
68d9865
Compare
This change decouples the NodeServer from the Linode API by replacing disk enumeration logic with local system inspection. A DRIVER_ROLE environment variable is introduced to distinguish Controller and NodeServer modes. When set to 'nodeserver', cloud API credentials are no longer required. Sanity tests for Controller and NodeServer are now executed independently to reflect this separation. update helm chart node-server daemonset and remove the need of LINODE_TOKEN to be set. remove LINODE_TOKEN from node-server kustomize manifests add lsblk as a dependency in the Dockerfile exclude non block devices from lsblk output remove linode api testing from node_server test TestNodeGetInfo added methods to detect the number of attached disks add hardwareinfo package and update attachedVolumeCount upodate mocks remove linode client dependency from node-server add driver role either controller or nodeserver this controls if the LINODE_TOKEN is required or not. skip some csi-sanity checks that requre linodeapi access fix golangci-lint remove unused functions fix golanci-lint errors fix TestNodeGetInfo so that it uses mockhw. fix: logic to detect attached disks. Move from counting disks with valid partitions to disk bus type. update deployment documentation Added section about driver_role env variables
added 2 commits
May 2, 2025 13:44
In order for the node-server sanity checks to pass we have to: 1. Add Linode token and api to the nodeserver daemonset There's a need for the nodeserver to have linode api access because sanity checks require CreateVolume and ListVolume. By patching the node-server Daemonset and injecting the LINODE_TOKEN The Controller Code Path is able to be initialized with a proper client. This is not needed for normal nodeserver operation, however the sanity-checks expect the ability for the nodeserver to also perform controller functions. E2E tests pass without the need of LINODE_TOKEN in the nodeserver
- Specify that `DRIVER_ROLE=controller` requires `LINODE_TOKEN` and initializes the CSI Controller with Linode API access. - Specify that `DRIVER_ROLE=nodeserver` does not require `LINODE_TOKEN` and only launches the NodeServer, operating without any Linode API calls.
jescarri
force-pushed
the
remove_linode_api_ds
branch
from
68d9865 to
d36f902
Compare
komer3
added
the
improvement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
General:
Pull Request Guidelines:
This PR removes the dependency on the Linode API from the NodeServer component of the CSI driver.
Disk enumeration logic previously relying on the Linode API has been refactored to use local system information instead.
CSI Sanity tests are now executed independently for the Controller and NodeServer roles to reflect their distinct responsibilities.
A new DRIVER_ROLE environment variable has been introduced to explicitly define the driver's operational mode.
When DRIVER_ROLE=nodeserver, Linode API credentials are no longer required, enabling the NodeServer to run in environments where cloud API access is restricted or unnecessary.