Skip to content

Commit b079776

Browse files
ishank12Ishank Gupta
andauthored
[in-proc] Fix sync trigger error when AzureWebjobsStorage is not set in Managed (#11214)
* Fix sync trigger error when AzureWebjobsStorage is not set in ManagedApp Environment (#10767) * Fix sync trigger error when AzureWebjobsStorage is not set in case of ManagedAppEnvironment for Hybrid Logic Apps Issue : #10686 * Add test * Update release notes * Update release_notes.md * Resolved comments * Fixed comment * fix comment - skip hash check when client is not set * update test for managedenv * add test for kubernetes managed env * resolve comments - to use cache disabled in test --------- Co-authored-by: Ishank Gupta <ishankgupta@microsoft.com> * fixed and enabled sync trigger tests for k8s environment (#10923) * fixed and enabled sync trigger tests for k8s environment * resolve comments * correct test --------- Co-authored-by: Ishank Gupta <ishankgupta@microsoft.com> (cherry picked from commit 4c4d44b) * Update release_notes.md --------- Co-authored-by: Ishank Gupta <ishankgupta@microsoft.com>
1 parent 2ad90a0 commit b079776

File tree

3 files changed

+62
-4
lines changed

3 files changed

+62
-4
lines changed

release_notes.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,6 @@
33
<!-- Please add your release notes in the following format:
44
- My change description (#PR)
55
-->
6-
- Moving to version 1.5.7 of Microsoft.Azure.AppService.Middleware.Functions (https://github.com/Azure/azure-functions-host/pull/11232)
6+
- Allow sync trigger to happen in managed environment when `AzureWebJobsStorage` is not set (#11214)
7+
- Moving to version 1.5.7 of Microsoft.Azure.AppService.Middleware.Functions (https://github.com/Azure/azure-functions-host/pull/11232)
8+

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool isBackgroundSync
114114
PrepareSyncTriggers();
115115

116116
var hashBlobClient = await GetHashBlobAsync();
117-
if (isBackgroundSync && hashBlobClient == null && !_environment.IsKubernetesManagedHosting())
117+
if (isBackgroundSync && hashBlobClient == null && !_environment.IsAnyKubernetesEnvironment())
118118
{
119119
// short circuit before doing any work in background sync
120120
// cases where we need to check/update hash but don't have
@@ -135,7 +135,7 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool isBackgroundSync
135135

136136
bool shouldSyncTriggers = true;
137137
string newHash = null;
138-
if (isBackgroundSync && !_environment.IsKubernetesManagedHosting())
138+
if (isBackgroundSync && hashBlobClient != null)
139139
{
140140
newHash = await CheckHashAsync(hashBlobClient, payload.Content);
141141
shouldSyncTriggers = newHash != null;

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

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,62 @@ public async Task TrySyncTriggers_LocalEnvironment_ReturnsFalse()
299299
}
300300
}
301301

302+
[Fact]
303+
public async Task TrySyncTriggers_ManagedAppEnv_WithNo_AzureWebJobsStorage_ReturnsTrue()
304+
{
305+
_vars.Add("AzureWebJobsStorage", null);
306+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteArmCacheEnabled)).Returns("0");
307+
308+
using (var env = new TestScopedEnvironmentVariable(_vars))
309+
{
310+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.ManagedEnvironment)).Returns("true");
311+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("FUNCTIONS_API_SERVER")).Returns("https://appname.azurewebsites.net");
312+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("CONTAINER_APP_NAME")).Returns("appname");
313+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("CONTAINER_APP_NAMESPACE")).Returns("appns");
314+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("CONTAINER_APP_REVISION")).Returns("appname--r1");
315+
316+
// _functionsSyncManager is initialized in the constructor with all the secrets from environment,
317+
// so HostAzureBlobStorageProvider will have AzureWebJobsStorage defined in both ActiveHostConfigurationSource
318+
// and the WebHost IConfiguration source from DI.
319+
// The TestScopedEnvironmentVariable only changes the WebHost level IConfiguration
320+
// When it is set to empty/null, the connection string from the ActiveHostConfigurationSource wins (never changed since it is set in
321+
// constructor as mentioned).
322+
// Therefore, we need to force refresh the configuration with an ActiveHostChanged event. This is because setting an empty/null environment variable
323+
// removes it, but will not remove it from the ActiveHostConfigurationSource.
324+
_scriptHostManager.OnActiveHostChanged();
325+
var result = await _functionsSyncManager.TrySyncTriggersAsync(isBackgroundSync: true);
326+
Assert.True(result.Success);
327+
VerifyResultWithCacheOff(durableVersion: "V1");
328+
}
329+
}
330+
331+
[Fact]
332+
public async Task TrySyncTriggers_KubernetesManagedEnv_WithNo_AzureWebJobsStorage_ReturnsTrue()
333+
{
334+
_vars.Add("AzureWebJobsStorage", null);
335+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteArmCacheEnabled)).Returns("0");
336+
337+
using (var env = new TestScopedEnvironmentVariable(_vars))
338+
{
339+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("FUNCTIONS_API_SERVER")).Returns("https://appname.azurewebsites.net");
340+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("KUBERNETES_SERVICE_HOST")).Returns("kubhost");
341+
_mockEnvironment.Setup(p => p.GetEnvironmentVariable("POD_NAMESPACE")).Returns("podns");
342+
343+
// _functionsSyncManager is initialized in the constructor with all the secrets from environment,
344+
// so HostAzureBlobStorageProvider will have AzureWebJobsStorage defined in both ActiveHostConfigurationSource
345+
// and the WebHost IConfiguration source from DI.
346+
// The TestScopedEnvironmentVariable only changes the WebHost level IConfiguration
347+
// When it is set to empty/null, the connection string from the ActiveHostConfigurationSource wins (never changed since it is set in
348+
// constructor as mentioned).
349+
// Therefore, we need to force refresh the configuration with an ActiveHostChanged event. This is because setting an empty/null environment variable
350+
// removes it, but will not remove it from the ActiveHostConfigurationSource.
351+
_scriptHostManager.OnActiveHostChanged();
352+
var result = await _functionsSyncManager.TrySyncTriggersAsync(isBackgroundSync: true);
353+
Assert.True(result.Success);
354+
VerifyResultWithCacheOff(durableVersion: "V1");
355+
}
356+
}
357+
302358
[Fact]
303359
public void ArmCacheEnabled_VerifyDefault()
304360
{
@@ -1147,4 +1203,4 @@ public void Reset()
11471203
}
11481204
}
11491205
}
1150-
}
1206+
}

0 commit comments

Comments
 (0)