From 2e3c1a68a82935783f4275b353dc2c6bd0e34ba6 Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Wed, 23 Jan 2019 15:55:12 -0800 Subject: [PATCH 1/3] (maint) Centralize puppet_collection_for logic This commit centralizes the puppet_collection_for_puppet_agent_version and puppet_collection_for_puppet_version logic into a single puppet_collection_for helper. --- .../install_utils/puppet_utils.rb | 62 ++++++----- .../install_utils/puppet_utils_spec.rb | 101 +++++++++--------- 2 files changed, 89 insertions(+), 74 deletions(-) diff --git a/lib/beaker-puppet/install_utils/puppet_utils.rb b/lib/beaker-puppet/install_utils/puppet_utils.rb index 2e6238ae..0835ac42 100644 --- a/lib/beaker-puppet/install_utils/puppet_utils.rb +++ b/lib/beaker-puppet/install_utils/puppet_utils.rb @@ -90,41 +90,51 @@ def get_puppet_collection(agent_version = 'latest') collection end - # Determine the puppet collection that matches a given version of the puppet-agent - # package (you can find this version in the `aio_agent_version` fact). + # Determine the puppet collection that matches a given package version. The package + # must be one of + # * :puppet_agent (you can get this version from the `aio_agent_version_fact`) + # * :puppet # - # @param agent_version [String] a semver version number of the puppet-agent package, or the string 'latest' + # + # @param package [Symbol] the package name. must be one of :puppet_agent, :puppet + # @param version [String] a version number or the string 'latest' # @returns [String|nil] the name of the corresponding puppet collection, if any - def puppet_collection_for_puppet_agent_version(agent_version) - agent_version = agent_version.to_s - return 'puppet' if agent_version.strip == 'latest' + def puppet_collection_for(package, version) + valid_packages = [ + :puppet_agent, + :puppet + ] - x, y, z = agent_version.to_s.split('.').map(&:to_i) - return nil if x.nil? || y.nil? || z.nil? + unless valid_packages.include?(package) + raise "package must be one of #{valid_packages.join(', ')}" + end - return 'pc1' if x == 1 + case package + when :puppet_agent + agent_version = version.to_s + return 'puppet' if agent_version.strip == 'latest' - # A y version >= 99 indicates a pre-release version of the next x release - x += 1 if y >= 99 - "puppet#{x}" if x > 4 - end + x, y, z = agent_version.to_s.split('.').map(&:to_i) + return nil if x.nil? || y.nil? || z.nil? - # Determine the puppet collection that matches a given version of the puppet gem. - # - # @param version [String] a semver version number of the puppet gem, or the string 'latest' - # @returns [String|nil] the name of the corresponding puppet collection, if any - def puppet_collection_for_puppet_version(puppet_version) - puppet_version = puppet_version.to_s - return 'puppet' if puppet_version.strip == 'latest' + return 'pc1' if x == 1 + + # A y version >= 99 indicates a pre-release version of the next x release + x += 1 if y >= 99 + "puppet#{x}" if x > 4 + when :puppet + puppet_version = version.to_s + return 'puppet' if puppet_version.strip == 'latest' - x, y, z = puppet_version.to_s.split('.').map(&:to_i) - return nil if x.nil? || y.nil? || z.nil? + x, y, z = puppet_version.to_s.split('.').map(&:to_i) + return nil if x.nil? || y.nil? || z.nil? - return 'pc1' if x == 4 + return 'pc1' if x == 4 - # A y version >= 99 indicates a pre-release version of the next x release - x += 1 if y >= 99 - "puppet#{x}" if x > 4 + # A y version >= 99 indicates a pre-release version of the next x release + x += 1 if y >= 99 + "puppet#{x}" if x > 4 + end end # Report the version of puppet-agent installed on `host` diff --git a/spec/beaker-puppet/install_utils/puppet_utils_spec.rb b/spec/beaker-puppet/install_utils/puppet_utils_spec.rb index 2708e8cf..7cc4490d 100644 --- a/spec/beaker-puppet/install_utils/puppet_utils_spec.rb +++ b/spec/beaker-puppet/install_utils/puppet_utils_spec.rb @@ -137,64 +137,69 @@ def logger end - describe '#puppet_collection_for_puppet_agent_version' do - context 'given a valid version of puppet-agent' do - { - '1.10.14' => 'pc1', - '1.10.x' => 'pc1', - '5.3.1' => 'puppet5', - '5.3.x' => 'puppet5', - '5.99.0' => 'puppet6', - '6.1.99-foo' => 'puppet6', - '6.99.99' => 'puppet7', - '7.0.0' => 'puppet7', - }.each do |version, collection| - it "returns collection '#{collection}' for version '#{version}'" do - expect(subject.puppet_collection_for_puppet_agent_version(version)).to eq(collection) + describe '#puppet_collection_for' do + it 'raises an error when given an invalid package' do + expect { subject.puppet_collection_for(:foo, '5.5.4') }.to raise_error + end + + context 'when the :puppet_agent package is passed in' do + context 'given a valid version' do + { + '1.10.14' => 'pc1', + '1.10.x' => 'pc1', + '5.3.1' => 'puppet5', + '5.3.x' => 'puppet5', + '5.99.0' => 'puppet6', + '6.1.99-foo' => 'puppet6', + '6.99.99' => 'puppet7', + '7.0.0' => 'puppet7', + }.each do |version, collection| + it "returns collection '#{collection}' for version '#{version}'" do + expect(subject.puppet_collection_for(:puppet_agent, version)).to eq(collection) + end end end - end - - it "returns the default, latest puppet collection given the version 'latest'" do - expect(subject.puppet_collection_for_puppet_agent_version('latest')).to eq('puppet') - end - - - context 'given an invalid version of puppet-agent' do - [nil, '', '0.1.0', '3.8.1', '', 'not-semver', 'not.semver.either'].each do |version| - it "returns a nil collection value for version '#{version}'" do - expect(subject.puppet_collection_for_puppet_agent_version(version)).to be_nil + + it "returns the default, latest puppet collection given the version 'latest'" do + expect(subject.puppet_collection_for(:puppet_agent, 'latest')).to eq('puppet') + end + + context 'given an invalid version' do + [nil, '', '0.1.0', '3.8.1', '', 'not-semver', 'not.semver.either'].each do |version| + it "returns a nil collection value for version '#{version}'" do + expect(subject.puppet_collection_for(:puppet_agent, version)).to be_nil + end end end end - end - describe '#puppet_collection_for_puppet_version' do - context 'given a valid version of puppet' do - { - '4.9.0' => 'pc1', - '4.10.x' => 'pc1', - '5.3.1' => 'puppet5', - '5.3.x' => 'puppet5', - '5.99.0' => 'puppet6', - '6.1.99-foo' => 'puppet6', - '6.99.99' => 'puppet7', - '7.0.0' => 'puppet7', - }.each do |version, collection| - it "returns collection '#{collection}' for version '#{version}'" do - expect(subject.puppet_collection_for_puppet_version(version)).to eq(collection) + context 'when the :puppet package is passed-in' do + context 'given a valid version' do + { + '4.9.0' => 'pc1', + '4.10.x' => 'pc1', + '5.3.1' => 'puppet5', + '5.3.x' => 'puppet5', + '5.99.0' => 'puppet6', + '6.1.99-foo' => 'puppet6', + '6.99.99' => 'puppet7', + '7.0.0' => 'puppet7', + }.each do |version, collection| + it "returns collection '#{collection}' for version '#{version}'" do + expect(subject.puppet_collection_for(:puppet, version)).to eq(collection) + end end end - end - it "returns the default, latest puppet collection given the version 'latest'" do - expect(subject.puppet_collection_for_puppet_version('latest')).to eq('puppet') - end + it "returns the default, latest puppet collection given the version 'latest'" do + expect(subject.puppet_collection_for(:puppet, 'latest')).to eq('puppet') + end - context 'given an invalid version of puppet' do - [nil, '', '0.1.0', '3.8.1', '', 'not-semver', 'not.semver.either'].each do |version| - it "returns a nil collection value for version '#{version}'" do - expect(subject.puppet_collection_for_puppet_version(version)).to be_nil + context 'given an invalid version' do + [nil, '', '0.1.0', '3.8.1', '', 'not-semver', 'not.semver.either'].each do |version| + it "returns a nil collection value for version '#{version}'" do + expect(subject.puppet_collection_for(:puppet, version)).to be_nil + end end end end From d73f70b026506651583c0f618aadc22738d3156b Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Wed, 23 Jan 2019 18:07:55 -0800 Subject: [PATCH 2/3] (maint) Clean-up redundant code in puppet_collection_for helper --- .../install_utils/puppet_utils.rb | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/lib/beaker-puppet/install_utils/puppet_utils.rb b/lib/beaker-puppet/install_utils/puppet_utils.rb index 0835ac42..3473478b 100644 --- a/lib/beaker-puppet/install_utils/puppet_utils.rb +++ b/lib/beaker-puppet/install_utils/puppet_utils.rb @@ -110,26 +110,15 @@ def puppet_collection_for(package, version) end case package - when :puppet_agent - agent_version = version.to_s - return 'puppet' if agent_version.strip == 'latest' + when :puppet_agent, :puppet + version = version.to_s + return 'puppet' if version.strip == 'latest' - x, y, z = agent_version.to_s.split('.').map(&:to_i) + x, y, z = version.to_s.split('.').map(&:to_i) return nil if x.nil? || y.nil? || z.nil? - return 'pc1' if x == 1 - - # A y version >= 99 indicates a pre-release version of the next x release - x += 1 if y >= 99 - "puppet#{x}" if x > 4 - when :puppet - puppet_version = version.to_s - return 'puppet' if puppet_version.strip == 'latest' - - x, y, z = puppet_version.to_s.split('.').map(&:to_i) - return nil if x.nil? || y.nil? || z.nil? - - return 'pc1' if x == 4 + pc1_x = package == :puppet ? 4 : 1 + return 'pc1' if x == pc1_x # A y version >= 99 indicates a pre-release version of the next x release x += 1 if y >= 99 From 916441c83811320611894f548d2daf84f06e337e Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Thu, 24 Jan 2019 10:46:46 -0800 Subject: [PATCH 3/3] (maint) Remove puppet_collection_for_ references There were some dangling references to the previous puppet_collection_for_ methods that should be removed. --- lib/beaker-puppet/install_utils/foss_utils.rb | 2 +- lib/beaker-puppet/install_utils/puppet_utils.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/beaker-puppet/install_utils/foss_utils.rb b/lib/beaker-puppet/install_utils/foss_utils.rb index ba3d42bf..0a77747c 100644 --- a/lib/beaker-puppet/install_utils/foss_utils.rb +++ b/lib/beaker-puppet/install_utils/foss_utils.rb @@ -360,7 +360,7 @@ def install_puppet_on(hosts, opts = options) def install_puppet_agent_on(hosts, opts = {}) opts = FOSS_DEFAULT_DOWNLOAD_URLS.merge(opts) opts[:puppet_agent_version] ||= opts[:version] #backwards compatability with old parameter name - opts[:puppet_collection] ||= puppet_collection_for_puppet_agent_version(opts[:puppet_agent_version]) || 'pc1' + opts[:puppet_collection] ||= puppet_collection_for(:puppet_agent, opts[:puppet_agent_version]) || 'pc1' opts[:puppet_collection].downcase! # the collection names are case sensitive diff --git a/lib/beaker-puppet/install_utils/puppet_utils.rb b/lib/beaker-puppet/install_utils/puppet_utils.rb index 3473478b..5310131f 100644 --- a/lib/beaker-puppet/install_utils/puppet_utils.rb +++ b/lib/beaker-puppet/install_utils/puppet_utils.rb @@ -76,8 +76,7 @@ def remove_puppet_paths_on(hosts) # # @param [String] agent_version version string or 'latest' # @deprecated This method returns 'PC1' as the latest puppet collection; - # this is incorrect. Use {#puppet_collection_for_puppet_agent_version} or - # {#puppet_collection_for_puppet_version} instead. + # this is incorrect. Use {#puppet_collection_for} instead. def get_puppet_collection(agent_version = 'latest') collection = "PC1" if agent_version != 'latest'