Fix KafkaContainer not running as expected on ARM64 #223
Conversation
There are two issues with the current implementation.
The first is that the ARM64 image (`niciqy/cp-kafka-arm64`) is not
marked as a compatible substitute for the confluent image
(`confluentinc/cp-kafka`). This means a runtime failure executing tests.
The second is that `System.getProperty("os.arch")` does not return
`aarch64` on an x86 (AMD64) JVM being emulated by Rosetta 2 on MacOS
(for example because other parts of compilation/testing require an x86
JDK). End-users _probably_ want to use an ARM64 Kafka image in this
situation as they (probably) won't need an emulated Kafka container
running. A call to `sysctl` can determine if the current process (the
JVM) is running under Rosetta 2 and so if the underlying CPU is actually
ARM64.
These changes are being used internally and have worked (so far!) for
users on both x86 and ARM64 MacOS machines.
| @@ -27,7 +27,7 @@ class SchemaRegistrySpec extends AnyFlatSpec with ForAllTestContainer with Match | |||
| //a way to communicate containers | |||
| val network: Network = Network.newNetwork() | |||
|
|
|||
| val kafkaContainer: KafkaContainer = KafkaContainer.Def(DockerImageName.parse(s"confluentinc/cp-kafka:$kafkaVersion")).createContainer() | |||
| val kafkaContainer: KafkaContainer = KafkaContainer.Def().createContainer() | |||
There was a problem hiding this comment.
Shouldn't we pin the version? it uses the latest, right?
There was a problem hiding this comment.
Ah, I had just removed this because I thought it was redundant (and was leading to failures on my machine). It does subtly change the test, but it still passes. Previously it used version 6.1.1 of Kafka (it still uses this version for Schema Registry), now it uses version 7.0.1 (pinned, as defined in the KafkaContainer companion object).
There was a problem hiding this comment.
Ah, I see it now. Thanks for the clarification. We can remove kafkaVersion variable then.
| // It returns `0` when running natively and `1` when running the current process under Rosetta 2. | ||
| private lazy val runningUnderRosetta2: Boolean = | ||
| // ProcessLogger redirects any error output away from console to a noop receiver (effectively `/dev/null`) | ||
| Try("sysctl -n sysctl.proc_translated".!!(ProcessLogger(_ => ()))).toOption.exists(_.trim == "1") |
There was a problem hiding this comment.
Two quick questions:
- I'm curious how this is handled in the Java version of this container? Can we reuse it from there?
- Will it work on windows? Per my understanding it'll go to "runningUnderRosetta2" execution path
There was a problem hiding this comment.
- Ah, good question, I'll take a look and report back 👍
- I don't have a windows machine to hand but I am 99% sure it will return
false. TheTryblock should fail (the command should fail), so theOptionis empty and.existsalways returnsfalsein an empty option
There was a problem hiding this comment.
Ah, if you mean that it will hit the runningUnderRosetta2 path than you are correct, sorry. I am just assuming that the sysctl block will subsequently fail for any number of reasons, so we'll return false.
E: I'm also assuming that hitting this codeblock is OK because we're running in tests. If this were non-test code I'd probably try to find a way to avoid the sysctl call unless we were sure we were running on macOS.
There was a problem hiding this comment.
got it. I makes sense. Let's see how the java lib handles it and then leave it as it if they don't have a better solution.
There was a problem hiding this comment.
From what I can see testcontainers-java does nothing special at all: https://github.com/testcontainers/testcontainers-java/blob/dace7e4d750552bb3d57036b579318045e203931/modules/kafka/src/main/java/org/testcontainers/containers/KafkaContainer.java
E: I also did a quick scan of GH issues and can't see any that are relevant (so I don't think they'll add support for this at all, instead waiting on the main Confluent images to support ARM64)
E2: Confluent still haven't added ARM64 support unfortunately... confluentinc/kafka-images#80
There was a problem hiding this comment.
Got it! Thanks. Just one last thought - would it make sense to contribute this change to the java library so it’s consistently available in the java and Scala versions?
|
Thank you for your contributions! Left two quick comments |
There are two issues with the current implementation.
The first is that the ARM64 image (
niciqy/cp-kafka-arm64) is notmarked as a compatible substitute for the confluent image
(
confluentinc/cp-kafka). This means a runtime failure executing tests.The second is that
System.getProperty("os.arch")does not returnaarch64on an x86 (AMD64) JVM being emulated by Rosetta 2 on MacOS(for example because other parts of compilation/testing require an x86
JDK). End-users probably want to use an ARM64 Kafka image in this
situation as they (probably) won't need an emulated Kafka container
running. A call to
sysctlcan determine if the current process (theJVM) is running under Rosetta 2 and so if the underlying CPU is actually
ARM64.
These changes are being used internally and have worked (so far!) for
users on both x86 and ARM64 MacOS machines.