From 18d408ecffc4f962c40326775b21572792566f9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Thu, 10 Aug 2023 13:27:28 -1000 Subject: [PATCH] Make testing and installing more reliable If a pattern database test is failing, it is not installed, but on subsequent run of Puppet the non-working pattern database is considered unchanged by puppet so not tested again and the configuration converge to a state where the updated pattern database is not installed. Rework the pattern database testing and installation so that a failling pattern database fail on each subsequent run of Puppet, so that errors can be spotted more easily. Fixes #42 --- manifests/parser.pp | 54 +++++++++++++------------------------ spec/defines/parser_spec.rb | 46 ++++++++++++++++--------------- 2 files changed, 43 insertions(+), 57 deletions(-) diff --git a/manifests/parser.pp b/manifests/parser.pp index dde73b9..8b0b042 100644 --- a/manifests/parser.pp +++ b/manifests/parser.pp @@ -15,19 +15,15 @@ $tmp = join($_modules,' --module=') $modules = "--module=${tmp}" } - ensure_resource('file', "${patterndb::config_dir}/${name}", { - 'ensure' => 'directory', - 'purge' => true, - 'force' => true, - 'recurse' => true, - }) + file { "${patterndb::config_dir}/${name}": + ensure => 'directory', + purge => true, + force => true, + recurse => true, + } ensure_resource ('file', "${patterndb::cache_dir}/patterndb", { 'ensure' => 'directory', }) - ensure_resource ('file', "patterndb::file::${name}", { - 'ensure' => 'present', - 'path' => "${patterndb::var_dir}/patterndb/${name}.xml" - }) exec { "patterndb::merge::${name}": command => "pdbtool merge -r --glob \\*.pdb -D ${patterndb::config_dir}/${name} -p ${patterndb::cache_dir}/patterndb/${name}.xml", path => $facts['path'], @@ -35,34 +31,22 @@ refreshonly => true, } - exec { "patterndb::test::${name}": - #command => "/usr/bin/pdbtool --validate test ${::patterndb::cache_dir}/patterndb/${name}.xml $modules", - command => "pdbtool test ${patterndb::cache_dir}/patterndb/${name}.xml ${modules}", - path => $facts['path'], - logoutput => true, - refreshonly => true, + $deploy_command = if $test_before_deploy.lest || { $patterndb::test_before_deploy } { + "rm -f ${patterndb::var_dir}/patterndb/${name}.xml && pdbtool test ${patterndb::cache_dir}/patterndb/${name}.xml ${modules} && cp ${patterndb::cache_dir}/patterndb/${name}.xml ${patterndb::var_dir}/patterndb/${name}.xml" + } else { + "cp ${patterndb::cache_dir}/patterndb/${name}.xml ${patterndb::var_dir}/patterndb/${name}.xml" } exec { "patterndb::deploy::${name}": - command => "cp ${patterndb::cache_dir}/patterndb/${name}.xml ${patterndb::var_dir}/patterndb/", - logoutput => true, - path => $facts['path'], - refreshonly => true, + command => $deploy_command, + logoutput => true, + path => $facts['path'], + onlyif => "[ ! -f ${patterndb::var_dir}/patterndb/${name}.xml -o ${patterndb::cache_dir}/patterndb/${name}.xml -nt ${patterndb::var_dir}/patterndb/${name}.xml ]", + require => Exec["patterndb::merge::${name}"], } - if $test_before_deploy =~ Undef { - $_test_before_deploy = $patterndb::test_before_deploy - } else { - $_test_before_deploy = $test_before_deploy - } - if $_test_before_deploy { - File["patterndb::file::${name}"] - ~> Exec["patterndb::merge::${name}"] - ~> Exec["patterndb::test::${name}"] - ~> Exec["patterndb::deploy::${name}"] - } else { - File["patterndb::file::${name}"] - ~> Exec["patterndb::merge::${name}"] - # we won't 'pdbtool test' the merged file before deploying - ~> Exec["patterndb::deploy::${name}"] + + file { "patterndb::file::${name}": + path => "${patterndb::var_dir}/patterndb/${name}.xml", + require => Exec["patterndb::deploy::${name}"], } } diff --git a/spec/defines/parser_spec.rb b/spec/defines/parser_spec.rb index f847e2d..d72bf71 100644 --- a/spec/defines/parser_spec.rb +++ b/spec/defines/parser_spec.rb @@ -14,15 +14,18 @@ end let(:pre_condition) { 'include patterndb' } + # Os-specific adjustments + let :vardir do + if facts[:os]['family'] == 'FreeBSD' + '/var/syslog-ng' + else + '/var/lib/syslog-ng' + end + end + context 'Catchall' do it { is_expected.to contain_class('Patterndb') } it { is_expected.to contain_exec('patterndb::merge::default') } - - it { - is_expected.to contain_file('patterndb::file::default').with( - 'ensure' => 'present' - ).that_notifies('Exec[patterndb::merge::default]') - } end context 'Default values (no parameters)' do @@ -33,8 +36,8 @@ end it { - is_expected.to contain_exec('patterndb::test::default').with( - 'command' => %r{patterndb/default\.xml $}m + is_expected.to contain_exec('patterndb::deploy::default').with( + 'command' => "rm -f #{vardir}/patterndb/default.xml && pdbtool test /var/cache/syslog-ng/patterndb/default.xml && cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml" ) } end @@ -47,8 +50,8 @@ end it { - is_expected.to contain_exec('patterndb::test::default').with( - 'command' => %r{patterndb/default\.xml --module=foo --module=bar$}m + is_expected.to contain_exec('patterndb::deploy::default').with( + 'command' => "rm -f #{vardir}/patterndb/default.xml && pdbtool test /var/cache/syslog-ng/patterndb/default.xml --module=foo --module=bar && cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml" ) } end @@ -59,14 +62,14 @@ end it { - is_expected.to contain_exec('patterndb::test::default').with( - 'command' => %r{patterndb/default\.xml $}m + is_expected.to contain_exec('patterndb::deploy::default').with( + 'command' => "rm -f #{vardir}/patterndb/default.xml && pdbtool test /var/cache/syslog-ng/patterndb/default.xml && cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml" ) } it { - is_expected.to contain_exec('patterndb::test::stage1').with( - 'command' => %r{patterndb/stage1\.xml $}m + is_expected.to contain_exec('patterndb::deploy::stage1').with( + 'command' => "rm -f #{vardir}/patterndb/stage1.xml && pdbtool test /var/cache/syslog-ng/patterndb/stage1.xml && cp /var/cache/syslog-ng/patterndb/stage1.xml #{vardir}/patterndb/stage1.xml" ) } end @@ -78,8 +81,8 @@ end it { - is_expected.to contain_exec('patterndb::test::default').with( - 'command' => %r{patterndb/default\.xml --module=foo --module=bar$}m + is_expected.to contain_exec('patterndb::deploy::default').with( + 'command' => "rm -f #{vardir}/patterndb/default.xml && pdbtool test /var/cache/syslog-ng/patterndb/default.xml --module=foo --module=bar && cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml" ) } end @@ -90,20 +93,20 @@ end it { - is_expected.to contain_exec('patterndb::test::default').with( - 'command' => %r{patterndb/default\.xml $}m + is_expected.to contain_exec('patterndb::deploy::default').with( + 'command' => "rm -f #{vardir}/patterndb/default.xml && pdbtool test /var/cache/syslog-ng/patterndb/default.xml && cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml" ) } end - context 'Without test_before_deploy' do + context 'Without deploy' do let :params do { test_before_deploy: false, } end - it { is_expected.to contain_exec('patterndb::merge::default').that_notifies('Exec[patterndb::deploy::default]') } + it { is_expected.to contain_exec('patterndb::deploy::default').with(command: "cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml") } end context 'With test_before_deploy' do @@ -113,8 +116,7 @@ } end - it { is_expected.to contain_exec('patterndb::merge::default').that_notifies('Exec[patterndb::test::default]') } - it { is_expected.to contain_exec('patterndb::test::default').that_notifies('Exec[patterndb::deploy::default]') } + it { is_expected.to contain_exec('patterndb::deploy::default').with(command: "rm -f #{vardir}/patterndb/default.xml && pdbtool test /var/cache/syslog-ng/patterndb/default.xml && cp /var/cache/syslog-ng/patterndb/default.xml #{vardir}/patterndb/default.xml") } end end end