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

Add support for multi-host Rabbitmq config #581

Merged
merged 10 commits into from
Nov 23, 2016

Conversation

dhgwilliam
Copy link
Contributor

@dhgwilliam dhgwilliam commented Nov 10, 2016

cluster config to be passed as array of hashes

puppet 3 doing something weird with passing the data around, improved munging may have fixed this or we may have found a weird edge case where an array that contains a single hash [{:k => :v}] gets converted in to an array of arrays [[:k, :v]], but if it contains more than one hash [{:k => :v}, {:k => :v}], it gets passed around correctly?

@@ -154,6 +154,10 @@
# Integer. The integer value for the RabbitMQ prefetch attribute
# Default: 1
#
# [*rabbitmq_cluster*]
# Array. Rabbitmq Cluster configuration and connection information for one or more Cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better described as "Array of hashes," similar to other parameters described as "Array of strings." In the interests of disambiguation, redis_sentinels parameter description probably needs the same change.

"Service[sensu-api]",
'Service[sensu-server]',
'Service[sensu-client]',
'Service[sensu-api]'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to (conditionally?) include Service[sensu-enterprise] here as well?

@cwjohnston cwjohnston changed the title [WIP] Add support for multi-host Rabbitmq config Add support for multi-host Rabbitmq config Nov 16, 2016
@cwjohnston
Copy link
Contributor

I have noticed that with repeated runs, the cluster is considered to be changed every time, even though I haven't made any changes to that portion of the manifest:

==> sensuserver-1-001-centos: Notice: /Stage[main]/Sensu::Rabbitmq::Config/Sensu_rabbitmq_config[sensuserver-1-001.vagrant.local]/cluster: cluster changed {'host' => '127.0.0.1', 'port'
 => '5672', 'prefetch' => '1', 'user' => 'sensu', 'vhost' => '/sensu'} to '{"host"=>"sensuserver-1-001.vagrant.local", "port"=>5672, "user"=>"sensu", "password"=>"secret", "vhost"=>"/se
nsu", "prefetch"=>100, "reconnect_on_error"=>true} {"host"=>"sensuserver-1-001.vagrant.local", "port"=>5672, "user"=>"sensu", "password"=>"secret", "vhost"=>"/sensu", "prefetch"=>100, "
reconnect_on_error"=>true}'

and delete prefetch default unit test which fails because we have
removed the default from the manifest, but left it in the type... in
other words, the catalog will not contain the default but it will be
flushed correctly to disk
@cwjohnston cwjohnston changed the title Add support for multi-host Rabbitmq config [WIP] Add support for multi-host Rabbitmq config Nov 16, 2016
@cwjohnston
Copy link
Contributor

I believe c1c6c8c has resolved the idempotence problem I described above.

@jaxxstorm @whermann I think this change should be considered for merge 😎

Closes #527
Closes #518
Closes #485
Closes #269

converting any value to_i fails on Symbols
@jaxxstorm
Copy link
Contributor

Amazing work! Great team effort and sincere thanks to @whermann @cwjohnston @dhgwilliam 👍

@jaxxstorm jaxxstorm merged commit dc81c22 into sensu:master Nov 23, 2016
@jaxxstorm jaxxstorm changed the title [WIP] Add support for multi-host Rabbitmq config Add support for multi-host Rabbitmq config Nov 27, 2016
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.

4 participants