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

append .config to rabbitmq-env.conf.erb CONFIG_FILE line #505

Merged
merged 3 commits into from
Feb 15, 2019

Conversation

JamesAwesome
Copy link

This change is to update line #18 in templates/default/rabbitmq-env.conf.erb to append the .config suffix to the value of node['rabbitmq']['config']

As per https://github.com/rabbitmq/chef-cookbook/blob/v5.x/recipes/default.rb#L237 the generated config file will always be at #{node['rabbitmq']['config']}.config and not at simply node['rabbitmq']['config']

Without this change rabbitmq-env.conf will never properly handle a non-default setting for the value of node['rabbitmq']['config']

Copy link
Member

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

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

I'm not following. Modern RabbitMQ versions support config files with both a .config extension and without it but older versions explicitly require the extension to be missing (this is a runtime requirement/convention that RabbitMQ 3.7 works around).

I'm not sure why this change would make any difference other than making it impossible to use this cookbook with 3.6.x nodes.

@JamesAwesome
Copy link
Author

The issue i'm having is that when i set: node['rabbitmq']['config'] the cookbook generated file will be placed at #{node['rabbitmq']['config']}.config. However in rabbitmq-env.conf the line will read CONFIG_FILE=#{node['rabbitmq']['config']} without the extra .config on the end of the value.

so if node['rabbitmq']['config'] = /etc/rabbitmq/rabbitmq.conf then the file /etc/rabbitmq/rabbitmq.conf.config will be created by this cookbook.

However rabbitmq-env.conf will have the line CONFIG_FILE = /etc/rabbitmq/rabbitmq.conf

when i do this with rabbitmq 3.7.6 the startup says that the the loaded config file is config file(s) : /etc/rabbitmq/rabbitmq.conf which would appear to be incorrect

@michaelklishin
Copy link
Member

@JamesAwesome I see. I'd like to see a different solution. We should only append the .config extension if the file name doesn't already end in .conf or .config. I think that should be done across the board. WDYT?

@michaelklishin
Copy link
Member

In other words, it makes sense to me to use whatever value the user has configured if there's already an extension.

@JamesAwesome
Copy link
Author

I dig it. I can put in an update to this PR or open a separate issue for you. Let me know how you would like to proceed 👍

@michaelklishin
Copy link
Member

Let's amend this PR. Thank you!

@michaelklishin
Copy link
Member

@JamesAwesome I will proceed to release a new 5.x version with what we have, then we can do another one with the future PR we discussed.

@JamesAwesome
Copy link
Author

JamesAwesome commented Feb 14, 2019

Sure no problem. I just pushed my updates to this PR, should I close and reopen then?

@michaelklishin
Copy link
Member

I tried this with our example Chef cookbook repo and it failed with a message that said that config_path is not a known method (sorry, I accidentally cleared recent shell history).

Will investigate later today.

@michaelklishin
Copy link
Member

Here's the failure:

==> default: ================================================================================
==> default: Recipe Compile Error in /var/chef/cache/cookbooks/rabbitmq/recipes/default.rb
==> default: ================================================================================
==> default:
==> default:
==> default: NameError
==> default: ---------
==> default: undefined local variable or method `config_path' for cookbook: rabbitmq, recipe: default :Chef::Recipe
==> default:
==> default:
==> default: Cookbook Trace:
==> default: ---------------
==> default:   /var/chef/cache/cookbooks/rabbitmq/recipes/default.rb:240:in `from_file'
==> default:
==> default:
==> default: Relevant File Content:
==> default: ----------------------
==> default: /var/chef/cache/cookbooks/rabbitmq/recipes/default.rb:
==> default:
==> default: 233:    mode 00644
==> default: 234:    notifies :restart, "service[#{node['rabbitmq']['service_name']}]"
==> default: 235:    variables(
==> default: 236:      :config_path => config_path
==> default: 237:    )
==> default: 238:  end
==> default: 239:
==> default: 240>> template config_path do
==> default: 241:    sensitive true if Gem::Version.new(Chef::VERSION.to_s) >= Gem::Version.new('11.14.2')
==> default: 242:    source 'rabbitmq.config.erb'
==> default: 243:    cookbook node['rabbitmq']['config_template_cookbook']
==> default: 244:    owner 'root'
==> default: 245:    group 'root'
==> default: 246:    mode 00644
==> default: 247:    variables(
==> default: 248:      :kernel => format_kernel_parameters,
==> default: 249:      :ssl_versions => (format_ssl_versions if node['rabbitmq']['ssl_versions']),
==> default:
==> default: System Info:
==> default: ------------
==> default: chef_version=14.10.9
==> default: platform=ubuntu
==> default: platform_version=18.04
==> default: ruby=ruby 2.5.3p105 (2018-10-18 revision 65156) [x86_64-linux]
==> default: program_name=/usr/bin/chef-solo
==> default: executable=/opt/chef/bin/chef-solo
==> default:
==> default:
==> default: Running handlers:
==> default: [2019-02-15T08:07:47+00:00] ERROR: Running exception handlers
==> default: Running handlers complete
==> default: [2019-02-15T08:07:47+00:00] ERROR: Exception handlers complete
==> default: Chef Client failed. 0 resources updated in 03 seconds
==> default: [2019-02-15T08:07:47+00:00] FATAL: Stacktrace dumped to /var/chef/cache/chef-stacktrace.out
==> default: [2019-02-15T08:07:47+00:00] FATAL: Please provide the contents of the stacktrace.out file if you file a bug report
==> default: [2019-02-15T08:07:47+00:00] FATAL: NameError: undefined local variable or method `config_path' for cookbook: rabbitmq, recipe: default :Chef::Recipe

@michaelklishin
Copy link
Member

I got it to work locally, not 100% sure I'm following best modern Chef practices but there are OpsCode-maintained cookbooks that do the same, so it should be OK.

Will run test suites and push later today.

@JamesAwesome
Copy link
Author

I got it to work locally, not 100% sure I'm following best modern Chef practices but there are OpsCode-maintained cookbooks that do the same, so it should be OK.

haha, i had the same thoughts, kind of been away from chef for a bit! i figured i would try and just keep in the style that the cookbook already had.

Thanks for all the help!

@michaelklishin michaelklishin merged commit fa35155 into rabbitmq:v5.x Feb 15, 2019
@michaelklishin
Copy link
Member

@JamesAwesome please give it a shot.

@JamesAwesome
Copy link
Author

Workin' thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants