ansible-middleware / amq_streams

Apache License 2.0
9 stars 7 forks source link

Adds custom amq_streams_zookeeper_zookeeper_id to zookeeper.properties.j2 #101

Closed gbaufake closed 11 months ago

gbaufake commented 1 year ago
rpelisse commented 1 year ago

LGTM, but I defer to @rmarting expertise :)

rmarting commented 1 year ago

@gbaufake Did you test it? The implementation looks like good to me, but I would like to confirm if you tested the implementation with a playbook in a distributed environment. For example, using the configuration described in the issue.

Thanks

gbaufake commented 1 year ago

@rmarting Give me some hours and I can put here the test results.

rpelisse commented 1 year ago

@gbaufake Did the fix worked?

gbaufake commented 1 year ago

Found some corner cases, I will work on that

gbaufake commented 1 year ago

Great news! I have successfully resolved the issue of checking the default variable amq_streams_zookeeper_zookeeper_id.

Situation 1

With no definitions for amq_streams_zookeeper_zookeeper_id inventory:

host1 name="zookeeper1" 
host2 name="zookeeper2" 
host3 name="zookeeper3" 

service:

systemctl status amq_streams_zookeeper.service:
● amq_streams_zookeeper.service - amq_streams_zookeeper
   Loaded: loaded (/usr/lib/systemd/system/amq_streams_zookeeper.service; enabled; vendor preset: disabled)
   Active: active (running) since Tue 2023-11-07 23:41:19 UTC; 11min ago

amq_streams_zookeeper.properties

# List of servers which should be members of the Zookeeper cluster.
server.1=host1:2888:3888:participant;host1:2181
server.2=host2:2888:3888:participant;host2:2181
server.3=host3:2888:3888:participant;host3:2181

With definitions amq_streams_zookeeper_zookeeper_id on the inventory

inventory:

host1 name="zookeeper1" amq_streams_zookeeper_zookeeper_id="101"
host2 name="zookeeper2" amq_streams_zookeeper_zookeeper_id="102"
host3 name="zookeeper3" amq_streams_zookeeper_zookeeper_id="403"

After the installation service:

systemctl status amq_streams_zookeeper.service -l
● amq_streams_zookeeper.service - amq_streams_zookeeper
   Loaded: loaded (/usr/lib/systemd/system/amq_streams_zookeeper.service; enabled; vendor preset: disabled)
   Active: active (running) since Wed 2023-11-08 00:58:25 UTC; 4h 10min ago

amq_streams_zookeeper.properties

# List of servers which should be members of the Zookeeper cluster.
server.101=host1:2888:3888:participant;host1:2181
server.102=host2:2888:3888:participant;host2:2181
server.403=host3:2888:3888:participant;host3:2181
# Client-to-Server Authentication
requireClientAuthScheme=sasl
authProvider.101=org.apache.zookeeper.server.auth.SASLAuthenticationProvider
authProvider.102=org.apache.zookeeper.server.auth.SASLAuthenticationProvider
authProvider.403=org.apache.zookeeper.server.auth.SASLAuthenticationProvider

I hope it is correct now.

rpelisse commented 11 months ago

@gbaufake @rmarting Where are we on this PR?

gbaufake commented 11 months ago

From my part, we are done, and we could merge it.

I tested it when the zookeeper id was filled and the zookeeper id was not filled.

rmarting commented 11 months ago

LGTM!