From 65a80ea8b71027b000897bbcf9fa47276f686e5f Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Tue, 4 Sep 2018 16:25:00 -0700 Subject: [PATCH] (MODULES-7758) Properly handle distro tag for Fedora platforms There were some recent changes made to remove the 'f' prefix for our older Fedora platforms, specifically Fedora 26 and 27. However, this inadvertently changed our distro tag for these platforms to also remove the 'f' prefix so that e.g. fedoraf26 became fedora26. This change was not carried over to the puppet_agent module. Furthermore, newer Fedora platforms like Fedora 28 use the newer fc distro tag for the puppet5 collection. The puppet_agent module was also not updated with this change. Thus, some agent upgrades were failing on our Fedora platforms. This commit updates the puppet_agent module to handle all possible cases of the distro tag for our Fedora platforms. --- manifests/install.pp | 21 ++- .../puppet_agent_osfamily_redhat_spec.rb | 127 +++++++++++++++--- 2 files changed, 125 insertions(+), 23 deletions(-) diff --git a/manifests/install.pp b/manifests/install.pp index 84f462bbd..aae814e1a 100644 --- a/manifests/install.pp +++ b/manifests/install.pp @@ -165,7 +165,26 @@ # Workaround PUP-5802/PUP-5025 if ($::operatingsystem == 'Fedora') { if $pa_collection == 'PC1' or $pa_collection == 'puppet5' { - $dist_tag = "fedoraf${::operatingsystemmajrelease}" + # There's three cases here due to some mistakes with how we + # set-up our distro tags for Fedora platforms: + # * For newer Fedora platforms (e.g. Fedora 28), we want + # to use the fc tag + # + # * For older Fedora platforms (e.g. Fedora 26 and 27), we + # have two separate cases: + # * If the package version's > 5.5.3, then we use the fedora + # tag, b/c in those versions we removed the 'f' prefix. + # + # * If the package version's <= 5.5.3, then we use the fedoraf + # tag b/c the 'f' prefix is still there. + # + if (versioncmp("${::operatingsystemmajrelease}", '27') > 0) { + $dist_tag = "fc${::operatingsystemmajrelease}" + } elsif (versioncmp("${package_version}", '5.5.3') > 0) { + $dist_tag = "fedora${::operatingsystemmajrelease}" + } else { + $dist_tag = "fedoraf${::operatingsystemmajrelease}" + } } else { $dist_tag = "fc${::operatingsystemmajrelease}" } diff --git a/spec/classes/puppet_agent_osfamily_redhat_spec.rb b/spec/classes/puppet_agent_osfamily_redhat_spec.rb index 4bd4e2d12..e8733daca 100644 --- a/spec/classes/puppet_agent_osfamily_redhat_spec.rb +++ b/spec/classes/puppet_agent_osfamily_redhat_spec.rb @@ -2,23 +2,27 @@ describe 'puppet_agent' do # All FOSS and all Puppet 4+ upgrades require the package_version - package_version = '1.2.5' + package_version = '5.5.4' let(:params) { { :package_version => package_version } } - [['Fedora', 'fedora/f$releasever', 7], ['CentOS', 'el/$releasever', 7], ['Amazon', 'el/6', 6]].each do |os, urlbit, osmajor| + let(:facts) do + { + :osfamily => 'RedHat', + :architecture => 'x64', + :servername => 'master.example.vm', + :clientcert => 'foo.example.vm', + } + end + + [['Fedora', 'fedora/f$releasever', 27], ['CentOS', 'el/$releasever', 7], ['Amazon', 'el/6', 6]].each do |os, urlbit, osmajor| context "with #{os} and #{urlbit}" do - let(:facts) {{ - :osfamily => 'RedHat', - :operatingsystem => os, - :architecture => 'x64', - :servername => 'master.example.vm', - :clientcert => 'foo.example.vm', - :operatingsystemmajrelease => osmajor, - }} + let(:facts) do + super().merge(:operatingsystem => os, :operatingsystemmajrelease => osmajor) + end it { is_expected.to contain_exec('import-GPG-KEY-puppetlabs').with({ 'path' => '/bin:/usr/bin:/sbin:/usr/sbin', @@ -116,6 +120,87 @@ end end + # There are a lot of special cases here, so it is best to have a separate + # unit test context for them. + context 'distro tag on Fedora platforms' do + def sets_distro_tag_to(expected_distro_tag) + is_expected.to contain_package('puppet-agent').with_ensure( + "#{params[:package_version]}-1.#{expected_distro_tag}" + ) + end + + let(:facts) do + super().merge(:operatingsystem => 'Fedora') + end + + context 'when the collection is PC1' do + let(:params) do + # Older agents use the PC1 collection, so set the package_version + # to 1.10.9 to simulate this. + super().merge(:collection => 'PC1', :package_version => '1.10.9') + end + + let(:facts) do + # Older agents do not support anything past Fedora 27 + super().merge(:operatingsystemmajrelease => 27) + end + + it { sets_distro_tag_to('fedoraf27') } + end + + context 'when the collection is puppet5' do + let(:params) do + super().merge(:collection => 'puppet5') + end + + context 'on newer Fedora versions' do + let(:facts) do + super().merge(:operatingsystemmajrelease => 28) + end + + it { sets_distro_tag_to('fc28') } + end + + context 'on older Fedora versions' do + let(:facts) do + super().merge(:operatingsystemmajrelease => 27) + end + + shared_examples 'a package version' do |package_version, distro_tag| + let(:params) do + super().merge(:package_version => package_version) + end + + it { sets_distro_tag_to(distro_tag) } + end + + context 'when package_version > 5.5.3' do + include_examples 'a package version', '5.5.4', 'fedora27' + end + + context 'when package_version == 5.5.3' do + include_examples 'a package version', '5.5.3', 'fedoraf27' + end + + context 'when package_version < 5.5.3' do + include_examples 'a package version', '5.5.1', 'fedoraf27' + end + end + end + + context 'when the collection is newer than PC1 and puppet5' do + let(:params) do + super().merge(:collection => 'puppet6', :package_version => '6.0.0') + end + + let(:facts) do + super().merge(:operatingsystemmajrelease => 27) + end + + it { sets_distro_tag_to('fc27') } + end + end + [['RedHat', 'el-7-x86_64', 'el-7-x86_64', 7], ['Amazon', '', 'el-6-x64', 6]].each do |os, tag, repodir, osmajor| context "when PE on #{os}" do before(:each) do @@ -126,20 +211,18 @@ end Puppet::Parser::Functions.newfunction(:pe_compiling_server_aio_build, :type => :rvalue) do |args| - '1.2.5' + '5.5.4' end end - let(:facts) {{ - :osfamily => 'RedHat', - :operatingsystem => os, - :architecture => 'x64', - :servername => 'master.example.vm', - :clientcert => 'foo.example.vm', - :is_pe => true, - :platform_tag => tag, - :operatingsystemmajrelease => osmajor, - }} + let(:facts) do + super().merge( + :operatingsystem => os, + :operatingsystemmajrelease => osmajor, + :platform_tag => tag, + is_pe: true + ) + end context 'with manage_repo enabled' do let(:params) { @@ -207,7 +290,7 @@ :package_version => package_version } } - it { is_expected.to contain_package('puppet-agent').with_ensure("1.2.5-1.el#{osmajor}") } + it { is_expected.to contain_package('puppet-agent').with_ensure("#{params[:package_version]}-1.el#{osmajor}") } end