Skip to content

Commit

Permalink
Do not query for the exact facter version
Browse files Browse the repository at this point in the history
This assumes that no exact match can be found in most cases. This saves
a call to FacterDB. Some testing on puppet-nginx this reduced the time
for a dry run by about 1 to 2 seconds, from about ~11 to ~9 seconds.

Implementation wise this switches to using Gem::Requirement to detect
matching versions. This means you can also specify `~> 3.8` as a valid
facterversion, or `>= 3 < 5`.
  • Loading branch information
ekohl committed Jul 6, 2023
1 parent 8d9370c commit e26ca6b
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 59 deletions.
73 changes: 53 additions & 20 deletions lib/rspec-puppet-facts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,32 +113,31 @@ def on_supported_os_implementation(opts = {})
end
end

facterversion_obj = Gem::Version.new(facterversion)
strict_requirement = RspecPuppetFacts::facter_version_to_strict_requirement(facterversion)

loose_requirement = RspecPuppetFacts::facter_version_to_loose_requirement(facterversion)

# FacterDB may have newer versions of facter data for which it contains a subset of all possible
# facter data (see FacterDB 0.5.2 for Facter releases 3.8 and 3.9). In this situation we need to
# cycle through and downgrade Facter versions per platform type until we find matching Facter data.
filter.each do |filter_spec|
facter_version_filter = RspecPuppetFacts.facter_version_to_filter(facterversion)
db = FacterDB.get_facts(filter_spec.merge({ :facterversion => facter_version_filter }))

if db.empty?
if RspecPuppetFacts.spec_facts_strict?
raise ArgumentError, "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, aborting"
end
versions = FacterDB.get_facts(filter_spec).map { |facts| Gem::Version.new(facts[:facterversion]) }.sort.reverse
next unless versions.any?

version = FacterDB.get_facts(filter_spec).map { |facts| Gem::Version.new(facts[:facterversion]) }.sort.reverse.detect { |v| v <= facterversion_obj }
version = versions.detect { |v| strict_requirement =~ v }

unless version
version = versions.detect { |v| loose_requirement =~ v } if loose_requirement
next unless version
version = version.to_s
facter_version_filter = "/\\A#{Regexp.escape(version)}/"

unless version == facterversion
RspecPuppetFacts.warning "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, using v#{version} instead"
if RspecPuppetFacts.spec_facts_strict?
raise ArgumentError, "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, aborting"
end

RspecPuppetFacts.warning "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, using v#{version} instead"
end

filter_spec[:facterversion] = facter_version_filter
filter_spec[:facterversion] = "/\\A#{Regexp.escape(version.to_s)}\\Z/"
end

received_facts = FacterDB::get_facts(filter)
Expand Down Expand Up @@ -353,13 +352,47 @@ def self.reset
@metadata = nil
end

# Generates a JGrep statement expression for a specific facter version
# @return [String] JGrep statement expression
# @param version [String] the Facter version
# Construct the strict facter version requirement
# @return [Gem::Requirement] The version requirement to match
# @api private
def self.facter_version_to_filter(version)
major, minor = version.split('.')
"/\\A#{major}\\.#{minor}\\./"
def self.facter_version_to_strict_requirement(version)
Gem::Requirement.new(facter_version_to_strict_requirement_string(version))
end

# Construct the strict facter version requirement string
# @return [String] The version requirement to match
# @api private
def self.facter_version_to_strict_requirement_string(version)
if /\A[0-9]+(\.[0-9]+)*\Z/.match?(version)
# Interpret 3 as ~> 3.0
"~> #{version}.0"
else
version
end
end

# Construct the loose facter version requirement
# @return [Optional[Gem::Requirement]] The version requirement to match
# @api private
def self.facter_version_to_loose_requirement(version)
string = facter_version_to_loose_requirement_string(version)
Gem::Requirement.new(string) if string
end

# Construct the facter version requirement string
# @return [String] The version requirement to match
# @api private
def self.facter_version_to_loose_requirement_string(version)
if (m = /\A(?<major>[0-9]+)\.(?<minor>[0-9]+)(?:\.(?<patch>[0-9]+))?\Z/.match(version))
# Interpret 3.1 as < 3.2 and 3.2.1 as < 3.3
"< #{m[:major]}.#{m[:minor].to_i + 1}"
elsif /\A[0-9]+\Z/.match?(version)
# Interpret 3 as < 4
"< #{version.to_i + 1}"
else
# This would be the same as the strict requirement
nil
end
end

def self.facter_version_for_puppet_version(puppet_version)
Expand Down
153 changes: 114 additions & 39 deletions spec/rspec_puppet_facts_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@
end

before do
RSpec.configuration.default_facter_version = '3.1.0'
RSpec.configuration.default_facter_version = '3.1.6'
end

after do
Expand Down Expand Up @@ -721,11 +721,10 @@
end


it 'returns facts from a facter version matching future and below' do
major, minor = Facter.version.split('.')
it 'returns facts from a facter version matching version and below' do
is_expected.to match(
'centos-7-x86_64' => include(
:facterversion => /\A#{major}\.[#{minor}#{minor.to_i + 1}]\./,
:facterversion => /\A2\.[0-6]\./,
),
)
end
Expand Down Expand Up @@ -856,8 +855,8 @@
before do
allow(FacterDB).to receive(:get_facts).and_call_original
allow(FacterDB).to receive(:get_facts).with(
a_hash_including(facterversion: "/\\A3\\.9\\./", operatingsystem: 'CentOS'),
).and_return([])
{:operatingsystem=>"CentOS", :operatingsystemrelease=>"/^7/", :hardwaremodel=>"x86_64"}
).and_wrap_original { |m, *args| m.call(*args).reject { |facts| facts[:facterversion].start_with?('3.9.') } }
end

it 'returns CentOS facts from a facter version matching 3.8' do
Expand Down Expand Up @@ -976,69 +975,145 @@ module MCollectiveStub # rubocop:todo Lint/ConstantDefinitionInBlock
end
end

describe '.facter_version_to_filter' do
describe '.facter_version_to_strict_requirement' do
subject { described_class.facter_version_to_strict_requirement(version) }

context 'when passed a version that is a complex requirement' do
let(:version) { '~> 2.4' }

it { is_expected.to be_instance_of(Gem::Requirement) }
end

context 'when passed a version that is major' do
let(:version) { '1' }

it { is_expected.to be_instance_of(Gem::Requirement) }
end
end

describe '.facter_version_to_strict_requirement_string' do
subject { described_class.facter_version_to_strict_requirement_string(version) }

context 'when passed a version that is a complex requirement' do
let(:version) { '~> 2.4' }

it { is_expected.to eq('~> 2.4') }
end

context 'when passed a version that is major' do
let(:version) { '1' }

it { is_expected.to eq('~> 1.0') }
end

context 'when passed a version that is major.minor' do
let(:version) { '1.2' }

it { is_expected.to eq('~> 1.2.0') }
end

context 'when passed a version that is major.minor.patch' do
let(:version) { '1.2.3' }

it { is_expected.to eq('~> 1.2.3.0') }
end
end

describe '.facter_version_to_loose_requirement' do
subject { described_class.facter_version_to_loose_requirement(version) }

context 'when passed a version that is a complex requirement' do
let(:version) { '~> 2.4' }

it { is_expected.to be_nil }
end

context 'when passed a version that is major' do
let(:version) { '1' }

it { is_expected.to be_instance_of(Gem::Requirement) }
end
end

describe '.facter_version_to_loose_requirement_string' do
subject { described_class.facter_version_to_loose_requirement_string(version) }

context 'when passed a version that is a complex requirement (1)' do
let(:version) { '~> 2.4' }

it { is_expected.to be_nil }
end

context 'when passed a version that is a complex requirement (2)' do
let(:version) { '>= 3 < 5' }

it { is_expected.to be_nil }
end

context 'when passed a version that is major (1)' do
let(:version) { '1' }

it { is_expected.to eq('< 2') }
end

context 'when passed a version that is major (2)' do
let(:version) { '9' }

it { is_expected.to eq('< 10') }
end

context 'when passed a version that is major (3)' do
let(:version) { '10' }

it { is_expected.to eq('< 11') }
end

context 'when passed a version that is major.minor (1)' do
subject { described_class.facter_version_to_filter('1.2') }
let(:version) { '1.2' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A1\.2\./')
end
it { is_expected.to eq('< 1.3') }
end

context 'when passed a version that is major.minor (2)' do
subject { described_class.facter_version_to_filter('10.2') }
let(:version) { '10.2' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A10\.2\./')
end
it { is_expected.to eq('< 10.3') }
end

context 'when passed a version that is major.minor (3)' do
subject { described_class.facter_version_to_filter('1.20') }
let(:version) { '1.20' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A1\.20\./')
end
it { is_expected.to eq('< 1.21') }
end

context 'when passed a version that is major.minor (4)' do
subject { described_class.facter_version_to_filter('10.20') }
let(:version) { '10.20' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A10\.20\./')
end
it { is_expected.to eq('< 10.21') }
end

context 'when passed a version that is major.minor.patch (1)' do
subject { described_class.facter_version_to_filter('1.2.3') }
let(:version) { '1.2.3' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A1\.2\./')
end
it { is_expected.to eq('< 1.3') }
end

context 'when passed a version that is major.minor.patch (2)' do
subject { described_class.facter_version_to_filter('10.2.3') }
let(:version) { '10.2.3' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A10\.2\./')
end
it { is_expected.to eq('< 10.3') }
end

context 'when passed a version that is major.minor.patch (3)' do
subject { described_class.facter_version_to_filter('1.20.3') }
let(:version) { '1.20.3' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A1\.20\./')
end
it { is_expected.to eq('< 1.21') }
end

context 'when passed a version that is major.minor.patch (4)' do
subject { described_class.facter_version_to_filter('10.20.3') }
let(:version) { '10.20.3' }

it 'returns the correct JGrep statement expression' do
is_expected.to eq('/\A10\.20\./')
end
it { is_expected.to eq('< 10.21') }
end
end
end

0 comments on commit e26ca6b

Please sign in to comment.