Skip to content

Commit

Permalink
Fallback behavior to ensure in-proc payload compatibility with `dotne…
Browse files Browse the repository at this point in the history
…t-isolated` as the `FUNCTIONS_WORKER_RUNTIME` value (#10439) (#10449)

* Adding diagnostic warning/error when function worker runtime does not match with deployed payload

* Making the new condition specific to "dotnet-isolated" worker to reduce impact.

* Adding a test

* PR feedback fixes

* Missed release notes in last push

* Removed an unnecessary code change.

* Minor cleanup

* Minor cleanup- made a method private and vairable renaming

* Updating HostingConfig test to reflect the new hosting config property addition.

* PR Feedback updates.

* Fix test to reflect renaming changes from PR feedback.
  • Loading branch information
kshyju authored Sep 4, 2024
1 parent f5b03de commit bb017c5
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 7 deletions.
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@
- Resolved thread safety issue in the `GrpcWorkerChannel.LoadResponse` method. (#10352)
- Worker termination path updated with sanitized logging (#10397)
- Avoid redundant DiagnosticEvents error message (#10395)
- Added fallback behavior to ensure in-proc payload compatibility with "dotnet-isolated" as the `FUNCTIONS_WORKER_RUNTIME` value (#10439)
- Migrated Scale Metrics to use `Azure.Data.Tables` SDK (#10276)
- Added support for Identity-based connections
8 changes: 8 additions & 0 deletions src/WebJobs.Script/Config/FunctionsHostingConfigOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,14 @@ internal bool ThrowOnMissingFunctionsWorkerRuntime
}
}

internal bool WorkerRuntimeStrictValidationEnabled
{
get
{
return GetFeatureAsBooleanOrDefault(RpcWorkerConstants.WorkerRuntimeStrictValidationEnabled, false);
}
}

/// <summary>
/// Gets feature by name.
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions src/WebJobs.Script/Diagnostics/DiagnosticEventConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,8 @@ internal static class DiagnosticEventConstants

public const string NonHISSecretLoaded = "AZFD0012";
public const string NonHISSecretLoadedHelpLink = "https://aka.ms/functions-non-his-secrets";

public const string WorkerRuntimeDoesNotMatchWithFunctionMetadataErrorCode = "AZFD0013";
public const string WorkerRuntimeDoesNotMatchWithFunctionMetadataHelpLink = "https://aka.ms/functions-invalid-worker-runtime";
}
}
49 changes: 48 additions & 1 deletion src/WebJobs.Script/Host/ScriptHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -758,14 +758,50 @@ private void TrySetDirectType(FunctionMetadata metadata)
}
}

// Ensure customer deployed application payload matches with the worker runtime configured for the function app and log a warning if not.
// If a customer has "dotnet-isolated" worker runtime configured for the function app, and then they deploy an in-proc app payload, this will warn/error
// If there is a mismatch, the method will return false, else true.
private static bool ValidateAndLogRuntimeMismatch(IEnumerable<FunctionMetadata> functionMetadata, string workerRuntime, IOptions<FunctionsHostingConfigOptions> hostingConfigOptions, ILogger logger)
{
if (functionMetadata != null && functionMetadata.Any() && !Utility.ContainsAnyFunctionMatchingWorkerRuntime(functionMetadata, workerRuntime))
{
var languages = string.Join(", ", functionMetadata.Select(f => f.Language).Distinct()).Replace(DotNetScriptTypes.DotNetAssembly, RpcWorkerConstants.DotNetLanguageWorkerName);
var baseMessage = $"The '{EnvironmentSettingNames.FunctionWorkerRuntime}' is set to '{workerRuntime}', which does not match the worker runtime metadata found in the deployed function app artifacts. The deployed artifacts are for '{languages}'. See {DiagnosticEventConstants.WorkerRuntimeDoesNotMatchWithFunctionMetadataHelpLink} for more information.";

if (hostingConfigOptions.Value.WorkerRuntimeStrictValidationEnabled)
{
logger.LogDiagnosticEventError(DiagnosticEventConstants.WorkerRuntimeDoesNotMatchWithFunctionMetadataErrorCode, baseMessage, DiagnosticEventConstants.WorkerRuntimeDoesNotMatchWithFunctionMetadataHelpLink, null);
throw new HostInitializationException(baseMessage);
}

var warningMessage = baseMessage + " The application will continue to run, but may throw an exception in the future.";
logger.LogDiagnosticEventWarning(DiagnosticEventConstants.WorkerRuntimeDoesNotMatchWithFunctionMetadataErrorCode, warningMessage, DiagnosticEventConstants.WorkerRuntimeDoesNotMatchWithFunctionMetadataHelpLink, null);
return false;
}

return true;
}

internal async Task<Collection<FunctionDescriptor>> GetFunctionDescriptorsAsync(IEnumerable<FunctionMetadata> functions, IEnumerable<FunctionDescriptorProvider> descriptorProviders, string workerRuntime, CancellationToken cancellationToken)
{
Collection<FunctionDescriptor> functionDescriptors = new Collection<FunctionDescriptor>();
if (!cancellationToken.IsCancellationRequested)
{
bool throwOnWorkerRuntimeAndPayloadMetadataMismatch = true;
// this dotnet isolated specific logic is temporary to ensure in-proc payload compatibility with "dotnet-isolated" as the FUNCTIONS_WORKER_RUNTIME value.
if (string.Equals(workerRuntime, RpcWorkerConstants.DotNetIsolatedLanguageWorkerName, StringComparison.OrdinalIgnoreCase))
{
bool payloadMatchesWorkerRuntime = ValidateAndLogRuntimeMismatch(functions, workerRuntime, _hostingConfigOptions, _logger);
if (!payloadMatchesWorkerRuntime)
{
UpdateFunctionMetadataLanguageForDotnetAssembly(functions, workerRuntime);
throwOnWorkerRuntimeAndPayloadMetadataMismatch = false; // we do not want to throw an exception in this case
}
}

var httpFunctions = new Dictionary<string, HttpTriggerAttribute>();

Utility.VerifyFunctionsMatchSpecifiedLanguage(functions, workerRuntime, _environment.IsPlaceholderModeEnabled(), _isHttpWorker, cancellationToken);
Utility.VerifyFunctionsMatchSpecifiedLanguage(functions, workerRuntime, _environment.IsPlaceholderModeEnabled(), _isHttpWorker, cancellationToken, throwOnMismatch: throwOnWorkerRuntimeAndPayloadMetadataMismatch);

foreach (FunctionMetadata metadata in functions)
{
Expand Down Expand Up @@ -804,6 +840,17 @@ internal async Task<Collection<FunctionDescriptor>> GetFunctionDescriptorsAsync(
return functionDescriptors;
}

private static void UpdateFunctionMetadataLanguageForDotnetAssembly(IEnumerable<FunctionMetadata> functions, string workerRuntime)
{
foreach (var function in functions)
{
if (function.Language == DotNetScriptTypes.DotNetAssembly)
{
function.Language = workerRuntime;
}
}
}

internal static void ValidateFunction(FunctionDescriptor function, Dictionary<string, HttpTriggerAttribute> httpFunctions, IEnvironment environment)
{
var httpTrigger = function.HttpTriggerAttribute;
Expand Down
17 changes: 15 additions & 2 deletions src/WebJobs.Script/Utility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ internal static bool TryReadFunctionConfig(string scriptDir, out string json, IF
return true;
}

internal static void VerifyFunctionsMatchSpecifiedLanguage(IEnumerable<FunctionMetadata> functions, string workerRuntime, bool isPlaceholderMode, bool isHttpWorker, CancellationToken cancellationToken)
internal static void VerifyFunctionsMatchSpecifiedLanguage(IEnumerable<FunctionMetadata> functions, string workerRuntime, bool isPlaceholderMode, bool isHttpWorker, CancellationToken cancellationToken, bool throwOnMismatch = true)
{
cancellationToken.ThrowIfCancellationRequested();

Expand All @@ -646,7 +646,10 @@ internal static void VerifyFunctionsMatchSpecifiedLanguage(IEnumerable<FunctionM
}
else
{
throw new HostInitializationException($"Did not find functions with language [{workerRuntime}].");
if (throwOnMismatch)
{
throw new HostInitializationException($"Did not find functions with language [{workerRuntime}].");
}
}
}
}
Expand Down Expand Up @@ -748,10 +751,20 @@ private static bool ContainsFunctionWithWorkerRuntime(IEnumerable<FunctionMetada
{
return functions.Any(f => dotNetLanguages.Any(l => l.Equals(f.Language, StringComparison.OrdinalIgnoreCase)));
}

return ContainsAnyFunctionMatchingWorkerRuntime(functions, workerRuntime);
}

/// <summary>
/// Inspect the functions metadata to determine if at least one function is of the specified worker runtime.
/// </summary>
internal static bool ContainsAnyFunctionMatchingWorkerRuntime(IEnumerable<FunctionMetadata> functions, string workerRuntime)
{
if (functions != null && functions.Any())
{
return functions.Any(f => !string.IsNullOrEmpty(f.Language) && f.Language.Equals(workerRuntime, StringComparison.OrdinalIgnoreCase));
}

return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ internal class RpcFunctionInvocationDispatcher : IFunctionInvocationDispatcher
private readonly IEnumerable<RpcWorkerConfig> _workerConfigs;
private readonly Lazy<Task<int>> _maxProcessCount;
private readonly IOptions<FunctionsHostingConfigOptions> _hostingConfigOptions;
private readonly TimeSpan _defaultProcessStartupInterval = TimeSpan.FromSeconds(5);
private readonly TimeSpan _defaultProcessRestartInterval = TimeSpan.FromSeconds(5);
private readonly TimeSpan _defaultProcessShutdownInterval = TimeSpan.FromSeconds(5);

private IScriptEventManager _eventManager;
private IWebHostRpcWorkerChannelManager _webHostLanguageWorkerChannelManager;
Expand Down Expand Up @@ -304,9 +307,9 @@ public async Task InitializeAsync(IEnumerable<FunctionMetadata> functions, Cance
}
else
{
_processStartupInterval = workerConfig.CountOptions.ProcessStartupInterval;
_restartWait = workerConfig.CountOptions.ProcessRestartInterval;
_shutdownTimeout = workerConfig.CountOptions.ProcessShutdownTimeout;
_processStartupInterval = workerConfig?.CountOptions?.ProcessStartupInterval ?? _defaultProcessStartupInterval;
_restartWait = workerConfig?.CountOptions.ProcessRestartInterval ?? _defaultProcessRestartInterval;
_shutdownTimeout = workerConfig?.CountOptions.ProcessShutdownTimeout ?? _defaultProcessShutdownInterval;
}
ErrorEventsThreshold = 3 * await _maxProcessCount.Value;

Expand Down
1 change: 1 addition & 0 deletions src/WebJobs.Script/Workers/Rpc/RpcWorkerConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,6 @@ public static class RpcWorkerConstants
public const string RevertWorkerShutdownBehavior = "REVERT_WORKER_SHUTDOWN_BEHAVIOR";
public const string ShutdownWebhostWorkerChannelsOnHostShutdown = "ShutdownWebhostWorkerChannelsOnHostShutdown";
public const string ThrowOnMissingFunctionsWorkerRuntime = "THROW_ON_MISSING_FUNCTIONS_WORKER_RUNTIME";
public const string WorkerRuntimeStrictValidationEnabled = "WORKER_RUNTIME_STRICT_VALIDATION_ENABLED";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Net;
using System.Threading.Tasks;
using Microsoft.Azure.WebJobs.Logging;
using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

Expand Down Expand Up @@ -45,6 +46,34 @@ public async Task ExternalStartup_Succeeds()
}
}

[Fact]
public async Task InProcAppsWorkWithDotnetIsolatedAsFunctionWorkerRuntimeValue()
{
// test uses an in-proc app, but we are setting "dotnet-isolated" as functions worker runtime value.
var fixture = new CSharpPrecompiledEndToEndTestFixture(_projectName, _envVars, functionWorkerRuntime: RpcWorkerConstants.DotNetIsolatedLanguageWorkerName);
try
{
await fixture.InitializeAsync();
var client = fixture.Host.HttpClient;

var response = await client.GetAsync($"api/Function1");

// The function does all the validation internally.
Assert.Equal(HttpStatusCode.OK, response.StatusCode);

const string expectedLogEntry =
"The 'FUNCTIONS_WORKER_RUNTIME' is set to 'dotnet-isolated', " +
"which does not match the worker runtime metadata found in the deployed function app artifacts. " +
"The deployed artifacts are for 'dotnet'. See https://aka.ms/functions-invalid-worker-runtime " +
"for more information. The application will continue to run, but may throw an exception in the future.";
Assert.Single(fixture.Host.GetScriptHostLogMessages(), p => p.FormattedMessage != null && p.FormattedMessage.EndsWith(expectedLogEntry));
}
finally
{
await fixture.DisposeAsync();
}
}

[Fact]
public async Task ExternalStartup_InvalidOverwrite_StopsHost()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ public void Property_Validation()

(nameof(FunctionsHostingConfigOptions.ThrowOnMissingFunctionsWorkerRuntime), "THROW_ON_MISSING_FUNCTIONS_WORKER_RUNTIME=1", true),
(nameof(FunctionsHostingConfigOptions.WorkerIndexingDisabledApps), "WORKER_INDEXING_DISABLED_APPS=teststring", "teststring"),
(nameof(FunctionsHostingConfigOptions.WorkerIndexingEnabled), "WORKER_INDEXING_ENABLED=1", true)
(nameof(FunctionsHostingConfigOptions.WorkerIndexingEnabled), "WORKER_INDEXING_ENABLED=1", true),
(nameof(FunctionsHostingConfigOptions.WorkerRuntimeStrictValidationEnabled), "WORKER_RUNTIME_STRICT_VALIDATION_ENABLED=1", true)
};

// use reflection to ensure that we have a test that uses every value exposed on FunctionsHostingConfigOptions
Expand Down

0 comments on commit bb017c5

Please sign in to comment.