Fix OS detection in DirectorStemcellOwner to read from system files #2640
Conversation
The DirectorStemcellOwner class was using complex regex parsing of uname output and hardcoded version-to-codename mappings, which was unmaintainable and failed for modern Ubuntu versions like Jammy. This fix refactors the implementation to read OS information directly from system files: - Operating system name: /var/vcap/bosh/etc/operating_system (with fallback to /etc/os-release ID field) - Codename: /etc/os-release UBUNTU_CODENAME field This approach is more reliable, requires no maintenance for new Ubuntu releases, and fixes the director info endpoint which was returning "-" instead of "ubuntu-jammy" for the Director Stemcell OS on Jammy-based stemcells. **Before (`bosh env` output):** * `Director Stemcell -/1.943` **After (`bosh env` output):** * `Director Stemcell ubuntu-jammy/1.943`
cf-foundation-community-automation
bot
moved this to Inbox
in Foundational Infrastructure Working Group
aramprice
requested review from
a team,
selzoc and
ystros
and removed request for
a team
aramprice
moved this from Inbox
to Pending Review | Discussion
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
Pull request overview
This PR refactors the DirectorStemcellOwner class to improve OS detection reliability by reading from system files instead of parsing uname output. This fixes the director info endpoint which was returning "-" instead of "ubuntu-jammy" for modern Ubuntu versions like Jammy.
Key Changes
- Replaced regex parsing of
unameoutput with direct file system reads from/var/vcap/bosh/etc/operating_systemand/etc/os-release - Removed hardcoded version-to-codename mappings (14.04→trusty, 16.04→xenial, 18.04→bionic)
- Added support for newer Ubuntu versions (Jammy, Noble) without code changes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/bosh-director/lib/bosh/director/director_stemcell_owner.rb |
Replaced Etc.uname parsing with file-based OS detection using frozen path constants and new helper methods |
src/bosh-director/spec/unit/bosh/director/director_stemcell_owner_spec.rb |
Updated test suite to mock file system interactions and added coverage for Jammy/Noble Ubuntu versions plus edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| File.readlines(OS_RELEASE_FILE).each do |line| | ||
| if line =~ /^ID=(.+)$/ | ||
| return ::Regexp.last_match(1).strip.delete('"').downcase | ||
| end | ||
| end |
There was a problem hiding this comment.
The File.readlines call could raise exceptions (e.g., Errno::ENOENT, Errno::EACCES) if the file becomes inaccessible between the existence check and the read. Consider wrapping this in a rescue block to return nil on error, similar to how stemcell_version handles the file read.
| File.readlines(OS_RELEASE_FILE).each do |line| | ||
| if line =~ /^UBUNTU_CODENAME=(.+)$/ | ||
| return ::Regexp.last_match(1).strip.delete('"') | ||
| end | ||
| end |
There was a problem hiding this comment.
The File.readlines call could raise exceptions (e.g., Errno::ENOENT, Errno::EACCES) if the file becomes inaccessible between the existence check and the read. Consider wrapping this in a rescue block to return nil on error, similar to how stemcell_version handles the file read.
|
|
||
| def read_operating_system | ||
| if File.exist?(OPERATING_SYSTEM_FILE) | ||
| return File.read(OPERATING_SYSTEM_FILE).chomp.downcase |
There was a problem hiding this comment.
The File.read call could raise exceptions (e.g., Errno::ENOENT, Errno::EACCES) if the file becomes inaccessible between the existence check and the read. Consider wrapping this in a rescue block to return nil on error, similar to how stemcell_version handles the file read.
1 participant
What is this change about?
The DirectorStemcellOwner class was using regex parsing of uname output and hardcoded version-to-codename mappings, which was unmaintainable and failed for modern Ubuntu versions like Jammy.
This fix refactors the implementation to read OS information directly from system files:
This approach is more reliable, requires no maintenance for new Ubuntu releases, and fixes the director info endpoint which was returning "-" instead of "ubuntu-jammy" for the Director Stemcell OS on Jammy-based stemcells.
Before (
bosh envoutput):Director Stemcell -/1.943After (
bosh envoutput):Director Stemcell ubuntu-jammy/1.943What tests have you run against this PR?
bundle exec rake spec:unit:directorHow should this change be described in bosh release notes?
Fix parsing of Stemcell OS from system files
Does this PR introduce a breaking change?
No