Skip to content

Conversation

@lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Dec 15, 2025

📝 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

make test-unit

Integration Testing

NOTE: The following tests assume you are running against an environment where this feature is available (e.g. DevCloud).

 make fixtures TEST_ARGS="-run TestInstance_Config_VolumeLimitExtension"

Copilot AI review requested due to automatic review settings December 15, 2025 18:55
@lgarber-akamai lgarber-akamai requested a review from a team as a code owner December 15, 2025 18:55
@lgarber-akamai lgarber-akamai requested review from vshanthe and yec-akamai and removed request for a team December 15, 2025 18:55
@lgarber-akamai lgarber-akamai added new-feature for new features in the changelog. project for new projects in the changelog. labels Dec 15, 2025
@lgarber-akamai lgarber-akamai marked this pull request as draft December 15, 2025 18:55
@lgarber-akamai lgarber-akamai changed the title New/volume limits expansion short term Add support for up to extended Block Storage assignment limits Dec 15, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 InstanceConfigDeviceMap struct 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",
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@lgarber-akamai lgarber-akamai changed the title Add support for up to extended Block Storage assignment limits project: Add support for Block Storage Volume Limit Increase Dec 15, 2025
@lgarber-akamai lgarber-akamai marked this pull request as ready for review December 15, 2025 19:03
Comment on lines +46 to +59
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"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 👍

@lgarber-akamai lgarber-akamai requested review from a team, ezilber-akamai and yec-akamai and removed request for a team December 15, 2025 19:05
Copy link
Collaborator

@jriddle-linode jriddle-linode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM works locally

@lgarber-akamai lgarber-akamai merged commit b374824 into linode:main Dec 15, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-feature for new features in the changelog. project for new projects in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants