project: Add support for Block Storage Volume Limit Increase#865
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands the volume limit support for Linode instances by adding support for additional device mappings beyond the original SDA-SDH range. The changes enable instances to support up to 38 device slots (SDI-SDZ, SDAA-SDAZ, and SDBA-SDBL), which is necessary for larger instance types that can accommodate more volumes.
Key Changes:
- Extended
InstanceConfigDeviceMapstruct to support device mappings from SDI through SDBL - Added comprehensive unit and integration tests to validate the expanded device mapping functionality
- Updated test fixtures to include the new device mapping fields
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| instance_configs.go | Extends InstanceConfigDeviceMap struct with 30 new device fields (SDI-SDL, SDAA-SDAZ, SDBA-SDBL) to support expanded volume limits |
| test/unit/instance_config_test.go | Adds device mapping assertions to existing create and update unit tests |
| test/unit/fixtures/instance_config_create.json | Updates fixture to include device mapping in create test response |
| test/unit/fixtures/instance_config_update.json | Updates fixture to include device mapping in update test response |
| test/integration/instance_config_test.go | Adds new integration test validating volume attachment to extended device slots (SDK, SDL) |
| test/integration/fixtures/TestInstance_Config_VolumeLimitExtension.yaml | Provides recorded API interactions for the new integration test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| VolumeID: volume2.ID, | ||
| }, | ||
| }, | ||
| RootDevice: "/dev/sdac", |
There was a problem hiding this comment.
The RootDevice is set to '/dev/sdac', but the devices map only defines SDA, SDL, and SDK. The root device path should correspond to one of the configured devices. Consider changing this to '/dev/sda' which is the actual disk device configured.
| SDI *InstanceConfigDevice `json:"sdi,omitempty"` | ||
| SDJ *InstanceConfigDevice `json:"sdj,omitempty"` | ||
| SDK *InstanceConfigDevice `json:"sdk,omitempty"` | ||
| SDL *InstanceConfigDevice `json:"sdl,omitempty"` | ||
| SDM *InstanceConfigDevice `json:"sdm,omitempty"` | ||
| SDN *InstanceConfigDevice `json:"sdn,omitempty"` | ||
| SDO *InstanceConfigDevice `json:"sdo,omitempty"` | ||
| SDP *InstanceConfigDevice `json:"sdp,omitempty"` | ||
| SDQ *InstanceConfigDevice `json:"sdq,omitempty"` | ||
| SDR *InstanceConfigDevice `json:"sdr,omitempty"` | ||
| SDS *InstanceConfigDevice `json:"sds,omitempty"` | ||
| SDT *InstanceConfigDevice `json:"sdt,omitempty"` | ||
| SDU *InstanceConfigDevice `json:"sdu,omitempty"` | ||
| SDV *InstanceConfigDevice `json:"sdv,omitempty"` |
There was a problem hiding this comment.
I originally wanted to deprecate Devices *InstanceConfigDeviceMap in favor of a new DeviceMap map[string]InstanceConfigDevice field, but this caused some ambiguous behavior in GetCreateOpts() and GetUpdateOpts().
Since the limit as of now is 64, I figure we can implement that in future major version 👍
jriddle-linode
left a comment
There was a problem hiding this comment.
LGTM works locally
📝 Description
This pull request adds support for the Block Storage Volume Limit increase project, which extends the maximum number of volumes that can be attached to a Linode to 64 (depending on amount of ram allocated for VM).
✔️ How to Test
The following test steps assume you have pulled down this PR locally.
Unit Testing
Integration Testing
NOTE: The following tests assume you are running against an environment where this feature is available (e.g. DevCloud).