-
Notifications
You must be signed in to change notification settings - Fork 514
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
Allow configuring volume behavior when using Aspire.Hosting.Testing #4462
Comments
I wonder if this is something that we can do in conjunction with DCP where we some how nominate a volume/bindmount as being volatile. So we still generate the volumes and bind mount but when DCP tears down the the container it also deletes the volume/bindmount contents. /cc @karolz-ms @danegsta |
Interesting idea. Currently volumes are never deleted automatically, but we could have an opt-in flag for cleaning them up, sort of like Containers have their persistent flag. Let us know if you want to pursue this idea and we can make it happen in DCP |
I think that's a good idea. It's likely required to make the randomization of volume names in tests reasonable. |
@karolz-ms I'll assign this to you for the DCP side first. Once you've got something that we can set in the spec for volumes/bindmounts then you can assign back to me and I'll work on the app model hook up. |
@mitchdenny Roger, makes sense. |
@karolz-ms sounds fair. What do folks think of doing a click-stop improvement in 8.1 of simply removing all volumes when using |
We can try an experiment with the |
Sorry I wasn't clear, I mean simply removing mount annotations from resources when under test. |
Oh, sounds like I completely misunderstood the ask here, sorry. You want the tests to use ephemeral volumes instead of persistent volumes, correct? Makes perfect sense. If I am not mistaken, this should work today with DCP. That is, you can have a @mitchdenny If my understanding of the ask is correct, there should be no work on the DCP orchestrator side to satisfy it. |
Ah that's wonderful to hear, thanks! So now it comes down to timing/resourcing/scheduling/sequencing 😄 Do we push to implement support for two volume behavior under test modes in 8.1: no volumes, and ephemeral volumes; or do we just for the initial click-stop of a "no volumes" behavior in 8.1 and introduce the ephemeral volumes option later, given that the ephemeral option is likely to be more work. Thoughts @mitchdenny @davidfowl? |
@DamianEdwards help me understand the ask a little bit more. So we've written a test case and the test case includes a This becomes a problem when running a test because you may execute it multiple times and you don't want the old data volume getting in the way. You could just not add a volume, and allow the container to use its own ephemeral storage without the path being bound. I don't think we have any resources today that require a volume mount that wouldn't be better served by a bind mount (e.g. Keycloak for realm import). HOWEVER ... sometimes you might have a resource that you created via an extension method and it sets up the volumes within the extension and you can't change that default behavior. Perhaps what we need here is an extension API in builder.AddSomeResourceThatAutomaticallyAddsAVolume(...)
.WithVolume("thevolume", mount => mount.IsEmphemeral = true); We would add a property to Thoughts? |
@mitchdenny the case is a bit more straightforward than that. In your AppHost project you have a container resource that uses a volume, e.g.: var pg = builder.AddPostgres("server").WithDataVolume();
var db = pg.AddDatabase("db"); This is done because you want the data to be persistent between runs during development. Then you write a test for your AppHost project using
In the samples repo tests, I have an extension method that the tests call that discovers volume annotations on resources and updates them so that they're scoped appropriately for the test, specifically:
The problem with this at the moment is that the second case above (randomized names for volumes shared across resources) results in volumes being created on each test run that are never cleaned up. That's where a new feature in DCP would likely help. A full solution here likely involves adding new API to
|
Moving to backlog. If we want to bring it into 9.0 then lets look at whether it can be squeezed in given our other priorities. |
Currently, when testing AppHost projects that include container resources configured to use volumes, the same volumes are used in the test as that are used in normal development. This isn't great because the test isn't as isolated from the normal running of the AppHost as it could be and could mutate the data in the volume.
We should allow for configuring the behavior of volumes when using
Aspire.Hosting.Testing
, and by default remove volumes from all container resources under test so that they are better isolated. Volume behavior should be configurable globally for all resources, or on a per resource basis (overriding the configured global behavior), by way of new API to support both scenarios.The text was updated successfully, but these errors were encountered: