[Service fabric] Retry for remaining commands #7607
Conversation
|
Ping |
|
|
||
| # If upgrade already started, don't retry | ||
| $upgradeStatus = Get-ServiceFabricApplicationUpgradeAction -ApplicationName $($UpgradeParameters["ApplicationName"]) | ||
| if ($upgradeStatus -and ($upgradeStatus.UpgradeState -ne "RollingBackCompleted" -and $upgradeStatus.UpgradeState -ne "RollingForwardCompleted")) |
There was a problem hiding this comment.
You dont need to put braces for the 2nd expression. However, I m not clear on the logic here
There was a problem hiding this comment.
it is checking if upgrade is already in progress (status should not be RollingBackCompleted or RollingForwardCompleted). if so, it will not retry starting upgrade.
There was a problem hiding this comment.
But you are returning false in those cases, wouldn't that stop the retry?
There was a problem hiding this comment.
yes, that is intended. The action being tried here is Start-Upgrade.. If upgrade has already started even though the call failed due to timeout, we would not like to try further.
Only in case of upgrade not started, we want to retry
There was a problem hiding this comment.
How do we differentiate between the last upgrade vs the current upgrade
There was a problem hiding this comment.
if current is first upgrade for application, upgrade status will be null, otherwise it will be RollingForwardCompleted or RollingBackCompleted depending upon whether last upgrade was success or rolled back
There was a problem hiding this comment.
this is the same logic which we use while waiting for upgrade to finish
There was a problem hiding this comment.
Is there a chance for us to not even attempt retry if the server didn't even accept our retry request.
There was a problem hiding this comment.
There is nothing like server will not accept request. It will accept and either fail or proceed with upgrade. In either of these cases we don't want to retry.
| ) | ||
|
|
||
| $global:operationId = $SF_Operations.RemoveApplicationPackage | ||
| $removeAction = { Remove-ServiceFabricApplicationPackage -ApplicationPackagePathInImageStore $applicationPackagePathInImageStore -ImageStoreConnectionString $ImageStoreConnectionString } |
There was a problem hiding this comment.
the casing in $application starts with lower case
| psr.run(path.join(__dirname, 'UnregisterApplicationTypeShouldRetry.ps1'), done); | ||
| }) | ||
| it('Create application type should retry till success', (done) => { | ||
| psr.run(path.join(__dirname, 'CreateApplicationShouldRetryTillSuccess.ps1'), done); |
There was a problem hiding this comment.
Yes, they were part of previous PR. However, during conflict resolution, these changes were reverted in that PR.
# Conflicts: # Tasks/ServiceFabricDeployV1/ServiceFabricSDK/Publish-UpgradedServiceFabricApplication.ps1
* Retry for remaining commands * PR comment
* Retry for remaining commands * PR comment
* [ServiceFabric] Re-tries for Unregister, connect, create and remove commands (#7588) * Re-tries for copy and register commands * PR comments * PR comments and few suggestion as per Oana * connect, unregister, create, remove re-tries * L0 for unregister retry * L0 for create and remove application * Utilities changes which were not picked in merge * PR comments * Moving retry logic for UpgradeApplication to corresponding helper fun… (#7593) * Moving retry logic for UpgradeApplication to corresponding helper function * Updated function name * Added new operation * Indentation * Adding timeout exception * [Service fabric] Retry for remaining commands (#7607) * Retry for remaining commands * PR comment * Adding logs for upgrade service fabric application. (#7217) * Adding logs for upgrade service fabric application. * resolving comments * Forming upgrade errors message. * Formatting unhealthy evaluation and upgrade status. * fixed merge changes * Formatting UnhealthyEvaluation and DomainUpgradeStatus while printing only conditionally. * Moved waiting for upgrade to complete and printing logic to function in Utilities file. * Keeping other exception as well. * 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 * Users/hiyada/s ftask log bug fixes (#7722) * Corrected typo. * Corrected retry message null check * Addressed bugs raised as a part of bugbash (#7739) * Added powershell helper module * Replacing Trace-VstsEnteringInvocation with print statement * Removing Trace-VstsLeavingInvocation $MyInvocation * Addressed review comments * Updated patch version (#7748)
No description provided.