Skip to content

Fix support for VolumeId dimension on nitro instances #1728

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

montaguethomas
Copy link

@montaguethomas montaguethomas commented Jun 15, 2025

Description of the issue

Linux allows mounting disks using a device alias (symlink) but the CWAgent is not able to resolve the EBS VolumeId for the device. Fixes #1727.

When PR #1156 implemented support to use ${aws:VolumeId} for a dimension value, it included extensive normalization of the device name:

func (c *cache) normalizeName(devName string) string {
normalized := c.fetchBlockName(devName)
if normalized == "" {
normalized = devName
}
// to match the disk device tag
return strings.ReplaceAll(normalized, "/dev/", "")
}
// find nvme block name by symlink, if symlink doesn't exist, return ""
func findNvmeBlockNameIfPresent(devName string) string {
// for nvme(ssd), there is a symlink from devName to nvme block name, i.e. /dev/xvda -> /dev/nvme0n1
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/nvme-ebs-volumes.html
hasRootFs := true
if _, err := os.Lstat("/rootfs/proc"); os.IsNotExist(err) {
hasRootFs = false
}
nvmeName := ""
if hasRootFs {
devName = "/rootfs" + devName
}
if info, err := os.Lstat(devName); err == nil {
if info.Mode()&os.ModeSymlink != 0 {
if path, err := filepath.EvalSymlinks(devName); err == nil {
nvmeName = path
}
}
}
if nvmeName != "" && hasRootFs {
nvmeName = strings.TrimPrefix(nvmeName, "/rootfs")
}
return nvmeName
}

This linux specific logic runs for all platforms, prevents the device names returned by the describeVolumesProvider (DescribeVolumes.Volumes.Attachments) from being included in the volume cache map. This normalization is not needed for the results of the linux hostProvider as it enumerates the devices registered in /sys/block/.

Description of changes

  • Move normalization of device name (trimming prefix /dev/) to describeVolumesProvider.DeviceToSerialMap method.
  • Remove normalization from volume provider.
  • Enables support for having multiple device names in volume cache with the same device serial (volumeId).

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

  • Updated and ran unit tests.
  • Built linux binary and tested on above system.

Requirements

Before commiting your code, please do the following steps.

  • Run make fmt and make fmt-sh
  • Run make lint

Integration Tests

To run integration tests against this PR, add the ready for testing label.

Purposely leaving support for a mixture of device names in volume cache.
By including both the EBS volume attachment device name and the linux
block device name, the processor is able lookup up the serial for either
xvdf or nvme1n1.
@montaguethomas montaguethomas requested a review from a team as a code owner June 15, 2025 02:49
@montaguethomas montaguethomas changed the title Bugfix: Fix support for VolumeId dimension on nitro instances Jun 15, 2025
Copy link
Contributor

This PR was marked stale due to lack of activity.

@github-actions github-actions bot added the Stale label Jun 28, 2025
@montaguethomas
Copy link
Author

This PR was marked stale due to lack of activity.

poke

@github-actions github-actions bot removed the Stale label Jun 29, 2025
Copy link
Contributor

github-actions bot commented Jul 7, 2025

This PR was marked stale due to lack of activity.

@github-actions github-actions bot added the Stale label Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CWAgent fails to resolve linux mount point device to EBS VolumeId on nitro instances
1 participant