Skip to content

Allow configuring the AlwaysPullPolicy #10188

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

Conversation

sebastian-steiner
Copy link
Contributor

This PR fixes a bug that did not allow configuring the default provided AlwaysPullPolicy through the properties files.

Previous behavior

When trying to configure the following testcontainers.properties:

pull.policy=org.testcontainers.images.CustomAlwaysPullPolicy

Running tests would fail with the following exception, because the configuration couldn't access the package-private constructor:

Configured ImagePullPolicy could not be loaded: org.testcontainers.images.AlwaysPullPolicy
java.lang.IllegalArgumentException: Configured ImagePullPolicy could not be loaded: org.testcontainers.images.AlwaysPullPolicy
	at org.testcontainers.images.PullPolicy.defaultPolicy(PullPolicy.java:46)
	at org.testcontainers.images.RemoteDockerImage.<init>(RemoteDockerImage.java:47)
	at org.testcontainers.containers.GenericContainer.<init>(GenericContainer.java:245)
	...
Caused by: java.lang.NoSuchMethodException: org.testcontainers.images.AlwaysPullPolicy.<init>()
	at java.base/java.lang.Class.getConstructor0(Class.java:3761)
	at java.base/java.lang.Class.getConstructor(Class.java:2442)
	at org.testcontainers.images.PullPolicy.defaultPolicy(PullPolicy.java:43)
	... 8 more

Expected behavior

With this fix, the AlwaysPullPolicy can be configured directly in withImagePullPolicy(PullPolicy.alwaysPull()) and through the properties files.

Docs change

I would also propose a small change to the docs to contain an example that is functioning without users needing to provide their own implementation for testing purposes. I'm not sure though if this is something that fits the overall docs, so feel free to not include it.

@sebastian-steiner sebastian-steiner requested a review from a team as a code owner April 17, 2025 09:54
@kiview kiview added type/bug and removed type/docs labels Apr 17, 2025
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

That's a good one, thanks @sebastian-steiner and thanks for also updating the docs accordingly.

Will wait for @eddumelendez to have a look before merging.

@sebastian-steiner
Copy link
Contributor Author

Thanks @kiview happy to hear that. :)
Is there anything to do for the two failing tests? It looks like they shouldn’t be connected to my changes.

@eddumelendez eddumelendez added this to the next milestone Apr 22, 2025
@eddumelendez eddumelendez merged commit 73e0637 into testcontainers:main Apr 22, 2025
106 checks passed
@eddumelendez
Copy link
Member

great catch! Thanks for your contribution, @sebastian-steiner !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants