Fix an issue when $node_ip_address is 'UNSET'.

[wyardley]

Fix an issue when $node_ip_address is 'UNSET'. Also, make curly-braces for conditionals consistent (and follow the style that I believe is used in puppet style guide).

See https://tickets.puppetlabs.com/browse/MODULES-3098 and
33dfa95

I did some basic testing, but should be tested some more for corner cases.

Also note that is_ipv6_address was introduced 2 months ago, so will cause problems for anyone with an older stdlib; if we wanted a simpler version, here's a patch against the commit above that I think will also work.

rabbitmqadmin.pp.patch.txt
stdlib.

[wyardley]

Not sure whether that build failure is related to these changes

Not done yet (but I can try to add): classes/rabbitmq_spec.rb around line 422 doesn't seem to have test cases with :node_ip_address as default (UNSET).

[wyardley]

Also opened rabbitmq/rabbitmq-server#692 seems that it might be simplest to just make rabbitmqadmin a file / template in puppet, even considering the issue of keeping it up to date. But seems like they're amenable to making the file part of the package (in which case, even if we need the default port / host to be right, should be fixable).

[madAndroid]

We're experiencing the same issue - would be great if this could be merged in.

[jaxxstorm]

This is a doubly breaking change because if you do set the node_ip_address, rabbitmqadmin fails because it's no longer listening on 127.0.01

[wyardley]

Would still appreciate someone reviewing this or providing feedback, but have merged changes from master, as well as made some updates to the code, so that the tests now pass.

[wyardley]

It also works to make a parameter for $noproxy_ip and pass that in (using the old style curl command line), but this is a little cleaner to me. I can implement it the other way if folks think that's better.

[wyardley]

@jaxxstorm This is a doubly breaking change because if you do set the node_ip_address, rabbitmqadmin fails because it's no longer listening on 127.0.01

I noticed the same thing when I was testing (though it's not important for my use case, because I want to bind to all interfaces anyway, which I would have thought was the most common use-case). But yeah, even though the tests pass, I don't think rabbitmqadmin will work when $node_ip_address is defined explicitly (except, presumably, when it's set to localhost).

assuming $node_ip_address maps to $RABBITMQ_NODE_IP_ADDRESS, might make some sense to use tcp_listeners instead, so that it could default to, or be configurable to, bind to localhost as well as one or more explicit IPs / interfaces https://www.rabbitmq.com/configure.html The tests do reference this, so maybe it's already doing it that way.

Maybe put in a separate bug report about that issue?

[jaxxstorm]

It does work, I'm using your MR in my fork with your changes pulled in, and it fixed my issue

[madAndroid]

We're also using this change in a fork of the module as well, works well so far.

[buzzdeee]

also works well for me on OpenBSD

[nibalizer]

We should definitely look at this, scanning the PR it looks fine to me.

[EmilienM]

It looks good to me. This change is really good, a lot of times I've seen people complaining about this UNSET thing, let's merge it.