feat: Add reuse support

[david-szabo97]

What does this PR do?

Why is it important?

Related issues

• Relates [Question] Use existing container if it exists #407

[netlify]

✅ Deploy Preview for testcontainers-dotnet ready!

Name	Link
🔨 Latest commit	bafb41b
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-dotnet/deploys/65abcb263272b90008ac526b
😎 Deploy Preview	https://deploy-preview-1051--testcontainers-dotnet.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[david-szabo97]

Currently, it's possible to use WithAutoRemove(true) and WithCleanUp(true) together with WithReuse(true). Should we add some restrictions in there to prevent incorrect configuration?

[HofmeisterAn]

Thanks for bringing this up and looking into it!

I like the idea of utilizing JSON serialization. Maybe we can build on top of that and ignore specific properties using JSON paths or at least implement something that removes configurations like the session ID, etc. before calculating the hash (the final create resource object that is sent to the Docker API requires those labels).

Currently, it's possible to use WithAutoRemove(true) and WithCleanUp(true) together with WithReuse(true). Should we add some restrictions in there to prevent incorrect configuration?

Yes, we should throw an exception for invalid configurations. I think we should do this in one of the Validate() base implementations.

[david-szabo97]

Thanks @HofmeisterAn for all your inputs! Rewrote the PR with those in mind.

In the end, the implementation:

• GetHash() added to IResourceConfiguration; each ResourceConfiguration must implement this function to provide reusability.
• WithReuse(true) only works if it's the last method called in the builder. WithReuse adds the label to the resource containing the hash. Therefore, if WithReuse is not the last method, the hash might change before calling Build(). An exception is thrown if the hash mismatches to help in debugging.
• I replaced System.Text.Json with Newtonsoft, which supports JSONPath and is also a mutable object. Which we could use to build a modifier pipeline for the JSON object. WithGetHashModifier(Action<JObject>) ?

Please let me know what you think about this implementation. Once we agree on an implementation, I'll implement the remaining GetHash() methods.

Does it make sense to have reusability for FutureDockerImage?

[HofmeisterAn]

Thank you for responding and addressing the input so quickly. I believe the overall implementation represents well what I had in mind 👍. We just need to improve a few things and probably find the right places for some other parts.

I have not tested the changes locally or debugged them to gain a better understanding yet. I will do that in the next few days. My suggestions are not set in stone; they are just inputs and ideas to iterate over. I am certain we will find better solutions together.

GetHash() added to IResourceConfiguration; each ResourceConfiguration must implement this function to provide reusability.

That makes sense; it is along the lines of what I had in mind too.

WithReuse(true) only works if it's the last method called in the builder. WithReuse adds the label to the resource containing the hash. Therefore, if WithReuse is not the last method, the hash might change before calling Build(). An exception is thrown if the hash mismatches to help in debugging.

We probably cannot rely on the order. Referring to some modules, it is necessary to adjust the configuration before returning the built resource. Usually, this is done to properly preconfigure the wait strategies, but I cannot promise it will not be used for other properties in the future. However, we can sort that out later; we will figure something out to append the reuse hash.

I replaced System.Text.Json with Newtonsoft, which supports JSONPath and is also a mutable object.

I would prefer to avoid adding another JSON library or additional dependencies in general. I try to minimize dependencies to make developers' lives easier, ensuring they do not encounter dependency clashes or similar issues.

Perhaps we can utilize annotations to configure JSON serialization, like JsonIgnore and JsonConverter (to ignore certain keys, etc.). Then we can simply serialze the configuration instance. I believe this has another advantage. It allows us to simply create the reuse hash for module configurations too.

Which we could use to build a modifier pipeline for the JSON object. WithGetHashModifier(Action<JObject>) ?

Yes, I like that idea as a follow-up. Allowing developers to provide their own hash implementation if necessary sounds good. Maybe we can use an interface for that.

Does it make sense to have reusability for FutureDockerImage?

I do not think it is necessary for images. Developers can already reuse them.

[kiview]

How is the reuse of volumes intended to work? By volume name? Or will volumes implement a getHash() function that takes their content into account?

It might be better to out-scope volumes for the initial implementation.

[HofmeisterAn]

How is the reuse of volumes intended to work? By volume name? Or will volumes implement a getHash() function that takes their content into account?

It might be better to out-scope volumes for the initial implementation.

If we follow the approach, TC for .NET will basically identify networks and volumes based on their names (not many other properties are included here in calculating the hash). We do not offer many other public APIs to create and configure volumes and networks. Essentially, the reuse hash prevents Ryuk from deleting them.

[kiview]

In general when using names for volumes and networks, as soon as reuse disabled, don't we run into potential issues with conflicting names?

Independent of this topics, can we please add the global opt-in mechanism for the reuse feature according to how tc-java does it? This mechanism would automatically integrate with Testcontainers Desktop's Enable Reusable Containers feature, which will set this flag.

[HofmeisterAn]

In general when using names for volumes and networks, as soon as reuse disabled, don't we run into potential issues with conflicting names?

Yep, it does. There are several ways we can deal with it, even make it configurable (.NET can partially already reuse existing resources), but for now, I would like to focus on generating and assigning the hash consistently. Anything afterward is just agreeing on the workflow.

Independent of this topics, can we please add the global opt-in mechanism for the reuse feature according to how tc-java does it?

👍

[david-szabo97]

@HofmeisterAn Removed Made a couple of changes. I'm not quite sure what we should JsonIgnore yet, but so far it seems to work fine with my simple test case. Added volumes and networks to the use case to cover more.

I've been thinking a lot about how to avoid relying on the order of the WithReuse. I figured out we could use a create parameter modifier for this. 7994956 let me know what you think about this, we can always revert it and figure out something better. TBH, I don't really like the implementation because of the reflection, but we can get rid of the reflection and just cast it to the right type. The current implementation is enough as a proof-of-concept.

[HofmeisterAn]

I'm not quite sure what we should JsonIgnore yet, but so far it seems to work fine with my simple test case.

I think the current configuration looks ok (we probably can ignore more). I will look into it more closely. Initially, we should only consider a few properties to keep it simple and gradually add support for more properties. For instance, enabling the ResourceMappings configuration would necessitate generating a hash value for the interface implementations as well (which I would like to avoid at the beginning).

I figured out we could use a create parameter modifier for this. 7994956 let me know what you think about this, we can always revert it and figure out something better.

I will take a look at it 👍.

[HofmeisterAn]

I reviewed the PR, tested a couple of things, and applied some changes and improvements. Overall, it looks very good already, just a few minor things left (I think).

I agree, it's not that easy to decide which properties should be ignored and which should not. As mentioned, I would prefer to opt-in supported properties, but unfortunately, this is not supported by System.Text.Json (compared to Json.NET). I prefer opt-in because I am certain that not all properties will support the reuse feature without custom serialization. Not being able to opt-in is especially unfortunate for modules (luckily, most of their properties are not composed types nor very complex). I ignored most of the properties for the generic builders, which we can undo soon as we are sure that they won't cause an issue in generating the hash (all necessary properties available (serializable) to calculate the hash).

What do you think about the latest changes?

TBH, I don't really like the implementation because of the reflection, but we can get rid of the reflection and just cast it to the right type.

I moved this part to the classes that create the actual resource. Then no reflection is necessary. It's a limitation of Docker.DotNet and the provided types. There's no inheritance; we don't need to create it artificially through reflection.

[HofmeisterAn]

@david-szabo97, thank you for the contribution and the efforts you put into this feature. In my opinion, it is a really great contribution. I did a few more underlying tests, and I believe it works very well already. It serves as a good foundation to continue adding new features (use cases). The next reasonable follow-ups will be:

• Adding the custom property testcontainers.reuse.enable (as Kevin mentioned).
• Including further builder configuration properties in calculating the hash value (using JSON custom serializers).

Thanks again 🙏.

[david-szabo97]

Thanks for finishing it up! Looking forward to the next release 🚀

[304NotModified]

When could we expect a release for this great feature? :)

[HofmeisterAn]

No ETA yet. I would like to include the upcoming changes regarding the logging builder API in the next version as well. If the remaining PRs take longer, I think I can publish a pre-release, if that helps. Please keep in mind that this feature is experimental, and not all modules may be compatible or work out of the box.

[mkedrzynskiAndea]

Having pre-release would be great

[304NotModified]

Pre-release would be great indeed!

[HofmeisterAn]

Here you are. Sorry for the delay; I wanted to ship the logging API changes in the beta version too. Before using the reuse feature, please double-check the docs and make sure you are familiar with the limitations.

[304NotModified]

Works great!

(Tested it with Testcontainers.MsSql and a fixed container name)