Skip to content
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

(maint) make apt testing more stable, cleanup #764

Merged
merged 1 commit into from
Jun 20, 2018
Merged

(maint) make apt testing more stable, cleanup #764

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
14 changes: 1 addition & 13 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,17 +184,5 @@
}

# 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'])
}
27 changes: 2 additions & 25 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,45 +74,22 @@

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 = {
'location' => 'http://archive.ubuntu.com/ubuntu',
'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"]')
Expand Down
9 changes: 0 additions & 9 deletions manifests/source.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
Boolean $notify_update = true,
) {

# This is needed for compat with 1.8.x
include ::apt

$_before = Apt::Setting["list-${title}"]
Expand All @@ -29,17 +28,9 @@
$_release = $release
}

# Some releases do not support https transport with default installation
Copy link
Collaborator

Choose a reason for hiding this comment

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

Removal of this is a regression. It's a feature we depend on.

Copy link
Author

Choose a reason for hiding this comment

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

https://tickets.puppetlabs.com/browse/MODULES-7540 has been created, this was my fault. I have a fix incoming, and will start a release soon after.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, just ran into it but hadn't had time to create an issue myself. Luckily for me I noticed it on a CI job and not in production.

Copy link
Author

Choose a reason for hiding this comment

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

fix and tests in #775

$_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')
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
{
"operatingsystem": "Debian",
"operatingsystemrelease": [
"8"
"8",
"9"
]
},
{
Expand Down
0 spec/acceptance/class_spec.rb → spec/acceptance/01_apt_class_spec.rb
View file
Open in desktop
File renamed without changes.
29 changes: 13 additions & 16 deletions spec/acceptance/apt_key_provider_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,11 @@
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':
Expand All @@ -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
Expand Down Expand Up @@ -612,7 +609,7 @@ def apply_manifest_twice(manifest_pp)
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)
Expand All @@ -630,7 +627,7 @@ def apply_manifest_twice(manifest_pp)
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

Expand Down Expand Up @@ -659,7 +656,7 @@ def apply_manifest_twice(manifest_pp)
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

Expand All @@ -670,7 +667,7 @@ def apply_manifest_twice(manifest_pp)

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

Expand All @@ -682,7 +679,7 @@ def apply_manifest_twice(manifest_pp)
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
Expand Down
30 changes: 10 additions & 20 deletions spec/acceptance/apt_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 => {
Expand Down Expand Up @@ -46,11 +38,9 @@ class { 'apt':
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')
Expand Down
40 changes: 0 additions & 40 deletions spec/defines/ppa_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,46 +91,6 @@
}
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": }'
Expand Down
52 changes: 6 additions & 46 deletions spec/spec_helper_acceptance.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,58 +1,17 @@
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?
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
Expand All @@ -65,9 +24,10 @@ def expect_multiple_regexes(result:, regexes:)
# 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
Expand Down