Skip to content

Commit

Permalink
Small refactor and new tests (#3835)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabiocav @kshyju
fabiocav authored and kshyju committed Nov 4, 2024
1 parent 26b8dbc commit 5b7f4c7
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 94 deletions.
134 changes: 52 additions & 82 deletions src/Azure.Functions.Cli/Actions/HostActions/StartHostAction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
using Microsoft.Azure.WebJobs.Script.Configuration;
using Microsoft.Azure.WebJobs.Script.Description;
using Microsoft.Azure.WebJobs.Script.WebHost;
using Microsoft.Azure.WebJobs.Script.Workers;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -357,42 +356,21 @@ private void UpdateEnvironmentVariables(IDictionary<string, string> secrets)
/// <summary>
/// Check local.settings.json to determine whether in-proc .NET8 is enabled.
/// </summary>
private async Task<bool> IsInProcNet8Enabled()
private async Task<bool> IsInProcDotNet8Enabled()
{
var localSettingsJObject = await GetLocalSettingsJsonAsJObjectAsync();
var inProcNet8EnabledSettingValue = localSettingsJObject?["Values"]?[Constants.FunctionsInProcNet8Enabled]?.Value<string>();
var isInProcNet8Enabled = string.Equals("1", inProcNet8EnabledSettingValue);
var inProcDotNet8EnabledValue = localSettingsJObject?["Values"]?[Constants.InProcDotNet8EnabledSetting]?.Value<string>();
var isInProcDotNet8Enabled = string.Equals("1", inProcDotNet8EnabledValue, StringComparison.Ordinal);

if (VerboseLogging == true)
{
var message = isInProcNet8Enabled
? $"{Constants.FunctionsInProcNet8Enabled} app setting enabled in local.settings.json"
: $"{Constants.FunctionsInProcNet8Enabled} app setting is not enabled in local.settings.json";
var message = isInProcDotNet8Enabled
? $"{Constants.InProcDotNet8EnabledSetting} app setting enabled in local.settings.json"
: $"{Constants.InProcDotNet8EnabledSetting} app setting is not enabled in local.settings.json";
ColoredConsole.WriteLine(VerboseColor(message));
}

return isInProcNet8Enabled;
}

// We launch the in-proc .NET8 application as a child process only if the SkipInProcessHost conditional compilation symbol is not defined.
// During build, we pass SkipInProcessHost=True only for artifacts used by our feed (we don't want to launch child process in that case).
private bool ShouldLaunchInProcNet8AsChildProcess()
{
#if SkipInProcessHost
if (VerboseLogging == true)
{
ColoredConsole.WriteLine(VerboseColor("SkipInProcessHost compilation symbol is defined."));
}

return false;
#else
if (VerboseLogging == true)
{
ColoredConsole.WriteLine(VerboseColor("SkipInProcessHost compilation symbol is not defined."));
}

return true;
#endif
return isInProcDotNet8Enabled;
}

private async Task<JObject> GetLocalSettingsJsonAsJObjectAsync()
Expand Down Expand Up @@ -423,8 +401,8 @@ public override async Task RunAsync()
await PreRunConditions();
var isVerbose = VerboseLogging.HasValue && VerboseLogging.Value;

// Return if we have already started a child process
if (await ShouldExitAfterDeterminingHostRuntime(isVerbose))
// Return if running is delegated to another version of Core Tools
if (await TryHandleInProcDotNetLaunchAsync())
{
return;
}
Expand Down Expand Up @@ -482,98 +460,90 @@ public override async Task RunAsync()
await runTask;
}

private async Task<bool> ShouldExitAfterDeterminingHostRuntime(bool isVerbose)
private async Task<bool> TryHandleInProcDotNetLaunchAsync()
{
var isInProc = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnet;

// If --runtime param is set, handle runtime param logic; otherwise we infer the host to launch on startup
if (HostRuntime is not null)
{
// Validate host runtime passed in
await ValidateHostRuntime(isInProc, isVerbose);
await ValidateHostRuntimeAsync(GlobalCoreToolsSettings.CurrentWorkerRuntime);

if (!string.Equals(HostRuntime, "default", StringComparison.OrdinalIgnoreCase))
{
var isNet8InProcSpecified = string.Equals(HostRuntime, DotnetConstants.InProc8HostRuntime, StringComparison.OrdinalIgnoreCase);
StartHostAsChildProcess(isNet8InProcSpecified);
return true;
}

return false;
}
else if (isInProc)
else if (GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnet)
{
// Infer host runtime by checking if .NET 8 is enabled
var shouldNet8InProcBeLaunched = await IsInProcNet8Enabled();
var isDotNet8Project = await IsInProcDotNet8Enabled();

if (shouldNet8InProcBeLaunched)
{
PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc8HostRuntime);
}
// Otherwise start .NET 6 child process since we are running an inproc app
else
{
PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc6HostRuntime);
}
var selectedRuntime = isDotNet8Project
? DotnetConstants.InProc8HostRuntime
: DotnetConstants.InProc6HostRuntime;

PrintVerboseForHostSelection(selectedRuntime);

StartHostAsChildProcess(shouldNet8InProcBeLaunched);
return true;
StartHostAsChildProcess(isDotNet8Project);

return true;
}

// If the host runtime parameter is not set and the app is not in-proc, we should default to out-of-process host
PrintVerboseForHostSelection(isVerbose, "out-of-process");
PrintVerboseForHostSelection("out-of-process");
return false;
}

private async Task ValidateHostRuntime(bool isInProc, bool isVerbose)
internal async Task ValidateHostRuntimeAsync(WorkerRuntime currentWorkerRuntime,
Func<Task<bool>> validateDotNet8ProjectEnablement = null)
{
var isOutOfProc = GlobalCoreToolsSettings.CurrentWorkerRuntime == WorkerRuntime.dotnetIsolated;
var isInProc6 = string.Equals(HostRuntime, DotnetConstants.InProc6HostRuntime, StringComparison.OrdinalIgnoreCase);
var isInProc8 = string.Equals(HostRuntime, DotnetConstants.InProc8HostRuntime, StringComparison.OrdinalIgnoreCase);
validateDotNet8ProjectEnablement ??= IsInProcDotNet8Enabled;

// If we are running an .NET isolated app and the user specifies inproc6 or inproc8, throw an error
if (isOutOfProc && (isInProc8 || isInProc6))
void ThrowCliException(string suffix)
{
throw new CliException($"The runtime host value passed in, {HostRuntime}, is not a valid host version for your project. The host runtime is only valid for the worker runtime {WorkerRuntime.dotnet}");
throw new CliException($"The runtime argument value provided, '{HostRuntime}', is invalid. {suffix}");
}
// If we are not running a .NET app and the user specifies inproc6 or inproc8, throw an error
if (!isOutOfProc && !isInProc && (isInProc8 || isInProc6))

if (DotnetConstants.ValidRuntimeValues.Contains(HostRuntime, StringComparer.OrdinalIgnoreCase) == false)
{
throw new CliException($"The runtime host value passed in, {HostRuntime}, is not a valid host version for your project. The runtime is only valid for {WorkerRuntime.dotnetIsolated} and {WorkerRuntime.dotnet}");
ThrowCliException($"Valid values are '{string.Join("', '", DotnetConstants.ValidRuntimeValues)}'.");
}

if (string.Equals(HostRuntime, "default", StringComparison.OrdinalIgnoreCase))
var isInproc6ArgumentValue = string.Equals(HostRuntime, DotnetConstants.InProc6HostRuntime, StringComparison.OrdinalIgnoreCase);
var isInproc8ArgumentValue = string.Equals(HostRuntime, DotnetConstants.InProc8HostRuntime, StringComparison.OrdinalIgnoreCase);

if (currentWorkerRuntime == WorkerRuntime.dotnet)
{
if (isInProc)
if (string.Equals(HostRuntime, "default", StringComparison.OrdinalIgnoreCase))
{
throw new CliException($"The runtime host value passed in, default, is not a valid host version for your project. For the default host runtime, the worker runtime must be set to {WorkerRuntime.dotnetIsolated}.");
ThrowCliException($"The provided value is only valid for the worker runtime '{WorkerRuntime.dotnetIsolated}'.");
}
PrintVerboseForHostSelection(isVerbose, "out-of-process");
return;
}
else if (isInProc8)
{
if (!await IsInProcNet8Enabled())

if (isInproc8ArgumentValue && !await validateDotNet8ProjectEnablement())
{
throw new CliException($"The runtime host value passed in, {DotnetConstants.InProc8HostRuntime}, is not a valid host version for your project. For the {DotnetConstants.InProc8HostRuntime} runtime, the {Constants.FunctionsInProcNet8Enabled} variable must be set while running a .NET 8 in-proc app.");
ThrowCliException($"For the '{DotnetConstants.InProc8HostRuntime}' runtime, the '{Constants.InProcDotNet8EnabledSetting}' environment variable must be set. See https://aka.ms/azure-functions/dotnet/net8-in-process.");
}
PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc8HostRuntime);
return;
}
else if (isInProc6)
{
if (await IsInProcNet8Enabled())
else if (isInproc6ArgumentValue && await validateDotNet8ProjectEnablement())
{
throw new CliException($"The runtime host value passed in, {DotnetConstants.InProc6HostRuntime}, is not a valid host version for your project. For the {DotnetConstants.InProc6HostRuntime} runtime, the {Constants.FunctionsInProcNet8Enabled} variable must not be set while running a .NET 6 in-proc app.");
ThrowCliException($"For the '{DotnetConstants.InProc6HostRuntime}' runtime, the '{Constants.InProcDotNet8EnabledSetting}' environment variable cannot be be set. See https://aka.ms/azure-functions/dotnet/net8-in-process.");
}
PrintVerboseForHostSelection(isVerbose, DotnetConstants.InProc6HostRuntime);
return;

}
else if (isInproc8ArgumentValue || isInproc6ArgumentValue)
{
ThrowCliException($"The provided value is only valid for the worker runtime '{WorkerRuntime.dotnet}'.");
}
// Throw an exception if HostRuntime is set to none of the expected values
throw new CliException($"Invalid host runtime '{HostRuntime}'. Valid values are 'default', '{DotnetConstants.InProc8HostRuntime}', '{DotnetConstants.InProc6HostRuntime}'.");

PrintVerboseForHostSelection(HostRuntime);
}

private void PrintVerboseForHostSelection(bool isVerbose, string hostRuntime)
private void PrintVerboseForHostSelection(string hostRuntime)
{
if (isVerbose)
if (VerboseLogging.GetValueOrDefault())
{
ColoredConsole.WriteLine(VerboseColor($"Selected {hostRuntime} host."));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Azure.Functions.Cli/Common/Constants.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ internal static class Constants
public const string LocalSettingsJsonFileName = "local.settings.json";
public const string EnableWorkerIndexEnvironmentVariableName = "FunctionsHostingConfig__WORKER_INDEXING_ENABLED";
public const string Dotnet = "dotnet";
public const string FunctionsInProcNet8Enabled = "FUNCTIONS_INPROC_NET8_ENABLED";
public const string InProcDotNet8EnabledSetting = "FUNCTIONS_INPROC_NET8_ENABLED";
public const string AzureDevSessionsRemoteHostName = "AzureDevSessionsRemoteHostName";
public const string AzureDevSessionsPortSuffixPlaceholder = "<port>";
// Sample format https://n12abc3t-<port>.asse.devtunnels.ms/
Expand Down
2 changes: 2 additions & 0 deletions src/Azure.Functions.Cli/Common/DotnetConstants.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ internal static class DotnetConstants
public const string InProc6DirectoryName = "in-proc6";
public const string InProc8HostRuntime = "inproc8";
public const string InProc6HostRuntime = "inproc6";

public static readonly string[] ValidRuntimeValues = [InProc8HostRuntime, InProc6HostRuntime, "default"];
}
}
63 changes: 62 additions & 1 deletion test/Azure.Functions.Cli.Tests/ActionsTests/StartHostActionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@
using System.Threading.Tasks;
using Azure.Functions.Cli.Actions.HostActions;
using Azure.Functions.Cli.Common;
using Azure.Functions.Cli.Helpers;
using Azure.Functions.Cli.Interfaces;
using Colors.Net;
using FluentAssertions;
using Moq;
using NSubstitute;
using Xunit;
using YamlDotNet.Core;

namespace Azure.Functions.Cli.Tests.ActionsTests
{
Expand Down Expand Up @@ -44,7 +48,7 @@ public async Task CheckNonOptionalSettingsThrowsOnMissingAzureWebJobsStorageAndM
exception.Should().NotBeNull();
exception.Should().BeOfType<CliException>();
exception.Message.Should().Contain($"Missing value for AzureWebJobsStorage in local.settings.json. " +
$"This is required for all triggers other than {string.Join(", ", Constants.TriggersWithoutStorage)}.");
$"This is required for all triggers other than {string.Join(", ", Common.Constants.TriggersWithoutStorage)}.");
}

[Fact]
Expand Down Expand Up @@ -186,6 +190,63 @@ private IFileSystem GetFakeFileSystem(IEnumerable<(string folder, string functio
return fileSystem;
}

[Theory]
// In-proc target, in-proc 8 argument, project configured for .NET 8. Succeeds.
[InlineData(WorkerRuntime.dotnet, DotnetConstants.InProc8HostRuntime, true, false)]
// In-proc target, in-proc 8 argument, project NOT configured for .NET 8. Fails.
[InlineData(WorkerRuntime.dotnet, DotnetConstants.InProc8HostRuntime, false, true)]
// In-proc target, in-proc 6 argument, project NOT configured for .NET 8. Succeeds.
[InlineData(WorkerRuntime.dotnet, DotnetConstants.InProc6HostRuntime, false, false)]
// In-proc target,'default' argument, project configured for .NET 8. Fails.
[InlineData(WorkerRuntime.dotnet, "default", true, true)]
// Isolated target,'default' argument, project NOT configured for .NET 8. Succeeds.
[InlineData(WorkerRuntime.dotnetIsolated, "default", true, false)]
// Isolated target,'default' argument, project configured for .NET 8. Succeeds.
[InlineData(WorkerRuntime.dotnetIsolated, "default", false, false)]
// Isolated target,in-proc 8 argument, project configured for .NET 8. Fails.
[InlineData(WorkerRuntime.dotnetIsolated, DotnetConstants.InProc8HostRuntime, true, true)]
// Isolated target,in-proc 6 argument, project not configured for .NET 8. Fails.
[InlineData(WorkerRuntime.dotnetIsolated, DotnetConstants.InProc6HostRuntime, false, true)]
// Unsupported runtime targets.
[InlineData(WorkerRuntime.dotnetIsolated, "somevalue", false, true)]
[InlineData(WorkerRuntime.dotnet, "somevalue", false, true)]
// Non .NET worker runtimes.
[InlineData(WorkerRuntime.python, "default", false, false)]
[InlineData(WorkerRuntime.java, "default", false, false)]
[InlineData(WorkerRuntime.node, "default", false, false)]
[InlineData(WorkerRuntime.python, DotnetConstants.InProc6HostRuntime, false, true)]
[InlineData(WorkerRuntime.java, DotnetConstants.InProc6HostRuntime, false, true)]
[InlineData(WorkerRuntime.node, DotnetConstants.InProc6HostRuntime, false, true)]
[InlineData(WorkerRuntime.python, DotnetConstants.InProc8HostRuntime, false, true)]
[InlineData(WorkerRuntime.java, DotnetConstants.InProc8HostRuntime, false, true)]
[InlineData(WorkerRuntime.node, DotnetConstants.InProc8HostRuntime, false, true)]
public async Task ValidateHostRuntimeAsync_MatchesExpectedResults(WorkerRuntime currentRuntime, string hostRuntimeArgument, bool validNet8Configuration, bool expectException)
{
try
{
Mock<IProcessManager> processManager = new();
Mock<ISecretsManager> secretsManager = new();

var startHostAction = new StartHostAction(secretsManager.Object, processManager.Object)
{
HostRuntime = hostRuntimeArgument
};

await startHostAction.ValidateHostRuntimeAsync(currentRuntime, () => Task.FromResult(validNet8Configuration));
}
catch (CliException)
{
if (!expectException)
{
throw;
}

return;
}

Assert.False(expectException, "Expected validation failure.");
}

public void Dispose()
{
FileSystemHelpers.Instance = null;
Expand Down
Loading

0 comments on commit 5b7f4c7

Please sign in to comment.