search

juju-solutions / bigtop

Mirror of Apache Bigtop
Apache License 2.0
0 stars 2 forks source link

Kafka bind address puppet #27

Closed pengale closed 8 years ago

pengale commented 8 years ago

@juju-solutions/bigdata

This is one of three pull requests for setting up the ability to bind kafka to an interface/ip address. I've run bundletester with the following permutations of 'bind_addr' in config.yaml:

1) Keeping the default value of null doesn't break anything, which is expected. 2) Passing in a value of 0.0.0.0 doesn't break anything, which is expected. 3) Passing in an address that doesn't match the address of the kafka server (e.g., 10.0.9.9) does break things.

My juju fu is not strong enough to setup a second interface on a machine and bind to that, but the above three tests do seem to indicate that the code does what I think that it does.

As part of this work, I also fixed up the kafka tests. It should be easy to get them to run, even if you aren't paying attention to anything other than having the right branch of layer-apache-bigtop-base checked out locally. (The branch is 'kafka-bind-address', incidentally)

kwmonroe commented 8 years ago

i don't understand why this is needed if https://github.com/juju-solutions/layer-apache-bigtop-base/pull/32 is in... Isn't this duplicating that functionality?

johnsca commented 8 years ago

@kwmonroe juju-solutions/layer-apache-bigtop-base#32 adds a post-deploy patch so that the charm can have this option available when deploying Apache Bigtop 1.1.0, while this PR requests that the option be added to Bigtop itself so that it is available, without a post-deploy patch, in subsequent releases (1.2.0+).

Clearly it's not ideal to apply patches post-deploy, but the alternative is for the charms to not be able to deploy 1.1.0 at all. The patches are a compromise. We should add documentation to bigtop-packages/src/charm/README.md clearly spelling out what patches are applied post-deployment for each Bigtop release (linking to the Jira tickets) and why.

johnsca commented 8 years ago

This PR should be against the upstream and follow the patch process outlined in the wiki.

We also need to update the apache-bigtop-base layer since this was updated after juju-solutions/layer-apache-bigtop-base#32 was merged.

pengale commented 8 years ago

Submitted upstream.