Skip to content

Commit f138d7c

Browse files
authored
Marking SyncTriggers [RequiresRunningHost]. (Addresses #9904) (#10225)
1 parent 6355815 commit f138d7c

File tree

6 files changed

+21
-91
lines changed

6 files changed

+21
-91
lines changed

src/WebJobs.Script.WebHost/Controllers/HostController.cs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -391,24 +391,11 @@ public IActionResult LaunchDebugger()
391391
[HttpPost]
392392
[Route("admin/host/synctriggers")]
393393
[Authorize(Policy = PolicyNames.AdminAuthLevelOrInternal)]
394+
[RequiresRunningHost]
394395
public async Task<IActionResult> SyncTriggers()
395396
{
396397
_metricsLogger.LogEvent(MetricEventNames.SyncTriggersInvoked);
397398

398-
// TEMP: Collecting temporary metrics on the host state during SyncTrigger requests
399-
if (!_scriptHostManager.HostIsInitialized())
400-
{
401-
_metricsLogger.LogEvent(MetricEventNames.SyncTriggersHostNotInitialized);
402-
}
403-
404-
// TODO: We plan on making SyncTriggers RequiresRunningHost across the board,
405-
// but for now only for Flex.
406-
// https://github.com/Azure/azure-functions-host/issues/9904
407-
if (_environment.IsFlexConsumptionSku())
408-
{
409-
await HttpContext.WaitForRunningHostAsync(_scriptHostManager, _applicationHostOptions.Value);
410-
}
411-
412399
var result = await _functionsSyncManager.TrySyncTriggersAsync();
413400

414401
// Return a dummy body to make it valid in ARM template action evaluation

src/WebJobs.Script.WebHost/Management/FunctionsSyncManager.cs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -316,17 +316,11 @@ private async Task<SyncTriggersPayload> GetSyncTriggersPayload()
316316
var triggersArray = new JArray(triggers);
317317
int count = triggersArray.Count;
318318

319+
// add host configuration
319320
JObject hostConfig = null;
320-
if (_environment.IsFlexConsumptionSku())
321+
if (Utility.TryGetHostService<IHostOptionsProvider>(_scriptHostManager, out IHostOptionsProvider hostOptionsProvider))
321322
{
322-
// TODO: Only adding host configuration for Flex. Once SyncTriggers is marked RequiresRunningHost,
323-
// we'll enable across the board.
324-
// https://github.com/Azure/azure-functions-host/issues/9904
325-
// If an active host is running, add host configuration.
326-
if (Utility.TryGetHostService<IHostOptionsProvider>(_scriptHostManager, out IHostOptionsProvider hostOptionsProvider))
327-
{
328-
hostConfig = hostOptionsProvider.GetOptions();
329-
}
323+
hostConfig = hostOptionsProvider.GetOptions();
330324
}
331325

332326
// Form the base minimal result

src/WebJobs.Script/Diagnostics/MetricEventNames.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ public static class MetricEventNames
2323
public const string HISStrictModeEnabled = "host.hismode.strict";
2424
public const string HISStrictModeWarn = "host.hismode.warn";
2525
public const string SyncTriggersInvoked = "host.synctriggers.invoke";
26-
public const string SyncTriggersHostNotInitialized = "host.synctriggers.hostnotinitialized";
2726

2827
// Script host level events
2928
public const string ScriptHostManagerBuildScriptHost = "scripthostmanager.buildscripthost.latency";

test/WebJobs.Script.Tests.Integration/Management/FunctionsSyncManagerTests.cs

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -269,13 +269,9 @@ public async Task TrySyncTriggers_StandbyMode_ReturnsFalse()
269269
}
270270
}
271271

272-
[Theory]
273-
[InlineData(ScriptConstants.DynamicSku)]
274-
[InlineData(ScriptConstants.FlexConsumptionSku)]
275-
public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds(string sku)
272+
[Fact]
273+
public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds()
276274
{
277-
_sku = sku;
278-
279275
// create a dummy file that pushes us over size
280276
string maxString = new string('x', ScriptConstants.MaxTriggersStringLength + 1);
281277
_function1 = $"{{ bindings: [], test: '{maxString}'}}";
@@ -291,19 +287,14 @@ public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds(string sku
291287
Assert.True(syncString.Length < ScriptConstants.MaxTriggersStringLength);
292288
var syncContent = JObject.Parse(syncString);
293289

294-
bool isFlexConsumptionSku = _mockEnvironment.Object.IsFlexConsumptionSku();
295-
int expectedCount = isFlexConsumptionSku ? 3 : 2;
296-
Assert.Equal(expectedCount, syncContent.Count);
290+
Assert.Equal(3, syncContent.Count);
297291
Assert.Equal("testhostid123", syncContent["hostId"]);
298292

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

302-
if (isFlexConsumptionSku)
303-
{
304-
JObject hostConfig = (JObject)syncContent["hostConfig"];
305-
Assert.Equal(2, hostConfig.Count);
306-
}
296+
JObject hostConfig = (JObject)syncContent["hostConfig"];
297+
Assert.Equal(2, hostConfig.Count);
307298
}
308299
}
309300

@@ -328,15 +319,11 @@ public void ArmCacheEnabled_VerifyDefault()
328319
}
329320

330321
[Theory]
331-
[InlineData(true, true, ScriptConstants.DynamicSku)]
332-
[InlineData(true, true, ScriptConstants.FlexConsumptionSku)]
333-
[InlineData(false, true, ScriptConstants.DynamicSku)]
334-
[InlineData(false, true, ScriptConstants.FlexConsumptionSku)]
335-
[InlineData(true, false, ScriptConstants.DynamicSku)]
336-
public async Task TrySyncTriggers_PostsExpectedContent(bool cacheEnabled, bool swtIssuerEnabled, string sku)
322+
[InlineData(true, true)]
323+
[InlineData(false, true)]
324+
[InlineData(true, false)]
325+
public async Task TrySyncTriggers_PostsExpectedContent(bool cacheEnabled, bool swtIssuerEnabled)
337326
{
338-
_sku = sku;
339-
340327
_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteArmCacheEnabled)).Returns(cacheEnabled ? "1" : "0");
341328
_hostingConfigOptions.SwtIssuerEnabled = swtIssuerEnabled;
342329

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

436-
if (_mockEnvironment.Object.IsFlexConsumptionSku())
437-
{
438-
// verify hostConfig by spot checking a couple properties
439-
var hostConfig = result["hostConfig"];
440-
Assert.Equal(2, hostConfig["extensions"]["testExtension2"]["p2"]);
441-
Assert.Equal("[Hidden Credential]", hostConfig["extensions"]["testExtension2"]["pwd"]);
442-
Assert.Equal(0.85f, (float)hostConfig["concurrency"]["CPUThreshold"]);
443-
}
423+
// verify hostConfig by spot checking a couple properties
424+
var hostConfig = result["hostConfig"];
425+
Assert.Equal(2, hostConfig["extensions"]["testExtension2"]["p2"]);
426+
Assert.Equal("[Hidden Credential]", hostConfig["extensions"]["testExtension2"]["pwd"]);
427+
Assert.Equal(0.85f, (float)hostConfig["concurrency"]["CPUThreshold"]);
444428

445429
return result;
446430
}

test/WebJobs.Script.Tests/Controllers/Admin/HostControllerTests.cs

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System;
55
using System.IO;
6-
using System.Linq;
76
using System.Net;
87
using System.Threading;
98
using System.Threading.Tasks;
@@ -12,7 +11,6 @@
1211
using Microsoft.Azure.WebJobs.Host;
1312
using Microsoft.Azure.WebJobs.Host.Executors;
1413
using Microsoft.Azure.WebJobs.Host.Scale;
15-
using Microsoft.Azure.WebJobs.Script.Diagnostics;
1614
using Microsoft.Azure.WebJobs.Script.ExtensionBundle;
1715
using Microsoft.Azure.WebJobs.Script.Models;
1816
using Microsoft.Azure.WebJobs.Script.Scale;
@@ -249,40 +247,6 @@ public async Task Ping_ReturnsOk()
249247
Assert.Equal((int)HttpStatusCode.OK, result.StatusCode);
250248
}
251249

252-
[Theory]
253-
[InlineData(true, ScriptConstants.FlexConsumptionSku)]
254-
[InlineData(false, ScriptConstants.FlexConsumptionSku)]
255-
[InlineData(true, ScriptConstants.DynamicSku)]
256-
[InlineData(false, ScriptConstants.DynamicSku)]
257-
public async Task SyncTriggers_RequiresRunningHost_ForFlexSku(bool hostInitialized, string sku)
258-
{
259-
if (!hostInitialized)
260-
{
261-
_scriptHostState = ScriptHostState.Default;
262-
263-
_ = Task.Run(async () =>
264-
{
265-
// simulate coming online after a short period
266-
await Task.Delay(1000);
267-
_scriptHostState = ScriptHostState.Running;
268-
});
269-
}
270-
271-
_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteSku)).Returns(sku);
272-
_functionsSyncManager.Setup(p => p.TrySyncTriggersAsync(false)).ReturnsAsync(new SyncTriggersResult { Success = true });
273-
274-
var result = (ObjectResult)(await _hostController.SyncTriggers());
275-
Assert.Equal((int)HttpStatusCode.OK, result.StatusCode);
276-
277-
var loggedEvents = _testMetricsLogger.LoggedEvents.ToArray();
278-
Assert.Single(loggedEvents.Where(p => p == MetricEventNames.SyncTriggersInvoked));
279-
280-
if (!hostInitialized)
281-
{
282-
Assert.Single(loggedEvents.Where(p => p == MetricEventNames.SyncTriggersHostNotInitialized));
283-
}
284-
}
285-
286250
[Theory]
287251
[InlineData(0, 0, DrainModeState.Disabled)]
288252
[InlineData(0, 0, DrainModeState.Completed)]

test/WebJobs.Script.Tests/Diagnostics/FileWriterTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ await TestHelpers.Await(() =>
9191
{
9292
files = directory.GetFiles().OrderByDescending(p => p.LastWriteTime).ToArray();
9393
return files.Length == 2;
94-
}, timeout: 5000);
94+
}, timeout: 10000);
9595

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

198+
await Task.Delay(100);
199+
198200
var directory = new DirectoryInfo(_logFilePath);
199201
int count = directory.EnumerateFiles().Count();
200202
Assert.Equal(1, count);

0 commit comments

Comments
 (0)