Add auto cluster configuration support #718
Conversation
|
I'd like to regenerate the docs. Does someone have a one liner for me. I'm new to spec test and such. |
|
@dn1s: it's been a while, but I believe the rake tasks can do it for you: Or you can just run I forget if this happens automagically as part of the release process, but I think probably you shouldn't run it and commit as part of this PR. @bastelfreak might know more, or check in the voxpupuli channels on IRC / Slack. |
There was a problem hiding this comment.
From a quick look, this seems sensible to me. Have you tested it?
How do you deal with the situation when multiple nodes are getting built at the same time?
I seem to remember having had trouble with cookie changes, especially since the cookie presumably needs to be generated / provided ahead of being added to the Puppet config.
I haven't been working with RMQ or Puppet lately, not sure who can give additional feedback or approve this, but I'm cautiously +1 on this.
It does seem like the acceptance test failure shows an issue that's probably a real one - have you looked into that yet?
https://github.com/voxpupuli/puppet-rabbitmq#clustering should probably get updated (in README)?
| end | ||
| end | ||
|
|
||
| context 'with dashes in clustername/hostname' do |
There was a problem hiding this comment.
Are these from another provider? Are all these valid and expected for cluster names as well as for node names?
There was a problem hiding this comment.
Since the default of cluster name is actually the hostname of the node which you joined. I would guess all hostname restraints apply.
|
automatic doc gneration is currently not implemented. feedback in voxpupuli/modulesync_config#397 is highly apreciated. |
manifests/init.pp
Outdated
| Array $cluster_nodes = $rabbitmq::params::cluster_nodes, | ||
| String $config = $rabbitmq::params::config, | ||
| Boolean $config_cluster = $rabbitmq::params::config_cluster, | ||
| Hash $cluster = $rabbitmq::params::cluster, |
There was a problem hiding this comment.
can you also define the structure within the Hash please?
There was a problem hiding this comment.
And If possible, don’t change the formatting before that open paren - I think the param spacing also doesn’t need to change, which would make this easier to review?
There was a problem hiding this comment.
@bastelfreak I don't see a hash in the module that has a definition of it's inner structure should I nevertheless add one?
There was a problem hiding this comment.
There is none yet, please add one. Some examples for complex datatypes and tests are at: https://github.com/voxpupuli/puppet-nginx/pull/1233/files. Maybe that helps you.
bastelfreak
added
enhancement
|
please also rebase against our latest master branch to pull in some changes we did to the testmatrix. |
|
I'll have a look into the travis issue and your feedback in the next days. |
|
Hmm I thought a bit more of this pull request and it may be more sane to do this with autocluster on my side. And change this pr to just have a fact for cluster_name. Thoughts about this idea? |
|
It's a very useful feature. Any plans to add it soon? |
|
Rebased and submitted #736. I took a liberty with replacing create_resources with a lambda |
|
Discontinued in favor of #736. |
|
Hello, Any plans for adding this feature soon? |
|
@rehan2908: it's been moved to #736 and still needs rebase + tests. If someone provides those things, then yes, should be able to be added. |
Pull Request (PR) description
This PR adds two things:
This Pull Request (PR) fixes the following issues
Fixes #130