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

fix(keycloak): tests on aarch64, use image from [jboss -> quay], change supported version [16+ -> 18+] #480

Merged

Conversation

max-pfeiffer
Copy link
Contributor

@max-pfeiffer max-pfeiffer commented Mar 15, 2024

fixes #451
fixes #483

Screenshot 2024-03-15 at 12 42 29

Copy link
Collaborator

@santi santi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Tested locally on my Apple Silicon M3 and works wonders 👍 Just a small nitty-gritty detail, decide yourself if you want to do it or not.

We should adapt the official Docker image.
Latest image versions has ARM support.
@max-pfeiffer max-pfeiffer force-pushed the bugfix/keycloak_tests branch from c90fbac to 83eb2ae Compare March 15, 2024 17:20
@matheusvnm
Copy link

matheusvnm commented Mar 19, 2024

@max-pfeiffer, allow me to give my 2 cents: The new Keycloak version has a specific endpoint for readiness checks (/health/ready as can be seen here). It can be activated by setting the KC_HEALTH_ENABLED=true. Why not use it to reliably check the state of the system?

You could do something like below:

def _configure(self):
    self.with_env('KEYCLOAK_ADMIN', self.username)
    self.with_env('KEYCLOAK_ADMIN_PASSWORD', self.password)
    self.with_env('KC_HEALTH_ENABLED', 'true')
    self.with_command('start-dev')

@wait_container_is_ready(requests.exceptions.ConnectionError, requests.exceptions.ReadTimeout)
def _connect(self):
    response = requests.get(f"{self.get_url()}/health/ready", timeout=1)
    response.raise_for_status()

@max-pfeiffer
Copy link
Contributor Author

max-pfeiffer commented Mar 19, 2024

@matheusvnm Good point! I was not aware that these endpoints became introduced. I agree: this would be a better way to implement a readiness probe.

Problem is here: it looks like these endpoints became introduced with v18.0. So we would cut off all the older versions which people might still be using.

But we also want to have ARM support here. Images with ARM64 support are available since v17.0, I just checked the container registry.

So supporting versions from 18.0 to 24.0.1 could be an acceptable trade off.

@santi
Copy link
Collaborator

santi commented Mar 20, 2024

Keycloak themselves only support the latest major version, so shrinking the supported version range to 18.x+ sounds fine by me.

@alexanderankin alexanderankin changed the title fix: keycloak tests for ARM CPUs, discontinued jboss/keycloak Docker image fix(keycloak): tests on aarch64, use image from [jboss -> quay], change supported version [16+ -> 18+] Mar 20, 2024
@alexanderankin
Copy link
Member

if anyone really needs ancient versions im sure we can put together some kind of code sample with extending the core class DockerContainer with whatever they need, but it doesn't need to be in the library that gets published to pypi. I think we have enough of a consensus on that to merge? i even remember seeing something about trying to get keycloak to maintain their own testcontainers, so im not trying to overthink this.

@alexanderankin alexanderankin merged commit 5758310 into testcontainers:main Mar 21, 2024
11 checks passed
alexanderankin pushed a commit that referenced this pull request Mar 24, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.2.0](testcontainers-v4.1.0...testcontainers-v4.2.0)
(2024-03-24)


### Features

* support influxdb
([#413](#413))
([13742a5](13742a5))


### Bug Fixes

* **arangodb:** tests to pass on ARM CPUs - change default image to
3.11.x where ARM image is published
([#479](#479))
([7b58a50](7b58a50))
* **core:** DinD issues
[#141](#141),
[#329](#329)
([#368](#368))
([b10d916](b10d916))
* **core:** raise an exception when docker compose fails to start
[#258](#258)
([#485](#485))
([d61af38](d61af38))
* **core:** use auto_remove=True with reaper instance
([#499](#499))
([274a400](274a400))
* **docs:** update the non-existent main.yml badge
([#493](#493))
([1d10c1c](1d10c1c))
* Fix the return type of `DockerContainer.get_logs`
([#487](#487))
([cd72f68](cd72f68))
* **keycloak:** tests on aarch64, use image from [jboss -> quay],
change supported version [16+ -> 18+]
([#480](#480))
([5758310](5758310))
* **postgres:** doctest
([#473](#473))
([c9c6f92](c9c6f92))
* read the docs build works again
([#496](#496))
([dfd1781](dfd1781))
* readthedocs build - take 1
([#495](#495))
([b3b9901](b3b9901))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
bearrito pushed a commit to bearrito/testcontainers-python that referenced this pull request Mar 30, 2024
…ge supported version [16+ -> 18+] (testcontainers#480)

- jboss/keycloak is discontinued, adapting the official Docker image:
https://quay.io/repository/keycloak/keycloak
- switched to the latest image version with ARM support

fixes testcontainers#451
fixes testcontainers#483

![Screenshot 2024-03-15 at 12 42
29](https://github.com/testcontainers/testcontainers-python/assets/13573675/f82b9372-a94e-45c8-a47b-7ddb4c1c6a57)
bearrito pushed a commit to bearrito/testcontainers-python that referenced this pull request Mar 30, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.2.0](testcontainers/testcontainers-python@testcontainers-v4.1.0...testcontainers-v4.2.0)
(2024-03-24)


### Features

* support influxdb
([testcontainers#413](testcontainers#413))
([13742a5](testcontainers@13742a5))


### Bug Fixes

* **arangodb:** tests to pass on ARM CPUs - change default image to
3.11.x where ARM image is published
([testcontainers#479](testcontainers#479))
([7b58a50](testcontainers@7b58a50))
* **core:** DinD issues
[testcontainers#141](testcontainers#141),
[testcontainers#329](testcontainers#329)
([testcontainers#368](testcontainers#368))
([b10d916](testcontainers@b10d916))
* **core:** raise an exception when docker compose fails to start
[testcontainers#258](testcontainers#258)
([testcontainers#485](testcontainers#485))
([d61af38](testcontainers@d61af38))
* **core:** use auto_remove=True with reaper instance
([testcontainers#499](testcontainers#499))
([274a400](testcontainers@274a400))
* **docs:** update the non-existent main.yml badge
([testcontainers#493](testcontainers#493))
([1d10c1c](testcontainers@1d10c1c))
* Fix the return type of `DockerContainer.get_logs`
([testcontainers#487](testcontainers#487))
([cd72f68](testcontainers@cd72f68))
* **keycloak:** tests on aarch64, use image from [jboss -&gt; quay],
change supported version [16+ -> 18+]
([testcontainers#480](testcontainers#480))
([5758310](testcontainers@5758310))
* **postgres:** doctest
([testcontainers#473](testcontainers#473))
([c9c6f92](testcontainers@c9c6f92))
* read the docs build works again
([testcontainers#496](testcontainers#496))
([dfd1781](testcontainers@dfd1781))
* readthedocs build - take 1
([testcontainers#495](testcontainers#495))
([b3b9901](testcontainers@b3b9901))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants