wurstmeister / kafka-docker

Dockerfile for Apache Kafka
http://wurstmeister.github.io/kafka-docker/
Apache License 2.0
6.89k stars 2.73k forks source link

Switch from Alpine to Debian to get newer multi-arch JRE #703

Closed jimbogithub closed 2 years ago

jimbogithub commented 2 years ago

See https://github.com/wurstmeister/kafka-docker/issues/699 for prior discussion.

jimbogithub commented 2 years ago

This is about as far as I can get. The two ### BEGIN .... ### END blocks show the extra packages that had to be installed to make progress in the CI build. As far as my tests have found, none of that is necessary for the image to work correctly otherwise. Perhaps someone could refactor to use a derived image for the tests?

I also added libsnappy1v5 to the main list of packages in case that was related to the broken test reported as compact-1645575814 topic not configured correctly: 'compact-1645575814:compact' but it doesn't make any difference so I'm not sure if it's really needed. It seems to me that the test may be wrong...

I tried something similar to the failing test on a local container:

kafka-topics.sh --bootstrap-server $KAFKA_BOOTSTRAP_SERVERS --create --topic CompactTest --partitions 1 --config cleanup.policy=compact --config=compression.type=snappy --replication-factor 3

kafka-configs.sh --bootstrap-server $KAFKA_BOOTSTRAP_SERVERS --entity-type topics --entity-name CompactTest --describe 

Topic: CompactTest  TopicId: KxUmDJ3IQg2gku8Nos7jnQ PartitionCount: 1   ReplicationFactor: 3    Configs: compression.type=snappy,cleanup.policy=compact,segment.bytes=1073741824,max.message.bytes=2000000
    Topic: CompactTest  Partition: 0    Leader: 1001    Replicas: 1001,1003,1002    Isr: 1001,1003,1002

The test uses this command to confirm settings:

kafka-configs.sh --bootstrap-server $KAFKA_BOOTSTRAP_SERVERS --entity-type topics --entity-name CompactTest --describe | grep 'Configs for topic' | awk -F'cleanup.policy=' '{print $2}'

If I do the first bit I see good values:

kafka-configs.sh --bootstrap-server $KAFKA_BOOTSTRAP_SERVERS --entity-type topics --entity-name CompactTest --describe
Dynamic configs for topic CompactTest are:
  compression.type=snappy sensitive=false synonyms={DYNAMIC_TOPIC_CONFIG:compression.type=snappy, DEFAULT_CONFIG:compression.type=producer}
  cleanup.policy=compact sensitive=false synonyms={DYNAMIC_TOPIC_CONFIG:cleanup.policy=compact, DEFAULT_CONFIG:log.cleanup.policy=delete}

But if you include the grep, e.g:

kafka-configs.sh --bootstrap-server $KAFKA_BOOTSTRAP_SERVERS --entity-type topics --entity-name CompactTest --describe | grep 'Configs for topic'

then the result is blank and the test fails.

jimbogithub commented 2 years ago

OK, building after a few more whacks with the hammer.

wurstmeister commented 2 years ago

Thanks for putting this together.

this change also switches from java 8 to java 11, as mentioned in https://github.com/wurstmeister/kafka-docker/issues/544#issuecomment-568071737, java 11 has only been officially supported since 2.1.0 (https://issues.apache.org/jira/browse/KAFKA-7264, https://archive.apache.org/dist/kafka/2.1.0/RELEASE_NOTES.html) so we should drop support for 2.0.1 (https://github.com/wurstmeister/kafka-docker/blob/master/.travis.yml#L17)

tednaleid commented 2 years ago

Confirming that #703 is much more stable for me on my M1 macs than the current latest release. Are there any blockers on merging #703 to make that support part of latest?

Is the PR from sfat above enough to get this merged, or are there other things that would need to be done?

jimbogithub commented 2 years ago

This will also work with openjdk:17-jdk-slim which appears to be supported (https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=181308223 - we support the two most recent LTS versions and the most recent non LTS version) but it further bloats the image due to being a full JDK rather than JRE. If we care about the image size I'd be happy to make the JRE if someone else wanted to help figure out how to remove the embedded Docker required for CI tests. Do we know why the embedded Docker is there? Is it just to get the list of ports?

jimbogithub commented 2 years ago

@wurstmeister Can this be merged now?

wurstmeister commented 2 years ago

Thank you!