-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
base: main
Are you sure you want to change the base?
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
9a7b3ec
to
7b59893
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 change of getting a review? |
Follow-up for #5822 with a new CI check for podman.
Open Issues
ImagePullPolicyTest::pullsByDefault
ImagePullPolicyTest::shouldAlwaysPull
ImagePullPolicyTest::shouldSupportCustomPolicies
ImagePullPolicyTest::shouldCheckPolicy
AmbiguousImagePullTest::testNotUsingParse
DockerClientFactoryTest::runCommandInsideDockerShouldNotFailIfImageDoesNotExistsLocally
AuthenticatedImagePullTest::testThatAuthLocatorIsUsedForDockerfileBuild
AuthenticatedImagePullTest::testThatAuthLocatorIsUsedForContainerCreation
GenericContainerTest::shouldOnlyPublishExposedPorts
host
andnone
don't create network entries (see [Bug]: network modesnone
andhost
should create entries inNetworkSettings.Networks
containers/podman#17385)DockerNetworkModeTest