From ca31cab98756aa2ed40cc4dc717ab6bbdb3e2afe Mon Sep 17 00:00:00 2001 From: tphoney Date: Tue, 19 Jun 2018 11:49:12 +0100 Subject: [PATCH] (maint) make apt testing more stable, cleanup --- manifests/init.pp | 14 +---- manifests/params.pp | 27 +--------- manifests/source.pp | 9 ---- metadata.json | 3 +- .../{class_spec.rb => 01_apt_class_spec.rb} | 0 spec/acceptance/apt_key_provider_spec.rb | 29 +++++------ spec/acceptance/apt_spec.rb | 30 ++++------- spec/defines/ppa_spec.rb | 40 -------------- spec/spec_helper_acceptance.rb | 52 +++---------------- 9 files changed, 34 insertions(+), 170 deletions(-) rename spec/acceptance/{class_spec.rb => 01_apt_class_spec.rb} (100%) diff --git a/manifests/init.pp b/manifests/init.pp index aa61986..e9a7e53 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -184,17 +184,5 @@ class apt ( } # required for adding GPG keys on Debian 9 (and derivatives) - case $facts['os']['name'] { - 'Debian': { - if versioncmp($facts['os']['release']['major'], '9') >= 0 { - ensure_packages(['dirmngr']) - } - } - 'Ubuntu': { - if versioncmp($facts['os']['release']['full'], '17.04') >= 0 { - ensure_packages(['dirmngr']) - } - } - default: { } - } + ensure_packages(['dirmngr']) } diff --git a/manifests/params.pp b/manifests/params.pp index c11deba..c954aa3 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -74,19 +74,13 @@ class apt::params { case $facts['os']['name']{ 'Debian': { - case $facts['os']['release']['full'] { - default: { $backports = { 'location' => 'http://deb.debian.org/debian', 'key' => 'A1BD8E9D78F7FE5C3E65D8AF8B48AD6246925553', 'repos' => 'main contrib non-free', } - } - } - $ppa_options = undef $ppa_package = undef - } 'Ubuntu': { $backports = { @@ -94,25 +88,8 @@ class apt::params { 'key' => '630239CC130E1A7FD81A27B140976EAF437D05B5', 'repos' => 'main universe multiverse restricted', } - - case $facts['os']['release']['full'] { - '10.04': { - $ppa_options = undef - $ppa_package = 'python-software-properties' - } - '12.04': { - $ppa_options = '-y' - $ppa_package = 'python-software-properties' - } - '14.04', '14.10', '15.04', '15.10', '16.04': { - $ppa_options = '-y' - $ppa_package = 'software-properties-common' - } - default: { - $ppa_options = '-y' - $ppa_package = 'python-software-properties' - } - } + $ppa_options = '-y' + $ppa_package = 'software-properties-common' } undef: { fail('Unable to determine value for fact os["name"]') diff --git a/manifests/source.pp b/manifests/source.pp index 670b830..afbb27a 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -14,7 +14,6 @@ define apt::source( Boolean $notify_update = true, ) { - # This is needed for compat with 1.8.x include ::apt $_before = Apt::Setting["list-${title}"] @@ -29,17 +28,9 @@ define apt::source( $_release = $release } - # Some releases do not support https transport with default installation - $_transport_https_releases = [ 'wheezy', 'jessie', 'stretch', 'trusty', 'xenial' ] - if $ensure == 'present' { if ! $location { fail('cannot create a source entry without specifying a location') - } elsif $_release in $_transport_https_releases { - $method = split($location, '[:\/]+')[0] - if $method == 'https' { - ensure_packages('apt-transport-https') - } } } diff --git a/metadata.json b/metadata.json index 15f6b79..11b2b02 100644 --- a/metadata.json +++ b/metadata.json @@ -17,7 +17,8 @@ { "operatingsystem": "Debian", "operatingsystemrelease": [ - "8" + "8", + "9" ] }, { diff --git a/spec/acceptance/class_spec.rb b/spec/acceptance/01_apt_class_spec.rb similarity index 100% rename from spec/acceptance/class_spec.rb rename to spec/acceptance/01_apt_class_spec.rb diff --git a/spec/acceptance/apt_key_provider_spec.rb b/spec/acceptance/apt_key_provider_spec.rb index 08bf61f..753eff8 100644 --- a/spec/acceptance/apt_key_provider_spec.rb +++ b/spec/acceptance/apt_key_provider_spec.rb @@ -10,17 +10,11 @@ CENTOS_GPG_KEY_LONG_ID = '0946FCA2C105B9DE'.freeze CENTOS_GPG_KEY_FINGERPRINT = 'C1DAC52D1664E8A4386DBA430946FCA2C105B9DE'.freeze CENTOS_REPO_URL = 'ftp.cvut.cz/centos'.freeze CENTOS_GPG_KEY_FILE = 'RPM-GPG-KEY-CentOS-6'.freeze - SHOULD_NEVER_EXIST_ID = 'EF8D349F'.freeze - KEY_CHECK_COMMAND = 'apt-key adv --list-keys --with-colons --fingerprint | grep '.freeze PUPPETLABS_KEY_CHECK_COMMAND = "#{KEY_CHECK_COMMAND} #{PUPPETLABS_GPG_KEY_FINGERPRINT}".freeze CENTOS_KEY_CHECK_COMMAND = "#{KEY_CHECK_COMMAND} #{CENTOS_GPG_KEY_FINGERPRINT}".freeze -MAX_TIMEOUT_RETRY = 3 -TIMEOUT_RETRY_WAIT = 5 -TIMEOUT_ERROR_MATCHER = %r{no valid OpenPGP data found} - def populate_default_options_pp(value) default_options_pp = <<-MANIFEST apt_key { 'puppetlabs': @@ -32,15 +26,18 @@ def populate_default_options_pp(value) end def install_key(key) - retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do - shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \ - --recv-keys #{key}") + retry_on_error_matching do + shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net --recv-keys #{key}") end end def apply_manifest_twice(manifest_pp) - apply_manifest(manifest_pp, catch_failures: true) - apply_manifest(manifest_pp, catch_changes: true) + retry_on_error_matching do + apply_manifest(manifest_pp, catch_failures: true) + end + retry_on_error_matching do + apply_manifest(manifest_pp, catch_changes: true) + end end invalid_key_length_pp = <<-MANIFEST @@ -612,7 +609,7 @@ describe 'apt_key' do end end - context 'when absent, added with long key', unless: (fact('operatingsystem') == 'Debian' && fact('operatingsystemmajrelease') == '6') do + context 'when absent, added with long key' do it 'is removed' do # Install the key first (retry because key pool may timeout) install_key(PUPPETLABS_GPG_KEY_LONG_ID) @@ -630,7 +627,7 @@ describe 'apt_key' do context 'with puppetlabs gpg key' do it 'works' do # Apply the manifest (Retry if timeout error is received from key pool) - retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do + retry_on_error_matching do apply_manifest(gpg_key_pp, catch_failures: true) end @@ -659,7 +656,7 @@ describe 'apt_key' do context 'with hkps.pool.sks-keyservers.net' do it 'works' do # Apply the manifest (Retry if timeout error is received from key pool) - retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do + retry_on_error_matching do apply_manifest(hkps_pool_pp, catch_failures: true) end @@ -670,7 +667,7 @@ describe 'apt_key' do context 'with hkp://hkps.pool.sks-keyservers.net:80' do it 'works' do - retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do + retry_on_error_matching do apply_manifest(hkp_pool_pp, catch_failures: true) end @@ -682,7 +679,7 @@ describe 'apt_key' do context 'with nonexistant.key.server' do it 'fails' do apply_manifest(nonexistant_key_server_pp, expect_failures: true) do |r| - expect(r.stderr).to match(%r{(Host not found|Couldn't resolve host)}) + expect(r.stderr).to match(%r{(Host not found|Couldn't resolve host|No name)}) end end end diff --git a/spec/acceptance/apt_spec.rb b/spec/acceptance/apt_spec.rb index 5505748..7fe1d03 100644 --- a/spec/acceptance/apt_spec.rb +++ b/spec/acceptance/apt_spec.rb @@ -1,24 +1,16 @@ require 'spec_helper_acceptance' -MAX_TIMEOUT_RETRY = 3 -TIMEOUT_RETRY_WAIT = 5 -TIMEOUT_ERROR_MATCHER = %r{no valid OpenPGP data found} - everything_everything_pp = <<-MANIFEST - if $::lsbdistcodename == 'lucid' { - $sources = undef - } else { - $sources = { - 'puppetlabs' => { - 'ensure' => present, - 'location' => 'http://apt.puppetlabs.com', - 'repos' => 'main', - 'key' => { - 'id' => '6F6B15509CF8E59E6E469F327F438280EF8D349F', - 'server' => 'hkps.pool.sks-keyservers.net', - }, + $sources = { + 'puppetlabs' => { + 'ensure' => present, + 'location' => 'http://apt.puppetlabs.com', + 'repos' => 'main', + 'key' => { + 'id' => '6F6B15509CF8E59E6E469F327F438280EF8D349F', + 'server' => 'pool.sks-keyservers.net', }, - } + }, } class { 'apt': update => { @@ -46,11 +38,9 @@ describe 'apt class' do context 'with all the things' do it 'works with no errors' do # Apply the manifest (Retry if timeout error is received from key pool) - retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do + retry_on_error_matching do apply_manifest(everything_everything_pp, catch_failures: true) end - - apply_manifest(everything_everything_pp, catch_failures: true) end it 'stills work' do shell('apt-get update') diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index 365d520..4978221 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -91,46 +91,6 @@ describe 'apt::ppa' do } end - describe 'package_manage => true, multiple ppas, MODULES-2873' do - let :pre_condition do - 'class { "apt": } - apt::ppa {"ppa:user/foo": - package_manage => true - }' - end - let :facts do - { - os: { family: 'Debian', name: 'Ubuntu', release: { major: '11', full: '11.04' } }, - lsbdistrelease: '11.04', - lsbdistcodename: 'natty', - operatingsystem: 'Ubuntu', - osfamily: 'Debian', - lsbdistid: 'Ubuntu', - puppetversion: Puppet.version, - } - end - let :params do - { - package_manage: true, - } - end - - let(:title) { 'ppa:user/bar' } - - it { is_expected.to contain_package('python-software-properties') } - it { - is_expected.to contain_exec('add-apt-repository-ppa:user/bar').that_notifies('Class[Apt::Update]').with('environment' => [], - 'command' => '/usr/bin/add-apt-repository -y ppa:user/bar', - 'unless' => '/usr/bin/test -f /etc/apt/sources.list.d/user-bar-natty.list', - 'user' => 'root', - 'logoutput' => 'on_failure') - } - - it { - is_expected.to contain_file('/etc/apt/sources.list.d/user-bar-natty.list').that_requires('Exec[add-apt-repository-ppa:user/bar]').with('ensure' => 'file') - } - end - describe 'package_manage => false' do let :pre_condition do 'class { "apt": }' diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index 258116c..763f03b 100644 --- a/spec/spec_helper_acceptance.rb +++ b/spec/spec_helper_acceptance.rb @@ -1,14 +1,7 @@ require 'beaker-rspec' require 'beaker/puppet_install_helper' require 'beaker/module_install_helper' - -def install_bolt_on(hosts) - on(hosts, "/opt/puppetlabs/puppet/bin/gem install --source http://rubygems.delivery.puppetlabs.net bolt -v '> 0.0.1'", acceptable_exit_codes: [0, 1]).stdout -end - -def pe_install? - ENV['PUPPET_INSTALL_TYPE'] =~ %r{pe}i -end +require 'beaker-task_helper' run_puppet_install_helper install_bolt_on(hosts) unless pe_install? @@ -16,43 +9,9 @@ install_module_on(hosts) install_module_dependencies_on(hosts) UNSUPPORTED_PLATFORMS = ['RedHat', 'Suse', 'windows', 'AIX', 'Solaris'].freeze - -DEFAULT_PASSWORD = if default[:hypervisor] == 'vagrant' - 'vagrant' - elsif default[:hypervisor] == 'vcloud' - 'Qu@lity!' - end - -def puppet_version - (on default, puppet('--version')).output.chomp -end - -def run_puppet_access_login(user:, password: - '~!@#$%^*-/ aZ', lifetime: '5y') - on(master, puppet('access', 'login', '--username', user, '--lifetime', lifetime), stdin: password) -end - -def run_task(task_name:, params: nil, password: DEFAULT_PASSWORD) - if pe_install? - run_puppet_task(task_name: task_name, params: params) - else - run_bolt_task(task_name: task_name, params: params, password: password) - end -end - -def run_bolt_task(task_name:, params: nil, password: DEFAULT_PASSWORD) - on(master, "/opt/puppetlabs/puppet/bin/bolt task run #{task_name} --modules /etc/puppetlabs/code/modules/service --nodes localhost --password #{password} #{params}", acceptable_exit_codes: [0, 1]).stdout # rubocop:disable Metrics/LineLength -end - -def run_puppet_task(task_name:, params: nil) - on(master, puppet('task', 'run', task_name, '--nodes', fact_on(master, 'fqdn'), params.to_s), acceptable_exit_codes: [0, 1]).stdout -end - -def expect_multiple_regexes(result:, regexes:) - regexes.each do |regex| - expect(result).to match(regex) - end -end +MAX_RETRY_COUNT = 12 +RETRY_WAIT = 10 +ERROR_MATCHER = %r{(no valid OpenPGP data found|keyserver timed out|keyserver receive failed)} # This method allows a block to be passed in and if an exception is raised # that matches the 'error_matcher' matcher, the block will wait a set number @@ -65,9 +24,10 @@ end # retry_on_error_matching(3, 5, /OpenGPG Error/) do # apply_manifest(pp, :catch_failures => true) # end -def retry_on_error_matching(max_retry_count = 3, retry_wait_interval_secs = 5, error_matcher = nil) +def retry_on_error_matching(max_retry_count = MAX_RETRY_COUNT, retry_wait_interval_secs = RETRY_WAIT, error_matcher = ERROR_MATCHER) try = 0 begin + puts "retry_on_error_matching: try #{try}" unless try.zero? try += 1 yield rescue StandardError => e -- 2.32.3