Remove partially installed snapshots (#290)

Remove partially installed snapshots on installation failures

Author: AnatoliB

src/DependencyManagement/DependencySnapshotInstaller.cs

@@ -84,6 +84,13 @@ public void InstallSnapshot(
 
                 _storage.PromoteInstallingSnapshotToInstalledAtomically(targetPath);
             }
+            catch (Exception e)
+            {
+                var message = string.Format(PowerShellWorkerStrings.FailedToInstallDependenciesSnapshot, targetPath);
+                logger.Log(isUserOnlyLog: false, LogLevel.Warning, message, e);
+                _storage.RemoveSnapshot(installingPath);
+                throw;
+            }
             finally
             {
                 _moduleProvider.Cleanup(pwsh);

src/resources/PowerShellWorkerStrings.resx

@@ -256,4 +256,7 @@
   <data name="UpdatingManagedDependencySnapshotHeartbeat" xml:space="preserve">
     <value>Updating dependencies folder heartbeat for '{0}''.</value>
   </data>
+  <data name="FailedToInstallDependenciesSnapshot" xml:space="preserve">
+    <value>Failed to install dependencies into '{0}', removing the folder.</value>
+  </data>
 </root>

test/Unit/DependencyManagement/DependencyManagementTests.cs

@@ -336,24 +336,28 @@ public void TestManagedDependencyRetryLogicMaxNumberOfTries()
 
                     var relevantLogs = _testLogger.FullLog.Where(
                         message => message.StartsWith("Error: Fail to install module")
-                                   || message.StartsWith("Trace: Installing FunctionApp dependent modules")).ToList();
+                                   || message.StartsWith("Trace: Installing FunctionApp dependent modules")
+                                   || message.StartsWith("Warning: Failed to install dependencies")).ToList();
 
-                    // Here we will get four logs: one that says that we are installing the
-                    // dependencies, and three for failing to install the module.
-                    Assert.Equal(4, relevantLogs.Count);
+                    // Here we will get five logs: one that says that we are installing the
+                    // dependencies, three for failing to install the module,
+                    // and one warning notifying of removing the dependencies folder.
+                    Assert.Equal(5, relevantLogs.Count);
 
                     // The first log should say "Installing FunctionApp dependent modules."
                     Assert.Contains(PowerShellWorkerStrings.InstallingFunctionAppDependentModules, relevantLogs[0]);
 
                     // The subsequent logs should contain the following:
-                    for (int index = 1; index < relevantLogs.Count; index++)
+                    for (int index = 1; index < 4; index++)
                     {
                         Assert.Contains("Fail to install module", relevantLogs[index]);
                         var currentAttempt = DependencySnapshotInstaller.GetCurrentAttemptMessage(index);
                         Assert.Contains(currentAttempt, relevantLogs[index]);
                     }
 
-                    // Lastly, DependencyError should get set after unsuccessfully  retyring 3 times.
+                    Assert.Matches("Warning: Failed to install dependencies into '(.+?)', removing the folder", relevantLogs[4]);
+
+                    // Lastly, DependencyError should get set after unsuccessfully retrying 3 times.
                     Assert.NotNull(dependencyError);
                     Assert.Contains("Fail to install FunctionApp dependencies. Error:", dependencyError.Message);
                 }

test/Unit/DependencyManagement/DependencySnapshotInstallerTests.cs

@@ -109,6 +109,8 @@ public void DoesNotSaveModuleIfGetLatestPublishedModuleVersionThrows()
                     _ => _.GetLatestPublishedModuleVersion(It.IsAny<string>(), It.IsAny<string>()))
                 .Throws(injectedException);
 
+            _mockStorage.Setup(_ => _.RemoveSnapshot(_targetPathInstalling));
+
             _mockModuleProvider.Setup(_ => _.Cleanup(dummyPowerShell));
 
             // Act
@@ -126,6 +128,7 @@ public void DoesNotSaveModuleIfGetLatestPublishedModuleVersionThrows()
                 Times.Never);
 
             _mockStorage.Verify(_ => _.PromoteInstallingSnapshotToInstalledAtomically(It.IsAny<string>()), Times.Never);
+            _mockStorage.Verify(_ => _.RemoveSnapshot(_targetPathInstalling));
             _mockModuleProvider.Verify(_ => _.Cleanup(dummyPowerShell), Times.Once);
         }
 
@@ -151,6 +154,8 @@ public void DoesNotPromoteDependenciesSnapshotIfSaveModuleKeepsThrowing()
                 _ => _.SaveModule(It.IsAny<PowerShell>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()))
                 .Throws(injectedException);
 
+            _mockStorage.Setup(_ => _.RemoveSnapshot(_targetPathInstalling));
+
             _mockModuleProvider.Setup(_ => _.Cleanup(dummyPowerShell));
 
             // Act
@@ -164,6 +169,7 @@ public void DoesNotPromoteDependenciesSnapshotIfSaveModuleKeepsThrowing()
             Assert.Contains(injectedException.Message, thrownException.Message);
 
             _mockStorage.Verify(_ => _.PromoteInstallingSnapshotToInstalledAtomically(It.IsAny<string>()), Times.Never);
+            _mockStorage.Verify(_ => _.RemoveSnapshot(_targetPathInstalling));
             _mockModuleProvider.Verify(_ => _.Cleanup(dummyPowerShell), Times.Once);
         }