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

Update mongodb_exporter to version 0.11.0 #455

Closed
wants to merge 6 commits into from
Closed
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
2 changes: 1 addition & 1 deletion data/defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ prometheus::mongodb_exporter::package_ensure: 'latest'
prometheus::mongodb_exporter::package_name: 'mongodb_exporter'
prometheus::mongodb_exporter::service_name: 'mongodb_exporter'
prometheus::mongodb_exporter::user: 'mongodb-exporter'
prometheus::mongodb_exporter::version: '0.3.1'
prometheus::mongodb_exporter::version: '0.11.0'
prometheus::mongodb_exporter::use_kingpin: false
prometheus::node_exporter::download_extension: 'tar.gz'
prometheus::node_exporter::download_url_base: 'https://github.com/prometheus/node_exporter/releases'
Expand Down
34 changes: 33 additions & 1 deletion manifests/mongodb_exporter.pp
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,40 @@

$options = "${flag_prefix}mongodb.uri=${cnf_uri} ${extra_options}"

if $install_method == 'url' and (versioncmp($version, '0.7.0') >= 0) {
# Not a big fan of copypasting but prometheus::daemon takes for granted
# a specific path embedded in the prometheus *_exporter tarball, which
# mongodb_exporter lacks.
# TODO: patch prometheus::daemon to support custom extract directories
$exporter_install_method = 'none'
$install_dir = "/opt/${service_name}-${version}.${os}-${arch}"
file { $install_dir:
ensure => 'directory',
owner => 'root',
group => 0, # 0 instead of root because OS X uses "wheel".
mode => '0555',
}
-> archive { "/tmp/${service_name}-${version}.${download_extension}":
Copy link
Member

Choose a reason for hiding this comment

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

We should not be using the arrow syntax between resources like this and should instead use before/require and notify/subscribe metaparameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like this was no problem two years ago:

Good luck with checking the remaining ~130 modules.

ensure => present,
extract => true,
extract_path => $install_dir,
source => $real_download_url,
checksum_verify => false,
creates => "${install_dir}/${service_name}",
cleanup => true,
}
-> file { "${bin_dir}/${service_name}":
ensure => link,
notify => $notify_service,
target => "${install_dir}/${service_name}",
before => Prometheus::Daemon[$service_name],
}
} else {
$exporter_install_method = $install_method
}

prometheus::daemon { 'mongodb_exporter':
install_method => $install_method,
install_method => $exporter_install_method,
version => $version,
download_extension => $download_extension,
os => $os,
Expand Down
54 changes: 54 additions & 0 deletions spec/acceptance/mongodb_exporter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
require 'spec_helper_acceptance'

describe 'prometheus mongodb_exporter' do
it 'mongodb_exporter works idempotently with no errors' do
pp = 'include prometheus::mongodb_exporter'
apply_manifest(pp, catch_failures: true)
apply_manifest(pp, expect_changes: true)
shell('sleep 30')
end

describe 'prometheus mongodb_exporter version 0.3.1' do
it ' mongodb_exporter installs with version 0.3.1' do
pp = "class { 'prometheus::mongodb_exporter': version => '0.3.1' }"
apply_manifest(pp, catch_failures: true)
apply_manifest(pp, catch_changes: true)
shell('sleep 30')
end

describe process('mongodb_exporter') do
its(:args) { is_expected.to match %r{\ -mongodb.uri} }
end
dhoppe marked this conversation as resolved.
Show resolved Hide resolved

describe service('mongodb_exporter') do
it { is_expected.to be_running }
it { is_expected.to be_enabled }
end

describe port(9216) do
it { is_expected.to be_listening.with('tcp6') }
end
end

describe 'prometheus mongodb_exporter version 0.11.0' do
it ' mongodb_exporter installs with version 0.11.0' do
pp = "class { 'prometheus::mongodb_exporter': version => '0.11.0', use_kingpin => true }"
apply_manifest(pp, catch_failures: true)
apply_manifest(pp, catch_changes: true)
shell('sleep 30')
Copy link
Member

Choose a reason for hiding this comment

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

Could it be this sleep is not working correctly? With systemd I'd suggest some proper ready check. Ideally using the notify type but the exporter probably doesn't support that. An alternative is theforeman/puppet-qpid@7534cd9

Copy link
Member Author

@dhoppe dhoppe May 27, 2020

Choose a reason for hiding this comment

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

That is an interesting thought.

In the case of puppet-jira, shell('sleep x') is used quite often and this certainly has a negative effect on the runtime of the build jobs.

In this case, however, you should only make sure that the service has been started before checking if the port is open.

Copy link
Member

Choose a reason for hiding this comment

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

With systemd, a properly written service will not return until it has properly started. Since they often run in the foreground, there is no forking and initialization in the background. The idea of that alternative is that systemd checks the port is open and only then does systemctl start $service return.

Copy link
Member

Choose a reason for hiding this comment

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

@ekohl that's clever hack!

end

describe process('mongodb_exporter') do
its(:args) { is_expected.to match %r{\ --mongodb.uri} }
end

describe service('mongodb_exporter') do
it { is_expected.to be_running }
it { is_expected.to be_enabled }
end

describe port(9216) do
it { is_expected.to be_listening.with('tcp6') }
end
end
end