Skip to content

Add instance storage discovery patterns in config#592

Merged
aramprice merged 2 commits into
cloudfoundry:ubuntu-jammyfrom
Ivaylogi98:ubuntu-jammy-add-instance-storage-patterns
Jun 11, 2026
Merged

Add instance storage discovery patterns in config#592
aramprice merged 2 commits into
cloudfoundry:ubuntu-jammyfrom
Ivaylogi98:ubuntu-jammy-add-instance-storage-patterns

Conversation

@Ivaylogi98

@Ivaylogi98 Ivaylogi98 commented May 4, 2026

Copy link
Copy Markdown
Contributor

Note: Should merge forward these changes to ubuntu-noble.

Summary

Adds two config fields to the AWS agent settings baked into the stemcell, required by
cloudfoundry/bosh-agent#407 (and subsequently by cloudfoundry/bosh-agent#396) to correctly discover NVMe instance storage on AWS Nitro
instances.

Background

On Nitro-based instances (e.g. m6id, i3en), NVMe PCIe enumeration order is
non-deterministic — /dev/nvme0n1 may be the root EBS volume, an attached EBS volume,
or instance storage depending on boot order. The bosh-agent now uses symlinks in
/dev/disk/by-id/ to identify and exclude EBS volumes, leaving only instance storage.

To keep the resolver IaaS-agnostic, the AWS-specific patterns are not hardcoded in the
agent binary — they are injected via agent config at stemcell build time.

Changes

stemcell_builder/stages/bosh_aws_agent_settings/apply.sh:

  • InstanceStorageDevicePattern: glob pattern matching all NVMe namespace devices
  • InstanceStorageManagedVolumePattern: glob pattern matching EBS volume symlinks in
    /dev/disk/by-id/, used to identify and exclude EBS volumes from instance storage
    discovery

Related

This repository uses a "Merge Forward" strategy

Changes should be made in the earliest applicable branch, and
merged forward through subsequent branches.

  1. Create a PR into the oldest branch (ubuntu-<short_name>)
  2. After this PR has been merged create a merge-to-<next_short_name> branch
  3. Merge ubuntu-<short_name> into merge-to-<next_short_name>
  4. Create a PR to merge merge-to-<next_short_name> into ubuntu-<next_short_name>
  5. Repeat as needed for subsequent branches

@coderabbitai

coderabbitai Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b828e812-74f2-4002-9e4b-9d855a3b6554

📥 Commits

Reviewing files that changed from the base of the PR and between 2f20fdd and fd038cb.

📒 Files selected for processing (1)
  • bosh-stemcell/spec/stemcells/aws_spec.rb

Walkthrough

The apply.sh emitter now adds two NVMe-related keys under Platform.Linux in the generated /var/vcap/bosh/agent.json: InstanceStorageDevicePattern set to /dev/nvmen1 and InstanceStorageManagedVolumePattern set to /dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_. The existing UseMonitIptablesFirewall setting remains. A new RSpec context parses agent.json and asserts those two values.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding instance storage discovery patterns to the AWS agent configuration, which is the primary focus of the changeset.
Description check ✅ Passed The pull request description is comprehensive and well-structured, covering summary, background, changes, and related items. It exceeds the template requirements by providing substantial context about the NVMe discovery problem and implementation approach.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@stemcell_builder/stages/bosh_aws_agent_settings/apply.sh`:
- Around line 15-16: The two config keys InstanceStorageDevicePattern and
InstanceStorageManagedVolumePattern are dead fields (not consumed by
bosh-agent); either remove them from the agent config output in apply.sh or
implement support in bosh-agent to consume them. To fix: if removing, delete the
two keys from the config generation in apply.sh and update any docs/tests; if
implementing, add parsing and use of
InstanceStorageDevicePattern/InstanceStorageManagedVolumePattern inside the
DevicePathResolver logic (respecting existing DevicePathResolutionType,
DIDTransformPattern and VolumeID matching semantics), add unit tests to
bosh-agent that verify matching/resolution against /dev/disk/by-id/ symlinks,
and update integration tests to exercise managed vs instance-storage resolution
paths.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 53804f69-c57a-4da5-8162-a0753a10bc96

📥 Commits

Reviewing files that changed from the base of the PR and between 265a297 and f8f6b3a.

📒 Files selected for processing (1)
  • stemcell_builder/stages/bosh_aws_agent_settings/apply.sh

Comment thread stemcell_builder/stages/bosh_aws_agent_settings/apply.sh
@github-project-automation github-project-automation Bot moved this from Inbox to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group May 4, 2026
@aramprice aramprice requested review from a team, Alphasite and fmoehler and removed request for a team May 7, 2026 14:43
@aramprice aramprice moved this from Waiting for Changes | Open for Contribution to Pending Merge | Prioritized in Foundational Infrastructure Working Group May 7, 2026
@beyhan beyhan moved this from Pending Merge | Prioritized to Pending Review | Discussion in Foundational Infrastructure Working Group May 21, 2026
@beyhan beyhan requested a review from neddp May 21, 2026 15:21
neddp
neddp previously approved these changes May 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds AWS-specific BOSH agent configuration needed for NVMe instance storage discovery on Nitro-based EC2 instances.

Changes:

  • Adds a glob for NVMe namespace devices.
  • Adds a glob for AWS EBS /dev/disk/by-id symlinks so managed volumes can be excluded from instance storage discovery.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread stemcell_builder/stages/bosh_aws_agent_settings/apply.sh
@aramprice

Copy link
Copy Markdown
Member

Double check that new agent config won't break agents that don't yet know how to consume it before merging.

@neddp

neddp commented Jun 1, 2026

Copy link
Copy Markdown
Member

Hi @aramprice,

The new config fields themselves are safe to add to the stemcell: agent.json is loaded via a plain json.Unmarshal into a Go struct (app/config.go#L30). Per the official Go docs (pkg.go.dev/encoding/json):

"When parsing a JSON object into a Go struct, unknown keys in the JSON object are ignored (unless a Decoder is used and Decoder.DisallowUnknownFields has been called)."

So an older agent binary without InstanceStorageDevicePattern / InstanceStorageManagedVolumePattern in its LinuxOptions will simply skip them - no error, no behaviour change.

Note that an old agent + new CPI on NVMe instances is not a new regression. The old CPI was already sending unreliable NVMe paths due to non-deterministic PCIe enumeration order - NVMe instance storage was already broken before this change set. A partial deployment (new CPI only, old agent) is no worse than the pre-existing broken state. The full fix requires new CPI + new agent + this stemcell config, all deployed together.

@aramprice aramprice requested a review from Copilot June 1, 2026 17:26
aramprice
aramprice previously approved these changes Jun 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@aramprice

Copy link
Copy Markdown
Member

/coderabbitai review

@Ivaylogi98 Ivaylogi98 dismissed stale reviews from aramprice and neddp via 2f20fdd June 9, 2026 11:59
@Ivaylogi98 Ivaylogi98 force-pushed the ubuntu-jammy-add-instance-storage-patterns branch from 2f20fdd to 430d6b3 Compare June 9, 2026 12:01
@Ivaylogi98 Ivaylogi98 force-pushed the ubuntu-jammy-add-instance-storage-patterns branch from 430d6b3 to fd038cb Compare June 9, 2026 12:03
@neddp

neddp commented Jun 9, 2026

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-project-automation github-project-automation Bot moved this from Pending Review | Discussion to Pending Merge | Prioritized in Foundational Infrastructure Working Group Jun 9, 2026
@aramprice aramprice requested a review from Copilot June 9, 2026 19:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@aramprice aramprice merged commit ef51888 into cloudfoundry:ubuntu-jammy Jun 11, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

5 participants