search

strimzi / strimzi-kafka-oauth

OAuth2 support for Apache Kafka® to work with many OAuth2 authorization servers
Apache License 2.0
143 stars 89 forks source link

Add s390x support to hydra testcase #144

Closed rposts closed 2 years ago

rposts commented 2 years ago

This PR address changes required to run hydra testcase on s390x architecture.

As per https://github.com/strimzi/strimzi-kafka-oauth/issues/124, a s390x maven profile is added to hydra-import testcase which will only be triggered on s390x architecture to build necessary images.

@mstruk FYI

mstruk commented 2 years ago

While the addition is not part of the main build (deployable jars), it is a part of the testsuite, which runs multiple times on every CI run.

Rather than pulling into the build the already built go-lang binary, the build of the binary from go-lang sources is performed as a part of the testsuite run, which probably makes this take quite some time. If this time is short (arbitrary number, but let's say 30s max) then I don't really see much of a problem. But make sure that the build is only performed if the image does not yet exist in the Docker container. The testsuite in CI runs multiple times with multiple Kafka versions. You don't want to perform the same build, producing the same binary again and again.

While I agree that this build doesn't quite belong there, it has to be somewhere, and if it works for @rposts I don't really object, as long as it doesn't affect any other platform (which based on the activation of the profile should be the case).

Ideally that part would be outside this project, but then - where?

And the binary of the build should then be uploaded somewhere to quay.io or hub.docker.com, where it's publicly available and should have a version that reflects the architecture in order to prevent confusion with default architectures (I guess). In that case our script would have to fetch the s390x version and re-tag it locally for the dependency to be met locally.

The current solution is convenient, but if the build takes a longer time the CI may terminate the testsuite run before it has a chance to complete, especially when testsuite run uses some older kafka version.

See: https://github.com/strimzi/strimzi-kafka-oauth/blob/main/.travis/build.sh for the actual script the CI executes on every run to get a better idea.

But as I said, I can not test this, and as long as it doesn't affect anything else, and works for you I'd say it's fine with me.

scholzj commented 2 years ago

I don't think you really need a separate repo. And we definitely don't want t build and push it under Strimzi. But I think the logic can be easily added for s390x runs into the ./travis directory to the other build scripts. It seems really weird to me to have it as part of the Maven build when it is just something for one platform which almost nobody uses.

mstruk commented 2 years ago

I see what you mean now.

Yeah, since the addition really has nothing to do with Java why even use maven to build it when the same can be done by using docker build command directly in a new script (let's call it .travis/prepare-s390x.sh) which can be invoked at the start of build.sh and would only execute if it determines that the build is running on s390x platform.

Let's do it like that then.

rposts commented 2 years ago

@mstruk - I have made some changes to the Travis build script where hydra image is only built for s390x arch when running the testcases. Currently, s390x arch does not have quay.io/strimzi/kafka:0.29.0-kafka-3.1.0 image available but it is expected to be there in the near future. I also think appropriate modifications will be needed in .travis.yml file to enable testing on s390x. Let's see if this brings us closer to addressing the original issue. Let me know your thoughts. Thanks!

rposts commented 2 years ago

@mstruk - I have pushed latest that incorporate your changes. Let me know if it looks ok. TravisCI is currently unresponsive for me but I will resubmit the job again. I am still debating whether s390x tests should be enabled in .travis.yml since existing arquillian framework does not work on s390x without considerable changes. Thanks.

rposts commented 2 years ago

@mstruk Let me know if I need to do anything else on my side. Tx!

mstruk commented 2 years ago

@rposts Do I understand correctly that the testsuite doesn't really work on s390x despite the changes you made, because you can not get Arquillian to work?

That would probably make this PR pointless, since the changes only come into play when testsuite is built and run?

rposts commented 2 years ago

@mstruk - <arquillian.cube.version>1.18.3-SNAPSHOT</arquillian.cube.version> (as opposed to existing <arquillian.cube.version>1.18.2</arquillian.cube.version> version in ../testsuite/pom.xml) works for s390x and connects with Docker but strangely it is unable to detect if Kafka server has started in TravisCI environment:

INFO: docker exec sh -c grep 'started (kafka.server.KafkaServer)' < /tmp/logs/server.log, exit status:1, stdout: Travis CI link

I do not have this issue in my local environment.

This is turning out to be more complicated than I earlier thought. However, I still think it will be useful to have s390x changes in place so that any future debugging can be made simpler.

mstruk commented 2 years ago

@rposts Very well. Let's merge it then. @scholzj WDYT?

scholzj commented 2 years ago

I guess first of all this needs to be rebased and we need to see the tests passing.

scholzj commented 2 years ago

@rposts Can you please rebase this so that the tests are triggered? Thanks.

rposts commented 2 years ago

Hi @scholzj - I will get to it shortly. Thanks!

rposts commented 2 years ago

@scholzj FYI - Travis CI in progress.

scholzj commented 2 years ago

Thanks @rposts