Giuseppe Lavagetto has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/393259 )
Change subject: [WiP] Move puppet CI to puppet 4.8.2 ...................................................................... [WiP] Move puppet CI to puppet 4.8.2 Change-Id: I9679b14f6bb861603d22e2d2cc5d32e7ccc50a95 --- M Gemfile M modules/apt/spec/classes/apt_spec.rb M modules/authdns/spec/classes/authdns_spec.rb M modules/authdns/spec/spec_helper.rb M modules/git/spec/defines/clone_spec.rb M modules/install_server/spec/classes/install_server_dhcp_server_spec.rb M modules/install_server/spec/classes/install_server_web_server_spec.rb M modules/install_server/spec/spec_helper.rb M modules/jenkins/spec/classes/jenkins_spec.rb M modules/jenkins/spec/hosts/master_and_slave_spec.rb M modules/jenkins/spec/spec_helper.rb M modules/mirrors/spec/classes/mirrors_debian_spec.rb M modules/mirrors/spec/classes/mirrors_ubuntu_spec.rb D modules/mirrors/spec/fixtures/manifests/site.pp A modules/mirrors/spec/fixtures/modules/passwords/manifests/mirrors.pp M modules/monitoring/manifests/host.pp M modules/monitoring/spec/defines/monitoring_host_spec.rb M rake_modules/taskgen.rb 18 files changed, 127 insertions(+), 44 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/operations/puppet refs/changes/59/393259/1 diff --git a/Gemfile b/Gemfile index f5e07ad..871d725 100644 --- a/Gemfile +++ b/Gemfile @@ -1,12 +1,11 @@ source 'https://rubygems.org' -gem 'puppet', ENV['PUPPET_GEM_VERSION'] || '~> 3.8.5' +gem 'puppet', ENV['PUPPET_GEM_VERSION'] || '4.9.2' gem 'xmlrpc' if RUBY_VERSION >= '2.4.0' gem 'puppet-strings', '~> 1.0.0' -gem 'rspec-puppet', '~> 2.5.0' +gem 'rspec-puppet', '~> 2.6.9' +gem 'rspec-puppet-facts', '~> 1.7', :require => false gem 'puppetlabs_spec_helper', '< 2.0.0' -# Puppet 3.7 fails on ruby 2.2+ -# https://tickets.puppetlabs.com/browse/PUP-3796 gem 'safe_yaml', '~> 1.0.4' gem 'rake', '~> 12.0.0' diff --git a/modules/apt/spec/classes/apt_spec.rb b/modules/apt/spec/classes/apt_spec.rb index ccbb7f6..cfb89c5 100644 --- a/modules/apt/spec/classes/apt_spec.rb +++ b/modules/apt/spec/classes/apt_spec.rb @@ -1,22 +1,25 @@ require 'spec_helper' describe 'apt' do + os = [ { :lsbdistid => 'Debian', :lsbdistrelease => '8.0', :operatingsystem => 'Debian', + :lsbdistcodename => 'jessie' }, { :lsbdistid => 'Ubuntu', :lsbdistrelease => '14.04', :operatingsystem => 'Ubuntu', + :lsbdistcodename => 'trusty' }, ] os.each do |os_facts| - context "with OS #{os_facts[:lsbdistid]} #{os_facts[:lsbdistrelease]}" do + context "with OS #{os_facts[:lsbdistid]} #{os_facts[:lsbdistrelease]}" do let(:facts) { os_facts } - + let(:node_params) { {'site' => 'eqiad'} } it { should compile } context "when not using a proxy" do diff --git a/modules/authdns/spec/classes/authdns_spec.rb b/modules/authdns/spec/classes/authdns_spec.rb index 0e7595f..4c4bddb 100644 --- a/modules/authdns/spec/classes/authdns_spec.rb +++ b/modules/authdns/spec/classes/authdns_spec.rb @@ -1,7 +1,20 @@ require 'spec_helper' +test_on = { + supported_os: [ + { + 'operatingsystem' => 'Debian', + 'operatingsystemrelease' => ['8'], # we cannot support stretch atm because of a bug in the service provider in + # the puppet gem + } + ] +} describe 'authdns' do - let(:node) { 'testhost.eqiad.wmnet' } + let(:node) { 'testhost.eqiad.wmnet' } + + on_supported_os(test_on).each do |os, facts| + facts[:initsystem] = 'systemd' + let(:facts) { facts } let(:params) { { :lvs_services => {}, :discovery_services => {}, @@ -14,9 +27,12 @@ 'class confd($prefix) {}', 'package{ "git": }', ] } - it { should compile } + it { is_expected.to compile.with_all_deps } + end end describe 'authdns::lint' do + on_supported_os(test_on).each do |os, facts| it { should compile } + end end diff --git a/modules/authdns/spec/spec_helper.rb b/modules/authdns/spec/spec_helper.rb index df9590b..7464689 100644 --- a/modules/authdns/spec/spec_helper.rb +++ b/modules/authdns/spec/spec_helper.rb @@ -1,6 +1,8 @@ require 'rspec-puppet' require 'puppetlabs_spec_helper/module_spec_helper' +require 'rspec-puppet-facts' +include RspecPuppetFacts fixture_path = File.expand_path(File.join(__FILE__, '..', 'fixtures')) RSpec.configure do |c| diff --git a/modules/git/spec/defines/clone_spec.rb b/modules/git/spec/defines/clone_spec.rb index 20d2271..4a81bfa 100644 --- a/modules/git/spec/defines/clone_spec.rb +++ b/modules/git/spec/defines/clone_spec.rb @@ -9,7 +9,7 @@ } } it 'checkouts a workspace' do should contain_exec('git_clone_operations/puppet') - .without_command(/ --bare /) + .with_command('/usr/bin/git clone https://gerrit.wikimedia.org/r/operations/puppet /srv/git/operations/puppet') end it 'tracks the proper created file' do should contain_exec('git_clone_operations/puppet') diff --git a/modules/install_server/spec/classes/install_server_dhcp_server_spec.rb b/modules/install_server/spec/classes/install_server_dhcp_server_spec.rb index 4daf72b..d4c3749 100644 --- a/modules/install_server/spec/classes/install_server_dhcp_server_spec.rb +++ b/modules/install_server/spec/classes/install_server_dhcp_server_spec.rb @@ -1,18 +1,34 @@ require 'spec_helper' +test_on = { + supported_os: [ + { + 'operatingsystem' => 'Debian', + 'operatingsystemrelease' => ['8'], # we cannot support stretch atm because of a bug in the service provider in + # the puppet gem, that is fixed in debian itself... + } + ] +} describe 'install_server::dhcp_server', :type => :class do - it { should compile } + on_supported_os(test_on).each do |os, facts| + context "On #{os}" do + let(:facts){ facts } + it { is_expected.to compile } - it 'should have isc-dhcp-server' do - should contain_package('isc-dhcp-server').with_ensure('present') - should contain_service('isc-dhcp-server').with_ensure('running') + it 'should have isc-dhcp-server' do + is_expected.to contain_package('isc-dhcp-server').with_ensure('present') + is_expected.to contain_service('isc-dhcp-server').with_ensure('running') - should contain_file('/etc/dhcp/').with({ - 'ensure' => 'directory', - 'mode' => '0444', - 'owner' => 'root', - 'group' => 'root', - 'recurse' => 'true', - }) + is_expected.to contain_file('/etc/dhcp').with( + { + 'ensure' => 'directory', + 'mode' => '0444', + 'owner' => 'root', + 'group' => 'root', + 'recurse' => 'true', + } + ) + end end + end end diff --git a/modules/install_server/spec/classes/install_server_web_server_spec.rb b/modules/install_server/spec/classes/install_server_web_server_spec.rb index aad82b8..a004057 100644 --- a/modules/install_server/spec/classes/install_server_web_server_spec.rb +++ b/modules/install_server/spec/classes/install_server_web_server_spec.rb @@ -5,8 +5,9 @@ let(:facts) { { :lsbdistrelease => '8.5', :lsbdistid => 'Debian', + :operatingsystem => 'Debian' } } - + let(:node_params) { {'realm' => 'production'} } it do should contain_file('/srv/index.html').with({ 'mode' => '0444', diff --git a/modules/install_server/spec/spec_helper.rb b/modules/install_server/spec/spec_helper.rb index 421fd71..f603826 100644 --- a/modules/install_server/spec/spec_helper.rb +++ b/modules/install_server/spec/spec_helper.rb @@ -1,6 +1,8 @@ require 'rspec-puppet' require 'puppetlabs_spec_helper/module_spec_helper' +require 'rspec-puppet-facts' +include RspecPuppetFacts fixture_path = File.expand_path(File.join(__FILE__, '..', 'fixtures')) RSpec.configure do |c| diff --git a/modules/jenkins/spec/classes/jenkins_spec.rb b/modules/jenkins/spec/classes/jenkins_spec.rb index 6a7458e..4b87d70 100644 --- a/modules/jenkins/spec/classes/jenkins_spec.rb +++ b/modules/jenkins/spec/classes/jenkins_spec.rb @@ -1,6 +1,14 @@ require 'spec_helper' describe 'jenkins' do + precondition = <<-EOF +class profile::base { + $notifications_enabled = '1' +} +include ::profile::base +EOF + let(:node_params) { {'cluster' => 'ci', 'site' => 'eqiad'} } + let(:pre_condition) { precondition } let(:params) { { :prefix => '/ci', } } diff --git a/modules/jenkins/spec/hosts/master_and_slave_spec.rb b/modules/jenkins/spec/hosts/master_and_slave_spec.rb index f1ec814..5c22b61 100644 --- a/modules/jenkins/spec/hosts/master_and_slave_spec.rb +++ b/modules/jenkins/spec/hosts/master_and_slave_spec.rb @@ -1,8 +1,13 @@ require 'spec_helper' describe 'Host being both a Jenkins master and a slave' do - let(:pre_condition) { + let(:node_params) { {'cluster' => 'ci', 'site' => 'eqiad'} } + let(:pre_condition) { """ + class profile::base { + $notifications_enabled = '1' + } + include ::profile::base class { 'jenkins': prefix => '/jenkins', } diff --git a/modules/jenkins/spec/spec_helper.rb b/modules/jenkins/spec/spec_helper.rb index 7af690f..4a20b40 100644 --- a/modules/jenkins/spec/spec_helper.rb +++ b/modules/jenkins/spec/spec_helper.rb @@ -1,14 +1,17 @@ require 'rspec-puppet' require 'puppetlabs_spec_helper/module_spec_helper' +require 'rspec-puppet-facts' + +include RspecPuppetFacts fixture_path = File.expand_path(File.join(__FILE__, '..', 'fixtures')) RSpec.configure do |c| c.module_path = File.join(fixture_path, 'modules') c.manifest_dir = File.join(fixture_path, 'manifests') - c.default_facts = { - :initsystem => 'systemd', - :lsbdistid => 'Debian', - :lsbdistrelease => '8.0', - } + test_on = { supported_os: [{'operatingsystem' => 'Debian', 'operatingsystemrelease' => ['8']}]} + on_supported_os(test_on).each do |_, facts| + facts[:initsystem] = 'systemd' + c.default_facts = facts + end end diff --git a/modules/mirrors/spec/classes/mirrors_debian_spec.rb b/modules/mirrors/spec/classes/mirrors_debian_spec.rb index bf168ff..c6ca028 100644 --- a/modules/mirrors/spec/classes/mirrors_debian_spec.rb +++ b/modules/mirrors/spec/classes/mirrors_debian_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe 'mirrors::debian', :type => :class do + let(:code) { "class passwords::mirrors {}"} it do should contain_file('/srv/mirrors/debian').with({ 'ensure' => 'directory', @@ -18,7 +19,7 @@ }) end it do - should contain_file('/var/log/ftpsync/').with({ + should contain_file('/var/log/ftpsync').with({ 'ensure' => 'directory', 'owner' => 'mirror', 'group' => 'mirror', diff --git a/modules/mirrors/spec/classes/mirrors_ubuntu_spec.rb b/modules/mirrors/spec/classes/mirrors_ubuntu_spec.rb index 3c72d73..b210601 100644 --- a/modules/mirrors/spec/classes/mirrors_ubuntu_spec.rb +++ b/modules/mirrors/spec/classes/mirrors_ubuntu_spec.rb @@ -2,7 +2,7 @@ describe 'mirrors::ubuntu', :type => :class do it do - should contain_file('/srv/mirrors/ubuntu/').with({ + should contain_file('/srv/mirrors/ubuntu').with({ 'ensure' => 'directory', 'owner' => 'mirror', 'group' => 'mirror', diff --git a/modules/mirrors/spec/fixtures/manifests/site.pp b/modules/mirrors/spec/fixtures/manifests/site.pp deleted file mode 100644 index 54f0d7f..0000000 --- a/modules/mirrors/spec/fixtures/manifests/site.pp +++ /dev/null @@ -1,4 +0,0 @@ -# lint:ignore:autoloader_layout -class passwords::mirrors { -} -# lint:endignore diff --git a/modules/mirrors/spec/fixtures/modules/passwords/manifests/mirrors.pp b/modules/mirrors/spec/fixtures/modules/passwords/manifests/mirrors.pp new file mode 100644 index 0000000..2f740b3 --- /dev/null +++ b/modules/mirrors/spec/fixtures/modules/passwords/manifests/mirrors.pp @@ -0,0 +1,3 @@ +class passwords::mirrors { + # nothing to see here +} diff --git a/modules/monitoring/manifests/host.pp b/modules/monitoring/manifests/host.pp index 4746827..f17fbf0 100644 --- a/modules/monitoring/manifests/host.pp +++ b/modules/monitoring/manifests/host.pp @@ -85,12 +85,15 @@ statusmap_image => undef, } } + } else { + $mgmt_host = undef } } else { $icon_image = undef $vrml_image = undef $statusmap_image = undef $real_parents = $parents + $mgmt_host = undef } $host = { "${title}" => { @@ -120,7 +123,7 @@ $rtype = '@@nagios_host' } create_resources($rtype, $host) - if $mgmt_host { + if !empty($mgmt_host) { create_resources($rtype, $mgmt_host) # We always monitor the BMC so never skip notifications monitoring::service { "dns_${title}.mgmt": diff --git a/modules/monitoring/spec/defines/monitoring_host_spec.rb b/modules/monitoring/spec/defines/monitoring_host_spec.rb index b108b7d..2b51025 100644 --- a/modules/monitoring/spec/defines/monitoring_host_spec.rb +++ b/modules/monitoring/spec/defines/monitoring_host_spec.rb @@ -6,9 +6,18 @@ 'fake_secret' } end - # We abuse RSpec a bit throughout the spec setting site to a fact - # Unfortunately RSpec node level parameters do not seem to work + context 'with a standard physical host' do + let(:pre_condition){ + """ + class passwords::nagios::mysql { + $mysql_check_pass='foo' + } + class {'passwords::nagios::mysql': } + """ + } + let(:node_params) { {'cluster' => 'ci', 'site' => 'eqiad'} } + let(:facts) { { :hostname => 'ahost', @@ -18,7 +27,6 @@ :lldp_parent => 'ahosts_parent', :has_ipmi => true, :ipmi_lan => { :ipaddress => '2.2.2.2', }, - :site => 'blabla', } } let(:title) { 'ahost' } @@ -64,6 +72,16 @@ end context 'with a standard virtual host' do + let(:pre_condition){ + """ + class passwords::nagios::mysql { + $mysql_check_pass='foo' + } + class {'passwords::nagios::mysql': } + """ + } + let(:node_params) { {'cluster' => 'ci', 'site' => 'eqiad'} } + let(:facts) { { :hostname => 'ahost', @@ -109,6 +127,18 @@ end context 'with an icinga host' do + let(:pre_condition){ + """ + class profile::base { $notifications_enabled = '1' } + class passwords::nagios::mysql { + $mysql_check_pass='foo' + } + include ::profile::base + include icinga + """ + } + let(:node_params) { {'cluster' => 'ci', 'site' => 'eqiad'} } + let(:facts) { { :hostname => 'icingahost', @@ -118,15 +148,8 @@ :lldp_parent => 'ahosts_parent', :has_ipmi => true, :ipmi_lan => { :ipaddress => '2.2.2.2', }, - :site => 'blabla', } } - let(:pre_condition) do - ''' - include icinga - class passwords::nagios::mysql {} - ''' - end describe 'monitoring itself' do let(:title) { 'icingahost' } diff --git a/rake_modules/taskgen.rb b/rake_modules/taskgen.rb index e406989..fe55ca4 100644 --- a/rake_modules/taskgen.rb +++ b/rake_modules/taskgen.rb @@ -224,6 +224,8 @@ def setup_spec # Modules known not to pass tests ignored_modules = ['mysql', 'osm', 'puppetdbquery', 'stdlib', 'wdqs', 'tilerator', 'wmflib'] + # Modules that failed to pass the spec tests: monitoring, network, puppetdbquery, mysql, osm, profile, stdlib, puppetmaster, rsync, service, tilerator, wdqs, systemd, wmflib, zuul, tlsproxy + deps = SpecDependencies.new spec_modules = deps.specs_to_run(@changed_files).select do |m| !ignored_modules.include?(m) -- To view, visit https://gerrit.wikimedia.org/r/393259 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9679b14f6bb861603d22e2d2cc5d32e7ccc50a95 Gerrit-PatchSet: 1 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
