Implemented async operations (#7631)

* Implemented Async operations * Added null checks * Modified code as per review comments * typo * Revert "typo" This reverts commit f04d14e. * Revert "Modified code as per review comments" This reverts commit e24bc53. * Modified code as per review comments * Modified code as per review comments * Addressed review comments * Added retry logic to Wait-ServiceFabricApplicationTypeRegistrationStatus * Added comments * Addressed review comments * typo * Added comments * typo * Addressed review comments * Updated test cases * Modified code as per review comments
microsoft · Jul 17, 2018 · 9cfd15e · 9cfd15e
1 parent 12d107f
commit 9cfd15e
Show file tree

Hide file tree

Showing 7 changed files with 155 additions and 243 deletions.
diff --git a/Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Publish-NewServiceFabricApplication.ps1 b/Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Publish-NewServiceFabricApplication.ps1
@@ -292,14 +292,8 @@
                 'ApplicationPathInImageStore' = $applicationPackagePathInImageStore
             }
 
-            if ($RegisterPackageTimeoutSec)
-            {
-                $registerParameters['TimeOutSec'] = $RegisterPackageTimeoutSec
-            }
-
             Write-Host (Get-VstsLocString -Key SFSDK_RegisterAppType)
-            Register-ServiceFabricApplicationTypeAction -RegisterParameters $registerParameters -ApplicationTypeName $names.ApplicationTypeName -ApplicationTypeVersion $names.ApplicationTypeVersion
-
+            Register-ServiceFabricApplicationTypeAction -RegisterParameters $registerParameters -ApplicationTypeName $names.ApplicationTypeName -ApplicationTypeVersion $names.ApplicationTypeVersion -TimeoutSec $RegisterPackageTimeoutSec
             Write-Host (Get-VstsLocString -Key SFSDK_RemoveAppPackage)
             Remove-ServiceFabricApplicationPackageAction -ApplicationPackagePathInImageStore $applicationPackagePathInImageStore -ImageStoreConnectionString $imageStoreConnectionString
         }

diff --git a/Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Publish-UpgradedServiceFabricApplication.ps1 b/Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Publish-UpgradedServiceFabricApplication.ps1
@@ -279,13 +279,8 @@ function Publish-UpgradedServiceFabricApplication
                 'ApplicationPathInImageStore' = $applicationPackagePathInImageStore
             }
 
-            if ($RegisterPackageTimeoutSec)
-            {
-                $registerParameters['TimeOutSec'] = $RegisterPackageTimeoutSec
-            }
-
             Write-Host (Get-VstsLocString -Key SFSDK_RegisterAppType)
-            Register-ServiceFabricApplicationTypeAction -RegisterParameters $registerParameters -ApplicationTypeName $names.ApplicationTypeName -ApplicationTypeVersion $names.ApplicationTypeVersion
+            Register-ServiceFabricApplicationTypeAction -RegisterParameters $registerParameters -ApplicationTypeName $names.ApplicationTypeName -ApplicationTypeVersion $names.ApplicationTypeVersion -TimeoutSec $RegisterPackageTimeoutSec
         }
     }
 

diff --git a/Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Utilities.ps1 b/Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Utilities.ps1
@@ -387,9 +387,14 @@ function Register-ServiceFabricApplicationTypeAction
         $ApplicationTypeName,
 
         [string]
-        $ApplicationTypeVersion
+        $ApplicationTypeVersion,
+
+        [int]
+        $TimeoutSec
     )
 
+    $RegisterParameters['Async'] = $true
+
     $global:operationId = $SF_Operations.RegisterApplicationType
     $registerAction = {
         Register-ServiceFabricApplicationType @RegisterParameters
@@ -401,50 +406,39 @@ function Register-ServiceFabricApplicationTypeAction
 
     $exceptionRetryEvaluator = {
         param($ex)
-        $appType = Get-ServiceFabricApplicationTypeAction -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
-        # If provisioning not started, retry register
-        if (!$appType)
-        {
-            Write-Host (Get-VstsLocString -Key SFSDK_ApplicationTypeProvisioningNotStarted)
-            return $true
-        }
-
-        # if provisioning started, wait for it to complete
-        if (($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Provisioning) -or ($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Unprovisioning))
-        {
-            Write-Host (Get-VstsLocString -Key SFSDK_ApplicationTypeProvisioningStarted)
-            $appType = Wait-ServiceFabricApplicationTypeTerminalStatus -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
-        }
-
-        # if app type got unprovisioned, retry register
-        if (!$appType)
-        {
-            Write-Host (Get-VstsLocString -Key SFSDK_ApplicationTypeUnprovisioned)
-            return $true
-        }
-
-        # if app type is provisioned, bail out
-        if ($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Available)
+        # If app already created, don't retry
+        if ($ex.GetType().FullName -eq "System.Fabric.FabricElementAlreadyExistsException")
         {
             return $false
         }
 
-        # if provisioning failed, throw and don't retry
-        throw (Get-VstsLocString -Key SFSDK_RegisterAppTypeFailedWithStatus -ArgumentList @($appType.Status, $appType.StatusDetails))
+        return $true
     }
 
     try
     {
         Invoke-ActionWithDefaultRetries -Action $registerAction `
             -RetryMessage (Get-VstsLocString -Key SFSDK_RetryingRegisterApplicationType) `
-            -ExceptionRetryEvaluator $exceptionRetryEvaluator
+            -ExceptionRetryEvaluator $exceptionRetryEvaluator `
+            -RetryableExceptions @("System.Fabric.FabricTransientException", "System.Fabric.FabricElementAlreadyExistsException", "System.TimeoutException")
     }
     catch
     {
+        try
+        {
+            #In case of any failure we need to keep the cluster clean as much as possible
+            Unregister-ServiceFabricApplicationTypeAction -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion -TimeoutSec $TimeoutSec
+        }
+        catch
+        {
+            #This is just for best effort, else no need to take any action here
+        }
         # print cluster health status if registering failed
         Trace-ServiceFabricClusterHealth
         throw
     }
+
+    Wait-ServiceFabricApplicationTypeRegistrationStatus -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion -TimeoutSec $TimeoutSec
 }
 
 function Get-ServiceFabricApplicationTypeAction
@@ -506,9 +500,9 @@ function Wait-ServiceFabricApplicationTypeTerminalStatus
         -RetryMessage (Get-VstsLocString -Key SFSDK_RetryingGetApplicationType)
 }
 
-function Unregister-ServiceFabricApplicationTypeAction
+function Wait-ServiceFabricApplicationTypeRegistrationStatus
 {
-    Param(
+    Param (
         [string]
         $ApplicationTypeName,
 
@@ -519,60 +513,134 @@ function Unregister-ServiceFabricApplicationTypeAction
         $TimeoutSec
     )
 
-    $global:operationId = $SF_Operations.UnregisterApplicationType
+    $global:operationId = $SF_Operations.GetApplicationType
+    $getAppTypeAction = { Get-ServiceFabricApplicationTypeAction -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion }
+    $getAppTypeRetryEvaluator = {
+        param($appType)
 
-    $unregisterAction = {
-        Unregister-ServiceFabricApplicationType -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion -Force -TimeoutSec $TimeoutSec
-        if (!$?)
+        # If provisioning not started, retry register
+        if(!$appType)
         {
-            throw (Get-VstsLocString -Key SFSDK_UnableToUnregisterAppType)
+            return $true
         }
-    }
-
-    $exceptionRetryEvaluator = {
-        param($ex)
-        $appType = Get-ServiceFabricApplicationTypeAction -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
-        # If app type already unprovisioned, don't retry
-        if (!$appType)
+        # if app type is provisioned, don't retry
+        elseif($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Available)
         {
             return $false
         }
-
-        # if unprovisioning started, wait for it to complete
-        if (($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Provisioning) -or ($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Unprovisioning))
+        # if app type exist and if its status has not changed to a terminal one, do retry
+        elseif(($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Provisioning) -or ($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Unprovisioning))
         {
-            Write-Host (Get-VstsLocString -Key SFSDK_ApplicationTypeUnprovisioningStarted)
-            $appType = Wait-ServiceFabricApplicationTypeTerminalStatus -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
+            return $true
         }
+        # if app type exist and if its status has changed to a terminal one, throw
+        elseif(($appType.Status -ne [System.Fabric.Query.ApplicationTypeStatus]::Provisioning) -and ($appType.Status -ne [System.Fabric.Query.ApplicationTypeStatus]::Unprovisioning))
+        {
+            throw (Get-VstsLocString -Key SFSDK_RegisterAppTypeFailedWithStatus -ArgumentList @($appType.Status, $appType.StatusDetails))
+        }
+    }
 
-        # if app type got unprovisioned, don't retry
-        if (!$appType)
+    $MaxTries = 1200
+    $RetryIntervalInSeconds = 3
+    if($TimeoutSec)
+    {
+        $MaxTries = [int]($TimeoutSec/$RetryIntervalInSeconds)
+    }
+
+    return Invoke-ActionWithRetries -Action $getAppTypeAction `
+        -ResultRetryEvaluator $getAppTypeRetryEvaluator `
+        -MaxTries $MaxTries `
+        -RetryIntervalInSeconds $RetryIntervalInSeconds `
+        -RetryableExceptions @("System.Fabric.FabricTransientException", "System.TimeoutException") `
+        -RetryMessage (Get-VstsLocString -Key SFSDK_RetryingGetApplicationType)
+}
+
+function Wait-ServiceFabricApplicationTypeUnregistrationStatus
+{
+    Param (
+        [string]
+        $ApplicationTypeName,
+
+        [string]
+        $ApplicationTypeVersion,
+
+        [int]
+        $TimeoutSec
+    )
+
+    $global:operationId = $SF_Operations.GetApplicationType
+    $getAppTypeAction = { Get-ServiceFabricApplicationTypeAction -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion }
+    $getAppTypeRetryEvaluator = {
+        param($appType)
+
+        # If app type unprovisioned, don't retry
+        if(!$appType)
         {
             return $false
         }
-
-        # if app type is still provisioned, retry unregister
-        if ($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Available)
+        # if app type exist and if its status has not changed to a terminal one, do retry
+        elseif(($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Provisioning) -or ($appType.Status -eq [System.Fabric.Query.ApplicationTypeStatus]::Unprovisioning))
         {
             return $true
         }
+        # if app type exist and if its status has changed to a terminal one, throw
+        elseif(($appType.Status -ne [System.Fabric.Query.ApplicationTypeStatus]::Provisioning) -and ($appType.Status -ne [System.Fabric.Query.ApplicationTypeStatus]::Unprovisioning))
+        {
+            throw (Get-VstsLocString -Key SFSDK_RegisterAppTypeFailedWithStatus -ArgumentList @($appType.Status, $appType.StatusDetails))
+        }
+    }
 
-        # if unprovisioning failed, throw and don't retry
-        throw (Get-VstsLocString -Key SFSDK_UnregisterAppTypeFailedWithStatus -ArgumentList @($appType.Status, $appType.StatusDetails))
+    $MaxTries = 1200
+    $RetryIntervalInSeconds = 3
+    if($TimeoutSec)
+    {
+        $MaxTries = [int]($TimeoutSec/$RetryIntervalInSeconds)
+    }
+
+    return Invoke-ActionWithRetries -Action $getAppTypeAction `
+        -ResultRetryEvaluator $getAppTypeRetryEvaluator `
+        -MaxTries $MaxTries `
+        -RetryIntervalInSeconds $RetryIntervalInSeconds `
+        -RetryableExceptions @("System.Fabric.FabricTransientException", "System.TimeoutException") `
+        -RetryMessage (Get-VstsLocString -Key SFSDK_RetryingGetApplicationType)
+}
+
+function Unregister-ServiceFabricApplicationTypeAction
+{
+    Param(
+        [string]
+        $ApplicationTypeName,
+
+        [string]
+        $ApplicationTypeVersion,
+
+        [int]
+        $TimeoutSec
+    )
+
+    $global:operationId = $SF_Operations.UnregisterApplicationType
+
+    $unregisterAction = {
+        Unregister-ServiceFabricApplicationType -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion -Async -Force
+        if (!$?)
+        {
+            throw (Get-VstsLocString -Key SFSDK_UnableToUnregisterAppType)
+        }
     }
 
     try
     {
         Invoke-ActionWithDefaultRetries -Action $unregisterAction `
-            -RetryMessage (Get-VstsLocString -Key SFSDK_RetryingUnregisterApplicationType) `
-            -ExceptionRetryEvaluator $exceptionRetryEvaluator
+            -RetryMessage (Get-VstsLocString -Key SFSDK_RetryingUnregisterApplicationType)
     }
     catch
     {
         # print cluster health status if unregistering failed
         Trace-ServiceFabricClusterHealth
         throw
     }
+
+    Wait-ServiceFabricApplicationTypeUnregistrationStatus -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion -TimeoutSec $TimeoutSec
 }
 
 function Remove-ServiceFabricApplicationAction
@@ -636,7 +704,7 @@ function New-ServiceFabricApplicationAction
     $exceptionRetryEvaluator = {
         param($ex)
 
-        # If app already creted, don't retry
+        # If app already created, don't retry
         if ($ex.GetType().FullName -eq "System.Fabric.FabricElementAlreadyExistsException")
         {
             return $false

diff --git a/Tasks/ServiceFabricDeployV1/Tests/RegisterApplicationTypeShouldRetry.ps1 b/Tasks/ServiceFabricDeployV1/Tests/RegisterApplicationTypeShouldRetry.ps1
@@ -7,63 +7,28 @@ Import-Module ServiceFabric
 $ApplicationTypeName = "app type name"
 $ApplicationTypeVersion = "app type version"
 $applicationPackagePathInImageStore = "image path"
-$applicationType = @{
-    ApplicationTypeName    = $ApplicationTypeName
-    ApplicationTypeVersion = $ApplicationTypeVersion
-    Status                 = [System.Fabric.Query.ApplicationTypeStatus]::Provisioning
-}
 $RegisterParameters = @{
-    'ApplicationPathInImageStore' = $applicationPackagePathInImageStore
+    'ApplicationPathInImageStore' = $applicationPackagePathInImageStore;
+    'Async' = $true
 }
-$getApplicationTypeParams = @{
+$waitApplicationTypeParams = @{
     'ApplicationTypeName'    = $ApplicationTypeName;
     'ApplicationTypeVersion' = $ApplicationTypeVersion
 }
 $global:registerRetriesAttempted = 0
-$global:getRetriesAttempted = 0
-$global:clusterHealthPrinted = $false
 
 Register-Mock Register-ServiceFabricApplicationType {
     $global:registerRetriesAttempted++
-    $global:getRetriesAttempted = 0
-    throw [System.Fabric.FabricTransientException]::new("Could not ping!")
-} -- @RegisterParameters
-
-Register-Mock Get-ServiceFabricApplicationType {
-    $global:getRetriesAttempted++
-    if ($global:getRetriesAttempted -eq 3)
-    {
-        $applicationType.Status = [System.Fabric.Query.ApplicationTypeStatus]::Provisioning
-        return $applicationType
-    }
-
-    if ($global:getRetriesAttempted -eq 2)
+    if ($global:registerRetriesAttempted -eq 3)
     {
-        return $null
+        throw [System.Fabric.FabricElementAlreadyExistsException]::new("AppType already exists")
     }
-
     throw [System.Fabric.FabricTransientException]::new("Could not ping!")
-} -- @getApplicationTypeParams
-
-Register-Mock Get-ServiceFabricApplicationType {
-    $global:waitRetriesAttempted++
-    if ($global:waitRetriesAttempted -eq 4)
-    {
-        return $null
-    }
-
-    if ($global:waitRetriesAttempted -eq 2)
-    {
-        $applicationType.Status = [System.Fabric.Query.ApplicationTypeStatus]::Provisioning
-        return $applicationType
-    }
-
-    throw [System.Fabric.FabricTransientException]::new("Could not ping!")
-} -- -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
+} -- @RegisterParameters
 
-Register-Mock Get-ServiceFabricClusterHealth {
-    $global:clusterHealthPrinted = $true
-}
+Register-Mock Wait-ServiceFabricApplicationTypeRegistrationStatus {
+    return
+} -- @waitApplicationTypeParams
 
 Register-Mock Start-Sleep {}
 Register-Mock Write-VstsTaskError
@@ -73,8 +38,5 @@ Register-Mock Write-VstsTaskError
 . $PSScriptRoot\..\..\..\Tasks\ServiceFabricDeployV1\ServiceFabricSDK\Utilities.ps1
 
 # Act/Assert
-Assert-Throws {
-    Register-ServiceFabricApplicationTypeAction -RegisterParameters $RegisterParameters -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
-}
-Assert-AreEqual 3 $global:registerRetriesAttempted "Number of register retries not correct"
-Assert-AreEqual $true $global:clusterHealthPrinted "cluster health not printed in case of error"
+Register-ServiceFabricApplicationTypeAction -RegisterParameters $RegisterParameters -ApplicationTypeName $ApplicationTypeName -ApplicationTypeVersion $ApplicationTypeVersion
+Assert-AreEqual 3 $global:registerRetriesAttempted "Number of register retries not correct"