From 34412d10ebebb63c9135978feac3b3a786fd2171 Mon Sep 17 00:00:00 2001 From: Simon Aquino Date: Wed, 18 Jun 2014 15:26:54 +0100 Subject: [PATCH 1/5] rabbitmq::manage_repos sets 'ensure' in apt::source The ensure paramenter in apt::source is set to 'present' or 'absent' based on the value of rabbitmq::manage_repo. --- manifests/repo/apt.pp | 6 ++++++ manifests/repo/rhel.pp | 14 ++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/manifests/repo/apt.pp b/manifests/repo/apt.pp index 4f7b5d2f7..641099ec7 100644 --- a/manifests/repo/apt.pp +++ b/manifests/repo/apt.pp @@ -15,7 +15,13 @@ Class['rabbitmq::repo::apt'] -> Package<| title == 'rabbitmq-server' |> + $ensure_repo = $rabbitmq::manage_repos ? { + false => 'absent', + default => 'present', + } + apt::source { 'rabbitmq': + ensure => $ensure_repo, location => $location, release => $release, repos => $repos, diff --git a/manifests/repo/rhel.pp b/manifests/repo/rhel.pp index daa994224..4ae751701 100644 --- a/manifests/repo/rhel.pp +++ b/manifests/repo/rhel.pp @@ -2,13 +2,15 @@ # Imports the gpg key if it doesn't already exist. class rabbitmq::repo::rhel { - $package_gpg_key = $rabbitmq::package_gpg_key + if $rabbitmq::manage_repos { - Class['rabbitmq::repo::rhel'] -> Package<| title == 'rabbitmq-server' |> + $package_gpg_key = $rabbitmq::package_gpg_key - exec { "rpm --import ${package_gpg_key}": - path => ['/bin','/usr/bin','/sbin','/usr/sbin'], - unless => 'rpm -q gpg-pubkey-056e8e56-468e43f2 2>/dev/null', - } + Class['rabbitmq::repo::rhel'] -> Package<| title == 'rabbitmq-server' |> + exec { "rpm --import ${package_gpg_key}": + path => ['/bin','/usr/bin','/sbin','/usr/sbin'], + unless => 'rpm -q gpg-pubkey-056e8e56-468e43f2 2>/dev/null', + } + } } From da246043494871d43a12e88732aeba768f2f2bb8 Mon Sep 17 00:00:00 2001 From: Simon Aquino Date: Wed, 18 Jun 2014 16:07:59 +0100 Subject: [PATCH 2/5] Added rspec tests for previous commit 4dad6be --- spec/classes/rabbitmq_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/classes/rabbitmq_spec.rb b/spec/classes/rabbitmq_spec.rb index 66e72846c..a60295cde 100644 --- a/spec/classes/rabbitmq_spec.rb +++ b/spec/classes/rabbitmq_spec.rb @@ -30,8 +30,10 @@ context 'on Debian' do let(:params) {{ :manage_repos => false }} let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} - it 'does not include rabbitmq::repo::apt when manage_repos is false' do - should_not contain_class('rabbitmq::repo::apt') + it 'does ensure rabbitmq apt::source is absent when manage_repos is false' do + should contain_apt__source('rabbitmq').with( + 'ensure' => 'absent' + ) end end @@ -45,8 +47,8 @@ context 'on Redhat' do let(:params) {{ :manage_repos => false }} let(:facts) {{ :osfamily => 'RedHat' }} - it 'does not include rabbitmq::repo::rhel when manage_repos is false' do - should_not contain_class('rabbitmq::repo::rhel') + it 'does not import repo public key when manage_repos is false' do + should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') end end From a5879f4eb93de1c6c769fd93432166f928946a80 Mon Sep 17 00:00:00 2001 From: Simon Aquino Date: Thu, 7 Aug 2014 18:53:13 +0100 Subject: [PATCH 3/5] Rename variable 'manage_repos' to 'repos_ensure' 'manage_repos' doesn't describe the intended behaviour of the variable. We decided to rename it to 'repos_ensure'. This will break backward compatibility. --- manifests/init.pp | 4 ++-- manifests/params.pp | 2 +- manifests/repo/apt.pp | 4 ++-- manifests/repo/rhel.pp | 2 +- spec/classes/rabbitmq_spec.rb | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 3d881df38..0cdae56b2 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -22,7 +22,7 @@ $package_name = $rabbitmq::params::package_name, $package_provider = $rabbitmq::params::package_provider, $package_source = $rabbitmq::params::package_source, - $manage_repos = $rabbitmq::params::manage_repos, + $repos_ensure = $rabbitmq::params::repos_ensure, $plugin_dir = $rabbitmq::params::plugin_dir, $port = $rabbitmq::params::port, $tcp_keepalive = $rabbitmq::params::tcp_keepalive, @@ -63,7 +63,7 @@ validate_string($package_gpg_key) validate_string($package_name) validate_string($package_provider) - validate_bool($manage_repos) + validate_bool($repos_ensure) validate_re($version, '^\d+\.\d+\.\d+(-\d+)*$') # Allow 3 digits and optional -n postfix. # Validate config parameters. validate_array($cluster_disk_nodes) diff --git a/manifests/params.pp b/manifests/params.pp index 6c61bbbc7..51ebe181a 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -52,7 +52,7 @@ $management_port = '15672' $package_apt_pin = '' $package_gpg_key = 'http://www.rabbitmq.com/rabbitmq-signing-key-public.asc' - $manage_repos = true + $repos_ensure = true $service_ensure = 'running' $service_manage = true #config diff --git a/manifests/repo/apt.pp b/manifests/repo/apt.pp index 641099ec7..50687a1cc 100644 --- a/manifests/repo/apt.pp +++ b/manifests/repo/apt.pp @@ -15,13 +15,13 @@ Class['rabbitmq::repo::apt'] -> Package<| title == 'rabbitmq-server' |> - $ensure_repo = $rabbitmq::manage_repos ? { + $ensure_source = $rabbitmq::repos_ensure ? { false => 'absent', default => 'present', } apt::source { 'rabbitmq': - ensure => $ensure_repo, + ensure => $ensure_source, location => $location, release => $release, repos => $repos, diff --git a/manifests/repo/rhel.pp b/manifests/repo/rhel.pp index 4ae751701..284909945 100644 --- a/manifests/repo/rhel.pp +++ b/manifests/repo/rhel.pp @@ -2,7 +2,7 @@ # Imports the gpg key if it doesn't already exist. class rabbitmq::repo::rhel { - if $rabbitmq::manage_repos { + if $rabbitmq::repos_ensure { $package_gpg_key = $rabbitmq::package_gpg_key diff --git a/spec/classes/rabbitmq_spec.rb b/spec/classes/rabbitmq_spec.rb index a60295cde..8006c8d6f 100644 --- a/spec/classes/rabbitmq_spec.rb +++ b/spec/classes/rabbitmq_spec.rb @@ -28,9 +28,9 @@ end context 'on Debian' do - let(:params) {{ :manage_repos => false }} + let(:params) {{ :repos_ensure => false }} let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} - it 'does ensure rabbitmq apt::source is absent when manage_repos is false' do + it 'does ensure rabbitmq apt::source is absent when repos_ensure is false' do should contain_apt__source('rabbitmq').with( 'ensure' => 'absent' ) @@ -45,9 +45,9 @@ end context 'on Redhat' do - let(:params) {{ :manage_repos => false }} + let(:params) {{ :repos_ensure => false }} let(:facts) {{ :osfamily => 'RedHat' }} - it 'does not import repo public key when manage_repos is false' do + it 'does not import repo public key when repos_ensure is false' do should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') end end From 54fdab4389dd411d96b5d530043702ca4774328c Mon Sep 17 00:00:00 2001 From: Simon Aquino Date: Fri, 5 Dec 2014 18:37:54 +0000 Subject: [PATCH 4/5] Introduced backward compatibility for $manage_repos $manage_repos was initially proposed for removal. This was making this module backward incompatible. This commit reintroduces $manage_repos, introducing a deprecation warning if used and falling back to the $ensure_repos behaviour. --- manifests/init.pp | 7 ++++++- manifests/params.pp | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/manifests/init.pp b/manifests/init.pp index 0cdae56b2..7fba34615 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -23,6 +23,7 @@ $package_provider = $rabbitmq::params::package_provider, $package_source = $rabbitmq::params::package_source, $repos_ensure = $rabbitmq::params::repos_ensure, + $manage_repos = $rabbitmq::params::manage_repos, $plugin_dir = $rabbitmq::params::plugin_dir, $port = $rabbitmq::params::port, $tcp_keepalive = $rabbitmq::params::tcp_keepalive, @@ -123,7 +124,11 @@ include '::rabbitmq::service' include '::rabbitmq::management' - if $rabbitmq::manage_repos == true { + if $manage_repos != undef { + warning('$manage_repos is now deprecated. Please use $repos_ensure instead') + } + + if $manage_repos != false { case $::osfamily { 'RedHat', 'SUSE': { include '::rabbitmq::repo::rhel' } diff --git a/manifests/params.pp b/manifests/params.pp index 51ebe181a..36313dcfa 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -53,6 +53,7 @@ $package_apt_pin = '' $package_gpg_key = 'http://www.rabbitmq.com/rabbitmq-signing-key-public.asc' $repos_ensure = true + $manage_repos = undef $service_ensure = 'running' $service_manage = true #config From 8086d73aafe4fa93ccd8cfd3b8c49125ed7cde16 Mon Sep 17 00:00:00 2001 From: Simon Aquino Date: Fri, 5 Dec 2014 19:31:18 +0000 Subject: [PATCH 5/5] Added rspec tests/$manage_repo backward compatible Added some rspec tests after reintroducing $manage_repo in order to make the module backward compatible, while still retaining the use of $repos_ensure. --- spec/classes/rabbitmq_spec.rb | 169 +++++++++++++++++++++++++++++++++- 1 file changed, 168 insertions(+), 1 deletion(-) diff --git a/spec/classes/rabbitmq_spec.rb b/spec/classes/rabbitmq_spec.rb index 8006c8d6f..507d8736c 100644 --- a/spec/classes/rabbitmq_spec.rb +++ b/spec/classes/rabbitmq_spec.rb @@ -19,6 +19,35 @@ describe 'apt::source default values' do it 'should add a repo with defaults values' do should contain_apt__source('rabbitmq').with( { + :ensure => 'present', + :location => 'http://www.rabbitmq.com/debian/', + :release => 'testing', + :repos => 'main', + }) + end + end + end + + context 'on Debian' do + let(:params) {{ :manage_repos => false }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + it 'does ensure rabbitmq apt::source is absent when manage_repos is false' do + should_not contain_apt__source('rabbitmq') + end + end + + context 'on Debian' do + let(:params) {{ :manage_repos => true }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + + it 'includes rabbitmq::repo::apt' do + should contain_class('rabbitmq::repo::apt') + end + + describe 'apt::source default values' do + it 'should add a repo with defaults values' do + should contain_apt__source('rabbitmq').with( { + :ensure => 'present', :location => 'http://www.rabbitmq.com/debian/', :release => 'testing', :repos => 'main', @@ -30,17 +59,91 @@ context 'on Debian' do let(:params) {{ :repos_ensure => false }} let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} - it 'does ensure rabbitmq apt::source is absent when repos_ensure is false' do + it 'does ensure rabbitmq apt::source is absent when repos_ensure is false' do should contain_apt__source('rabbitmq').with( 'ensure' => 'absent' ) end end + context 'on Debian' do + let(:params) {{ :repos_ensure => true }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + + it 'includes rabbitmq::repo::apt' do + should contain_class('rabbitmq::repo::apt') + end + + describe 'apt::source default values' do + it 'should add a repo with defaults values' do + should contain_apt__source('rabbitmq').with( { + :ensure => 'present', + :location => 'http://www.rabbitmq.com/debian/', + :release => 'testing', + :repos => 'main', + }) + end + end + end + + context 'on Debian' do + let(:params) {{ :manage_repos => true, :repos_ensure => false }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + + it 'includes rabbitmq::repo::apt' do + should contain_class('rabbitmq::repo::apt') + end + + describe 'apt::source default values' do + it 'should add a repo with defaults values' do + should contain_apt__source('rabbitmq').with( { + :ensure => 'absent', + }) + end + end + end + + context 'on Debian' do + let(:params) {{ :manage_repos => true, :repos_ensure => true }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + + it 'includes rabbitmq::repo::apt' do + should contain_class('rabbitmq::repo::apt') + end + + describe 'apt::source default values' do + it 'should add a repo with defaults values' do + should contain_apt__source('rabbitmq').with( { + :ensure => 'present', + :location => 'http://www.rabbitmq.com/debian/', + :release => 'testing', + :repos => 'main', + }) + end + end + end + + context 'on Debian' do + let(:params) {{ :manage_repos => false, :repos_ensure => true }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + it 'does ensure rabbitmq apt::source is absent when manage_repos is false and repos_ensure is true' do + should_not contain_apt__source('rabbitmq') + end + end + + context 'on Debian' do + let(:params) {{ :manage_repos => false, :repos_ensure => false }} + let(:facts) {{ :osfamily => 'Debian', :lsbdistid => 'Debian', :lsbdistcodename => 'squeeze' }} + it 'does ensure rabbitmq apt::source is absent when manage_repos is false and repos_ensure is false' do + should_not contain_apt__source('rabbitmq') + end + end + context 'on Redhat' do let(:facts) {{ :osfamily => 'RedHat' }} it 'includes rabbitmq::repo::rhel' do should contain_class('rabbitmq::repo::rhel') + should contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') end end @@ -48,6 +151,70 @@ let(:params) {{ :repos_ensure => false }} let(:facts) {{ :osfamily => 'RedHat' }} it 'does not import repo public key when repos_ensure is false' do + should contain_class('rabbitmq::repo::rhel') + should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :repos_ensure => true }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does import repo public key when repos_ensure is true' do + should contain_class('rabbitmq::repo::rhel') + should contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :manage_repos => false }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does not import repo public key when manage_repos is false' do + should_not contain_class('rabbitmq::repo::rhel') + should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :manage_repos => true }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does import repo public key when manage_repos is true' do + should contain_class('rabbitmq::repo::rhel') + should contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :manage_repos => false, :repos_ensure => true }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does not import repo public key when manage_repos is false and repos_ensure is true' do + should_not contain_class('rabbitmq::repo::rhel') + should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :manage_repos => true, :repos_ensure => true }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does import repo public key when manage_repos is true and repos_ensure is true' do + should contain_class('rabbitmq::repo::rhel') + should contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :manage_repos => false, :repos_ensure => false }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does not import repo public key when manage_repos is false and repos_ensure is false' do + should_not contain_class('rabbitmq::repo::rhel') + should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') + end + end + + context 'on Redhat' do + let(:params) {{ :manage_repos => true, :repos_ensure => false }} + let(:facts) {{ :osfamily => 'RedHat' }} + it 'does not import repo public key when manage_repos is true and repos_ensure is false' do + should contain_class('rabbitmq::repo::rhel') should_not contain_exec('rpm --import http://www.rabbitmq.com/rabbitmq-signing-key-public.asc') end end