/bigtop forked from apache/bigtop

Kafka bind address fixed #28

Merged
merged 3 commits into from Jul 22, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@petevg @johnsca @kwmonroe
@petevg

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 20, 2016

@petevg
Fixed up tests. 
Tests were failing due to some ambiguities in the version of zookeeper
that we are deploying. Fixed them so that they work with my setup (juju
2.0, installing zookeeper from the charms store), and have some checks
that should make them work with other setups.

Also fixed a mistake, where the deploy test was calling self.unit, but
had set self.kafka.
3626ba6
@petevg
Added ability to override host.name (bind address) for kafka
4a95c0b
bigtop-packages/src/charm/kafka/layer-kafka/tests/01-deploy.py
@@ -33,9 +33,19 @@ def setUpClass(cls):
cls.d.configure('openjdk', {'java-type': 'jdk',
'java-major': '8'})
- cls.d.relate('kafka:zookeeper', 'zk:zkclient')
+ try:
@kwmonroe

kwmonroe Jul 21, 2016

Member

Let's just be backwards compatible without the try/except. Can we rename the relation to zookeeper instead of zkclient in the current bigtop zookeeper charm?

@johnsca

johnsca Jul 22, 2016

Owner

+1 to renaming the relation to be consistent. We haven't promulgated that yet, have we?

@petevg

petevg Jul 22, 2016

I believe that the old relation is "zkclient". The new relation is "zookeeper", which matches the name in the promulgated apache-zookeeper.

Basically, we decided not to be backwards compatible before my time :-p

I can change the new relation to be zkclient if you want, though.

@petevg
Got rid of try-catch in tests, and fixed defaults. 
config.yaml should default to null, rather than 0.0.0.0 (they both do
the same thing, basically, but 'null' results in nothing being written
to server.properties.
452101e

@johnsca johnsca merged commit 63c8a7c into kafka Jul 22, 2016

@kwmonroe kwmonroe deleted the kafka-bind-address-fixed branch Aug 11, 2016

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