Skip to content

Commit

Permalink
Marking SyncTriggers [RequiresRunningHost]. (Addresses #9904)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mathewc
mathewc committed Jun 14, 2024
1 parent 6355815 commit 1097388
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 91 deletions.
15 changes: 1 addition & 14 deletions src/WebJobs.Script.WebHost/Controllers/HostController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,24 +391,11 @@ public IActionResult LaunchDebugger()
[HttpPost]
[Route("admin/host/synctriggers")]
[Authorize(Policy = PolicyNames.AdminAuthLevelOrInternal)]
[RequiresRunningHost]
public async Task<IActionResult> SyncTriggers()
{
_metricsLogger.LogEvent(MetricEventNames.SyncTriggersInvoked);

// TEMP: Collecting temporary metrics on the host state during SyncTrigger requests
if (!_scriptHostManager.HostIsInitialized())
{
_metricsLogger.LogEvent(MetricEventNames.SyncTriggersHostNotInitialized);
}

// TODO: We plan on making SyncTriggers RequiresRunningHost across the board,
// but for now only for Flex.
// https://github.com/Azure/azure-functions-host/issues/9904
if (_environment.IsFlexConsumptionSku())
{
await HttpContext.WaitForRunningHostAsync(_scriptHostManager, _applicationHostOptions.Value);
}

var result = await _functionsSyncManager.TrySyncTriggersAsync();

// Return a dummy body to make it valid in ARM template action evaluation
Expand Down
12 changes: 3 additions & 9 deletions src/WebJobs.Script.WebHost/Management/FunctionsSyncManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,17 +316,11 @@ private async Task<SyncTriggersPayload> GetSyncTriggersPayload()
var triggersArray = new JArray(triggers);
int count = triggersArray.Count;

// add host configuration
JObject hostConfig = null;
if (_environment.IsFlexConsumptionSku())
if (Utility.TryGetHostService<IHostOptionsProvider>(_scriptHostManager, out IHostOptionsProvider hostOptionsProvider))
{
// TODO: Only adding host configuration for Flex. Once SyncTriggers is marked RequiresRunningHost,
// we'll enable across the board.
// https://github.com/Azure/azure-functions-host/issues/9904
// If an active host is running, add host configuration.
if (Utility.TryGetHostService<IHostOptionsProvider>(_scriptHostManager, out IHostOptionsProvider hostOptionsProvider))
{
hostConfig = hostOptionsProvider.GetOptions();
}
hostConfig = hostOptionsProvider.GetOptions();
}

// Form the base minimal result
Expand Down
1 change: 0 additions & 1 deletion src/WebJobs.Script/Diagnostics/MetricEventNames.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ public static class MetricEventNames
public const string HISStrictModeEnabled = "host.hismode.strict";
public const string HISStrictModeWarn = "host.hismode.warn";
public const string SyncTriggersInvoked = "host.synctriggers.invoke";
public const string SyncTriggersHostNotInitialized = "host.synctriggers.hostnotinitialized";

// Script host level events
public const string ScriptHostManagerBuildScriptHost = "scripthostmanager.buildscripthost.latency";
Expand Down
44 changes: 14 additions & 30 deletions test/WebJobs.Script.Tests.Integration/Management/FunctionsSyncManagerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,9 @@ public async Task TrySyncTriggers_StandbyMode_ReturnsFalse()
}
}

[Theory]
[InlineData(ScriptConstants.DynamicSku)]
[InlineData(ScriptConstants.FlexConsumptionSku)]
public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds(string sku)
[Fact]
public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds()
{
_sku = sku;

// create a dummy file that pushes us over size
string maxString = new string('x', ScriptConstants.MaxTriggersStringLength + 1);
_function1 = $"{{ bindings: [], test: '{maxString}'}}";
Expand All @@ -291,19 +287,14 @@ public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds(string sku
Assert.True(syncString.Length < ScriptConstants.MaxTriggersStringLength);
var syncContent = JObject.Parse(syncString);

bool isFlexConsumptionSku = _mockEnvironment.Object.IsFlexConsumptionSku();
int expectedCount = isFlexConsumptionSku ? 3 : 2;
Assert.Equal(expectedCount, syncContent.Count);
Assert.Equal(3, syncContent.Count);
Assert.Equal("testhostid123", syncContent["hostId"]);

JArray triggers = (JArray)syncContent["triggers"];
Assert.Equal(2, triggers.Count);

if (isFlexConsumptionSku)
{
JObject hostConfig = (JObject)syncContent["hostConfig"];
Assert.Equal(2, hostConfig.Count);
}
JObject hostConfig = (JObject)syncContent["hostConfig"];
Assert.Equal(2, hostConfig.Count);
}
}

Expand All @@ -328,15 +319,11 @@ public void ArmCacheEnabled_VerifyDefault()
}

[Theory]
[InlineData(true, true, ScriptConstants.DynamicSku)]
[InlineData(true, true, ScriptConstants.FlexConsumptionSku)]
[InlineData(false, true, ScriptConstants.DynamicSku)]
[InlineData(false, true, ScriptConstants.FlexConsumptionSku)]
[InlineData(true, false, ScriptConstants.DynamicSku)]
public async Task TrySyncTriggers_PostsExpectedContent(bool cacheEnabled, bool swtIssuerEnabled, string sku)
[InlineData(true, true)]
[InlineData(false, true)]
[InlineData(true, false)]
public async Task TrySyncTriggers_PostsExpectedContent(bool cacheEnabled, bool swtIssuerEnabled)
{
_sku = sku;

_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteArmCacheEnabled)).Returns(cacheEnabled ? "1" : "0");
_hostingConfigOptions.SwtIssuerEnabled = swtIssuerEnabled;

Expand Down Expand Up @@ -433,14 +420,11 @@ public JObject VerifyResultCommon(string connection = DefaultTestConnection, str
Assert.Equal(expectedTriggersPayload, logObject["triggers"].ToString(Formatting.None));
Assert.False(triggersLog.Contains("secrets"));

if (_mockEnvironment.Object.IsFlexConsumptionSku())
{
// verify hostConfig by spot checking a couple properties
var hostConfig = result["hostConfig"];
Assert.Equal(2, hostConfig["extensions"]["testExtension2"]["p2"]);
Assert.Equal("[Hidden Credential]", hostConfig["extensions"]["testExtension2"]["pwd"]);
Assert.Equal(0.85f, (float)hostConfig["concurrency"]["CPUThreshold"]);
}
// verify hostConfig by spot checking a couple properties
var hostConfig = result["hostConfig"];
Assert.Equal(2, hostConfig["extensions"]["testExtension2"]["p2"]);
Assert.Equal("[Hidden Credential]", hostConfig["extensions"]["testExtension2"]["pwd"]);
Assert.Equal(0.85f, (float)hostConfig["concurrency"]["CPUThreshold"]);

return result;
}
Expand Down
36 changes: 0 additions & 36 deletions test/WebJobs.Script.Tests/Controllers/Admin/HostControllerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.IO;
using System.Linq;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -12,7 +11,6 @@
using Microsoft.Azure.WebJobs.Host;
using Microsoft.Azure.WebJobs.Host.Executors;
using Microsoft.Azure.WebJobs.Host.Scale;
using Microsoft.Azure.WebJobs.Script.Diagnostics;
using Microsoft.Azure.WebJobs.Script.ExtensionBundle;
using Microsoft.Azure.WebJobs.Script.Models;
using Microsoft.Azure.WebJobs.Script.Scale;
Expand Down Expand Up @@ -249,40 +247,6 @@ public async Task Ping_ReturnsOk()
Assert.Equal((int)HttpStatusCode.OK, result.StatusCode);
}

[Theory]
[InlineData(true, ScriptConstants.FlexConsumptionSku)]
[InlineData(false, ScriptConstants.FlexConsumptionSku)]
[InlineData(true, ScriptConstants.DynamicSku)]
[InlineData(false, ScriptConstants.DynamicSku)]
public async Task SyncTriggers_RequiresRunningHost_ForFlexSku(bool hostInitialized, string sku)
{
if (!hostInitialized)
{
_scriptHostState = ScriptHostState.Default;

_ = Task.Run(async () =>
{
// simulate coming online after a short period
await Task.Delay(1000);
_scriptHostState = ScriptHostState.Running;
});
}

_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteSku)).Returns(sku);
_functionsSyncManager.Setup(p => p.TrySyncTriggersAsync(false)).ReturnsAsync(new SyncTriggersResult { Success = true });

var result = (ObjectResult)(await _hostController.SyncTriggers());
Assert.Equal((int)HttpStatusCode.OK, result.StatusCode);

var loggedEvents = _testMetricsLogger.LoggedEvents.ToArray();
Assert.Single(loggedEvents.Where(p => p == MetricEventNames.SyncTriggersInvoked));

if (!hostInitialized)
{
Assert.Single(loggedEvents.Where(p => p == MetricEventNames.SyncTriggersHostNotInitialized));
}
}

[Theory]
[InlineData(0, 0, DrainModeState.Disabled)]
[InlineData(0, 0, DrainModeState.Completed)]
Expand Down
4 changes: 3 additions & 1 deletion test/WebJobs.Script.Tests/Diagnostics/FileWriterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ await TestHelpers.Await(() =>
{
files = directory.GetFiles().OrderByDescending(p => p.LastWriteTime).ToArray();
return files.Length == 2;
}, timeout: 5000);
}, timeout: 10000);

// expect only 2 files to remain - the new file we just created, as well
// as the oversize file we just wrote to last (it has a new timestamp now so
Expand Down Expand Up @@ -195,6 +195,8 @@ public async Task LogsAreFlushedOnTimer()
await Task.Delay(5);
}

await Task.Delay(100);

var directory = new DirectoryInfo(_logFilePath);
int count = directory.EnumerateFiles().Count();
Assert.Equal(1, count);
Expand Down

0 comments on commit 1097388

Please sign in to comment.