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

Added ability to resolve target port #3305

Merged
merged 3 commits into from
Apr 3, 2024
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
26 changes: 23 additions & 3 deletions src/Aspire.Hosting.Dapr/CommandLineBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

namespace Aspire.Hosting.Dapr;

internal delegate IEnumerable<string> CommandLineArgBuilder();
internal delegate IEnumerable<object> CommandLineArgBuilder();

internal sealed record CommandLine(string FileName, IEnumerable<string> Arguments)
internal sealed record CommandLine(string FileName, IEnumerable<object> Arguments)
{
public string ArgumentString
{
Expand Down Expand Up @@ -178,6 +178,26 @@ private static CommandLineArgBuilder ModelNamedStringArg(string name, string val
};
}

public static CommandLineArgBuilder ModelNamedObjectArg(string name, object value)
{
return () =>
{
return [name, value];
};
}

public static CommandLineArgBuilder ModelNamedObjectArg<T>(string name, object value, bool assignValue = false) where T : struct
{
return () =>
{
string? stringValue = Convert.ToString(value, CultureInfo.InvariantCulture);

return stringValue is not null
? ModelNamedStringArg(name, stringValue, assignValue)()
: Enumerable.Empty<string>();
};
}

public static CommandLineArgBuilder PostOptionsArgs(params CommandLineArgBuilder[] args)
{
return PostOptionsArgs(null, args);
Expand All @@ -190,7 +210,7 @@ public static CommandLineArgBuilder PostOptionsArgs(string? separator, params Co

public static CommandLineArgBuilder PostOptionsArgs(string? separator, IEnumerable<CommandLineArgBuilder> args)
{
IEnumerable<string> GeneratePostOptionsArgs()
IEnumerable<object> GeneratePostOptionsArgs()
{
bool postOptions = false;

Expand Down
24 changes: 16 additions & 8 deletions src/Aspire.Hosting.Dapr/DaprDistributedApplicationLifecycleHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ public async Task BeforeStartAsync(DistributedApplicationModel appModel, Cancell
}

var daprAppPortArg = (int? port) => ModelNamedArg("--app-port", port);
var daprGrpcPortArg = (string? port) => ModelNamedArg("--dapr-grpc-port", port);
var daprHttpPortArg = (string? port) => ModelNamedArg("--dapr-http-port", port);
var daprMetricsPortArg = (string? port) => ModelNamedArg("--metrics-port", port);
var daprProfilePortArg = (string? port) => ModelNamedArg("--profile-port", port);
var daprGrpcPortArg = (object port) => ModelNamedObjectArg("--dapr-grpc-port", port);
var daprHttpPortArg = (object port) => ModelNamedObjectArg("--dapr-http-port", port);
var daprMetricsPortArg = (object port) => ModelNamedObjectArg("--metrics-port", port);
var daprProfilePortArg = (object port) => ModelNamedObjectArg("--profile-port", port);
var daprAppChannelAddressArg = (string? address) => ModelNamedArg("--app-channel-address", address);

var appId = sidecarOptions?.AppId ?? resource.Name;
Expand Down Expand Up @@ -182,13 +182,21 @@ public async Task BeforeStartAsync(DistributedApplicationModel appModel, Cancell
}
}

updatedArgs.AddRange(daprGrpcPortArg($"{{{{- portForServing \"{daprCliResourceName}_grpc\" -}}}}")());
updatedArgs.AddRange(daprHttpPortArg($"{{{{- portForServing \"{daprCliResourceName}_http\" -}}}}")());
updatedArgs.AddRange(daprMetricsPortArg($"{{{{- portForServing \"{daprCliResourceName}_metrics\" -}}}}")());
var grpc = daprCli.GetEndpoint("grpc");
var http = daprCli.GetEndpoint("http");
var metrics = daprCli.GetEndpoint("metrics");

updatedArgs.AddRange(daprGrpcPortArg(grpc.Property(EndpointProperty.TargetPort))());
updatedArgs.AddRange(daprHttpPortArg(http.Property(EndpointProperty.TargetPort))());
updatedArgs.AddRange(daprMetricsPortArg(metrics.Property(EndpointProperty.TargetPort))());

if (sidecarOptions?.EnableProfiling == true)
{
updatedArgs.AddRange(daprProfilePortArg($"{{{{- portForServing \"{daprCliResourceName}_profile\" -}}}}")());
var profiling = daprCli.GetEndpoint("profiling");

updatedArgs.AddRange(daprProfilePortArg(profiling.Property(EndpointProperty.TargetPort))());
}

if (sidecarOptions?.AppChannelAddress is null && httpEndPoint is not null)
{
updatedArgs.AddRange(daprAppChannelAddressArg(httpEndPoint.Host)());
Expand Down
9 changes: 8 additions & 1 deletion src/Aspire.Hosting/ApplicationModel/AllocatedEndpoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public class AllocatedEndpoint
/// <param name="address">The IP address of the endpoint.</param>
/// <param name="containerHostAddress">The address of the container host.</param>
/// <param name="port">The port number of the endpoint.</param>
public AllocatedEndpoint(EndpointAnnotation endpoint, string address, int port, string? containerHostAddress = null)
/// <param name="dcpServiceName">The name of the DCP service.</param>
public AllocatedEndpoint(EndpointAnnotation endpoint, string address, int port, string? containerHostAddress = null, string? dcpServiceName = null)
{
ArgumentNullException.ThrowIfNull(endpoint);
ArgumentOutOfRangeException.ThrowIfLessThan(port, 1, nameof(port));
Expand All @@ -28,6 +29,7 @@ public AllocatedEndpoint(EndpointAnnotation endpoint, string address, int port,
Address = address;
ContainerHostAddress = containerHostAddress;
Port = port;
DcpServiceName = dcpServiceName;
}

/// <summary>
Expand Down Expand Up @@ -65,6 +67,11 @@ public AllocatedEndpoint(EndpointAnnotation endpoint, string address, int port,
/// </summary>
public string UriString => $"{UriScheme}://{EndPointString}";

/// <summary>
/// The associated service name created in DCP for this endpoint.
/// </summary>
public string? DcpServiceName { get; }
Comment on lines +71 to +73
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is a great public name in our application model.

Users don't really know (and shouldn't know IMO) about DCP at all. Is there a better name we can use here?

Copy link
Member

Choose a reason for hiding this comment

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

OrchestratorServiceName?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately this isn’t generic even if we change the property name (which is fine).

Also we have lots of dcp in the config, logging etc do we want to do a single pass?

Copy link
Member

Choose a reason for hiding this comment

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

Can we make it internal for now? We can always make it public later.

Copy link
Member Author

@davidfowl davidfowl Apr 3, 2024

Choose a reason for hiding this comment

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

That makes it impossible to write a test. This is difficult because we need something that can appear in string interpolated expression. It doesn’t need to be the dcp expression but it has to be something that we can detect and replace with the dcp expression, at runtime .

There’s another alternative that comes to mind:

Make it internal: Delete the property:

Don’t evaluate this object inside of the IValueProvider implementation, but have that object flow all the way out to evaluation phase (GetValue) in ApplicationExecutor.

I think that means we’d need to manually process ReferenceExpressions in GetValue, handling target port specially.


/// <summary>
/// Returns a string representation of the allocated endpoint URI.
/// </summary>
Expand Down
9 changes: 1 addition & 8 deletions src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ public sealed class EndpointAnnotation : IResourceAnnotation
/// <param name="port">Desired port for the service.</param>
/// <param name="targetPort">This is the port the resource is listening on. If the endpoint is used for the container, it is the container port.</param>
/// <param name="isExternal">Indicates that this endpoint should be exposed externally at publish time.</param>
/// <param name="env">The name of the environment variable that will be set to the port number of this endpoint.</param>
/// <param name="isProxied">Specifies if the endpoint will be proxied by DCP. Defaults to true.</param>
public EndpointAnnotation(ProtocolType protocol, string? uriScheme = null, string? transport = null, string? name = null, int? port = null, int? targetPort = null, bool? isExternal = null, string? env = null, bool isProxied = true)
public EndpointAnnotation(ProtocolType protocol, string? uriScheme = null, string? transport = null, string? name = null, int? port = null, int? targetPort = null, bool? isExternal = null, bool isProxied = true)
{
// If the URI scheme is null, we'll adopt either udp:// or tcp:// based on the
// protocol. If the name is null, we'll use the URI scheme as the default. This
Expand All @@ -47,7 +46,6 @@ public EndpointAnnotation(ProtocolType protocol, string? uriScheme = null, strin
Port = port;
TargetPort = targetPort ?? port;
IsExternal = isExternal ?? false;
EnvironmentVariable = env;
IsProxied = isProxied;
}

Expand Down Expand Up @@ -93,11 +91,6 @@ public string Transport
/// </summary>
public bool IsExternal { get; set; }

/// <summary>
/// The name of the environment variable that will be set to the port number of this endpoint.
/// </summary>
public string? EnvironmentVariable { get; set; }

/// <summary>
/// Indicates that this endpoint should be managed by DCP. This means it can be replicated and use a different port internally than the one publicly exposed.
/// Setting to false means the endpoint will be handled and exposed by the resource.
Expand Down
41 changes: 37 additions & 4 deletions src/Aspire.Hosting/ApplicationModel/EndpointReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ public sealed class EndpointReference : IManifestExpressionProvider, IValueProvi
/// </summary>
public bool IsAllocated => _isAllocated ??= GetAllocatedEndpoint() is not null;

/// <summary>
/// Gets a value indicating whether the endpoint exists.
/// </summary>
public bool Exists => GetEndpointAnnotation() is not null;

string IManifestExpressionProvider.ValueExpression => GetExpression();

ValueTask<string?> IValueProvider.GetValueAsync(CancellationToken cancellationToken) => new(Url);
Expand All @@ -49,6 +54,7 @@ internal string GetExpression(EndpointProperty property = EndpointProperty.Url)
EndpointProperty.Host or EndpointProperty.IPV4Host => "host",
EndpointProperty.Port => "port",
EndpointProperty.Scheme => "scheme",
EndpointProperty.TargetPort => "targetPort",
_ => throw new InvalidOperationException($"The property '{property}' is not supported for the endpoint '{EndpointName}'.")
};

Expand All @@ -72,6 +78,11 @@ public EndpointReferenceExpression Property(EndpointProperty property)
/// </summary>
public int Port => AllocatedEndpoint.Port;

/// <summary>
/// Gets the target port for this endpoint. If the port is dynamically allocated, this will return <see langword="null"/>.
/// </summary>
public int? TargetPort => EndpointAnnotation.TargetPort;

/// <summary>
/// Gets the host for this endpoint.
/// </summary>
Expand All @@ -85,14 +96,14 @@ public EndpointReferenceExpression Property(EndpointProperty property)
/// <summary>
/// Gets the scheme for this endpoint.
/// </summary>
public string Scheme => AllocatedEndpoint.UriScheme;
public string Scheme => EndpointAnnotation.UriScheme;

/// <summary>
/// Gets the URL for this endpoint.
/// </summary>
public string Url => AllocatedEndpoint.UriString;

private AllocatedEndpoint AllocatedEndpoint =>
internal AllocatedEndpoint AllocatedEndpoint =>
GetAllocatedEndpoint()
?? throw new InvalidOperationException($"The endpoint `{EndpointName}` is not allocated for the resource `{Resource.Name}`.");

Expand Down Expand Up @@ -158,7 +169,7 @@ public class EndpointReferenceExpression(EndpointReference endpointReference, En
/// Gets the value of the property of the endpoint.
/// </summary>
/// <param name="cancellationToken">A <see cref="CancellationToken"/>.</param>
/// <returns>A <see cref="String"/> containing the selected <see cref="EndpointProperty"/> value.</returns>
/// <returns>A <see cref="string"/> containing the selected <see cref="EndpointProperty"/> value.</returns>
/// <exception cref="InvalidOperationException">Throws when the selected <see cref="EndpointProperty"/> enumeration is not known.</exception>
public ValueTask<string?> GetValueAsync(CancellationToken cancellationToken) => Property switch
{
Expand All @@ -167,9 +178,27 @@ public class EndpointReferenceExpression(EndpointReference endpointReference, En
EndpointProperty.IPV4Host => new("127.0.0.1"),
EndpointProperty.Port => new(Endpoint.Port.ToString(CultureInfo.InvariantCulture)),
EndpointProperty.Scheme => new(Endpoint.Scheme),
EndpointProperty.TargetPort => new(ComputeTargetPort()),
_ => throw new InvalidOperationException($"The property '{Property}' is not supported for the endpoint '{Endpoint.EndpointName}'.")
};

private string? ComputeTargetPort()
{
// We have a target port, so we can return it directly.
if (Endpoint.TargetPort is int port)
{
return port.ToString(CultureInfo.InvariantCulture);
}

// There is no way to resolve the value of the target port until runtime. Even then, replicas make this very complex because
// the target port is not known until the replica is allocated.
// Instead, we return an expression that will be resolved at runtime by the orchestrator.
return $$$"""{{- portForServing "{{{DcpServiceName}}}" -}}""";
}

private string DcpServiceName => Endpoint.AllocatedEndpoint.DcpServiceName
?? throw new InvalidOperationException("The endpoint does not have an associated service name in the orchestrator.");

IEnumerable<object> IValueWithReferences.References => [Endpoint];
}

Expand Down Expand Up @@ -197,5 +226,9 @@ public enum EndpointProperty
/// <summary>
/// The scheme of the endpoint.
/// </summary>
Scheme
Scheme,
/// <summary>
/// The target port of the endpoint.
/// </summary>
TargetPort
}
Loading