Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion playground/mysql/MySqlDb.AppHost/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
var catalogDbName = "catalog"; // MySql database & table names are case-sensitive on non-Windows.
var catalogDb = builder.AddMySql("mysql")
.WithEnvironment("MYSQL_DATABASE", catalogDbName)
.WithVolumeMount("../MySql.ApiService/data", "/docker-entrypoint-initdb.d", VolumeMountType.Bind)
.WithBindMount("../MySql.ApiService/data", "/docker-entrypoint-initdb.d")
.WithPhpMyAdmin()
.AddDatabase(catalogDbName);

Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting.Azure/AzureResourceExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public static IResourceBuilder<AzureStorageEmulatorResource> UsePersistence(this
{
path = path ?? $".azurite/{builder.Resource.Name}";
var fullyQualifiedPath = Path.GetFullPath(path, builder.ApplicationBuilder.AppHostDirectory);
return builder.WithVolumeMount(fullyQualifiedPath, "/data", VolumeMountType.Bind, false);
return builder.WithBindMount(fullyQualifiedPath, "/data", isReadOnly: false);

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public static IResourceBuilder<AzureBicepStorageResource> UseEmulator(this IReso

if (storagePath is not null)
{
var volumeAnnotation = new VolumeMountAnnotation(storagePath, "/data", VolumeMountType.Bind, false);
var volumeAnnotation = new ContainerMountAnnotation(storagePath, "/data", ContainerMountType.Bind, false);
return builder.WithAnnotation(volumeAnnotation);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,45 @@
namespace Aspire.Hosting.ApplicationModel;

/// <summary>
/// Represents a volume mount annotation for a resource.
/// Represents a mount annotation for a container resource.
/// </summary>
[DebuggerDisplay("Type = {GetType().Name,nq}, Source = {Source}, Target = {Target}")]
public sealed class VolumeMountAnnotation : IResourceAnnotation
public sealed class ContainerMountAnnotation : IResourceAnnotation
{
/// <summary>
/// Instantiates a volume mount annotation that specifies the source and target paths for a volume mount.
/// Instantiates a mount annotation that specifies the source and target paths for a mount.
/// </summary>
/// <param name="source">The source path of the volume mount.</param>
/// <param name="target">The target path of the volume mount.</param>
/// <param name="type">The type of the volume mount.</param>
/// <param name="isReadOnly">A value indicating whether the volume mount is read-only.</param>
public VolumeMountAnnotation(string source, string target, VolumeMountType type = default, bool isReadOnly = false)
/// <param name="source">The source path of the mount.</param>
/// <param name="target">The target path of the mount.</param>
/// <param name="type">The type of the mount.</param>
/// <param name="isReadOnly">A value indicating whether the mount is read-only.</param>
public ContainerMountAnnotation(string? source, string target, ContainerMountType type, bool isReadOnly)
{
if (source == null && ContainerMountType.Bind == type)
{
throw new ArgumentException("The source path must be specified for a bind mount.", nameof(source));
}

Source = source;
Target = target;
Type = type;
IsReadOnly = isReadOnly;
}

/// <summary>
/// Gets or sets the source of the volume mount.
/// Gets or sets the source of the mount.
/// </summary>
public string Source { get; set; }
public string? Source { get; set; }

/// <summary>
/// Gets or sets the target of the volume mount.
/// Gets or sets the target of the mount.
/// </summary>
public string Target { get; set; }

/// <summary>
/// Gets or sets the type of the volume mount.
/// Gets or sets the type of the mount.
/// </summary>
public VolumeMountType Type { get; set; }
public ContainerMountType Type { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the volume mount is read-only.
Expand All @@ -48,9 +53,9 @@ public VolumeMountAnnotation(string source, string target, VolumeMountType type
}

/// <summary>
/// Represents the type of a volume mount.
/// Represents the type of a container mount.
/// </summary>
public enum VolumeMountType
public enum ContainerMountType
{
/// <summary>
/// A local folder that is mounted into the container.
Expand Down
26 changes: 20 additions & 6 deletions src/Aspire.Hosting/Dcp/ApplicationExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -716,19 +716,33 @@ private void PrepareContainers()
ctr.Annotate(Container.ResourceNameAnnotation, container.Name);
ctr.Annotate(Container.OtelServiceNameAnnotation, container.Name);

if (container.TryGetVolumeMounts(out var volumeMounts))
if (container.TryGetContainerMounts(out var containerMounts))
{
ctr.Spec.VolumeMounts = new();

foreach (var mount in volumeMounts)
foreach (var mount in containerMounts)
{
bool isBound = mount.Type == ApplicationModel.VolumeMountType.Bind;
var isBound = mount.Type == ContainerMountType.Bind;
var resolvedSource = mount.Source;
if (isBound)
{
// Source is only optional for creating anonymous volume mounts.
if (mount.Source == null)
{
throw new InvalidDataException($"Bind mount for container '{container.Name}' is missing required source.");
}

if (!Path.IsPathRooted(mount.Source))
{
resolvedSource = Path.GetFullPath(mount.Source);
}
}

var volumeSpec = new VolumeMount
{
Source = isBound && !Path.IsPathRooted(mount.Source) ?
Path.GetFullPath(mount.Source) : mount.Source,
Source = resolvedSource,
Target = mount.Target,
Type = isBound ? Model.VolumeMountType.Bind : Model.VolumeMountType.Named,
Type = isBound ? VolumeMountType.Bind : VolumeMountType.Volume,
IsReadOnly = mount.IsReadOnly
};
ctr.Spec.VolumeMounts.Add(volumeSpec);
Expand Down
4 changes: 2 additions & 2 deletions src/Aspire.Hosting/Dcp/Model/Container.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ internal static class VolumeMountType
// A volume mount to a host directory
public const string Bind = "bind";

// A volume mount to a named volume managed by the container orchestrator
public const string Named = "volume";
// A volume mount to a volume managed by the container orchestrator
public const string Volume = "volume";
}

internal sealed class VolumeMount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,28 @@ public static IResourceBuilder<ContainerResource> AddContainer(this IDistributed
/// </summary>
/// <typeparam name="T">The resource type.</typeparam>
/// <param name="builder">The resource builder.</param>
/// <param name="source">The source path of the volume. This is the physical location on the host.</param>
/// <param name="target">The target path in the container.</param>
/// <param name="type">The type of volume mount.</param>
/// <param name="source">The source name of the volume. If the name is <c>null</c> then an anonymous volume is mounted.</param>
/// <param name="target">The target path where the file or directory is mounted in the container.</param>
/// <param name="isReadOnly">A flag that indicates if this is a read-only mount.</param>
/// <returns>The <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<T> WithVolumeMount<T>(this IResourceBuilder<T> builder, string source, string target, VolumeMountType type = default, bool isReadOnly = false) where T : ContainerResource
public static IResourceBuilder<T> WithVolumeMount<T>(this IResourceBuilder<T> builder, string? source, string target, bool isReadOnly = false) where T : ContainerResource
{
var annotation = new VolumeMountAnnotation(source, target, type, isReadOnly);
var annotation = new ContainerMountAnnotation(source, target, ContainerMountType.Named, isReadOnly);
return builder.WithAnnotation(annotation);
}

/// <summary>
/// Adds a bind mount to a container resource.
/// </summary>
/// <typeparam name="T">The resource type.</typeparam>
/// <param name="builder">The resource builder.</param>
/// <param name="source">The source path of the mount. This is the path to the file or directory on the host.</param>
/// <param name="target">The target path where the file or directory is mounted in the container.</param>
/// <param name="isReadOnly">A flag that indicates if this is a read-only mount.</param>
/// <returns>The <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<T> WithBindMount<T>(this IResourceBuilder<T> builder, string source, string target, bool isReadOnly = false) where T : ContainerResource
{
var annotation = new ContainerMountAnnotation(source, target, ContainerMountType.Bind, isReadOnly);
return builder.WithAnnotation(annotation);
}

Expand Down
6 changes: 3 additions & 3 deletions src/Aspire.Hosting/Extensions/ResourceExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ public static bool TryGetEnvironmentVariables(this IResource resource, [NotNullW
}

/// <summary>
/// Attempts to get the volume mounts for the specified resource.
/// Attempts to get the container mounts for the specified resource.
/// </summary>
/// <param name="resource">The resource to get the volume mounts for.</param>
/// <param name="volumeMounts">When this method returns, contains the volume mounts for the specified resource, if found; otherwise, <c>null</c>.</param>
/// <returns><c>true</c> if the volume mounts were successfully retrieved; otherwise, <c>false</c>.</returns>
public static bool TryGetVolumeMounts(this IResource resource, [NotNullWhen(true)] out IEnumerable<VolumeMountAnnotation>? volumeMounts)
public static bool TryGetContainerMounts(this IResource resource, [NotNullWhen(true)] out IEnumerable<ContainerMountAnnotation>? volumeMounts)
{
return TryGetAnnotationsOfType<VolumeMountAnnotation>(resource, out volumeMounts);
return TryGetAnnotationsOfType<ContainerMountAnnotation>(resource, out volumeMounts);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/MySql/MySqlBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public static IResourceBuilder<T> WithPhpMyAdmin<T>(this IResourceBuilder<T> bui
builder.ApplicationBuilder.AddResource(phpMyAdminContainer)
.WithAnnotation(new ContainerImageAnnotation { Image = "phpmyadmin", Tag = "latest" })
.WithHttpEndpoint(containerPort: 80, hostPort: hostPort, name: containerName)
.WithVolumeMount(Path.GetTempFileName(), "/etc/phpmyadmin/config.user.inc.php")
.WithBindMount(Path.GetTempFileName(), "/etc/phpmyadmin/config.user.inc.php")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What was the intended volume mount type here? The previous WithVolumeMount usage would have created a bind mount (unintentionally?), but it looks like what was desired was a volume mount with a random name.

Can someone who knows this code confirm?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO in this, and the other case, a volume mount makes more sense, both for "I do not care about the data being deleted at the end of the run" scenario, as well as for "I want the data to be preserved" scenario.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If there isn't a decision made either way by someone who knows this code before this PR is ready to merge, then I'll create an issue to look into it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So in the code highlighted the file that is there is produced by the app model and injected into the container, so i think that bind mount is what is preferred - although it is an internal implementation detail.

.ExcludeFromManifest();

return builder;
Expand Down
4 changes: 2 additions & 2 deletions src/Aspire.Hosting/MySql/PhpMyAdminConfigWriterHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal class PhpMyAdminConfigWriterHook : IDistributedApplicationLifecycleHook
public Task AfterEndpointsAllocatedAsync(DistributedApplicationModel appModel, CancellationToken cancellationToken)
{
var adminResource = appModel.Resources.OfType<PhpMyAdminContainerResource>().Single();
var serverFileMount = adminResource.Annotations.OfType<VolumeMountAnnotation>().Single(v => v.Target == "/etc/phpmyadmin/config.user.inc.php");
var serverFileMount = adminResource.Annotations.OfType<ContainerMountAnnotation>().Single(v => v.Target == "/etc/phpmyadmin/config.user.inc.php");
var mySqlInstances = appModel.Resources.OfType<MySqlServerResource>();

if (appModel.Resources.OfType<PhpMyAdminContainerResource>().SingleOrDefault() is not { } myAdminResource)
Expand Down Expand Up @@ -42,7 +42,7 @@ public Task AfterEndpointsAllocatedAsync(DistributedApplicationModel appModel, C
}
else
{
using var stream = new FileStream(serverFileMount.Source, FileMode.Create);
using var stream = new FileStream(serverFileMount.Source!, FileMode.Create);
using var writer = new StreamWriter(stream);

writer.WriteLine("<?php");
Expand Down
4 changes: 2 additions & 2 deletions src/Aspire.Hosting/Postgres/PgAdminConfigWriterHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ internal class PgAdminConfigWriterHook : IDistributedApplicationLifecycleHook
public Task AfterEndpointsAllocatedAsync(DistributedApplicationModel appModel, CancellationToken cancellationToken)
{
var adminResource = appModel.Resources.OfType<PgAdminContainerResource>().Single();
var serverFileMount = adminResource.Annotations.OfType<VolumeMountAnnotation>().Single(v => v.Target == "/pgadmin4/servers.json");
var serverFileMount = adminResource.Annotations.OfType<ContainerMountAnnotation>().Single(v => v.Target == "/pgadmin4/servers.json");
var postgresInstances = appModel.Resources.OfType<PostgresServerResource>();

var serverFileBuilder = new StringBuilder();

using var stream = new FileStream(serverFileMount.Source, FileMode.Create);
using var stream = new FileStream(serverFileMount.Source!, FileMode.Create);
using var writer = new Utf8JsonWriter(stream);

var serverIndex = 1;
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/Postgres/PostgresBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static IResourceBuilder<T> WithPgAdmin<T>(this IResourceBuilder<T> builde
.WithAnnotation(new ContainerImageAnnotation { Image = "dpage/pgadmin4", Tag = "latest" })
.WithHttpEndpoint(containerPort: 80, hostPort: hostPort, name: containerName)
.WithEnvironment(SetPgAdminEnviromentVariables)
.WithVolumeMount(Path.GetTempFileName(), "/pgadmin4/servers.json")
.WithBindMount(Path.GetTempFileName(), "/pgadmin4/servers.json")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Same as previous comment. What mount type should this be?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be a bind mount as well.

.ExcludeFromManifest();

return builder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ public void AzureStorageUserEmulatorCallbackWithUsePersistenceResultsInVolumeAnn

var computedPath = Path.GetFullPath("mydata");

var volumeAnnotation = storage.Resource.Annotations.OfType<VolumeMountAnnotation>().Single();
var volumeAnnotation = storage.Resource.Annotations.OfType<ContainerMountAnnotation>().Single();
Assert.Equal(computedPath, volumeAnnotation.Source);
Assert.Equal("/data", volumeAnnotation.Target);
Assert.Equal(VolumeMountType.Bind, volumeAnnotation.Type);
Assert.Equal(ContainerMountType.Bind, volumeAnnotation.Type);
}

[Fact]
Expand Down
69 changes: 64 additions & 5 deletions tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Microsoft.Extensions.Logging;
using Xunit;
using Xunit.Abstractions;
using VolumeMountType = Aspire.Hosting.ApplicationModel.VolumeMountType;

namespace Aspire.Hosting.Tests;

Expand Down Expand Up @@ -326,13 +325,13 @@ public async Task VerifyDockerWithEntrypointWorks()
}

[LocalOnlyFact("docker")]
public async Task VerifyDockerWithBoundVolumeMountWorksWithAbsolutePaths()
public async Task VerifyDockerWithBindMountWorksWithAbsolutePaths()
{
var testProgram = CreateTestProgram();
testProgram.AppBuilder.Services.AddLogging(b => b.AddXunit(_testOutputHelper));

testProgram.AppBuilder.AddContainer("redis-cli", "redis")
.WithVolumeMount("/etc/path-here", $"path-here", VolumeMountType.Bind);
.WithBindMount("/etc/path-here", $"path-here");

await using var app = testProgram.Build();

Expand All @@ -356,13 +355,13 @@ public async Task VerifyDockerWithBoundVolumeMountWorksWithAbsolutePaths()
}

[LocalOnlyFact("docker")]
public async Task VerifyDockerWithBoundVolumeMountWorksWithRelativePaths()
public async Task VerifyDockerWithBindMountWorksWithRelativePaths()
{
var testProgram = CreateTestProgram();
testProgram.AppBuilder.Services.AddLogging(b => b.AddXunit(_testOutputHelper));

testProgram.AppBuilder.AddContainer("redis-cli", "redis")
.WithVolumeMount("etc/path-here", $"path-here", VolumeMountType.Bind);
.WithBindMount("etc/path-here", $"path-here");

await using var app = testProgram.Build();

Expand All @@ -386,6 +385,66 @@ public async Task VerifyDockerWithBoundVolumeMountWorksWithRelativePaths()
await app.StopAsync();
}

[LocalOnlyFact("docker")]
public async Task VerifyDockerWithVolumeMountWorksWithName()
{
var testProgram = CreateTestProgram();
testProgram.AppBuilder.Services.AddLogging(b => b.AddXunit(_testOutputHelper));

testProgram.AppBuilder.AddContainer("redis-cli", "redis")
.WithVolumeMount("test-volume-name", $"/path-here");

await using var app = testProgram.Build();

await app.StartAsync();

var s = app.Services.GetRequiredService<IKubernetesService>();

using var cts = new CancellationTokenSource(Debugger.IsAttached ? Timeout.InfiniteTimeSpan : TimeSpan.FromSeconds(10));
var token = cts.Token;

var redisContainer = await KubernetesHelper.GetResourceByNameAsync<Container>(
s,
"redis-cli", r => r.Spec.VolumeMounts != null,
token);

Assert.NotNull(redisContainer.Spec.VolumeMounts);
Assert.NotEmpty(redisContainer.Spec.VolumeMounts);
Assert.Equal("test-volume-name", redisContainer.Spec.VolumeMounts[0].Source);

await app.StopAsync();
}

[LocalOnlyFact("docker")]
public async Task VerifyDockerWithVolumeMountWorksWithoutName()
{
var testProgram = CreateTestProgram();
testProgram.AppBuilder.Services.AddLogging(b => b.AddXunit(_testOutputHelper));

testProgram.AppBuilder.AddContainer("redis-cli", "redis")
.WithVolumeMount(source: null, $"/path-here");

await using var app = testProgram.Build();

await app.StartAsync();

var s = app.Services.GetRequiredService<IKubernetesService>();

using var cts = new CancellationTokenSource(Debugger.IsAttached ? Timeout.InfiniteTimeSpan : TimeSpan.FromSeconds(10));
var token = cts.Token;

var redisContainer = await KubernetesHelper.GetResourceByNameAsync<Container>(
s,
"redis-cli", r => r.Spec.VolumeMounts != null,
token);

Assert.NotNull(redisContainer.Spec.VolumeMounts);
Assert.NotEmpty(redisContainer.Spec.VolumeMounts);
Assert.Equal("", redisContainer.Spec.VolumeMounts[0].Source);

await app.StopAsync();
}

[LocalOnlyFact("docker")]
public async Task KubernetesHasResourceNameForContainersAndExes()
{
Expand Down
6 changes: 3 additions & 3 deletions tests/Aspire.Hosting.Tests/MySql/AddMySqlTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public void WithPhpMyAdminAddsContainer()
builder.AddMySql("mySql").WithPhpMyAdmin();

var container = builder.Resources.Single(r => r.Name == "mySql-phpmyadmin");
var volume = container.Annotations.OfType<VolumeMountAnnotation>().Single();
var volume = container.Annotations.OfType<ContainerMountAnnotation>().Single();

Assert.True(File.Exists(volume.Source)); // File should exist, but will be empty.
Assert.Equal("/etc/phpmyadmin/config.user.inc.php", volume.Target);
Expand All @@ -223,15 +223,15 @@ public void WithPhpMyAdminProducesValidServerConfigFile()
mysql2.WithAnnotation(new AllocatedEndpointAnnotation("tcp", ProtocolType.Tcp, "host.docker.internal", 5002, "tcp"));

var myAdmin = builder.Resources.Single(r => r.Name.EndsWith("-phpmyadmin"));
var volume = myAdmin.Annotations.OfType<VolumeMountAnnotation>().Single();
var volume = myAdmin.Annotations.OfType<ContainerMountAnnotation>().Single();

var app = builder.Build();
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();

var hook = new PhpMyAdminConfigWriterHook();
hook.AfterEndpointsAllocatedAsync(appModel, CancellationToken.None);

using var stream = File.OpenRead(volume.Source);
using var stream = File.OpenRead(volume.Source!);
var fileContents = new StreamReader(stream).ReadToEnd();

// check to see that the two hosts are in the file
Expand Down
Loading