From af614bb1c2c1881c3e72b983412232124d42fecd Mon Sep 17 00:00:00 2001 From: Fabio Cavalcante Date: Mon, 5 Aug 2024 17:27:18 -0700 Subject: [PATCH 1/7] Forwarding the LanguageWorkerOptions from the host to the job host scope and ensuring we use the provided instances. --- .../Description/FunctionDescriptorProvider.cs | 10 ++++----- .../Host/WorkerFunctionMetadataProvider.cs | 8 +++---- .../ScriptHostBuilderExtensions.cs | 14 ++++++++++--- .../FunctionInvocationDispatcherFactory.cs | 4 ++-- .../LanguageWorkerOptionsSetup.cs | 21 +++++++++++++++++++ .../RpcFunctionInvocationDispatcher.cs | 8 +++++-- 6 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/WebJobs.Script/Description/FunctionDescriptorProvider.cs b/src/WebJobs.Script/Description/FunctionDescriptorProvider.cs index a3905803ce..7688c3143d 100644 --- a/src/WebJobs.Script/Description/FunctionDescriptorProvider.cs +++ b/src/WebJobs.Script/Description/FunctionDescriptorProvider.cs @@ -181,7 +181,7 @@ private bool TryParseTriggerParameter(BindingMetadata metadata, out ParameterDes protected internal virtual void ValidateFunction(FunctionMetadata functionMetadata) { - HashSet names = new HashSet(StringComparer.OrdinalIgnoreCase); + HashSet names = new(StringComparer.OrdinalIgnoreCase); foreach (var binding in functionMetadata.Bindings) { ValidateBinding(binding); @@ -189,12 +189,10 @@ protected internal virtual void ValidateFunction(FunctionMetadata functionMetada // Ensure no duplicate binding names if (names.Contains(binding.Name)) { - throw new InvalidOperationException(string.Format("Multiple bindings with name '{0}' discovered. Binding names must be unique.", binding.Name)); - } - else - { - names.Add(binding.Name); + throw new InvalidOperationException($"{nameof(FunctionDescriptorProvider)}: Multiple bindings with name '{binding.Name}' discovered. Binding names must be unique."); } + + names.Add(binding.Name); } // Verify there aren't multiple triggers defined diff --git a/src/WebJobs.Script/Host/WorkerFunctionMetadataProvider.cs b/src/WebJobs.Script/Host/WorkerFunctionMetadataProvider.cs index f57377bdcf..6841f2b092 100644 --- a/src/WebJobs.Script/Host/WorkerFunctionMetadataProvider.cs +++ b/src/WebJobs.Script/Host/WorkerFunctionMetadataProvider.cs @@ -257,13 +257,11 @@ internal static FunctionMetadata ValidateBindings(IEnumerable rawBinding // Ensure no duplicate binding names exist if (bindingNames.Contains(functionBinding.Name)) { - throw new InvalidOperationException(string.Format("Multiple bindings with name '{0}' discovered. Binding names must be unique.", functionBinding.Name)); - } - else - { - bindingNames.Add(functionBinding.Name); + throw new InvalidOperationException($"{nameof(WorkerFunctionDescriptorProvider)}: Multiple bindings with name '{functionBinding.Name}' discovered. Binding names must be unique."); } + bindingNames.Add(functionBinding.Name); + // add binding to function.Bindings once validation is complete function.Bindings.Add(functionBinding); } diff --git a/src/WebJobs.Script/ScriptHostBuilderExtensions.cs b/src/WebJobs.Script/ScriptHostBuilderExtensions.cs index 814811a8d6..dc514a451d 100644 --- a/src/WebJobs.Script/ScriptHostBuilderExtensions.cs +++ b/src/WebJobs.Script/ScriptHostBuilderExtensions.cs @@ -311,10 +311,9 @@ public static IHostBuilder AddScriptHostCore(this IHostBuilder builder, ScriptAp // Configuration services.AddSingleton>(new OptionsWrapper(applicationHostOptions)); services.AddSingleton>(new ScriptApplicationHostOptionsMonitor(applicationHostOptions)); + services.ConfigureOptions(); services.ConfigureOptions(); - // LanguageWorkerOptionsSetup should be registered in WebHostServiceCollection as well to enable starting worker processing in placeholder mode. - services.ConfigureOptions(); services.AddOptions(); services.ConfigureOptions(); services.ConfigureOptions(); @@ -329,8 +328,17 @@ public static IHostBuilder AddScriptHostCore(this IHostBuilder builder, ScriptAp services.AddSingleton(); - if (!applicationHostOptions.HasParentScope) + if (applicationHostOptions.HasParentScope) + { + // Forward th host LanguageWorkerOptions to the Job Host. + var languageWorkerOptions = applicationHostOptions.RootServiceProvider.GetService>(); + services.AddSingleton(languageWorkerOptions); + services.AddSingleton>((s) => new OptionsWrapper(languageWorkerOptions.CurrentValue)); + services.ConfigureOptions(); + } + else { + services.ConfigureOptions(); AddCommonServices(services); } diff --git a/src/WebJobs.Script/Workers/FunctionInvocationDispatcherFactory.cs b/src/WebJobs.Script/Workers/FunctionInvocationDispatcherFactory.cs index c228915e97..0b5dc643a4 100644 --- a/src/WebJobs.Script/Workers/FunctionInvocationDispatcherFactory.cs +++ b/src/WebJobs.Script/Workers/FunctionInvocationDispatcherFactory.cs @@ -27,7 +27,7 @@ public FunctionInvocationDispatcherFactory(IOptions script IHttpWorkerChannelFactory httpWorkerChannelFactory, IRpcWorkerChannelFactory rpcWorkerChannelFactory, IOptions httpWorkerOptions, - IOptionsMonitor rpcWorkerOptions, + IOptionsMonitor workerOptions, IEnvironment environment, IWebHostRpcWorkerChannelManager webHostLanguageWorkerChannelManager, IJobHostRpcWorkerChannelManager jobHostLanguageWorkerChannelManager, @@ -54,7 +54,7 @@ public FunctionInvocationDispatcherFactory(IOptions script eventManager, loggerFactory, rpcWorkerChannelFactory, - rpcWorkerOptions, + workerOptions, webHostLanguageWorkerChannelManager, jobHostLanguageWorkerChannelManager, managedDependencyOptions, diff --git a/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs b/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs index 4cfa5a14e9..11a8970409 100644 --- a/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs +++ b/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using Microsoft.Azure.WebJobs.Script.Diagnostics; using Microsoft.Azure.WebJobs.Script.Workers.Profiles; using Microsoft.Extensions.Configuration; @@ -56,4 +57,24 @@ public void Configure(LanguageWorkerOptions options) options.WorkerConfigs = configFactory.GetConfigs(); } } + + internal class JobHostLanguageWorkerOptionsSetup : IPostConfigureOptions + { + private readonly ILoggerFactory _loggerFactory; + + public JobHostLanguageWorkerOptionsSetup(ILoggerFactory loggerFactory) + { + _loggerFactory = loggerFactory; + } + + public void PostConfigure(string name, LanguageWorkerOptions options) + { + var message = $"Call to configure {nameof(LanguageWorkerOptions)} from the JobHost scope. " + + $"If using {nameof(IOptions)}, please use {nameof(IOptionsMonitor)} instead."; + Debug.Fail(message); + + var logger = _loggerFactory.CreateLogger("Host.LanguageWorkerConfig"); + logger.LogInformation(message); + } + } } diff --git a/src/WebJobs.Script/Workers/Rpc/FunctionRegistration/RpcFunctionInvocationDispatcher.cs b/src/WebJobs.Script/Workers/Rpc/FunctionRegistration/RpcFunctionInvocationDispatcher.cs index ab48b8f8cc..8b7393bc88 100644 --- a/src/WebJobs.Script/Workers/Rpc/FunctionRegistration/RpcFunctionInvocationDispatcher.cs +++ b/src/WebJobs.Script/Workers/Rpc/FunctionRegistration/RpcFunctionInvocationDispatcher.cs @@ -67,7 +67,7 @@ public RpcFunctionInvocationDispatcher(IOptions scriptHost IScriptEventManager eventManager, ILoggerFactory loggerFactory, IRpcWorkerChannelFactory rpcWorkerChannelFactory, - IOptionsMonitor languageWorkerOptions, + IOptionsMonitor workerOptions, IWebHostRpcWorkerChannelManager webHostLanguageWorkerChannelManager, IJobHostRpcWorkerChannelManager jobHostLanguageWorkerChannelManager, IOptions managedDependencyOptions, @@ -83,7 +83,11 @@ public RpcFunctionInvocationDispatcher(IOptions scriptHost _webHostLanguageWorkerChannelManager = webHostLanguageWorkerChannelManager; _jobHostLanguageWorkerChannelManager = jobHostLanguageWorkerChannelManager; _eventManager = eventManager; - _workerConfigs = languageWorkerOptions?.CurrentValue?.WorkerConfigs ?? throw new ArgumentNullException(nameof(languageWorkerOptions)); + + // The state of worker configuration and the LanguageWorkerOptions will match the lifetime of the JobHost and the + // RpcFunctionInvocationDispatcher. So, we can safely cache the workerConfigs and workerRuntime here. + // Using IOptionsMonitor here to get the same cached version used by other copmponents and avoid a new instance initialization. + _workerConfigs = workerOptions?.CurrentValue?.WorkerConfigs ?? throw new ArgumentNullException(nameof(workerOptions)); _managedDependencyOptions = managedDependencyOptions ?? throw new ArgumentNullException(nameof(managedDependencyOptions)); _logger = loggerFactory.CreateLogger(); _rpcWorkerChannelFactory = rpcWorkerChannelFactory; From bfbfe568e4410aeb39245414853ce352929e43ea Mon Sep 17 00:00:00 2001 From: Shyju Krishnankutty Date: Tue, 10 Sep 2024 07:32:17 -0700 Subject: [PATCH 2/7] Adding a change notification system to refresh Options. Using this to refresh LanguageWorkerOption after script host is built. This will cause the LanguageWorkerOption to be refreshed after specialization. --- .../HostBuiltChangeTokenSource.cs | 28 +++++++++ .../WebHostServiceCollectionExtensions.cs | 7 ++- .../WebJobsScriptHostService.cs | 8 ++- .../LanguageWorkerOptionsSetup.cs | 27 +++++++-- .../HostBuiltChangeTokenSourceTests.cs | 59 +++++++++++++++++++ .../LanguageWorkerOptionsSetupTests.cs | 3 +- .../WebJobsScriptHostServiceTests.cs | 24 ++++---- 7 files changed, 138 insertions(+), 18 deletions(-) create mode 100644 src/WebJobs.Script.WebHost/Configuration/HostBuiltChangeTokenSource.cs create mode 100644 test/WebJobs.Script.Tests/Configuration/HostBuiltChangeTokenSourceTests.cs diff --git a/src/WebJobs.Script.WebHost/Configuration/HostBuiltChangeTokenSource.cs b/src/WebJobs.Script.WebHost/Configuration/HostBuiltChangeTokenSource.cs new file mode 100644 index 0000000000..cb800a8ae8 --- /dev/null +++ b/src/WebJobs.Script.WebHost/Configuration/HostBuiltChangeTokenSource.cs @@ -0,0 +1,28 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System; +using System.Threading; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.Azure.WebJobs.Script.WebHost.Configuration +{ + public sealed class HostBuiltChangeTokenSource : IOptionsChangeTokenSource, IDisposable + { + private CancellationTokenSource _cts = new(); + + public string Name => Options.DefaultName; + + public IChangeToken GetChangeToken() => new CancellationChangeToken(_cts.Token); + + public void TriggerChange() + { + var previousCts = Interlocked.Exchange(ref _cts, new CancellationTokenSource()); + previousCts.Cancel(); + previousCts.Dispose(); + } + + public void Dispose() => _cts.Dispose(); + } +} \ No newline at end of file diff --git a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs index fcac478611..642f3ea1a8 100644 --- a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs +++ b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs @@ -207,15 +207,18 @@ public static void AddWebJobsScriptHost(this IServiceCollection services, IConfi // will reset the ScriptApplicationHostOptions only after StandbyOptions have been reset. services.ConfigureOptions(); services.AddSingleton, ScriptApplicationHostOptionsChangeTokenSource>(); - services.ConfigureOptions(); - services.ConfigureOptionsWithChangeTokenSource>(); + services.ConfigureOptions(); services.ConfigureOptionsWithChangeTokenSource>(); services.ConfigureOptionsWithChangeTokenSource>(); services.ConfigureOptions(); services.ConfigureOptions(); services.AddHostingConfigOptions(configuration); + // Refresh LanguageWorkerOptions when HostBuiltChangeTokenSource is triggered. + services.AddSingleton>(); + services.AddSingleton>(s => s.GetRequiredService>()); + services.TryAddSingleton(); services.TryAddSingleton(s => DefaultMiddlewarePipeline.Empty); diff --git a/src/WebJobs.Script.WebHost/WebJobsScriptHostService.cs b/src/WebJobs.Script.WebHost/WebJobsScriptHostService.cs index 6c0f21484b..3bff5b15f2 100644 --- a/src/WebJobs.Script.WebHost/WebJobsScriptHostService.cs +++ b/src/WebJobs.Script.WebHost/WebJobsScriptHostService.cs @@ -20,6 +20,7 @@ using Microsoft.Azure.WebJobs.Script.Eventing; using Microsoft.Azure.WebJobs.Script.Metrics; using Microsoft.Azure.WebJobs.Script.Scale; +using Microsoft.Azure.WebJobs.Script.WebHost.Configuration; using Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics.Extensions; using Microsoft.Azure.WebJobs.Script.Workers; using Microsoft.Azure.WebJobs.Script.Workers.Rpc; @@ -58,6 +59,7 @@ public class WebJobsScriptHostService : IHostedService, IScriptHostManager, ISer private readonly bool _originalStandbyModeValue; private readonly string _originalFunctionsWorkerRuntime; private readonly string _originalFunctionsWorkerRuntimeVersion; + private readonly HostBuiltChangeTokenSource _hostBuiltChangeTokenSourceForLanguageWorkerOptions; private IScriptEventManager _eventManager; private IHost _host; @@ -76,7 +78,8 @@ public WebJobsScriptHostService(IOptionsMonitor ap IScriptWebHostEnvironment scriptWebHostEnvironment, IEnvironment environment, HostPerformanceManager hostPerformanceManager, IOptions healthMonitorOptions, IMetricsLogger metricsLogger, IApplicationLifetime applicationLifetime, IConfiguration config, IScriptEventManager eventManager, IHostMetrics hostMetrics, - IOptions hostingConfigOptions) + IOptions hostingConfigOptions, + HostBuiltChangeTokenSource hostBuiltChangeTokenSource) { ArgumentNullException.ThrowIfNull(loggerFactory); @@ -87,6 +90,7 @@ public WebJobsScriptHostService(IOptionsMonitor ap RegisterApplicationLifetimeEvents(); _metricsLogger = metricsLogger; + _hostBuiltChangeTokenSourceForLanguageWorkerOptions = hostBuiltChangeTokenSource ?? throw new ArgumentNullException(nameof(hostBuiltChangeTokenSource)); _applicationHostOptions = applicationHostOptions ?? throw new ArgumentNullException(nameof(applicationHostOptions)); _scriptWebHostEnvironment = scriptWebHostEnvironment ?? throw new ArgumentNullException(nameof(scriptWebHostEnvironment)); _scriptHostBuilder = scriptHostBuilder ?? throw new ArgumentNullException(nameof(scriptHostBuilder)); @@ -348,6 +352,8 @@ private async Task UnsynchronizedStartHostAsync(ScriptHostStartupOperation activ ActiveHost = localHost; + _hostBuiltChangeTokenSourceForLanguageWorkerOptions.TriggerChange(); + var scriptHost = (ScriptHost)ActiveHost.Services.GetService(); if (scriptHost != null) { diff --git a/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs b/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs index 11a8970409..c43ba52831 100644 --- a/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs +++ b/src/WebJobs.Script/Workers/Rpc/Configuration/LanguageWorkerOptionsSetup.cs @@ -7,6 +7,7 @@ using Microsoft.Azure.WebJobs.Script.Diagnostics; using Microsoft.Azure.WebJobs.Script.Workers.Profiles; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -19,18 +20,21 @@ internal class LanguageWorkerOptionsSetup : IConfigureOptions(); return; } - var configFactory = new RpcWorkerConfigFactory(_configuration, _logger, SystemRuntimeInformation.Instance, _environment, _metricsLogger, _workerProfileManager); + // Use the latest configuration from the ScriptHostManager if available. + // After specialization, the ScriptHostManager will have the latest IConfiguration reflecting additional configuration entries added during specialization. + var configuration = _configuration; + if (_scriptHostManager is IServiceProvider scriptHostManagerServiceProvider) + { + var latestConfiguration = scriptHostManagerServiceProvider.GetService(); + if (latestConfiguration is not null) + { + configuration = new ConfigurationBuilder() + .AddConfiguration(_configuration) + .AddConfiguration(latestConfiguration) + .Build(); + } + } + + var configFactory = new RpcWorkerConfigFactory(configuration, _logger, SystemRuntimeInformation.Instance, _environment, _metricsLogger, _workerProfileManager); options.WorkerConfigs = configFactory.GetConfigs(); } } diff --git a/test/WebJobs.Script.Tests/Configuration/HostBuiltChangeTokenSourceTests.cs b/test/WebJobs.Script.Tests/Configuration/HostBuiltChangeTokenSourceTests.cs new file mode 100644 index 0000000000..6cb13ea6e2 --- /dev/null +++ b/test/WebJobs.Script.Tests/Configuration/HostBuiltChangeTokenSourceTests.cs @@ -0,0 +1,59 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System; +using Microsoft.Azure.WebJobs.Script.WebHost.Configuration; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests.Configuration +{ + public sealed class HostBuiltChangeTokenSourceTests + { + [Fact] + public void GetChangeToken_ReturnsValidToken() + { + var changeTokenSource = new HostBuiltChangeTokenSource(); + + IChangeToken changeToken = changeTokenSource.GetChangeToken(); + + Assert.NotNull(changeToken); + Assert.False(changeToken.HasChanged); + } + + [Fact] + public void TriggerChange_SignalsChange() + { + var changeTokenSource = new HostBuiltChangeTokenSource(); + IChangeToken changeToken = changeTokenSource.GetChangeToken(); + + changeTokenSource.TriggerChange(); + + Assert.True(changeToken.HasChanged); + } + + [Fact] + public void TriggerChange_CreatesNewToken() + { + var changeTokenSource = new HostBuiltChangeTokenSource(); + IChangeToken initialToken = changeTokenSource.GetChangeToken(); + + changeTokenSource.TriggerChange(); + IChangeToken newToken = changeTokenSource.GetChangeToken(); + + Assert.NotSame(initialToken, newToken); + Assert.True(initialToken.HasChanged); + Assert.False(newToken.HasChanged); + } + + [Fact] + public void Dispose_DisposesTokenSource() + { + var changeTokenSource = new HostBuiltChangeTokenSource(); + + changeTokenSource.Dispose(); + + Assert.Throws(() => changeTokenSource.GetChangeToken()); + } + } +} diff --git a/test/WebJobs.Script.Tests/Configuration/LanguageWorkerOptionsSetupTests.cs b/test/WebJobs.Script.Tests/Configuration/LanguageWorkerOptionsSetupTests.cs index 2e55713b9b..d3f2dea6c8 100644 --- a/test/WebJobs.Script.Tests/Configuration/LanguageWorkerOptionsSetupTests.cs +++ b/test/WebJobs.Script.Tests/Configuration/LanguageWorkerOptionsSetupTests.cs @@ -26,6 +26,7 @@ public void LanguageWorkerOptions_Expected_ListOfConfigs(string workerRuntime) .Add(new ScriptEnvironmentVariablesConfigurationSource()); var configuration = configurationBuilder.Build(); var testProfileManager = new Mock(); + var testScriptHostManager = new Mock(); if (!string.IsNullOrEmpty(workerRuntime)) { @@ -52,7 +53,7 @@ public void LanguageWorkerOptions_Expected_ListOfConfigs(string workerRuntime) } }); - LanguageWorkerOptionsSetup setup = new LanguageWorkerOptionsSetup(configuration, NullLoggerFactory.Instance, testEnvironment, testMetricLogger, testProfileManager.Object); + LanguageWorkerOptionsSetup setup = new LanguageWorkerOptionsSetup(configuration, NullLoggerFactory.Instance, testEnvironment, testMetricLogger, testProfileManager.Object, testScriptHostManager.Object); LanguageWorkerOptions options = new LanguageWorkerOptions(); setup.Configure(options); diff --git a/test/WebJobs.Script.Tests/WebJobsScriptHostServiceTests.cs b/test/WebJobs.Script.Tests/WebJobsScriptHostServiceTests.cs index a2b8593ccb..092beeabf5 100644 --- a/test/WebJobs.Script.Tests/WebJobsScriptHostServiceTests.cs +++ b/test/WebJobs.Script.Tests/WebJobsScriptHostServiceTests.cs @@ -13,6 +13,7 @@ using Microsoft.Azure.WebJobs.Script.Metrics; using Microsoft.Azure.WebJobs.Script.Scale; using Microsoft.Azure.WebJobs.Script.WebHost; +using Microsoft.Azure.WebJobs.Script.WebHost.Configuration; using Microsoft.Azure.WebJobs.Script.WebHost.DependencyInjection; using Microsoft.Azure.WebJobs.Script.Workers.Rpc; using Microsoft.Extensions.Configuration; @@ -38,6 +39,7 @@ public class WebJobsScriptHostServiceTests private ILoggerFactory _loggerFactory; private Mock _mockScriptWebHostEnvironment; private Mock _mockEnvironment; + private HostBuiltChangeTokenSource _hostBuiltChangeTokenSource = new(); private IConfiguration _mockConfig; private OptionsWrapper _healthMonitorOptions; private HostPerformanceManager _hostPerformanceManager; @@ -116,7 +118,8 @@ public async Task StartAsync_Succeeds() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, - _mockConfig, mockEventManager.Object, _hostMetrics, _functionsHostingConfigOptions); + _mockConfig, mockEventManager.Object, _hostMetrics, _functionsHostingConfigOptions, + _hostBuiltChangeTokenSource); await _hostService.StartAsync(CancellationToken.None); @@ -147,7 +150,8 @@ public async Task HostInitialization_OnInitializationException_MaintainsErrorInf _monitor, hostBuilder.Object, NullLoggerFactory.Instance, _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, - new Mock().Object, _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions); + new Mock().Object, _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions, + _hostBuiltChangeTokenSource); await _hostService.StartAsync(CancellationToken.None); Assert.True(AreRequiredMetricsGenerated(metricsLogger)); @@ -177,7 +181,7 @@ public async Task HostRestart_Specialization_Succeeds() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, - _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions); + _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); await _hostService.StartAsync(CancellationToken.None); Assert.True(AreRequiredMetricsGenerated(metricsLogger)); @@ -232,7 +236,7 @@ public async Task HostRestart_DuringInitializationWithError_Recovers() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, - _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions); + _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); TestLoggerProvider hostALogger = hostA.Object.GetTestLoggerProvider(); TestLoggerProvider hostBLogger = hostB.Object.GetTestLoggerProvider(); @@ -308,7 +312,7 @@ public async Task HostRestart_DuringInitialization_Cancels() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, - _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions); + _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); TestLoggerProvider hostALogger = hostA.Object.GetTestLoggerProvider(); @@ -378,7 +382,7 @@ public async Task DisposedHost_ServicesNotExposed() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, _mockConfig, new TestScriptEventManager(), - _hostMetrics, _functionsHostingConfigOptions); + _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); Task startTask = _hostService.StartAsync(CancellationToken.None); @@ -429,7 +433,7 @@ public async Task DisposesScriptHost() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, - _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions); + _mockConfig, new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); var hostLogger = host.Object.GetTestLoggerProvider(); @@ -467,7 +471,7 @@ public async Task HostRestart_BeforeStart_WaitsForStartToContinue() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, _mockConfig, - new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions); + new TestScriptEventManager(), _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); // Simulate a call to specialize coming from the PlaceholderSpecializationMiddleware. This // can happen before we ever start the service, which could create invalid state. @@ -523,7 +527,7 @@ public void ShouldEnforceSequentialRestart_WithCorrectConfig(string value, bool _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, metricsLogger, new Mock().Object, config, new TestScriptEventManager(), - _hostMetrics, _functionsHostingConfigOptions); + _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource); Assert.Equal(expectedResult, _hostService.ShouldEnforceSequentialRestart()); } @@ -561,7 +565,7 @@ public async Task DependencyTrackingTelemetryModule_Race() _mockScriptWebHostEnvironment.Object, _mockEnvironment.Object, _hostPerformanceManager, _healthMonitorOptions, new TestMetricsLogger(), new Mock().Object, _mockConfig, new TestScriptEventManager(), - _hostMetrics, _functionsHostingConfigOptions)) + _hostMetrics, _functionsHostingConfigOptions, _hostBuiltChangeTokenSource)) { await _hostService.StartAsync(CancellationToken.None); From a1257e722d5d7dabd37f26cf6dc155e19ad57a82 Mon Sep 17 00:00:00 2001 From: Shyju Krishnankutty Date: Wed, 11 Sep 2024 08:27:26 -0700 Subject: [PATCH 3/7] Fixed a test to reflect the new code changes. --- .../WebJobs.Script.Tests/WorkerFunctionMetadataProviderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/WebJobs.Script.Tests/WorkerFunctionMetadataProviderTests.cs b/test/WebJobs.Script.Tests/WorkerFunctionMetadataProviderTests.cs index b370a24e77..5efdac22f0 100644 --- a/test/WebJobs.Script.Tests/WorkerFunctionMetadataProviderTests.cs +++ b/test/WebJobs.Script.Tests/WorkerFunctionMetadataProviderTests.cs @@ -48,7 +48,7 @@ public void ValidateBindings_DuplicateBindingNames_Throws() WorkerFunctionMetadataProvider.ValidateBindings(rawBindings, functionMetadata); }); - Assert.Equal("Multiple bindings with name 'dupe' discovered. Binding names must be unique.", ex.Message); + Assert.Equal($"{nameof(WorkerFunctionDescriptorProvider)}: Multiple bindings with name 'dupe' discovered. Binding names must be unique.", ex.Message); } [Fact] From eec58064707d571a7fabaf37472af8beee3b979e Mon Sep 17 00:00:00 2001 From: Shyju Krishnankutty Date: Wed, 18 Sep 2024 07:08:25 -0700 Subject: [PATCH 4/7] Shutdown channel when language worker options change --- .../Workers/Rpc/WebHostRpcWorkerChannelManager.cs | 11 +++++++++++ .../Middleware/HostWarmupMiddlewareTests.cs | 2 +- .../Rpc/WebHostRpcWorkerChannelManagerTests.cs | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs b/src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs index 47398efa48..5519e21b1d 100644 --- a/src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs +++ b/src/WebJobs.Script/Workers/Rpc/WebHostRpcWorkerChannelManager.cs @@ -44,6 +44,7 @@ public WebHostRpcWorkerChannelManager(IScriptEventManager eventManager, IMetricsLogger metricsLogger, IConfiguration config, IWorkerProfileManager workerProfileManager, + IOptionsMonitor languageWorkerOptions, IOptions hostingConfigOptions) { _environment = environment ?? throw new ArgumentNullException(nameof(environment)); @@ -57,6 +58,16 @@ public WebHostRpcWorkerChannelManager(IScriptEventManager eventManager, _applicationHostOptions = applicationHostOptions; _hostingConfigOptions = hostingConfigOptions; + languageWorkerOptions.OnChange(async languageWorkerOptions => + { + IRpcWorkerChannel rpcWorkerChannel = await GetChannelAsync(_workerRuntime); + if (rpcWorkerChannel != null && !UsePlaceholderChannel(rpcWorkerChannel)) + { + _logger.LogInformation("Language worker options changed, and the placeholder worker channel is invalid for other reasons. Shutting down the channel."); + await ShutdownChannelIfExistsAsync(_workerRuntime, rpcWorkerChannel.Id); + } + }); + _shutdownStandbyWorkerChannels = ScheduleShutdownStandbyChannels; _shutdownStandbyWorkerChannels = _shutdownStandbyWorkerChannels.Debounce(milliseconds: 5000); } diff --git a/test/WebJobs.Script.Tests/Middleware/HostWarmupMiddlewareTests.cs b/test/WebJobs.Script.Tests/Middleware/HostWarmupMiddlewareTests.cs index 85e0e62153..277e80b555 100644 --- a/test/WebJobs.Script.Tests/Middleware/HostWarmupMiddlewareTests.cs +++ b/test/WebJobs.Script.Tests/Middleware/HostWarmupMiddlewareTests.cs @@ -79,7 +79,7 @@ public HostWarmupMiddlewareTests() _testLogger = new TestLogger("WebHostLanguageWorkerChannelManagerTests"); _rpcWorkerChannelFactory = new TestRpcWorkerChannelFactory(_eventManager, _testLogger, _scriptRootPath); _emptyConfig = new ConfigurationBuilder().Build(); - _rpcWorkerChannelManager = new WebHostRpcWorkerChannelManager(_eventManager, _testEnvironment, _loggerFactory, _rpcWorkerChannelFactory, _optionsMonitor, new TestMetricsLogger(), _emptyConfig, _workerProfileManager, new OptionsWrapper(new FunctionsHostingConfigOptions())); + _rpcWorkerChannelManager = new WebHostRpcWorkerChannelManager(_eventManager, _testEnvironment, _loggerFactory, _rpcWorkerChannelFactory, _optionsMonitor, new TestMetricsLogger(), _emptyConfig, _workerProfileManager, new TestOptionsMonitor(TestHelpers.GetTestLanguageWorkerOptions()), new OptionsWrapper(new FunctionsHostingConfigOptions())); } [Fact] diff --git a/test/WebJobs.Script.Tests/Workers/Rpc/WebHostRpcWorkerChannelManagerTests.cs b/test/WebJobs.Script.Tests/Workers/Rpc/WebHostRpcWorkerChannelManagerTests.cs index 9766e9cd3a..33cbb3b150 100644 --- a/test/WebJobs.Script.Tests/Workers/Rpc/WebHostRpcWorkerChannelManagerTests.cs +++ b/test/WebJobs.Script.Tests/Workers/Rpc/WebHostRpcWorkerChannelManagerTests.cs @@ -521,7 +521,7 @@ private WebHostRpcWorkerChannelManager CreateChannelManager(IRpcWorkerChannelFac channelFactory ??= _rpcWorkerChannelFactory; config ??= _emptyConfig; return new WebHostRpcWorkerChannelManager(_eventManager, _testEnvironment, _loggerFactory, channelFactory, - _optionsMonitor, metrics, config, _workerProfileManager, new OptionsWrapper(new FunctionsHostingConfigOptions())); + _optionsMonitor, metrics, config, _workerProfileManager, new TestOptionsMonitor(TestHelpers.GetTestLanguageWorkerOptions()), new OptionsWrapper(new FunctionsHostingConfigOptions())); } private bool AreRequiredMetricsEmitted(TestMetricsLogger metricsLogger) From 706dc1f4af744d2d2ed5d8bf910c0127a1cc4607 Mon Sep 17 00:00:00 2001 From: Shyju Krishnankutty Date: Fri, 27 Sep 2024 15:07:55 -0700 Subject: [PATCH 5/7] Fix test to reflect the exception message change. --- .../Description/FunctionDescriptorProviderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/WebJobs.Script.Tests/Description/FunctionDescriptorProviderTests.cs b/test/WebJobs.Script.Tests/Description/FunctionDescriptorProviderTests.cs index aca077409a..30c7a491f2 100644 --- a/test/WebJobs.Script.Tests/Description/FunctionDescriptorProviderTests.cs +++ b/test/WebJobs.Script.Tests/Description/FunctionDescriptorProviderTests.cs @@ -82,7 +82,7 @@ public void ValidateFunction_DuplicateBindingNames_Throws() _provider.ValidateFunction(functionMetadata); }); - Assert.Equal("Multiple bindings with name 'dupe' discovered. Binding names must be unique.", ex.Message); + Assert.Equal($"{nameof(FunctionDescriptorProvider)}: Multiple bindings with name 'dupe' discovered. Binding names must be unique.", ex.Message); } [Fact] From 624a870063e62d5559059370295a308bec26d86e Mon Sep 17 00:00:00 2001 From: Fabio Cavalcante Date: Tue, 8 Oct 2024 11:49:58 -0700 Subject: [PATCH 6/7] Update src/WebJobs.Script/ScriptHostBuilderExtensions.cs Co-authored-by: Jacob Viau --- src/WebJobs.Script/ScriptHostBuilderExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WebJobs.Script/ScriptHostBuilderExtensions.cs b/src/WebJobs.Script/ScriptHostBuilderExtensions.cs index dc514a451d..564a262fbb 100644 --- a/src/WebJobs.Script/ScriptHostBuilderExtensions.cs +++ b/src/WebJobs.Script/ScriptHostBuilderExtensions.cs @@ -333,7 +333,7 @@ public static IHostBuilder AddScriptHostCore(this IHostBuilder builder, ScriptAp // Forward th host LanguageWorkerOptions to the Job Host. var languageWorkerOptions = applicationHostOptions.RootServiceProvider.GetService>(); services.AddSingleton(languageWorkerOptions); - services.AddSingleton>((s) => new OptionsWrapper(languageWorkerOptions.CurrentValue)); + services.AddSingleton>(s => new OptionsWrapper(languageWorkerOptions.CurrentValue)); services.ConfigureOptions(); } else From 1b8cff084971745aca2c66cf9a75e9b5da0863c5 Mon Sep 17 00:00:00 2001 From: Fabio Cavalcante Date: Tue, 8 Oct 2024 11:50:08 -0700 Subject: [PATCH 7/7] Update src/WebJobs.Script/ScriptHostBuilderExtensions.cs Co-authored-by: Jacob Viau --- src/WebJobs.Script/ScriptHostBuilderExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/WebJobs.Script/ScriptHostBuilderExtensions.cs b/src/WebJobs.Script/ScriptHostBuilderExtensions.cs index 564a262fbb..d9790e27c2 100644 --- a/src/WebJobs.Script/ScriptHostBuilderExtensions.cs +++ b/src/WebJobs.Script/ScriptHostBuilderExtensions.cs @@ -330,7 +330,7 @@ public static IHostBuilder AddScriptHostCore(this IHostBuilder builder, ScriptAp if (applicationHostOptions.HasParentScope) { - // Forward th host LanguageWorkerOptions to the Job Host. + // Forward the host LanguageWorkerOptions to the Job Host. var languageWorkerOptions = applicationHostOptions.RootServiceProvider.GetService>(); services.AddSingleton(languageWorkerOptions); services.AddSingleton>(s => new OptionsWrapper(languageWorkerOptions.CurrentValue));