From bad8566afaa1ad0f0c43d61658c380d940ed7525 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Mon, 7 Oct 2019 17:01:37 -0700 Subject: [PATCH 1/5] Move AcceptableFunctionAppDependenciesAlreadyInstalled trace from BackgroundDependencySnapshotMaintainer to DependencyManager.InstallFunctionAppDependencies, so that it happens only once on PS Worker start --- .../BackgroundDependencySnapshotMaintainer.cs | 5 ----- src/DependencyManagement/DependencyManager.cs | 5 +++++ ...ackgroundDependencySnapshotMaintainerTests.cs | 16 ---------------- .../DependencyManagerTests.cs | 8 ++++++++ 4 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs b/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs index acaaef98..bb4de00b 100644 --- a/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs +++ b/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs @@ -56,11 +56,6 @@ public string InstallAndPurgeSnapshots(Func pwshFactory, ILogger log { try { - logger.Log( - isUserOnlyLog: false, - RpcLog.Types.Level.Trace, - PowerShellWorkerStrings.AcceptableFunctionAppDependenciesAlreadyInstalled); - // Purge before installing a new snapshot, as we may be able to free some space. _purger.Purge(logger); diff --git a/src/DependencyManagement/DependencyManager.cs b/src/DependencyManagement/DependencyManager.cs index 8c4d5dac..098b6b82 100644 --- a/src/DependencyManagement/DependencyManager.cs +++ b/src/DependencyManagement/DependencyManager.cs @@ -185,6 +185,11 @@ internal Exception InstallFunctionAppDependencies(PowerShell firstPwsh, Func _.Purge(_mockLogger.Object), Times.Exactly(2)); } - - _mockLogger.Verify( - _ => _.Log( - false, - LogLevel.Trace, - It.Is(message => message.Contains(PowerShellWorkerStrings.AcceptableFunctionAppDependenciesAlreadyInstalled)), - It.IsAny()), - Times.Once); } [Fact] @@ -97,14 +89,6 @@ public void DoesNotInstallSnapshotIfRecentlyInstalledSnapshotFound() _mockPurger.Verify(_ => _.Purge(_mockLogger.Object), Times.Once); } - - _mockLogger.Verify( - _ => _.Log( - false, - LogLevel.Trace, - It.Is(message => message.Contains(PowerShellWorkerStrings.AcceptableFunctionAppDependenciesAlreadyInstalled)), - It.IsAny()), - Times.Once); } [Fact] diff --git a/test/Unit/DependencyManagement/DependencyManagerTests.cs b/test/Unit/DependencyManagement/DependencyManagerTests.cs index 1a90c1a5..21fadb75 100644 --- a/test/Unit/DependencyManagement/DependencyManagerTests.cs +++ b/test/Unit/DependencyManagement/DependencyManagerTests.cs @@ -184,6 +184,14 @@ public void StartDependencyInstallationIfNeeded_InvokesBackgroundMaintainer_When _ => _.InstallAndPurgeSnapshots(powerShellFactory, _mockLogger.Object), Times.Once); } + + _mockLogger.Verify( + _ => _.Log( + false, + LogLevel.Trace, + It.Is(message => message.Contains(PowerShellWorkerStrings.AcceptableFunctionAppDependenciesAlreadyInstalled)), + It.IsAny()), + Times.Once); } [Fact] From 63854854ea3e346d782754bcc202ddb2c6535b2d Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Wed, 9 Oct 2019 12:30:38 -0700 Subject: [PATCH 2/5] Pass DependencySnapshotInstallationMode instead of a boolean to IDependencySnapshotInstaller.InstallSnapshot --- .../BackgroundDependencySnapshotMaintainer.cs | 9 ++- src/DependencyManagement/DependencyManager.cs | 10 +-- .../DependencySnapshotInstallationMode.cs | 15 +++++ .../DependencySnapshotInstaller.cs | 30 +++++++-- .../IDependencySnapshotInstaller.cs | 2 +- ...groundDependencySnapshotMaintainerTests.cs | 6 +- .../DependencyManagerTests.cs | 8 +-- .../DependencySnapshotInstallerTests.cs | 64 +++++++++---------- 8 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 src/DependencyManagement/DependencySnapshotInstallationMode.cs diff --git a/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs b/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs index bb4de00b..deba2e90 100644 --- a/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs +++ b/src/DependencyManagement/BackgroundDependencySnapshotMaintainer.cs @@ -72,12 +72,11 @@ public string InstallAndPurgeSnapshots(Func pwshFactory, ILogger log _dependencyManifest, nextSnapshotPath, pwsh, - // If the new snapshot turns out to be equivalent to the latest one, - // removing it helps us save storage space and avoid unnecessary worker restarts. - // It is ok to do that during background upgrade because the current + // Background dependency upgrades are optional because the current // worker already has a good enough snapshot, and nothing depends on - // the new snapshot yet. - removeIfEquivalentToLatest: true, + // the new snapshot yet, so installation failures will not affect + // function invocations. + DependencySnapshotInstallationMode.Optional, logger); } diff --git a/src/DependencyManagement/DependencyManager.cs b/src/DependencyManagement/DependencyManager.cs index 098b6b82..563d1dbf 100644 --- a/src/DependencyManagement/DependencyManager.cs +++ b/src/DependencyManagement/DependencyManager.cs @@ -215,13 +215,9 @@ private void InstallSnapshotInForeground(PowerShell pwsh, ILogger logger) _dependenciesFromManifest, _currentSnapshotPath, pwsh, - // Even if the new snapshot turns out to be equivalent to the latest one, - // removing it would not be safe because the current worker already depends - // on it, as it has the path to this snapshot already added to PSModulePath. - // As opposed to the background upgrade case, this snapshot is *required* for - // this worker to run, even though it occupies some space (until the workers - // restart and the redundant snapshots are purged). - removeIfEquivalentToLatest: false, + // As opposed to the background upgrade case, the worker does not have an acceptable + // snapshot to use yet, so the initial snapshot is *required* for this worker to run. + DependencySnapshotInstallationMode.Required, logger); } catch (Exception e) diff --git a/src/DependencyManagement/DependencySnapshotInstallationMode.cs b/src/DependencyManagement/DependencySnapshotInstallationMode.cs new file mode 100644 index 00000000..ef32428c --- /dev/null +++ b/src/DependencyManagement/DependencySnapshotInstallationMode.cs @@ -0,0 +1,15 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +#pragma warning disable 1591 // "Mixing XML comments for publicly visible type" + +namespace Microsoft.Azure.Functions.PowerShellWorker.DependencyManagement +{ + public enum DependencySnapshotInstallationMode + { + Required, + Optional + } +} diff --git a/src/DependencyManagement/DependencySnapshotInstaller.cs b/src/DependencyManagement/DependencySnapshotInstaller.cs index ee54dfe1..a8bfa711 100644 --- a/src/DependencyManagement/DependencySnapshotInstaller.cs +++ b/src/DependencyManagement/DependencySnapshotInstaller.cs @@ -40,7 +40,7 @@ public void InstallSnapshot( IEnumerable dependencies, string targetPath, PowerShell pwsh, - bool removeIfEquivalentToLatest, + DependencySnapshotInstallationMode installationMode, ILogger logger) { var installingPath = CreateInstallingSnapshot(targetPath); @@ -57,13 +57,29 @@ public void InstallSnapshot( InstallModule(module, installingPath, pwsh, logger); } - if (removeIfEquivalentToLatest) + switch (installationMode) { - PromoteToInstalledOrRemove(installingPath, targetPath, logger); - } - else - { - PromoteToInstalled(installingPath, targetPath, logger); + case DependencySnapshotInstallationMode.Optional: + // If the new snapshot turns out to be equivalent to the latest one, + // removing it helps us save storage space and avoid unnecessary worker restarts. + // It is ok to do that during background upgrade because the current + // worker already has a good enough snapshot, and nothing depends on + // the new snapshot yet. + PromoteToInstalledOrRemove(installingPath, targetPath, logger); + break; + + case DependencySnapshotInstallationMode.Required: + // Even if the new snapshot turns out to be equivalent to the latest one, + // removing it would not be safe because the current worker already depends + // on it, as it has the path to this snapshot already added to PSModulePath. + // As opposed to the background upgrade case, this snapshot is *required* for + // this worker to run, even though it occupies some space (until the workers + // restart and the redundant snapshots are purged). + PromoteToInstalled(installingPath, targetPath, logger); + break; + + default: + throw new ArgumentException($"Unexpected installation mode: {installationMode}", nameof(installationMode)); } } catch (Exception e) diff --git a/src/DependencyManagement/IDependencySnapshotInstaller.cs b/src/DependencyManagement/IDependencySnapshotInstaller.cs index 20cfb16b..402f42da 100644 --- a/src/DependencyManagement/IDependencySnapshotInstaller.cs +++ b/src/DependencyManagement/IDependencySnapshotInstaller.cs @@ -15,7 +15,7 @@ void InstallSnapshot( IEnumerable dependencies, string targetPath, PowerShell pwsh, - bool removeIfEquivalentToLatest, + DependencySnapshotInstallationMode installationMode, ILogger logger); } } diff --git a/test/Unit/DependencyManagement/BackgroundDependencySnapshotMaintainerTests.cs b/test/Unit/DependencyManagement/BackgroundDependencySnapshotMaintainerTests.cs index 9aca4690..aac937d9 100644 --- a/test/Unit/DependencyManagement/BackgroundDependencySnapshotMaintainerTests.cs +++ b/test/Unit/DependencyManagement/BackgroundDependencySnapshotMaintainerTests.cs @@ -52,7 +52,7 @@ public void InstallsSnapshotIfNoRecentlyInstalledSnapshotFound() It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), + It.IsAny(), It.IsAny())); using (var dummyPowerShell = PowerShell.Create()) @@ -66,7 +66,7 @@ public void InstallsSnapshotIfNoRecentlyInstalledSnapshotFound() // ReSharper disable once AccessToDisposedClosure _mockInstaller.Verify( - _ => _.InstallSnapshot(_dependencyManifest, "new snapshot path", dummyPowerShell, true, _mockLogger.Object), + _ => _.InstallSnapshot(_dependencyManifest, "new snapshot path", dummyPowerShell, DependencySnapshotInstallationMode.Optional, _mockLogger.Object), Times.Once); _mockPurger.Verify(_ => _.Purge(_mockLogger.Object), Times.Exactly(2)); @@ -107,7 +107,7 @@ public void LogsWarningIfCannotInstallSnapshot() It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), + It.IsAny(), It.IsAny())) .Throws(injectedException); diff --git a/test/Unit/DependencyManagement/DependencyManagerTests.cs b/test/Unit/DependencyManagement/DependencyManagerTests.cs index 21fadb75..24a29f57 100644 --- a/test/Unit/DependencyManagement/DependencyManagerTests.cs +++ b/test/Unit/DependencyManagement/DependencyManagerTests.cs @@ -137,7 +137,7 @@ public void StartDependencyInstallationIfNeeded_InstallsSnapshotInForeground_Whe "NewSnapshot", // Must run on the same runspace It.Is(powerShell => ReferenceEquals(firstPowerShellRunspace, powerShell)), - false, // removeIfEquivalentToLatest + DependencySnapshotInstallationMode.Required, _mockLogger.Object)); _mockStorage.Setup(_ => _.SnapshotExists("NewSnapshot")).Returns(false); @@ -210,7 +210,7 @@ public void StartDependencyInstallationIfNeeded_InstallsSnapshotInForeground_Whe "NewSnapshot", // Must run on the same runspace It.Is(powerShell => ReferenceEquals(firstPowerShellRunspace, powerShell)), - false, // removeIfEquivalentToLatest + DependencySnapshotInstallationMode.Required, _mockLogger.Object)); _mockStorage.Setup(_ => _.SnapshotExists("NewSnapshot")).Returns(false); @@ -252,7 +252,7 @@ public void StartDependencyInstallationIfNeeded_HandlesExceptionThrownBy_Install It.IsAny>(), It.IsAny(), It.IsAny(), - It.IsAny(), + It.IsAny(), It.IsAny())) .Throws(injectedException); @@ -281,7 +281,7 @@ private void VerifyExactlyOneSnapshotInstalled() It.IsAny>(), It.IsAny(), It.IsAny(), - It.IsAny(), + It.IsAny(), It.IsAny()), Times.Once()); diff --git a/test/Unit/DependencyManagement/DependencySnapshotInstallerTests.cs b/test/Unit/DependencyManagement/DependencySnapshotInstallerTests.cs index 7ddfe8a0..ce8ba6b5 100644 --- a/test/Unit/DependencyManagement/DependencySnapshotInstallerTests.cs +++ b/test/Unit/DependencyManagement/DependencySnapshotInstallerTests.cs @@ -49,15 +49,15 @@ public void DoesNothingOnConstruction() } [Theory] - [InlineData(true)] - [InlineData(false)] - public void SavesSpecifiedVersion_WhenExactVersionIsSpecified(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void SavesSpecifiedVersion_WhenExactVersionIsSpecified(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.ExactVersion, "Exact version") }; var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest, _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), installationMode, _mockLogger.Object); _mockModuleProvider.Verify( _ => _.SaveModule(It.IsAny(), "Module", "Exact version", _targetPathInstalling), @@ -69,9 +69,9 @@ public void SavesSpecifiedVersion_WhenExactVersionIsSpecified(bool removeIfEquiv } [Theory] - [InlineData(true)] - [InlineData(false)] - public void SavesLatestPublishedVersion_WhenMajorVersionIsSpecified(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void SavesLatestPublishedVersion_WhenMajorVersionIsSpecified(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.MajorVersion, "Major version") }; @@ -81,7 +81,7 @@ public void SavesLatestPublishedVersion_WhenMajorVersionIsSpecified(bool removeI .Returns("Latest version"); var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest, _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), installationMode, _mockLogger.Object); _mockModuleProvider.Verify( _ => _.SaveModule(It.IsAny(), "Module", "Latest version", _targetPathInstalling), @@ -89,15 +89,15 @@ public void SavesLatestPublishedVersion_WhenMajorVersionIsSpecified(bool removeI } [Theory] - [InlineData(true)] - [InlineData(false)] - public void PromotesInstallingSnapshotToInstalledIfSaveModuleDoesNotThrow(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void PromotesInstallingSnapshotToInstalledIfSaveModuleDoesNotThrow(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.ExactVersion, "Version") }; var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest, _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), installationMode, _mockLogger.Object); _mockStorage.Verify(_ => _.CreateInstallingSnapshot(_targetPathInstalled), Times.Once); _mockStorage.Verify(_ => _.PromoteInstallingSnapshotToInstalledAtomically(_targetPathInstalled), Times.Once); @@ -114,7 +114,7 @@ public void PromotesInstallingSnapshotToInstalledIfNotEquivalentToLatest() _mockSnapshotComparer.Setup(_ => _.AreEquivalent(_targetPathInstalling, "snapshot", _mockLogger.Object)).Returns(false); var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest: true, logger: _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), DependencySnapshotInstallationMode.Optional, logger: _mockLogger.Object); _mockStorage.Verify(_ => _.CreateInstallingSnapshot(_targetPathInstalled), Times.Once); _mockStorage.Verify(_ => _.PromoteInstallingSnapshotToInstalledAtomically(_targetPathInstalled), Times.Once); @@ -122,7 +122,7 @@ public void PromotesInstallingSnapshotToInstalledIfNotEquivalentToLatest() } [Fact] - public void PromotesInstallingSnapshotToInstalledIfNotAskedToRemoveEquivalentSnapshots() + public void DoesNotLookForEquivalentSnapshotsInRequiredInstallationMode() { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.ExactVersion, "Version") }; @@ -130,7 +130,7 @@ public void PromotesInstallingSnapshotToInstalledIfNotAskedToRemoveEquivalentSna _mockStorage.Setup(_ => _.GetLatestInstalledSnapshot()).Returns("snapshot"); var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest: false, logger: _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), DependencySnapshotInstallationMode.Required, logger: _mockLogger.Object); _mockSnapshotComparer.VerifyNoOtherCalls(); _mockStorage.Verify(_ => _.CreateInstallingSnapshot(_targetPathInstalled), Times.Once); @@ -139,9 +139,9 @@ public void PromotesInstallingSnapshotToInstalledIfNotAskedToRemoveEquivalentSna } [Theory] - [InlineData(true)] - [InlineData(false)] - public void CleansUpPowerShellRunspaceAfterSuccessfullySavingModule(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void CleansUpPowerShellRunspaceAfterSuccessfullySavingModule(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.ExactVersion, "Version") }; @@ -149,15 +149,15 @@ public void CleansUpPowerShellRunspaceAfterSuccessfullySavingModule(bool removeI var dummyPowerShell = PowerShell.Create(); var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, dummyPowerShell, removeIfEquivalentToLatest, _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, dummyPowerShell, installationMode, _mockLogger.Object); _mockModuleProvider.Verify(_ => _.Cleanup(dummyPowerShell), Times.Once); } [Theory] - [InlineData(true)] - [InlineData(false)] - public void LogsInstallationStartAndFinish(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void LogsInstallationStartAndFinish(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] @@ -171,7 +171,7 @@ public void LogsInstallationStartAndFinish(bool removeIfEquivalentToLatest) .Returns("Exact B version"); var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest, _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), installationMode, _mockLogger.Object); VerifyLoggedOnce(new[] { "Started installing", "A", "Exact A version" }); VerifyLoggedOnce(new[] { "has been installed", "A", "Exact A version" }); @@ -180,9 +180,9 @@ public void LogsInstallationStartAndFinish(bool removeIfEquivalentToLatest) } [Theory] - [InlineData(true)] - [InlineData(false)] - public void DoesNotSaveModuleIfGetLatestPublishedModuleVersionThrows(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void DoesNotSaveModuleIfGetLatestPublishedModuleVersionThrows(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.MajorVersion, "Version") }; @@ -201,7 +201,7 @@ public void DoesNotSaveModuleIfGetLatestPublishedModuleVersionThrows(bool remove var installer = CreateDependenciesSnapshotInstallerWithMocks(); var caughtException = Assert.Throws( - () => installer.InstallSnapshot(manifestEntries, _targetPathInstalled, dummyPowerShell, removeIfEquivalentToLatest, _mockLogger.Object)); + () => installer.InstallSnapshot(manifestEntries, _targetPathInstalled, dummyPowerShell, installationMode, _mockLogger.Object)); Assert.Contains(injectedException.Message, caughtException.Message); @@ -215,9 +215,9 @@ public void DoesNotSaveModuleIfGetLatestPublishedModuleVersionThrows(bool remove } [Theory] - [InlineData(true)] - [InlineData(false)] - public void DoesNotPromoteSnapshotIfSaveModuleKeepsThrowing(bool removeIfEquivalentToLatest) + [InlineData(DependencySnapshotInstallationMode.Required)] + [InlineData(DependencySnapshotInstallationMode.Optional)] + public void DoesNotPromoteSnapshotIfSaveModuleKeepsThrowing(DependencySnapshotInstallationMode installationMode) { var manifestEntries = new[] { new DependencyManifestEntry("Module", VersionSpecificationType.ExactVersion, "Version") }; @@ -234,7 +234,7 @@ public void DoesNotPromoteSnapshotIfSaveModuleKeepsThrowing(bool removeIfEquival var installer = CreateDependenciesSnapshotInstallerWithMocks(); var thrownException = Assert.Throws( - () => installer.InstallSnapshot(manifestEntries, _targetPathInstalled, dummyPowerShell, removeIfEquivalentToLatest, _mockLogger.Object)); + () => installer.InstallSnapshot(manifestEntries, _targetPathInstalled, dummyPowerShell, installationMode, _mockLogger.Object)); Assert.Contains(injectedException.Message, thrownException.Message); @@ -255,7 +255,7 @@ public void DoesNotPromoteSnapshotIfItIsEquivalentToLatest() _mockStorage.Setup(_ => _.SetSnapshotCreationTimeToUtcNow("snapshot")); var installer = CreateDependenciesSnapshotInstallerWithMocks(); - installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), removeIfEquivalentToLatest: true, logger: _mockLogger.Object); + installer.InstallSnapshot(manifestEntries, _targetPathInstalled, PowerShell.Create(), DependencySnapshotInstallationMode.Optional, logger: _mockLogger.Object); _mockStorage.Verify(_ => _.PromoteInstallingSnapshotToInstalledAtomically(It.IsAny()), Times.Never); _mockStorage.Verify(_ => _.RemoveSnapshot(_targetPathInstalling), Times.Once); From 2ef36a0fda3d9c7d0a01877ad9498ff1789ca2b2 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Wed, 9 Oct 2019 16:50:53 -0700 Subject: [PATCH 3/5] Add installation mode to InstallingFunctionAppRequiredModules and FailedToInstallDependenciesSnapshot messages --- src/DependencyManagement/DependencySnapshotInstaller.cs | 5 ++++- src/resources/PowerShellWorkerStrings.resx | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/DependencyManagement/DependencySnapshotInstaller.cs b/src/DependencyManagement/DependencySnapshotInstaller.cs index a8bfa711..9f993ba0 100644 --- a/src/DependencyManagement/DependencySnapshotInstaller.cs +++ b/src/DependencyManagement/DependencySnapshotInstaller.cs @@ -48,7 +48,10 @@ public void InstallSnapshot( logger.Log( isUserOnlyLog: false, LogLevel.Trace, - string.Format(PowerShellWorkerStrings.InstallingFunctionAppRequiredModules, installingPath)); + string.Format( + PowerShellWorkerStrings.InstallingFunctionAppRequiredModules, + installingPath, + installationMode)); try { diff --git a/src/resources/PowerShellWorkerStrings.resx b/src/resources/PowerShellWorkerStrings.resx index 29d82432..1d55a656 100644 --- a/src/resources/PowerShellWorkerStrings.resx +++ b/src/resources/PowerShellWorkerStrings.resx @@ -200,7 +200,7 @@ Failed to resolve '{0}' path in App Service. - Installing function app required modules to '{0}'. + Installing function app required modules to '{0}' (installation mode: {1}). The enforced concurrency level (pool size limit) is '{0}'. @@ -257,7 +257,7 @@ Updating dependencies folder heartbeat for '{0}''. - Failed to install dependencies into '{0}', removing the folder. + Failed to install dependencies into '{0}' (installation mode: {1}), removing the folder. The number of entries in the '{0}' file is {1}, which exceeds the maximum supported number of entries ({2}). From 390e8a9637650ebeb6b12030be2dae301269acf4 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Wed, 9 Oct 2019 17:35:23 -0700 Subject: [PATCH 4/5] Actually add installation mode to FailedToInstallDependenciesSnapshot --- src/DependencyManagement/DependencySnapshotInstaller.cs | 6 +++++- test/Unit/DependencyManagement/DependencyManagementTests.cs | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/DependencyManagement/DependencySnapshotInstaller.cs b/src/DependencyManagement/DependencySnapshotInstaller.cs index 9f993ba0..7e31aab4 100644 --- a/src/DependencyManagement/DependencySnapshotInstaller.cs +++ b/src/DependencyManagement/DependencySnapshotInstaller.cs @@ -87,7 +87,11 @@ public void InstallSnapshot( } catch (Exception e) { - var message = string.Format(PowerShellWorkerStrings.FailedToInstallDependenciesSnapshot, targetPath); + var message = string.Format( + PowerShellWorkerStrings.FailedToInstallDependenciesSnapshot, + targetPath, + installationMode); + logger.Log(isUserOnlyLog: false, LogLevel.Warning, message, e); _storage.RemoveSnapshot(installingPath); throw; diff --git a/test/Unit/DependencyManagement/DependencyManagementTests.cs b/test/Unit/DependencyManagement/DependencyManagementTests.cs index 4484692a..80f1a22b 100644 --- a/test/Unit/DependencyManagement/DependencyManagementTests.cs +++ b/test/Unit/DependencyManagement/DependencyManagementTests.cs @@ -352,7 +352,7 @@ public void TestManagedDependencyRetryLogicMaxNumberOfTries() Assert.Contains(currentAttempt, relevantLogs[index]); } - Assert.Matches("Warning: Failed to install dependencies into '(.+?)', removing the folder", relevantLogs[4]); + Assert.Matches("Warning: Failed to install dependencies into '(.+?)'", relevantLogs[4]); // Lastly, DependencyError should get set after unsuccessfully retrying 3 times. Assert.NotNull(dependencyError); From 0e1668148eb2e7f544798fdfc9198c45ad6dc524 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Wed, 9 Oct 2019 18:03:38 -0700 Subject: [PATCH 5/5] Add installation mode to PromotedDependencySnapshot --- .../DependencySnapshotInstaller.cs | 20 +++++++++++++------ src/resources/PowerShellWorkerStrings.resx | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/DependencyManagement/DependencySnapshotInstaller.cs b/src/DependencyManagement/DependencySnapshotInstaller.cs index 7e31aab4..d8af575d 100644 --- a/src/DependencyManagement/DependencySnapshotInstaller.cs +++ b/src/DependencyManagement/DependencySnapshotInstaller.cs @@ -68,7 +68,7 @@ public void InstallSnapshot( // It is ok to do that during background upgrade because the current // worker already has a good enough snapshot, and nothing depends on // the new snapshot yet. - PromoteToInstalledOrRemove(installingPath, targetPath, logger); + PromoteToInstalledOrRemove(installingPath, targetPath, installationMode, logger); break; case DependencySnapshotInstallationMode.Required: @@ -78,7 +78,7 @@ public void InstallSnapshot( // As opposed to the background upgrade case, this snapshot is *required* for // this worker to run, even though it occupies some space (until the workers // restart and the redundant snapshots are purged). - PromoteToInstalled(installingPath, targetPath, logger); + PromoteToInstalled(installingPath, targetPath, installationMode, logger); break; default: @@ -153,7 +153,11 @@ private void InstallModule(DependencyInfo module, string installingPath, PowerSh } } - private void PromoteToInstalledOrRemove(string installingPath, string installedPath, ILogger logger) + private void PromoteToInstalledOrRemove( + string installingPath, + string installedPath, + DependencySnapshotInstallationMode installationMode, + ILogger logger) { var latestSnapshot = _storage.GetLatestInstalledSnapshot(); if (latestSnapshot != null && _snapshotComparer.AreEquivalent(installingPath, latestSnapshot, logger)) @@ -171,18 +175,22 @@ private void PromoteToInstalledOrRemove(string installingPath, string installedP } else { - PromoteToInstalled(installingPath, installedPath, logger); + PromoteToInstalled(installingPath, installedPath, installationMode, logger); } } - private void PromoteToInstalled(string installingPath, string installedPath, ILogger logger) + private void PromoteToInstalled( + string installingPath, + string installedPath, + DependencySnapshotInstallationMode installationMode, + ILogger logger) { _storage.PromoteInstallingSnapshotToInstalledAtomically(installedPath); logger.Log( isUserOnlyLog: false, LogLevel.Trace, - string.Format(PowerShellWorkerStrings.PromotedDependencySnapshot, installingPath, installedPath)); + string.Format(PowerShellWorkerStrings.PromotedDependencySnapshot, installingPath, installedPath, installationMode)); _snapshotContentLogger.LogDependencySnapshotContent(installedPath, logger); } diff --git a/src/resources/PowerShellWorkerStrings.resx b/src/resources/PowerShellWorkerStrings.resx index 1d55a656..1e9b737a 100644 --- a/src/resources/PowerShellWorkerStrings.resx +++ b/src/resources/PowerShellWorkerStrings.resx @@ -269,7 +269,7 @@ Removing dependency snapshot '{0}' because it is equivalent to '{1}'. - Promoted dependency snapshot '{0}' to '{1}'. + Promoted dependency snapshot '{0}' to '{1}' (installation mode: {2}). Looking for a dependency snapshot newer than {0}.