Kafka bind address puppet #27

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants

petevg commented Jul 21, 2016

@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)

petevg added some commits Jul 19, 2016

Added bind_addr value for server.properties
If set to something that ruby evaluates as true, will be written to
server.properties. Causes the server to listen on the ip/interface
indicated by the value of the string.
Member

kwmonroe commented Jul 21, 2016

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

@@ -25,7 +25,11 @@ broker.id=<%= @broker_id %>
port=<%= @port %>
# Hostname the broker will bind to. If not set, the server will bind to all interfaces
+<% if !@bind_addr.nil? && !@bind_addr.empty? %>
+host.name=<%= @bind_addr %>
@johnsca

johnsca Jul 22, 2016

Owner

host.name is deprecated in favor of listeners in the latest release of Kakfa (0.10.0). This is fine for now, as Bigtop is using 0.8.1, but it's something to keep in mind for the future.

@@ -191,6 +191,7 @@ hadoop::common::tez_jars: "/usr/lib/tez"
#kafka
kafka::server::port: "9092"
kafka::server::zookeeper_connection_string: "%{hiera('bigtop::hadoop_head_node')}:2181"
+kafka::server::bind_addr: ""
@johnsca

johnsca Jul 22, 2016

Owner

We were instructed not to put default values in cluster.yaml.

@petevg

petevg Jul 22, 2016

@johnsca What are we supposed to do instead? Do we just leave the value out of cluster.yaml?

Owner

johnsca commented Jul 22, 2016

@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.

Owner

johnsca commented Jul 22, 2016

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.

petevg commented Jul 25, 2016

Submitted upstream.

@petevg petevg closed this Jul 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment