config-changed breaks configuration in Kafka

[Samuel Cozannet]

In the Kafka charm / utils.py the following block matches the config-changed hook:

def update_kafka_config(path, zks):
    with open(path) as fh:
        config = []
        for l in fh.readlines():
            if l.startswith('broker.id'):
                svc, seq = os.environ['JUJU_UNIT_NAME'].split('/', 1)
                config.append('broker.id=%s' % seq)
                continue
            elif l.startswith('zookeeper.connect'):
                # If no change just exit
                if l.split('=', 1) == zks:
                    return False
                config.append('zookeeper.connect=%s' % zks)
                continue
            config.append(l)
    with open(path, 'w') as fh:
        fh.write("\n".join(config))
    return True

We see that if the line starts with "zookeeper.connect" then it will be replaced by the list of ZooKeeper servers. Unfortunately there are 2 lines that start with zookeeper.connect.

############################# Zookeeper #############################

# Zookeeper connection string (see zookeeper docs for details).
# This is a comma separated host:port pairs, each corresponding to a zk
# server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002".
# You can also append an optional chroot string to the urls to specify the
# root directory for all kafka znodes.
# zookeeper.connect=localhost:2181
zookeeper.connect=localhost:2181

# Timeout in ms for connecting to zookeeper
zookeeper.connection.timeout.ms=1000000

As a consequence, applying config-changed will update that into:

############################# Zookeeper #############################

# Zookeeper connection string (see zookeeper docs for details).
# This is a comma separated host:port pairs, each corresponding to a zk
# server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002".
# You can also append an optional chroot string to the urls to specify the
# root directory for all kafka znodes.
zookeeper.connect=X.X.X.X:2181

# Timeout in ms for connecting to zookeeper
zookeeper.connect=X.X.X.X:2181

This is not desirable, as the timeout may actually be useful to debug a bad network state or kill nodes automagically on relation-broken / departed.

A better match for the line would be

def update_kafka_config(path, zks):
    with open(path) as fh:
        config = []
        for l in fh.readlines():
            if l.startswith('broker.id'):
                svc, seq = os.environ['JUJU_UNIT_NAME'].split('/', 1)
                config.append('broker.id=%s' % seq)
                continue
            elif l.startswith('zookeeper.connect='):
                # If no change just exit
                if l.split('=', 1) == zks:
                    return False
                config.append('zookeeper.connect=%s' % zks)
                continue
            config.append(l)
    with open(path, 'w') as fh:
        fh.write("\n".join(config))
    return True