-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add rootless podman support #6158
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
Add rootless podman support #6158
Conversation
0dedfbe to
174e350
Compare
|
@SoMuchForSubtlety thanks again for submitting a PR! However, Podman support should not alter the current tests. |
0f56dfa to
f565283
Compare
I reverted the test changes and skip them instead. |
f565283 to
67c04bc
Compare
|
I'll try to get the outstanding compatibility issues fixed in podman. I would be grateful if this PR could stay open to track the progress. Would it be OK to fix a (probable) bug in the tests? testcontainers-java/core/src/test/java/org/testcontainers/DockerRegistryContainer.java Lines 102 to 103 in 98ddbb8
According to the docker API spec, the repo parameter should not contain a tag. |
|
Thanks a lot for your work on this PR and your openness to collaboration @SoMuchForSubtlety, this is much appreciated. We can of course keep the PR open to track the progress. Regarding:
Of course, maybe even better as a small distinct PR, to decouple it from the Podman development work (since it is indeed independent of it). |
ae32866 to
eaef054
Compare
eaef054 to
10f285a
Compare
10f285a to
024512e
Compare
024512e to
9fea983
Compare
|
Podman 4.5.0 with the last required fix landed today, this is ready for review 🙂 |
7205750 to
2321321
Compare
|
Does it work w/ docker compose? |
No, because testcontainers currently uses the deprecated link feature for the compose ambassador container. If/when the implementation is changed to not use links, it should work, but that is out of scope for this PR. |
|
Got it, thanks @SoMuchForSubtlety |
f1bd0fa to
d1926a6
Compare
65a53e6 to
9b70281
Compare
4bd4287 to
80ad983
Compare
295a8e6 to
394f812
Compare
65cc501 to
c565edf
Compare
c565edf to
c0166ec
Compare
6fa1da4 to
ca94c0b
Compare
ca94c0b to
2e5b16c
Compare
The implementation is mostly borrowed from the rootless docker strategy.
The unqualified registries setting is no longer needed, see containers/podman#12435
2e5b16c to
945bc17
Compare
|
This PR has been ready for a while now, any chance of getting a review? |
|
Hi, thanks for the contribution. Our docs provides references to how to configure Testcontainers with other container runtimes already. Due to the existing list of issues related to podman, I am afraid we can not merge this PR. Container runtimes must be compatible with Docker APIs. |
|
This PR passes all tests with podman. What are your concerns? |
Follow-up for #5822 with a new CI check for podman.
Open Issues
ImagePullPolicyTest::pullsByDefaultImagePullPolicyTest::shouldAlwaysPullImagePullPolicyTest::shouldSupportCustomPoliciesImagePullPolicyTest::shouldCheckPolicyAmbiguousImagePullTest::testNotUsingParseDockerClientFactoryTest::runCommandInsideDockerShouldNotFailIfImageDoesNotExistsLocallyAuthenticatedImagePullTest::testThatAuthLocatorIsUsedForDockerfileBuildAuthenticatedImagePullTest::testThatAuthLocatorIsUsedForContainerCreationGenericContainerTest::shouldOnlyPublishExposedPortshostandnonedon't create network entries (see [Bug]: network modesnoneandhostshould create entries inNetworkSettings.Networkscontainers/podman#17385)DockerNetworkModeTest