Merge branch 'master' into dev

mathewc · mathewc · commit 3614b3b121fd · 2019-03-04T19:25:31.000-08:00
diff --git a/src/WebJobs.Script.WebHost/FunctionsSyncService.cs b/src/WebJobs.Script.WebHost/FunctionsSyncService.cs
@@ -59,8 +59,8 @@ public Task StartAsync(CancellationToken cancellationToken)
 
         public Task StopAsync(CancellationToken cancellationToken)
         {
-            // cancel the timer
-            _syncTimer.Change(Timeout.Infinite, Timeout.Infinite);
+            // cancel the timer if it has been started
+            _syncTimer?.Change(Timeout.Infinite, Timeout.Infinite);
 
             return Task.CompletedTask;
         }
diff --git a/src/WebJobs.Script.WebHost/Management/FunctionsSyncManager.cs b/src/WebJobs.Script.WebHost/Management/FunctionsSyncManager.cs
@@ -40,11 +40,13 @@ public class FunctionsSyncManager : IFunctionsSyncManager, IDisposable
         private readonly ISecretManagerProvider _secretManagerProvider;
         private readonly IConfiguration _configuration;
         private readonly IHostIdProvider _hostIdProvider;
+        private readonly IScriptWebHostEnvironment _webHostEnvironment;
+        private readonly IEnvironment _environment;
         private readonly SemaphoreSlim _syncSemaphore = new SemaphoreSlim(1, 1);
 
         private CloudBlockBlob _hashBlob;
 
-        public FunctionsSyncManager(IConfiguration configuration, IHostIdProvider hostIdProvider, IOptionsMonitor<ScriptApplicationHostOptions> applicationHostOptions, IOptions<LanguageWorkerOptions> languageWorkerOptions, ILoggerFactory loggerFactory, HttpClient httpClient, ISecretManagerProvider secretManagerProvider)
+        public FunctionsSyncManager(IConfiguration configuration, IHostIdProvider hostIdProvider, IOptionsMonitor<ScriptApplicationHostOptions> applicationHostOptions, IOptions<LanguageWorkerOptions> languageWorkerOptions, ILoggerFactory loggerFactory, HttpClient httpClient, ISecretManagerProvider secretManagerProvider, IScriptWebHostEnvironment webHostEnvironment, IEnvironment environment)
         {
             _applicationHostOptions = applicationHostOptions;
             _logger = loggerFactory?.CreateLogger(ScriptConstants.LogCategoryHostGeneral);
@@ -53,17 +55,27 @@ public FunctionsSyncManager(IConfiguration configuration, IHostIdProvider hostId
             _secretManagerProvider = secretManagerProvider;
             _configuration = configuration;
             _hostIdProvider = hostIdProvider;
+            _webHostEnvironment = webHostEnvironment;
+            _environment = environment;
         }
 
         public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool checkHash = false)
         {
-            try
+            var result = new SyncTriggersResult
             {
-                var result = new SyncTriggersResult
-                {
-                    Success = true
-                };
+                Success = true
+            };
 
+            if (!IsSyncTriggersEnvironment(_webHostEnvironment, _environment))
+            {
+                result.Success = false;
+                result.Error = "Invalid environment for SyncTriggers operation.";
+                _logger.LogWarning(result.Error);
+                return result;
+            }
+
+            try
+            {
                 await _syncSemaphore.WaitAsync();
 
                 var hashBlob = await GetHashBlobAsync();
@@ -96,13 +108,28 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool checkHash = fals
                     result.Success = success;
                     result.Error = error;
                 }
-
-                return result;
+            }
+            catch (Exception ex)
+            {
+                // best effort - log error and continue
+                result.Success = false;
+                result.Error = "SyncTriggers operation failed.";
+                _logger.LogError(ex, result.Error);
             }
             finally
             {
                 _syncSemaphore.Release();
             }
+
+            return result;
+        }
+
+        internal static bool IsSyncTriggersEnvironment(IScriptWebHostEnvironment webHostEnvironment, IEnvironment environment)
+        {
+            // only want to do background sync triggers when NOT
+            // in standby mode and not running locally
+            return !environment.IsCoreToolsEnvironment() &&
+                !webHostEnvironment.InStandbyMode && environment.IsContainerReady();
         }
 
         internal async Task<string> CheckHashAsync(CloudBlockBlob hashBlob, string content)
diff --git a/src/WebJobs.Script.WebHost/WebScriptHostBuilderExtension.cs b/src/WebJobs.Script.WebHost/WebScriptHostBuilderExtension.cs
@@ -10,6 +10,7 @@
 using Microsoft.Azure.WebJobs.Script.WebHost.Configuration;
 using Microsoft.Azure.WebJobs.Script.WebHost.DependencyInjection;
 using Microsoft.Azure.WebJobs.Script.WebHost.Diagnostics;
+using Microsoft.Azure.WebJobs.Script.WebHost.Management;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Hosting;
@@ -77,7 +78,13 @@ public static IHostBuilder AddWebScriptHost(this IHostBuilder builder, IServiceP
                 })
                 .ConfigureServices(services =>
                 {
-                    services.AddSingleton<IHostedService, FunctionsSyncService>();
+                    var webHostEnvironment = rootServiceProvider.GetService<IScriptWebHostEnvironment>();
+                    var environment = rootServiceProvider.GetService<IEnvironment>();
+                    if (FunctionsSyncManager.IsSyncTriggersEnvironment(webHostEnvironment, environment))
+                    {
+                        services.AddSingleton<IHostedService, FunctionsSyncService>();
+                    }
+
                     services.AddSingleton<HttpRequestQueue>();
                     services.AddSingleton<IHostLifetime, JobHostHostLifetime>();
                     services.TryAddSingleton<IWebJobsExceptionHandler, WebScriptHostExceptionHandler>();
diff --git a/test/WebJobs.Script.Tests.Shared/TestHostBuilderExtensions.cs b/test/WebJobs.Script.Tests.Shared/TestHostBuilderExtensions.cs
@@ -39,6 +39,7 @@ public static IHostBuilder ConfigureDefaultTestWebScriptHost(this IHostBuilder b
             var services = new ServiceCollection();
             AddMockedSingleton<IDebugStateProvider>(services);
             AddMockedSingleton<IScriptHostManager>(services);
+            AddMockedSingleton<IEnvironment>(services);
             AddMockedSingleton<IScriptWebHostEnvironment>(services);
             AddMockedSingleton<IEventGenerator>(services);
             AddMockedSingleton<AspNetCore.Hosting.IApplicationLifetime>(services);
diff --git a/test/WebJobs.Script.Tests/FunctionsSyncServiceTests.cs b/test/WebJobs.Script.Tests/FunctionsSyncServiceTests.cs
@@ -21,6 +21,8 @@ public class FunctionsSyncServiceTests
         private readonly Mock<IScriptHostManager> _mockScriptHostManager;
         private readonly Mock<IPrimaryHostStateProvider> _mockPrimaryHostStateProviderMock;
         private readonly Mock<IFunctionsSyncManager> _mockSyncManager;
+        private readonly Mock<IScriptWebHostEnvironment> _mockWebHostEnvironment;
+        private readonly Mock<IEnvironment> _mockEnvironment;
         private readonly int _testDueTime = 250;
 
         public FunctionsSyncServiceTests()
@@ -37,6 +39,12 @@ public FunctionsSyncServiceTests()
             _mockScriptHostManager.Setup(p => p.State).Returns(ScriptHostState.Running);
             _mockSyncManager.Setup(p => p.TrySyncTriggersAsync(true)).ReturnsAsync(new SyncTriggersResult { Success = true });
 
+            _mockWebHostEnvironment = new Mock<IScriptWebHostEnvironment>(MockBehavior.Strict);
+            _mockWebHostEnvironment.SetupGet(p => p.InStandbyMode).Returns(false);
+            _mockEnvironment = new Mock<IEnvironment>(MockBehavior.Strict);
+            _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteContainerReady)).Returns("1");
+            _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.CoreToolsEnvironment)).Returns((string)null);
+
             _syncService = new FunctionsSyncService(loggerFactory, _mockScriptHostManager.Object, _mockPrimaryHostStateProviderMock.Object, _mockSyncManager.Object);
             _syncService.DueTime = _testDueTime;
         }
@@ -98,5 +106,30 @@ public async Task UnhandledExceptions_HandledInCallback()
 
             _mockSyncManager.Verify(p => p.TrySyncTriggersAsync(true), Times.Once);
         }
+
+        [Theory]
+        [InlineData(true, true, false)]
+        [InlineData(true, false, false)]
+        [InlineData(false, true, true)]
+        [InlineData(false, false, false)]
+        public void IsSyncTriggersEnvironment_StandbyMode_ReturnsExpectedResult(bool standbyMode, bool containerReady, bool expected)
+        {
+            _mockWebHostEnvironment.SetupGet(p => p.InStandbyMode).Returns(standbyMode);
+            _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteContainerReady)).Returns(containerReady ? "1" : null);
+
+            var result = FunctionsSyncManager.IsSyncTriggersEnvironment(_mockWebHostEnvironment.Object, _mockEnvironment.Object);
+            Assert.Equal(expected, result);
+        }
+
+        [Theory]
+        [InlineData(true, false)]
+        [InlineData(false, true)]
+        public void IsSyncTriggersEnvironment_LocalEnvironment_ReturnsExpectedResult(bool coreToolsEnvironment, bool expected)
+        {
+            _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.CoreToolsEnvironment)).Returns(coreToolsEnvironment ? "1" : null);
+
+            var result = FunctionsSyncManager.IsSyncTriggersEnvironment(_mockWebHostEnvironment.Object, _mockEnvironment.Object);
+            Assert.Equal(expected, result);
+        }
     }
 }
diff --git a/test/WebJobs.Script.Tests/Managment/FunctionsSyncManagerTests.cs b/test/WebJobs.Script.Tests/Managment/FunctionsSyncManagerTests.cs
@@ -37,6 +37,8 @@ public class FunctionsSyncManagerTests : IDisposable
         private readonly string _expectedSyncTriggersPayload;
         private readonly MockHttpHandler _mockHttpHandler;
         private readonly TestLoggerProvider _loggerProvider;
+        private readonly Mock<IScriptWebHostEnvironment> _mockWebHostEnvironment;
+        private readonly Mock<IEnvironment> _mockEnvironment;
 
         public FunctionsSyncManagerTests()
         {
@@ -78,7 +80,12 @@ public FunctionsSyncManagerTests()
             var configuration = ScriptSettingsManager.BuildDefaultConfiguration();
             var hostIdProviderMock = new Mock<IHostIdProvider>(MockBehavior.Strict);
             hostIdProviderMock.Setup(p => p.GetHostIdAsync(CancellationToken.None)).ReturnsAsync("testhostid123");
-            _functionsSyncManager = new FunctionsSyncManager(configuration, hostIdProviderMock.Object, optionsMonitor, new OptionsWrapper<LanguageWorkerOptions>(CreateLanguageWorkerConfigSettings()), loggerFactory, httpClient, secretManagerProviderMock.Object);
+            _mockWebHostEnvironment = new Mock<IScriptWebHostEnvironment>(MockBehavior.Strict);
+            _mockWebHostEnvironment.SetupGet(p => p.InStandbyMode).Returns(false);
+            _mockEnvironment = new Mock<IEnvironment>(MockBehavior.Strict);
+            _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteContainerReady)).Returns("1");
+            _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.CoreToolsEnvironment)).Returns((string)null);
+            _functionsSyncManager = new FunctionsSyncManager(configuration, hostIdProviderMock.Object, optionsMonitor, new OptionsWrapper<LanguageWorkerOptions>(CreateLanguageWorkerConfigSettings()), loggerFactory, httpClient, secretManagerProviderMock.Object, _mockWebHostEnvironment.Object, _mockEnvironment.Object);
 
             _expectedSyncTriggersPayload = "[{\"authLevel\":\"anonymous\",\"type\":\"httpTrigger\",\"direction\":\"in\",\"name\":\"req\",\"functionName\":\"function1\"}," +
                 "{\"name\":\"myQueueItem\",\"type\":\"orchestrationTrigger\",\"direction\":\"in\",\"queueName\":\"myqueue-items\",\"connection\":\"DurableStorage\",\"functionName\":\"function2\",\"taskHubName\":\"TestHubValue\"}," +
@@ -91,6 +98,32 @@ private void ResetMockFileSystem(string hostJsonContent = null)
             FileUtility.Instance = fileSystem;
         }
 
+        [Fact]
+        public async Task TrySyncTriggers_StandbyMode_ReturnsFalse()
+        {
+            using (var env = new TestScopedEnvironmentVariable(_vars))
+            {
+                _mockWebHostEnvironment.SetupGet(p => p.InStandbyMode).Returns(true);
+                var result = await _functionsSyncManager.TrySyncTriggersAsync();
+                Assert.False(result.Success);
+                var expectedMessage = "Invalid environment for SyncTriggers operation.";
+                Assert.Equal(expectedMessage, result.Error);
+            }
+        }
+
+        [Fact]
+        public async Task TrySyncTriggers_LocalEnvironment_ReturnsFalse()
+        {
+            using (var env = new TestScopedEnvironmentVariable(_vars))
+            {
+                _mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.CoreToolsEnvironment)).Returns("1");
+                var result = await _functionsSyncManager.TrySyncTriggersAsync();
+                Assert.False(result.Success);
+                var expectedMessage = "Invalid environment for SyncTriggers operation.";
+                Assert.Equal(expectedMessage, result.Error);
+            }
+        }
+
         [Fact]
         public async Task TrySyncTriggers_PostsExpectedContent()
         {
diff --git a/test/WebJobs.Script.Tests/Managment/WebFunctionsManagerTests.cs b/test/WebJobs.Script.Tests/Managment/WebFunctionsManagerTests.cs
@@ -67,7 +67,12 @@ public void ReadFunctionsMetadataSucceeds()
 
             var configurationMock = new Mock<IConfiguration>(MockBehavior.Strict);
             var hostIdProviderMock = new Mock<IHostIdProvider>(MockBehavior.Strict);
-            var functionsSyncManager = new FunctionsSyncManager(configurationMock.Object, hostIdProviderMock.Object, optionsMonitor, new OptionsWrapper<LanguageWorkerOptions>(CreateLanguageWorkerConfigSettings()), loggerFactory, httpClient, secretManagerProviderMock.Object);
+            var mockWebHostEnvironment = new Mock<IScriptWebHostEnvironment>(MockBehavior.Strict);
+            mockWebHostEnvironment.SetupGet(p => p.InStandbyMode).Returns(false);
+            var mockEnvironment = new Mock<IEnvironment>(MockBehavior.Strict);
+            mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteContainerReady)).Returns("1");
+            mockEnvironment.Setup(p => p.GetEnvironmentVariable(EnvironmentSettingNames.CoreToolsEnvironment)).Returns((string)null);
+            var functionsSyncManager = new FunctionsSyncManager(configurationMock.Object, hostIdProviderMock.Object, optionsMonitor, new OptionsWrapper<LanguageWorkerOptions>(CreateLanguageWorkerConfigSettings()), loggerFactory, httpClient, secretManagerProviderMock.Object, mockWebHostEnvironment.Object, mockEnvironment.Object);
             var webManager = new WebFunctionsManager(optionsMonitor, new OptionsWrapper<LanguageWorkerOptions>(CreateLanguageWorkerConfigSettings()), loggerFactory, httpClient, secretManagerProviderMock.Object, functionsSyncManager);
 
             FileUtility.Instance = fileSystem;

Original file line number	Diff line number	Diff line change
`@@ -59,8 +59,8 @@ public Task StartAsync(CancellationToken cancellationToken)`
`59`	`59`
`60`	`60`	`public Task StopAsync(CancellationToken cancellationToken)`
`61`	`61`	`{`
`62`		`- // cancel the timer`
`63`		`- _syncTimer.Change(Timeout.Infinite, Timeout.Infinite);`
	`62`	`+ // cancel the timer if it has been started`
	`63`	`+ _syncTimer?.Change(Timeout.Infinite, Timeout.Infinite);`
`64`	`64`
`65`	`65`	`return Task.CompletedTask;`
`66`	`66`	`}`