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

Open file limit not set correctly #127

Merged
merged 16 commits into from Apr 23, 2015
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -8,7 +8,7 @@ end

group :unit do
gem 'berkshelf'
gem 'chefspec'
gem 'chefspec', '~> 4.0'

Choose a reason for hiding this comment

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

@jjasghar Is this required?

Choose a reason for hiding this comment

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

Paging @jjasghar
Will Gemfile.lock help on this?
/CC @michaelklishin

Copy link
Contributor

Choose a reason for hiding this comment

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

Meh, you should be running the newest version of chefspec right?

gem 'fauxhai'
end

Expand Down
8 changes: 8 additions & 0 deletions recipes/default.rb
Expand Up @@ -184,6 +184,14 @@ class Chef::Resource # rubocop:disable all
notifies :restart, "service[#{node['rabbitmq']['service_name']}]", :immediately
end

template "/etc/default/#{node['rabbitmq']['service_name']}" do
source 'default.rabbitmq-server.erb'

Choose a reason for hiding this comment

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

How about rabbitmq.service.default.erb?

Choose a reason for hiding this comment

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

default.rabbitmq-server.erb is looking good to me. 👍

owner 'root'
group 'root'
mode 00644
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a 4 digit string: '0644'

Copy link
Collaborator

Choose a reason for hiding this comment

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

The 5 digit should be fine. I believe this just forces the octet rule

notifies :restart, "service[#{node['rabbitmq']['service_name']}]", :immediately
end

if File.exist?(node['rabbitmq']['erlang_cookie_path']) && File.readable?((node['rabbitmq']['erlang_cookie_path']))
existing_erlang_key = File.read(node['rabbitmq']['erlang_cookie_path']).strip
else
Expand Down
9 changes: 9 additions & 0 deletions spec/default_spec.rb
Expand Up @@ -47,6 +47,15 @@
expect(chef_run).to install_package('erlang')
end

it 'should create the rabbitmq /etc/default file' do
expect(chef_run).to create_template("/etc/default/#{chef_run.node['rabbitmq']['service_name']}").with(
:user => 'root',
:group => 'root',
:source => 'default.rabbitmq-server.erb',
:mode => 00644
Copy link
Contributor

Choose a reason for hiding this comment

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

This one as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

The 5 digit should be fine. I believe this just forces the octet rule

)
end

describe 'suse' do
let(:runner) { ChefSpec::ServerRunner.new(SUSE_OPTS) }
let(:node) { runner.node }
Expand Down
14 changes: 14 additions & 0 deletions templates/default/default.rabbitmq-server.erb
@@ -0,0 +1,14 @@
###
# Generated by Chef
###

# This file is sourced by /etc/init.d/rabbitmq-server. Its primary

Choose a reason for hiding this comment

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

Add comment below in the beginning of the file?

%%%
%% Generated by Chef
%%%

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

# reason for existing is to allow adjustment of system limits for the
# rabbitmq-server process.
#
# Maximum number of open file handles. This will need to be increased
# to handle many simultaneous connections. Refer to the system
# documentation for ulimit (in man bash) for more information.
#
#ulimit -n 1024
<% if node['rabbitmq']['open_file_limit'] -%>ulimit -n <%= node['rabbitmq']['open_file_limit'] %><% end %>

Choose a reason for hiding this comment

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

Open question: Which attribute we should use here? node['rabbitmq']['open_file_limit'] or node['rabbitmq']['max_file_descriptors']?

Choose a reason for hiding this comment

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

Looks like we should use node['rabbitmq']['open_file_limit'] here as it has default value nil. node['rabbitmq']['max_file_descriptors'] has default value 1024 and is only used when node['rabbitmq']['job_control'] is upstart. 💔

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to depreciate the upstart feature flip. Chef-client 12 now supports the cough_ubuntu_cough crazyness.

2 changes: 0 additions & 2 deletions templates/default/rabbitmq-env.conf.erb
Expand Up @@ -18,5 +18,3 @@ export ERL_EPMD_ADDRESS=<%= node['rabbitmq']['erl_networking_bind_address'] %>
<% if node['rabbitmq']['config'] -%>CONFIG_FILE=<%= node['rabbitmq']['config'] %><% end %>
<% if node['rabbitmq']['logdir'] -%>LOG_BASE=<%= node['rabbitmq']['logdir'] %><% end %>
<% if node['rabbitmq']['mnesiadir'] -%>MNESIA_BASE=<%= node['rabbitmq']['mnesiadir'] %><% end %>

<% if node['rabbitmq']['open_file_limit'] -%>ulimit -n <%= node['rabbitmq']['open_file_limit'] %><% end %>
@@ -0,0 +1,24 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

require File.expand_path('../support/helpers', __FILE__)

describe 'rabbitmq_test::cook-openfiles' do
include Helpers::RabbitMQ

it 'properly sets open_file_limit' do
if node['rabbitmq']['open_file_limit']
command("grep 'Max open files' /proc/$(sudo pgrep -u rabbitmq beam)/limits | awk '{print $5}'").stdout.chomp == "#{node['rabbitmq']['open_file_limit']}"
end
end
end
26 changes: 26 additions & 0 deletions test/cookbooks/rabbitmq_test/recipes/cook-openfiles.rb
@@ -0,0 +1,26 @@
#
# Cookbook Name:: rabbitmq_test
# Recipe:: cook-openfiles
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

node.set['rabbitmq']['open_file_limit'] = 2048
include_recipe 'rabbitmq::default'

# HACK: Give rabbit time to spin up before the tests, it seems
# # to be responding that it has started before it really has
execute 'sleep 10' do
action :nothing
subscribes :run, "service[#{node['rabbitmq']['service_name']}]", :delayed
end