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

Implement $server_max_open_files #671

Merged
merged 1 commit into from
Feb 14, 2019
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
1 change: 1 addition & 0 deletions .fixtures.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ fixtures:
inifile: 'https://github.com/puppetlabs/puppetlabs-inifile.git'
puppetdb: 'https://github.com/puppetlabs/puppetlabs-puppetdb.git'
stdlib: 'https://github.com/puppetlabs/puppetlabs-stdlib.git'
systemd: 'https://github.com/camptocamp/puppet-systemd.git'
yumrepo_core:
repo: 'https://github.com/puppetlabs/puppetlabs-yumrepo_core'
puppet_version: '>= 6.0.0'
4 changes: 4 additions & 0 deletions manifests/init.pp
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,9 @@
# $server_ca_enable_infra_crl:: Enable the separate CRL for Puppet infrastructure nodes
# Defaults to false
#
# $server_max_open_files:: Increase the max open files limit for Puppetserver.
# Defaults to undef
#
# === Usage:
#
# * Simple usage:
Expand Down Expand Up @@ -703,6 +706,7 @@
Boolean $server_ca_allow_sans = $puppet::params::server_ca_allow_sans,
Boolean $server_ca_allow_auth_extensions = $puppet::params::server_ca_allow_auth_extensions,
Boolean $server_ca_enable_infra_crl = $puppet::params::server_ca_enable_infra_crl,
Optional[Integer[1]] $server_max_open_files = $puppet::params::server_max_open_files,
) inherits puppet::params {
contain puppet::config

Expand Down
1 change: 1 addition & 0 deletions manifests/params.pp
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@
$server_ca_allow_sans = false
$server_ca_allow_auth_extensions = false
$server_ca_enable_infra_crl = false
$server_max_open_files = undef

$server_puppetserver_version = undef

Expand Down
1 change: 1 addition & 0 deletions manifests/server.pp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@
Boolean $ca_allow_sans = $::puppet::server_ca_allow_sans,
Boolean $ca_allow_auth_extensions = $::puppet::server_ca_allow_auth_extensions,
Boolean $ca_enable_infra_crl = $::puppet::server_ca_enable_infra_crl,
Optional[Integer[1]] $max_open_files = $::puppet::server_max_open_files,
) {
if $ca {
$ssl_ca_cert = "${ssl_dir}/ca/ca_crt.pem"
Expand Down
21 changes: 21 additions & 0 deletions manifests/server/puppetserver.pp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
$ca_allow_sans = $::puppet::server::ca_allow_sans,
$ca_allow_auth_extensions = $::puppet::server::ca_allow_auth_extensions,
$ca_enable_infra_crl = $::puppet::server::ca_enable_infra_crl,
$max_open_files = $::puppet::server::max_open_files,
) {
include ::puppet::server

Expand Down Expand Up @@ -192,6 +193,26 @@
changes => $jruby_jar_changes,
}
}

$ensure_max_open_files = $max_open_files ? {
undef => 'absent',
default => 'present',
}
if $facts['service_provider'] == 'systemd' {
systemd::dropin_file { 'puppetserver.service-limits.conf':
ensure => $ensure_max_open_files,
filename => 'limits.conf',
unit => 'puppetserver.service',
content => "[Service]\nLimitNOFILE=${max_open_files}\n",
}
} else {
file_line { 'puppet::server::puppetserver::max_open_files':
ensure => $ensure_max_open_files,
path => $config,
line => "ulimit -n ${max_open_files}",
match => '^ulimit\ -n',
}
}
}

$servicesd = "${server_puppetserver_dir}/services.d"
Expand Down
46 changes: 46 additions & 0 deletions spec/acceptance/puppetserver_config_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require 'spec_helper_acceptance'

describe 'Puppetserver config options', unless: ENV['BEAKER_PUPPET_COLLECTION'] == 'pc1' && fact('lsbdistcodename') == 'stretch' do
before(:context) do
if fact('lsbdistcodename') == 'jessie' && ENV['BEAKER_PUPPET_COLLECTION'] != 'pc1'
on default, "echo 'deb http://deb.debian.org/debian jessie-backports main' >/etc/apt/sources.list.d/backports.list"
on default, 'apt update'
on default, 'apt -y -t jessie-backports install openjdk-8-jdk-headless'
end
if check_for_package(default, 'puppetserver')
on default, puppet('resource package puppetserver ensure=purged')
on default, 'rm -rf /etc/sysconfig/puppetserver /etc/puppetlabs/puppetserver'
on default, 'find /etc/puppetlabs/puppet/ssl/ -type f -delete'
end

# puppetserver won't start with lower than 2GB memory
memoryfree_mb = fact('memoryfree_mb').to_i
raise 'At least 2048MB free memory required' if memoryfree_mb < 256
end

describe 'server_max_open_files' do
let(:pp) do
<<-MANIFEST
class { '::puppet':
server => true,
server_foreman => false,
server_reports => 'store',
server_external_nodes => '',
# only for install test - don't think to use this in production!
# https://docs.puppet.com/puppetserver/latest/tuning_guide.html
server_jvm_max_heap_size => '256m',
server_jvm_min_heap_size => '256m',
server_max_open_files => 32143,
}
MANIFEST
end

it_behaves_like 'a idempotent resource'

# pgrep -f java.*puppetserver would be better. But i cannot get it to work. Shellwords.escape() seems to break something
describe command("grep '^Max open files' /proc/`cat /var/run/puppetlabs/puppetserver/puppetserver.pid`/limits"), :sudo => true do
Copy link
Member

Choose a reason for hiding this comment

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

You may be able to parse this via systemctl status puppetserver as well. Both the main pid as the limit. On the other hand, this is a real test on the result so I'm fine with this.

its(:exit_status) { is_expected.to eq 0 }
its(:stdout) { is_expected.to match %r{^Max open files\s+32143\s+32143\s+files\s*$} }
end
end
end
34 changes: 34 additions & 0 deletions spec/classes/puppet_server_puppetserver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,40 @@
end
end

describe 'server_max_open_files', unless: facts[:osfamily] == 'FreeBSD' do
context 'when server_max_open_files => undef' do
it do
if facts['service_provider'] == 'systemd'
should contain_systemd__dropin_file('puppetserver.service-limits.conf')
.with_ensure('absent')
else
should contain_file_line('puppet::server::puppetserver::max_open_files')
.with_ensure('absent')
end
end
end

context 'when server_max_open_files => 32143' do
let(:params) { super().merge(server_max_open_files: 32143) }

it do
if facts['service_provider'] == 'systemd'
should contain_systemd__dropin_file('puppetserver.service-limits.conf')
.with_ensure('present')
.with_filename('limits.conf')
.with_unit('puppetserver.service')
.with_content("[Service]\nLimitNOFILE=32143\n")
else
should contain_file_line('puppet::server::puppetserver::max_open_files')
.with_ensure('present')
.with_path('/etc/default/puppetserver')
.with_line('ulimit -n 32143')
.with_match('^ulimit\ -n')
end
end
end
end

describe 'with extra_args parameter' do
let(:params) { super().merge(server_jvm_extra_args: ['-XX:foo=bar', '-XX:bar=foo']) }
if facts[:osfamily] == 'FreeBSD'
Expand Down
32 changes: 32 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,38 @@
# Original fact sources:
add_custom_fact :puppet_environmentpath, '/etc/puppetlabs/code/environments' # puppetlabs-stdlib
add_custom_fact :root_home, '/root' # puppetlabs-stdlib
# Rough conversion of grepping in the puppet source:
# grep defaultfor lib/puppet/provider/service/*.rb
add_custom_fact :service_provider, ->(os, facts) do
case facts[:osfamily].downcase
when 'archlinux'
'systemd'
when 'darwin'
'launchd'
when 'debian'
if facts[:operatingsystem] == 'Ubuntu'
facts[:operatingsystemmajrelease].to_i >= 15 ? 'systemd' : 'upstart'
elsif facts[:operatingsystem] == 'Debian' && facts[:operatingsystemmajrelease].to_i >= 8
'systemd'
else
'debian'
end
when 'freebsd'
'freebsd'
when 'gentoo'
'openrc'
when 'openbsd'
'openbsd'
when 'redhat'
facts[:operatingsystemrelease].to_i >= 7 ? 'systemd' : 'redhat'
when 'suse'
facts[:operatingsystemmajrelease].to_i >= 12 ? 'systemd' : 'redhat'
when 'windows'
'windows'
else
'init'
end
end

# Workaround for no method in rspec-puppet to pass undef through :params
class Undef
Expand Down
1 change: 1 addition & 0 deletions spec/spec_helper_acceptance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
run_puppet_install_helper unless ENV['BEAKER_provision'] == 'no'
install_module_on(hosts)
install_module_dependencies_on(hosts)
install_module_from_forge('camptocamp-systemd', '>= 2.0.0 < 3.0.0')

RSpec.configure do |c|
# Readable test descriptions
Expand Down