don't try to copy code/config/data folder if it does not exist #6380
Conversation
| @@ -148,6 +148,10 @@ function Copy-DiffPackage | |||
| $localPkgPath += ".zip" | |||
| $diffPkgPath += ".zip" | |||
| } | |||
| elseif (!(Test-Path -Path ($localPkgPath))) | |||
There was a problem hiding this comment.
Nit-pick: ($localPkgPath) could just be $localPkgPath
|
Additional non-blocking Nit-Pick: a comment around why we are continuing would be nice. |
| @@ -148,6 +148,11 @@ function Copy-DiffPackage | |||
| $localPkgPath += ".zip" | |||
| $diffPkgPath += ".zip" | |||
| } | |||
| # The Code package for containerized service does not exist, but we want to continue the deployment | |||
| elseif (!(Test-Path -Path $localPkgPath)) | |||
There was a problem hiding this comment.
This ignores the scenario when package does not exist when service is non-containerized. Do we want to not throw in such case?
This should also be fine as we do a validation downstream using Test-ServiceFabricApplicationPackage - which anyways would fail
However, would it be better to move Test-ServiceFabricApplicationPackage before doing diff'ing as well. This will ensure that we do have a valid package before diffing - giving us more confidence that diffing has not invalidated the package.
There was a problem hiding this comment.
@bishal-pdMSFT Added Test-ServiceFabricApplicationPackage before creating diff package in deploy.ps1
There was a problem hiding this comment.
@bishal-pdMSFT Do you have any other concern on this PR?
| @@ -130,6 +130,8 @@ Register-Mock Get-ServiceFabricServiceManifest {$serviceManifest3} -- -Applicati | |||
| Register-Mock Get-ServiceFabricServiceManifest {$serviceManifest4} -- -ApplicationTypeName $applicationTypeName -ApplicationTypeVersion $applicationTypeVersion -ServiceManifestName "Stateless4Pkg" | |||
|
|
|||
| Register-Mock Copy-Item {} $appManifestPath $appManifestDiffPath -Force | |||
| Register-Mock Test-Path { $true } -Path $codePkg1 | |||
There was a problem hiding this comment.
Add a case for false also
Tasks/ServiceFabricDeploy/deploy.ps1
Outdated
| @@ -102,8 +102,15 @@ try { | |||
| $useDiffPackage = [System.Boolean]::Parse((Get-VstsInput -Name useDiffPackage)) | |||
| if ($useDiffPackage) | |||
| { | |||
| Import-Module "$PSScriptRoot\Create-DiffPackage.psm1" | |||
| $diffPackagePath = Create-DiffPackage -ApplicationName $applicationName -ApplicationPackagePath $applicationPackagePath -ConnectedServiceEndpoint $connectedServiceEndpoint -ClusterConnectionParameters $clusterConnectionParameters | |||
| if (Test-ServiceFabricApplicationPackage -ApplicationPackagePath $applicationPackagePath) | |||
There was a problem hiding this comment.
We should ignore Test-ServiceFabricApplicationPackage if skipValidation is set to true
Tasks/ServiceFabricDeploy/deploy.ps1
Outdated
| } | ||
| else | ||
| { | ||
| Write-Host (Get-VstsLocString -Key DIFFPKG_TestAppPkgFailed) |
There was a problem hiding this comment.
This should be a warning
Tasks/ServiceFabricDeploy/deploy.ps1
Outdated
| { | ||
| $isPackageValid = Test-ServiceFabricApplicationPackage -ApplicationPackagePath $applicationPackagePath | ||
| } | ||
| if ($isPackageValid) |
There was a problem hiding this comment.
Put an empty line before starting if block
No description provided.