Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
re-review
  • Loading branch information
DeagleGross committed Jul 15, 2025
commit ac40f556b2c96cf46c05be94f44a559e83554d63
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Aspire.Hosting.ApplicationModel;

/// <summary>
/// Executable resource that runs in a container.
/// </summary>
internal class ContainerExecutableResource(string name, ContainerResource containerResource, string command, string? workingDirectory)
: Resource(name), IResourceWithEnvironment, IResourceWithArgs, IResourceWithEndpoints, IResourceWithWaitSupport
Copy link
Member

Choose a reason for hiding this comment

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

@DeagleGross this merged in before I got a chance to fully review it - but one thing we should do here is ditch the TargetContainerResource property and make this resource implement the IResourceWithParent interface.

Copy link
Member

Choose a reason for hiding this comment

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

We should also get rid of the Args property and instead make it work similar to the ExecutableResource. Because you are implementing IResourceWithArgs it means that this resource should be able to work with the WithArgs method - and it should honor all of the semantics around deferred evaluation etc.

{
Expand All @@ -19,8 +22,14 @@ internal class ContainerExecutableResource(string name, ContainerResource contai
/// </summary>
public string? WorkingDirectory { get; } = workingDirectory;

/// <summary>
/// Args of the command to run in the container.
/// </summary>
public ICollection<string>? Args { get; init; }

/// <summary>
/// Target container resource that this executable runs in.
/// </summary>
public ContainerResource? TargetContainerResource { get; } = containerResource ?? throw new ArgumentNullException(nameof(containerResource));

private static string ThrowIfNullOrEmpty([NotNull] string? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null)
Expand Down
1 change: 0 additions & 1 deletion src/Aspire.Hosting/Dcp/DcpExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,6 @@ await _executorEvents.PublishAsync(new OnResourceChangedContext(

try
{
// await CreateExecutableAsync(er, resourceLogger, cancellationToken).ConfigureAwait(false);
await createResourceFunc(er, resourceLogger, cancellationToken).ConfigureAwait(false);
}
catch (FailedToApplyEnvironmentException)
Expand Down
2 changes: 0 additions & 2 deletions src/Aspire.Hosting/Dcp/ResourceSnapshotBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,8 @@ public CustomResourceSnapshot ToSnapshot(ContainerExec executable, CustomResourc
State = state,
ExitCode = executable.Status?.ExitCode,
Properties = previous.Properties.SetResourcePropertyRange([
// new(KnownProperties.Executable.Path, executable.Spec.ExecutablePath),
new(KnownProperties.Executable.WorkDir, executable.Spec.WorkingDirectory),
new(KnownProperties.Executable.Args, executable.Status?.EffectiveArgs ?? []) { IsSensitive = true },
// new(KnownProperties.Executable.Pid, executable.Status?.ProcessId),
new(KnownProperties.Resource.AppArgs, launchArguments?.Args) { IsSensitive = launchArguments?.IsSensitive ?? false },
new(KnownProperties.Resource.AppArgsSensitivity, launchArguments?.ArgsAreSensitive) { IsSensitive = launchArguments?.IsSensitive ?? false },
]),
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Hosting/Exec/ExecResourceManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public async IAsyncEnumerable<CommandOutput> StreamExecResourceLogs([EnumeratorC

// hack: https://github.com/dotnet/aspire/issues/10245
// workarounds the race-condition between streaming all logs from the resource, and resource completion
await Task.Delay(2000, CancellationToken.None).ConfigureAwait(false);
await Task.Delay(1000, CancellationToken.None).ConfigureAwait(false);

_resourceLoggerService.Complete(dcpExecResourceName); // complete stops the `WatchAsync` async-foreach below
}, cancellationToken);
Expand Down
82 changes: 0 additions & 82 deletions tests/Aspire.Cli.Tests/E2E/ExecContainerResourceTests.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public static IDistributedApplicationTestingBuilder WithTestAndResourceLogging(t
{
builder.Services.AddXunitLogging(testOutputHelper);
builder.Services.AddLogging(builder => builder.AddFilter("Aspire.Hosting", LogLevel.Trace));
builder.Services.AddLogging(builder => builder.AddFilter("Aspire.Hosting.Dcp", LogLevel.Trace));
return builder;
}

Expand Down
Loading