Skip to content

Fix OS detection in DirectorStemcellOwner to read from system files #2640

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
+126 −45
Open

Fix OS detection in DirectorStemcellOwner to read from system files #2640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 40 additions & 21 deletions src/bosh-director/lib/bosh/director/director_stemcell_owner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,58 @@
require 'etc'

module Bosh::Director
class DirectorStemcellOwner
OS_RELEASE_FILE = '/etc/os-release'.freeze
OPERATING_SYSTEM_FILE = '/var/vcap/bosh/etc/operating_system'.freeze
STEMCELL_VERSION_FILE = '/var/vcap/bosh/etc/stemcell_version'.freeze

def stemcell_os
@stemcell_os ||= os_and_version
end

def stemcell_version
return @stemcell_version unless @stemcell_version.nil?

stemcell_version_path = '/var/vcap/bosh/etc/stemcell_version'
return '-' unless File.exist?(stemcell_version_path)
return '-' unless File.exist?(STEMCELL_VERSION_FILE)

@stemcell_version = File.read(stemcell_version_path).chomp
@stemcell_version = File.read(STEMCELL_VERSION_FILE).chomp
end

private

def os_and_version
results = Etc.uname[:version].scan(/~([^ ]*)-([^ ]*) .*$/)[0]
return '-' if Array(results).empty?

os = results[1].downcase
version_number = results[0]
version_name = if version_number.start_with?('16.')
'xenial'
elsif version_number.start_with?('14.')
'trusty'
elsif version_number.start_with?('18.')
'bionic'
else
version_number
end

"#{os}-#{version_name}"
os = read_operating_system
codename = read_codename

return '-' if os.nil? || codename.nil?

"#{os}-#{codename}"
end

def read_operating_system
if File.exist?(OPERATING_SYSTEM_FILE)
return File.read(OPERATING_SYSTEM_FILE).chomp.downcase
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
end

return nil unless File.exist?(OS_RELEASE_FILE)

File.readlines(OS_RELEASE_FILE).each do |line|
if line =~ /^ID=(.+)$/
return ::Regexp.last_match(1).strip.delete('"').downcase
end
end
Comment on lines +37 to +41
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

nil
end

def read_codename
return nil unless File.exist?(OS_RELEASE_FILE)

File.readlines(OS_RELEASE_FILE).each do |line|
if line =~ /^UBUNTU_CODENAME=(.+)$/
return ::Regexp.last_match(1).strip.delete('"')
end
end
Comment on lines +49 to +53
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

nil
end
end
end
110 changes: 86 additions & 24 deletions src/bosh-director/spec/unit/bosh/director/director_stemcell_owner_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,48 +5,107 @@ module Bosh::Director
subject { DirectorStemcellOwner.new }

before do
allow(Etc).to receive(:uname).and_return({ version: version_string })
allow(File).to receive(:exist?).with('/var/vcap/bosh/etc/operating_system').and_return(operating_system_exists)
allow(File).to receive(:read).with('/var/vcap/bosh/etc/operating_system').and_return(operating_system_content) if operating_system_exists
allow(File).to receive(:exist?).with('/etc/os-release').and_return(os_release_exists)
allow(File).to receive(:readlines).with('/etc/os-release').and_return(os_release_content) if os_release_exists
end

let(:version_string) { '#35~16.04.1-Ubuntu SMP Fri Aug 10 21:54:34 UTC 2018' }
let(:operating_system_exists) { false }
let(:operating_system_content) { '' }
let(:os_release_exists) { false }
let(:os_release_content) { [] }

describe '#stemcell_os' do
context 'trusty' do
let(:version_string) { '#35~14.04.1-Ubuntu SMP Fri Aug 10 21:54:34 UTC 2018' }

it 'should be ubuntu-trusty' do
expect(subject.stemcell_os).to eq('ubuntu-trusty')
context 'when operating_system file exists' do
let(:operating_system_exists) { true }
let(:operating_system_content) { "ubuntu\n" }
let(:os_release_exists) { true }

context 'jammy' do
let(:os_release_content) do
<<~OS_RELEASE.lines
NAME="Ubuntu"
VERSION="22.04.1 LTS (Jammy Jellyfish)"
ID=ubuntu
UBUNTU_CODENAME=jammy
OS_RELEASE
end

it 'should read os from operating_system file and codename from os-release' do
expect(subject.stemcell_os).to eq('ubuntu-jammy')
end
end
end

context 'xenial' do
let(:version_string) { '#35~16.04.1-Ubuntu SMP Fri Aug 10 21:54:34 UTC 2018' }

it 'should be ubuntu-xenial' do
expect(subject.stemcell_os).to eq('ubuntu-xenial')
context 'noble' do
let(:os_release_content) do
<<~OS_RELEASE.lines
NAME="Ubuntu"
VERSION="24.04 LTS (Noble Numbat)"
ID=ubuntu
UBUNTU_CODENAME=noble
OS_RELEASE
end

it 'should read os from operating_system file and codename from os-release' do
expect(subject.stemcell_os).to eq('ubuntu-noble')
end
end
end

context 'bionic' do
let(:version_string) { '#35~18.04.1-Ubuntu SMP Fri Aug 10 21:54:34 UTC 2018' }
context 'when operating_system file does not exist but os-release does' do
let(:operating_system_exists) { false }
let(:os_release_exists) { true }

context 'fallback to os-release for both os and codename' do
let(:os_release_content) do
<<~OS_RELEASE.lines
NAME="Ubuntu"
VERSION="22.04 LTS (Jammy Jellyfish)"
ID=ubuntu
UBUNTU_CODENAME=jammy
OS_RELEASE
end

it 'should read both from os-release file' do
expect(subject.stemcell_os).to eq('ubuntu-jammy')
end
end

it 'should be ubuntu-bionic' do
expect(subject.stemcell_os).to eq('ubuntu-bionic')
context 'os-release with bionic' do
let(:os_release_content) do
<<~OS_RELEASE.lines
ID=ubuntu
UBUNTU_CODENAME=bionic
OS_RELEASE
end

it 'should be ubuntu-bionic' do
expect(subject.stemcell_os).to eq('ubuntu-bionic')
end
end
end

context 'other ubuntu' do
let(:version_string) { '#35~12.04.1-Ubuntu SMP Fri Aug 10 21:54:34 UTC 2018' }
context 'when neither file exists' do
let(:operating_system_exists) { false }
let(:os_release_exists) { false }

it 'should be the raw number' do
expect(subject.stemcell_os).to eq('ubuntu-12.04.1')
it 'should return dash' do
expect(subject.stemcell_os).to eq('-')
end
end

context 'other' do
let(:version_string) { 'some random version string' }
context 'when os-release exists but has no codename' do
let(:operating_system_exists) { false }
let(:os_release_exists) { true }
let(:os_release_content) do
<<~OS_RELEASE.lines
ID=ubuntu
VERSION_ID="22.04"
OS_RELEASE
end

it 'should be a dash' do
it 'should return dash when codename is missing' do
expect(subject.stemcell_os).to eq('-')
end
end
Expand All @@ -57,6 +116,7 @@ module Bosh::Director
before do
allow(File).to receive(:read).with('/var/vcap/bosh/etc/stemcell_version').and_return("123.45\n")
allow(File).to receive(:exist?).with('/var/vcap/bosh/etc/stemcell_version').and_return(true)
allow(File).to receive(:exist?).with('/etc/os-release').and_call_original
end

it 'returns the stemcell_version specified in the config' do
Expand All @@ -67,6 +127,7 @@ module Bosh::Director
context 'there is no file' do
before do
allow(File).to receive(:exist?).with('/var/vcap/bosh/etc/stemcell_version').and_return(false)
allow(File).to receive(:exist?).with('/etc/os-release').and_call_original
end

it 'returns -' do
Expand All @@ -78,6 +139,7 @@ module Bosh::Director
before do
allow(File).to receive(:read).with('/var/vcap/bosh/etc/stemcell_version').and_return("123.45\n")
allow(File).to receive(:exist?).with('/var/vcap/bosh/etc/stemcell_version').and_return(true)
allow(File).to receive(:exist?).with('/etc/os-release').and_call_original
end

it 'returns the stemcell_version specified in the config' do
Expand Down
Loading