Skip to content
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

Split WithVolumeMount into two seperate methods #1437

Closed
stbau04 opened this issue Dec 16, 2023 · 7 comments · Fixed by #2245
Closed

Split WithVolumeMount into two seperate methods #1437

stbau04 opened this issue Dec 16, 2023 · 7 comments · Fixed by #2245
Assignees
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication area-app-model-offsite

Comments

@stbau04
Copy link
Contributor

stbau04 commented Dec 16, 2023

WithVolumeMount is mixing two different concepts: named volumes and bind mounts. They should probably be seperated into two methods.

Mentioned in #1132 and #837

Suggestion for usage:

var database = builder.AddPostgresContainer("database")
    .WithBindMount("../../data/database/data", "/var/lib/postgresql/data") // currenty equal to VolumeMountType.Bind
    .WithVolume("volume-name", "/var/lib/postgresql/data") // currenty equal to VolumeMountType.Named
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Dec 16, 2023
@josephaw1022
Copy link

What if the volume path for every aspire component had a default value for each resource (redis, postgres, rabbitmq, etc...)? And, you would only need to specify the name by default and the path was an optional parameter. With the approach that I am describing, the devs wouldn't need to look and figure out the volume path for containers but could change it if they wanted to.

builder.addPostgresContainer().withvolumemount("postgresVolume");

builder.addRabbitmqContainer().withvolumemount("rabbitmqVolume");

builder.addrediscache().withvolumemount("rabbitmqVolume", "");

@stbau04
Copy link
Contributor Author

stbau04 commented Dec 17, 2023

@josephaw1022 I think you are describing #837

@josephaw1022
Copy link

@stbau04 You're right. I misread it initially.

@mitchdenny
Copy link
Member

I'm exploring adding volumes as a first class resource to the Aspire application model. it still needs work so input is welcome.

#1521

@josephaw1022
Copy link

@mitchdenny, I think that would be interesting. I am curious exactly that would mean? Do you mean having them be independent of container creation and having more 1-1 mapping of methods to volume functionality? Or do you have something else in mind?

Not related in the sense that it's not directly related to volumes specifically, but I do think it relates to this
#1575
as it pushes the ball towards the existing principles in place with kubernetes and containers.

@stbau04
Copy link
Contributor Author

stbau04 commented Jan 7, 2024

Probably we should wait for #1521 and then see if this is still necessary

@mitchdenny
Copy link
Member

Tagging for offsite because we need to figure out what we are doing in the app model around volumes and how that translates to deployment.

@mitchdenny mitchdenny assigned JamesNK and unassigned mitchdenny Jan 29, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication area-app-model-offsite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants