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.

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

(#463) Ensure sensu::plugins are managed before checks #755

Merged
merged 1 commit into from
Jul 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions lib/puppet/type/sensu_check_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ def initialize(*args)
'Service[sensu-enterprise]',
'Service[sensu-api]',
].select { |ref| c.resource(ref) }
# (#463) All plugins must come before all checks. Collections are not used to
# avoid realizing any resources.
self[:subscribe] = [
'Anchor[plugins_before_checks]',
].select { |ref| c.resource(ref) }
end
end

Expand Down
6 changes: 5 additions & 1 deletion manifests/check.pp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@
Variant[Undef,Enum['absent'],Hash] $subdue = undef,
Variant[Undef,Enum['absent'],Hash] $proxy_requests = undef,
) {

if $ensure == 'present' and !$command {
fail("sensu::check{${name}}: a command must be given when ensure is present")
}
Expand Down Expand Up @@ -201,6 +200,11 @@
}
}

# (#463) All plugins must come before all checks. Collections are not used to
# avoid realizing any resources.
Anchor['plugins_before_checks']
~> Sensu::Check[$name]

file { "${conf_dir}/checks/${check_name}.json":
ensure => $ensure,
owner => $user,
Expand Down
6 changes: 4 additions & 2 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -563,9 +563,7 @@
Hash $filter_defaults = {},
Hash $mutators = {},
### END Hiera Lookups ###

) {

if $dashboard { fail('Sensu-dashboard is deprecated, use a dashboard module. See https://github.com/sensu/sensu-puppet#dashboards')}
if $purge_config { fail('purge_config is deprecated, set the purge parameter to a hash containing `config => true` instead') }
if $purge_plugins_dir { fail('purge_plugins_dir is deprecated, set the purge parameter to a hash containing `plugins => true` instead') }
Expand Down Expand Up @@ -637,6 +635,10 @@
$_purge_mutators = $full_purge_hash['mutators']
}

# (#463) This well-known anchor serves as a reference point so all checks are
# managed after all plugins. It must always exist in the catalog.
anchor { 'plugins_before_checks': }
Copy link
Collaborator

Choose a reason for hiding this comment

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

We want to kill off the anchor pattern in the next big refactor after the data types. Is there another way we can do this, such as using the spaceship operator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about using the spaceship operator but didn't because it's very likely some end users will have virtual or exported check resources. Collections realize resources, which would be a PITA for the end users.

This isn't the full blown anchor pattern, which is more about class containment. This is more of a lightweight, "Here's a single, well-known reference point to establish before and after."

I chatted with some people in the Puppet slack, and the best alternative to the anchor idea was https://github.com/binford2k/binford2k-manifold, but I did get some validating feedback a single anchor is reasonable in this situation and I like how simple it is to explain. All plugins are managed before the anchor, all checks are managed after the anchor, the anchor always exists in the catalog.


# Create resources from hiera lookups
create_resources('::sensu::extension', $extensions)
create_resources('::sensu::handler', $handlers, $handler_defaults)
Expand Down
8 changes: 6 additions & 2 deletions manifests/plugin.pp
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,18 @@
Boolean $nocheckcertificate = false,
Any $gem_install_options = $::sensu::gem_install_options,
) {

File {
owner => 'sensu',
group => 'sensu',
}

Sensu::Plugin[$name]
-> Class['sensu::client::service']
~> Service['sensu-client']

# (#463) All plugins must come before all checks. Collections are not used to
# avoid realizing any resources.
Sensu::Plugin[$name]
-> Anchor['plugins_before_checks']

case $type {
'file': {
Expand Down
2 changes: 1 addition & 1 deletion spec/classes/sensu_init_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
it { should contain_file('/etc/default/sensu').without_content(%r{^CLIENT_DEREGISTER_ON_STOP=true\nCLIENT_DEREGISTER_HANDLER=.*$}) }
it { should contain_file('/etc/default/sensu').with_content(%r{^SERVICE_MAX_WAIT="10"$}) }
it { should contain_file('/etc/default/sensu').with_content(%r{^PATH=\$PATH$}) }
it { should contain_anchor('plugins_before_checks') }
it { should_not contain_file('C:/opt/sensu/bin/sensu-client.xml') }

describe 'osfamily windows' do
Expand Down Expand Up @@ -409,5 +410,4 @@
end # var[:name].each
end # validations.sort.each
end # describe 'variable type and content validations'

end
5 changes: 5 additions & 0 deletions spec/defines/sensu_check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,9 @@
it { is_expected.to contain_sensu_check('mycheck').with(interval: 'absent') }
end
end

describe 'relationships (#463)' do
let(:expected) { { notify: ["Sensu::Check[#{title}]"] } }
it { is_expected.to contain_anchor('plugins_before_checks').with(expected)}
end
end
39 changes: 28 additions & 11 deletions spec/defines/sensu_plugin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class { '::sensu':
let(:title) { 'puppet:///data/plug1' }

context 'defaults' do

it { should contain_file('/etc/sensu/plugins/plug1').with(
:source => 'puppet:///data/plug1'
) }
Expand All @@ -28,7 +27,7 @@ class { '::sensu':
:source => 'puppet:///data/plug1'
) }
end
end #file
end

context 'url' do
let(:title) { 'https://raw.githubusercontent.com/sensu/sensu-community-plugins/master/plugins/system/check-mem.sh' }
Expand All @@ -44,7 +43,6 @@ class { '::sensu':
:path => '/etc/sensu/plugins/check-mem.sh',
:checksum => '1d58b78e9785f893889458f8e9fe8627'
) }

end

context 'setting params' do
Expand All @@ -59,7 +57,6 @@ class { '::sensu':
:path => '/var/sensu/plugins/check-mem.sh',
:checksum => '1d58b78e9785f893889458f8e9fe8627'
) }

end

context 'new plugin should provide source' do
Expand All @@ -74,10 +71,8 @@ class { '::sensu':
:path => '/var/sensu/plugins/check-puppet-last-run.rb',
:source => 'https://raw.githubusercontent.com/sensu-plugins/sensu-plugins-puppet/master/bin/check-puppet-last-run.rb'
) }

end

end #url
end

context 'directory' do
let(:title) { 'puppet:///data/sensu/plugins' }
Expand Down Expand Up @@ -115,7 +110,7 @@ class { '::sensu':
'group' => 'sensu'
) }
end
end #directory
end

context 'package' do
let(:title) { 'sensu-plugins' }
Expand Down Expand Up @@ -162,12 +157,34 @@ class { '::sensu':
let(:params) { { :type => 'package', :gem_install_options => [{ '-p' => 'http://user:[email protected]:8080' }], :pkg_provider => 'gem' } }
it { should contain_package('sensu-plugins').with_install_options([{ '-p' => 'http://user:[email protected]:8080' }]) }
end

end #package
end

context 'default' do
let(:params) { { :type => 'unknown' } }
it { expect { should raise_error(Puppet::Error) } }
end #default
end

describe 'ordering (#463)' do
let(:pre_condition) do
<<-'ENDofPUPPETcode'
class { '::sensu':
manage_plugins_dir => false,
}
sensu::check { 'ntp':
command => 'check_ntp_time -H pool.ntp.org -w 30 -c 60',
handlers => 'default',
subscribers => 'sensu-test',
}
ENDofPUPPETcode
end
let(:title) { 'puppet:///data/plug1' }
describe 'notifies the sensu-client service' do
let(:expected) { { notify: ['Service[sensu-client]'] } }
it { is_expected.to contain_sensu__plugin(title).with(expected)}
end
describe 'comes before sensu checks via Anchor[plugins_before_checks]' do
let(:expected) { { before: ['Anchor[plugins_before_checks]'] } }
it { is_expected.to contain_sensu__plugin(title).with(expected)}
end
end
end
5 changes: 5 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@
:kernel => 'Linux',
:osfamily => 'RedHat',
}
config.backtrace_exclusion_patterns = [
%r{/\.bundle/},
%r{/\.rbenv/},
%r{/.rvm/},
]
end
20 changes: 20 additions & 0 deletions spec/unit/sensu_check_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,24 @@
end
end
end

describe 'relationships' do
let(:resource_hash) do
c = Puppet::Resource::Catalog.new
r = Puppet::Type.type(:anchor).new(name: 'plugins_before_checks')
c.add_resource(r)
{
:title => 'foo.example.com',
:catalog => c
}
end
let(:resource) do
end
context 'plugins before checks (#463)' do
subject { described_class.new(resource_hash)[:subscribe].map(&:ref) }
it 'subscribes to Anchor[plugins_before_checks]' do
is_expected.to eq ['Anchor[plugins_before_checks]']
end
end
end
end