Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 20 commits into from
May 11, 2022
Merged

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

merged 20 commits into from
May 11, 2022

Conversation

jimbogithub
Copy link
Contributor

See #699 for prior discussion.

@jimbogithub
Copy link
Contributor Author

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
Copy link
Contributor Author

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

  • Are the changes to the test OK?
  • Having docker there just for testing is adding 150MB to the image size, but that's how it was before.

@wurstmeister
Copy link
Owner

wurstmeister commented Feb 26, 2022

Thanks for putting this together.

this change also switches from java 8 to java 11, as mentioned in #544 (comment), 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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@wurstmeister Can this be merged now?

@wurstmeister wurstmeister merged commit 9efdf10 into wurstmeister:master May 11, 2022
@wurstmeister
Copy link
Owner

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants